fix: require resourceVersion on workspace updates and validate coder URL

Author: ThomasK33

app_dispatch.go

@@ -4,6 +4,8 @@ import (
 	"context"
 	"flag"
 	"fmt"
+	"net/url"
+	"strings"
 	"time"
 
 	ctrl "sigs.k8s.io/controller-runtime"
@@ -62,6 +64,23 @@ func run(args []string) error {
 		return err
 	}
 
+	if coderURL != "" {
+		parsedCoderURL, err := url.Parse(coderURL)
+		if err != nil {
+			return fmt.Errorf("assertion failed: invalid --coder-url %q: %w", coderURL, err)
+		}
+		if parsedCoderURL.Scheme == "" || parsedCoderURL.Host == "" {
+			return fmt.Errorf(
+				"assertion failed: invalid --coder-url %q: must include scheme and host (for example, https://coder.example.com)",
+				coderURL,
+			)
+		}
+		scheme := strings.ToLower(parsedCoderURL.Scheme)
+		if scheme != "http" && scheme != "https" {
+			return fmt.Errorf("assertion failed: invalid --coder-url %q: scheme must be http or https", coderURL)
+		}
+	}
+
 	switch appMode {
 	case "controller":
 		return runControllerApp(setupSignalHandler())

internal/aggregated/storage/storage_test.go

@@ -692,6 +692,52 @@ func TestWorkspaceStorageUpdateRejectsDifferentTemplateVersionID(t *testing.T) {
 	}
 }
 
+func TestWorkspaceStorageUpdateRejectsMissingResourceVersion(t *testing.T) {
+	t.Parallel()
+
+	server, state := newMockCoderServer(t)
+	defer server.Close()
+
+	workspaceStorage := NewWorkspaceStorage(newTestClientProvider(t, server.URL))
+	ctx := namespacedContext("control-plane")
+
+	currentObj, err := workspaceStorage.Get(ctx, "acme.alice.dev-workspace", nil)
+	if err != nil {
+		t.Fatalf("expected workspace get to succeed: %v", err)
+	}
+
+	currentWorkspace, ok := currentObj.(*aggregationv1alpha1.CoderWorkspace)
+	if !ok {
+		t.Fatalf("expected *CoderWorkspace from get, got %T", currentObj)
+	}
+	if currentWorkspace.ResourceVersion == "" {
+		t.Fatal("expected current workspace resourceVersion to be populated")
+	}
+
+	desiredWorkspace := currentWorkspace.DeepCopy()
+	desiredWorkspace.Spec.Running = !currentWorkspace.Spec.Running
+	desiredWorkspace.ResourceVersion = ""
+
+	_, _, err = workspaceStorage.Update(
+		ctx,
+		desiredWorkspace.Name,
+		testUpdatedObjectInfo{obj: desiredWorkspace},
+		nil,
+		rest.ValidateAllObjectUpdateFunc,
+		false,
+		nil,
+	)
+	if !apierrors.IsBadRequest(err) {
+		t.Fatalf("expected BadRequest for missing resourceVersion, got %v", err)
+	}
+	if err == nil || !strings.Contains(err.Error(), "metadata.resourceVersion is required for update") {
+		t.Fatalf("expected missing resourceVersion error message, got %v", err)
+	}
+	if transitions := state.buildTransitionsSnapshot(); len(transitions) != 0 {
+		t.Fatalf("expected no workspace build transitions when resourceVersion is missing, got %v", transitions)
+	}
+}
+
 func TestWorkspaceStorageUpdateRejectsStaleResourceVersion(t *testing.T) {
 	t.Parallel()
 

internal/aggregated/storage/workspace.go

@@ -333,7 +333,10 @@ func (s *WorkspaceStorage) Update(
 			fmt.Sprintf("metadata.namespace %q does not match request namespace %q", desiredObj.Namespace, namespace),
 		)
 	}
-	if desiredObj.ResourceVersion != "" && desiredObj.ResourceVersion != currentK8sObj.ResourceVersion {
+	if desiredObj.ResourceVersion == "" {
+		return nil, false, apierrors.NewBadRequest("metadata.resourceVersion is required for update")
+	}
+	if desiredObj.ResourceVersion != currentK8sObj.ResourceVersion {
 		return nil, false, apierrors.NewConflict(
 			aggregationv1alpha1.Resource("coderworkspaces"),
 			name,

main_test.go

@@ -139,6 +139,66 @@ func TestRunDispatchesAggregatedAPIServerMode(t *testing.T) {
 	}
 }
 
+func TestRunRejectsAggregatedAPIServerModeWithCoderURLMissingScheme(t *testing.T) {
+	t.Helper()
+	installMockSignalHandler(t)
+
+	previous := runAggregatedAPIServerApp
+	t.Cleanup(func() {
+		runAggregatedAPIServerApp = previous
+	})
+
+	called := false
+	runAggregatedAPIServerApp = func(context.Context, apiserverapp.Options) error {
+		called = true
+		return nil
+	}
+
+	err := run([]string{
+		"--app=aggregated-apiserver",
+		"--coder-url=coder.example.com",
+	})
+	if err == nil {
+		t.Fatal("expected an error when --coder-url omits scheme")
+	}
+	if !strings.Contains(err.Error(), "must include scheme and host") {
+		t.Fatalf("expected missing scheme/host validation error, got %v", err)
+	}
+	if called {
+		t.Fatal("expected aggregated apiserver runner not to be called on invalid --coder-url")
+	}
+}
+
+func TestRunRejectsAggregatedAPIServerModeWithUnsupportedCoderURLScheme(t *testing.T) {
+	t.Helper()
+	installMockSignalHandler(t)
+
+	previous := runAggregatedAPIServerApp
+	t.Cleanup(func() {
+		runAggregatedAPIServerApp = previous
+	})
+
+	called := false
+	runAggregatedAPIServerApp = func(context.Context, apiserverapp.Options) error {
+		called = true
+		return nil
+	}
+
+	err := run([]string{
+		"--app=aggregated-apiserver",
+		"--coder-url=ftp://coder.example.com",
+	})
+	if err == nil {
+		t.Fatal("expected an error when --coder-url has unsupported scheme")
+	}
+	if !strings.Contains(err.Error(), "scheme must be http or https") {
+		t.Fatalf("expected scheme validation error, got %v", err)
+	}
+	if called {
+		t.Fatal("expected aggregated apiserver runner not to be called on invalid --coder-url")
+	}
+}
+
 func TestRunDispatchesMCPHTTPMode(t *testing.T) {
 	t.Helper()
 	installMockSignalHandler(t)