minimize no-repo actions

repokid/exceptions.py

@@ -61,3 +61,7 @@ class NotFoundError(Exception):
 
 class DynamoDBMaxItemSizeError(Exception):
     pass
+
+
+class IAMActionError(Exception):
+    pass

repokid/role.py

@@ -16,6 +16,7 @@
 
 import copy
 import datetime
+import fnmatch
 import logging
 import time
 from typing import Any
@@ -41,6 +42,7 @@
 from repokid.datasource.iam import IAMDatasource
 from repokid.exceptions import DynamoDBError
 from repokid.exceptions import DynamoDBMaxItemSizeError
+from repokid.exceptions import IAMActionError
 from repokid.exceptions import IAMError
 from repokid.exceptions import IntegrityError
 from repokid.exceptions import MissingRepoableServices
@@ -249,9 +251,19 @@ def _calculate_no_repo_permissions(self) -> None:
         except IndexError:
             previous_policy = {}
         new_policy = self.policies[-1]
-        newly_added_permissions = find_newly_added_permissions(
-            previous_policy.get("Policy", {}), new_policy.get("Policy", {})
-        )
+        try:
+            newly_added_permissions = find_newly_added_permissions(
+                previous_policy.get("Policy", {}),
+                new_policy.get("Policy", {}),
+                minimize=True,
+            )
+        except IAMActionError:
+            logger.error(
+                "failed to calculate no-repo permissions for %s",
+                self.arn,
+                exc_info=True,
+            )
+            return
         current_time = int(time.time())
 
         # iterate through a copy of self.no_repo_permissions and remove expired items from
@@ -261,8 +273,10 @@ def _calculate_no_repo_permissions(self) -> None:
                 self.no_repo_permissions.pop(permission)
 
         expire_time = current_time + self._no_repo_secs
+        existing_no_repo = self.no_repo_permissions.keys()
         for permission in newly_added_permissions:
-            self.no_repo_permissions[permission] = expire_time
+            if not fnmatch.filter(existing_no_repo, permission):
+                self.no_repo_permissions[permission] = expire_time
 
     def get_repoed_policy(
         self, scheduled: bool = False
@@ -407,6 +421,15 @@ def store(self, fields: Optional[List[str]] = None) -> None:
                 and self.last_updated
                 and remote_role_data.last_updated > self.last_updated
             ):
+                # Fetch the rest of the role data for debugging
+                remote_role_data.fetch()
+                logger.debug(
+                    "role has been updated since last fetch",
+                    extra={
+                        "stored_role": remote_role_data.dict(),
+                        "current_role": self.dict(),
+                    },
+                )
                 raise IntegrityError("stored role has been updated since last fetch")
         except RoleNotFoundError:
             create = True

repokid/utils/permissions.py

@@ -1,5 +1,6 @@
 import copy
 import datetime
+import fnmatch
 import logging
 import time
 from collections import defaultdict
@@ -12,9 +13,11 @@
 from typing import Tuple
 
 from policyuniverse import all_permissions
-from policyuniverse import expand_policy
-from policyuniverse import get_actions_from_statement
+from policyuniverse.expander_minimizer import expand_policy
+from policyuniverse.expander_minimizer import get_actions_from_statement
+from policyuniverse.expander_minimizer import minimize_statement_actions
 
+from repokid.exceptions import IAMActionError
 from repokid.hooks import call_hooks
 from repokid.types import RepokidHooks
 
@@ -37,7 +40,7 @@ def __repr__(self) -> str:
 
 
 def find_newly_added_permissions(
-    old_policy: Dict[str, Any], new_policy: Dict[str, Any]
+    old_policy: Dict[str, Any], new_policy: Dict[str, Any], minimize: bool = True
 ) -> Set[str]:
     """
     Compare and old version of policies to a new version and return a set of permissions that were added.  This will
@@ -46,13 +49,17 @@ def find_newly_added_permissions(
     Args:
         old_policy
         new_policy
+        minimize
 
     Returns:
         set: Exapnded set of permissions that are in the new policy and not the old one
     """
     old_permissions, _ = get_permissions_in_policy(old_policy)
     new_permissions, _ = get_permissions_in_policy(new_policy)
-    return new_permissions - old_permissions
+    newly_added = new_permissions - old_permissions
+    if minimize:
+        newly_added = _minimize_actions(newly_added)
+    return newly_added
 
 
 def convert_repoable_perms_to_perms_and_services(
@@ -213,18 +220,15 @@ def get_permissions_in_policy(
         policy = expand_policy(policy=policy, expand_deny=False) if policy else {}
         for statement in policy.get("Statement"):
             if statement["Effect"].lower() == "allow":
-                total_permissions = total_permissions.union(
-                    get_actions_from_statement(statement)
-                )
+                statement_actions = get_actions_from_statement(statement)
+                total_permissions = total_permissions.union(statement_actions)
                 if not (
                     "Sid" in statement
                     and statement["Sid"].startswith(STATEMENT_SKIP_SID)
                 ):
                     # No Sid
                     # Sid exists, but doesn't start with STATEMENT_SKIP_SID
-                    eligible_permissions = eligible_permissions.union(
-                        get_actions_from_statement(statement)
-                    )
+                    eligible_permissions = eligible_permissions.union(statement_actions)
 
     weird_permissions = total_permissions.difference(all_permissions)
     if weird_permissions and warn_unknown_perms:
@@ -233,6 +237,15 @@ def get_permissions_in_policy(
     return total_permissions, eligible_permissions
 
 
+def _minimize_actions(actions: Set[str]) -> Set[str]:
+    temp_statement = {"Effect": "Allow", "Action": list(actions)}
+    try:
+        minimized_actions = set(minimize_statement_actions(temp_statement))
+    except Exception as e:
+        raise IAMActionError("could not minimize actions") from e
+    return minimized_actions
+
+
 def _get_potentially_repoable_permissions(
     role_name: str,
     account_number: str,
@@ -256,7 +269,7 @@ def _get_potentially_repoable_permissions(
     potentially_repoable_permissions = {
         permission: RepoablePermissionDecision()
         for permission in permissions
-        if permission not in no_repo_list
+        if not fnmatch.filter(no_repo_list, permission)
     }
 
     used_services = set()

repokid/utils/roledata.py

@@ -20,6 +20,7 @@
 from typing import Tuple
 
 import repokid.hooks
+from repokid.role import Role
 from repokid.role import RoleList
 from repokid.types import RepokidHooks
 from repokid.utils.dynamo import get_all_role_ids_for_account
@@ -41,7 +42,9 @@ def find_and_mark_inactive(account_number: str, active_roles: RoleList) -> None:
         None
     """
     known_roles = set(get_all_role_ids_for_account(account_number))
-    inactive_roles = {role for role in active_roles if role.role_id not in known_roles}
+    inactive_roles: Set[Role] = {
+        role for role in active_roles if role.role_id not in known_roles
+    }
 
     for role in inactive_roles:
         if role.active:

tests/test_roledata.py

@@ -51,10 +51,19 @@ def test_find_newly_added_permissions(self):
         new_policy = ROLE_POLICIES["unused_ec2"]
 
         new_perms = repokid.utils.permissions.find_newly_added_permissions(
-            old_policy, new_policy
+            old_policy, new_policy, minimize=False
         )
         assert new_perms == {"ec2:allocatehosts", "ec2:associateaddress"}
 
+    def test_find_newly_added_permissions_minimize(self):
+        old_policy = ROLE_POLICIES["all_services_used"]
+        new_policy = ROLE_POLICIES["unused_ec2"]
+
+        new_perms = repokid.utils.permissions.find_newly_added_permissions(
+            old_policy, new_policy, minimize=True
+        )
+        assert new_perms == {"ec2:allocateh*", "ec2:associatea*"}
+
     def test_convert_repoable_perms_to_perms_and_services(self):
         all_perms = {"a:j", "a:k", "b:l", "c:m", "c:n"}
         repoable_perms = {"b:l", "c:m"}