diff --git a/cli/server.go b/cli/server.go index 59993b5577..1794044bce 100644 --- a/cli/server.go +++ b/cli/server.go @@ -1124,7 +1124,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) + ctx, options.Database, options.Pubsub, 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/apidoc/docs.go b/coderd/apidoc/docs.go index f59fcd308c..95e2cc0f48 100644 --- a/coderd/apidoc/docs.go +++ b/coderd/apidoc/docs.go @@ -11998,6 +11998,10 @@ const docTemplate = `{ "dry_run": { "type": "boolean" }, + "enable_dynamic_parameters": { + "description": "EnableDynamicParameters skips some of the static parameter checking.\nIt will default to whatever the template has marked as the default experience.\nRequires the \"dynamic-experiment\" to be used.", + "type": "boolean" + }, "log_level": { "description": "Log level changes the default logging verbosity of a provider (\"info\" if empty).", "enum": [ diff --git a/coderd/apidoc/swagger.json b/coderd/apidoc/swagger.json index 25f3c21667..02212d9944 100644 --- a/coderd/apidoc/swagger.json +++ b/coderd/apidoc/swagger.json @@ -10716,6 +10716,10 @@ "dry_run": { "type": "boolean" }, + "enable_dynamic_parameters": { + "description": "EnableDynamicParameters skips some of the static parameter checking.\nIt will default to whatever the template has marked as the default experience.\nRequires the \"dynamic-experiment\" to be used.", + "type": "boolean" + }, "log_level": { "description": "Log level changes the default logging verbosity of a provider (\"info\" if empty).", "enum": ["debug"], diff --git a/coderd/autobuild/lifecycle_executor.go b/coderd/autobuild/lifecycle_executor.go index eedcc812bb..b0cba60111 100644 --- a/coderd/autobuild/lifecycle_executor.go +++ b/coderd/autobuild/lifecycle_executor.go @@ -27,6 +27,7 @@ import ( "github.com/coder/coder/v2/coderd/notifications" "github.com/coder/coder/v2/coderd/schedule" "github.com/coder/coder/v2/coderd/wsbuilder" + "github.com/coder/coder/v2/codersdk" ) // Executor automatically starts or stops workspaces. @@ -43,6 +44,7 @@ type Executor struct { // NotificationsEnqueuer handles enqueueing notifications for delivery by SMTP, webhook, etc. notificationsEnqueuer notifications.Enqueuer reg prometheus.Registerer + experiments codersdk.Experiments metrics executorMetrics } @@ -59,7 +61,7 @@ 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) *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 { factory := promauto.With(reg) le := &Executor{ //nolint:gocritic // Autostart has a limited set of permissions. @@ -73,6 +75,7 @@ func NewExecutor(ctx context.Context, db database.Store, ps pubsub.Pubsub, reg p accessControlStore: acs, notificationsEnqueuer: enqueuer, reg: reg, + experiments: exp, metrics: executorMetrics{ autobuildExecutionDuration: factory.NewHistogram(prometheus.HistogramOpts{ Namespace: "coderd", @@ -258,6 +261,7 @@ func (e *Executor) runOnce(t time.Time) Stats { builder := wsbuilder.New(ws, nextTransition). SetLastWorkspaceBuildInTx(&latestBuild). SetLastWorkspaceBuildJobInTx(&latestJob). + Experiments(e.experiments). Reason(reason) log.Debug(e.ctx, "auto building workspace", slog.F("transition", nextTransition)) if nextTransition == database.WorkspaceTransitionStart && diff --git a/coderd/coderdtest/coderdtest.go b/coderd/coderdtest/coderdtest.go index 90a29e0f0d..a8f444c8f6 100644 --- a/coderd/coderdtest/coderdtest.go +++ b/coderd/coderdtest/coderdtest.go @@ -354,6 +354,7 @@ func NewOptions(t testing.TB, options *Options) (func(http.Handler), context.Can auditor.Store(&options.Auditor) ctx, cancelFunc := context.WithCancel(context.Background()) + experiments := coderd.ReadExperiments(*options.Logger, options.DeploymentValues.Experiments) lifecycleExecutor := autobuild.NewExecutor( ctx, options.Database, @@ -365,6 +366,7 @@ func NewOptions(t testing.TB, options *Options) (func(http.Handler), context.Can *options.Logger, options.AutobuildTicker, options.NotificationsEnqueuer, + experiments, ).WithStatsChannel(options.AutobuildStats) lifecycleExecutor.Run() diff --git a/coderd/parameters.go b/coderd/parameters.go index c3fc4ffdee..13b1346991 100644 --- a/coderd/parameters.go +++ b/coderd/parameters.go @@ -12,13 +12,13 @@ import ( "golang.org/x/sync/errgroup" "golang.org/x/xerrors" - "github.com/coder/coder/v2/apiversion" "github.com/coder/coder/v2/coderd/database" "github.com/coder/coder/v2/coderd/database/dbauthz" "github.com/coder/coder/v2/coderd/files" "github.com/coder/coder/v2/coderd/httpapi" "github.com/coder/coder/v2/coderd/httpmw" "github.com/coder/coder/v2/coderd/util/ptr" + "github.com/coder/coder/v2/coderd/wsbuilder" "github.com/coder/coder/v2/codersdk" "github.com/coder/coder/v2/codersdk/wsjson" sdkproto "github.com/coder/coder/v2/provisionersdk/proto" @@ -69,13 +69,10 @@ func (api *API) templateVersionDynamicParameters(rw http.ResponseWriter, r *http return } - major, minor, err := apiversion.Parse(tf.ProvisionerdVersion) - // If the api version is not valid or less than 1.5, we need to use the static parameters - useStaticParams := err != nil || major < 1 || (major == 1 && minor < 6) - if useStaticParams { - api.handleStaticParameters(rw, r, templateVersion.ID) - } else { + if wsbuilder.ProvisionerVersionSupportsDynamicParameters(tf.ProvisionerdVersion) { api.handleDynamicParameters(rw, r, tf, templateVersion) + } else { + api.handleStaticParameters(rw, r, templateVersion.ID) } } diff --git a/coderd/workspacebuilds.go b/coderd/workspacebuilds.go index 719d4e2a48..08b90b834c 100644 --- a/coderd/workspacebuilds.go +++ b/coderd/workspacebuilds.go @@ -338,6 +338,7 @@ func (api *API) postWorkspaceBuilds(rw http.ResponseWriter, r *http.Request) { RichParameterValues(createBuild.RichParameterValues). LogLevel(string(createBuild.LogLevel)). DeploymentValues(api.Options.DeploymentValues). + Experiments(api.Experiments). TemplateVersionPresetID(createBuild.TemplateVersionPresetID) var ( @@ -383,6 +384,22 @@ func (api *API) postWorkspaceBuilds(rw http.ResponseWriter, r *http.Request) { builder = builder.State(createBuild.ProvisionerState) } + // Only defer to dynamic parameters if the experiment is enabled. + if api.Experiments.Enabled(codersdk.ExperimentDynamicParameters) { + if createBuild.EnableDynamicParameters != nil { + // Explicit opt-in + builder = builder.DynamicParameters(*createBuild.EnableDynamicParameters) + } + } else { + if createBuild.EnableDynamicParameters != nil { + api.Logger.Warn(ctx, "ignoring dynamic parameter field sent by request, the experiment is not enabled", + slog.F("field", *createBuild.EnableDynamicParameters), + slog.F("user", apiKey.UserID.String()), + slog.F("transition", string(createBuild.Transition)), + ) + } + } + workspaceBuild, provisionerJob, provisionerDaemons, err = builder.Build( ctx, tx, diff --git a/coderd/workspaces.go b/coderd/workspaces.go index 35960d1f95..fe0c2d3f60 100644 --- a/coderd/workspaces.go +++ b/coderd/workspaces.go @@ -704,6 +704,8 @@ func createWorkspace( Reason(database.BuildReasonInitiator). Initiator(initiatorID). ActiveVersion(). + Experiments(api.Experiments). + DeploymentValues(api.DeploymentValues). RichParameterValues(req.RichParameterValues) if req.TemplateVersionID != uuid.Nil { builder = builder.VersionID(req.TemplateVersionID) @@ -716,7 +718,7 @@ func createWorkspace( } if req.EnableDynamicParameters && api.Experiments.Enabled(codersdk.ExperimentDynamicParameters) { - builder = builder.UsingDynamicParameters() + builder = builder.DynamicParameters(req.EnableDynamicParameters) } workspaceBuild, provisionerJob, provisionerDaemons, err = builder.Build( diff --git a/coderd/wsbuilder/wsbuilder.go b/coderd/wsbuilder/wsbuilder.go index 64389b7532..46035f28dd 100644 --- a/coderd/wsbuilder/wsbuilder.go +++ b/coderd/wsbuilder/wsbuilder.go @@ -13,7 +13,9 @@ import ( "github.com/hashicorp/hcl/v2" "github.com/hashicorp/hcl/v2/hclsyntax" + "github.com/coder/coder/v2/apiversion" "github.com/coder/coder/v2/coderd/rbac/policy" + "github.com/coder/coder/v2/coderd/util/ptr" "github.com/coder/coder/v2/provisioner/terraform/tfparse" "github.com/coder/coder/v2/provisionersdk" sdkproto "github.com/coder/coder/v2/provisionersdk/proto" @@ -51,9 +53,11 @@ type Builder struct { state stateTarget logLevel string deploymentValues *codersdk.DeploymentValues + experiments codersdk.Experiments - richParameterValues []codersdk.WorkspaceBuildParameter - dynamicParametersEnabled bool + richParameterValues []codersdk.WorkspaceBuildParameter + // dynamicParametersEnabled is non-nil if set externally + dynamicParametersEnabled *bool initiator uuid.UUID reason database.BuildReason templateVersionPresetID uuid.UUID @@ -66,6 +70,7 @@ type Builder struct { template *database.Template templateVersion *database.TemplateVersion templateVersionJob *database.ProvisionerJob + terraformValues *database.TemplateVersionTerraformValue templateVersionParameters *[]database.TemplateVersionParameter templateVersionVariables *[]database.TemplateVersionVariable templateVersionWorkspaceTags *[]database.TemplateVersionWorkspaceTag @@ -155,6 +160,14 @@ func (b Builder) DeploymentValues(dv *codersdk.DeploymentValues) Builder { return b } +func (b Builder) Experiments(exp codersdk.Experiments) Builder { + // nolint: revive + cpy := make(codersdk.Experiments, len(exp)) + copy(cpy, exp) + b.experiments = cpy + return b +} + func (b Builder) Initiator(u uuid.UUID) Builder { // nolint: revive b.initiator = u @@ -187,8 +200,9 @@ func (b Builder) MarkPrebuiltWorkspaceClaim() Builder { return b } -func (b Builder) UsingDynamicParameters() Builder { - b.dynamicParametersEnabled = true +func (b Builder) DynamicParameters(using bool) Builder { + // nolint: revive + b.dynamicParametersEnabled = ptr.Ref(using) return b } @@ -516,6 +530,22 @@ func (b *Builder) getTemplateVersionID() (uuid.UUID, error) { return bld.TemplateVersionID, nil } +func (b *Builder) getTemplateTerraformValues() (*database.TemplateVersionTerraformValue, error) { + if b.terraformValues != nil { + return b.terraformValues, nil + } + v, err := b.getTemplateVersion() + if err != nil { + return nil, xerrors.Errorf("get template version so we can get terraform values: %w", err) + } + 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) + } + b.terraformValues = &vals + return b.terraformValues, err +} + func (b *Builder) getLastBuild() (*database.WorkspaceBuild, error) { if b.lastBuild != nil { return b.lastBuild, nil @@ -593,9 +623,10 @@ func (b *Builder) getParameters() (names, values []string, err error) { return nil, nil, BuildError{http.StatusBadRequest, "Unable to build workspace with unsupported parameters", err} } - if b.dynamicParametersEnabled { - // Dynamic parameters skip all parameter validation. - // Pass the user's input as is. + // 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 @@ -989,3 +1020,36 @@ func (b *Builder) checkRunningBuild() error { } return nil } + +func (b *Builder) usingDynamicParameters() bool { + if !b.experiments.Enabled(codersdk.ExperimentDynamicParameters) { + // Experiment required + return false + } + + vals, err := b.getTemplateTerraformValues() + if err != nil { + return false + } + + if !ProvisionerVersionSupportsDynamicParameters(vals.ProvisionerdVersion) { + return false + } + + if b.dynamicParametersEnabled != nil { + return *b.dynamicParametersEnabled + } + + tpl, err := b.getTemplate() + if err != nil { + return false // Let another part of the code get this error + } + return !tpl.UseClassicParameterFlow +} + +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 00b7b5f0ae..abe5e3fe9b 100644 --- a/coderd/wsbuilder/wsbuilder_test.go +++ b/coderd/wsbuilder/wsbuilder_test.go @@ -839,6 +839,32 @@ func TestWorkspaceBuildWithPreset(t *testing.T) { 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 { diff --git a/codersdk/workspaces.go b/codersdk/workspaces.go index b39b220ca3..e0f1b9b1e2 100644 --- a/codersdk/workspaces.go +++ b/codersdk/workspaces.go @@ -110,6 +110,10 @@ type CreateWorkspaceBuildRequest struct { LogLevel ProvisionerLogLevel `json:"log_level,omitempty" validate:"omitempty,oneof=debug"` // TemplateVersionPresetID is the ID of the template version preset to use for the build. TemplateVersionPresetID uuid.UUID `json:"template_version_preset_id,omitempty" format:"uuid"` + // EnableDynamicParameters skips some of the static parameter checking. + // It will default to whatever the template has marked as the default experience. + // Requires the "dynamic-experiment" to be used. + EnableDynamicParameters *bool `json:"enable_dynamic_parameters,omitempty"` } type WorkspaceOptions struct { diff --git a/docs/reference/api/builds.md b/docs/reference/api/builds.md index 00417c700c..3cfd25f2a6 100644 --- a/docs/reference/api/builds.md +++ b/docs/reference/api/builds.md @@ -1731,6 +1731,7 @@ curl -X POST http://coder-server:8080/api/v2/workspaces/{workspace}/builds \ ```json { "dry_run": true, + "enable_dynamic_parameters": true, "log_level": "debug", "orphan": true, "rich_parameter_values": [ diff --git a/docs/reference/api/schemas.md b/docs/reference/api/schemas.md index b35c35361c..9325d751bc 100644 --- a/docs/reference/api/schemas.md +++ b/docs/reference/api/schemas.md @@ -1917,6 +1917,7 @@ This is required on creation to enable a user-flow of validating a template work ```json { "dry_run": true, + "enable_dynamic_parameters": true, "log_level": "debug", "orphan": true, "rich_parameter_values": [ @@ -1939,6 +1940,7 @@ This is required on creation to enable a user-flow of validating a template work | Name | Type | Required | Restrictions | Description | |------------------------------|-------------------------------------------------------------------------------|----------|--------------|---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| | `dry_run` | boolean | false | | | +| `enable_dynamic_parameters` | boolean | false | | Enable dynamic parameters skips some of the static parameter checking. It will default to whatever the template has marked as the default experience. Requires the "dynamic-experiment" to be used. | | `log_level` | [codersdk.ProvisionerLogLevel](#codersdkprovisionerloglevel) | false | | Log level changes the default logging verbosity of a provider ("info" if empty). | | `orphan` | boolean | false | | Orphan may be set for the Destroy transition. | | `rich_parameter_values` | array of [codersdk.WorkspaceBuildParameter](#codersdkworkspacebuildparameter) | false | | Rich parameter values are optional. It will write params to the 'workspace' scope. This will overwrite any existing parameters with the same name. This will not delete old params not included in this list. | diff --git a/enterprise/coderd/workspaces_test.go b/enterprise/coderd/workspaces_test.go index 7005c93ca3..226232f37b 100644 --- a/enterprise/coderd/workspaces_test.go +++ b/enterprise/coderd/workspaces_test.go @@ -1659,6 +1659,119 @@ func TestTemplateDoesNotAllowUserAutostop(t *testing.T) { }) } +// TestWorkspaceTemplateParamsChange tests a workspace with a parameter that +// validation changes on apply. The params used in create workspace are invalid +// according to the static params on import. +// +// This is testing that dynamic params defers input validation to terraform. +// It does not try to do this in coder/coder. +func TestWorkspaceTemplateParamsChange(t *testing.T) { + mainTfTemplate := ` + terraform { + required_providers { + coder = { + source = "coder/coder" + } + } + } + provider "coder" {} + data "coder_workspace" "me" {} + data "coder_workspace_owner" "me" {} + + data "coder_parameter" "param_min" { + name = "param_min" + type = "number" + default = 10 + } + + data "coder_parameter" "param" { + name = "param" + type = "number" + default = 12 + validation { + min = data.coder_parameter.param_min.value + } + } + ` + tfCliConfigPath := downloadProviders(t, mainTfTemplate) + t.Setenv("TF_CLI_CONFIG_FILE", tfCliConfigPath) + + logger := slogtest.Make(t, &slogtest.Options{IgnoreErrors: false}) + dv := coderdtest.DeploymentValues(t) + dv.Experiments = []string{string(codersdk.ExperimentDynamicParameters)} + client, owner := coderdenttest.New(t, &coderdenttest.Options{ + Options: &coderdtest.Options{ + Logger: &logger, + // We intentionally do not run a built-in provisioner daemon here. + IncludeProvisionerDaemon: false, + DeploymentValues: dv, + }, + LicenseOptions: &coderdenttest.LicenseOptions{ + Features: license.Features{ + codersdk.FeatureExternalProvisionerDaemons: 1, + }, + }, + }) + templateAdmin, _ := coderdtest.CreateAnotherUser(t, client, owner.OrganizationID, rbac.RoleTemplateAdmin()) + member, memberUser := coderdtest.CreateAnotherUser(t, client, owner.OrganizationID) + + _ = coderdenttest.NewExternalProvisionerDaemonTerraform(t, client, owner.OrganizationID, nil) + + // This can take a while, so set a relatively long timeout. + ctx := testutil.Context(t, 2*testutil.WaitSuperLong) + + // Creating a template as a template admin must succeed + templateFiles := map[string]string{"main.tf": mainTfTemplate} + tarBytes := testutil.CreateTar(t, templateFiles) + fi, err := templateAdmin.Upload(ctx, "application/x-tar", bytes.NewReader(tarBytes)) + require.NoError(t, err, "failed to upload file") + + tv, err := templateAdmin.CreateTemplateVersion(ctx, owner.OrganizationID, codersdk.CreateTemplateVersionRequest{ + Name: testutil.GetRandomName(t), + FileID: fi.ID, + StorageMethod: codersdk.ProvisionerStorageMethodFile, + Provisioner: codersdk.ProvisionerTypeTerraform, + UserVariableValues: []codersdk.VariableValue{}, + }) + require.NoError(t, err, "failed to create template version") + coderdtest.AwaitTemplateVersionJobCompleted(t, templateAdmin, tv.ID) + tpl := coderdtest.CreateTemplate(t, templateAdmin, owner.OrganizationID, tv.ID) + require.False(t, tpl.UseClassicParameterFlow, "template to use dynamic parameters") + + // When: we create a workspace build using the above template but with + // parameter values that are different from those defined in the template. + // The new values are not valid according to the original plan, but are valid. + ws, err := member.CreateUserWorkspace(ctx, memberUser.Username, codersdk.CreateWorkspaceRequest{ + TemplateID: tpl.ID, + Name: coderdtest.RandomUsername(t), + RichParameterValues: []codersdk.WorkspaceBuildParameter{ + { + Name: "param_min", + Value: "5", + }, + { + Name: "param", + Value: "7", + }, + }, + EnableDynamicParameters: true, + }) + + // Then: the build should succeed. The updated value of param_min should be + // used to validate param instead of the value defined in the temp + require.NoError(t, err, "failed to create workspace") + createBuild := coderdtest.AwaitWorkspaceBuildJobCompleted(t, member, ws.LatestBuild.ID) + require.Equal(t, createBuild.Status, codersdk.WorkspaceStatusRunning) + + // Now delete the workspace + build, err := member.CreateWorkspaceBuild(ctx, ws.ID, codersdk.CreateWorkspaceBuildRequest{ + Transition: codersdk.WorkspaceTransitionDelete, + }) + require.NoError(t, err) + build = coderdtest.AwaitWorkspaceBuildJobCompleted(t, member, build.ID) + require.Equal(t, codersdk.WorkspaceStatusDeleted, build.Status) +} + // TestWorkspaceTagsTerraform tests that a workspace can be created with tags. // This is an end-to-end-style test, meaning that we actually run the // real Terraform provisioner and validate that the workspace is created diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index 9a73fc9f3d..d367302186 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -490,6 +490,7 @@ export interface CreateWorkspaceBuildRequest { readonly rich_parameter_values?: readonly WorkspaceBuildParameter[]; readonly log_level?: ProvisionerLogLevel; readonly template_version_preset_id?: string; + readonly enable_dynamic_parameters?: boolean; } // From codersdk/workspaceproxy.go