feat: allow new immutable parameters for existing workspaces (#18579)

Closes https://github.com/coder/coder/issues/18578

coderd/coderdtest/dynamicparameters.go

@@ -22,6 +22,9 @@ type DynamicParameterTemplateParams struct {
 
 	// StaticParams is used if the provisioner daemon version does not support dynamic parameters.
 	StaticParams []*proto.RichParameter
+
+	// TemplateID is used to update an existing template instead of creating a new one.
+	TemplateID uuid.UUID
 }
 
 func DynamicParameterTemplate(t *testing.T, client *codersdk.Client, org uuid.UUID, args DynamicParameterTemplateParams) (codersdk.Template, codersdk.TemplateVersion) {
@@ -40,16 +43,30 @@ func DynamicParameterTemplate(t *testing.T, client *codersdk.Client, org uuid.UU
 		},
 	}}
 
-	version := CreateTemplateVersion(t, client, org, files)
+	version := CreateTemplateVersion(t, client, org, files, func(request *codersdk.CreateTemplateVersionRequest) {
+		if args.TemplateID != uuid.Nil {
+			request.TemplateID = args.TemplateID
+		}
+	})
 	AwaitTemplateVersionJobCompleted(t, client, version.ID)
-	tpl := CreateTemplate(t, client, org, version.ID)
+
+	tplID := args.TemplateID
+	if args.TemplateID == uuid.Nil {
+		tpl := CreateTemplate(t, client, org, version.ID)
+		tplID = tpl.ID
+	}
 
 	var err error
-	tpl, err = client.UpdateTemplateMeta(t.Context(), tpl.ID, codersdk.UpdateTemplateMeta{
+	tpl, err := client.UpdateTemplateMeta(t.Context(), tplID, codersdk.UpdateTemplateMeta{
 		UseClassicParameterFlow: ptr.Ref(false),
 	})
 	require.NoError(t, err)
 
+	err = client.UpdateActiveTemplateVersion(t.Context(), tpl.ID, codersdk.UpdateActiveTemplateVersion{
+		ID: version.ID,
+	})
+	require.NoError(t, err)
+
 	return tpl, version
 }
 

coderd/dynamicparameters/rendermock/mock.go

@@ -0,0 +1,2 @@
+//go:generate mockgen -destination ./rendermock.go -package rendermock github.com/coder/coder/v2/coderd/dynamicparameters Renderer
+package rendermock

coderd/dynamicparameters/rendermock/rendermock.go

@@ -0,0 +1,71 @@
+// Code generated by MockGen. DO NOT EDIT.
+// Source: github.com/coder/coder/v2/coderd/dynamicparameters (interfaces: Renderer)
+//
+// Generated by this command:
+//
+//	mockgen -destination ./rendermock.go -package rendermock github.com/coder/coder/v2/coderd/dynamicparameters Renderer
+//
+
+// Package rendermock is a generated GoMock package.
+package rendermock
+
+import (
+	context "context"
+	reflect "reflect"
+
+	preview "github.com/coder/preview"
+	uuid "github.com/google/uuid"
+	hcl "github.com/hashicorp/hcl/v2"
+	gomock "go.uber.org/mock/gomock"
+)
+
+// MockRenderer is a mock of Renderer interface.
+type MockRenderer struct {
+	ctrl     *gomock.Controller
+	recorder *MockRendererMockRecorder
+	isgomock struct{}
+}
+
+// MockRendererMockRecorder is the mock recorder for MockRenderer.
+type MockRendererMockRecorder struct {
+	mock *MockRenderer
+}
+
+// NewMockRenderer creates a new mock instance.
+func NewMockRenderer(ctrl *gomock.Controller) *MockRenderer {
+	mock := &MockRenderer{ctrl: ctrl}
+	mock.recorder = &MockRendererMockRecorder{mock}
+	return mock
+}
+
+// EXPECT returns an object that allows the caller to indicate expected use.
+func (m *MockRenderer) EXPECT() *MockRendererMockRecorder {
+	return m.recorder
+}
+
+// Close mocks base method.
+func (m *MockRenderer) Close() {
+	m.ctrl.T.Helper()
+	m.ctrl.Call(m, "Close")
+}
+
+// Close indicates an expected call of Close.
+func (mr *MockRendererMockRecorder) Close() *gomock.Call {
+	mr.mock.ctrl.T.Helper()
+	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Close", reflect.TypeOf((*MockRenderer)(nil).Close))
+}
+
+// Render mocks base method.
+func (m *MockRenderer) Render(ctx context.Context, ownerID uuid.UUID, values map[string]string) (*preview.Output, hcl.Diagnostics) {
+	m.ctrl.T.Helper()
+	ret := m.ctrl.Call(m, "Render", ctx, ownerID, values)
+	ret0, _ := ret[0].(*preview.Output)
+	ret1, _ := ret[1].(hcl.Diagnostics)
+	return ret0, ret1
+}
+
+// Render indicates an expected call of Render.
+func (mr *MockRendererMockRecorder) Render(ctx, ownerID, values any) *gomock.Call {
+	mr.mock.ctrl.T.Helper()
+	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Render", reflect.TypeOf((*MockRenderer)(nil).Render), ctx, ownerID, values)
+}

coderd/dynamicparameters/resolver.go

@@ -169,9 +169,15 @@ func ResolveParameters(
 		parameterNames[parameter.Name] = struct{}{}
 
 		if !firstBuild && !parameter.Mutable {
+			originalValue, ok := originalValues[parameter.Name]
 			// Immutable parameters should not be changed after the first build.
-			// They can match the original value though!
-			if parameter.Value.AsString() != originalValues[parameter.Name].Value {
+			// If the value matches the original value, that is fine.
+			//
+			// If the original value is not set, that means this is a new parameter. New
+			// immutable parameters are allowed. This is an opinionated choice to prevent
+			// workspaces failing to update or delete. Ideally we would block this, as
+			// immutable parameters should only be able to be set at creation time.
+			if ok && parameter.Value.AsString() != originalValue.Value {
 				var src *hcl.Range
 				if parameter.Source != nil {
 					src = &parameter.Source.HCLBlock().TypeRange

coderd/dynamicparameters/resolver_test.go

@@ -0,0 +1,59 @@
+package dynamicparameters_test
+
+import (
+	"testing"
+
+	"github.com/google/uuid"
+	"github.com/stretchr/testify/require"
+	"go.uber.org/mock/gomock"
+
+	"github.com/coder/coder/v2/coderd/database"
+	"github.com/coder/coder/v2/coderd/dynamicparameters"
+	"github.com/coder/coder/v2/coderd/dynamicparameters/rendermock"
+	"github.com/coder/coder/v2/codersdk"
+	"github.com/coder/coder/v2/testutil"
+	"github.com/coder/preview"
+	previewtypes "github.com/coder/preview/types"
+	"github.com/coder/terraform-provider-coder/v2/provider"
+)
+
+func TestResolveParameters(t *testing.T) {
+	t.Parallel()
+
+	t.Run("NewImmutable", func(t *testing.T) {
+		t.Parallel()
+
+		ctrl := gomock.NewController(t)
+		render := rendermock.NewMockRenderer(ctrl)
+
+		// A single immutable parameter with no previous value.
+		render.EXPECT().
+			Render(gomock.Any(), gomock.Any(), gomock.Any()).
+			AnyTimes().
+			Return(&preview.Output{
+				Parameters: []previewtypes.Parameter{
+					{
+						ParameterData: previewtypes.ParameterData{
+							Name:         "immutable",
+							Type:         previewtypes.ParameterTypeString,
+							FormType:     provider.ParameterFormTypeInput,
+							Mutable:      false,
+							DefaultValue: previewtypes.StringLiteral("foo"),
+							Required:     true,
+						},
+						Value:       previewtypes.StringLiteral("foo"),
+						Diagnostics: nil,
+					},
+				},
+			}, nil)
+
+		ctx := testutil.Context(t, testutil.WaitShort)
+		values, err := dynamicparameters.ResolveParameters(ctx, uuid.New(), render, false,
+			[]database.WorkspaceBuildParameter{},        // No previous values
+			[]codersdk.WorkspaceBuildParameter{},        // No new build values
+			[]database.TemplateVersionPresetParameter{}, // No preset values
+		)
+		require.NoError(t, err)
+		require.Equal(t, map[string]string{"immutable": "foo"}, values)
+	})
+}

enterprise/coderd/dynamicparameters_test.go

@@ -302,6 +302,57 @@ func TestDynamicParameterBuild(t *testing.T) {
 			require.ErrorContains(t, err, "Number must be between 0 and 10")
 		})
 	})
+
+	t.Run("ImmutableValidation", func(t *testing.T) {
+		t.Parallel()
+
+		// NewImmutable tests the case where a new immutable parameter is added to a template
+		// after a workspace has been created with an older version of the template.
+		// The test tries to delete the workspace, which should succeed.
+		t.Run("NewImmutable", func(t *testing.T) {
+			t.Parallel()
+
+			ctx := testutil.Context(t, testutil.WaitShort)
+			// Start with a new template that has 0 parameters
+			empty, _ := coderdtest.DynamicParameterTemplate(t, templateAdmin, orgID, coderdtest.DynamicParameterTemplateParams{
+				MainTF: string(must(os.ReadFile("testdata/parameters/none/main.tf"))),
+			})
+
+			// Create the workspace with 0 parameters
+			wrk, err := templateAdmin.CreateUserWorkspace(ctx, codersdk.Me, codersdk.CreateWorkspaceRequest{
+				TemplateID:          empty.ID,
+				Name:                coderdtest.RandomUsername(t),
+				RichParameterValues: []codersdk.WorkspaceBuildParameter{},
+			})
+			require.NoError(t, err)
+			coderdtest.AwaitWorkspaceBuildJobCompleted(t, templateAdmin, wrk.LatestBuild.ID)
+
+			// Update the template with a new immutable parameter
+			_, immutable := coderdtest.DynamicParameterTemplate(t, templateAdmin, orgID, coderdtest.DynamicParameterTemplateParams{
+				MainTF:     string(must(os.ReadFile("testdata/parameters/immutable/main.tf"))),
+				TemplateID: empty.ID,
+			})
+
+			bld, err := templateAdmin.CreateWorkspaceBuild(ctx, wrk.ID, codersdk.CreateWorkspaceBuildRequest{
+				TemplateVersionID: immutable.ID, // Use the new template version with the immutable parameter
+				Transition:        codersdk.WorkspaceTransitionDelete,
+				DryRun:            false,
+			})
+			require.NoError(t, err)
+			coderdtest.AwaitWorkspaceBuildJobCompleted(t, templateAdmin, bld.ID)
+
+			// Verify the immutable parameter is set on the workspace build
+			params, err := templateAdmin.WorkspaceBuildParameters(ctx, bld.ID)
+			require.NoError(t, err)
+			require.Len(t, params, 1)
+			require.Equal(t, "Hello World", params[0].Value)
+
+			// Verify the workspace is deleted
+			deleted, err := templateAdmin.DeletedWorkspace(ctx, wrk.ID)
+			require.NoError(t, err)
+			require.Equal(t, wrk.ID, deleted.ID, "workspace should be deleted")
+		})
+	})
 }
 
 // TestDynamicParameterTemplate uses a template with some dynamic elements, and

enterprise/coderd/testdata/parameters/immutable/main.tf

@@ -0,0 +1,16 @@
+terraform {
+  required_providers {
+    coder = {
+      source = "coder/coder"
+    }
+  }
+}
+
+data "coder_workspace_owner" "me" {}
+
+data "coder_parameter" "immutable" {
+  name    = "immutable"
+  type    = "string"
+  mutable = false
+  default = "Hello World"
+}

enterprise/coderd/testdata/parameters/none/main.tf

@@ -0,0 +1,10 @@
+terraform {
+  required_providers {
+    coder = {
+      source = "coder/coder"
+    }
+  }
+}
+
+data "coder_workspace_owner" "me" {}
+