🤖 fix: trust only Coder's own 404 in the CoderTemplateTest and pool member workspace lookups, and refresh the AI provider how-to

docs/how-to/manage-ai-providers.md

@@ -10,7 +10,7 @@ A `CoderAIProvider` manages one AI provider in Coder (Coder's `/api/v2/ai/provid
 - A `CoderControlPlane` in the same namespace with `status.operatorAccessReady: true`. The operator user must be a site owner. Coder answers `403` to anyone else on the provider routes.
 - An API key for the provider, for example an OpenAI key.
 - Coder v2.37.2 or later. The Kind end-to-end test runs Coder v2.37.2, and the operator is built against the v2.37.3 SDK. Coder has the provider API since v2.34.0, but versions before v2.37.2 are not tested.
-- An operator release after v0.1.3. The `dist/install.yaml` on `main` installs the `CoderAIProvider` CRD, but its `ghcr.io/coder/coder-k8s:latest` image is v0.1.3 until the next release, and v0.1.3 has no `CoderAIProvider` controller. Install from a release tag that includes `CoderAIProvider`, and pin the image to the same version (see [Deploy a Coder Control Plane](../tutorials/getting-started.md#1-install-the-operator)).
+- coder-k8s v0.1.4 or later. Install from the release tag, and pin the image to the same version (see [Deploy a Coder Control Plane](../tutorials/getting-started.md#1-install-the-operator)).
 
 ## 1. Prepare the key Secret
 
@@ -76,7 +76,7 @@ The object has the conditions `Ready`, `Reconciling` and `Stalled`. A stalled ob
 | `CredentialSecretNotFound`, `CredentialNotOptedIn`, `CredentialDestinationNotAllowed`, `CredentialRefused`, `CredentialKeyMissing`, `CredentialMalformed` | The key Secret fails the credential contract. See [the credential kill switch](#the-credential-kill-switch). |
 | `NameConflict` | Coder has a provider with this name that the operator did not create. |
 | `NameReservedByEnv` | The control plane can seed a provider with this name. See [Seeding limits](#seeding-limits). |
-| `CoderRejected` | Coder rejected the create request. Fix the spec. |
+| `CoderRejected` | Coder rejected the create or update request. Fix the spec. |
 | `DisabledExternally`, `KeyChangedExternally`, `DeletedExternally` | Someone disabled the provider, changed its key or deleted it in Coder. See [Kill switches](#kill-switches). |
 
 ## Kill switches

internal/controller/codertemplatetest_confirm.go

@@ -61,7 +61,8 @@ func (r *CoderTemplateTestReconciler) confirmCreate(
 	// a deleted one only without a live one (workspaceByOwnerAndName).
 	workspace, err := sdk.WorkspaceByOwnerAndName(ctx, tt.Status.OwnerID, tt.Status.WorkspaceName, codersdk.WorkspaceOptions{IncludeDeleted: true})
 	found := err == nil
-	if !found && !isCoderNotFound(err) {
+	// Only Coder's own 404 proves that no workspace exists (#351).
+	if !found && !isCoderResourceNotFound(err) {
 		return coderUnavailable("get workspace by name", err), nil
 	}
 	live := false

internal/controller/codertemplatetest_confirm_test.go

@@ -156,6 +156,26 @@ func TestTemplateTestConfirmSettleWindow(t *testing.T) {
 	require.Equal(t, 1, e.fake.requestCount(routeCreateWorkspace), "never a second create request")
 }
 
+// Only Coder's own 404 proves that no workspace exists. A proxy's 404 page,
+// or "Route not found." for a wrong base URL, keeps the marker and waits,
+// also after the settle window (#351).
+func TestTemplateTestConfirmTrustsOnlyCodersNotFound(t *testing.T) {
+	t.Parallel()
+	e := newTemplateTestEnv(t)
+	for _, tc := range nonCoderNotFounds {
+		t.Run(tc.name, func(t *testing.T) { // Not parallel: the cases share e and its clock.
+			creates := e.fake.requestCount(routeCreateWorkspace)
+			key := e.uncertainCreate(t, fakeFault{Status: 502}, 3600)
+			e.clock.SetTime(e.clock.Now().Add(15 * time.Minute))
+			e.fake.failNext(routeWorkspaceByName, tc.fault)
+			requireTemplateTestRunning(t, e.reconcile(t, key, 1), "CoderUnavailable", "get workspace by name: Coder answered 404")
+			// Coder's own 404 after the window proves that no workspace exists.
+			requireTemplateTestFailed(t, e.settle(t, key), "CreateOutcomeUnknown")
+			require.Equal(t, creates+1, e.fake.requestCount(routeCreateWorkspace), "never a second create request")
+		})
+	}
+}
+
 func TestTemplateTestConfirmWrongAnswers(t *testing.T) {
 	t.Parallel()
 	e := newTemplateTestEnv(t)

internal/controller/codertemplatetest_controller.go

@@ -281,7 +281,7 @@ func (r *CoderTemplateTestReconciler) resolveInputs(
 		// Nothing of this test exists yet, so the workspace belongs to
 		// someone else. The controller never touches it.
 		return templateTestFail("WorkspaceNameConflict", "Coder user %s already has a workspace named %s.", ownerID, workspaceName), nil
-	case !isCoderNotFound(err):
+	case !isCoderResourceNotFound(err): // Only Coder's own 404 proves that the name is free (#351).
 		return coderUnavailable("get workspace by name", err), nil
 	}
 	return nil, nil

internal/controller/codertemplatetest_create_test.go

@@ -241,6 +241,37 @@ func TestTemplateTestConfirmKeepsCreateAnswer(t *testing.T) {
 	}
 }
 
+// nonCoderNotFounds are 404 answers that Coder itself never gives on the
+// workspace routes: a proxy's page, and Coder's own answer to a wrong route
+// (coderd/httpapi/httpapi.go:180 in v2.37.3).
+var nonCoderNotFounds = []struct {
+	name  string
+	fault fakeFault
+}{
+	{name: "HTML 404", fault: fakeFault{Status: 404, HTML: "<html>404 Not Found</html>"}},
+	{name: "Route not found", fault: fakeFault{Status: 404, Message: "Route not found."}},
+}
+
+// Before the create, only Coder's own 404 proves that the workspace name is
+// free. Another 404 waits and sends no create request (#351).
+func TestTemplateTestLookupTrustsOnlyCodersNotFound(t *testing.T) {
+	t.Parallel()
+	e := newTemplateTestEnv(t)
+	for _, tc := range nonCoderNotFounds {
+		t.Run(tc.name, func(t *testing.T) { // Not parallel: the cases share e.
+			creates := e.fake.requestCount(routeCreateWorkspace)
+			key := e.createTest(t, "default.docker", coderv1alpha1.CoderTemplateTestVersion{Name: "v1"})
+			e.fake.failNext(routeWorkspaceByName, tc.fault)
+			tt := e.reconcile(t, key, 3) // Finalizer, initialization, then lookups.
+			requireTemplateTestWaiting(t, tt, "CoderUnavailable", "get workspace by name: Coder answered 404")
+			require.Nil(t, tt.Status.CreateAttemptTime, "no marker without a create request")
+			require.Equal(t, creates, e.fake.requestCount(routeCreateWorkspace), "no create request")
+			requireTemplateTestRunning(t, e.settle(t, key), "WaitingForBuild", "")
+			require.Equal(t, creates+1, e.fake.requestCount(routeCreateWorkspace))
+		})
+	}
+}
+
 func TestTemplateTestDeadlineAfterCreate(t *testing.T) {
 	t.Parallel()
 	e := newTemplateTestEnv(t)

internal/controller/coderworkspacepoolmember_ready.go

@@ -57,7 +57,8 @@ func confirmPoolMemberCreate(
 	// With include_deleted, Coder answers the live workspace first.
 	workspace, err := sdk.WorkspaceByOwnerAndName(ctx, spec.OwnerID, name, codersdk.WorkspaceOptions{IncludeDeleted: true})
 	found := err == nil
-	if !found && !isCoderNotFound(err) {
+	// Only Coder's own 404 proves that no workspace exists (#351).
+	if !found && !isCoderResourceNotFound(err) {
 		return coderStepUnavailable("get workspace by name", err), nil
 	}
 	live := false

internal/controller/coderworkspacepoolmember_ready_test.go

@@ -138,6 +138,30 @@ func TestPoolMemberReadinessFailures(t *testing.T) {
 	}
 }
 
+// Only Coder's own 404 proves that no workspace of the member exists. A
+// proxy's 404 page, or "Route not found.", keeps the marker and waits, also
+// after the settle window (#351).
+func TestPoolMemberConfirmTrustsOnlyCodersNotFound(t *testing.T) {
+	t.Parallel()
+	e := newPoolMemberEnv(t)
+	long := func(m *coderv1alpha1.CoderWorkspacePoolMember) { m.Spec.ReadyTimeoutSeconds = 7200 }
+	for _, tc := range nonCoderNotFounds {
+		t.Run(tc.name, func(t *testing.T) { // Not parallel: the cases share e and its clock.
+			creates := e.fake.requestCount(routeCreateWorkspace)
+			key := e.uncertainMember(t, fakeFault{Status: 502}, long)
+			marker := e.reconcileMember(t, key, 0).Status.CreateAttemptTime.Time
+			e.clock.SetTime(marker.Add(15 * time.Minute))
+			e.fake.failNext(routeWorkspaceByName, tc.fault)
+			member := e.reconcileMember(t, key, 1)
+			requireMember(t, member, coderv1alpha1.CoderWorkspacePoolMemberPhaseCreating, "CoderUnavailable", "get workspace by name: Coder answered 404")
+			require.NotNil(t, member.Status.CreateAttemptTime, "the marker stays")
+			// Coder's own 404 after the window proves that no workspace exists.
+			requireMemberFailed(t, e.reconcileMember(t, key, 1), "CreateOutcomeUnknown", "appeared within 15m0s", metav1.ConditionTrue)
+			require.Equal(t, creates+1, e.fake.requestCount(routeCreateWorkspace), "never a second create request")
+		})
+	}
+}
+
 func TestPoolMemberConfirmsCreate(t *testing.T) {
 	t.Parallel()
 	e := newPoolMemberEnv(t)

internal/controller/fakecoder_test.go

@@ -615,11 +615,12 @@ func (f *fakeCoder) createWorkspace(r *http.Request) (int, any) {
 }
 
 // getWorkspaceByOwnerAndName prefers the live workspace. With
-// include_deleted=true it falls back to the newest deleted one.
+// include_deleted=true it falls back to the newest deleted one. Its 404 has
+// Coder's own message, which the controllers check (#351).
 func (f *fakeCoder) getWorkspaceByOwnerAndName(r *http.Request) (int, any) {
 	owner, ok := f.userByIdent(r.PathValue("user"))
 	if !ok {
-		return fakeError(http.StatusNotFound, "Resource not found.")
+		return http.StatusNotFound, fakeResourceNotFound
 	}
 	name := r.PathValue("name")
 	if fw := f.liveWorkspace(owner.ID, name); fw != nil {
@@ -634,7 +635,7 @@ func (f *fakeCoder) getWorkspaceByOwnerAndName(r *http.Request) (int, any) {
 		}
 	}
 	if newest == nil {
-		return fakeError(http.StatusNotFound, "Resource not found.")
+		return http.StatusNotFound, fakeResourceNotFound
 	}
 	return http.StatusOK, newest.view()
 }