From 5b332362e3f0327b567dea72717328de392c3232 Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Fri, 21 Apr 2023 12:36:52 +0200 Subject: [PATCH] fix: coder_parameter fallbacks to default --- coderd/workspacebuilds.go | 9 +++++++++ coderd/workspacebuilds_test.go | 26 ++++++++++++++++++++++++-- site/src/api/api.ts | 10 +++++++--- 3 files changed, 40 insertions(+), 5 deletions(-) diff --git a/coderd/workspacebuilds.go b/coderd/workspacebuilds.go index 687233a01cba3..c99e3b9f75d9b 100644 --- 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 index be91a94902b2a..d78b09836e116 100644 --- 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 index 206a62c452afc..e778b9fc0a4fb 100644 --- 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 }