Skip to content

Commit c000f2e

Browse files
authored
fix: coder_parameter fallbacks to default (#7240)
1 parent ab077d1 commit c000f2e

File tree

3 files changed

+40
-5
lines changed

3 files changed

+40
-5
lines changed

coderd/workspacebuilds.go

+9
Original file line numberDiff line numberDiff line change
@@ -544,6 +544,15 @@ func (api *API) postWorkspaceBuilds(rw http.ResponseWriter, r *http.Request) {
544544
// Check if parameter is defined in previous build
545545
if buildParameter, found := findWorkspaceBuildParameter(apiLastBuildParameters, templateVersionParameter.Name); found {
546546
parameters = append(parameters, *buildParameter)
547+
continue
548+
}
549+
550+
// Check if default parameter value is in schema
551+
if templateVersionParameter.DefaultValue != "" {
552+
parameters = append(parameters, codersdk.WorkspaceBuildParameter{
553+
Name: templateVersionParameter.Name,
554+
Value: templateVersionParameter.DefaultValue,
555+
})
547556
}
548557
}
549558

coderd/workspacebuilds_test.go

+24-2
Original file line numberDiff line numberDiff line change
@@ -901,12 +901,23 @@ func TestWorkspaceBuildWithRichParameters(t *testing.T) {
901901
nextBuildParameters := []codersdk.WorkspaceBuildParameter{
902902
{Name: newImmutableParameterName, Value: "good"},
903903
}
904-
_, err = client.CreateWorkspaceBuild(ctx, workspace.ID, codersdk.CreateWorkspaceBuildRequest{
904+
nextWorkspaceBuild, err := client.CreateWorkspaceBuild(ctx, workspace.ID, codersdk.CreateWorkspaceBuildRequest{
905905
TemplateVersionID: version2.ID,
906906
Transition: codersdk.WorkspaceTransitionStart,
907907
RichParameterValues: nextBuildParameters,
908908
})
909909
require.NoError(t, err)
910+
require.NotEqual(t, workspaceBuild, nextWorkspaceBuild)
911+
coderdtest.AwaitWorkspaceBuildJob(t, client, nextWorkspaceBuild.ID)
912+
913+
workspaceBuildParameters, err := client.WorkspaceBuildParameters(ctx, nextWorkspaceBuild.ID)
914+
require.NoError(t, err)
915+
916+
expectedNextBuildParameters := append(initialBuildParameters, codersdk.WorkspaceBuildParameter{
917+
Name: newImmutableParameterName,
918+
Value: "good",
919+
})
920+
require.ElementsMatch(t, expectedNextBuildParameters, workspaceBuildParameters)
910921
})
911922

912923
t.Run("NewImmutableOptionalParameterUsesDefault", func(t *testing.T) {
@@ -961,12 +972,23 @@ func TestWorkspaceBuildWithRichParameters(t *testing.T) {
961972
defer cancel()
962973

963974
var nextBuildParameters []codersdk.WorkspaceBuildParameter
964-
_, err = client.CreateWorkspaceBuild(ctx, workspace.ID, codersdk.CreateWorkspaceBuildRequest{
975+
nextWorkspaceBuild, err := client.CreateWorkspaceBuild(ctx, workspace.ID, codersdk.CreateWorkspaceBuildRequest{
965976
TemplateVersionID: version2.ID,
966977
Transition: codersdk.WorkspaceTransitionStart,
967978
RichParameterValues: nextBuildParameters,
968979
})
969980
require.NoError(t, err)
981+
require.NotEqual(t, workspaceBuild, nextWorkspaceBuild)
982+
coderdtest.AwaitWorkspaceBuildJob(t, client, nextWorkspaceBuild.ID)
983+
984+
workspaceBuildParameters, err := client.WorkspaceBuildParameters(ctx, nextWorkspaceBuild.ID)
985+
require.NoError(t, err)
986+
987+
expectedNextBuildParameters := append(initialBuildParameters, codersdk.WorkspaceBuildParameter{
988+
Name: newImmutableParameterName,
989+
Value: "12345",
990+
})
991+
require.ElementsMatch(t, expectedNextBuildParameters, workspaceBuildParameters)
970992
})
971993
}
972994

site/src/api/api.ts

+7-3
Original file line numberDiff line numberDiff line change
@@ -1057,15 +1057,19 @@ const getMissingParameters = (
10571057
const requiredParameters: TypesGen.TemplateVersionParameter[] = []
10581058

10591059
templateParameters.forEach((p) => {
1060-
// Legacy parameters should be required. So we can migrate them.
1061-
const isLegacy = p.legacy_variable_name === undefined
1060+
// Legacy parameters should not be required. Backend can just migrate them.
1061+
const isLegacy = p.legacy_variable_name !== undefined
10621062
// It is mutable and required. Mutable values can be changed after so we
10631063
// don't need to ask them if they are not required.
10641064
const isMutableAndRequired = p.mutable && p.required
10651065
// Is immutable, so we can check if it is its first time on the build
10661066
const isImmutable = !p.mutable
10671067

1068-
if (isLegacy || isMutableAndRequired || isImmutable) {
1068+
if (isLegacy) {
1069+
return
1070+
}
1071+
1072+
if (isMutableAndRequired || isImmutable) {
10691073
requiredParameters.push(p)
10701074
return
10711075
}

0 commit comments

Comments
 (0)