Round 14 fixes: align revoke key form with create path, wildcard short-circuit

- _retained_keys_for_rule emits bare resource names (matching the
  create path's _all_resources_from_rules), not the
  permission-file 'resource/resourceName::foo' form. The perms
  configmap stores bare names from create; the old form meant
  revoke's touched_keys/retained_keys computed against a key space
  the configmap never used, so 'dropped' was always empty for
  resourceName-qualified grants and the configmap never reflected
  the revoke.
- Remove _permission_keys helper — no remaining callers after the
  above simplification.
- If retained_keys contains '*', short-circuit the configmap filter
  to preserve every current resource. A surviving rule with
  resources:['*'] grants everything the configmap lists, so nothing
  should be dropped even when specific revokes touched it.
- Update the 2 unit tests that asserted the old qualified-key form.

Author: anniegracehu

provider-kubeconfig.py

@@ -210,20 +210,19 @@ def _strip_verbs(self, existing_verbs, revoke_verbs):
                 remaining = [v for v in existing_list if v not in revoke_set]
                 return remaining or None
 
-        def _permission_keys(self, resource, resource_names):
-                """Permission-file key form used by _parse_permission_rules / the perms configmap."""
-                if resource_names:
-                        return [resource + RESOURCE_NAME_SEP + rn for rn in resource_names]
-                return [resource]
-
         def _retained_keys_for_rule(self, rule):
-                """All permission-file keys covered by an (unmodified) k8s rule."""
+                """Resource keys covered by a rule, in the form stored in the perms configmap.
+
+                The create path's _all_resources_from_rules writes bare resource names, so
+                revoke must emit the same form to diff against current_resources. URL and
+                resourceName granularity is intentionally dropped here — the configmap
+                doesn't carry it.
+                """
                 keys = []
                 for url in rule.get("nonResourceURLs") or []:
                         keys.append(NON_RESOURCE_URL_PREFIX + url)
-                resource_names = rule.get("resourceNames") or []
                 for res in rule.get("resources") or []:
-                        keys.extend(self._permission_keys(res, resource_names))
+                        keys.append(res)
                 return keys
 
         def _dim_matches(self, revoke_dim, existing_dim):
@@ -841,8 +840,12 @@ def _revoke_rbac(self, permissionfile, sa, namespace, kubeconfig):
                         self._kubectl_or_raise("delete clusterrolebinding " + rolebinding_name, kubeconfig)
 
                 current_resources = self._read_perm_configmap_resources(sa, namespace, kubeconfig)
-                dropped = touched_keys - retained_keys
-                remaining_resources = [res for res in current_resources if res not in dropped]
+                if "*" in retained_keys:
+                        # A surviving '*' rule grants everything the configmap still lists.
+                        remaining_resources = list(current_resources)
+                else:
+                        dropped = touched_keys - retained_keys
+                        remaining_resources = [res for res in current_resources if res not in dropped]
                 self._write_perm_configmap_resources(sa, namespace, kubeconfig, remaining_resources)
                 return True
 
@@ -945,10 +948,7 @@ def _split_rule_after_revoke(self, rule, matched_revoke_rules, retained_keys):
                                         updated.pop("resourceNames", None)
                                 updated["verbs"] = list(verbs_key)
                                 out_rules.append(updated)
-                                group_names = names if has_names else []
-                                for res in ress:
-                                        for key in self._permission_keys(res, group_names):
-                                                retained_keys.add(key)
+                                retained_keys.update(ress)
                         else:
                                 for (ag, res, rn) in cells:
                                         updated = dict(rule)
@@ -960,9 +960,7 @@ def _split_rule_after_revoke(self, rule, matched_revoke_rules, retained_keys):
                                                 updated.pop("resourceNames", None)
                                         updated["verbs"] = list(verbs_key)
                                         out_rules.append(updated)
-                                        cell_names = [rn] if rn is not None else []
-                                        for key in self._permission_keys(res, cell_names):
-                                                retained_keys.add(key)
+                                        retained_keys.add(res)
                 return out_rules
 
         def _atomic_rule_after_revoke(self, rule, matched_revoke_rules, retained_keys):

tests/test_provider_kubeconfig.py

@@ -198,7 +198,7 @@ def test_split_rule_resourcename_qualified_partial_revoke(self):
         result = self.generator._split_rule_after_revoke(rule, matched, retained)
         self.assertEqual(len(result), 1)
         self.assertEqual(sorted(result[0]["verbs"]), ["get"])
-        self.assertIn("configmaps/resourceName::foo", retained)
+        self.assertIn("configmaps", retained)
 
     def test_split_rule_nonresource_urls_partial_revoke(self):
         rule = {"nonResourceURLs": ["/metrics", "/healthz"], "verbs": ["get"]}
@@ -269,7 +269,7 @@ def test_split_rule_partial_resourcename_revoke(self):
         self.assertEqual(by_name[("b",)], ["delete", "get"])
         self.assertEqual(
             retained,
-            {"configmaps/resourceName::a", "configmaps/resourceName::b"},
+            {"configmaps"},
         )
 
     def test_split_rule_non_rectangular_emits_per_cell(self):