Skip to content

Commit 04dc9d2

Browse files
committed
Add RBAC shadow parity check while keeping legacy runtime path.
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
1 parent 6720c57 commit 04dc9d2

File tree

2 files changed

+432
-33
lines changed

2 files changed

+432
-33
lines changed

PR_DESCRIPTION.md

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
# Provider Kubeconfig Refactor
2+
3+
## Summary
4+
5+
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.
6+
7+
---
8+
9+
## Bug Fixes (master had these bugs)
10+
11+
### nonResourceURL parsing (`_update_rbac`)
12+
13+
Master used `parts[0]` after splitting on `nonResourceURL::`; the correct value is in `parts[1]`. For `"foo/nonResourceURL::bar"`: master→`"foo/"`, PR→`"bar"`.
14+
15+
### Namespace creation (create action)
16+
17+
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.
18+
19+
### Token extraction (`_extract_kubeconfig`)
20+
21+
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.
22+
23+
### Sleep placement (token extraction)
24+
25+
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.
26+
27+
### Default kubeconfig path
28+
29+
Master used `os.getenv("HOME") + "/.kube/config"`, which can fail if `HOME` is unset. PR uses `os.path.expanduser("~")` for robustness.
30+
31+
---
32+
33+
## Main Changes (master → PR)
34+
35+
### RBAC refactor
36+
37+
- Added a temporary shadow-compare path:
38+
- keep legacy grouped rules ("pink") and unified `rule_list` rules ("green") side by side
39+
- optional parity assertion via `KUBEPLUS_RBAC_EQ_CHECK=1`
40+
- runtime still uses legacy grouped rules in this PR to minimize risk
41+
- Same permissions target: consumer (read + apps + impersonate + portforward), provider (full platform operator).
42+
- `-perms` ConfigMap behavior:
43+
- consumer `all_resources` includes wildcard entries as before
44+
- provider `all_resources` excludes `"*"` to match master behavior (master effectively omitted wildcard groups from provider perms inventory)
45+
46+
### Code cleanup
47+
48+
- Use module-level `run_command` instead of `self.run_command`.
49+
- `create_role_rolebinding`: use `with open(..., encoding="utf-8")`, `os.path.join`.
50+
- `run_command`: use context manager, handle `None` from `communicate()`.
51+
- `sorted(list(set(x)))``sorted(set(x))`.
52+
53+
---
54+
55+
## Tests (new; master has none)
56+
57+
New file `tests/test_provider_kubeconfig.py`:
58+
59+
- **CLI:** `--help` shows actions, flags, namespace; `update` without `-p` exits with error.
60+
- **Integration:** Provider and consumer kubeconfigs have non-empty fields, correct namespace, SA creation; flags `-s`, `-x`, `-f` reflected in output.
61+
- **Consumer RBAC:** `test_consumer_cannot_create_pod_in_other_namespace` verifies create in another namespace is forbidden.
62+
63+
Integration tests skip when no cluster is reachable (`KUBECONFIG` unset).
64+
65+
---
66+
67+
## How to test
68+
69+
```bash
70+
# CLI tests (no cluster needed)
71+
python -m unittest tests.test_provider_kubeconfig.TestCli -v
72+
73+
# Full suite (requires cluster)
74+
KUBECONFIG=/path/to/kubeconfig python -m unittest tests.test_provider_kubeconfig -v
75+
```

0 commit comments

Comments
 (0)