Add OpenShift (OKD/OCP) Emulation with ROSA and ARO Provisioning - #399
Add OpenShift (OKD/OCP) Emulation with ROSA and ARO Provisioning#399thzgajendra wants to merge 5 commits into
Conversation
…ntion count (stackshy#383) referencedInterface counted every driver.<Name> selector, so an optional capability probed via repeated type assertions (networking's driver.NetworkInterfaces) outranked the stored driver.Networking field. Count only field/parameter type positions, which exclude type-assertion expressions. Fixes networking understating its surface (3 ops -> full Networking) and Azure/GCP showing blank.
Layer a full OpenShift API surface on the existing Kubernetes data-plane emulator, plus both cloud provisioning paths, so `oc` and `rosa`/ARO clients operate end-to-end. Data plane (services/kubernetes): - Flavor gating: OpenShift clusters serve the *.openshift.io groups and seed ClusterVersion/Infrastructure identity singletons; Kubernetes clusters are unchanged. RegisterClusterWithFlavor selects the surface. - All base + secondary *.openshift.io groups registered as CRUD (apps, route, build, image, project, user, oauth, security, quota, authorization, template, console, operator, machine, autoscaling); discovery auto-derives. - Tier-A behavior: Route admission (status.ingress), ImageStream/Project/ DeploymentConfig reconcile, `oc new-project` (ProjectRequest), Build + `oc start-build` (BuildConfig instantiate), and `oc process`. - `oc login`: OAuth server (well-known + authorize + PKCE token), whoami, and permissive authorization reviews — validated against the real `oc` CLI. Provisioning: - ARO via Azure ARM (Microsoft.RedHatOpenShift/openShiftClusters) with listAdminCredentials. - ROSA via the OCM REST API (/api/clusters_mgmt/v1/clusters + SSO token), registered on the AWS server. Each control plane registers an OpenShift-flavored data plane and returns a working oc kubeconfig.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
The authorize/token endpoints echoed the client-supplied redirect_uri into the 302 Location, so a crafted redirect_uri could exfiltrate the access token (carried in the URL fragment) to an attacker-controlled origin — flagged by CodeQL (go/unvalidated-url-redirection). Resolve the redirect target from the server's own absolute base (<cluster>/oauth/token/implicit) instead. The client redirect_uri is only accepted when it exactly matches that endpoint; the value written to Location is always server-constructed, so the tainted string never reaches the sink. This is also the correct OAuth behavior — validating redirect_uri against the registered endpoint — and keeps the real `oc login` flow working (its challenging client sends exactly that URI). Add a negative test asserting a cross-origin redirect_uri is rejected with 400.
Addressed CodeQL review (9ce6d54)Both
|
thzgajendra
left a comment
thzgajendra
left a comment
There was a problem hiding this comment.
In-depth review — OpenShift emulation (backend test agent + source verification)
Reviewed by building & running the full suites in an isolated worktree via a backend test agent, then verifying the security- and blast-radius-critical pieces in source myself. The PR is solid — test plan holds green, security fix is correct, blast radius is genuinely contained.
✅ Build / vet / test — all green (Go 1.25)
go build ./...✅ ·go vet(touched pkgs) ✅go testacross the 5 new packages: 199 subtests PASS / 0 FAIL / 0 SKIP.- Real
oc(v4.22.8) E2E actually ran (not skipped) —oc loginchallenging-client OAuth flow →oc whoami == developer. Full wire flow against the real CLI. - ARO (Azure ARM) and ROSA (OCM REST) provisioning lifecycles each create a cluster → reach
config.openshift.io/v1/clusterversions/versionthrough the returned kubeconfig → delete → assert teardown (IsNotFound). Data-plane reached end-to-end, not status-code-only. - Data-plane behavior (Route admission,
oc new-project,oc start-build,oc process, OAuth metadata/challenge/token) asserts real data shapes.
✅ Security — the open-redirect fix (PR head 9ce6d54) is correct and complete
Verified in source: the authorize/token endpoints no longer echo the client redirect_uri into Location. allowedRedirectTarget accepts a supplied redirect_uri only on exact match to the server's own absBase + /oauth/token/implicit, and the value written to Location is always server-constructed — so the tainted string never reaches the redirect sink (resolves CodeQL go/unvalidated-url-redirection). absBase derives from r.Host, which in the browser attack scenario is the real host the victim contacted, not attacker-controllable via the crafted link. There's a real regression test for it (openshift_test.go:709 — cross-host redirect_uri → 400, no attacker Location).
✅ Blast radius — additive and flavor-gated, verified in source
The "EKS/AKS/GKE byte-for-byte unchanged" claim holds: state.go:130 adds openshiftRegistryDefs() only if flavor == FlavorOpenShift; singletons seed only for OpenShift (state.go:182); the OAuth path (apiserver.go:201) and serveOpenShiftIntercept (openshift.go:135) both no-op for non-OpenShift clusters. A vanilla/cloud cluster (FlavorKubernetes) activates none of the new surface. Provisioning handlers register on paths disjoint from the AWS-SDK / Azure-ARM routes.
ℹ️ Non-blocking
- One lint finding is pre-existing, not this PR:
services/kubernetes/pod.go:510unused//nolint:gosec. Confirmedgit diff <base>..HEAD -- pod.gois empty — it's part of the stacked, unmerged Kubernetes base. It'll surface when that base merges; nothing to fix here. - Stacked-PR reminder: this branch sits on the unmerged k8s data-plane emulator, so it can't land until that does (you noted this). The OpenShift-specific diff (~22 files, +4350) is what I reviewed.
- The documented follow-ups (DeploymentConfig ReplicationController fidelity — DC deprecated since 4.14; server-side
TemplateInstancematerialization; console/operator singleton seeding) are reasonable deferrals given the direct pod-convergence model.
Verdict: implementation is idiomatic (plugs into the declarative registry + reconcile engine rather than hand-written handlers), the security fix is airtight, the blast radius is contained, and the tests genuinely exercise the behavior — including real-oc login and both provisioning lifecycles. No defect found in the OpenShift change. Clean from my side, pending the k8s base merge.
thzgajendra
left a comment
thzgajendra
left a comment
There was a problem hiding this comment.
Live-server E2E (correction to my earlier "clean" verdict)
I went further than the test suite: booted the actual serve binary and drove it with curl + the real oc v4.22.8 CLI across all 9 cases. 7/9 pass live — including the two I most wanted proven — but the live drive surfaced 2 real issues the go test suite masks. Both verified in source.
✅ Proven live
- Open-redirect fix (#6): GET
/oauth/authorize?…redirect_uri=https://evil.example.com/x→ 400, noLocationheader at all, bodyredirect_uri does not match the cluster OAuth endpoint. Empty/exact-match redirect_uri → 302 to the server-built target. Airtight at the live edge. - Blast radius (#7): a live EKS cluster's
/apis→ 12 groups, 0*.openshift.io; OAuth metadata / whoami / projectrequests all 404. Flavor gating holds live. - OAuth metadata, Route admission (
status.ingresspopulated), all 16 openshift groups +oc api-resources(69 resources),oc start-build/oc process, and both ARO and ROSA lifecycles (create → reachclusterversions/version 4.16.0via kubeconfig → delete → 404) — all PASS live.
⚠️ Found only by hitting the real server
1. oc new-project fails against the live emulator — real, and in this PR. oc new-project first issues a GET /apis/project.openshift.io/v1/projectrequests (confirmed via oc --loglevel=6), which 404s and aborts the command. Root cause: projectRequestPath is wired POST-only (openshift.go:157 → serveOpenShiftPost) and projectrequests is not a registered resourceDef, so the GET falls through to the generic registry → 404. The POST RPC itself works (201, valid Project) — so the direct-POST test passes, but the actual oc new-project CLI command the PR claims to support doesn't complete. Note the comment at openshift_authz.go:14-17 anticipates the authorization review ("may I list projectrequests") but not the literal GET on the collection. Fix: serve GET on projectrequests (empty list / list-kind).
2. Real oc login fails against the serve binary on the serving cert — pre-existing base, but it undercuts this PR's headline + hides a test gap. internal/k8spki/pki.go mints the k8s leaf with caValidYears = 10 (NotAfter +10y); Go-1.23+ x509 (oc 4.22.8) rejects a server leaf with validity >825 days as "not standards compliant" — even with --insecure-skip-tls-verify. Through a compliant-cert proxy, oc login + oc whoami → developer work perfectly. pki.go is unchanged by this PR (stacked k8s base), so the fix belongs there (cap leaf NotAfter ≤398d). But two things for #399: (a) the objective — "oc/rosa/ARO clients operate end-to-end" — isn't actually met against the real binary until the base cert is capped; (b) openshift_oc_e2e_test.go passes only because it uses httptest.NewTLSServer's compliant cert, not the real serve cert path — so the E2E test can't catch this. Worth pointing the real-oc E2E at the actual serve cert.
3. Minor: on the shared AWS endpoint, a form-encoded OCM token POST is intercepted by the AWS Query handler (InvalidAction); without a form body it returns a Bearer token. Real rosa's exact token request shape is worth confirming against this handler ordering.
Revised verdict
The OpenShift logic, the security fix, and the blast-radius gating are all correct and now proven live. But the emulator's stated goal — real oc/rosa clients working end-to-end against the serve binary — is not fully met today: oc new-project needs a GET projectrequests handler (this PR), and real oc login needs the base serving cert capped to ≤398 days (stacked base). Both are small, and both are invisible to go test because the suite uses direct POSTs and httptest certs rather than the real CLI against the real server.
thzgajendra
left a comment
thzgajendra
left a comment
There was a problem hiding this comment.
Inline comments for the live-drive findings (details in the two reviews above). Security fix + blast-radius gating verified live and correct; the two actionable items are pinned to their lines below.
| // it served the request. | ||
| func (s *ClusterState) serveOpenShiftPost(w http.ResponseWriter, r *http.Request) bool { | ||
| switch r.URL.Path { | ||
| case projectRequestPath: |
There was a problem hiding this comment.
oc new-project fails against the live server (High, this PR). projectrequests is dispatched POST-only here, and it's not a registered resourceDef — so a GET /apis/project.openshift.io/v1/projectrequests falls through to the generic registry → 404. Real oc new-project issues that GET first (confirmed via oc --loglevel=6) and aborts on the 404. The POST RPC works (201, valid Project), so the direct-POST unit test passes — but the actual CLI command the PR lists as supported doesn't complete. openshift_authz.go:14-17 anticipates the authorization review ("may I list projectrequests") but not the literal GET on the collection. Fix: serve GET on projectrequests (empty list / list-kind).
| // returns the Project. On a real cluster the project controller creates the | ||
| // Namespace from the request; the emulator does both inline so the new project | ||
| // is immediately usable for `oc apply`. | ||
| func (s *ClusterState) serveProjectRequest(w http.ResponseWriter, r *http.Request) { |
There was a problem hiding this comment.
This handler covers the POST (oc new-project create) correctly. The gap is the read side: add a GET path that returns an empty ProjectRequestList (or reuse the generic list machinery) so oc new-project's pre-flight GET on the collection gets a 200 instead of a 404. Same-file locality makes this a small addition.
| // exactly matches it (the challenging client always sends this). The returned | ||
| // value is ALWAYS the server-constructed URL, never the client string, so it | ||
| // cannot be an open redirect regardless of what the client sends. | ||
| func allowedRedirectTarget(absBase, redirectURI string) (string, bool) { |
There was a problem hiding this comment.
✅ Verified live: the open-redirect fix is airtight. A cross-host redirect_uri=https://evil.example.com/x returns 400 with no Location header at all (never leaks the token fragment to the attacker origin), while empty/exact-match redirect_uri 302s to the server-built target. target is always server-constructed from absBase (r.Host, which is the real host in the browser attack path), so the tainted string never reaches the sink. Good CodeQL resolution, and there's a real regression test at openshift_test.go:709.
| uid, _ := api.RegisterClusterWithFlavor(kubernetes.FlavorOpenShift) | ||
| // oc's OAuth challenge path assumes an HTTPS endpoint (real clusters always | ||
| // are) and dereferences the TLS transport, so serve over TLS here. | ||
| ts := httptest.NewTLSServer(api) |
There was a problem hiding this comment.
Test-fidelity gap. This E2E drives real oc against httptest.NewTLSServer, which issues Go's own standards-compliant cert — so the test passes. But the real serve binary mints its k8s serving leaf via internal/k8spki/pki.go with caValidYears = 10 (NotAfter +10y), and oc's Go-1.23 x509 rejects a server leaf with validity >825 days ("not standards compliant") even with --insecure-skip-tls-verify — so real oc login against the running server fails (works only through a compliant-cert proxy). pki.go is unchanged by this PR (stacked base), so the cert cap (≤398d) belongs there — but consider pointing this E2E at the actual serve cert path so it would have caught it, since "real clients work end-to-end" is the emulator's stated goal.
Three issues surfaced by driving the real serve binary with oc/rosa: - oc new-project aborted: it GETs the projectrequests collection before POSTing, which 404'd because only POST was wired. Serve GET with an empty ProjectRequestList so the CLI proceeds to the create. - OCM SSO token request was answered with InvalidAction on the shared AWS endpoint: the AWS Query handlers claim any form-encoded POST and were registered ahead of OCM. Register the OCM handler first so its specific path match (/api/clusters_mgmt/, /auth/realms/.../token) wins. - Real oc login was rejected on the serving cert: the leaf reused the 10-year CA validity, and Go 1.23+ x509 (oc 4.22.8) rejects a leaf >825 days even under InsecureSkipVerify. Cap the serving leaf to 397 days (CA stays long-lived). Point the real-oc E2E at the actual ServingTLSConfig cert instead of httptest's own cert, so it exercises the real cert path. Add regression tests: GET projectrequests, and an OCM token POST driven through the full AWS server (form-encoded) asserting a Bearer token.
Addressed the live-server findings (1ac9047)Thanks for driving the real 1. 3. OCM token → 2. Real Verified: real |
thzgajendra
left a comment
thzgajendra
left a comment
There was a problem hiding this comment.
Re-review of 1ac90473 — live re-drive against the real serve binary
Re-verified all fixes live (booted the real server, real oc v4.22.8, no proxy). 3 of 4 fixes confirmed working end-to-end; the oc new-project fix is incomplete — the command still aborts. Verified in source.
✅ Confirmed working live
- Serving cert (Fix 1) — PASS.
oc loginagainst the realservebinary now succeeds without any proxy andoc whoami → developer. The served leaf is exactly 397 days (Aug 14 2026 → Sep 15 2027), under the 825/398-day limits. 👍 - Real-cert E2E (Fix 4) — great call.
openshift_oc_e2e_test.gonow drivesocagainstk8spki.ServingTLSConfig(the real serve cert), so it would now catch a cert regression instead of masking it withhttptest's own cert. - OCM token routing (Fix 3) — PASS. Form-encoded OCM SSO token POST on the shared AWS endpoint now returns a Bearer token (not
InvalidAction); full ROSA create→describe→delete works. Registering OCM first is the right ordering. - Regressions hold: open-redirect still 400/no-Location; blast radius intact — a vanilla EKS cluster still serves 0
*.openshift.iogroups and the new projectrequests GET is properly flavor-gated (404 on non-OpenShift, 200 on OpenShift), verified in source atopenshift.go:135.
⚠️ oc new-project (Fix 2) — still broken end-to-end
The 404 is gone (GET now returns 200), but oc new-project <name> still aborts and creates nothing:
error: no kind "ProjectRequestList" is registered for version "project.openshift.io/v1"
Root cause (confirmed via oc --loglevel=8): serveProjectRequestList returns kind: "ProjectRequestList", which is not a registered kind in oc's project.openshift.io/v1 scheme (it registers Project, ProjectList, ProjectRequest — not ProjectRequestList). oc fails the client-side decode of the pre-flight GET before it ever POSTs, so the project is never created (oc get project <name> → NotFound). The direct-curl GET and the direct-POST both work, which is why the unit test passes — but the real CLI command the PR lists as supported does not complete.
And the new test locks in the wrong value: openshift_test.go:590 asserts kind == "ProjectRequestList", so it will keep passing while oc new-project keeps failing. See inline comments for the fix.
Verdict
Cert, OCM, and test-fidelity fixes are solid and confirmed live. oc new-project needs one more change — return a kind oc can decode (ProjectList is the high-confidence answer; worth confirming against a real cluster's GET-projectrequests response) and update the test assertion to match. Once that lands I'll re-drive it.
| func serveProjectRequestList(w http.ResponseWriter) { | ||
| writeJSON(w, http.StatusOK, map[string]any{ | ||
| "apiVersion": apiGroupOSProject + "/v1", | ||
| "kind": "ProjectRequestList", |
There was a problem hiding this comment.
This kind is undecodable by oc, so oc new-project still fails. oc's project.openshift.io/v1 scheme registers Project, ProjectList, ProjectRequest — but not ProjectRequestList. Returning it makes oc abort the pre-flight GET with no kind "ProjectRequestList" is registered for version "project.openshift.io/v1" (confirmed via oc --loglevel=8 against the real server), before it POSTs — so the project is never created even though the GET is now 200. Fix: emit kind: "ProjectList" (apiVersion project.openshift.io/v1, empty items) — which oc can decode and is what the analogous projects list returns. Worth double-checking against a real cluster's actual GET-projectrequests response, but ProjectRequestList is definitively not it.
|
|
||
| mustDecode(t, resp.Body, &list) | ||
|
|
||
| if list.Kind != "ProjectRequestList" { |
There was a problem hiding this comment.
This assertion locks in the wrong kind — it asserts exactly the value that makes the live oc new-project fail, so the test stays green while the CLI aborts. This is why the regression slipped through. Update it to the corrected kind (ProjectList) alongside the handler fix. Better still: the real coverage for this behavior is the oc-driven E2E (openshift_oc_e2e_test.go) — consider extending it to run oc new-project end-to-end so a decode-incompatible kind can't pass again.
NitinKumar004
left a comment
NitinKumar004
left a comment
There was a problem hiding this comment.
Review notes
Real data-plane engine (per #427): PRESENT — the engine-eligible seam (shared Kubernetes data plane) is wired end-to-end. providers/openshift/ocm and providers/azure/aro both call k8sAPI.RegisterClusterWithFlavor(kubernetes.FlavorOpenShift), point api.url/kubeconfig at /k8s/, return a working kubeconfig via kubernetes.RenderKubeconfig, and DeregisterCluster on delete.
Staleness / integration: CONFLICTS(providers/azure/azure.go, server/azure/from_provider.go). git merge origin/development into the PR head (32 commits behind) conflicts in exactly two files — both are gofmt-alignment conflicts where the PR adds the ARO field and development independently added the ContainerInstances field to the same aligned struct/literal block.
Terraform compat: N/A — the contrib/terraform harness only exercises core resources (aws_vpc, aws_subnet, aws_s3_, aws_dynamodb_table, aws_iam_role, aws_security_group, aws_route_table), and docs/compat/compat.json is the fixed 16-category SDK-compat matrix. No managed-cluster resource (aks/eks/gke) is covered anywhere, so OpenShift/ARO having no terraform round-trip or compat entry is consistent with existing precedent.
coveragegen / docs autogen: OK — go generate ./... on the merged (development+PR) tree leaves NO diff, so the committed docs/coverage is deterministic and complete. The generator change in internal/coveragegen/services.go (countSelectors -> countFieldTypes + selectorType) is a correct, deterministic fix: it counts a driver interface only where it appears as a struct-field/param/result TYPE, so an optional capability probed via a type assertion (x.(driver.NetworkInterfaces)) is no longer mis-picked as the primary interface.
Findings
High · staleness — PR is 32 commits behind development and does not merge cleanly — needs rebase
providers/azure/azure.go:139
Cannot fast-merge to development. A blind squash/merge that mis-resolves the aligned block would drop either ARO or ContainerInstances from the provider factory / server DriversFrom, silently disabling one service. Requires a rebase onto development; after a correct union resolution build/vet/test/-race are all green, so the fix is mechanical but mandatory before merge.
git merge origin/development conflicts in providers/azure/azure.go and server/azure/from_provider.go. Both are gofmt-alignment conflicts: this PR adds the ARO field while development added the ContainerInstances field to the same aligned Provider struct/literal and Drivers literal. The PR's green CI ran against a base 32 commits stale and never compiled against the ContainerInstances field, so the conflict is invisible to it.
Medium · wire-fidelity — /oauth/token reads the whole request body with no cap (gosec G120); sibling review handler caps but this does not
services/kubernetes/openshift_oauth.go:175
If a client (or fuzzer) POSTs a multi-GB body to /k8s//oauth/token during the oc-login PKCE exchange -> the server buffers it entirely and can OOM, since Go's http server sets no default body limit; wrap the body in http.MaxBytesReader (or io.LimitReader) as reviewNamespace already does. Also the PR's own 'golangci-lint 0 issues' checkbox is false until this is addressed.
serveOAuthToken calls r.FormValue("code"), which triggers ParseForm and buffers the entire POST body into memory. This endpoint is unauthenticated and reachable by any client on an OpenShift-flavored cluster. The sibling reviewNamespace (openshift_authz.go:110) deliberately wraps its read in io.LimitReader(maxReviewBodyBytes=1<<20) — the author knew the pattern but did not apply it here. golangci-lint flags it (G120).
Low · wire-fidelity — OCM error envelope is malformed on the common not-found/error path (id/code text instead of numeric, no href)
server/openshift/ocm/operations.go:141
If a client or the OCM SDK decodes the error .id/.code numerically (or matches CLUSTERS-MGMT-) on a describe/delete of a missing cluster -> it sees 'CLUSTERS-MGMT-Not Found' and mis-parses; rosa mostly surfaces .reason so user-visible impact is low, but it is a wire-shape divergence for SDK error branches.
writeCErr calls writeOCMError with http.StatusText(status), so a NotFound from GET/DELETE /clusters/{id} yields id="Not Found" and code="CLUSTERS-MGMT-Not Found". Real OCM returns id="404", code="CLUSTERS-MGMT-404", and includes an href field ("/api/clusters_mgmt/v1/errors/404") that ocmError omits entirely. The explicit-404 handler paths pass "404" correctly, but the mapped-error path (the most common) does not.
Low · coverage — gosec G101 hardcoded-credential false positive on ARO listCredentials needs a nolint to keep the lint gate green
server/azure/aro/operations.go:98
If a maintainer runs the mandated golangci-lint run ./... before pushing -> it reports a false-positive credential leak; add //nolint:gosec // static placeholder for the unauthenticated emulator so the intent is explicit and the gate stays clean.
KubeadminPassword: "cloudemu-kubeadmin" is a deliberate static placeholder for the unauthenticated emulator (documented in the function comment), but gosec G101 flags it. No //nolint:gosec directive is present, so the CLAUDE.md 0-issue local gate is red.
Low · correctness — oc process template-parameter generator has no cap on the {count} quantifier (unbounded allocation)
services/kubernetes/openshift_template.go:236
If a user runs oc process on a template whose parameter carries a huge {count} quantifier -> genRandomString allocates unboundedly and can OOM the process. Low because the template is the user's own input on a local test backend; still worth a sane cap (e.g. clamp count to a few KB).
parseCount accumulates n = n*10 + digit with no upper bound, and genRandomString does make([]byte, n). A processedtemplates POST with a parameter from:"[a-z]{999999999}" and generate:"expression" drives a ~1GB allocation. serveProcessedTemplate holds no lock, so it does not block the cluster, but it can exhaust memory.
Low · structure — New top-level providers/openshift + server/openshift namespace sits outside the documented set
providers/openshift/ocm/ocm.go:1
If STRUCTURE.md is treated as strict source-of-truth -> a future 'openshift' cloud family is undocumented and contributors won't know it exists as a placement option; confirm with the STRUCTURE owner whether to document 'openshift' as a provider family or relocate under providers/aws/rosa.
docs/STRUCTURE.md §2 defines as exactly aws|azure|gcp for providers// and server//. This PR introduces providers/openshift/ocm and server/openshift/ocm — a new family. Because OCM registers on the AWS server (ROSA is AWS-hosted per the handler doc), providers/aws/rosa + server/aws/rosa would fit the documented layout. The CI Structure gate still passes (no cloud-prefixed leaf, snake_case files, provider<->wire names match), and 'openshift' is arguably defensible since OCM is Red Hat's multi-cl
Low · structure — ROSA/OCM wired only in cmd/cloudemu serve, not in awsserver.NewFromProvider — asymmetric with ARO
cmd/cloudemu/serve.go:211
If a library user embeds the AWS server via awsserver.NewFromProvider and points rosa at it -> the OCM paths 404 because no OCM handler is registered; consistent with how K8sAPI is also serve-only, so impact is low, but the ARO/ROSA asymmetry is worth a doc note.
OCM is constructed inline in runServe (ocm.New + SetK8sAPI + d.OCM = ...) and is not part of the aws.Provider struct, so awsserver.NewFromProvider(p) never wires it. ARO, by contrast, is a field on azure.Provider and flows through DriversFrom, so it works in both NewFromProvider and serve. A library embedder using NewFromProvider gets ARO but not ROSA.
Low · docs — Two stale in-code comments: per-file group claim and OCM 'uninstalling' transition that never happens
services/kubernetes/openshift.go:63
If a contributor greps for openshift_route.go/openshift_apps.go per the comment -> the files don't exist; and a reader expecting an observable 'uninstalling' state on delete won't find one. Cosmetic doc drift only.
openshift.go:59-63 says each OpenShift group 'lives in its own file (openshift_.go)', but all group defs are functions inside the single openshift_defs.go — no per-group files exist. Separately, providers/openshift/ocm/ocm.go:172-173 says DeleteCluster 'transitions to uninstalling and is removed', but the code deletes immediately and never sets State="uninstalling".
Low · docs — docs/services.md not updated for the new managed-OpenShift surfaces (ARO + ROSA/OCM)
docs/services.md:33
The reference doc omits a whole new managed-cluster capability; users/tools reading services.md won't discover ARO/ROSA support. Documentation-completeness only — no runtime effect.
services.md is the hand-maintained provider reference (row 33 already lists Kubernetes eks/aks/gke + shared services/kubernetes/). The PR adds Azure Red Hat OpenShift (Microsoft.RedHatOpenShift) and ROSA/OCM (api.openshift.com cluster-manager) but adds no row/section describing them. coveragegen itself is clean (go generate leaves no diff); this is the non-generated docs half of the definition-of-done.
Low · terraform — No terraform round-trip or compat-matrix entry for OpenShift/ARO (informational)
contrib/terraform/terraform_test.go:1
Wire-shape drift from the real terraform providers would go uncaught. Consistent with existing managed-cluster precedent (no such coverage exists yet), so this is a scope observation, not a regression introduced by this PR.
The terraform harness covers only core resources (aws_vpc/subnet/s3/dynamodb/iam_role/security_group/route tables) and docs/compat/compat.json is the fixed 16-category matrix. Neither AKS/EKS/GKE nor ARO/OpenShift is present, so the ARM shape (Microsoft.RedHatOpenShift/openShiftClusters, ProvisioningState/apiserverProfile/consoleProfile) and OCM REST shape are never validated against the real azurerm_redhat_openshift_cluster / rhcs ROSA providers.
3 participants
Objective
Add full-parity OpenShift (OKD/OCP 4.x) emulation to cloudemu, so
oc,rosa, and Azure/ARO clients operate against the emulator end-to-end. Tracking: #398.What we found
OpenShift is a strict superset of Kubernetes: it adds ~35
*.openshift.ioAPI groups, an OAuth server, an integrated registry, and a build system on top of a normal cluster. The existing Kubernetes emulator already exposes a declarativeresourceDefregistry + always-converged reconcile engine, so most of the surface plugs in as registrations and reconcile hooks rather than hand-written handlers. Wire shapes were captured from a live ROSA cluster (OCP 4.21) to match reality.Blast radius: additive and flavor-gated. A new
FlavoronClusterStatemeans only OpenShift-flavored clusters serve the new groups/singletons; EKS/AKS/GKE clusters are byte-for-byte unchanged. The provisioning handlers register on paths disjoint from existing AWS SDK / Azure ARM routes.How we fixed it
Data plane (
services/kubernetes/openshift_*.go)ClusterVersion/Infrastructureidentity singletons; discovery auto-derives from the registry.*.openshift.iogroups registered as CRUD (apps, route, build, image, project, user, oauth, security, quota, authorization, template, console, operator, machine, autoscaling) withocshort names.status.ingress), ImageStream registry repos, Projectsa.scc.*annotations, DeploymentConfig rollout (pods +latestVersion), Build → completion, and the POST RPCsoc new-project(ProjectRequest),oc start-build(BuildConfig instantiate), andoc process(processedtemplates with parameter generation +${PARAM}substitution).oc login: OAuth server (/.well-known/oauth-authorization-server, authorize with Basic challenge, PKCE token exchange),oc whoami, and permissive authorization reviews.Provisioning
providers/azure/aro+server/azure/aro(Microsoft.RedHatOpenShift/openShiftClusters,listAdminCredentials), wired into the Azure server.providers/openshift/ocm+server/openshift/ocm(OCM REST/api/clusters_mgmt/v1/clusters+ SSO token), registered on the AWS server (ROSA is AWS-hosted; paths are disjoint).ockubeconfig.Alternatives not taken
oc get dc/podsread correctly; noted as a possible fidelity follow-up.rosa --urlpoints at the AWS endpoint.Docs / Test / Playground
oc new-project, Build +oc start-build,oc process, OAuth metadata/challenge/token.openshift_oc_e2e_test.godrives the actualocbinary (oc login+oc whoami) against the emulator — auto-skips where noocis present, so CI stays green.clusterversions/versionthrough the returned kubeconfig, then delete + verify teardown.Test plan
go build ./...— greengo vet ./...on touched packages — greengolangci-lint runon new packages — 0 issuesgo test ./services/kubernetes/ ./providers/azure/aro/ ./server/azure/aro/ ./providers/openshift/ocm/ ./server/openshift/ocm/— greenOC_BIN=<oc> go test ./services/kubernetes/ -run OcLoginE2E— realoc loginpassesRisk & Rollback
Low risk: purely additive and flavor-gated; no change to existing Kubernetes/EKS/AKS/GKE behavior. Rollback is reverting this commit — the new packages are self-contained and the wiring edits are guarded (
if d.OCM != nil,flavor == FlavorOpenShift).Conclusion
Delivers OpenShift data-plane parity, real-
oc-validated login, the S2I app workflow, and both provisioning paths (ARO, ROSA) in one branch.Follow-ups (optional, low usage): server-side
TemplateInstanceobject materialization; ReplicationController fidelity for DeploymentConfig; console/operator singleton default seeding.