Add revoke action and YAML support for permission updates by anniegracehu · Pull Request #1461 · cloud-ark/kubeplus

anniegracehu · 2026-03-30T18:22:55Z

Summary

add revoke action to provider-kubeconfig.py
support both JSON and YAML permission files for update and revoke
keep <sa>-perms configmap synchronized when updating and revoking permissions
add tests for revoke CLI validation and permission file parsing
add manual YAML fixture for update/revoke e2e at tests/manual/update-revoke-perms.yaml

Test plan

python3 -m unittest tests/test_provider_kubeconfig.py

Manual update/revoke e2e (existing SA)

Use an existing service account (example: argocd-application-controller in argocd) and verify update/revoke without creating a new SA.

# Inputs
NS=default
SA=default
KCFG="$HOME/.kube/config"
PERMS=tests/manual/update-revoke-perms.yaml

# 1) Baseline (expected: no)
kubectl auth can-i create secrets -n "$NS" --as=system:serviceaccount:$NS:$SA --kubeconfig="$KCFG"

# 2) Grant via update (expected: yes)
python3 provider-kubeconfig.py update "$NS" -c "$SA" -p "$PERMS" -k "$KCFG"
kubectl auth can-i create secrets -n "$NS" --as=system:serviceaccount:$NS:$SA --kubeconfig="$KCFG"

# 3) Revoke (expected: no)
python3 provider-kubeconfig.py revoke "$NS" -c "$SA" -p "$PERMS" -k "$KCFG"
kubectl auth can-i create secrets -n "$NS" --as=system:serviceaccount:$NS:$SA --kubeconfig="$KCFG"

Support permission updates and revocations from both JSON and YAML files, reuse shared parsing/configmap helpers, and extend tests to cover revoke CLI validation and multi-format permission file parsing. Made-with: Cursor

Run new update parser in shadow mode behind an env flag and assert old/new parity, with tests validating parser output equivalence. Made-with: Cursor

Inline legacy update parsing in _update_rbac for source-of-truth behavior and keep old/new parity assertions behind KUBEPLUS_UPDATE_EQ_CHECK, while removing extra helper methods. Made-with: Cursor

Use _load_permission_data only in _update_rbac and keep the original rule loop. Drop dual-parser parity and its test. Match _parse_permission_rules to legacy by always appending rule_group. Return early from revoke when no update ClusterRole exists to avoid rewriting perms configmap. Made-with: Cursor

Add _parse_update_rules_legacy for a single legacy implementation, optional KUBEPLUS_UPDATE_EQ_CHECK assertion against _parse_permission_rules in _update_rbac, and a unit test including non-apigroup edge cases. Made-with: Cursor

Remove duplicate _parse_update_rules_legacy and env parity self-check; add a focused test for non-apigroup edge case. Made-with: Cursor

Provide a minimal permissions file to validate update/revoke against an existing service account during manual testing. Made-with: Cursor

Provide three paired examples under tests/permission_files to validate update/revoke behavior with auth can-i for both input formats. Made-with: Cursor

Parse all JSON and YAML fixtures in tests/permission_files through provider-kubeconfig helpers to validate real file-based inputs used in manual update/revoke checks. Made-with: Cursor

Use tests/permission_files fixtures for manual checks and test coverage instead of the old tests/manual file. Made-with: Cursor

Verify update/revoke using matching JSON and YAML permission fixtures produce the same auth can-i transition on an existing service account (deny -> allow -> deny). Made-with: Cursor

Use standard '' key notation instead of explicit ? mapping form for readability while preserving equivalent fixture semantics. Made-with: Cursor

Prefer <sa>-update for revoke, then fall back to <sa> if needed; keep perms configmap sync and add integration coverage. Normalize permission fixtures so JSON/YAML stay equivalent and single-key per rule entry. Made-with: Cursor

Replace explicit '? ""' mapping form with standard '' key notation for readability and consistency. Made-with: Cursor

devdattakulkarni

Added some comments.

devdattakulkarni · 2026-04-07T21:40:42Z

+                            if resource not in resources:
+                                resources.append(resource.strip())
+                            rule_group = {}
+                            if api_group == "non-apigroup":


This is present in the old permission file.

devdattakulkarni · 2026-04-14T15:06:46Z

+      - get
+      - update
+      - patch
+      configmaps/resourceName::cert-manager-cainjector-leader-election-core:


This should be -configmaps/resourceName::

devdattakulkarni · 2026-04-14T15:12:36Z

+                revoke_rule_list, revoke_resources = self._parse_permission_rules(perms)
+                revoke_norm = set(tuple(sorted(r.items())) for r in self._normalize_rule_list(revoke_rule_list))
+
+                role_name = sa + "-update"


Here we are assuming that the role name (sa-"update") already exists on the cluster.

devdattakulkarni · 2026-04-14T15:21:05Z

+                remaining_rules = []
+                for rule in existing_rules:
+                    norm = self._normalize_rule(rule)
+                    norm_key = tuple(sorted(norm.items()))


Could we add a small example in comment. What would be norm_key and what would be the rule.

devdattakulkarni · 2026-04-14T15:21:10Z

-                )
+        def _revoke_rbac(self, permissionfile, sa, namespace, kubeconfig):
+                """Revoke permissions from JSON/YAML file for provider/consumer update role."""
+                perms = self._load_permission_data(permissionfile)


Suppose originally the SA has following perms:

""
pod:

get, delete

Suppose permissionfile has the following:

""
pod:

delete

After revoke, we want the SA to have the following perms:

""
pod:

get

devdattakulkarni · 2026-04-14T15:23:07Z

+                    norm = self._normalize_rule(rule)
+                    norm_key = tuple(sorted(norm.items()))
+                    if norm_key not in revoke_norm:
+                        remaining_rules.append(rule)


Iterate through the existing rules on the SA. If the rule does not exist in the revoke_norm then add it to the remaining rules.

Handle null rules safely, only fall back to base role on NotFound, apply minimal role objects during revoke updates, and use system temp files in tests while validating verb-level revoke behavior. Made-with: Cursor

Fix NotFound fallback checks, preserve labels/annotations on role rewrite, handle null rules safely, keep configmap resources when scope/verbs remain, and add integration coverage for multi-resource and wildcard revoke behavior. Made-with: Cursor

Consolidate follow-up bug fixes into a single, readable revoke path that preserves intended RBAC semantics (including nonResourceURL matching, base-role fallback, and scoped multi-resource revocation) while trimming test complexity to focused behavior coverage.

Restructure revoke logic for readability with plain-language scope comments, inline flow-oriented rule rewriting, and reduced helper complexity while keeping documented revoke behavior and test coverage aligned. Made-with: Cursor

Add revoke action and YAML/JSON permission parsing.

6e0c42b

Support permission updates and revocations from both JSON and YAML files, reuse shared parsing/configmap helpers, and extend tests to cover revoke CLI validation and multi-format permission file parsing. Made-with: Cursor

anniegracehu force-pushed the annie/revoke-update-perm-yaml-json branch from 5e4ab35 to 6e0c42b Compare March 30, 2026 18:23

anniegracehu added 3 commits March 30, 2026 11:26

Keep legacy update parser as runtime source of truth.

4719e7c

Run new update parser in shadow mode behind an env flag and assert old/new parity, with tests validating parser output equivalence. Made-with: Cursor

Minimize update parser parity diff.

49356a3

Inline legacy update parsing in _update_rbac for source-of-truth behavior and keep old/new parity assertions behind KUBEPLUS_UPDATE_EQ_CHECK, while removing extra helper methods. Made-with: Cursor

anniegracehu requested a review from devdattakulkarni March 30, 2026 19:49

anniegracehu self-assigned this Mar 30, 2026

anniegracehu added 10 commits March 30, 2026 12:52

Restore update parser parity checks.

32ebc09

Add _parse_update_rules_legacy for a single legacy implementation, optional KUBEPLUS_UPDATE_EQ_CHECK assertion against _parse_permission_rules in _update_rbac, and a unit test including non-apigroup edge cases. Made-with: Cursor

Use single _parse_permission_rules for update and revoke.

28d25b5

Remove duplicate _parse_update_rules_legacy and env parity self-check; add a focused test for non-apigroup edge case. Made-with: Cursor

Add manual YAML fixture for update/revoke e2e checks.

146a8e4

Provide a minimal permissions file to validate update/revoke against an existing service account during manual testing. Made-with: Cursor

Add JSON/YAML permission fixtures for manual update/revoke checks.

15ee238

Provide three paired examples under tests/permission_files to validate update/revoke behavior with auth can-i for both input formats. Made-with: Cursor

Add test coverage for permission fixture files.

b2cf020

Parse all JSON and YAML fixtures in tests/permission_files through provider-kubeconfig helpers to validate real file-based inputs used in manual update/revoke checks. Made-with: Cursor

Remove obsolete manual update/revoke permission fixture.

eeff860

Use tests/permission_files fixtures for manual checks and test coverage instead of the old tests/manual file. Made-with: Cursor

Add integration test for JSON/YAML auth can-i equivalence.

ff11a68

Verify update/revoke using matching JSON and YAML permission fixtures produce the same auth can-i transition on an existing service account (deny -> allow -> deny). Made-with: Cursor

Normalize empty-string YAML keys in permission fixtures.

3afe4df

Use standard '' key notation instead of explicit ? mapping form for readability while preserving equivalent fixture semantics. Made-with: Cursor

Fallback revoke to base role when update role missing.

5bfb1e1

Prefer <sa>-update for revoke, then fall back to <sa> if needed; keep perms configmap sync and add integration coverage. Normalize permission fixtures so JSON/YAML stay equivalent and single-key per rule entry. Made-with: Cursor

Normalize empty-string key style in YAML permission fixtures.

731a6bc

Replace explicit '? ""' mapping form with standard '' key notation for readability and consistency. Made-with: Cursor

devdattakulkarni reviewed Apr 14, 2026

View reviewed changes

anniegracehu added 3 commits April 14, 2026 08:49

Harden revoke path and test semantics.

9ffb0d8

Handle null rules safely, only fall back to base role on NotFound, apply minimal role objects during revoke updates, and use system temp files in tests while validating verb-level revoke behavior. Made-with: Cursor

anniegracehu force-pushed the annie/revoke-update-perm-yaml-json branch from 948d3d2 to 9475a4b Compare April 16, 2026 05:21

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add revoke action and YAML support for permission updates#1461

Add revoke action and YAML support for permission updates#1461
anniegracehu wants to merge 18 commits intocloud-ark:masterfrom
anniegracehu:annie/revoke-update-perm-yaml-json

anniegracehu commented Mar 30, 2026 •

edited

Loading

Uh oh!

devdattakulkarni left a comment

Uh oh!

devdattakulkarni Apr 7, 2026

Uh oh!

devdattakulkarni Apr 14, 2026

Uh oh!

devdattakulkarni Apr 14, 2026

Uh oh!

devdattakulkarni Apr 14, 2026

Uh oh!

devdattakulkarni Apr 14, 2026

Uh oh!

devdattakulkarni Apr 14, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

anniegracehu commented Mar 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test plan

Manual update/revoke e2e (existing SA)

Uh oh!

devdattakulkarni left a comment

Choose a reason for hiding this comment

Uh oh!

devdattakulkarni Apr 7, 2026

Choose a reason for hiding this comment

Uh oh!

devdattakulkarni Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

devdattakulkarni Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

devdattakulkarni Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

devdattakulkarni Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

devdattakulkarni Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

anniegracehu commented Mar 30, 2026 •

edited

Loading