fix: coder_parameter fallbacks to default

mtojek · mtojek · commit 5b332362e3f0 · 2023-04-21T12:36:52.000+02:00
diff --git a/coderd/workspacebuilds.go b/coderd/workspacebuilds.go
@@ -544,6 +544,15 @@ func (api *API) postWorkspaceBuilds(rw http.ResponseWriter, r *http.Request) {
 		// Check if parameter is defined in previous build
 		if buildParameter, found := findWorkspaceBuildParameter(apiLastBuildParameters, templateVersionParameter.Name); found {
 			parameters = append(parameters, *buildParameter)
+			continue
+		}
+
+		// Check if default parameter value is in schema
+		if templateVersionParameter.DefaultValue != "" {
+			parameters = append(parameters, codersdk.WorkspaceBuildParameter{
+				Name:  templateVersionParameter.Name,
+				Value: templateVersionParameter.DefaultValue,
+			})
 		}
 	}
 
diff --git a/coderd/workspacebuilds_test.go b/coderd/workspacebuilds_test.go
@@ -898,12 +898,23 @@ func TestWorkspaceBuildWithRichParameters(t *testing.T) {
 		nextBuildParameters := []codersdk.WorkspaceBuildParameter{
 			{Name: newImmutableParameterName, Value: "good"},
 		}
-		_, err = client.CreateWorkspaceBuild(ctx, workspace.ID, codersdk.CreateWorkspaceBuildRequest{
+		nextWorkspaceBuild, err := client.CreateWorkspaceBuild(ctx, workspace.ID, codersdk.CreateWorkspaceBuildRequest{
 			TemplateVersionID:   version2.ID,
 			Transition:          codersdk.WorkspaceTransitionStart,
 			RichParameterValues: nextBuildParameters,
 		})
 		require.NoError(t, err)
+		require.NotEqual(t, workspaceBuild, nextWorkspaceBuild)
+		coderdtest.AwaitWorkspaceBuildJob(t, client, nextWorkspaceBuild.ID)
+
+		workspaceBuildParameters, err := client.WorkspaceBuildParameters(ctx, nextWorkspaceBuild.ID)
+		require.NoError(t, err)
+
+		expectedNextBuildParameters := append(initialBuildParameters, codersdk.WorkspaceBuildParameter{
+			Name:  newImmutableParameterName,
+			Value: "good",
+		})
+		require.ElementsMatch(t, expectedNextBuildParameters, workspaceBuildParameters)
 	})
 
 	t.Run("NewImmutableOptionalParameterUsesDefault", func(t *testing.T) {
@@ -958,12 +969,23 @@ func TestWorkspaceBuildWithRichParameters(t *testing.T) {
 		defer cancel()
 
 		var nextBuildParameters []codersdk.WorkspaceBuildParameter
-		_, err = client.CreateWorkspaceBuild(ctx, workspace.ID, codersdk.CreateWorkspaceBuildRequest{
+		nextWorkspaceBuild, err := client.CreateWorkspaceBuild(ctx, workspace.ID, codersdk.CreateWorkspaceBuildRequest{
 			TemplateVersionID:   version2.ID,
 			Transition:          codersdk.WorkspaceTransitionStart,
 			RichParameterValues: nextBuildParameters,
 		})
 		require.NoError(t, err)
+		require.NotEqual(t, workspaceBuild, nextWorkspaceBuild)
+		coderdtest.AwaitWorkspaceBuildJob(t, client, nextWorkspaceBuild.ID)
+
+		workspaceBuildParameters, err := client.WorkspaceBuildParameters(ctx, nextWorkspaceBuild.ID)
+		require.NoError(t, err)
+
+		expectedNextBuildParameters := append(initialBuildParameters, codersdk.WorkspaceBuildParameter{
+			Name:  newImmutableParameterName,
+			Value: "12345",
+		})
+		require.ElementsMatch(t, expectedNextBuildParameters, workspaceBuildParameters)
 	})
 }
 
diff --git a/site/src/api/api.ts b/site/src/api/api.ts
@@ -1057,15 +1057,19 @@ const getMissingParameters = (
   const requiredParameters: TypesGen.TemplateVersionParameter[] = []
 
   templateParameters.forEach((p) => {
-    // Legacy parameters should be required. So we can migrate them.
-    const isLegacy = p.legacy_variable_name === undefined
+    // Legacy parameters should not be required. Backend can just migrate them.
+    const isLegacy = p.legacy_variable_name !== undefined
     // It is mutable and required. Mutable values can be changed after so we
     // don't need to ask them if they are not required.
     const isMutableAndRequired = p.mutable && p.required
     // Is immutable, so we can check if it is its first time on the build
     const isImmutable = !p.mutable
 
-    if (isLegacy || isMutableAndRequired || isImmutable) {
+    if (isLegacy) {
+      return
+    }
+
+    if (isMutableAndRequired || isImmutable) {
       requiredParameters.push(p)
       return
     }
