🤖 feat: add WATCH support to aggregated API server storage#72
Conversation
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0171590c9c
ℹ️ 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 Addressed P2 feedback: workspace delete watch event now populates |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4f807a7aee
ℹ️ 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 Implemented the requested watch-list handling change by explicitly rejecting unsupported watch-list options with BadRequest:
Applied to both WorkspaceStorage.Watch and TemplateStorage.Watch, and added focused unit tests for these paths. |
25c5bfc to
cf9327b
Compare
|
@codex review Rebased onto current |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cf9327bc76
ℹ️ 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 Addressed the new P1:
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d70ee1d64d
ℹ️ 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 Addressed the latest P1 by reverting legacy watch-list acceptance:
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: faf95ad4c4
ℹ️ 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 Addressed both latest P1 threads:
Also updated watch tests to cover these behaviors (resourceVersion rejection and legacy/defaulted watch-list rejection expectations). |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7738fe2d38
ℹ️ 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 Addressed both latest P1 findings with a compromise that preserves correctness without blocking writes:
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dc07eba7e2
ℹ️ 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 Addressed the latest P1s:
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 430048757f
ℹ️ 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 Addressed the latest DoS concern:
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ff88b2d274
ℹ️ 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 Applied the requested broadcaster-mode change:
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9b13ec5d96
ℹ️ 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 Addressed the latest watch stopper leak concern:
|
|
Codex Review: Didn't find any major issues. Breezy! ℹ️ 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
Add WATCH support (
rest.Watcher) to the aggregated API server's codersdk-backed storage forcoderworkspacesandcodertemplatesresources, enablingkubectl get --watch, informers, and other standard Kubernetes watch clients.Background
The aggregated API server (
aggregation.coder.com/v1alpha1) previously served CoderWorkspace and CoderTemplate resources through custom REST storage backed by codersdk, but did not implementrest.Watcher. This meant clients could not use?watch=true— a fundamental Kubernetes API capability used bykubectl --watch, controller informers, and client-go watch helpers.This is a best-effort watch implementation: events are emitted when mutations pass through this API server. Out-of-band Coder changes (e.g., builds completing, changes via the Coder UI) are not reflected until a future enhancement adds background polling or bridges codersdk watch streams.
Implementation
New shared helpers (
internal/aggregated/storage/watch.go)filterForListOptions(requestNamespace, opts)— builds awatch.FilterFuncthat applies namespace scoping, label selector, and field selector filteringvalidateFieldSelector(sel)— rejects unsupported fields; allowsmetadata.nameandmetadata.namespacewatchBroadcasterQueueLenconstant (100 events)Storage changes (
workspace.go,template.go)*watch.Broadcasterandsync.Onceto each storage structwatch.DropIfChannelFullpolicyDestroy()now shuts down broadcaster (idempotent viasync.Once)Watch(ctx, opts)implementingrest.Watcher:_ rest.Watcher = (*Storage)(nil)assertionsAddedon Create,Modifiedon Update (non-no-op only),Modifiedon Delete (async deletion)Addedon Create,Modifiedon Update,Deletedon Delete (synchronous)Tests (
internal/aggregated/storage/watch_test.go)TestTemplateStorageWatch_AddedModifiedDeleted— full lifecycleTestWorkspaceStorageWatch_AddedModified— create + update toggleTestWatchRespectsFieldSelectorMetadataName— field selector filteringTestWatchStopsOnContextCancel— cleanup on disconnectTestValidateFieldSelector— shared helper unit testsTestFilterForListOptions— filter construction unit testsValidation
make build✅make test✅make lint✅make verify-vendor✅Risks
📋 Implementation Plan
Plan: Add WATCH support to aggregated API server storage
Context / Why
The aggregated API server currently serves
aggregation.coder.com/v1alpha1resources (coderworkspaces,codertemplates) via custom, codersdk-backed REST storage.Today those storage implementations do not implement
k8s.io/apiserver/pkg/registry/rest.Watcher, so Kubernetes clients cannot do?watch=true(e.g.kubectl get … --watch, informers).Goal: implement a best-effort WATCH endpoint by:
rest.Watcheron both storages.CREATE/UPDATE/DELETEoperations.Evidence (what I verified)
internal/aggregated/storage/workspace.goandinternal/aggregated/storage/template.goimplement CRUD and list/get, but do not implementrest.Watcher.internal/app/apiserverapp/apiserverapp.go(VersionedResourcesStorageMapmapscoderworkspaces/codertemplatestostorage.New*Storage).vendor/k8s.io/apiserver/pkg/registry/rest/rest.godefinestype Watcher interface { Watch(ctx, *internalversion.ListOptions) (watch.Interface, error) }.vendor/k8s.io/apimachinery/pkg/watch/mux.goprovideswatch.Broadcaster.vendor/k8s.io/apimachinery/pkg/watch/filter.goprovideswatch.Filter.Implementation details
1) Add a watch broadcaster to each storage and make
Destroy()shut it downFiles:
internal/aggregated/storage/workspace.gointernal/aggregated/storage/template.goChanges:
broadcaster *watch.BroadcasterbroadcasterShutdown sync.Once(soDestroy()can be called multiple times)watch.NewBroadcaster(<queueLen>, watch.DropIfChannelFull)DropIfChannelFullso slow/abandoned watchers can’t stall API requests.Destroy():broadcasterShutdown.Do(func(){ broadcaster.Shutdown() })Also update interface assertions at the top of each file:
2) Implement
Watch(ctx, options)for both storage typesFiles:
internal/aggregated/storage/workspace.gointernal/aggregated/storage/template.gointernal/aggregated/storage/watch.go(shared helpers)High-level behavior:
watch.Filter.Suggested helper shape (shared):
Watch method pseudo-code:
Selector behavior details:
LabelSelector:ObjectMeta.Labels.FieldSelector:BadRequestfor unsupported fields.metadata.namemetadata.namespacemetadata.uidfields.Set{...}+sel.Matches(...).metadata.namespacediffers.--all-namespaces), do not apply a namespace filter.3) Emit watch events from successful mutations
Files:
internal/aggregated/storage/workspace.gointernal/aggregated/storage/template.goPattern:
watch.AddedafterCreatewatch.ModifiedafterUpdatewatch.Deletedafter templateDeleteTemplate delete:
TemplateByName). Convert that to k8s once and reuse it for:DeletedeventWorkspace delete (special):
Deleteis asynchronous in Coder and returnsdeleted=false.Deletedwatch event.workspacestruct likeUpdate()does, and emit aModifiedevent:Important: do not fail the API request if broadcasting fails (broadcaster may be shutting down). Treat broadcaster errors as best-effort.
4) Add focused unit tests for watch behavior
Files:
internal/aggregated/storage/watch_test.go(preferred) or extendinternal/aggregated/storage/storage_test.go.Tests to add:
TestTemplateStorageWatchEmitsAddedModifiedDeletedTestWorkspaceStorageWatchEmitsAddedModifiedAddedthenModifiedevents.TestWatchRespectsFieldSelectorMetadataNamefields.OneTermEqualSelector("metadata.name", "acme.ops-template").TestWatchStopsOnContextCancelUse existing helpers:
newMockCoderServer(t)newTestClientProvider(t, server.URL)namespacedContext("control-plane")5) Validation
Run (in Exec mode):
make testmake lintandmake buildFuture enhancements (intentionally out of MVP scope)
codersdk.Client.WatchWorkspace(ctx, id)for per-object watches.sendInitialEvents=true) in some configurations.Broadcaster.WatchWithPrefix.Generated with
mux• Model:anthropic:claude-opus-4-6• Thinking:xhigh• Cost:$2.27