feat: implement dynamic parameter validation (#18482)

# What does this do?

This does parameter validation for dynamic parameters in `wsbuilder`. All input parameters are validated in `coder/coder` before being sent to terraform.

The heart of this PR is [`ResolveParameters`](https://github.com/coder/coder/blob/b65001e89c0577199a8e470c138c51e91cf2350c/coderd/dynamicparameters/resolver.go#L30-L30).

# What else changes?

`wsbuilder` now needs to load the terraform files into memory to succeed. This does add a larger memory requirement to workspace builds.

# Future work

- Sort autostart handling workspaces by template version id. So workspaces with the same template version only load the terraform files once from the db, and store them in the cache.
This commit is contained in:
Steven Masley
2025-06-23 12:35:15 -05:00
committed by GitHub
parent 7254c08af4
commit 82af2e019d
23 changed files with 961 additions and 159 deletions
+53 -46
View File
@@ -8,6 +8,10 @@ import (
"testing"
"time"
"github.com/prometheus/client_golang/prometheus"
"github.com/coder/coder/v2/coderd/coderdtest"
"github.com/coder/coder/v2/coderd/files"
"github.com/coder/coder/v2/provisionersdk"
"github.com/google/uuid"
@@ -94,11 +98,12 @@ func TestBuilder_NoOptions(t *testing.T) {
asrt.Empty(params.Value)
}),
)
fc := files.New(prometheus.NewRegistry(), &coderdtest.FakeAuthorizer{})
ws := database.Workspace{ID: workspaceID, TemplateID: templateID, OwnerID: userID}
uut := wsbuilder.New(ws, database.WorkspaceTransitionStart)
// nolint: dogsled
_, _, _, err := uut.Build(ctx, mDB, nil, audit.WorkspaceBuildBaggage{})
_, _, _, err := uut.Build(ctx, mDB, fc, nil, audit.WorkspaceBuildBaggage{})
req.NoError(err)
}
@@ -133,11 +138,12 @@ func TestBuilder_Initiator(t *testing.T) {
}),
withBuild,
)
fc := files.New(prometheus.NewRegistry(), &coderdtest.FakeAuthorizer{})
ws := database.Workspace{ID: workspaceID, TemplateID: templateID, OwnerID: userID}
uut := wsbuilder.New(ws, database.WorkspaceTransitionStart).Initiator(otherUserID)
// nolint: dogsled
_, _, _, err := uut.Build(ctx, mDB, nil, audit.WorkspaceBuildBaggage{})
_, _, _, err := uut.Build(ctx, mDB, fc, nil, audit.WorkspaceBuildBaggage{})
req.NoError(err)
}
@@ -178,11 +184,12 @@ func TestBuilder_Baggage(t *testing.T) {
}),
withBuild,
)
fc := files.New(prometheus.NewRegistry(), &coderdtest.FakeAuthorizer{})
ws := database.Workspace{ID: workspaceID, TemplateID: templateID, OwnerID: userID}
uut := wsbuilder.New(ws, database.WorkspaceTransitionStart).Initiator(otherUserID)
// nolint: dogsled
_, _, _, err := uut.Build(ctx, mDB, nil, audit.WorkspaceBuildBaggage{IP: "127.0.0.1"})
_, _, _, err := uut.Build(ctx, mDB, fc, nil, audit.WorkspaceBuildBaggage{IP: "127.0.0.1"})
req.NoError(err)
}
@@ -216,11 +223,12 @@ func TestBuilder_Reason(t *testing.T) {
}),
withBuild,
)
fc := files.New(prometheus.NewRegistry(), &coderdtest.FakeAuthorizer{})
ws := database.Workspace{ID: workspaceID, TemplateID: templateID, OwnerID: userID}
uut := wsbuilder.New(ws, database.WorkspaceTransitionStart).Reason(database.BuildReasonAutostart)
// nolint: dogsled
_, _, _, err := uut.Build(ctx, mDB, nil, audit.WorkspaceBuildBaggage{})
_, _, _, err := uut.Build(ctx, mDB, fc, nil, audit.WorkspaceBuildBaggage{})
req.NoError(err)
}
@@ -259,11 +267,12 @@ func TestBuilder_ActiveVersion(t *testing.T) {
}),
withBuild,
)
fc := files.New(prometheus.NewRegistry(), &coderdtest.FakeAuthorizer{})
ws := database.Workspace{ID: workspaceID, TemplateID: templateID, OwnerID: userID}
uut := wsbuilder.New(ws, database.WorkspaceTransitionStart).ActiveVersion()
// nolint: dogsled
_, _, _, err := uut.Build(ctx, mDB, nil, audit.WorkspaceBuildBaggage{})
_, _, _, err := uut.Build(ctx, mDB, fc, nil, audit.WorkspaceBuildBaggage{})
req.NoError(err)
}
@@ -373,11 +382,12 @@ func TestWorkspaceBuildWithTags(t *testing.T) {
}),
withBuild,
)
fc := files.New(prometheus.NewRegistry(), &coderdtest.FakeAuthorizer{})
ws := database.Workspace{ID: workspaceID, TemplateID: templateID, OwnerID: userID}
uut := wsbuilder.New(ws, database.WorkspaceTransitionStart).RichParameterValues(buildParameters)
// nolint: dogsled
_, _, _, err := uut.Build(ctx, mDB, nil, audit.WorkspaceBuildBaggage{})
_, _, _, err := uut.Build(ctx, mDB, fc, nil, audit.WorkspaceBuildBaggage{})
req.NoError(err)
}
@@ -455,11 +465,12 @@ func TestWorkspaceBuildWithRichParameters(t *testing.T) {
}),
withBuild,
)
fc := files.New(prometheus.NewRegistry(), &coderdtest.FakeAuthorizer{})
ws := database.Workspace{ID: workspaceID, TemplateID: templateID, OwnerID: userID}
uut := wsbuilder.New(ws, database.WorkspaceTransitionStart).RichParameterValues(nextBuildParameters)
// nolint: dogsled
_, _, _, err := uut.Build(ctx, mDB, nil, audit.WorkspaceBuildBaggage{})
_, _, _, err := uut.Build(ctx, mDB, fc, nil, audit.WorkspaceBuildBaggage{})
req.NoError(err)
})
t.Run("UsePreviousParameterValues", func(t *testing.T) {
@@ -502,11 +513,12 @@ func TestWorkspaceBuildWithRichParameters(t *testing.T) {
}),
withBuild,
)
fc := files.New(prometheus.NewRegistry(), &coderdtest.FakeAuthorizer{})
ws := database.Workspace{ID: workspaceID, TemplateID: templateID, OwnerID: userID}
uut := wsbuilder.New(ws, database.WorkspaceTransitionStart).RichParameterValues(nextBuildParameters)
// nolint: dogsled
_, _, _, err := uut.Build(ctx, mDB, nil, audit.WorkspaceBuildBaggage{})
_, _, _, err := uut.Build(ctx, mDB, fc, nil, audit.WorkspaceBuildBaggage{})
req.NoError(err)
})
@@ -533,17 +545,17 @@ func TestWorkspaceBuildWithRichParameters(t *testing.T) {
mDB := expectDB(t,
// Inputs
withTemplate,
withInactiveVersion(richParameters),
withInactiveVersionNoParams(),
withLastBuildFound,
withTemplateVersionVariables(inactiveVersionID, nil),
withRichParameters(nil),
withParameterSchemas(inactiveJobID, schemas),
withWorkspaceTags(inactiveVersionID, nil),
)
fc := files.New(prometheus.NewRegistry(), &coderdtest.FakeAuthorizer{})
ws := database.Workspace{ID: workspaceID, TemplateID: templateID, OwnerID: userID}
uut := wsbuilder.New(ws, database.WorkspaceTransitionStart)
_, _, _, err := uut.Build(ctx, mDB, nil, audit.WorkspaceBuildBaggage{})
_, _, _, err := uut.Build(ctx, mDB, fc, nil, audit.WorkspaceBuildBaggage{})
bldErr := wsbuilder.BuildError{}
req.ErrorAs(err, &bldErr)
asrt.Equal(http.StatusBadRequest, bldErr.Status)
@@ -575,11 +587,12 @@ func TestWorkspaceBuildWithRichParameters(t *testing.T) {
// Outputs
// no transaction, since we failed fast while validation build parameters
)
fc := files.New(prometheus.NewRegistry(), &coderdtest.FakeAuthorizer{})
ws := database.Workspace{ID: workspaceID, TemplateID: templateID, OwnerID: userID}
uut := wsbuilder.New(ws, database.WorkspaceTransitionStart).RichParameterValues(nextBuildParameters)
// nolint: dogsled
_, _, _, err := uut.Build(ctx, mDB, nil, audit.WorkspaceBuildBaggage{})
_, _, _, err := uut.Build(ctx, mDB, fc, nil, audit.WorkspaceBuildBaggage{})
bldErr := wsbuilder.BuildError{}
req.ErrorAs(err, &bldErr)
asrt.Equal(http.StatusBadRequest, bldErr.Status)
@@ -639,12 +652,13 @@ func TestWorkspaceBuildWithRichParameters(t *testing.T) {
}),
withBuild,
)
fc := files.New(prometheus.NewRegistry(), &coderdtest.FakeAuthorizer{})
ws := database.Workspace{ID: workspaceID, TemplateID: templateID, OwnerID: userID}
uut := wsbuilder.New(ws, database.WorkspaceTransitionStart).
RichParameterValues(nextBuildParameters).
VersionID(activeVersionID)
_, _, _, err := uut.Build(ctx, mDB, nil, audit.WorkspaceBuildBaggage{})
_, _, _, err := uut.Build(ctx, mDB, fc, nil, audit.WorkspaceBuildBaggage{})
req.NoError(err)
})
@@ -702,12 +716,13 @@ func TestWorkspaceBuildWithRichParameters(t *testing.T) {
}),
withBuild,
)
fc := files.New(prometheus.NewRegistry(), &coderdtest.FakeAuthorizer{})
ws := database.Workspace{ID: workspaceID, TemplateID: templateID, OwnerID: userID}
uut := wsbuilder.New(ws, database.WorkspaceTransitionStart).
RichParameterValues(nextBuildParameters).
VersionID(activeVersionID)
_, _, _, err := uut.Build(ctx, mDB, nil, audit.WorkspaceBuildBaggage{})
_, _, _, err := uut.Build(ctx, mDB, fc, nil, audit.WorkspaceBuildBaggage{})
req.NoError(err)
})
@@ -763,13 +778,14 @@ func TestWorkspaceBuildWithRichParameters(t *testing.T) {
}),
withBuild,
)
fc := files.New(prometheus.NewRegistry(), &coderdtest.FakeAuthorizer{})
ws := database.Workspace{ID: workspaceID, TemplateID: templateID, OwnerID: userID}
uut := wsbuilder.New(ws, database.WorkspaceTransitionStart).
RichParameterValues(nextBuildParameters).
VersionID(activeVersionID)
// nolint: dogsled
_, _, _, err := uut.Build(ctx, mDB, nil, audit.WorkspaceBuildBaggage{})
_, _, _, err := uut.Build(ctx, mDB, fc, nil, audit.WorkspaceBuildBaggage{})
req.NoError(err)
})
}
@@ -829,13 +845,14 @@ func TestWorkspaceBuildWithPreset(t *testing.T) {
asrt.Empty(params.Value)
}),
)
fc := files.New(prometheus.NewRegistry(), &coderdtest.FakeAuthorizer{})
ws := database.Workspace{ID: workspaceID, TemplateID: templateID, OwnerID: userID}
uut := wsbuilder.New(ws, database.WorkspaceTransitionStart).
ActiveVersion().
TemplateVersionPresetID(presetID)
// nolint: dogsled
_, _, _, err := uut.Build(ctx, mDB, nil, audit.WorkspaceBuildBaggage{})
_, _, _, err := uut.Build(ctx, mDB, fc, nil, audit.WorkspaceBuildBaggage{})
req.NoError(err)
}
@@ -899,8 +916,10 @@ func TestWorkspaceBuildDeleteOrphan(t *testing.T) {
ws := database.Workspace{ID: workspaceID, TemplateID: templateID, OwnerID: userID}
uut := wsbuilder.New(ws, database.WorkspaceTransitionDelete).Orphan()
fc := files.New(prometheus.NewRegistry(), &coderdtest.FakeAuthorizer{})
// nolint: dogsled
_, _, _, err := uut.Build(ctx, mDB, nil, audit.WorkspaceBuildBaggage{})
_, _, _, err := uut.Build(ctx, mDB, fc, nil, audit.WorkspaceBuildBaggage{})
req.NoError(err)
})
@@ -974,38 +993,13 @@ func TestWorkspaceBuildDeleteOrphan(t *testing.T) {
ws := database.Workspace{ID: workspaceID, TemplateID: templateID, OwnerID: userID}
uut := wsbuilder.New(ws, database.WorkspaceTransitionDelete).Orphan()
fc := files.New(prometheus.NewRegistry(), &coderdtest.FakeAuthorizer{})
// nolint: dogsled
_, _, _, err := uut.Build(ctx, mDB, nil, audit.WorkspaceBuildBaggage{})
_, _, _, err := uut.Build(ctx, mDB, fc, nil, audit.WorkspaceBuildBaggage{})
req.NoError(err)
})
}
func TestProvisionerVersionSupportsDynamicParameters(t *testing.T) {
t.Parallel()
for v, dyn := range map[string]bool{
"": false,
"na": false,
"0.0": false,
"0.10": false,
"1.4": false,
"1.5": false,
"1.6": true,
"1.7": true,
"1.8": true,
"2.0": true,
"2.17": true,
"4.0": true,
} {
t.Run(v, func(t *testing.T) {
t.Parallel()
does := wsbuilder.ProvisionerVersionSupportsDynamicParameters(v)
require.Equal(t, dyn, does)
})
}
}
type txExpect func(mTx *dbmock.MockStore)
func expectDB(t *testing.T, opts ...txExpect) *dbmock.MockStore {
@@ -1052,7 +1046,7 @@ func withInTx(mTx *dbmock.MockStore) {
)
}
func withActiveVersion(params []database.TemplateVersionParameter) func(mTx *dbmock.MockStore) {
func withActiveVersionNoParams() func(mTx *dbmock.MockStore) {
return func(mTx *dbmock.MockStore) {
mTx.EXPECT().GetTemplateVersionByID(gomock.Any(), activeVersionID).
Times(1).
@@ -1082,6 +1076,12 @@ func withActiveVersion(params []database.TemplateVersionParameter) func(mTx *dbm
UpdatedAt: time.Now(),
CompletedAt: sql.NullTime{Time: dbtime.Now(), Valid: true},
}, nil)
}
}
func withActiveVersion(params []database.TemplateVersionParameter) func(mTx *dbmock.MockStore) {
return func(mTx *dbmock.MockStore) {
withActiveVersionNoParams()(mTx)
paramsCall := mTx.EXPECT().GetTemplateVersionParameters(gomock.Any(), activeVersionID).
Times(1)
if len(params) > 0 {
@@ -1092,7 +1092,7 @@ func withActiveVersion(params []database.TemplateVersionParameter) func(mTx *dbm
}
}
func withInactiveVersion(params []database.TemplateVersionParameter) func(mTx *dbmock.MockStore) {
func withInactiveVersionNoParams() func(mTx *dbmock.MockStore) {
return func(mTx *dbmock.MockStore) {
mTx.EXPECT().GetTemplateVersionByID(gomock.Any(), inactiveVersionID).
Times(1).
@@ -1122,6 +1122,13 @@ func withInactiveVersion(params []database.TemplateVersionParameter) func(mTx *d
UpdatedAt: time.Now(),
CompletedAt: sql.NullTime{Time: dbtime.Now(), Valid: true},
}, nil)
}
}
func withInactiveVersion(params []database.TemplateVersionParameter) func(mTx *dbmock.MockStore) {
return func(mTx *dbmock.MockStore) {
withInactiveVersionNoParams()(mTx)
paramsCall := mTx.EXPECT().GetTemplateVersionParameters(gomock.Any(), inactiveVersionID).
Times(1)
if len(params) > 0 {