From 0523a2fa22d6ebb9d5d24d15a351bac8107833db Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Fri, 20 Jun 2025 15:03:57 -0500 Subject: [PATCH] feat: implement dynamic parameter validation Validation to occur in coder/coder inside wsbuilder for all dynamic parameters. --- cli/server.go | 2 +- coderd/autobuild/lifecycle_executor.go | 7 +- coderd/coderdtest/coderdtest.go | 2 + coderd/dynamicparameters/render.go | 4 +- coderd/dynamicparameters/render_test.go | 35 ++ coderd/dynamicparameters/resolver.go | 189 +++++++++++ coderd/parameters_test.go | 40 ++- coderd/testdata/parameters/modules/main.tf | 44 ++- coderd/util/slice/slice.go | 10 + coderd/workspacebuilds.go | 1 + coderd/workspaces.go | 1 + coderd/wsbuilder/wsbuilder.go | 232 +++++++++---- coderd/wsbuilder/wsbuilder_test.go | 99 +++--- enterprise/coderd/coderd.go | 2 +- enterprise/coderd/dynamicparameters_test.go | 311 ++++++++++++++++++ enterprise/coderd/prebuilds/claim_test.go | 4 +- .../coderd/prebuilds/metricscollector_test.go | 8 +- enterprise/coderd/prebuilds/reconcile.go | 5 + enterprise/coderd/prebuilds/reconcile_test.go | 43 ++- .../testdata/parameters/dynamic/main.tf | 16 +- .../testdata/parameters/ephemeral/main.tf | 25 ++ .../testdata/parameters/numbers/main.tf | 20 ++ .../coderd/testdata/parameters/regex/main.tf | 18 + 23 files changed, 960 insertions(+), 158 deletions(-) create mode 100644 coderd/dynamicparameters/render_test.go create mode 100644 coderd/dynamicparameters/resolver.go create mode 100644 enterprise/coderd/testdata/parameters/ephemeral/main.tf create mode 100644 enterprise/coderd/testdata/parameters/numbers/main.tf create mode 100644 enterprise/coderd/testdata/parameters/regex/main.tf diff --git a/cli/server.go b/cli/server.go index 7d587f5e1ae14..9af9965210d72 100644 --- a/cli/server.go +++ b/cli/server.go @@ -1125,7 +1125,7 @@ func (r *RootCmd) Server(newAPI func(context.Context, *coderd.Options) (*coderd. autobuildTicker := time.NewTicker(vals.AutobuildPollInterval.Value()) defer autobuildTicker.Stop() autobuildExecutor := autobuild.NewExecutor( - ctx, options.Database, options.Pubsub, options.PrometheusRegistry, coderAPI.TemplateScheduleStore, &coderAPI.Auditor, coderAPI.AccessControlStore, logger, autobuildTicker.C, options.NotificationsEnqueuer, coderAPI.Experiments) + ctx, options.Database, options.Pubsub, coderAPI.FileCache, options.PrometheusRegistry, coderAPI.TemplateScheduleStore, &coderAPI.Auditor, coderAPI.AccessControlStore, logger, autobuildTicker.C, options.NotificationsEnqueuer, coderAPI.Experiments) autobuildExecutor.Run() jobReaperTicker := time.NewTicker(vals.JobReaperDetectorInterval.Value()) diff --git a/coderd/autobuild/lifecycle_executor.go b/coderd/autobuild/lifecycle_executor.go index b0cba60111335..f71d267028270 100644 --- a/coderd/autobuild/lifecycle_executor.go +++ b/coderd/autobuild/lifecycle_executor.go @@ -17,6 +17,7 @@ import ( "golang.org/x/xerrors" "cdr.dev/slog" + "github.com/coder/coder/v2/coderd/files" "github.com/coder/coder/v2/coderd/audit" "github.com/coder/coder/v2/coderd/database" @@ -35,6 +36,7 @@ type Executor struct { ctx context.Context db database.Store ps pubsub.Pubsub + fileCache *files.Cache templateScheduleStore *atomic.Pointer[schedule.TemplateScheduleStore] accessControlStore *atomic.Pointer[dbauthz.AccessControlStore] auditor *atomic.Pointer[audit.Auditor] @@ -61,13 +63,14 @@ type Stats struct { } // New returns a new wsactions executor. -func NewExecutor(ctx context.Context, db database.Store, ps pubsub.Pubsub, reg prometheus.Registerer, tss *atomic.Pointer[schedule.TemplateScheduleStore], auditor *atomic.Pointer[audit.Auditor], acs *atomic.Pointer[dbauthz.AccessControlStore], log slog.Logger, tick <-chan time.Time, enqueuer notifications.Enqueuer, exp codersdk.Experiments) *Executor { +func NewExecutor(ctx context.Context, db database.Store, ps pubsub.Pubsub, fc *files.Cache, reg prometheus.Registerer, tss *atomic.Pointer[schedule.TemplateScheduleStore], auditor *atomic.Pointer[audit.Auditor], acs *atomic.Pointer[dbauthz.AccessControlStore], log slog.Logger, tick <-chan time.Time, enqueuer notifications.Enqueuer, exp codersdk.Experiments) *Executor { factory := promauto.With(reg) le := &Executor{ //nolint:gocritic // Autostart has a limited set of permissions. ctx: dbauthz.AsAutostart(ctx), db: db, ps: ps, + fileCache: fc, templateScheduleStore: tss, tick: tick, log: log.Named("autobuild"), @@ -276,7 +279,7 @@ func (e *Executor) runOnce(t time.Time) Stats { } } - nextBuild, job, _, err = builder.Build(e.ctx, tx, nil, audit.WorkspaceBuildBaggage{IP: "127.0.0.1"}) + nextBuild, job, _, err = builder.Build(e.ctx, tx, e.fileCache, nil, audit.WorkspaceBuildBaggage{IP: "127.0.0.1"}) if err != nil { return xerrors.Errorf("build workspace with transition %q: %w", nextTransition, err) } diff --git a/coderd/coderdtest/coderdtest.go b/coderd/coderdtest/coderdtest.go index b509db10e3eba..55e62561af60a 100644 --- a/coderd/coderdtest/coderdtest.go +++ b/coderd/coderdtest/coderdtest.go @@ -52,6 +52,7 @@ import ( "cdr.dev/slog" "cdr.dev/slog/sloggers/sloghuman" "cdr.dev/slog/sloggers/slogtest" + "github.com/coder/coder/v2/coderd/files" "github.com/coder/quartz" "github.com/coder/coder/v2/coderd" @@ -359,6 +360,7 @@ func NewOptions(t testing.TB, options *Options) (func(http.Handler), context.Can ctx, options.Database, options.Pubsub, + files.New(prometheus.NewRegistry(), options.Authorizer), prometheus.NewRegistry(), &templateScheduleStore, &auditor, diff --git a/coderd/dynamicparameters/render.go b/coderd/dynamicparameters/render.go index d9ae7f9a4a6b5..ed9b2734b259c 100644 --- a/coderd/dynamicparameters/render.go +++ b/coderd/dynamicparameters/render.go @@ -49,7 +49,7 @@ type loader struct { // Prepare is the entrypoint for this package. It loads the necessary objects & // files from the database and returns a Renderer that can be used to render the // template version's parameters. -func Prepare(ctx context.Context, db database.Store, cache *files.Cache, versionID uuid.UUID, options ...func(r *loader)) (Renderer, error) { +func Prepare(ctx context.Context, db database.Store, cache files.FileAcquirer, versionID uuid.UUID, options ...func(r *loader)) (Renderer, error) { l := &loader{ templateVersionID: versionID, } @@ -137,7 +137,7 @@ func (r *loader) loadData(ctx context.Context, db database.Store) error { // Static parameter rendering is required to support older template versions that // do not have the database state to support dynamic parameters. A constant // warning will be displayed for these template versions. -func (r *loader) Renderer(ctx context.Context, db database.Store, cache *files.Cache) (Renderer, error) { +func (r *loader) Renderer(ctx context.Context, db database.Store, cache files.FileAcquirer) (Renderer, error) { err := r.loadData(ctx, db) if err != nil { return nil, xerrors.Errorf("load data: %w", err) diff --git a/coderd/dynamicparameters/render_test.go b/coderd/dynamicparameters/render_test.go new file mode 100644 index 0000000000000..c71230c14e19b --- /dev/null +++ b/coderd/dynamicparameters/render_test.go @@ -0,0 +1,35 @@ +package dynamicparameters_test + +import ( + "testing" + + "github.com/stretchr/testify/require" + + "github.com/coder/coder/v2/coderd/dynamicparameters" +) + +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 := dynamicparameters.ProvisionerVersionSupportsDynamicParameters(v) + require.Equal(t, dyn, does) + }) + } +} diff --git a/coderd/dynamicparameters/resolver.go b/coderd/dynamicparameters/resolver.go new file mode 100644 index 0000000000000..bda422ad4b840 --- /dev/null +++ b/coderd/dynamicparameters/resolver.go @@ -0,0 +1,189 @@ +package dynamicparameters + +import ( + "context" + "fmt" + + "github.com/google/uuid" + "github.com/hashicorp/hcl/v2" + + "github.com/coder/coder/v2/coderd/database" + "github.com/coder/coder/v2/coderd/util/slice" + "github.com/coder/coder/v2/codersdk" +) + +type parameterValueSource int + +const ( + sourceDefault parameterValueSource = iota + sourcePrevious + sourceBuild + sourcePreset +) + +type parameterValue struct { + Value string + Source parameterValueSource +} + +//nolint:revive // firstbuild is a control flag to turn on immutable validation +func ResolveParameters( + ctx context.Context, + ownerID uuid.UUID, + renderer Renderer, + firstBuild bool, + previousValues []database.WorkspaceBuildParameter, + buildValues []codersdk.WorkspaceBuildParameter, + presetValues []database.TemplateVersionPresetParameter, +) (map[string]string, hcl.Diagnostics) { + previousValuesMap := slice.ToMapFunc(previousValues, func(p database.WorkspaceBuildParameter) (string, string) { + return p.Name, p.Value + }) + + // Start with previous + values := parameterValueMap(slice.ToMapFunc(previousValues, func(p database.WorkspaceBuildParameter) (string, parameterValue) { + return p.Name, parameterValue{Source: sourcePrevious, Value: p.Value} + })) + + // Add build values (overwrite previous values if they exist) + for _, buildValue := range buildValues { + values[buildValue.Name] = parameterValue{Source: sourceBuild, Value: buildValue.Value} + } + + // Add preset values (overwrite previous and build values if they exist) + for _, preset := range presetValues { + values[preset.Name] = parameterValue{Source: sourcePreset, Value: preset.Value} + } + + // originalValues is going to be used to detect if a user tried to change + // an immutable parameter after the first build. + originalValues := make(map[string]parameterValue, len(values)) + for name, value := range values { + // Store the original values for later use. + originalValues[name] = value + } + + // Render the parameters using the values that were supplied to the previous build. + // + // This is how the form should look to the user on their workspace settings page. + // This is the original form truth that our validations should initially be based on. + output, diags := renderer.Render(ctx, ownerID, values.ValuesMap()) + if diags.HasErrors() { + // Top level diagnostics should break the build. Previous values (and new) should + // always be valid. If there is a case where this is not true, then this has to + // be changed to allow the build to continue with a different set of values. + + return nil, diags + } + + // The user's input now needs to be validated against the parameters. + // Mutability & Ephemeral parameters depend on sequential workspace builds. + // + // To enforce these, the user's input values are trimmed based on the + // mutability and ephemeral parameters defined in the template version. + for _, parameter := range output.Parameters { + // Ephemeral parameters should not be taken from the previous build. + // They must always be explicitly set in every build. + // So remove their values if they are sourced from the previous build. + if parameter.Ephemeral { + v := values[parameter.Name] + if v.Source == sourcePrevious { + delete(values, parameter.Name) + } + } + + // Immutable parameters should also not be allowed to be changed from + // the previous build. Remove any values taken from the preset or + // new build params. This forces the value to be the same as it was before. + // + // We do this so the next form render uses the original immutable value. + if !firstBuild && !parameter.Mutable { + delete(values, parameter.Name) + prev, ok := previousValuesMap[parameter.Name] + if ok { + values[parameter.Name] = parameterValue{ + Value: prev, + Source: sourcePrevious, + } + } + } + } + + // This is the final set of values that will be used. Any errors at this stage + // are fatal. Additional validation for immutability has to be done manually. + output, diags = renderer.Render(ctx, ownerID, values.ValuesMap()) + if diags.HasErrors() { + return nil, diags + } + + // parameterNames is going to be used to remove any excess values that were left + // around without a parameter. + parameterNames := make(map[string]struct{}, len(output.Parameters)) + for _, parameter := range output.Parameters { + parameterNames[parameter.Name] = struct{}{} + + if !firstBuild && !parameter.Mutable { + // Immutable parameters should not be changed after the first build. + // They can match the original value though! + if parameter.Value.AsString() != originalValues[parameter.Name].Value { + var src *hcl.Range + if parameter.Source != nil { + src = ¶meter.Source.HCLBlock().TypeRange + } + + // An immutable parameter was changed, which is not allowed. + // Add the failed diagnostic to the output. + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Immutable parameter changed", + Detail: fmt.Sprintf("Parameter %q is not mutable, so it can't be updated after creating a workspace.", parameter.Name), + Subject: src, + }) + } + } + + // TODO: Fix the `hcl.Diagnostics(...)` type casting. It should not be needed. + if hcl.Diagnostics(parameter.Diagnostics).HasErrors() { + // All validation errors are raised here. + diags = diags.Extend(hcl.Diagnostics(parameter.Diagnostics)) + } + + // If the parameter has a value, but it was not set explicitly by the user at any + // build, then save the default value. An example where this is important is if a + // template has a default value of 'region = us-west-2', but the user never sets + // it. If the default value changes to 'region = us-east-1', we want to preserve + // the original value of 'us-west-2' for the existing workspaces. + // + // parameter.Value will be populated from the default at this point. So grab it + // from there. + if _, ok := values[parameter.Name]; !ok && parameter.Value.IsKnown() && parameter.Value.Valid() { + values[parameter.Name] = parameterValue{ + Value: parameter.Value.AsString(), + Source: sourceDefault, + } + } + } + + // Delete any values that do not belong to a parameter. This is to not save + // parameter values that have no effect. These leaky parameter values can cause + // problems in the future, as it makes it challenging to remove values from the + // database + for k := range values { + if _, ok := parameterNames[k]; !ok { + delete(values, k) + } + } + + // Return the values to be saved for the build. + return values.ValuesMap(), diags +} + +type parameterValueMap map[string]parameterValue + +func (p parameterValueMap) ValuesMap() map[string]string { + values := make(map[string]string, len(p)) + for name, paramValue := range p { + values[name] = paramValue.Value + } + return values +} diff --git a/coderd/parameters_test.go b/coderd/parameters_test.go index 794ff8db3354d..a18000673b50e 100644 --- a/coderd/parameters_test.go +++ b/coderd/parameters_test.go @@ -100,10 +100,11 @@ func TestDynamicParametersWithTerraformValues(t *testing.T) { require.Equal(t, -1, preview.ID) require.Empty(t, preview.Diagnostics) - require.Len(t, preview.Parameters, 1) - require.Equal(t, "jetbrains_ide", preview.Parameters[0].Name) - require.True(t, preview.Parameters[0].Value.Valid) - require.Equal(t, "CL", preview.Parameters[0].Value.Value) + require.Len(t, preview.Parameters, 2) + coderdtest.AssertParameter(t, "jetbrains_ide", preview.Parameters). + Exists().Value("CL") + coderdtest.AssertParameter(t, "region", preview.Parameters). + Exists().Value("na") }) // OldProvisioners use the static parameters in the dynamic param flow @@ -241,10 +242,11 @@ func TestDynamicParametersWithTerraformValues(t *testing.T) { require.Equal(t, -1, preview.ID) require.Empty(t, preview.Diagnostics) - require.Len(t, preview.Parameters, 1) - require.Equal(t, "jetbrains_ide", preview.Parameters[0].Name) - require.True(t, preview.Parameters[0].Value.Valid) - require.Equal(t, "CL", preview.Parameters[0].Value.Value) + require.Len(t, preview.Parameters, 2) + coderdtest.AssertParameter(t, "jetbrains_ide", preview.Parameters). + Exists().Value("CL") + coderdtest.AssertParameter(t, "region", preview.Parameters). + Exists().Value("na") _ = stream.Close(websocket.StatusGoingAway) wrk := coderdtest.CreateWorkspace(t, setup.client, setup.template.ID, func(request *codersdk.CreateWorkspaceRequest) { @@ -253,6 +255,10 @@ func TestDynamicParametersWithTerraformValues(t *testing.T) { Name: preview.Parameters[0].Name, Value: "GO", }, + { + Name: preview.Parameters[1].Name, + Value: "eu", + }, } request.EnableDynamicParameters = true }) @@ -260,22 +266,24 @@ func TestDynamicParametersWithTerraformValues(t *testing.T) { params, err := setup.client.WorkspaceBuildParameters(ctx, wrk.LatestBuild.ID) require.NoError(t, err) - require.Len(t, params, 1) - require.Equal(t, "jetbrains_ide", params[0].Name) - require.Equal(t, "GO", params[0].Value) + require.ElementsMatch(t, []codersdk.WorkspaceBuildParameter{ + {Name: "jetbrains_ide", Value: "GO"}, {Name: "region", Value: "eu"}, + }, params) + + regionOptions := []string{"na", "af", "sa", "as"} // A helper function to assert params doTransition := func(t *testing.T, trans codersdk.WorkspaceTransition) { t.Helper() - fooVal := coderdtest.RandomUsername(t) + regionVal := regionOptions[0] + regionOptions = regionOptions[1:] // Choose the next region on the next build + bld, err := setup.client.CreateWorkspaceBuild(ctx, wrk.ID, codersdk.CreateWorkspaceBuildRequest{ TemplateVersionID: setup.template.ActiveVersionID, Transition: trans, RichParameterValues: []codersdk.WorkspaceBuildParameter{ - // No validation, so this should work as is. - // Overwrite the value on each transition - {Name: "foo", Value: fooVal}, + {Name: "region", Value: regionVal}, }, EnableDynamicParameters: ptr.Ref(true), }) @@ -286,7 +294,7 @@ func TestDynamicParametersWithTerraformValues(t *testing.T) { require.NoError(t, err) require.ElementsMatch(t, latestParams, []codersdk.WorkspaceBuildParameter{ {Name: "jetbrains_ide", Value: "GO"}, - {Name: "foo", Value: fooVal}, + {Name: "region", Value: regionVal}, }) } diff --git a/coderd/testdata/parameters/modules/main.tf b/coderd/testdata/parameters/modules/main.tf index 18f14ece154f2..21bb235574d3f 100644 --- a/coderd/testdata/parameters/modules/main.tf +++ b/coderd/testdata/parameters/modules/main.tf @@ -1,5 +1,47 @@ -terraform {} +terraform { + required_providers { + coder = { + source = "coder/coder" + version = "2.5.3" + } + } +} module "jetbrains_gateway" { source = "jetbrains_gateway" } + +data "coder_parameter" "region" { + name = "region" + display_name = "Where would you like to travel to next?" + type = "string" + form_type = "dropdown" + mutable = true + default = "na" + order = 1000 + + option { + name = "North America" + value = "na" + } + + option { + name = "South America" + value = "sa" + } + + option { + name = "Europe" + value = "eu" + } + + option { + name = "Africa" + value = "af" + } + + option { + name = "Asia" + value = "as" + } +} diff --git a/coderd/util/slice/slice.go b/coderd/util/slice/slice.go index 2a510e24d2b53..bb2011c05d1b2 100644 --- a/coderd/util/slice/slice.go +++ b/coderd/util/slice/slice.go @@ -230,3 +230,13 @@ func Convert[F any, T any](a []F, f func(F) T) []T { } return tmp } + +func ToMapFunc[T any, K comparable, V any](a []T, cnv func(t T) (K, V)) map[K]V { + m := make(map[K]V, len(a)) + + for i := range a { + k, v := cnv(a[i]) + m[k] = v + } + return m +} diff --git a/coderd/workspacebuilds.go b/coderd/workspacebuilds.go index 15614f84b4f70..526be7e4e4f89 100644 --- a/coderd/workspacebuilds.go +++ b/coderd/workspacebuilds.go @@ -392,6 +392,7 @@ func (api *API) postWorkspaceBuilds(rw http.ResponseWriter, r *http.Request) { workspaceBuild, provisionerJob, provisionerDaemons, err = builder.Build( ctx, tx, + api.FileCache, func(action policy.Action, object rbac.Objecter) bool { // Special handling for prebuilt workspace deletion if object.RBACObject().Type == rbac.ResourceWorkspace.Type && action == policy.ActionDelete { diff --git a/coderd/workspaces.go b/coderd/workspaces.go index b1520776464c0..58375ae4c4d45 100644 --- a/coderd/workspaces.go +++ b/coderd/workspaces.go @@ -724,6 +724,7 @@ func createWorkspace( workspaceBuild, provisionerJob, provisionerDaemons, err = builder.Build( ctx, db, + api.FileCache, func(action policy.Action, object rbac.Objecter) bool { return api.Authorize(r, action, object) }, diff --git a/coderd/wsbuilder/wsbuilder.go b/coderd/wsbuilder/wsbuilder.go index 577ec5eca4577..aaccd337ee793 100644 --- a/coderd/wsbuilder/wsbuilder.go +++ b/coderd/wsbuilder/wsbuilder.go @@ -13,8 +13,8 @@ import ( "github.com/hashicorp/hcl/v2" "github.com/hashicorp/hcl/v2/hclsyntax" - "github.com/coder/coder/v2/apiversion" "github.com/coder/coder/v2/coderd/dynamicparameters" + "github.com/coder/coder/v2/coderd/files" "github.com/coder/coder/v2/coderd/rbac/policy" "github.com/coder/coder/v2/coderd/util/ptr" "github.com/coder/coder/v2/provisioner/terraform/tfparse" @@ -65,8 +65,9 @@ type Builder struct { templateVersionPresetID uuid.UUID // used during build, makes function arguments less verbose - ctx context.Context - store database.Store + ctx context.Context + store database.Store + fileCache *files.CacheCloser // cache of objects, so we only fetch once template *database.Template @@ -82,7 +83,8 @@ type Builder struct { lastBuildJob *database.ProvisionerJob parameterNames *[]string parameterValues *[]string - templateVersionPresetParameterValues []database.TemplateVersionPresetParameter + templateVersionPresetParameterValues *[]database.TemplateVersionPresetParameter + parameterRender dynamicparameters.Renderer prebuiltWorkspaceBuildStage sdkproto.PrebuiltWorkspaceBuildStage verifyNoLegacyParametersOnce bool @@ -258,6 +260,7 @@ func (e BuildError) Unwrap() error { func (b *Builder) Build( ctx context.Context, store database.Store, + fileCache *files.Cache, authFunc func(action policy.Action, object rbac.Objecter) bool, auditBaggage audit.WorkspaceBuildBaggage, ) ( @@ -269,6 +272,10 @@ func (b *Builder) Build( return nil, nil, nil, xerrors.Errorf("create audit baggage: %w", err) } + b.fileCache = files.NewCacheCloser(fileCache) + // Always close opened files during the build + defer b.fileCache.Close() + // Run the build in a transaction with RepeatableRead isolation, and retries. // RepeatableRead isolation ensures that we get a consistent view of the database while // computing the new build. This simplifies the logic so that we do not need to worry if @@ -592,10 +599,54 @@ func (b *Builder) getTemplateTerraformValues() (*database.TemplateVersionTerrafo } vals, err := b.store.GetTemplateVersionTerraformValues(b.ctx, v.ID) if err != nil { - return nil, xerrors.Errorf("get template version terraform values %s: %w", v.JobID, err) + if !xerrors.Is(err, sql.ErrNoRows) { + return nil, xerrors.Errorf("builder get template version terraform values %s: %w", v.JobID, err) + } + + // Old versions do not have terraform values, so we can ignore ErrNoRows and use an empty value. + vals = database.TemplateVersionTerraformValue{ + TemplateVersionID: v.ID, + UpdatedAt: time.Time{}, + CachedPlan: nil, + CachedModuleFiles: uuid.NullUUID{}, + ProvisionerdVersion: "", + } } b.terraformValues = &vals - return b.terraformValues, err + return b.terraformValues, nil +} + +func (b *Builder) getDynamicParameterRenderer() (dynamicparameters.Renderer, error) { + if b.parameterRender != nil { + return b.parameterRender, nil + } + + tv, err := b.getTemplateVersion() + if err != nil { + return nil, xerrors.Errorf("get template version to get parameters: %w", err) + } + + job, err := b.getTemplateVersionJob() + if err != nil { + return nil, xerrors.Errorf("get template version job to get parameters: %w", err) + } + + tfVals, err := b.getTemplateTerraformValues() + if err != nil { + return nil, xerrors.Errorf("get template version terraform values: %w", err) + } + + renderer, err := dynamicparameters.Prepare(b.ctx, b.store, b.fileCache, tv.ID, + dynamicparameters.WithTemplateVersion(*tv), + dynamicparameters.WithProvisionerJob(*job), + dynamicparameters.WithTerraformValues(*tfVals), + ) + if err != nil { + return nil, xerrors.Errorf("get template version renderer: %w", err) + } + + b.parameterRender = renderer + return renderer, nil } func (b *Builder) getLastBuild() (*database.WorkspaceBuild, error) { @@ -617,6 +668,19 @@ func (b *Builder) getLastBuild() (*database.WorkspaceBuild, error) { return b.lastBuild, nil } +// firstBuild returns true if this is the first build of the workspace, i.e. there are no prior builds. +func (b *Builder) firstBuild() (bool, error) { + _, err := b.getLastBuild() + if xerrors.Is(err, sql.ErrNoRows) { + // first build! + return true, nil + } + if err != nil { + return false, err + } + return false, nil +} + func (b *Builder) getBuildNumber() (int32, error) { bld, err := b.getLastBuild() if xerrors.Is(err, sql.ErrNoRows) { @@ -654,67 +718,91 @@ func (b *Builder) getParameters() (names, values []string, err error) { return *b.parameterNames, *b.parameterValues, nil } - templateVersionParameters, err := b.getTemplateVersionParameters() + // Always reject legacy parameters. + err = b.verifyNoLegacyParameters() if err != nil { - return nil, nil, BuildError{http.StatusInternalServerError, "failed to fetch template version parameters", err} + return nil, nil, BuildError{http.StatusBadRequest, "Unable to build workspace with unsupported parameters", err} + } + + if b.usingDynamicParameters() { + names, values, err = b.getDynamicParameters() + } else { + names, values, err = b.getClassicParameters() } + + if err != nil { + return nil, nil, xerrors.Errorf("get parameters: %w", err) + } + + b.parameterNames = &names + b.parameterValues = &values + return names, values, nil +} + +func (b *Builder) getDynamicParameters() (names, values []string, err error) { lastBuildParameters, err := b.getLastBuildParameters() if err != nil { return nil, nil, BuildError{http.StatusInternalServerError, "failed to fetch last build parameters", err} } - if b.templateVersionPresetID != uuid.Nil { - // Fetch and cache these, since we'll need them to override requested values if a preset was chosen - presetParameters, err := b.store.GetPresetParametersByPresetID(b.ctx, b.templateVersionPresetID) - if err != nil { - return nil, nil, BuildError{http.StatusInternalServerError, "failed to get preset parameters", err} - } - b.templateVersionPresetParameterValues = presetParameters - } - err = b.verifyNoLegacyParameters() + + presetParameterValues, err := b.getPresetParameterValues() if err != nil { - return nil, nil, BuildError{http.StatusBadRequest, "Unable to build workspace with unsupported parameters", err} + return nil, nil, BuildError{http.StatusInternalServerError, "failed to fetch preset parameter values", err} } - lastBuildParameterValues := db2sdk.WorkspaceBuildParameters(lastBuildParameters) - resolver := codersdk.ParameterResolver{ - Rich: lastBuildParameterValues, + render, err := b.getDynamicParameterRenderer() + if err != nil { + return nil, nil, BuildError{http.StatusInternalServerError, "failed to get dynamic parameter renderer", err} } - // Dynamic parameters skip all parameter validation. - // Deleting a workspace also should skip parameter validation. - // Pass the user's input as is. - if b.usingDynamicParameters() { - // TODO: The previous behavior was only to pass param values - // for parameters that exist. Since dynamic params can have - // conditional parameter existence, the static frame of reference - // is not sufficient. So assume the user is correct, or pull in the - // dynamic param code to find the actual parameters. - latestValues := make(map[string]string, len(b.richParameterValues)) - for _, latest := range b.richParameterValues { - latestValues[latest.Name] = latest.Value + firstBuild, err := b.firstBuild() + if err != nil { + return nil, nil, BuildError{http.StatusInternalServerError, "failed to check if first build", err} + } + + buildValues, diagnostics := dynamicparameters.ResolveParameters(b.ctx, b.workspace.OwnerID, render, firstBuild, + lastBuildParameters, + b.richParameterValues, + presetParameterValues) + + if diagnostics.HasErrors() { + // TODO: Improve the error response. The response should include the validations for each failed + // parameter. The response should also indicate it's a validation error or a more general form failure. + // For now, any error is sufficient. + return nil, nil, BuildError{ + Status: http.StatusBadRequest, + Message: fmt.Sprintf("%d errors occurred while resolving parameters", len(diagnostics)), + Wrapped: diagnostics, } + } - // Merge the inputs with values from the previous build. - for _, last := range lastBuildParameterValues { - // TODO: Ideally we use the resolver here and look at parameter - // fields such as 'ephemeral'. This requires loading the terraform - // files. For now, just send the previous inputs as is. - if _, exists := latestValues[last.Name]; exists { - // latestValues take priority, so skip this previous value. - continue - } - names = append(names, last.Name) - values = append(values, last.Value) - } + names = make([]string, 0, len(buildValues)) + values = make([]string, 0, len(buildValues)) + for k, v := range buildValues { + names = append(names, k) + values = append(values, v) + } - for _, value := range b.richParameterValues { - names = append(names, value.Name) - values = append(values, value.Value) - } + return names, values, nil +} + +func (b *Builder) getClassicParameters() (names, values []string, err error) { + templateVersionParameters, err := b.getTemplateVersionParameters() + if err != nil { + return nil, nil, BuildError{http.StatusInternalServerError, "failed to fetch template version parameters", err} + } + lastBuildParameters, err := b.getLastBuildParameters() + if err != nil { + return nil, nil, BuildError{http.StatusInternalServerError, "failed to fetch last build parameters", err} + } + presetParameterValues, err := b.getPresetParameterValues() + if err != nil { + return nil, nil, BuildError{http.StatusInternalServerError, "failed to fetch preset parameter values", err} + } - b.parameterNames = &names - b.parameterValues = &values - return names, values, nil + lastBuildParameterValues := db2sdk.WorkspaceBuildParameters(lastBuildParameters) + resolver := codersdk.ParameterResolver{ + Rich: lastBuildParameterValues, } for _, templateVersionParameter := range templateVersionParameters { @@ -725,7 +813,7 @@ func (b *Builder) getParameters() (names, values []string, err error) { value, err := resolver.ValidateResolve( tvp, - b.findNewBuildParameterValue(templateVersionParameter.Name), + b.findNewBuildParameterValue(templateVersionParameter.Name, presetParameterValues), ) if err != nil { // At this point, we've queried all the data we need from the database, @@ -743,8 +831,8 @@ func (b *Builder) getParameters() (names, values []string, err error) { return names, values, nil } -func (b *Builder) findNewBuildParameterValue(name string) *codersdk.WorkspaceBuildParameter { - for _, v := range b.templateVersionPresetParameterValues { +func (b *Builder) findNewBuildParameterValue(name string, presets []database.TemplateVersionPresetParameter) *codersdk.WorkspaceBuildParameter { + for _, v := range presets { if v.Name == name { return &codersdk.WorkspaceBuildParameter{ Name: v.Name, @@ -949,6 +1037,24 @@ func (b *Builder) getTemplateVersionWorkspaceTags() ([]database.TemplateVersionW return *b.templateVersionWorkspaceTags, nil } +func (b *Builder) getPresetParameterValues() ([]database.TemplateVersionPresetParameter, error) { + if b.templateVersionPresetParameterValues != nil { + return *b.templateVersionPresetParameterValues, nil + } + + if b.templateVersionPresetID == uuid.Nil { + return []database.TemplateVersionPresetParameter{}, nil + } + + // Fetch and cache these, since we'll need them to override requested values if a preset was chosen + presetParameters, err := b.store.GetPresetParametersByPresetID(b.ctx, b.templateVersionPresetID) + if err != nil { + return nil, xerrors.Errorf("failed to get preset parameters: %w", err) + } + b.templateVersionPresetParameterValues = ptr.Ref(presetParameters) + return *b.templateVersionPresetParameterValues, nil +} + // authorize performs build authorization pre-checks using the provided authFunc func (b *Builder) authorize(authFunc func(action policy.Action, object rbac.Objecter) bool) error { // Doing this up front saves a lot of work if the user doesn't have permission. @@ -1117,21 +1223,5 @@ func (b *Builder) usingDynamicParameters() bool { return false } - vals, err := b.getTemplateTerraformValues() - if err != nil { - return false - } - - if !ProvisionerVersionSupportsDynamicParameters(vals.ProvisionerdVersion) { - return false - } - return true } - -func ProvisionerVersionSupportsDynamicParameters(version string) bool { - major, minor, err := apiversion.Parse(version) - // If the api version is not valid or less than 1.6, we need to use the static parameters - useStaticParams := err != nil || major < 1 || (major == 1 && minor < 6) - return !useStaticParams -} diff --git a/coderd/wsbuilder/wsbuilder_test.go b/coderd/wsbuilder/wsbuilder_test.go index f6b4756455f14..f07e2f99f774a 100644 --- a/coderd/wsbuilder/wsbuilder_test.go +++ b/coderd/wsbuilder/wsbuilder_test.go @@ -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 { diff --git a/enterprise/coderd/coderd.go b/enterprise/coderd/coderd.go index f46848812a69e..9b168c8e3f366 100644 --- a/enterprise/coderd/coderd.go +++ b/enterprise/coderd/coderd.go @@ -1164,7 +1164,7 @@ func (api *API) setupPrebuilds(featureEnabled bool) (agplprebuilds.Reconciliatio return agplprebuilds.DefaultReconciler, agplprebuilds.DefaultClaimer } - reconciler := prebuilds.NewStoreReconciler(api.Database, api.Pubsub, api.DeploymentValues.Prebuilds, + reconciler := prebuilds.NewStoreReconciler(api.Database, api.Pubsub, api.AGPL.FileCache, api.DeploymentValues.Prebuilds, api.Logger.Named("prebuilds"), quartz.NewReal(), api.PrometheusRegistry, api.NotificationsEnqueuer) return reconciler, prebuilds.NewEnterpriseClaimer(api.Database) } diff --git a/enterprise/coderd/dynamicparameters_test.go b/enterprise/coderd/dynamicparameters_test.go index 60d68fecd87d1..8bbe5da470f42 100644 --- a/enterprise/coderd/dynamicparameters_test.go +++ b/enterprise/coderd/dynamicparameters_test.go @@ -1,15 +1,19 @@ package coderd_test import ( + "context" _ "embed" "os" "testing" + "github.com/google/uuid" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/coder/coder/v2/coderd/coderdtest" "github.com/coder/coder/v2/coderd/database" "github.com/coder/coder/v2/coderd/rbac" + "github.com/coder/coder/v2/coderd/util/slice" "github.com/coder/coder/v2/codersdk" "github.com/coder/coder/v2/enterprise/coderd/coderdenttest" "github.com/coder/coder/v2/enterprise/coderd/license" @@ -17,6 +21,289 @@ import ( "github.com/coder/websocket" ) +func TestDynamicParameterBuild(t *testing.T) { + t.Parallel() + + owner, _, _, first := coderdenttest.NewWithAPI(t, &coderdenttest.Options{ + Options: &coderdtest.Options{IncludeProvisionerDaemon: true}, + LicenseOptions: &coderdenttest.LicenseOptions{ + Features: license.Features{ + codersdk.FeatureTemplateRBAC: 1, + }, + }, + }) + + orgID := first.OrganizationID + + templateAdmin, templateAdminData := coderdtest.CreateAnotherUser(t, owner, orgID, rbac.ScopedRoleOrgTemplateAdmin(orgID)) + + coderdtest.CreateGroup(t, owner, orgID, "developer") + coderdtest.CreateGroup(t, owner, orgID, "admin", templateAdminData) + coderdtest.CreateGroup(t, owner, orgID, "auditor") + + // Create a set of templates to test with + numberValidation, _ := coderdtest.DynamicParameterTemplate(t, templateAdmin, orgID, coderdtest.DynamicParameterTemplateParams{ + MainTF: string(must(os.ReadFile("testdata/parameters/numbers/main.tf"))), + }) + + regexValidation, _ := coderdtest.DynamicParameterTemplate(t, templateAdmin, orgID, coderdtest.DynamicParameterTemplateParams{ + MainTF: string(must(os.ReadFile("testdata/parameters/regex/main.tf"))), + }) + + ephemeralValidation, _ := coderdtest.DynamicParameterTemplate(t, templateAdmin, orgID, coderdtest.DynamicParameterTemplateParams{ + MainTF: string(must(os.ReadFile("testdata/parameters/ephemeral/main.tf"))), + }) + + // complexValidation does conditional parameters, conditional options, and more. + complexValidation, _ := coderdtest.DynamicParameterTemplate(t, templateAdmin, orgID, coderdtest.DynamicParameterTemplateParams{ + MainTF: string(must(os.ReadFile("testdata/parameters/dynamic/main.tf"))), + }) + + t.Run("NumberValidation", func(t *testing.T) { + t.Parallel() + + t.Run("OK", func(t *testing.T) { + t.Parallel() + ctx := testutil.Context(t, testutil.WaitShort) + wrk, err := templateAdmin.CreateUserWorkspace(ctx, codersdk.Me, codersdk.CreateWorkspaceRequest{ + TemplateID: numberValidation.ID, + Name: coderdtest.RandomUsername(t), + RichParameterValues: []codersdk.WorkspaceBuildParameter{ + {Name: "number", Value: `7`}, + }, + }) + require.NoError(t, err) + coderdtest.AwaitWorkspaceBuildJobCompleted(t, templateAdmin, wrk.LatestBuild.ID) + }) + + t.Run("TooLow", func(t *testing.T) { + t.Parallel() + ctx := testutil.Context(t, testutil.WaitShort) + _, err := templateAdmin.CreateUserWorkspace(ctx, codersdk.Me, codersdk.CreateWorkspaceRequest{ + TemplateID: numberValidation.ID, + Name: coderdtest.RandomUsername(t), + RichParameterValues: []codersdk.WorkspaceBuildParameter{ + {Name: "number", Value: `-10`}, + }, + }) + require.ErrorContains(t, err, "Number must be between 0 and 10") + }) + + t.Run("TooHigh", func(t *testing.T) { + t.Parallel() + ctx := testutil.Context(t, testutil.WaitShort) + _, err := templateAdmin.CreateUserWorkspace(ctx, codersdk.Me, codersdk.CreateWorkspaceRequest{ + TemplateID: numberValidation.ID, + Name: coderdtest.RandomUsername(t), + RichParameterValues: []codersdk.WorkspaceBuildParameter{ + {Name: "number", Value: `15`}, + }, + }) + require.ErrorContains(t, err, "Number must be between 0 and 10") + }) + }) + + t.Run("RegexValidation", func(t *testing.T) { + t.Parallel() + + t.Run("OK", func(t *testing.T) { + t.Parallel() + ctx := testutil.Context(t, testutil.WaitShort) + wrk, err := templateAdmin.CreateUserWorkspace(ctx, codersdk.Me, codersdk.CreateWorkspaceRequest{ + TemplateID: regexValidation.ID, + Name: coderdtest.RandomUsername(t), + RichParameterValues: []codersdk.WorkspaceBuildParameter{ + {Name: "string", Value: `Hello World!`}, + }, + }) + require.NoError(t, err) + coderdtest.AwaitWorkspaceBuildJobCompleted(t, templateAdmin, wrk.LatestBuild.ID) + }) + + t.Run("NoValue", func(t *testing.T) { + t.Parallel() + ctx := testutil.Context(t, testutil.WaitShort) + _, err := templateAdmin.CreateUserWorkspace(ctx, codersdk.Me, codersdk.CreateWorkspaceRequest{ + TemplateID: regexValidation.ID, + Name: coderdtest.RandomUsername(t), + RichParameterValues: []codersdk.WorkspaceBuildParameter{}, + }) + require.ErrorContains(t, err, "All messages must start with 'Hello'") + }) + + t.Run("Invalid", func(t *testing.T) { + t.Parallel() + ctx := testutil.Context(t, testutil.WaitShort) + _, err := templateAdmin.CreateUserWorkspace(ctx, codersdk.Me, codersdk.CreateWorkspaceRequest{ + TemplateID: regexValidation.ID, + Name: coderdtest.RandomUsername(t), + RichParameterValues: []codersdk.WorkspaceBuildParameter{ + {Name: "string", Value: `Goodbye!`}, + }, + }) + require.ErrorContains(t, err, "All messages must start with 'Hello'") + }) + }) + + t.Run("EphemeralValidation", func(t *testing.T) { + t.Parallel() + + t.Run("OK_EphemeralNoPrevious", func(t *testing.T) { + t.Parallel() + + // Ephemeral params do not take the previous values into account. + ctx := testutil.Context(t, testutil.WaitShort) + wrk, err := templateAdmin.CreateUserWorkspace(ctx, codersdk.Me, codersdk.CreateWorkspaceRequest{ + TemplateID: ephemeralValidation.ID, + Name: coderdtest.RandomUsername(t), + RichParameterValues: []codersdk.WorkspaceBuildParameter{ + {Name: "required", Value: `Hello World!`}, + {Name: "defaulted", Value: `Changed`}, + }, + }) + require.NoError(t, err) + coderdtest.AwaitWorkspaceBuildJobCompleted(t, templateAdmin, wrk.LatestBuild.ID) + assertWorkspaceBuildParameters(ctx, t, templateAdmin, wrk.LatestBuild.ID, map[string]string{ + "required": "Hello World!", + "defaulted": "Changed", + }) + + bld, err := templateAdmin.CreateWorkspaceBuild(ctx, wrk.ID, codersdk.CreateWorkspaceBuildRequest{ + Transition: codersdk.WorkspaceTransitionStart, + RichParameterValues: []codersdk.WorkspaceBuildParameter{ + {Name: "required", Value: `Hello World, Again!`}, + }, + }) + require.NoError(t, err) + coderdtest.AwaitWorkspaceBuildJobCompleted(t, templateAdmin, bld.ID) + assertWorkspaceBuildParameters(ctx, t, templateAdmin, bld.ID, map[string]string{ + "required": "Hello World, Again!", + "defaulted": "original", // Reverts back to the original default value. + }) + }) + + t.Run("Immutable", func(t *testing.T) { + t.Parallel() + + ctx := testutil.Context(t, testutil.WaitShort) + wrk, err := templateAdmin.CreateUserWorkspace(ctx, codersdk.Me, codersdk.CreateWorkspaceRequest{ + TemplateID: numberValidation.ID, + Name: coderdtest.RandomUsername(t), + RichParameterValues: []codersdk.WorkspaceBuildParameter{ + {Name: "number", Value: `7`}, + }, + }) + require.NoError(t, err) + coderdtest.AwaitWorkspaceBuildJobCompleted(t, templateAdmin, wrk.LatestBuild.ID) + assertWorkspaceBuildParameters(ctx, t, templateAdmin, wrk.LatestBuild.ID, map[string]string{ + "number": "7", + }) + + _, err = templateAdmin.CreateWorkspaceBuild(ctx, wrk.ID, codersdk.CreateWorkspaceBuildRequest{ + Transition: codersdk.WorkspaceTransitionStart, + RichParameterValues: []codersdk.WorkspaceBuildParameter{ + {Name: "number", Value: `8`}, + }, + }) + require.ErrorContains(t, err, `Parameter "number" is not mutable`) + }) + + t.Run("RequiredMissing", func(t *testing.T) { + t.Parallel() + ctx := testutil.Context(t, testutil.WaitShort) + _, err := templateAdmin.CreateUserWorkspace(ctx, codersdk.Me, codersdk.CreateWorkspaceRequest{ + TemplateID: ephemeralValidation.ID, + Name: coderdtest.RandomUsername(t), + RichParameterValues: []codersdk.WorkspaceBuildParameter{}, + }) + require.ErrorContains(t, err, "Required parameter not provided") + }) + }) + + t.Run("ComplexValidation", func(t *testing.T) { + t.Parallel() + + t.Run("OK", func(t *testing.T) { + t.Parallel() + ctx := testutil.Context(t, testutil.WaitShort) + wrk, err := templateAdmin.CreateUserWorkspace(ctx, codersdk.Me, codersdk.CreateWorkspaceRequest{ + TemplateID: complexValidation.ID, + Name: coderdtest.RandomUsername(t), + RichParameterValues: []codersdk.WorkspaceBuildParameter{ + {Name: "groups", Value: `["admin"]`}, + {Name: "colors", Value: `["red"]`}, + {Name: "thing", Value: "apple"}, + }, + }) + require.NoError(t, err) + coderdtest.AwaitWorkspaceBuildJobCompleted(t, templateAdmin, wrk.LatestBuild.ID) + }) + + t.Run("BadGroup", func(t *testing.T) { + // Template admin is not in the "auditor" group, so this should fail. + t.Parallel() + ctx := testutil.Context(t, testutil.WaitShort) + _, err := templateAdmin.CreateUserWorkspace(ctx, codersdk.Me, codersdk.CreateWorkspaceRequest{ + TemplateID: complexValidation.ID, + Name: coderdtest.RandomUsername(t), + RichParameterValues: []codersdk.WorkspaceBuildParameter{ + {Name: "groups", Value: `["auditor", "admin"]`}, + {Name: "colors", Value: `["red"]`}, + {Name: "thing", Value: "apple"}, + }, + }) + require.ErrorContains(t, err, "is not a valid option") + }) + + t.Run("BadColor", func(t *testing.T) { + t.Parallel() + ctx := testutil.Context(t, testutil.WaitShort) + _, err := templateAdmin.CreateUserWorkspace(ctx, codersdk.Me, codersdk.CreateWorkspaceRequest{ + TemplateID: complexValidation.ID, + Name: coderdtest.RandomUsername(t), + RichParameterValues: []codersdk.WorkspaceBuildParameter{ + {Name: "groups", Value: `["admin"]`}, + {Name: "colors", Value: `["purple"]`}, + }, + }) + require.ErrorContains(t, err, "is not a valid option") + require.ErrorContains(t, err, "purple") + }) + + t.Run("BadThing", func(t *testing.T) { + t.Parallel() + ctx := testutil.Context(t, testutil.WaitShort) + _, err := templateAdmin.CreateUserWorkspace(ctx, codersdk.Me, codersdk.CreateWorkspaceRequest{ + TemplateID: complexValidation.ID, + Name: coderdtest.RandomUsername(t), + RichParameterValues: []codersdk.WorkspaceBuildParameter{ + {Name: "groups", Value: `["admin"]`}, + {Name: "colors", Value: `["red"]`}, + {Name: "thing", Value: "leaf"}, + }, + }) + require.ErrorContains(t, err, "must be defined as one of options") + require.ErrorContains(t, err, "leaf") + }) + + t.Run("BadNumber", func(t *testing.T) { + t.Parallel() + ctx := testutil.Context(t, testutil.WaitShort) + _, err := templateAdmin.CreateUserWorkspace(ctx, codersdk.Me, codersdk.CreateWorkspaceRequest{ + TemplateID: complexValidation.ID, + Name: coderdtest.RandomUsername(t), + RichParameterValues: []codersdk.WorkspaceBuildParameter{ + {Name: "groups", Value: `["admin"]`}, + {Name: "colors", Value: `["green"]`}, + {Name: "thing", Value: "leaf"}, + {Name: "number", Value: "100"}, + }, + }) + require.ErrorContains(t, err, "Number must be between 0 and 10") + }) + }) +} + // TestDynamicParameterTemplate uses a template with some dynamic elements, and // tests the parameters, values, etc are all as expected. func TestDynamicParameterTemplate(t *testing.T) { @@ -127,3 +414,27 @@ func TestDynamicParameterTemplate(t *testing.T) { coderdtest.AssertParameter(t, "thing", resp.Parameters). Exists().Value("banana").Options("banana", "ocean", "sky") } + +func assertWorkspaceBuildParameters(ctx context.Context, t *testing.T, client *codersdk.Client, buildID uuid.UUID, values map[string]string) { + t.Helper() + + params, err := client.WorkspaceBuildParameters(ctx, buildID) + require.NoError(t, err) + + for name, value := range values { + param, ok := slice.Find(params, func(parameter codersdk.WorkspaceBuildParameter) bool { + return parameter.Name == name + }) + if !ok { + assert.Failf(t, "parameter not found", "expected parameter %q to exist with value %q", name, value) + continue + } + assert.Equalf(t, value, param.Value, "parameter %q should have value %q", name, value) + } + + for _, param := range params { + if _, ok := values[param.Name]; !ok { + assert.Failf(t, "unexpected parameter", "parameter %q should not exist", param.Name) + } + } +} diff --git a/enterprise/coderd/prebuilds/claim_test.go b/enterprise/coderd/prebuilds/claim_test.go index ec1d6dbbd5a98..67c1f0dd21ade 100644 --- a/enterprise/coderd/prebuilds/claim_test.go +++ b/enterprise/coderd/prebuilds/claim_test.go @@ -15,6 +15,7 @@ import ( "github.com/stretchr/testify/require" "golang.org/x/xerrors" + "github.com/coder/coder/v2/coderd/files" "github.com/coder/quartz" "github.com/coder/coder/v2/coderd/coderdtest" @@ -164,7 +165,8 @@ func TestClaimPrebuild(t *testing.T) { }) defer provisionerCloser.Close() - reconciler := prebuilds.NewStoreReconciler(spy, pubsub, codersdk.PrebuildsConfig{}, logger, quartz.NewMock(t), prometheus.NewRegistry(), newNoopEnqueuer()) + cache := files.New(prometheus.NewRegistry(), &coderdtest.FakeAuthorizer{}) + reconciler := prebuilds.NewStoreReconciler(spy, pubsub, cache, codersdk.PrebuildsConfig{}, logger, quartz.NewMock(t), prometheus.NewRegistry(), newNoopEnqueuer()) var claimer agplprebuilds.Claimer = prebuilds.NewEnterpriseClaimer(spy) api.AGPL.PrebuildsClaimer.Store(&claimer) diff --git a/enterprise/coderd/prebuilds/metricscollector_test.go b/enterprise/coderd/prebuilds/metricscollector_test.go index 3b8461979c87a..057f310fa2bc2 100644 --- a/enterprise/coderd/prebuilds/metricscollector_test.go +++ b/enterprise/coderd/prebuilds/metricscollector_test.go @@ -13,6 +13,8 @@ import ( prometheus_client "github.com/prometheus/client_model/go" "cdr.dev/slog/sloggers/slogtest" + "github.com/coder/coder/v2/coderd/coderdtest" + "github.com/coder/coder/v2/coderd/files" "github.com/coder/quartz" "github.com/coder/coder/v2/coderd/database" @@ -198,7 +200,8 @@ func TestMetricsCollector(t *testing.T) { }) clock := quartz.NewMock(t) db, pubsub := dbtestutil.NewDB(t) - reconciler := prebuilds.NewStoreReconciler(db, pubsub, codersdk.PrebuildsConfig{}, logger, quartz.NewMock(t), prometheus.NewRegistry(), newNoopEnqueuer()) + cache := files.New(prometheus.NewRegistry(), &coderdtest.FakeAuthorizer{}) + reconciler := prebuilds.NewStoreReconciler(db, pubsub, cache, codersdk.PrebuildsConfig{}, logger, quartz.NewMock(t), prometheus.NewRegistry(), newNoopEnqueuer()) ctx := testutil.Context(t, testutil.WaitLong) createdUsers := []uuid.UUID{database.PrebuildsSystemUserID} @@ -334,7 +337,8 @@ func TestMetricsCollector_DuplicateTemplateNames(t *testing.T) { logger := slogtest.Make(t, &slogtest.Options{IgnoreErrors: true}) clock := quartz.NewMock(t) db, pubsub := dbtestutil.NewDB(t) - reconciler := prebuilds.NewStoreReconciler(db, pubsub, codersdk.PrebuildsConfig{}, logger, quartz.NewMock(t), prometheus.NewRegistry(), newNoopEnqueuer()) + cache := files.New(prometheus.NewRegistry(), &coderdtest.FakeAuthorizer{}) + reconciler := prebuilds.NewStoreReconciler(db, pubsub, cache, codersdk.PrebuildsConfig{}, logger, quartz.NewMock(t), prometheus.NewRegistry(), newNoopEnqueuer()) ctx := testutil.Context(t, testutil.WaitLong) collector := prebuilds.NewMetricsCollector(db, logger, reconciler) diff --git a/enterprise/coderd/prebuilds/reconcile.go b/enterprise/coderd/prebuilds/reconcile.go index 911336d36c426..8240ac78a58bc 100644 --- a/enterprise/coderd/prebuilds/reconcile.go +++ b/enterprise/coderd/prebuilds/reconcile.go @@ -14,6 +14,7 @@ import ( "github.com/hashicorp/go-multierror" "github.com/prometheus/client_golang/prometheus" + "github.com/coder/coder/v2/coderd/files" "github.com/coder/quartz" "github.com/coder/coder/v2/coderd/audit" @@ -40,6 +41,7 @@ type StoreReconciler struct { store database.Store cfg codersdk.PrebuildsConfig pubsub pubsub.Pubsub + fileCache *files.Cache logger slog.Logger clock quartz.Clock registerer prometheus.Registerer @@ -57,6 +59,7 @@ var _ prebuilds.ReconciliationOrchestrator = &StoreReconciler{} func NewStoreReconciler(store database.Store, ps pubsub.Pubsub, + fileCache *files.Cache, cfg codersdk.PrebuildsConfig, logger slog.Logger, clock quartz.Clock, @@ -66,6 +69,7 @@ func NewStoreReconciler(store database.Store, reconciler := &StoreReconciler{ store: store, pubsub: ps, + fileCache: fileCache, logger: logger, cfg: cfg, clock: clock, @@ -780,6 +784,7 @@ func (c *StoreReconciler) provision( _, provisionerJob, _, err := builder.Build( ctx, db, + c.fileCache, func(_ policy.Action, _ rbac.Objecter) bool { return true // TODO: harden? }, diff --git a/enterprise/coderd/prebuilds/reconcile_test.go b/enterprise/coderd/prebuilds/reconcile_test.go index de8c65546334e..8524786dec404 100644 --- a/enterprise/coderd/prebuilds/reconcile_test.go +++ b/enterprise/coderd/prebuilds/reconcile_test.go @@ -13,7 +13,9 @@ import ( "github.com/stretchr/testify/assert" "golang.org/x/xerrors" + "github.com/coder/coder/v2/coderd/coderdtest" "github.com/coder/coder/v2/coderd/database/dbtime" + "github.com/coder/coder/v2/coderd/files" "github.com/coder/coder/v2/coderd/notifications" "github.com/coder/coder/v2/coderd/notifications/notificationstest" "github.com/coder/coder/v2/coderd/util/slice" @@ -53,7 +55,8 @@ func TestNoReconciliationActionsIfNoPresets(t *testing.T) { ReconciliationInterval: serpent.Duration(testutil.WaitLong), } logger := testutil.Logger(t) - controller := prebuilds.NewStoreReconciler(db, ps, cfg, logger, quartz.NewMock(t), prometheus.NewRegistry(), newNoopEnqueuer()) + cache := files.New(prometheus.NewRegistry(), &coderdtest.FakeAuthorizer{}) + controller := prebuilds.NewStoreReconciler(db, ps, cache, cfg, logger, quartz.NewMock(t), prometheus.NewRegistry(), newNoopEnqueuer()) // given a template version with no presets org := dbgen.Organization(t, db, database.Organization{}) @@ -98,7 +101,8 @@ func TestNoReconciliationActionsIfNoPrebuilds(t *testing.T) { ReconciliationInterval: serpent.Duration(testutil.WaitLong), } logger := testutil.Logger(t) - controller := prebuilds.NewStoreReconciler(db, ps, cfg, logger, quartz.NewMock(t), prometheus.NewRegistry(), newNoopEnqueuer()) + cache := files.New(prometheus.NewRegistry(), &coderdtest.FakeAuthorizer{}) + controller := prebuilds.NewStoreReconciler(db, ps, cache, cfg, logger, quartz.NewMock(t), prometheus.NewRegistry(), newNoopEnqueuer()) // given there are presets, but no prebuilds org := dbgen.Organization(t, db, database.Organization{}) @@ -375,7 +379,8 @@ func TestPrebuildReconciliation(t *testing.T) { if useBrokenPubsub { pubSub = &brokenPublisher{Pubsub: pubSub} } - controller := prebuilds.NewStoreReconciler(db, pubSub, cfg, logger, quartz.NewMock(t), prometheus.NewRegistry(), newNoopEnqueuer()) + cache := files.New(prometheus.NewRegistry(), &coderdtest.FakeAuthorizer{}) + controller := prebuilds.NewStoreReconciler(db, pubSub, cache, cfg, logger, quartz.NewMock(t), prometheus.NewRegistry(), newNoopEnqueuer()) // Run the reconciliation multiple times to ensure idempotency // 8 was arbitrary, but large enough to reasonably trust the result @@ -452,7 +457,8 @@ func TestMultiplePresetsPerTemplateVersion(t *testing.T) { t, &slogtest.Options{IgnoreErrors: true}, ).Leveled(slog.LevelDebug) db, pubSub := dbtestutil.NewDB(t) - controller := prebuilds.NewStoreReconciler(db, pubSub, cfg, logger, quartz.NewMock(t), prometheus.NewRegistry(), newNoopEnqueuer()) + cache := files.New(prometheus.NewRegistry(), &coderdtest.FakeAuthorizer{}) + controller := prebuilds.NewStoreReconciler(db, pubSub, cache, cfg, logger, quartz.NewMock(t), prometheus.NewRegistry(), newNoopEnqueuer()) ownerID := uuid.New() dbgen.User(t, db, database.User{ @@ -577,7 +583,8 @@ func TestPrebuildScheduling(t *testing.T) { t, &slogtest.Options{IgnoreErrors: true}, ).Leveled(slog.LevelDebug) db, pubSub := dbtestutil.NewDB(t) - controller := prebuilds.NewStoreReconciler(db, pubSub, cfg, logger, clock, prometheus.NewRegistry(), newNoopEnqueuer()) + cache := files.New(prometheus.NewRegistry(), &coderdtest.FakeAuthorizer{}) + controller := prebuilds.NewStoreReconciler(db, pubSub, cache, cfg, logger, clock, prometheus.NewRegistry(), newNoopEnqueuer()) ownerID := uuid.New() dbgen.User(t, db, database.User{ @@ -681,7 +688,8 @@ func TestInvalidPreset(t *testing.T) { t, &slogtest.Options{IgnoreErrors: true}, ).Leveled(slog.LevelDebug) db, pubSub := dbtestutil.NewDB(t) - controller := prebuilds.NewStoreReconciler(db, pubSub, cfg, logger, quartz.NewMock(t), prometheus.NewRegistry(), newNoopEnqueuer()) + cache := files.New(prometheus.NewRegistry(), &coderdtest.FakeAuthorizer{}) + controller := prebuilds.NewStoreReconciler(db, pubSub, cache, cfg, logger, quartz.NewMock(t), prometheus.NewRegistry(), newNoopEnqueuer()) ownerID := uuid.New() dbgen.User(t, db, database.User{ @@ -745,7 +753,8 @@ func TestDeletionOfPrebuiltWorkspaceWithInvalidPreset(t *testing.T) { t, &slogtest.Options{IgnoreErrors: true}, ).Leveled(slog.LevelDebug) db, pubSub := dbtestutil.NewDB(t) - controller := prebuilds.NewStoreReconciler(db, pubSub, cfg, logger, quartz.NewMock(t), prometheus.NewRegistry(), newNoopEnqueuer()) + cache := files.New(prometheus.NewRegistry(), &coderdtest.FakeAuthorizer{}) + controller := prebuilds.NewStoreReconciler(db, pubSub, cache, cfg, logger, quartz.NewMock(t), prometheus.NewRegistry(), newNoopEnqueuer()) ownerID := uuid.New() dbgen.User(t, db, database.User{ @@ -841,7 +850,8 @@ func TestSkippingHardLimitedPresets(t *testing.T) { db, pubSub := dbtestutil.NewDB(t) fakeEnqueuer := newFakeEnqueuer() registry := prometheus.NewRegistry() - controller := prebuilds.NewStoreReconciler(db, pubSub, cfg, logger, clock, registry, fakeEnqueuer) + cache := files.New(prometheus.NewRegistry(), &coderdtest.FakeAuthorizer{}) + controller := prebuilds.NewStoreReconciler(db, pubSub, cache, cfg, logger, clock, registry, fakeEnqueuer) // Template admin to receive a notification. templateAdmin := dbgen.User(t, db, database.User{ @@ -1003,7 +1013,8 @@ func TestHardLimitedPresetShouldNotBlockDeletion(t *testing.T) { db, pubSub := dbtestutil.NewDB(t) fakeEnqueuer := newFakeEnqueuer() registry := prometheus.NewRegistry() - controller := prebuilds.NewStoreReconciler(db, pubSub, cfg, logger, clock, registry, fakeEnqueuer) + cache := files.New(prometheus.NewRegistry(), &coderdtest.FakeAuthorizer{}) + controller := prebuilds.NewStoreReconciler(db, pubSub, cache, cfg, logger, clock, registry, fakeEnqueuer) // Template admin to receive a notification. templateAdmin := dbgen.User(t, db, database.User{ @@ -1215,7 +1226,8 @@ func TestRunLoop(t *testing.T) { t, &slogtest.Options{IgnoreErrors: true}, ).Leveled(slog.LevelDebug) db, pubSub := dbtestutil.NewDB(t) - reconciler := prebuilds.NewStoreReconciler(db, pubSub, cfg, logger, clock, prometheus.NewRegistry(), newNoopEnqueuer()) + cache := files.New(prometheus.NewRegistry(), &coderdtest.FakeAuthorizer{}) + reconciler := prebuilds.NewStoreReconciler(db, pubSub, cache, cfg, logger, clock, prometheus.NewRegistry(), newNoopEnqueuer()) ownerID := uuid.New() dbgen.User(t, db, database.User{ @@ -1345,7 +1357,8 @@ func TestFailedBuildBackoff(t *testing.T) { t, &slogtest.Options{IgnoreErrors: true}, ).Leveled(slog.LevelDebug) db, ps := dbtestutil.NewDB(t) - reconciler := prebuilds.NewStoreReconciler(db, ps, cfg, logger, clock, prometheus.NewRegistry(), newNoopEnqueuer()) + cache := files.New(prometheus.NewRegistry(), &coderdtest.FakeAuthorizer{}) + reconciler := prebuilds.NewStoreReconciler(db, ps, cache, cfg, logger, clock, prometheus.NewRegistry(), newNoopEnqueuer()) // Given: an active template version with presets and prebuilds configured. const desiredInstances = 2 @@ -1461,9 +1474,11 @@ func TestReconciliationLock(t *testing.T) { wg.Add(1) go func() { defer wg.Done() + cache := files.New(prometheus.NewRegistry(), &coderdtest.FakeAuthorizer{}) reconciler := prebuilds.NewStoreReconciler( db, ps, + cache, codersdk.PrebuildsConfig{}, slogtest.Make(t, &slogtest.Options{IgnoreErrors: true}).Leveled(slog.LevelDebug), quartz.NewMock(t), @@ -1501,7 +1516,8 @@ func TestTrackResourceReplacement(t *testing.T) { fakeEnqueuer := newFakeEnqueuer() registry := prometheus.NewRegistry() - reconciler := prebuilds.NewStoreReconciler(db, ps, codersdk.PrebuildsConfig{}, logger, clock, registry, fakeEnqueuer) + cache := files.New(registry, &coderdtest.FakeAuthorizer{}) + reconciler := prebuilds.NewStoreReconciler(db, ps, cache, codersdk.PrebuildsConfig{}, logger, clock, registry, fakeEnqueuer) // Given: a template admin to receive a notification. templateAdmin := dbgen.User(t, db, database.User{ @@ -1656,7 +1672,8 @@ func TestExpiredPrebuildsMultipleActions(t *testing.T) { db, pubSub := dbtestutil.NewDB(t) fakeEnqueuer := newFakeEnqueuer() registry := prometheus.NewRegistry() - controller := prebuilds.NewStoreReconciler(db, pubSub, cfg, logger, clock, registry, fakeEnqueuer) + cache := files.New(registry, &coderdtest.FakeAuthorizer{}) + controller := prebuilds.NewStoreReconciler(db, pubSub, cache, cfg, logger, clock, registry, fakeEnqueuer) // Set up test environment with a template, version, and preset ownerID := uuid.New() diff --git a/enterprise/coderd/testdata/parameters/dynamic/main.tf b/enterprise/coderd/testdata/parameters/dynamic/main.tf index 615f57dc9c074..a6926f46b66a2 100644 --- a/enterprise/coderd/testdata/parameters/dynamic/main.tf +++ b/enterprise/coderd/testdata/parameters/dynamic/main.tf @@ -1,8 +1,7 @@ terraform { required_providers { coder = { - source = "coder/coder" - version = "2.5.3" + source = "coder/coder" } } } @@ -52,6 +51,7 @@ locals { "red" : ["apple", "ruby"] "yellow" : ["banana"] "blue" : ["ocean", "sky"] + "green" : ["grass", "leaf"] } } @@ -101,3 +101,15 @@ data "coder_parameter" "cool" { order = 102 default = "true" } + +data "coder_parameter" "number" { + count = contains(local.selected, "green") ? 1 : 0 + name = "number" + type = "number" + order = 103 + validation { + error = "Number must be between 0 and 10" + min = 0 + max = 10 + } +} diff --git a/enterprise/coderd/testdata/parameters/ephemeral/main.tf b/enterprise/coderd/testdata/parameters/ephemeral/main.tf new file mode 100644 index 0000000000000..f632fcf11aea4 --- /dev/null +++ b/enterprise/coderd/testdata/parameters/ephemeral/main.tf @@ -0,0 +1,25 @@ +terraform { + required_providers { + coder = { + source = "coder/coder" + } + } +} + +data "coder_workspace_owner" "me" {} + +data "coder_parameter" "required" { + name = "required" + type = "string" + mutable = true + ephemeral = true +} + + +data "coder_parameter" "defaulted" { + name = "defaulted" + type = "string" + mutable = true + ephemeral = true + default = "original" +} diff --git a/enterprise/coderd/testdata/parameters/numbers/main.tf b/enterprise/coderd/testdata/parameters/numbers/main.tf new file mode 100644 index 0000000000000..c4950db326419 --- /dev/null +++ b/enterprise/coderd/testdata/parameters/numbers/main.tf @@ -0,0 +1,20 @@ +terraform { + required_providers { + coder = { + source = "coder/coder" + } + } +} + +data "coder_workspace_owner" "me" {} + +data "coder_parameter" "number" { + name = "number" + type = "number" + mutable = false + validation { + error = "Number must be between 0 and 10" + min = 0 + max = 10 + } +} diff --git a/enterprise/coderd/testdata/parameters/regex/main.tf b/enterprise/coderd/testdata/parameters/regex/main.tf new file mode 100644 index 0000000000000..9fbaa5e245056 --- /dev/null +++ b/enterprise/coderd/testdata/parameters/regex/main.tf @@ -0,0 +1,18 @@ +terraform { + required_providers { + coder = { + source = "coder/coder" + } + } +} + +data "coder_workspace_owner" "me" {} + +data "coder_parameter" "string" { + name = "string" + type = "string" + validation { + error = "All messages must start with 'Hello'" + regex = "^Hello" + } +}