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

Commite76d58f

Browse files
authored
chore: disable parameter validatation for dynamic params for all transitions (#17926)
Dynamic params skip parameter validation in coder/coder.This is because conditional parameters cannot be validated with the static parameters in the database.
1 parent93f17bc commite76d58f

File tree

15 files changed

+258
-17
lines changed

15 files changed

+258
-17
lines changed

‎cli/server.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1124,7 +1124,7 @@ func (r *RootCmd) Server(newAPI func(context.Context, *coderd.Options) (*coderd.
11241124
autobuildTicker:=time.NewTicker(vals.AutobuildPollInterval.Value())
11251125
deferautobuildTicker.Stop()
11261126
autobuildExecutor:=autobuild.NewExecutor(
1127-
ctx,options.Database,options.Pubsub,options.PrometheusRegistry,coderAPI.TemplateScheduleStore,&coderAPI.Auditor,coderAPI.AccessControlStore,logger,autobuildTicker.C,options.NotificationsEnqueuer)
1127+
ctx,options.Database,options.Pubsub,options.PrometheusRegistry,coderAPI.TemplateScheduleStore,&coderAPI.Auditor,coderAPI.AccessControlStore,logger,autobuildTicker.C,options.NotificationsEnqueuer,coderAPI.Experiments)
11281128
autobuildExecutor.Run()
11291129

11301130
jobReaperTicker:=time.NewTicker(vals.JobReaperDetectorInterval.Value())

‎coderd/apidoc/docs.go

Lines changed: 4 additions & 0 deletions
Some generated files are not rendered by default. Learn more aboutcustomizing how changed files appear on GitHub.

‎coderd/apidoc/swagger.json

Lines changed: 4 additions & 0 deletions
Some generated files are not rendered by default. Learn more aboutcustomizing how changed files appear on GitHub.

‎coderd/autobuild/lifecycle_executor.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ import (
2727
"github.com/coder/coder/v2/coderd/notifications"
2828
"github.com/coder/coder/v2/coderd/schedule"
2929
"github.com/coder/coder/v2/coderd/wsbuilder"
30+
"github.com/coder/coder/v2/codersdk"
3031
)
3132

3233
// Executor automatically starts or stops workspaces.
@@ -43,6 +44,7 @@ type Executor struct {
4344
// NotificationsEnqueuer handles enqueueing notifications for delivery by SMTP, webhook, etc.
4445
notificationsEnqueuer notifications.Enqueuer
4546
reg prometheus.Registerer
47+
experiments codersdk.Experiments
4648

4749
metricsexecutorMetrics
4850
}
@@ -59,7 +61,7 @@ type Stats struct {
5961
}
6062

6163
// New returns a new wsactions executor.
62-
funcNewExecutor(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 {
64+
funcNewExecutor(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 {
6365
factory:=promauto.With(reg)
6466
le:=&Executor{
6567
//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
7375
accessControlStore:acs,
7476
notificationsEnqueuer:enqueuer,
7577
reg:reg,
78+
experiments:exp,
7679
metrics:executorMetrics{
7780
autobuildExecutionDuration:factory.NewHistogram(prometheus.HistogramOpts{
7881
Namespace:"coderd",
@@ -258,6 +261,7 @@ func (e *Executor) runOnce(t time.Time) Stats {
258261
builder:=wsbuilder.New(ws,nextTransition).
259262
SetLastWorkspaceBuildInTx(&latestBuild).
260263
SetLastWorkspaceBuildJobInTx(&latestJob).
264+
Experiments(e.experiments).
261265
Reason(reason)
262266
log.Debug(e.ctx,"auto building workspace",slog.F("transition",nextTransition))
263267
ifnextTransition==database.WorkspaceTransitionStart&&

‎coderd/coderdtest/coderdtest.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -354,6 +354,7 @@ func NewOptions(t testing.TB, options *Options) (func(http.Handler), context.Can
354354
auditor.Store(&options.Auditor)
355355

356356
ctx,cancelFunc:=context.WithCancel(context.Background())
357+
experiments:=coderd.ReadExperiments(*options.Logger,options.DeploymentValues.Experiments)
357358
lifecycleExecutor:=autobuild.NewExecutor(
358359
ctx,
359360
options.Database,
@@ -365,6 +366,7 @@ func NewOptions(t testing.TB, options *Options) (func(http.Handler), context.Can
365366
*options.Logger,
366367
options.AutobuildTicker,
367368
options.NotificationsEnqueuer,
369+
experiments,
368370
).WithStatsChannel(options.AutobuildStats)
369371
lifecycleExecutor.Run()
370372

‎coderd/parameters.go

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -12,13 +12,13 @@ import (
1212
"golang.org/x/sync/errgroup"
1313
"golang.org/x/xerrors"
1414

15-
"github.com/coder/coder/v2/apiversion"
1615
"github.com/coder/coder/v2/coderd/database"
1716
"github.com/coder/coder/v2/coderd/database/dbauthz"
1817
"github.com/coder/coder/v2/coderd/files"
1918
"github.com/coder/coder/v2/coderd/httpapi"
2019
"github.com/coder/coder/v2/coderd/httpmw"
2120
"github.com/coder/coder/v2/coderd/util/ptr"
21+
"github.com/coder/coder/v2/coderd/wsbuilder"
2222
"github.com/coder/coder/v2/codersdk"
2323
"github.com/coder/coder/v2/codersdk/wsjson"
2424
sdkproto"github.com/coder/coder/v2/provisionersdk/proto"
@@ -69,13 +69,10 @@ func (api *API) templateVersionDynamicParameters(rw http.ResponseWriter, r *http
6969
return
7070
}
7171

72-
major,minor,err:=apiversion.Parse(tf.ProvisionerdVersion)
73-
// If the api version is not valid or less than 1.5, we need to use the static parameters
74-
useStaticParams:=err!=nil||major<1|| (major==1&&minor<6)
75-
ifuseStaticParams {
76-
api.handleStaticParameters(rw,r,templateVersion.ID)
77-
}else {
72+
ifwsbuilder.ProvisionerVersionSupportsDynamicParameters(tf.ProvisionerdVersion) {
7873
api.handleDynamicParameters(rw,r,tf,templateVersion)
74+
}else {
75+
api.handleStaticParameters(rw,r,templateVersion.ID)
7976
}
8077
}
8178

‎coderd/workspacebuilds.go

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -338,6 +338,7 @@ func (api *API) postWorkspaceBuilds(rw http.ResponseWriter, r *http.Request) {
338338
RichParameterValues(createBuild.RichParameterValues).
339339
LogLevel(string(createBuild.LogLevel)).
340340
DeploymentValues(api.Options.DeploymentValues).
341+
Experiments(api.Experiments).
341342
TemplateVersionPresetID(createBuild.TemplateVersionPresetID)
342343

343344
var (
@@ -383,6 +384,22 @@ func (api *API) postWorkspaceBuilds(rw http.ResponseWriter, r *http.Request) {
383384
builder=builder.State(createBuild.ProvisionerState)
384385
}
385386

387+
// Only defer to dynamic parameters if the experiment is enabled.
388+
ifapi.Experiments.Enabled(codersdk.ExperimentDynamicParameters) {
389+
ifcreateBuild.EnableDynamicParameters!=nil {
390+
// Explicit opt-in
391+
builder=builder.DynamicParameters(*createBuild.EnableDynamicParameters)
392+
}
393+
}else {
394+
ifcreateBuild.EnableDynamicParameters!=nil {
395+
api.Logger.Warn(ctx,"ignoring dynamic parameter field sent by request, the experiment is not enabled",
396+
slog.F("field",*createBuild.EnableDynamicParameters),
397+
slog.F("user",apiKey.UserID.String()),
398+
slog.F("transition",string(createBuild.Transition)),
399+
)
400+
}
401+
}
402+
386403
workspaceBuild,provisionerJob,provisionerDaemons,err=builder.Build(
387404
ctx,
388405
tx,

‎coderd/workspaces.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -704,6 +704,8 @@ func createWorkspace(
704704
Reason(database.BuildReasonInitiator).
705705
Initiator(initiatorID).
706706
ActiveVersion().
707+
Experiments(api.Experiments).
708+
DeploymentValues(api.DeploymentValues).
707709
RichParameterValues(req.RichParameterValues)
708710
ifreq.TemplateVersionID!=uuid.Nil {
709711
builder=builder.VersionID(req.TemplateVersionID)
@@ -716,7 +718,7 @@ func createWorkspace(
716718
}
717719

718720
ifreq.EnableDynamicParameters&&api.Experiments.Enabled(codersdk.ExperimentDynamicParameters) {
719-
builder=builder.UsingDynamicParameters()
721+
builder=builder.DynamicParameters(req.EnableDynamicParameters)
720722
}
721723

722724
workspaceBuild,provisionerJob,provisionerDaemons,err=builder.Build(

‎coderd/wsbuilder/wsbuilder.go

Lines changed: 71 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,9 @@ import (
1313
"github.com/hashicorp/hcl/v2"
1414
"github.com/hashicorp/hcl/v2/hclsyntax"
1515

16+
"github.com/coder/coder/v2/apiversion"
1617
"github.com/coder/coder/v2/coderd/rbac/policy"
18+
"github.com/coder/coder/v2/coderd/util/ptr"
1719
"github.com/coder/coder/v2/provisioner/terraform/tfparse"
1820
"github.com/coder/coder/v2/provisionersdk"
1921
sdkproto"github.com/coder/coder/v2/provisionersdk/proto"
@@ -51,9 +53,11 @@ type Builder struct {
5153
statestateTarget
5254
logLevelstring
5355
deploymentValues*codersdk.DeploymentValues
56+
experiments codersdk.Experiments
5457

55-
richParameterValues []codersdk.WorkspaceBuildParameter
56-
dynamicParametersEnabledbool
58+
richParameterValues []codersdk.WorkspaceBuildParameter
59+
// dynamicParametersEnabled is non-nil if set externally
60+
dynamicParametersEnabled*bool
5761
initiator uuid.UUID
5862
reason database.BuildReason
5963
templateVersionPresetID uuid.UUID
@@ -66,6 +70,7 @@ type Builder struct {
6670
template*database.Template
6771
templateVersion*database.TemplateVersion
6872
templateVersionJob*database.ProvisionerJob
73+
terraformValues*database.TemplateVersionTerraformValue
6974
templateVersionParameters*[]database.TemplateVersionParameter
7075
templateVersionVariables*[]database.TemplateVersionVariable
7176
templateVersionWorkspaceTags*[]database.TemplateVersionWorkspaceTag
@@ -155,6 +160,14 @@ func (b Builder) DeploymentValues(dv *codersdk.DeploymentValues) Builder {
155160
returnb
156161
}
157162

163+
func (bBuilder)Experiments(exp codersdk.Experiments)Builder {
164+
// nolint: revive
165+
cpy:=make(codersdk.Experiments,len(exp))
166+
copy(cpy,exp)
167+
b.experiments=cpy
168+
returnb
169+
}
170+
158171
func (bBuilder)Initiator(u uuid.UUID)Builder {
159172
// nolint: revive
160173
b.initiator=u
@@ -187,8 +200,9 @@ func (b Builder) MarkPrebuiltWorkspaceClaim() Builder {
187200
returnb
188201
}
189202

190-
func (bBuilder)UsingDynamicParameters()Builder {
191-
b.dynamicParametersEnabled=true
203+
func (bBuilder)DynamicParameters(usingbool)Builder {
204+
// nolint: revive
205+
b.dynamicParametersEnabled=ptr.Ref(using)
192206
returnb
193207
}
194208

@@ -516,6 +530,22 @@ func (b *Builder) getTemplateVersionID() (uuid.UUID, error) {
516530
returnbld.TemplateVersionID,nil
517531
}
518532

533+
func (b*Builder)getTemplateTerraformValues() (*database.TemplateVersionTerraformValue,error) {
534+
ifb.terraformValues!=nil {
535+
returnb.terraformValues,nil
536+
}
537+
v,err:=b.getTemplateVersion()
538+
iferr!=nil {
539+
returnnil,xerrors.Errorf("get template version so we can get terraform values: %w",err)
540+
}
541+
vals,err:=b.store.GetTemplateVersionTerraformValues(b.ctx,v.ID)
542+
iferr!=nil {
543+
returnnil,xerrors.Errorf("get template version terraform values %s: %w",v.JobID,err)
544+
}
545+
b.terraformValues=&vals
546+
returnb.terraformValues,err
547+
}
548+
519549
func (b*Builder)getLastBuild() (*database.WorkspaceBuild,error) {
520550
ifb.lastBuild!=nil {
521551
returnb.lastBuild,nil
@@ -593,9 +623,10 @@ func (b *Builder) getParameters() (names, values []string, err error) {
593623
returnnil,nil,BuildError{http.StatusBadRequest,"Unable to build workspace with unsupported parameters",err}
594624
}
595625

596-
ifb.dynamicParametersEnabled {
597-
// Dynamic parameters skip all parameter validation.
598-
// Pass the user's input as is.
626+
// Dynamic parameters skip all parameter validation.
627+
// Deleting a workspace also should skip parameter validation.
628+
// Pass the user's input as is.
629+
ifb.usingDynamicParameters() {
599630
// TODO: The previous behavior was only to pass param values
600631
// for parameters that exist. Since dynamic params can have
601632
// conditional parameter existence, the static frame of reference
@@ -989,3 +1020,36 @@ func (b *Builder) checkRunningBuild() error {
9891020
}
9901021
returnnil
9911022
}
1023+
1024+
func (b*Builder)usingDynamicParameters()bool {
1025+
if!b.experiments.Enabled(codersdk.ExperimentDynamicParameters) {
1026+
// Experiment required
1027+
returnfalse
1028+
}
1029+
1030+
vals,err:=b.getTemplateTerraformValues()
1031+
iferr!=nil {
1032+
returnfalse
1033+
}
1034+
1035+
if!ProvisionerVersionSupportsDynamicParameters(vals.ProvisionerdVersion) {
1036+
returnfalse
1037+
}
1038+
1039+
ifb.dynamicParametersEnabled!=nil {
1040+
return*b.dynamicParametersEnabled
1041+
}
1042+
1043+
tpl,err:=b.getTemplate()
1044+
iferr!=nil {
1045+
returnfalse// Let another part of the code get this error
1046+
}
1047+
return!tpl.UseClassicParameterFlow
1048+
}
1049+
1050+
funcProvisionerVersionSupportsDynamicParameters(versionstring)bool {
1051+
major,minor,err:=apiversion.Parse(version)
1052+
// If the api version is not valid or less than 1.6, we need to use the static parameters
1053+
useStaticParams:=err!=nil||major<1|| (major==1&&minor<6)
1054+
return!useStaticParams
1055+
}

‎coderd/wsbuilder/wsbuilder_test.go

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -839,6 +839,32 @@ func TestWorkspaceBuildWithPreset(t *testing.T) {
839839
req.NoError(err)
840840
}
841841

842+
funcTestProvisionerVersionSupportsDynamicParameters(t*testing.T) {
843+
t.Parallel()
844+
845+
forv,dyn:=rangemap[string]bool{
846+
"":false,
847+
"na":false,
848+
"0.0":false,
849+
"0.10":false,
850+
"1.4":false,
851+
"1.5":false,
852+
"1.6":true,
853+
"1.7":true,
854+
"1.8":true,
855+
"2.0":true,
856+
"2.17":true,
857+
"4.0":true,
858+
} {
859+
t.Run(v,func(t*testing.T) {
860+
t.Parallel()
861+
862+
does:=wsbuilder.ProvisionerVersionSupportsDynamicParameters(v)
863+
require.Equal(t,dyn,does)
864+
})
865+
}
866+
}
867+
842868
typetxExpectfunc(mTx*dbmock.MockStore)
843869

844870
funcexpectDB(t*testing.T,opts...txExpect)*dbmock.MockStore {

‎codersdk/workspaces.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,10 @@ type CreateWorkspaceBuildRequest struct {
110110
LogLevelProvisionerLogLevel`json:"log_level,omitempty" validate:"omitempty,oneof=debug"`
111111
// TemplateVersionPresetID is the ID of the template version preset to use for the build.
112112
TemplateVersionPresetID uuid.UUID`json:"template_version_preset_id,omitempty" format:"uuid"`
113+
// EnableDynamicParameters skips some of the static parameter checking.
114+
// It will default to whatever the template has marked as the default experience.
115+
// Requires the "dynamic-experiment" to be used.
116+
EnableDynamicParameters*bool`json:"enable_dynamic_parameters,omitempty"`
113117
}
114118

115119
typeWorkspaceOptionsstruct {

‎docs/reference/api/builds.md

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more aboutcustomizing how changed files appear on GitHub.

‎docs/reference/api/schemas.md

Lines changed: 2 additions & 0 deletions
Some generated files are not rendered by default. Learn more aboutcustomizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)

[8]ページ先頭

©2009-2025 Movatter.jp