Skip to content

Refactor provider-kubeconfig.py and add tests#1457

Open
anniegracehu wants to merge 14 commits intocloud-ark:masterfrom
anniegracehu:annie/provider_kubeconfig
Open

Refactor provider-kubeconfig.py and add tests#1457
anniegracehu wants to merge 14 commits intocloud-ark:masterfrom
anniegracehu:annie/provider_kubeconfig

Conversation

@anniegracehu
Copy link
Copy Markdown
Collaborator

@anniegracehu anniegracehu commented Feb 17, 2026

Provider Kubeconfig Refactor

Summary

Refactors provider-kubeconfig.py to use unified RBAC rule lists, fixes several bugs present on master, and adds tests. The script generates provider and consumer kubeconfigs with appropriate RBAC for the KubePlus platform.


Bug Fixes (master had these bugs)

nonResourceURL parsing (_update_rbac)

Master used parts[0] after splitting on nonResourceURL::; the correct value is in parts[1]. For "foo/nonResourceURL::bar": master→"foo/", PR→"bar".

Namespace creation (create action)

Master had create_ns = "kubectl get ns " + ... and ran that same GET command twice when the namespace was not found. The namespace was never created. PR runs kubectl create ns <namespace> when the namespace does not exist.

Token extraction (_extract_kubeconfig)

Master used if 'token' in line, which matches any line containing "token" (e.g. Type: kubernetes.io/service-account-token, Labels: kubernetes.io/legacy-token-last-used). PR only treats lines where the key is exactly "token" as token lines.

Sleep placement (token extraction)

Master had time.sleep(2) inside the line-parsing loop, so it ran once per line when the token wasn't found. PR moves it outside the loop so it runs once per retry.

Default kubeconfig path

Master used os.getenv("HOME") + "/.kube/config", which can fail if HOME is unset. PR uses os.path.expanduser("~") for robustness.


Main Changes (master → PR)

RBAC refactor

  • Added a temporary shadow-compare path:
    • keep legacy grouped rules ("pink") and unified rule_list rules ("green") side by side
    • optional parity assertion via KUBEPLUS_RBAC_EQ_CHECK=1
    • runtime still uses legacy grouped rules in this PR to minimize risk
  • Same permissions target: consumer (read + apps + impersonate + portforward), provider (full platform operator).
  • -perms ConfigMap behavior:
    • consumer all_resources includes wildcard entries as before
    • provider all_resources excludes "*" to match master behavior (master effectively omitted wildcard groups from provider perms inventory)

Code cleanup

  • Use module-level run_command instead of self.run_command.
  • create_role_rolebinding: use with open(..., encoding="utf-8"), os.path.join.
  • run_command: use context manager, handle None from communicate().
  • sorted(list(set(x)))sorted(set(x)).

Tests (new; master has none)

New file tests/test_provider_kubeconfig.py:

  • CLI: --help shows actions, flags, namespace; update without -p exits with error.
  • Integration: Provider and consumer kubeconfigs have non-empty fields, correct namespace, SA creation; flags -s, -x, -f reflected in output.
  • Consumer RBAC: test_consumer_cannot_create_pod_in_other_namespace verifies create in another namespace is forbidden.

Integration tests skip when no cluster is reachable (KUBECONFIG unset).


How to test

# CLI tests (no cluster needed)
python -m unittest tests.test_provider_kubeconfig.TestCli -v

# Full suite (requires cluster)
KUBECONFIG=/path/to/kubeconfig python -m unittest tests.test_provider_kubeconfig -v

Follow-up plan

  • This PR keeps legacy grouped RBAC (pink) as runtime source of truth with optional parity checks against unified rules (green).
  • In a follow-up PR, we will remove the legacy grouped RBAC path after parity verification is complete.

@anniegracehu anniegracehu self-assigned this Feb 17, 2026
@anniegracehu anniegracehu marked this pull request as ready for review February 17, 2026 20:11
@@ -856,10 +856,10 @@ def _generate_kubeconfig(self, sa, namespace, filename, api_server_ip='', kubeco
print("Permissions file should be used with update command.")
Copy link
Copy Markdown
Contributor

@devdattakulkarni devdattakulkarni Mar 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Add a generic help text/explanation for the purpose of provider-kubeconfig.py
  2. In the tests, verity that the various fields of the generated kubeconfig file are non-empty. And if we are passing any command-line flags (such as API server URL, cluster name, etc.) then the generated kubeconfig file has these values set for the appropriate fields.
  3. Update the help text for the namespace argument.
  4. Verify in the code that the scope of permissions for consumer is restricted to that namespace. The generated kubeconfig should have the provided namespace included in it. Verify manually that using this kubeconfig, we should not be able to do create/delete operations in other namespaces (try with creating/deleting a pod in other namespaces).

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have added the tests and updated the help text!

@anniegracehu anniegracehu changed the title Refactor kubeconfig: provider/consumer split, kubeconfig_generator, tests Refactor provider-kubeconfig.py and add tests Mar 10, 2026
@anniegracehu anniegracehu force-pushed the annie/provider_kubeconfig branch from f49b13e to b924df6 Compare March 10, 2026 05:13
Keep old grouped provider/consumer RBAC as active source, add optional old-vs-new parity assertion for safer review, and document provider perms wildcard handling to match master behavior.

Made-with: Cursor
Move PR notes to the GitHub PR description and drop the committed markdown helper file from the branch.

Made-with: Cursor
Compute old/new all_resources from their respective rule builders, assert parity under the existing RBAC equivalence flag, and keep runtime behavior sourced from old rules and old all_resources.

Made-with: Cursor
Use the current kubeconfig server URL for generated consumer kubeconfig in the cross-namespace authz test, and skip when API connectivity is transient so RBAC assertions only run on reachable clusters.

Made-with: Cursor
Keep comment text neutral while preserving old-rule source-of-truth behavior for the shadow parity flow.

Made-with: Cursor
Validate that -x sets both cluster entry names and context cluster reference, and ensure the cross-namespace consumer denial test verifies the forbidden error references the target namespace.

Made-with: Cursor
…ied pod create.

Replace the cross-namespace pod check with a behavior-valid assertion pair for current consumer RBAC: deployment creation succeeds while pod creation is forbidden using the generated consumer kubeconfig.

Made-with: Cursor
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants