refactor can_do_action to return boolean and shift error reporting to…

… validate_resource
Azure · Feb 9, 2024 · e495642 · e495642
1 parent 43f4ed2
commit e495642
Show file tree

Hide file tree

Showing 2 changed files with 15 additions and 16 deletions.
diff --git a/python/az/aro/azext_aro/_dynamic_validators.py b/python/az/aro/azext_aro/_dynamic_validators.py
@@ -47,9 +47,9 @@ def can_do_action(perms, action):
                 break
 
         if matched:
-            return None
+            return True
 
-    return f"{action} permission is missing"
+    return False
 
 
 def validate_resource(client, key, resource, actions):
@@ -62,9 +62,8 @@ def validate_resource(client, key, resource, actions):
     for action in actions:
         perms, perms_copy = tee(perms)
         perms_list = list(perms_copy)
-        error = can_do_action(perms_list, action)
-        if error is not None:
-            row = [key, resource['name'], log_entry_type["error"], error]
+        if not can_do_action(perms_list, action):
+            row = [key, resource['name'], log_entry_type["error"], f"{action} permission is missing"]
             errors.append(row)
 
     return errors

diff --git a/python/az/aro/azext_aro/tests/latest/unit/test_dynamic_validators.py b/python/az/aro/azext_aro/tests/latest/unit/test_dynamic_validators.py
@@ -19,7 +19,7 @@
         "empty permissions list",
         [],
         "Microsoft.Network/virtualNetworks/subnets/join/action",
-        "Microsoft.Network/virtualNetworks/subnets/join/action permission is missing"
+        False
     ),
     (
         "has permission - exact",
@@ -28,15 +28,15 @@
             Permission(actions=["Microsoft.Network/virtualNetworks/subnets/join/action"], not_actions=[]),
         ],
         "Microsoft.Network/virtualNetworks/subnets/join/action",
-        None
+        True
     ),
     (
         "has permission - wildcard",
         [
             Permission(actions=["Microsoft.Network/virtualNetworks/subnets/*/action"], not_actions=[]),
         ],
         "Microsoft.Network/virtualNetworks/subnets/join/action",
-        None
+        True
     ),
     (
         "has permission - exact, conflict",
@@ -45,39 +45,39 @@
             Permission(actions=["Microsoft.Network/virtualNetworks/subnets/join/action"], not_actions=[]),
         ],
         "Microsoft.Network/virtualNetworks/subnets/join/action",
-        None
+        True
     ),
     (
         "has permission excluded - exact",
         [
             Permission(actions=["Microsoft.Network/*"], not_actions=["Microsoft.Network/virtualNetworks/subnets/join/action"]),
         ],
         "Microsoft.Network/virtualNetworks/subnets/join/action",
-        "Microsoft.Network/virtualNetworks/subnets/join/action permission is missing"
+        False
     ),
     (
         "has permission excluded - wildcard",
         [
             Permission(actions=["Microsoft.Network/*"], not_actions=["Microsoft.Network/virtualNetworks/subnets/*/action"]),
         ],
         "Microsoft.Network/virtualNetworks/subnets/join/action",
-        "Microsoft.Network/virtualNetworks/subnets/join/action permission is missing"
+        False
     )
 ]
 
 
 @pytest.mark.parametrize(
-    "test_description, perms, action, expected_error",
+    "test_description, perms, action, expected",
     test_can_do_action_data,
     ids=[i[0] for i in test_can_do_action_data]
 )
 def test_can_do_action(
-    test_description, perms, action, expected_error
+    test_description, perms, action, expected
 ):
-    error = can_do_action(perms, action)
+    actual = can_do_action(perms, action)
 
-    if error != expected_error:
-        raise Exception(f"Error mismatch, expected: {expected_error}, actual: {error}")
+    if actual != expected:
+        raise Exception(f"Error mismatch, expected: {expected}, actual: {actual}")
 
 
 test_validate_cidr_data = [