🤖 tests: cover watch option decoding over HTTP for workspaces and templates - #119
Merged
Merged
Conversation
Existing watch tests call storage.Watch directly and skip the generic handler's option decoding, WatchList defaulting, and ListOptions validation. Add an in-process HTTP test that sends real watch requests to both coderworkspaces and codertemplates and freezes one expected result per case: - no resourceVersion, and resourceVersion=0: 400 BadRequest (defaulting injects sendInitialEvents=true, which storage rejects) - resourceVersion from a GET of the object: 200 watch stream - sendInitialEvents=false only: 422 Invalid (upstream validation) - sendInitialEvents=false with resourceVersionMatch=NotOlderThan: 400 BadRequest (storage rejects resourceVersionMatch) - sendInitialEvents=true with resourceVersionMatch=NotOlderThan: 400 BadRequest (storage rejects sendInitialEvents=true) - resourceVersionMatch=NotOlderThan only, with or without a resourceVersion: 422 Invalid (upstream validation) The test asserts that the WatchList feature gate is enabled under the production server configuration, so a dependency bump that changes the default fails loudly. The server bootstrap moves into a shared harness helper with t.Cleanup, and the mock Coder server now serves the template version and source zip that template GET needs. Part of #112 _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-opus-5-5` • Thinking: `xhigh`_ Change-Id: I445d78628e2855ee1f5c83e5acf26efbbe9270c5 Signed-off-by: Thomas Kosiewski <tk@coder.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ 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. |
Member
Author
|
@codex security review |
🛡️ 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. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds an HTTP-level test that sends real watch requests through the generic API server path (option decoding, WatchList defaulting, validation, then storage) for both
coderworkspacesandcodertemplates. It freezes the eight-case matrix of supported and rejected watch options. Test-only; no product or docs change.Background
#112 asked for coverage of watch options through the generic request path, not only direct storage calls, for both resources. The existing tests in
internal/aggregated/storage/watch_test.gocallWatchdirectly, so they skip Kubernetes' defaulting. That defaulting turns a watch with noresourceVersion(or0) into an initial-events request that this server rejects. The workspace behaviour was observed on a deployed server during #109; templates were never exercised over HTTP. The supported request form is already documented indocs/how-to/deploy-aggregated-apiserver.md(from #115), and this test confirms it.Implementation
startIntegrationAggregatedAPIServerextracts the existing in-process server setup fromintegration_test.go, with the same cleanup order. The existing bootstrap/list test uses it unchanged.TestIntegrationWatchOptionsThroughGenericAPIServergets a fresh resource version with an HTTP GET, then checks each case for exactly one status, plus the decodedStatusreason, message and (for 422) the single field cause:watch=trueplus)sendInitialEvents=true is not supportedresourceVersion=0resourceVersion=<token from GET>sendInitialEvents=falseresourceVersionMatch=NotOlderThansendInitialEvents=false&resourceVersionMatch=NotOlderThanresourceVersionMatch "NotOlderThan" is not supportedsendInitialEvents=true&resourceVersionMatch=NotOlderThansendInitialEvents=true is not supportedresourceVersionMatch=NotOlderThansendInitialEventsis providedresourceVersion=<token>&resourceVersionMatch=NotOlderThanThe test asserts that the WatchList feature gate is enabled in the normal server configuration (the default since Kubernetes 1.34). It does not force the gate, so a future default change fails loudly instead of silently shifting expectations.
Validation
sendInitialEvents=truerejection makes the three affected cases fail for both resources. Removing both storage checks makes the unexpected 200s fail fast rather than hang.pr112-watch-matrix-live.webm
Risks
Test-only. The 200 case checks for a chunked response, which assumes the harness's HTTP/1.1 client; that would need adjusting if the client moved to HTTP/2.
Part of #112.
Generated with
xum• Model:anthropic:claude-opus-5-5• Thinking:xhigh• Cost:$1413.83