|
| 1 | +# TODO: Enhancement Proposal — Deprecate spec.serviceAccount |
| 2 | + |
| 3 | +Based on [EP #1860](https://github.com/openshift/enhancements/pull/1860) (closed), |
| 4 | +[PR #2544 design doc](https://github.com/operator-framework/operator-controller/pull/2544), |
| 5 | +and [implementation PR #2770](https://github.com/operator-framework/operator-controller/pull/2770). |
| 6 | + |
| 7 | +## Source materials |
| 8 | + |
| 9 | +- [X] Copy EP #1860 content as starting point → `docs/draft/project/service-account-deprecation-ep.md` |
| 10 | +- [X] Save #1860 reviewer comments as reference → `docs/draft/project/ep-reviewer-feedback.md` |
| 11 | +- [X] Save #2544 design doc as reference → `docs/draft/project/single-tenant-design-ref.md` |
| 12 | + |
| 13 | +## Reviewer concerns to address (from EP #1860) |
| 14 | + |
| 15 | +### everettraven (API reviewer) |
| 16 | + |
| 17 | +- [ ] "Rarely helpful" not justified — use #2544's detailed explanation of SA model failures (false sense of security, SA reference escalation, usability burden) |
| 18 | +- [ ] Privilege escalation risk with cluster-admin — explain risk already exists (any CE writer can reference any SA in any namespace); cluster-admin makes it explicit |
| 19 | +- [ ] Deprecated fields should still function — clarify: field is accepted but ignored (no validation error), existing CEs continue working |
| 20 | +- [ ] Downgrade handling — OCP doesn't support minor version downgrades; field remains in API so no serialization issue |
| 21 | +- [ ] Older client compatibility — field stays in API with `omitzero`; clients reading it see empty value, not missing field |
| 22 | +- [ ] `x-kubernetes-deprecated` doesn't exist — we use ValidatingAdmissionPolicy with `validationActions: [Warn]` instead (kubernetes/kubernetes#131817) |
| 23 | +- [ ] Alternative: better client tooling — address as considered alternative, doesn't solve fundamental SA model issues |
| 24 | +- [ ] Automatic upgrade privilege escalation — OLM could stamp out malicious code; address in risks section |
| 25 | +- [ ] How do users manage controller permissions — document RBAC + VAP as the scoping mechanism |
| 26 | +- [ ] What are "Synthetic Permissions" — explain and justify removal |
| 27 | + |
| 28 | +### JoelSpeed (API reviewer) |
| 29 | + |
| 30 | +- [ ] Should support three paths (no SA, namespace-scoped admin, full least-privilege) — biggest tension; propose RBAC + VAP as replacement for per-SA scoping (Stream 6 / OPRUN-4634) |
| 31 | +- [ ] Still need EP process for this change — yes, this EP is that process |
| 32 | + |
| 33 | +### grokspawn |
| 34 | + |
| 35 | +- [ ] Deprecation metrics — note as follow-up; currently log + VAP warning; consider `clusterextension_requested_deprecated_api` gauge |
| 36 | +- [ ] Include preflight permissions removal — yes, those are alpha/not GA'd, removed in implementation |
| 37 | +- [ ] GA'd OCP APIs have never been removed — discuss field removal timeline, keeping field in API indefinitely as ignored |
| 38 | + |
| 39 | +## EP revision tasks |
| 40 | + |
| 41 | +- [ ] Clean up EP frontmatter (update authors, dates, tracking-link to OCPSTRAT-3040) |
| 42 | +- [ ] Rewrite Motivation section using #2544's stronger rationale |
| 43 | +- [ ] Update Goals to match OCPSTRAT-3040 requirements (15 items) |
| 44 | +- [ ] Update Proposal/API Changes to reflect actual implementation: |
| 45 | + - `omitzero` instead of pointer |
| 46 | + - Godoc following OpenShift convention (`Deprecated:` at end) |
| 47 | + - Relaxed immutability rule (allow clearing) |
| 48 | + - ValidatingAdmissionPolicy for kubectl warnings |
| 49 | + - CRB renamed to `operator-controller-cluster-admin-rolebinding` |
| 50 | + - Install script handles upgrade (old CRB + ClusterRole cleanup) |
| 51 | +- [ ] Update Proposal/Controller Logic to reflect actual implementation: |
| 52 | + - Controller uses own SA for all interactions |
| 53 | + - `ServiceAccountDeprecationWarning` logs when field is set |
| 54 | + - All SA impersonation/token/synthetic code removed |
| 55 | + - ContentManager replaced with shared TrackingCache |
| 56 | +- [ ] Add Design Details section: |
| 57 | + - TrackingCache architecture (single global cache, event suppression) |
| 58 | + - `WithWatchesRawSource` builder option gated on Helm runtime |
| 59 | + - Nil guard for TrackingCache in Apply path |
| 60 | +- [ ] Update Test Plan to reflect actual testing: |
| 61 | + - Unit tests for admission, helm applier |
| 62 | + - envtest for VAP warning capture |
| 63 | + - e2e: standard (49 scenarios), experimental (54 scenarios), upgrade (st2st) |
| 64 | + - Deprecation warning scenario |
| 65 | +- [ ] Update Graduation Criteria / Deprecation Plan for OCP timeline |
| 66 | +- [ ] Update Upgrade / Downgrade Strategy: |
| 67 | + - CRB rename handles roleRef immutability |
| 68 | + - Old RBAC resources cleaned up by install script |
| 69 | + - Field remains in API, no downgrade issue |
| 70 | +- [ ] Add Alternatives Considered: |
| 71 | + - Keep SA functional during deprecation (everettraven's preference) |
| 72 | + - Synthetic permissions / impersonation model (#1897) |
| 73 | + - Better client tooling for SA derivation |
| 74 | + - Three paths: no SA / namespace admin / full least-privilege (JoelSpeed) |
| 75 | + - RBAC + VAP as replacement for per-SA scoping |
| 76 | + |
| 77 | +## Final steps |
| 78 | + |
| 79 | +- [ ] Review draft EP for completeness and consistency |
| 80 | +- [ ] Port to openshift/enhancements repo (fork, branch from master) |
| 81 | +- [ ] Open PR against openshift/enhancements |
| 82 | +- [ ] Link EP PR to OPRUN-4629 and OCPSTRAT-3040 |
0 commit comments