🤖 feat: replace WorkspaceProxy with CoderWorkspaceProxy#61
Conversation
|
@codex review Please review this PR for correctness and readiness. |
5994604 to
b4c5ec5
Compare
|
@codex review Rebased on latest main to resolve merge conflicts. Please re-review. |
b4c5ec5 to
0a6f43d
Compare
|
@codex review Resolved merge conflict after rebasing onto latest main and revalidated locally. |
|
@codex review Added cspell entries for CoderWorkspaceProxy terms to fix docs-quality CI. Please re-review. |
|
Codex Review: Didn't find any major issues. 🚀 ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
@codex review Moved remaining workspace proxy types/constants into |
There was a problem hiding this comment.
💡 Codex Review
coder-k8s/internal/controller/coderworkspaceproxy_controller.go
Lines 178 to 179 in 5404ec2
Upgrades from builds that created WorkspaceProxy objects can fail reconciliation because child resource names are unchanged (wsproxy-...), so existing Deployments/Services/Secrets may already be present with an ownerRef to the old kind; in that case SetControllerReference returns an AlreadyOwned error and the new CoderWorkspaceProxy never converges. This makes migration to the renamed CRD brittle in clusters with pre-existing proxy resources.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Remove the unreleased WorkspaceProxy legacy kind/controller path and keep CoderWorkspaceProxy as the single workspace-proxy API surface. - add CoderWorkspaceProxy API type, CRD, docs, sample, and controller/tests - remove WorkspaceProxy API roots, CRD, docs, sample, and controller/tests - update controller wiring, RBAC, and scheme tests to reference only the new kind --- _Generated with [`mux`](https://github.com/coder/mux) • Model: `openai:gpt-5.3-codex` • Thinking: `xhigh` • Cost: `$0.62`_ <!-- mux-attribution: model=openai:gpt-5.3-codex thinking=xhigh costs=0.62 -->
The docs-quality check failed on new CoderWorkspaceProxy resource names. Add the new singular/plural words to `.cspell.json` so docs CI passes. --- _Generated with [`mux`](https://github.com/coder/mux) • Model: `openai:gpt-5.3-codex` • Thinking: `xhigh` • Cost: `$0.62`_ <!-- mux-attribution: model=openai:gpt-5.3-codex thinking=xhigh costs=0.62 -->
Move the remaining shared workspace proxy constants/spec/status types from `workspaceproxy_types.go` into `coderworkspaceproxy_types.go` and delete the old file. --- _Generated with [`mux`](https://github.com/coder/mux) • Model: `openai:gpt-5.3-codex` • Thinking: `xhigh` • Cost: `$0.62`_ <!-- mux-attribution: model=openai:gpt-5.3-codex thinking=xhigh costs=0.62 -->
|
@codex review Rebased again on latest main to clear merge conflicts; retained the |
5404ec2 to
fe92f51
Compare
There was a problem hiding this comment.
💡 Codex Review
coder-k8s/internal/controller/coderworkspaceproxy_controller.go
Lines 178 to 179 in fe92f51
If a cluster already has WorkspaceProxy objects from the prior API, their Deployment/Service/token Secret keep controller ownerRefs to that legacy kind. This reconciler still computes the same child names (wsproxy-<name>), so upgrade-time reconciles will hit existing children and SetControllerReference will return an AlreadyOwned error instead of adopting them, leaving CoderWorkspaceProxy stuck and blocking migration for existing proxies.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review Context on the migration concern: this API rename is still pre-release/internal development and For this PR we intentionally removed the legacy path (CRD/controller/docs/sample) and are not supporting in-place migration/adoption from legacy-owned children. Existing dev clusters are expected to recreate these resources if they still have old Given that scope, please re-review for correctness of the no-legacy implementation. |
|
Codex Review: Didn't find any major issues. Bravo. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Summary
Replace the unreleased
WorkspaceProxyCRD/controller withCoderWorkspaceProxyas the single workspace-proxy API incoder.com/v1alpha1.Background
WorkspaceProxywas still under development and had not been released. Keeping both Kinds introduced unnecessary API/controller duplication and naming inconsistency with otherCoder*resources.Implementation
CoderWorkspaceProxyAPI roots and registration.CoderWorkspaceProxyReconcileronly.CoderWorkspaceProxy.WorkspaceProxyroot kind/list, CRD, sample, docs, controller, and controller tests.Validation
make verify-vendormake testmake buildmake lintmake docs-check(run with generated docs staged)Risks
Low-to-moderate: this removes an unreleased API surface and consolidates onto one Kind. Main regression risk is controller behavior drift during the rename, mitigated by preserving reconciliation logic and running full local validation.
📋 Implementation Plan
Plan: Rename
WorkspaceProxy→CoderWorkspaceProxyContext / Why
Today the operator exposes a namespaced CRD Kind
WorkspaceProxyincoder.com/v1alpha1, while most other public CRDs in this repo use aCoder*prefix (e.g.CoderControlPlane,CoderProvisioner,CoderWorkspace,CoderTemplate). This is inconsistent in docs and creates Go naming collisions/confusion with upstream Coder concepts (there is also acodersdk.WorkspaceProxy).Goal: introduce a
CoderWorkspaceProxyCRD and controller as the long-term supported API, with a practical migration path from the existingWorkspaceProxy.Evidence (repo facts)
api/v1alpha1/workspaceproxy_types.go(type WorkspaceProxy struct { ... },+kubebuilder:resource:scope=Namespaced).internal/controller/workspaceproxy_controller.goand is wired ininternal/app/controllerapp/controllerapp.go.config/crd/bases/coder.com_workspaceproxies.yamlapi/v1alpha1/zz_generated.deepcopy.godocs/reference/api/workspaceproxy.mdmkdocs.ymlspec.names.kind/pluralin-place (they are immutable). Therefore a pure “rename” requires either:Recommended approach (non-destructive): new CRD + migration, then remove old
Phase 1 (Release N): Add
CoderWorkspaceProxyalongsideWorkspaceProxy1) Add new API types (new Kind + plural)
api/v1alpha1/coderworkspaceproxy_types.go(or extendworkspaceproxy_types.goif preferred).CoderWorkspaceProxyCoderWorkspaceProxyListWorkspaceProxySpec,WorkspaceProxyStatus,ProxyBootstrapSpec) to avoid duplicating schema and docs.init()or existing one):SchemeBuilder.Register(&CoderWorkspaceProxy{}, &CoderWorkspaceProxyList{})2) Generate new CRD + deepcopy
make codegenmake manifestsconfig/crd/bases/coder.com_coderworkspaceproxies.yamlworkspaceproxies.coder.com.3) Add a new controller for
CoderWorkspaceProxyinternal/controller/coderworkspaceproxy_controller.goCoderWorkspaceProxyReconcilerWorkspaceProxyReconcilerinto helper functions (naming/labels, secret reads, deployment/service reconcile) so the behavior stays identical and future fixes apply to both during the transition.coderworkspaceproxies(+ status/finalizers).4) Handle adoption of existing child resources (important for smooth migration)
Existing
WorkspaceProxyinstances may already own a Deployment/Service/Secret via a controller ownerRef. When a user creates a newCoderWorkspaceProxywith the same name, the new controller should “adopt” those children instead of failing onSetControllerReference.WorkspaceProxy, fail fast (this prevents accidental adoption of unrelated resources).5) Wire controller + keep legacy behavior clear
internal/app/controllerapp/controllerapp.goto set up the new reconciler.WorkspaceProxyReconcilerin the manager (so we don’t have two controllers fighting over the same child resources), but keep the CRD/types temporarily so users can still read/export their legacy objects.DeprecatedCondition onWorkspaceProxyobjects (no child management) to guide users.6) Update/extend tests
internal/controller/workspaceproxy_controller_test.go, or add a parallel file (e.g.internal/controller/coderworkspaceproxy_controller_test.go).WorkspaceProxyand reconcile once to create children.CoderWorkspaceProxywith the same name/namespace.WorkspaceProxy→CoderWorkspaceProxy.Phase 2 (same release N): Update samples + docs to prefer
CoderWorkspaceProxy7) Samples / manifests
config/samples/coder_v1alpha1_coderworkspaceproxy.yaml(KindCoderWorkspaceProxy).8) Docs + docs nav
README.md: replaceWorkspaceProxymentions withCoderWorkspaceProxy.mkdocs.yml: generated API nav entry should becomeCoderWorkspaceProxy: reference/api/coderworkspaceproxy.md.make docs-referencemake docs-checkpasses)Phase 3 (Release N+1): Remove legacy
WorkspaceProxy9) Remove legacy API + CRD
WorkspaceProxyroot types and its CRD.make codegenmake manifestsmake docs-referenceMigration guide (for Release N)
WorkspaceProxynamedX:CoderWorkspaceProxywith the same name/namespace and the same spec.WorkspaceProxyonce stable after adoption completes (otherwise Kubernetes GC may delete the child Deployment/Service/Secret).Alternative (development-only): destructive rename
Because CRD Kind/Plural are immutable, the only way to keep the same REST resource
workspaceproxies.coder.comwhile changing the Kind toCoderWorkspaceProxyis:WorkspaceProxyobjects,This is not recommended for production clusters.
Validation (what to run when implementing)
make testmake test-integration(if controller behavior changes materially)make lintmake buildmake verify-vendor(should be a no-op unless deps changed)make manifests && git diff --exit-code config/(confirm generated output is committed)make docs-reference && make docs-checkGenerated with
mux• Model:openai:gpt-5.3-codex• Thinking:xhigh• Cost:$0.62