PR suggestions, mostly comments

Emyrk · Emyrk · commit cfe21201d927 · 2025-05-20T07:14:46.000-05:00
diff --git a/coderd/workspacebuilds.go b/coderd/workspacebuilds.go
@@ -385,10 +385,19 @@ func (api *API) postWorkspaceBuilds(rw http.ResponseWriter, r *http.Request) {
 		}
 
 		// Only defer to dynamic parameters if the experiment is enabled.
-		if api.Experiments.Enabled(codersdk.ExperimentDynamicParameters) &&
-			createBuild.EnableDynamicParameters != nil {
-			// Explicit opt-in
-			builder = builder.DynamicParameters(*createBuild.EnableDynamicParameters)
+		if api.Experiments.Enabled(codersdk.ExperimentDynamicParameters) {
+			if createBuild.EnableDynamicParameters != nil {
+				// Explicit opt-in
+				builder = builder.DynamicParameters(*createBuild.EnableDynamicParameters)
+			}
+		} else {
+			if createBuild.EnableDynamicParameters != nil {
+				api.Logger.Warn(ctx, "ignoring dynamic parameter field sent by request, the experiment is not enabled",
+					slog.F("field", *createBuild.EnableDynamicParameters),
+					slog.F("user", apiKey.UserID.String()),
+					slog.F("transition", string(createBuild.Transition)),
+				)
+			}
 		}
 
 		workspaceBuild, provisionerJob, provisionerDaemons, err = builder.Build(
diff --git a/coderd/wsbuilder/wsbuilder.go b/coderd/wsbuilder/wsbuilder.go
@@ -162,7 +162,9 @@ func (b Builder) DeploymentValues(dv *codersdk.DeploymentValues) Builder {
 
 func (b Builder) Experiments(exp codersdk.Experiments) Builder {
 	// nolint: revive
-	b.experiments = exp
+	cpy := make(codersdk.Experiments, len(exp))
+	copy(cpy, exp)
+	b.experiments = cpy
 	return b
 }
 
diff --git a/codersdk/workspaces.go b/codersdk/workspaces.go
@@ -112,6 +112,7 @@ type CreateWorkspaceBuildRequest struct {
 	TemplateVersionPresetID uuid.UUID `json:"template_version_preset_id,omitempty" format:"uuid"`
 	// EnableDynamicParameters skips some of the static parameter checking.
 	// It will default to whatever the template has marked as the default experience.
+	// Requires the "dynamic-experiment" to be used.
 	EnableDynamicParameters *bool `json:"enable_dynamic_parameters,omitempty"`
 }
 
diff --git a/enterprise/coderd/workspaces_test.go b/enterprise/coderd/workspaces_test.go
@@ -1738,7 +1738,9 @@ func TestWorkspaceTemplateParamsChange(t *testing.T) {
 	tpl := coderdtest.CreateTemplate(t, templateAdmin, owner.OrganizationID, tv.ID)
 	require.False(t, tpl.UseClassicParameterFlow, "template to use dynamic parameters")
 
-	// Creating a workspace as a non-privileged user must succeed
+	// When: we create a workspace build using the above template but with
+	// parameter values that are different from those defined in the template.
+	// The new values are not valid according to the original plan, but are valid.
 	ws, err := member.CreateUserWorkspace(ctx, memberUser.Username, codersdk.CreateWorkspaceRequest{
 		TemplateID: tpl.ID,
 		Name:       coderdtest.RandomUsername(t),
@@ -1754,11 +1756,14 @@ func TestWorkspaceTemplateParamsChange(t *testing.T) {
 		},
 		EnableDynamicParameters: true,
 	})
+
+	// Then: the build should succeed. The updated value of param_min should be
+	// used to validate param instead of the value defined in the temp
 	require.NoError(t, err, "failed to create workspace")
 	createBuild := coderdtest.AwaitWorkspaceBuildJobCompleted(t, member, ws.LatestBuild.ID)
 	require.Equal(t, createBuild.Status, codersdk.WorkspaceStatusRunning)
 
-	// Now restart the workspace
+	// Now delete the workspace
 	build, err := member.CreateWorkspaceBuild(ctx, ws.ID, codersdk.CreateWorkspaceBuildRequest{
 		Transition: codersdk.WorkspaceTransitionDelete,
 	})

Original file line number	Diff line number	Diff line change
`@@ -162,7 +162,9 @@ func (b Builder) DeploymentValues(dv *codersdk.DeploymentValues) Builder {`
`162`	`162`
`163`	`163`	`func (b Builder) Experiments(exp codersdk.Experiments) Builder {`
`164`	`164`	`// nolint: revive`
`165`		`- b.experiments = exp`
	`165`	`+ cpy := make(codersdk.Experiments, len(exp))`
	`166`	`+ copy(cpy, exp)`
	`167`	`+ b.experiments = cpy`
`166`	`168`	`return b`
`167`	`169`	`}`
`168`	`170`
Original file line number	Diff line number	Diff line change
`@@ -112,6 +112,7 @@ type CreateWorkspaceBuildRequest struct {`
`112`	`112`	TemplateVersionPresetID uuid.UUID `json:"template_version_preset_id,omitempty" format:"uuid"`
`113`	`113`	`// EnableDynamicParameters skips some of the static parameter checking.`
`114`	`114`	`// It will default to whatever the template has marked as the default experience.`
	`115`	`+ // Requires the "dynamic-experiment" to be used.`
`115`	`116`	EnableDynamicParameters *bool `json:"enable_dynamic_parameters,omitempty"`
`116`	`117`	`}`
`117`	`118`