Skip to content

Commit ca8660c

Browse files
EmyrkEmyrk
authored
chore: keep previous workspace build parameters for dynamic params (coder#18059)
The existing code persists all static parameters and their values. Using the previous build as the source if no new inputs are found. Dynamic params do not have a state of the parameters saved to disk. So instead, all previous values are persisted always, and new inputs override.
1 parent 6e255c7 commit ca8660c

File tree

2 files changed

+114
-11
lines changed

2 files changed

+114
-11
lines changed

‎coderd/parameters_test.go

Lines changed: 90 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import (
1515
"github.com/coder/coder/v2/coderd/database/dbtestutil"
1616
"github.com/coder/coder/v2/coderd/database/pubsub"
1717
"github.com/coder/coder/v2/coderd/rbac"
18+
"github.com/coder/coder/v2/coderd/util/ptr"
1819
"github.com/coder/coder/v2/codersdk"
1920
"github.com/coder/coder/v2/codersdk/wsjson"
2021
"github.com/coder/coder/v2/provisioner/echo"
@@ -211,6 +212,86 @@ func TestDynamicParametersWithTerraformValues(t *testing.T) {
211212
require.Zero(t, setup.api.FileCache.Count())
212213
})
213214

215+
t.Run("RebuildParameters", func(t *testing.T) {
216+
t.Parallel()
217+
218+
dynamicParametersTerraformSource, err := os.ReadFile("testdata/parameters/modules/main.tf")
219+
require.NoError(t, err)
220+
221+
modulesArchive, err := terraform.GetModulesArchive(os.DirFS("testdata/parameters/modules"))
222+
require.NoError(t, err)
223+
224+
setup := setupDynamicParamsTest(t, setupDynamicParamsTestParams{
225+
provisionerDaemonVersion: provProto.CurrentVersion.String(),
226+
mainTF: dynamicParametersTerraformSource,
227+
modulesArchive: modulesArchive,
228+
plan: nil,
229+
static: nil,
230+
})
231+
232+
ctx := testutil.Context(t, testutil.WaitMedium)
233+
stream := setup.stream
234+
previews := stream.Chan()
235+
236+
// Should see the output of the module represented
237+
preview := testutil.RequireReceive(ctx, t, previews)
238+
require.Equal(t, -1, preview.ID)
239+
require.Empty(t, preview.Diagnostics)
240+
241+
require.Len(t, preview.Parameters, 1)
242+
require.Equal(t, "jetbrains_ide", preview.Parameters[0].Name)
243+
require.True(t, preview.Parameters[0].Value.Valid)
244+
require.Equal(t, "CL", preview.Parameters[0].Value.Value)
245+
_ = stream.Close(websocket.StatusGoingAway)
246+
247+
wrk := coderdtest.CreateWorkspace(t, setup.client, setup.template.ID, func(request *codersdk.CreateWorkspaceRequest) {
248+
request.RichParameterValues = []codersdk.WorkspaceBuildParameter{
249+
{
250+
Name: preview.Parameters[0].Name,
251+
Value: "GO",
252+
},
253+
}
254+
})
255+
coderdtest.AwaitWorkspaceBuildJobCompleted(t, setup.client, wrk.LatestBuild.ID)
256+
257+
params, err := setup.client.WorkspaceBuildParameters(ctx, wrk.LatestBuild.ID)
258+
require.NoError(t, err)
259+
require.Len(t, params, 1)
260+
require.Equal(t, "jetbrains_ide", params[0].Name)
261+
require.Equal(t, "GO", params[0].Value)
262+
263+
// A helper function to assert params
264+
doTransition := func(t *testing.T, trans codersdk.WorkspaceTransition) {
265+
t.Helper()
266+
267+
fooVal := coderdtest.RandomUsername(t)
268+
bld, err := setup.client.CreateWorkspaceBuild(ctx, wrk.ID, codersdk.CreateWorkspaceBuildRequest{
269+
TemplateVersionID: setup.template.ActiveVersionID,
270+
Transition: trans,
271+
RichParameterValues: []codersdk.WorkspaceBuildParameter{
272+
// No validation, so this should work as is.
273+
// Overwrite the value on each transition
274+
{Name: "foo", Value: fooVal},
275+
},
276+
EnableDynamicParameters: ptr.Ref(true),
277+
})
278+
require.NoError(t, err)
279+
coderdtest.AwaitWorkspaceBuildJobCompleted(t, setup.client, wrk.LatestBuild.ID)
280+
281+
latestParams, err := setup.client.WorkspaceBuildParameters(ctx, bld.ID)
282+
require.NoError(t, err)
283+
require.ElementsMatch(t, latestParams, []codersdk.WorkspaceBuildParameter{
284+
{Name: "jetbrains_ide", Value: "GO"},
285+
{Name: "foo", Value: fooVal},
286+
})
287+
}
288+
289+
// Restart the workspace, then delete. Asserting params on all builds.
290+
doTransition(t, codersdk.WorkspaceTransitionStop)
291+
doTransition(t, codersdk.WorkspaceTransitionStart)
292+
doTransition(t, codersdk.WorkspaceTransitionDelete)
293+
})
294+
214295
t.Run("BadOwner", func(t *testing.T) {
215296
t.Parallel()
216297

@@ -266,9 +347,10 @@ type setupDynamicParamsTestParams struct {
266347
}
267348

268349
type dynamicParamsTest struct {
269-
client *codersdk.Client
270-
api *coderd.API
271-
stream *wsjson.Stream[codersdk.DynamicParametersResponse, codersdk.DynamicParametersRequest]
350+
client *codersdk.Client
351+
api *coderd.API
352+
stream *wsjson.Stream[codersdk.DynamicParametersResponse, codersdk.DynamicParametersRequest]
353+
template codersdk.Template
272354
}
273355

274356
func setupDynamicParamsTest(t *testing.T, args setupDynamicParamsTestParams) dynamicParamsTest {
@@ -300,7 +382,7 @@ func setupDynamicParamsTest(t *testing.T, args setupDynamicParamsTestParams) dyn
300382

301383
version := coderdtest.CreateTemplateVersion(t, templateAdmin, owner.OrganizationID, files)
302384
coderdtest.AwaitTemplateVersionJobCompleted(t, templateAdmin, version.ID)
303-
_ = coderdtest.CreateTemplate(t, templateAdmin, owner.OrganizationID, version.ID)
385+
tpl := coderdtest.CreateTemplate(t, templateAdmin, owner.OrganizationID, version.ID)
304386

305387
ctx := testutil.Context(t, testutil.WaitShort)
306388
stream, err := templateAdmin.TemplateVersionDynamicParameters(ctx, version.ID)
@@ -321,9 +403,10 @@ func setupDynamicParamsTest(t *testing.T, args setupDynamicParamsTestParams) dyn
321403
})
322404

323405
return dynamicParamsTest{
324-
client: ownerClient,
325-
stream: stream,
326-
api: api,
406+
client: ownerClient,
407+
api: api,
408+
stream: stream,
409+
template: tpl,
327410
}
328411
}
329412

‎coderd/wsbuilder/wsbuilder.go

Lines changed: 24 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -623,6 +623,11 @@ func (b *Builder) getParameters() (names, values []string, err error) {
623623
return nil, nil, BuildError{http.StatusBadRequest, "Unable to build workspace with unsupported parameters", err}
624624
}
625625

626+
lastBuildParameterValues := db2sdk.WorkspaceBuildParameters(lastBuildParameters)
627+
resolver := codersdk.ParameterResolver{
628+
Rich: lastBuildParameterValues,
629+
}
630+
626631
// Dynamic parameters skip all parameter validation.
627632
// Deleting a workspace also should skip parameter validation.
628633
// Pass the user's input as is.
@@ -632,19 +637,34 @@ func (b *Builder) getParameters() (names, values []string, err error) {
632637
// conditional parameter existence, the static frame of reference
633638
// is not sufficient. So assume the user is correct, or pull in the
634639
// dynamic param code to find the actual parameters.
640+
latestValues := make(map[string]string, len(b.richParameterValues))
641+
for _, latest := range b.richParameterValues {
642+
latestValues[latest.Name] = latest.Value
643+
}
644+
645+
// Merge the inputs with values from the previous build.
646+
for _, last := range lastBuildParameterValues {
647+
// TODO: Ideally we use the resolver here and look at parameter
648+
// fields such as 'ephemeral'. This requires loading the terraform
649+
// files. For now, just send the previous inputs as is.
650+
if _, exists := latestValues[last.Name]; exists {
651+
// latestValues take priority, so skip this previous value.
652+
continue
653+
}
654+
names = append(names, last.Name)
655+
values = append(values, last.Value)
656+
}
657+
635658
for _, value := range b.richParameterValues {
636659
names = append(names, value.Name)
637660
values = append(values, value.Value)
638661
}
662+
639663
b.parameterNames = &names
640664
b.parameterValues = &values
641665
return names, values, nil
642666
}
643667

644-
resolver := codersdk.ParameterResolver{
645-
Rich: db2sdk.WorkspaceBuildParameters(lastBuildParameters),
646-
}
647-
648668
for _, templateVersionParameter := range templateVersionParameters {
649669
tvp, err := db2sdk.TemplateVersionParameter(templateVersionParameter)
650670
if err != nil {

0 commit comments

Comments
 (0)