Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

chore: disable parameter validatation for dynamic params for all transitions#17926

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
Emyrk merged 16 commits intomainfromstevenmasley/deleting_workspace_params
May 20, 2025
Merged
Show file tree
Hide file tree
Changes fromall commits
Commits
Show all changes
16 commits
Select commitHold shift + click to select a range
d4d52e9
chore: unit test to showcase param validation drift
EmyrkMay 19, 2025
f6b6485
trying some things
EmyrkMay 19, 2025
2b03908
chore: dynamic parameters on workspace builds, not just create
EmyrkMay 20, 2025
266b1e0
working on enabling dynamic params for builds
EmyrkMay 20, 2025
10fe129
working on enabling dynamic params for builds
EmyrkMay 20, 2025
0c648fb
use expierment in detection
EmyrkMay 20, 2025
b90650d
add experiments to wsbuilder
EmyrkMay 20, 2025
671ef69
linting
EmyrkMay 20, 2025
07fc321
make gen
EmyrkMay 20, 2025
93b0974
add provisioenr version check too
EmyrkMay 20, 2025
8100193
add unit test
EmyrkMay 20, 2025
1ce5e7b
Merge remote-tracking branch 'origin/main' into stevenmasley/deleting…
EmyrkMay 20, 2025
1e4c183
linting
EmyrkMay 20, 2025
cfe2120
PR suggestions, mostly comments
EmyrkMay 20, 2025
2cd42e3
Merge remote-tracking branch 'origin/main' into stevenmasley/deleting…
EmyrkMay 20, 2025
0944436
make gen
EmyrkMay 20, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletioncli/server.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -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()

hangDetectorTicker := time.NewTicker(vals.JobHangDetectorInterval.Value())
Expand Down
4 changes: 4 additions & 0 deletionscoderd/apidoc/docs.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more abouthow customized files appear on GitHub.

4 changes: 4 additions & 0 deletionscoderd/apidoc/swagger.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more abouthow customized files appear on GitHub.

6 changes: 5 additions & 1 deletioncoderd/autobuild/lifecycle_executor.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -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.
Expand All@@ -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
}
Expand All@@ -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.
Expand All@@ -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",
Expand DownExpand Up@@ -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 &&
Expand Down
2 changes: 2 additions & 0 deletionscoderd/coderdtest/coderdtest.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -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,
Expand All@@ -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()

Expand Down
11 changes: 4 additions & 7 deletionscoderd/parameters.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -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"
Expand DownExpand Up@@ -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)
}
}

Expand Down
17 changes: 17 additions & 0 deletionscoderd/workspacebuilds.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -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 (
Expand DownExpand Up@@ -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,
Expand Down
4 changes: 3 additions & 1 deletioncoderd/workspaces.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -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)
Expand All@@ -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(
Expand Down
78 changes: 71 additions & 7 deletionscoderd/wsbuilder/wsbuilder.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -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"
Expand DownExpand Up@@ -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
Expand All@@ -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
Expand DownExpand Up@@ -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
Expand DownExpand Up@@ -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
}

Expand DownExpand Up@@ -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
Expand DownExpand Up@@ -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
Expand DownExpand Up@@ -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
}
Comment on lines +1050 to +1055
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

nit: this feels like something that might want to be inprovisionersdk

Copy link
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Hmm. Really tough to know where to place it. I'm going to keep it with wsbuilder right now, since it only affects workspace builds.

I think I'll create a new package for dynamic params more generally as more code is added to support the feature, and I can move it there.

johnstcn reacted with thumbs up emoji
26 changes: 26 additions & 0 deletionscoderd/wsbuilder/wsbuilder_test.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -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 {
Expand Down
4 changes: 4 additions & 0 deletionscodersdk/workspaces.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -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 {
Expand Down
1 change: 1 addition & 0 deletionsdocs/reference/api/builds.md
View file
Open in desktop

Some generated files are not rendered by default. Learn more abouthow customized files appear on GitHub.

2 changes: 2 additions & 0 deletionsdocs/reference/api/schemas.md
View file
Open in desktop

Some generated files are not rendered by default. Learn more abouthow customized files appear on GitHub.

Loading
Loading

[8]ページ先頭

©2009-2025 Movatter.jp