Skip to content

Commit 8c5eee9

Browse files
mtojekmtojek
committed
verify: no legacy parameters
1 parent 5261b99 commit 8c5eee9

File tree

2 files changed

+110
-22
lines changed

2 files changed

+110
-22
lines changed

‎coderd/wsbuilder/wsbuilder.go

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,8 @@ type Builder struct {
5656
lastBuildErr *error
5757
lastBuildParameters *[]database.WorkspaceBuildParameter
5858
lastBuildJob *database.ProvisionerJob
59+
60+
verifyNoLegacyParametersOnce bool
5961
}
6062

6163
type Option func(Builder) Builder
@@ -481,6 +483,10 @@ func (b *Builder) getParameters() (names, values []string, err error) {
481483
if err != nil {
482484
return nil, nil, BuildError{http.StatusInternalServerError, "failed to fetch last build parameters", err}
483485
}
486+
err = b.verifyNoLegacyParameters()
487+
if err != nil {
488+
return nil, nil, BuildError{http.StatusBadRequest, "Legacy parameters verification", err}
489+
}
484490
resolver := codersdk.ParameterResolver{
485491
Rich: db2sdk.WorkspaceBuildParameters(lastBuildParameters),
486492
}
@@ -551,6 +557,34 @@ func (b *Builder) getTemplateVersionParameters() ([]database.TemplateVersionPara
551557
return tvp, nil
552558
}
553559

560+
// verifyNoLegacyParameters verifies that initiator can't start the workspace build
561+
// if it uses legacy parameters (database.ParameterValues).
562+
func (b *Builder) verifyNoLegacyParameters() error {
563+
if b.verifyNoLegacyParametersOnce {
564+
return nil
565+
}
566+
b.verifyNoLegacyParametersOnce = true
567+
568+
// Block starting the workspace with legacy parameters.
569+
if b.trans != database.WorkspaceTransitionStart {
570+
return nil
571+
}
572+
573+
_, err := b.store.ParameterValues(b.ctx, database.ParameterValuesParams{
574+
Scopes: []database.ParameterScope{database.ParameterScopeWorkspace},
575+
ScopeIds: []uuid.UUID{b.workspace.ID},
576+
})
577+
if xerrors.Is(err, sql.ErrNoRows) {
578+
return nil
579+
}
580+
if err != nil {
581+
return xerrors.Errorf("get workspace %w parameter values: %w", b.workspace.ID, err)
582+
}
583+
584+
// len(parameterValues) > 0
585+
return xerrors.Errorf("Legacy parameters are not supported anymore, delete the workspace and the related template.")
586+
}
587+
554588
func (b *Builder) getLastBuildJob() (*database.ProvisionerJob, error) {
555589
if b.lastBuildJob != nil {
556590
return b.lastBuildJob, nil

‎coderd/wsbuilder/wsbuilder_test.go

Lines changed: 76 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -23,19 +23,21 @@ import (
2323

2424
var (
2525
// use fixed IDs so logs are easier to read
26-
templateID = uuid.MustParse("12341234-0000-0000-0001-000000000000")
27-
activeVersionID = uuid.MustParse("12341234-0000-0000-0002-000000000000")
28-
inactiveVersionID = uuid.MustParse("12341234-0000-0000-0003-000000000000")
29-
activeJobID = uuid.MustParse("12341234-0000-0000-0004-000000000000")
30-
inactiveJobID = uuid.MustParse("12341234-0000-0000-0005-000000000000")
31-
orgID = uuid.MustParse("12341234-0000-0000-0006-000000000000")
32-
workspaceID = uuid.MustParse("12341234-0000-0000-0007-000000000000")
33-
userID = uuid.MustParse("12341234-0000-0000-0008-000000000000")
34-
activeFileID = uuid.MustParse("12341234-0000-0000-0009-000000000000")
35-
inactiveFileID = uuid.MustParse("12341234-0000-0000-000a-000000000000")
36-
lastBuildID = uuid.MustParse("12341234-0000-0000-000b-000000000000")
37-
lastBuildJobID = uuid.MustParse("12341234-0000-0000-000c-000000000000")
38-
otherUserID = uuid.MustParse("12341234-0000-0000-000d-000000000000")
26+
templateID = uuid.MustParse("12341234-0000-0000-0001-000000000000")
27+
activeVersionID = uuid.MustParse("12341234-0000-0000-0002-000000000000")
28+
inactiveVersionID = uuid.MustParse("12341234-0000-0000-0003-000000000000")
29+
activeJobID = uuid.MustParse("12341234-0000-0000-0004-000000000000")
30+
inactiveJobID = uuid.MustParse("12341234-0000-0000-0005-000000000000")
31+
orgID = uuid.MustParse("12341234-0000-0000-0006-000000000000")
32+
workspaceID = uuid.MustParse("12341234-0000-0000-0007-000000000000")
33+
userID = uuid.MustParse("12341234-0000-0000-0008-000000000000")
34+
activeFileID = uuid.MustParse("12341234-0000-0000-0009-000000000000")
35+
inactiveFileID = uuid.MustParse("12341234-0000-0000-000a-000000000000")
36+
lastBuildID = uuid.MustParse("12341234-0000-0000-000b-000000000000")
37+
lastBuildJobID = uuid.MustParse("12341234-0000-0000-000c-000000000000")
38+
otherUserID = uuid.MustParse("12341234-0000-0000-000d-000000000000")
39+
notReplacedParamID = uuid.MustParse("12341234-0000-0000-000e-000000000000")
40+
replacedParamID = uuid.MustParse("12341234-0000-0000-000f-000000000000")
3941
)
4042

4143
func TestBuilder_NoOptions(t *testing.T) {
@@ -53,7 +55,7 @@ func TestBuilder_NoOptions(t *testing.T) {
5355
withTemplate,
5456
withInactiveVersion(nil),
5557
withLastBuildFound,
56-
withRichParameters(nil),
58+
withRichParameters(nil), withLegacyParameters(nil),
5759

5860
// Outputs
5961
expectProvisionerJob(func(job database.InsertProvisionerJobParams) {
@@ -101,7 +103,7 @@ func TestBuilder_Initiator(t *testing.T) {
101103
withTemplate,
102104
withInactiveVersion(nil),
103105
withLastBuildFound,
104-
withRichParameters(nil),
106+
withRichParameters(nil), withLegacyParameters(nil),
105107

106108
// Outputs
107109
expectProvisionerJob(func(job database.InsertProvisionerJobParams) {
@@ -133,7 +135,7 @@ func TestBuilder_Reason(t *testing.T) {
133135
withTemplate,
134136
withInactiveVersion(nil),
135137
withLastBuildFound,
136-
withRichParameters(nil),
138+
withRichParameters(nil), withLegacyParameters(nil),
137139

138140
// Outputs
139141
expectProvisionerJob(func(job database.InsertProvisionerJobParams) {
@@ -164,6 +166,7 @@ func TestBuilder_ActiveVersion(t *testing.T) {
164166
withTemplate,
165167
withActiveVersion(nil),
166168
withLastBuildNotFound,
169+
withLegacyParameters(nil),
167170
// previous rich parameters are not queried because there is no previous build.
168171

169172
// Outputs
@@ -240,7 +243,7 @@ func TestWorkspaceBuildWithRichParameters(t *testing.T) {
240243
withTemplate,
241244
withInactiveVersion(richParameters),
242245
withLastBuildFound,
243-
withRichParameters(initialBuildParameters),
246+
withRichParameters(initialBuildParameters), withLegacyParameters(nil),
244247

245248
// Outputs
246249
expectProvisionerJob(func(job database.InsertProvisionerJobParams) {}),
@@ -280,7 +283,7 @@ func TestWorkspaceBuildWithRichParameters(t *testing.T) {
280283
withTemplate,
281284
withInactiveVersion(richParameters),
282285
withLastBuildFound,
283-
withRichParameters(initialBuildParameters),
286+
withRichParameters(initialBuildParameters), withLegacyParameters(nil),
284287

285288
// Outputs
286289
expectProvisionerJob(func(job database.InsertProvisionerJobParams) {}),
@@ -301,6 +304,40 @@ func TestWorkspaceBuildWithRichParameters(t *testing.T) {
301304
req.NoError(err)
302305
})
303306

307+
t.Run("StartWorkspaceWithLegacyParameterValues", func(t *testing.T) {
308+
t.Parallel()
309+
310+
req := require.New(t)
311+
asrt := assert.New(t)
312+
313+
ctx, cancel := context.WithCancel(context.Background())
314+
defer cancel()
315+
316+
oldParams := []database.ParameterValue{
317+
{Name: "not-replaced", SourceValue: "nr", ID: notReplacedParamID},
318+
{Name: "replaced", SourceValue: "r", ID: replacedParamID},
319+
}
320+
321+
mDB := expectDB(t,
322+
// Inputs
323+
withTemplate,
324+
withInactiveVersion(richParameters),
325+
withLastBuildFound,
326+
withRichParameters(nil), withLegacyParameters(oldParams),
327+
328+
// Outputs
329+
expectProvisionerJob(func(job database.InsertProvisionerJobParams) {}),
330+
expectBuild(func(bld database.InsertWorkspaceBuildParams) {}),
331+
)
332+
333+
ws := database.Workspace{ID: workspaceID, TemplateID: templateID, OwnerID: userID}
334+
uut := wsbuilder.New(ws, database.WorkspaceTransitionStart)
335+
_, _, err := uut.Build(ctx, mDB, nil)
336+
bldErr := wsbuilder.BuildError{}
337+
req.ErrorAs(err, &bldErr)
338+
asrt.Equal(http.StatusBadRequest, bldErr.Status)
339+
})
340+
304341
t.Run("DoNotModifyImmutables", func(t *testing.T) {
305342
t.Parallel()
306343

@@ -319,7 +356,7 @@ func TestWorkspaceBuildWithRichParameters(t *testing.T) {
319356
withTemplate,
320357
withInactiveVersion(richParameters),
321358
withLastBuildFound,
322-
withRichParameters(initialBuildParameters),
359+
withRichParameters(initialBuildParameters), withLegacyParameters(nil),
323360

324361
// Outputs
325362
expectProvisionerJob(func(job database.InsertProvisionerJobParams) {}),
@@ -370,7 +407,7 @@ func TestWorkspaceBuildWithRichParameters(t *testing.T) {
370407
withTemplate,
371408
withActiveVersion(version2params),
372409
withLastBuildFound,
373-
withRichParameters(initialBuildParameters),
410+
withRichParameters(initialBuildParameters), withLegacyParameters(nil),
374411

375412
// Outputs
376413
expectProvisionerJob(func(job database.InsertProvisionerJobParams) {}),
@@ -427,7 +464,7 @@ func TestWorkspaceBuildWithRichParameters(t *testing.T) {
427464
withTemplate,
428465
withActiveVersion(version2params),
429466
withLastBuildFound,
430-
withRichParameters(initialBuildParameters),
467+
withRichParameters(initialBuildParameters), withLegacyParameters(nil),
431468

432469
// Outputs
433470
expectProvisionerJob(func(job database.InsertProvisionerJobParams) {}),
@@ -482,7 +519,7 @@ func TestWorkspaceBuildWithRichParameters(t *testing.T) {
482519
withTemplate,
483520
withActiveVersion(version2params),
484521
withLastBuildFound,
485-
withRichParameters(initialBuildParameters),
522+
withRichParameters(initialBuildParameters), withLegacyParameters(nil),
486523

487524
// Outputs
488525
expectProvisionerJob(func(job database.InsertProvisionerJobParams) {}),
@@ -659,6 +696,23 @@ func withLastBuildNotFound(mTx *dbmock.MockStore) {
659696
Return(database.WorkspaceBuild{}, sql.ErrNoRows)
660697
}
661698

699+
func withLegacyParameters(params []database.ParameterValue) func(mTx *dbmock.MockStore) {
700+
return func(mTx *dbmock.MockStore) {
701+
c := mTx.EXPECT().ParameterValues(
702+
gomock.Any(),
703+
database.ParameterValuesParams{
704+
Scopes: []database.ParameterScope{database.ParameterScopeWorkspace},
705+
ScopeIds: []uuid.UUID{workspaceID},
706+
}).
707+
Times(1)
708+
if len(params) > 0 {
709+
c.Return(params, nil)
710+
} else {
711+
c.Return(nil, sql.ErrNoRows)
712+
}
713+
}
714+
}
715+
662716
func withRichParameters(params []database.WorkspaceBuildParameter) func(mTx *dbmock.MockStore) {
663717
return func(mTx *dbmock.MockStore) {
664718
c := mTx.EXPECT().GetWorkspaceBuildParameters(gomock.Any(), lastBuildID).

0 commit comments

Comments
 (0)