🤖 fix: enable best-effort SSA create-on-update fallback (#82)

ThomasK33 · web-flow · commit 515a7e4a1736 · 2026-02-13T17:58:04.000+01:00
## Summary Enable a best-effort create-on-update fallback in aggregated storage for `coderworkspaces` and `codertemplates` when Kubernetes calls `Update` with `forceAllowCreate=true` (SSA apply create path). ## Background Our aggregated API is a stateless proxy to Coder. SSA (`kubectl apply --server-side`) uses PATCH apply semantics that route through update with create-on-missing behavior. We previously rejected that path with `405 Method Not Allowed`, which blocked SSA users from creating resources via server-side apply. ## Implementation - Updated `TemplateStorage.Update` and `WorkspaceStorage.Update` to: - detect `NotFound` on existing object lookup when `forceAllowCreate=true` - materialize a create candidate via `objInfo.UpdatedObject(ctx, s.New())` - enforce defensive assertions/name checks - delegate to `Create(...)` and return `created=true` - Added prominent `FIXME` comments in both storage implementations documenting this as a compatibility hack and listing future options: 1. **Preferred**: add first-class metadata support in Coder/codersdk for durable k8s metadata round-tripping 2. shadow k8s metadata in ConfigMap/CRD side storage 3. keep fallback + docs - Added tests: - `TestTemplateStorageUpdateForceAllowCreateCreatesWhenMissing` - `TestWorkspaceStorageUpdateForceAllowCreateCreatesWhenMissing` - Updated docs (`docs/how-to/deploy-aggregated-apiserver.md`) with SSA behavior and limitations. ## Validation - `make verify-vendor` - `go test ./internal/aggregated/storage -count=1` - `make test` - `make build` - `make lint` - `make docs-check` ## Risks - This is intentionally best-effort SSA support; durable field ownership/conflict semantics remain limited because `metadata.managedFields` is not persisted in the Coder backend. - Risk is constrained to create-on-update path for aggregated resources; existing update validation semantics remain intact. --- _Generated with [`mux`](https://github.com/coder/mux) • Model: `openai:gpt-5.3-codex` • Thinking: `xhigh` • Cost: `$2.58`_
diff --git a/docs/how-to/deploy-aggregated-apiserver.md b/docs/how-to/deploy-aggregated-apiserver.md
@@ -90,6 +90,25 @@ kubectl get codertemplates.aggregation.coder.com -A
 kubectl logs -n coder-system deploy/coder-k8s
 ```
 
+## Server-Side Apply (SSA) behavior
+
+`coder-k8s` now includes a compatibility fallback for SSA create-on-update requests
+(for example, `kubectl apply --server-side` when the target resource does not exist yet).
+
+- For missing `coderworkspaces` / `codertemplates`, the aggregated API server's `Update`
+  path can delegate to `Create` when `forceAllowCreate=true`.
+- This is intentionally **best-effort**: Coder resources do not currently provide a
+  first-class metadata store for Kubernetes `metadata.managedFields`, so SSA field-owner
+  conflict semantics are not durable.
+
+Planned follow-up options (in order of preference):
+
+1. **Preferred:** add first-class metadata support on template/workspace resources in
+   Coder + `codersdk`, and round-trip Kubernetes-managed metadata there.
+2. Persist Kubernetes-only metadata in a shadow Kubernetes resource (ConfigMap/CRD)
+   managed by the aggregated API server.
+3. Keep the compatibility fallback and continue documenting the limitations.
+
 ## Template build wait tuning
 
 When updating `CoderTemplate.spec.files`, the aggregated API server now waits for
diff --git a/internal/aggregated/storage/storage_test.go b/internal/aggregated/storage/storage_test.go
@@ -1477,6 +1477,58 @@ func TestTemplateStorageUpdateAllowsMetadataSpecChanges(t *testing.T) {
 	}
 }
 
+func TestTemplateStorageUpdateForceAllowCreateCreatesWhenMissing(t *testing.T) {
+	t.Parallel()
+
+	server, state := newMockCoderServer(t)
+	defer server.Close()
+
+	templateStorage := NewTemplateStorage(newTestClientProvider(t, server.URL))
+	ctx := namespacedContext("control-plane")
+
+	desiredTemplate := &aggregationv1alpha1.CoderTemplate{
+		Spec: aggregationv1alpha1.CoderTemplateSpec{
+			Organization: "acme",
+			DisplayName:  "SSA-created template",
+			Description:  "Created via forceAllowCreate update hack",
+			Icon:         "/icons/ssa-template.png",
+			Files: map[string]string{
+				"main.tf": "resource \"null_resource\" \"ssa_create\" {}",
+			},
+		},
+	}
+
+	updatedObj, created, err := templateStorage.Update(
+		ctx,
+		"acme.ssa-template",
+		testUpdatedObjectInfo{obj: desiredTemplate},
+		rest.ValidateAllObjectFunc,
+		rest.ValidateAllObjectUpdateFunc,
+		true,
+		nil,
+	)
+	if err != nil {
+		t.Fatalf("expected create-on-update to succeed for missing template: %v", err)
+	}
+	if !created {
+		t.Fatal("expected update created=true when forceAllowCreate creates a template")
+	}
+
+	createdTemplate, ok := updatedObj.(*aggregationv1alpha1.CoderTemplate)
+	if !ok {
+		t.Fatalf("expected *CoderTemplate from update, got %T", updatedObj)
+	}
+	if createdTemplate.Name != "acme.ssa-template" {
+		t.Fatalf("expected created template name acme.ssa-template, got %q", createdTemplate.Name)
+	}
+	if createdTemplate.Spec.Organization != "acme" {
+		t.Fatalf("expected created template organization acme, got %q", createdTemplate.Spec.Organization)
+	}
+	if !state.hasTemplate("acme", "ssa-template") {
+		t.Fatal("expected create-on-update template to be persisted in mock server state")
+	}
+}
+
 func TestTemplateStorageUpdateRejectsMissingResourceVersion(t *testing.T) {
 	t.Parallel()
 
@@ -1846,6 +1898,54 @@ func TestWorkspaceStorageCreateAllowsMatchingTemplateVersionID(t *testing.T) {
 	}
 }
 
+func TestWorkspaceStorageUpdateForceAllowCreateCreatesWhenMissing(t *testing.T) {
+	t.Parallel()
+
+	server, state := newMockCoderServer(t)
+	defer server.Close()
+
+	workspaceStorage := NewWorkspaceStorage(newTestClientProvider(t, server.URL))
+	ctx := namespacedContext("control-plane")
+
+	desiredWorkspace := &aggregationv1alpha1.CoderWorkspace{
+		Spec: aggregationv1alpha1.CoderWorkspaceSpec{
+			Organization: "acme",
+			TemplateName: "starter-template",
+			Running:      false,
+		},
+	}
+
+	updatedObj, created, err := workspaceStorage.Update(
+		ctx,
+		"acme.alice.ssa-workspace",
+		testUpdatedObjectInfo{obj: desiredWorkspace},
+		rest.ValidateAllObjectFunc,
+		rest.ValidateAllObjectUpdateFunc,
+		true,
+		nil,
+	)
+	if err != nil {
+		t.Fatalf("expected create-on-update to succeed for missing workspace: %v", err)
+	}
+	if !created {
+		t.Fatal("expected update created=true when forceAllowCreate creates a workspace")
+	}
+
+	createdWorkspace, ok := updatedObj.(*aggregationv1alpha1.CoderWorkspace)
+	if !ok {
+		t.Fatalf("expected *CoderWorkspace from update, got %T", updatedObj)
+	}
+	if createdWorkspace.Name != "acme.alice.ssa-workspace" {
+		t.Fatalf("expected created workspace name acme.alice.ssa-workspace, got %q", createdWorkspace.Name)
+	}
+	if !state.hasWorkspace("alice", "ssa-workspace") {
+		t.Fatal("expected create-on-update workspace to be persisted in mock server state")
+	}
+	if !containsTransition(state.buildTransitionsSnapshot(), codersdk.WorkspaceTransitionStop) {
+		t.Fatal("expected create-on-update workspace with spec.running=false to queue stop transition")
+	}
+}
+
 func TestWorkspaceStorageUpdateRejectsNonRunningSpecChanges(t *testing.T) {
 	t.Parallel()
 
diff --git a/internal/aggregated/storage/template.go b/internal/aggregated/storage/template.go
@@ -415,7 +415,7 @@ func (s *TemplateStorage) Update(
 	ctx context.Context,
 	name string,
 	objInfo rest.UpdatedObjectInfo,
-	_ rest.ValidateObjectFunc,
+	createValidation rest.ValidateObjectFunc,
 	updateValidation rest.ValidateObjectUpdateFunc,
 	forceAllowCreate bool,
 	_ *metav1.UpdateOptions,
@@ -435,16 +435,57 @@ func (s *TemplateStorage) Update(
 	if s.broadcaster == nil {
 		return nil, false, fmt.Errorf("assertion failed: template broadcaster must not be nil")
 	}
-	if forceAllowCreate {
-		return nil, false, apierrors.NewMethodNotSupported(
-			aggregationv1alpha1.Resource("codertemplates"),
-			"create on update",
-		)
-	}
 
 	currentObj, err := s.Get(ctx, name, nil)
 	if err != nil {
-		return nil, false, err
+		if !forceAllowCreate || !apierrors.IsNotFound(err) {
+			return nil, false, err
+		}
+
+		// FIXME: This branch intentionally enables best-effort SSA create-on-update
+		// semantics for codertemplates.
+		//
+		// The aggregated API is a stateless proxy to Coder and currently does not
+		// persist Kubernetes metadata.managedFields (or other Kubernetes-only metadata),
+		// so SSA field ownership/conflict semantics are not durable.
+		//
+		// Future options to remove this hack:
+		//  1. Preferred: add first-class metadata support on templates/workspaces in
+		//     Coder + codersdk and round-trip Kubernetes-managed metadata there.
+		//  2. Persist Kubernetes-only metadata in a shadow Kubernetes resource
+		//     (ConfigMap/CRD) managed by this aggregated API server.
+		//  3. Keep this fallback and continue documenting SSA limitations.
+		createObj, objInfoErr := objInfo.UpdatedObject(ctx, s.New())
+		if objInfoErr != nil {
+			return nil, false, objInfoErr
+		}
+		if createObj == nil {
+			return nil, false, fmt.Errorf("assertion failed: create-on-update template object must not be nil")
+		}
+
+		createTemplate, ok := createObj.(*aggregationv1alpha1.CoderTemplate)
+		if !ok {
+			return nil, false, fmt.Errorf("assertion failed: expected *CoderTemplate, got %T", createObj)
+		}
+		createTemplate = createTemplate.DeepCopy()
+		if createTemplate == nil {
+			return nil, false, fmt.Errorf("assertion failed: create-on-update template deep copy must not be nil")
+		}
+		if createTemplate.Name == "" {
+			createTemplate.Name = name
+		}
+		if createTemplate.Name != name {
+			return nil, false, apierrors.NewBadRequest(
+				fmt.Sprintf("updated object metadata.name %q must match request name %q", createTemplate.Name, name),
+			)
+		}
+
+		createdObj, createErr := s.Create(ctx, createTemplate, createValidation, nil)
+		if createErr != nil {
+			return nil, false, createErr
+		}
+
+		return createdObj, true, nil
 	}
 
 	currentObjForUpdate := currentObj.DeepCopyObject()
diff --git a/internal/aggregated/storage/workspace.go b/internal/aggregated/storage/workspace.go
@@ -397,7 +397,7 @@ func (s *WorkspaceStorage) Update(
 	ctx context.Context,
 	name string,
 	objInfo rest.UpdatedObjectInfo,
-	_ rest.ValidateObjectFunc,
+	createValidation rest.ValidateObjectFunc,
 	updateValidation rest.ValidateObjectUpdateFunc,
 	forceAllowCreate bool,
 	_ *metav1.UpdateOptions,
@@ -417,12 +417,6 @@ func (s *WorkspaceStorage) Update(
 	if s.broadcaster == nil {
 		return nil, false, fmt.Errorf("assertion failed: workspace broadcaster must not be nil")
 	}
-	if forceAllowCreate {
-		return nil, false, apierrors.NewMethodNotSupported(
-			aggregationv1alpha1.Resource("coderworkspaces"),
-			"create on update",
-		)
-	}
 
 	namespace, badNamespaceErr := requiredNamespaceFromRequestContext(ctx)
 	if badNamespaceErr != nil {
@@ -441,7 +435,55 @@ func (s *WorkspaceStorage) Update(
 
 	currentWorkspace, err := sdk.WorkspaceByOwnerAndName(ctx, userName, workspaceName, codersdk.WorkspaceOptions{})
 	if err != nil {
-		return nil, false, coder.MapCoderError(err, aggregationv1alpha1.Resource("coderworkspaces"), name)
+		mappedErr := coder.MapCoderError(err, aggregationv1alpha1.Resource("coderworkspaces"), name)
+		if !forceAllowCreate || !apierrors.IsNotFound(mappedErr) {
+			return nil, false, mappedErr
+		}
+
+		// FIXME: This branch intentionally enables best-effort SSA create-on-update
+		// semantics for coderworkspaces.
+		//
+		// The aggregated API is a stateless proxy to Coder and currently does not
+		// persist Kubernetes metadata.managedFields (or other Kubernetes-only metadata),
+		// so SSA field ownership/conflict semantics are not durable.
+		//
+		// Future options to remove this hack:
+		//  1. Preferred: add first-class metadata support on templates/workspaces in
+		//     Coder + codersdk and round-trip Kubernetes-managed metadata there.
+		//  2. Persist Kubernetes-only metadata in a shadow Kubernetes resource
+		//     (ConfigMap/CRD) managed by this aggregated API server.
+		//  3. Keep this fallback and continue documenting SSA limitations.
+		createObj, objInfoErr := objInfo.UpdatedObject(ctx, s.New())
+		if objInfoErr != nil {
+			return nil, false, objInfoErr
+		}
+		if createObj == nil {
+			return nil, false, fmt.Errorf("assertion failed: create-on-update workspace object must not be nil")
+		}
+
+		createWorkspace, ok := createObj.(*aggregationv1alpha1.CoderWorkspace)
+		if !ok {
+			return nil, false, fmt.Errorf("assertion failed: expected *CoderWorkspace, got %T", createObj)
+		}
+		createWorkspace = createWorkspace.DeepCopy()
+		if createWorkspace == nil {
+			return nil, false, fmt.Errorf("assertion failed: create-on-update workspace deep copy must not be nil")
+		}
+		if createWorkspace.Name == "" {
+			createWorkspace.Name = name
+		}
+		if createWorkspace.Name != name {
+			return nil, false, apierrors.NewBadRequest(
+				fmt.Sprintf("updated object metadata.name %q must match request name %q", createWorkspace.Name, name),
+			)
+		}
+
+		createdObj, createErr := s.Create(ctx, createWorkspace, createValidation, nil)
+		if createErr != nil {
+			return nil, false, createErr
+		}
+
+		return createdObj, true, nil
 	}
 	if currentWorkspace.OrganizationName != orgName {
 		return nil, false, apierrors.NewNotFound(aggregationv1alpha1.Resource("coderworkspaces"), name)
