Repository navigation
🤖 feat: activate the CoderTemplateTest controller - #210
Conversation
Register the CoderTemplateTest reconciler with the manager, add its RBAC markers, and ship its CRD in config/crd/bases, the install bundle, and the API reference. The dormant-CRD mechanism and its CI guard are gone, and the Kind E2E now runs on changes to the controller and its API. The controller refuses dormant owners: Coder creates API users as dormant and refuses a dormant owner's agents. A short how-to covers the tester, RBAC, cleanup, and the known limits, including #198. The Kind E2E runs three passing tests (exactly one workspace each), counts the tester's API keys before and after, and deletes the namespace while a test runs and Coder is unreachable. The driver keeps request bodies and the token out of argv, reuses an existing tester on a rerun, and CI retries the agent image pull. Refs #152 Change-Id: Idf51eead4cedefdbd04180c3bfbfe4a5079bef04 Signed-off-by: Thomas Kosiewski <tk@coder.com>
|
@codex security review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
Security findingsAdvisory findings (1)
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
🛡️ Codex Security Review
Here are some automated security review suggestions for this pull request.
Reviewed commit: af897bd3ce
ℹ️ About Codex security reviews in GitHub
This is an experimental Codex feature. Security reviews are triggered when:
- You comment "@codex security review"
- A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review
Once complete, Codex will leave suggestions, or a comment if no findings are found.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: af897bd3ce
ℹ️ 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".
Anyone who creates a CoderTemplateTest can set coder.com/deletion-policy: retain, and RBAC cannot see annotations. Retain now works only when the CoderControlPlane sets spec.templateTests.allowRetain. Otherwise the controller ignores it, says so in the WorkspaceDeleted message, and deletes the workspace. Also drop the stale "controller not enabled" sentence from ownerUserID, keep the template name format out of raw HTML in the API reference, and add the kind to the spell list. Refs #152 Change-Id: Ie668cc34e04dfdbe94bda6623d63b787ea6b2617 Signed-off-by: Thomas Kosiewski <tk@coder.com>
|
@codex review |
|
@codex security review |
|
Codex Review: Didn't find any major issues. 🚀 Reviewed commit: ℹ️ 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 Security ReviewSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
1 participant
Summary
Plan PR 7 of the
CoderTemplateTeststack (Refs #152): activation. The operator now runs theCoderTemplateTestcontroller and installs its CRD. The Kind E2E runs real tests against the agent fixture from #208.Required items
Kind namespace-deletion test (plan A13.2, A1). This is the last driver phase. A test with
startup_delay=120reachesRunning/WaitingForAgents. The driver then setsspec.replicas=0on theCoderControlPlane, so Coder is unreachable while the control plane still exists. The operator keeps that desired state and does not undo it. The test reportsCoderUnavailable, and then the driver deletes namespacecoder. Within 300 s, the test and the control plane must be gone, and the namespace must reportNamespaceContentRemaining=FalseandNamespaceFinalizersRemaining=False. In the local Kind run, this took about 11 s.Terminating. This is a pre-existing aggregated API bug that I filed as 🤖 Namespace deletion hangs: the aggregated API answers LIST with 503 when the namespace has no control plane #209: a namespaced LIST without an eligible control plane answers 503, so the namespace controller never finishes. The driver logs it as a known issue, and its check becomes "the namespace disappears" once 🤖 Namespace deletion hangs: the aggregated API answers LIST with 503 when the namespace has no control plane #209 is fixed.CoderTemplateTestis not involved.How-to notes (
docs/how-to/test-templates.md):ControlPlaneUnavailableorCoderUnavailabletest,RetainedorControlPlaneGonetests,CoderTemplateTestcalls Coder at the internalhttp://service URL, so its operator token crosses the cluster network as plain HTTP. Separately, theCoderProvisionercontroller and the aggregated API server use thehttps://service URL and can fail TLS verification. I did not verify that failure in a live cluster.Dormant-CRD CI guard. Removed, together with the whole dormant mechanism:
config/crd/dormant/,DORMANT_CRDSinhack/update-manifests.sh, the dormant skip inhack/update-reference-docs.sh, the second envtest CRD directory, and the CI drift paths.Tester key count in Kind (plan A4). The driver counts the tester's
api_keysrows with a read-onlySELECT count(*)in the CNPG primary, before and after three tests. As a check that the query works, the operator user's key count must be above 0. Result: 0 before and 0 after. Coder v2.37.2 explains this: every start build creates the owner's workspace session token, and every stop or delete build removes it (coderd/provisionerdserver/provisionerdserver.go:744-756,:3296-3332). Keys do not accumulate, so I filed no follow-up issue. The how-to states this, including that a retained workspace keeps its key.Dormant owners (approved). Coder creates API users as
dormantand refuses a dormant owner's agents with 401. The owner check now allows onlyactiveusers, and the message says to activate the user. Test: thedormant userrow inTestTemplateTestOwnerEligibility.retainneeds the control plane's opt-in (review round 1, plan A17). Anyone who can create a test can setcoder.com/deletion-policy: retainat create time, and RBAC cannot see annotations. The new fieldCoderControlPlane.spec.templateTests.allowRetain(defaultfalse) gates it.retain, names the reason in theWorkspaceDeletedmessage, and runs normal cleanup. It deletes the workspace, or keeps the finalizer while Coder is unreachable.retainworks as before.TestTemplateTestRetain(ignored without the opt-in, then a release with it),TestTemplateTestControlPlaneUnavailable(without the opt-in the finalizer stays while Coder is down, and with it the release needs no Coder call), and theOwnershipUnknownpath. Mutation check: allowingretainwithout the opt-in fails those tests.#208 follow-ups
coder_apisends the token as-H @fileand the body as--data @file, from 0600 files that are removed at exit. No secret appears in argv, and the offline tests check this.docker pull3 times, 10 s apart.e2e-testeruser (409) reuses it after the same checks (password login, default organization) and still activates it. Offline scenarios:tester-existsandtester-exists-oidc.What
internal/app/controllerapp: registersCoderTemplateTestReconcilerwith the real clock.internal/controller/codertemplatetest_controller.go: RBAC markers (codertemplatetestsget/list/watch/update/patch/delete, the status and finalizers subresources), and an updated type comment.config/crd/bases,config/rbac/role.yaml,dist/install.yaml,config/default/kustomization.yaml, anddocs/reference/api/codertemplatetest.md.e2epaths filter addsapi/v1alpha1/**,internal/controller/codertemplatetest*, andinternal/app/controllerapp/**(plan 5.5).ownerUserID.Succeeded,Ready=True,WorkspaceDeleted=TrueDeleted), with a check that the first run created exactly one workspace row, deleted rows included.deploy/coderdid not roll after the owner patch (plan risk R5).Review round 1
retainset at create time bypasses cleanup): fixed with theallowRetainopt-in.ownerUserID): removed. The CRD, the install bundle, and the API reference are regenerated.docs-quality: markdownlint flagged raw HTML from<organization>.<template>in thetemplatefield comment. The comment now puts it in backticks, and.cspell.jsonlistscodertemplatetest(s). Both linters pass locally with CI's versions.Validation
e2e-kindjob including the APIService wrong-CA step after the driver: the driver exited 0 with 22 passed cases in 152 s, and the job took 297 s. The three tests took 23 s, 22 s, and 16 s.make test-scripts, shellcheck, and actionlint pass.make verify-vendor,make test,make test-integration,make build,make lint,make codegen,make manifests,make docs-reference(no diff afterwards),go test -race ./internal/controller/...,make docs-check, and CI'smarkdownlint-cli2andcspellversions.Line count
Hand-written: 17 files changed, 412 insertions(+), 81 deletions(-). Generated: 6 files changed, 450 insertions(+), 6 deletions(-) (CRD, RBAC role, install bundle, API reference).
Generated with
xum• Model:anthropic:claude-opus-5-5• Thinking:high