From e4956425aeb8ddb295ac897ce4f0d8ad5f32709f Mon Sep 17 00:00:00 2001
From: Tanmay Satam <tsatam@redhat.com>
Date: Fri, 9 Feb 2024 09:58:44 -0500
Subject: [PATCH] refactor can_do_action to return boolean and shift error
 reporting to validate_resource

---
 .../az/aro/azext_aro/_dynamic_validators.py   |  9 ++++----
 .../latest/unit/test_dynamic_validators.py    | 22 +++++++++----------
 2 files changed, 15 insertions(+), 16 deletions(-)

diff --git a/python/az/aro/azext_aro/_dynamic_validators.py b/python/az/aro/azext_aro/_dynamic_validators.py
index 3494c1b0864..4625961ad09 100644
--- 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
index 8e5b64abe0d..c7373f4c6b8 100644
--- 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,7 +28,7 @@
             Permission(actions=["Microsoft.Network/virtualNetworks/subnets/join/action"], not_actions=[]),
         ],
         "Microsoft.Network/virtualNetworks/subnets/join/action",
-        None
+        True
     ),
     (
         "has permission - wildcard",
@@ -36,7 +36,7 @@
             Permission(actions=["Microsoft.Network/virtualNetworks/subnets/*/action"], not_actions=[]),
         ],
         "Microsoft.Network/virtualNetworks/subnets/join/action",
-        None
+        True
     ),
     (
         "has permission - exact, conflict",
@@ -45,7 +45,7 @@
             Permission(actions=["Microsoft.Network/virtualNetworks/subnets/join/action"], not_actions=[]),
         ],
         "Microsoft.Network/virtualNetworks/subnets/join/action",
-        None
+        True
     ),
     (
         "has permission excluded - exact",
@@ -53,7 +53,7 @@
             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",
@@ -61,23 +61,23 @@
             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 = [