Skip to content

Commit cf296a0

Browse files
committed
🤖 fix: enable best-effort SSA create-on-update
Allow aggregated storage Update() to fall back to Create() when forceAllowCreate=true and the target coder template/workspace does not exist. This unblocks SSA create-on-update requests while keeping existing update validation and defensive assertions for normal update flows. Added a prominent FIXME in both storage implementations documenting that this is a compatibility hack and outlining future options, with the preferred long-term fix being first-class metadata support in Coder/codersdk. Also added storage tests for both codertemplates and coderworkspaces to validate create-on-update behavior, and documented the SSA limitation and roadmap in the aggregated API deployment guide. --- _Generated with [`mux`](https://github.com/coder/mux) • Model: `openai:gpt-5.3-codex` • Thinking: `xhigh` • Cost: `$2.58`_ <!-- mux-attribution: model=openai:gpt-5.3-codex thinking=xhigh costs=2.58 -->
1 parent 3608eab commit cf296a0

File tree

4 files changed

+218
-16
lines changed

4 files changed

+218
-16
lines changed

‎docs/how-to/deploy-aggregated-apiserver.md‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,25 @@ kubectl get codertemplates.aggregation.coder.com -A
9090
kubectl logs -n coder-system deploy/coder-k8s
9191
```
9292

93+
## Server-Side Apply (SSA) behavior
94+
95+
`coder-k8s` now includes a compatibility fallback for SSA create-on-update requests
96+
(for example, `kubectl apply --server-side` when the target resource does not exist yet).
97+
98+
- For missing `coderworkspaces` / `codertemplates`, the aggregated API server's `Update`
99+
path can delegate to `Create` when `forceAllowCreate=true`.
100+
- This is intentionally **best-effort**: Coder resources do not currently provide a
101+
first-class metadata store for Kubernetes `metadata.managedFields`, so SSA field-owner
102+
conflict semantics are not durable.
103+
104+
Planned follow-up options (in order of preference):
105+
106+
1. **Preferred:** add first-class metadata support on template/workspace resources in
107+
Coder + `codersdk`, and round-trip Kubernetes-managed metadata there.
108+
2. Persist Kubernetes-only metadata in a shadow Kubernetes resource (ConfigMap/CRD)
109+
managed by the aggregated API server.
110+
3. Keep the compatibility fallback and continue documenting the limitations.
111+
93112
## Template build wait tuning
94113

95114
When updating `CoderTemplate.spec.files`, the aggregated API server now waits for

‎internal/aggregated/storage/storage_test.go‎

Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1477,6 +1477,58 @@ func TestTemplateStorageUpdateAllowsMetadataSpecChanges(t *testing.T) {
14771477
}
14781478
}
14791479

1480+
func TestTemplateStorageUpdateForceAllowCreateCreatesWhenMissing(t *testing.T) {
1481+
t.Parallel()
1482+
1483+
server, state := newMockCoderServer(t)
1484+
defer server.Close()
1485+
1486+
templateStorage := NewTemplateStorage(newTestClientProvider(t, server.URL))
1487+
ctx := namespacedContext("control-plane")
1488+
1489+
desiredTemplate := &aggregationv1alpha1.CoderTemplate{
1490+
Spec: aggregationv1alpha1.CoderTemplateSpec{
1491+
Organization: "acme",
1492+
DisplayName: "SSA-created template",
1493+
Description: "Created via forceAllowCreate update hack",
1494+
Icon: "/icons/ssa-template.png",
1495+
Files: map[string]string{
1496+
"main.tf": "resource \"null_resource\" \"ssa_create\" {}",
1497+
},
1498+
},
1499+
}
1500+
1501+
updatedObj, created, err := templateStorage.Update(
1502+
ctx,
1503+
"acme.ssa-template",
1504+
testUpdatedObjectInfo{obj: desiredTemplate},
1505+
rest.ValidateAllObjectFunc,
1506+
rest.ValidateAllObjectUpdateFunc,
1507+
true,
1508+
nil,
1509+
)
1510+
if err != nil {
1511+
t.Fatalf("expected create-on-update to succeed for missing template: %v", err)
1512+
}
1513+
if !created {
1514+
t.Fatal("expected update created=true when forceAllowCreate creates a template")
1515+
}
1516+
1517+
createdTemplate, ok := updatedObj.(*aggregationv1alpha1.CoderTemplate)
1518+
if !ok {
1519+
t.Fatalf("expected *CoderTemplate from update, got %T", updatedObj)
1520+
}
1521+
if createdTemplate.Name != "acme.ssa-template" {
1522+
t.Fatalf("expected created template name acme.ssa-template, got %q", createdTemplate.Name)
1523+
}
1524+
if createdTemplate.Spec.Organization != "acme" {
1525+
t.Fatalf("expected created template organization acme, got %q", createdTemplate.Spec.Organization)
1526+
}
1527+
if !state.hasTemplate("acme", "ssa-template") {
1528+
t.Fatal("expected create-on-update template to be persisted in mock server state")
1529+
}
1530+
}
1531+
14801532
func TestTemplateStorageUpdateRejectsMissingResourceVersion(t *testing.T) {
14811533
t.Parallel()
14821534

@@ -1846,6 +1898,54 @@ func TestWorkspaceStorageCreateAllowsMatchingTemplateVersionID(t *testing.T) {
18461898
}
18471899
}
18481900

1901+
func TestWorkspaceStorageUpdateForceAllowCreateCreatesWhenMissing(t *testing.T) {
1902+
t.Parallel()
1903+
1904+
server, state := newMockCoderServer(t)
1905+
defer server.Close()
1906+
1907+
workspaceStorage := NewWorkspaceStorage(newTestClientProvider(t, server.URL))
1908+
ctx := namespacedContext("control-plane")
1909+
1910+
desiredWorkspace := &aggregationv1alpha1.CoderWorkspace{
1911+
Spec: aggregationv1alpha1.CoderWorkspaceSpec{
1912+
Organization: "acme",
1913+
TemplateName: "starter-template",
1914+
Running: false,
1915+
},
1916+
}
1917+
1918+
updatedObj, created, err := workspaceStorage.Update(
1919+
ctx,
1920+
"acme.alice.ssa-workspace",
1921+
testUpdatedObjectInfo{obj: desiredWorkspace},
1922+
rest.ValidateAllObjectFunc,
1923+
rest.ValidateAllObjectUpdateFunc,
1924+
true,
1925+
nil,
1926+
)
1927+
if err != nil {
1928+
t.Fatalf("expected create-on-update to succeed for missing workspace: %v", err)
1929+
}
1930+
if !created {
1931+
t.Fatal("expected update created=true when forceAllowCreate creates a workspace")
1932+
}
1933+
1934+
createdWorkspace, ok := updatedObj.(*aggregationv1alpha1.CoderWorkspace)
1935+
if !ok {
1936+
t.Fatalf("expected *CoderWorkspace from update, got %T", updatedObj)
1937+
}
1938+
if createdWorkspace.Name != "acme.alice.ssa-workspace" {
1939+
t.Fatalf("expected created workspace name acme.alice.ssa-workspace, got %q", createdWorkspace.Name)
1940+
}
1941+
if !state.hasWorkspace("alice", "ssa-workspace") {
1942+
t.Fatal("expected create-on-update workspace to be persisted in mock server state")
1943+
}
1944+
if !containsTransition(state.buildTransitionsSnapshot(), codersdk.WorkspaceTransitionStop) {
1945+
t.Fatal("expected create-on-update workspace with spec.running=false to queue stop transition")
1946+
}
1947+
}
1948+
18491949
func TestWorkspaceStorageUpdateRejectsNonRunningSpecChanges(t *testing.T) {
18501950
t.Parallel()
18511951

‎internal/aggregated/storage/template.go‎

Lines changed: 49 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -415,7 +415,7 @@ func (s *TemplateStorage) Update(
415415
ctx context.Context,
416416
name string,
417417
objInfo rest.UpdatedObjectInfo,
418-
_ rest.ValidateObjectFunc,
418+
createValidation rest.ValidateObjectFunc,
419419
updateValidation rest.ValidateObjectUpdateFunc,
420420
forceAllowCreate bool,
421421
_ *metav1.UpdateOptions,
@@ -435,16 +435,57 @@ func (s *TemplateStorage) Update(
435435
if s.broadcaster == nil {
436436
return nil, false, fmt.Errorf("assertion failed: template broadcaster must not be nil")
437437
}
438-
if forceAllowCreate {
439-
return nil, false, apierrors.NewMethodNotSupported(
440-
aggregationv1alpha1.Resource("codertemplates"),
441-
"create on update",
442-
)
443-
}
444438

445439
currentObj, err := s.Get(ctx, name, nil)
446440
if err != nil {
447-
return nil, false, err
441+
if !forceAllowCreate || !apierrors.IsNotFound(err) {
442+
return nil, false, err
443+
}
444+
445+
// FIXME: This branch intentionally enables best-effort SSA create-on-update
446+
// semantics for codertemplates.
447+
//
448+
// The aggregated API is a stateless proxy to Coder and currently does not
449+
// persist Kubernetes metadata.managedFields (or other Kubernetes-only metadata),
450+
// so SSA field ownership/conflict semantics are not durable.
451+
//
452+
// Future options to remove this hack:
453+
// 1. Preferred: add first-class metadata support on templates/workspaces in
454+
// Coder + codersdk and round-trip Kubernetes-managed metadata there.
455+
// 2. Persist Kubernetes-only metadata in a shadow Kubernetes resource
456+
// (ConfigMap/CRD) managed by this aggregated API server.
457+
// 3. Keep this fallback and continue documenting SSA limitations.
458+
createObj, objInfoErr := objInfo.UpdatedObject(ctx, s.New())
459+
if objInfoErr != nil {
460+
return nil, false, objInfoErr
461+
}
462+
if createObj == nil {
463+
return nil, false, fmt.Errorf("assertion failed: create-on-update template object must not be nil")
464+
}
465+
466+
createTemplate, ok := createObj.(*aggregationv1alpha1.CoderTemplate)
467+
if !ok {
468+
return nil, false, fmt.Errorf("assertion failed: expected *CoderTemplate, got %T", createObj)
469+
}
470+
createTemplate = createTemplate.DeepCopy()
471+
if createTemplate == nil {
472+
return nil, false, fmt.Errorf("assertion failed: create-on-update template deep copy must not be nil")
473+
}
474+
if createTemplate.Name == "" {
475+
createTemplate.Name = name
476+
}
477+
if createTemplate.Name != name {
478+
return nil, false, apierrors.NewBadRequest(
479+
fmt.Sprintf("updated object metadata.name %q must match request name %q", createTemplate.Name, name),
480+
)
481+
}
482+
483+
createdObj, createErr := s.Create(ctx, createTemplate, createValidation, nil)
484+
if createErr != nil {
485+
return nil, false, createErr
486+
}
487+
488+
return createdObj, true, nil
448489
}
449490

450491
currentObjForUpdate := currentObj.DeepCopyObject()

‎internal/aggregated/storage/workspace.go‎

Lines changed: 50 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -397,7 +397,7 @@ func (s *WorkspaceStorage) Update(
397397
ctx context.Context,
398398
name string,
399399
objInfo rest.UpdatedObjectInfo,
400-
_ rest.ValidateObjectFunc,
400+
createValidation rest.ValidateObjectFunc,
401401
updateValidation rest.ValidateObjectUpdateFunc,
402402
forceAllowCreate bool,
403403
_ *metav1.UpdateOptions,
@@ -417,12 +417,6 @@ func (s *WorkspaceStorage) Update(
417417
if s.broadcaster == nil {
418418
return nil, false, fmt.Errorf("assertion failed: workspace broadcaster must not be nil")
419419
}
420-
if forceAllowCreate {
421-
return nil, false, apierrors.NewMethodNotSupported(
422-
aggregationv1alpha1.Resource("coderworkspaces"),
423-
"create on update",
424-
)
425-
}
426420

427421
namespace, badNamespaceErr := requiredNamespaceFromRequestContext(ctx)
428422
if badNamespaceErr != nil {
@@ -441,7 +435,55 @@ func (s *WorkspaceStorage) Update(
441435

442436
currentWorkspace, err := sdk.WorkspaceByOwnerAndName(ctx, userName, workspaceName, codersdk.WorkspaceOptions{})
443437
if err != nil {
444-
return nil, false, coder.MapCoderError(err, aggregationv1alpha1.Resource("coderworkspaces"), name)
438+
mappedErr := coder.MapCoderError(err, aggregationv1alpha1.Resource("coderworkspaces"), name)
439+
if !forceAllowCreate || !apierrors.IsNotFound(mappedErr) {
440+
return nil, false, mappedErr
441+
}
442+
443+
// FIXME: This branch intentionally enables best-effort SSA create-on-update
444+
// semantics for coderworkspaces.
445+
//
446+
// The aggregated API is a stateless proxy to Coder and currently does not
447+
// persist Kubernetes metadata.managedFields (or other Kubernetes-only metadata),
448+
// so SSA field ownership/conflict semantics are not durable.
449+
//
450+
// Future options to remove this hack:
451+
// 1. Preferred: add first-class metadata support on templates/workspaces in
452+
// Coder + codersdk and round-trip Kubernetes-managed metadata there.
453+
// 2. Persist Kubernetes-only metadata in a shadow Kubernetes resource
454+
// (ConfigMap/CRD) managed by this aggregated API server.
455+
// 3. Keep this fallback and continue documenting SSA limitations.
456+
createObj, objInfoErr := objInfo.UpdatedObject(ctx, s.New())
457+
if objInfoErr != nil {
458+
return nil, false, objInfoErr
459+
}
460+
if createObj == nil {
461+
return nil, false, fmt.Errorf("assertion failed: create-on-update workspace object must not be nil")
462+
}
463+
464+
createWorkspace, ok := createObj.(*aggregationv1alpha1.CoderWorkspace)
465+
if !ok {
466+
return nil, false, fmt.Errorf("assertion failed: expected *CoderWorkspace, got %T", createObj)
467+
}
468+
createWorkspace = createWorkspace.DeepCopy()
469+
if createWorkspace == nil {
470+
return nil, false, fmt.Errorf("assertion failed: create-on-update workspace deep copy must not be nil")
471+
}
472+
if createWorkspace.Name == "" {
473+
createWorkspace.Name = name
474+
}
475+
if createWorkspace.Name != name {
476+
return nil, false, apierrors.NewBadRequest(
477+
fmt.Sprintf("updated object metadata.name %q must match request name %q", createWorkspace.Name, name),
478+
)
479+
}
480+
481+
createdObj, createErr := s.Create(ctx, createWorkspace, createValidation, nil)
482+
if createErr != nil {
483+
return nil, false, createErr
484+
}
485+
486+
return createdObj, true, nil
445487
}
446488
if currentWorkspace.OrganizationName != orgName {
447489
return nil, false, apierrors.NewNotFound(aggregationv1alpha1.Resource("coderworkspaces"), name)

0 commit comments

Comments
 (0)