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

Commit82af2e0

Browse files
authored
feat: implement dynamic parameter validation (#18482)
# What does this do?This does parameter validation for dynamic parameters in `wsbuilder`. All input parameters are validated in `coder/coder` before being sent to terraform.The heart of this PR is [`ResolveParameters`](https://github.com/coder/coder/blob/b65001e89c0577199a8e470c138c51e91cf2350c/coderd/dynamicparameters/resolver.go#L30-L30).# What else changes?`wsbuilder` now needs to load the terraform files into memory to succeed. This does add a larger memory requirement to workspace builds.# Future work- Sort autostart handling workspaces by template version id. So workspaces with the same template version only load the terraform files once from the db, and store them in the cache.
1 parent7254c08 commit82af2e0

File tree

23 files changed

+960
-158
lines changed

23 files changed

+960
-158
lines changed

‎cli/server.go

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

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

‎coderd/autobuild/lifecycle_executor.go

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import (
1717
"golang.org/x/xerrors"
1818

1919
"cdr.dev/slog"
20+
"github.com/coder/coder/v2/coderd/files"
2021

2122
"github.com/coder/coder/v2/coderd/audit"
2223
"github.com/coder/coder/v2/coderd/database"
@@ -35,6 +36,7 @@ type Executor struct {
3536
ctx context.Context
3637
db database.Store
3738
ps pubsub.Pubsub
39+
fileCache*files.Cache
3840
templateScheduleStore*atomic.Pointer[schedule.TemplateScheduleStore]
3941
accessControlStore*atomic.Pointer[dbauthz.AccessControlStore]
4042
auditor*atomic.Pointer[audit.Auditor]
@@ -61,13 +63,14 @@ type Stats struct {
6163
}
6264

6365
// New returns a new wsactions 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 {
66+
funcNewExecutor(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 {
6567
factory:=promauto.With(reg)
6668
le:=&Executor{
6769
//nolint:gocritic // Autostart has a limited set of permissions.
6870
ctx:dbauthz.AsAutostart(ctx),
6971
db:db,
7072
ps:ps,
73+
fileCache:fc,
7174
templateScheduleStore:tss,
7275
tick:tick,
7376
log:log.Named("autobuild"),
@@ -276,7 +279,7 @@ func (e *Executor) runOnce(t time.Time) Stats {
276279
}
277280
}
278281

279-
nextBuild,job,_,err=builder.Build(e.ctx,tx,nil, audit.WorkspaceBuildBaggage{IP:"127.0.0.1"})
282+
nextBuild,job,_,err=builder.Build(e.ctx,tx,e.fileCache,nil, audit.WorkspaceBuildBaggage{IP:"127.0.0.1"})
280283
iferr!=nil {
281284
returnxerrors.Errorf("build workspace with transition %q: %w",nextTransition,err)
282285
}

‎coderd/coderdtest/coderdtest.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ import (
5252
"cdr.dev/slog"
5353
"cdr.dev/slog/sloggers/sloghuman"
5454
"cdr.dev/slog/sloggers/slogtest"
55+
"github.com/coder/coder/v2/coderd/files"
5556
"github.com/coder/quartz"
5657

5758
"github.com/coder/coder/v2/coderd"
@@ -359,6 +360,7 @@ func NewOptions(t testing.TB, options *Options) (func(http.Handler), context.Can
359360
ctx,
360361
options.Database,
361362
options.Pubsub,
363+
files.New(prometheus.NewRegistry(),options.Authorizer),
362364
prometheus.NewRegistry(),
363365
&templateScheduleStore,
364366
&auditor,

‎coderd/dynamicparameters/render.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ type loader struct {
4949
// Prepare is the entrypoint for this package. It loads the necessary objects &
5050
// files from the database and returns a Renderer that can be used to render the
5151
// template version's parameters.
52-
funcPrepare(ctx context.Context,db database.Store,cache*files.Cache,versionID uuid.UUID,options...func(r*loader)) (Renderer,error) {
52+
funcPrepare(ctx context.Context,db database.Store,cache files.FileAcquirer,versionID uuid.UUID,options...func(r*loader)) (Renderer,error) {
5353
l:=&loader{
5454
templateVersionID:versionID,
5555
}
@@ -137,7 +137,7 @@ func (r *loader) loadData(ctx context.Context, db database.Store) error {
137137
// Static parameter rendering is required to support older template versions that
138138
// do not have the database state to support dynamic parameters. A constant
139139
// warning will be displayed for these template versions.
140-
func (r*loader)Renderer(ctx context.Context,db database.Store,cache*files.Cache) (Renderer,error) {
140+
func (r*loader)Renderer(ctx context.Context,db database.Store,cache files.FileAcquirer) (Renderer,error) {
141141
err:=r.loadData(ctx,db)
142142
iferr!=nil {
143143
returnnil,xerrors.Errorf("load data: %w",err)
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
package dynamicparameters_test
2+
3+
import (
4+
"testing"
5+
6+
"github.com/stretchr/testify/require"
7+
8+
"github.com/coder/coder/v2/coderd/dynamicparameters"
9+
)
10+
11+
funcTestProvisionerVersionSupportsDynamicParameters(t*testing.T) {
12+
t.Parallel()
13+
14+
forv,dyn:=rangemap[string]bool{
15+
"":false,
16+
"na":false,
17+
"0.0":false,
18+
"0.10":false,
19+
"1.4":false,
20+
"1.5":false,
21+
"1.6":true,
22+
"1.7":true,
23+
"1.8":true,
24+
"2.0":true,
25+
"2.17":true,
26+
"4.0":true,
27+
} {
28+
t.Run(v,func(t*testing.T) {
29+
t.Parallel()
30+
31+
does:=dynamicparameters.ProvisionerVersionSupportsDynamicParameters(v)
32+
require.Equal(t,dyn,does)
33+
})
34+
}
35+
}

‎coderd/dynamicparameters/resolver.go

Lines changed: 189 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,189 @@
1+
package dynamicparameters
2+
3+
import (
4+
"context"
5+
"fmt"
6+
7+
"github.com/google/uuid"
8+
"github.com/hashicorp/hcl/v2"
9+
10+
"github.com/coder/coder/v2/coderd/database"
11+
"github.com/coder/coder/v2/coderd/util/slice"
12+
"github.com/coder/coder/v2/codersdk"
13+
)
14+
15+
typeparameterValueSourceint
16+
17+
const (
18+
sourceDefaultparameterValueSource=iota
19+
sourcePrevious
20+
sourceBuild
21+
sourcePreset
22+
)
23+
24+
typeparameterValuestruct {
25+
Valuestring
26+
SourceparameterValueSource
27+
}
28+
29+
//nolint:revive // firstbuild is a control flag to turn on immutable validation
30+
funcResolveParameters(
31+
ctx context.Context,
32+
ownerID uuid.UUID,
33+
rendererRenderer,
34+
firstBuildbool,
35+
previousValues []database.WorkspaceBuildParameter,
36+
buildValues []codersdk.WorkspaceBuildParameter,
37+
presetValues []database.TemplateVersionPresetParameter,
38+
) (map[string]string, hcl.Diagnostics) {
39+
previousValuesMap:=slice.ToMapFunc(previousValues,func(p database.WorkspaceBuildParameter) (string,string) {
40+
returnp.Name,p.Value
41+
})
42+
43+
// Start with previous
44+
values:=parameterValueMap(slice.ToMapFunc(previousValues,func(p database.WorkspaceBuildParameter) (string,parameterValue) {
45+
returnp.Name,parameterValue{Source:sourcePrevious,Value:p.Value}
46+
}))
47+
48+
// Add build values (overwrite previous values if they exist)
49+
for_,buildValue:=rangebuildValues {
50+
values[buildValue.Name]=parameterValue{Source:sourceBuild,Value:buildValue.Value}
51+
}
52+
53+
// Add preset values (overwrite previous and build values if they exist)
54+
for_,preset:=rangepresetValues {
55+
values[preset.Name]=parameterValue{Source:sourcePreset,Value:preset.Value}
56+
}
57+
58+
// originalValues is going to be used to detect if a user tried to change
59+
// an immutable parameter after the first build.
60+
originalValues:=make(map[string]parameterValue,len(values))
61+
forname,value:=rangevalues {
62+
// Store the original values for later use.
63+
originalValues[name]=value
64+
}
65+
66+
// Render the parameters using the values that were supplied to the previous build.
67+
//
68+
// This is how the form should look to the user on their workspace settings page.
69+
// This is the original form truth that our validations should initially be based on.
70+
output,diags:=renderer.Render(ctx,ownerID,values.ValuesMap())
71+
ifdiags.HasErrors() {
72+
// Top level diagnostics should break the build. Previous values (and new) should
73+
// always be valid. If there is a case where this is not true, then this has to
74+
// be changed to allow the build to continue with a different set of values.
75+
76+
returnnil,diags
77+
}
78+
79+
// The user's input now needs to be validated against the parameters.
80+
// Mutability & Ephemeral parameters depend on sequential workspace builds.
81+
//
82+
// To enforce these, the user's input values are trimmed based on the
83+
// mutability and ephemeral parameters defined in the template version.
84+
for_,parameter:=rangeoutput.Parameters {
85+
// Ephemeral parameters should not be taken from the previous build.
86+
// They must always be explicitly set in every build.
87+
// So remove their values if they are sourced from the previous build.
88+
ifparameter.Ephemeral {
89+
v:=values[parameter.Name]
90+
ifv.Source==sourcePrevious {
91+
delete(values,parameter.Name)
92+
}
93+
}
94+
95+
// Immutable parameters should also not be allowed to be changed from
96+
// the previous build. Remove any values taken from the preset or
97+
// new build params. This forces the value to be the same as it was before.
98+
//
99+
// We do this so the next form render uses the original immutable value.
100+
if!firstBuild&&!parameter.Mutable {
101+
delete(values,parameter.Name)
102+
prev,ok:=previousValuesMap[parameter.Name]
103+
ifok {
104+
values[parameter.Name]=parameterValue{
105+
Value:prev,
106+
Source:sourcePrevious,
107+
}
108+
}
109+
}
110+
}
111+
112+
// This is the final set of values that will be used. Any errors at this stage
113+
// are fatal. Additional validation for immutability has to be done manually.
114+
output,diags=renderer.Render(ctx,ownerID,values.ValuesMap())
115+
ifdiags.HasErrors() {
116+
returnnil,diags
117+
}
118+
119+
// parameterNames is going to be used to remove any excess values that were left
120+
// around without a parameter.
121+
parameterNames:=make(map[string]struct{},len(output.Parameters))
122+
for_,parameter:=rangeoutput.Parameters {
123+
parameterNames[parameter.Name]=struct{}{}
124+
125+
if!firstBuild&&!parameter.Mutable {
126+
// Immutable parameters should not be changed after the first build.
127+
// They can match the original value though!
128+
ifparameter.Value.AsString()!=originalValues[parameter.Name].Value {
129+
varsrc*hcl.Range
130+
ifparameter.Source!=nil {
131+
src=&parameter.Source.HCLBlock().TypeRange
132+
}
133+
134+
// An immutable parameter was changed, which is not allowed.
135+
// Add the failed diagnostic to the output.
136+
diags=diags.Append(&hcl.Diagnostic{
137+
Severity:hcl.DiagError,
138+
Summary:"Immutable parameter changed",
139+
Detail:fmt.Sprintf("Parameter %q is not mutable, so it can't be updated after creating a workspace.",parameter.Name),
140+
Subject:src,
141+
})
142+
}
143+
}
144+
145+
// TODO: Fix the `hcl.Diagnostics(...)` type casting. It should not be needed.
146+
ifhcl.Diagnostics(parameter.Diagnostics).HasErrors() {
147+
// All validation errors are raised here.
148+
diags=diags.Extend(hcl.Diagnostics(parameter.Diagnostics))
149+
}
150+
151+
// If the parameter has a value, but it was not set explicitly by the user at any
152+
// build, then save the default value. An example where this is important is if a
153+
// template has a default value of 'region = us-west-2', but the user never sets
154+
// it. If the default value changes to 'region = us-east-1', we want to preserve
155+
// the original value of 'us-west-2' for the existing workspaces.
156+
//
157+
// parameter.Value will be populated from the default at this point. So grab it
158+
// from there.
159+
if_,ok:=values[parameter.Name];!ok&&parameter.Value.IsKnown()&&parameter.Value.Valid() {
160+
values[parameter.Name]=parameterValue{
161+
Value:parameter.Value.AsString(),
162+
Source:sourceDefault,
163+
}
164+
}
165+
}
166+
167+
// Delete any values that do not belong to a parameter. This is to not save
168+
// parameter values that have no effect. These leaky parameter values can cause
169+
// problems in the future, as it makes it challenging to remove values from the
170+
// database
171+
fork:=rangevalues {
172+
if_,ok:=parameterNames[k];!ok {
173+
delete(values,k)
174+
}
175+
}
176+
177+
// Return the values to be saved for the build.
178+
returnvalues.ValuesMap(),diags
179+
}
180+
181+
typeparameterValueMapmap[string]parameterValue
182+
183+
func (pparameterValueMap)ValuesMap()map[string]string {
184+
values:=make(map[string]string,len(p))
185+
forname,paramValue:=rangep {
186+
values[name]=paramValue.Value
187+
}
188+
returnvalues
189+
}

‎coderd/parameters_test.go

Lines changed: 24 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -100,10 +100,11 @@ func TestDynamicParametersWithTerraformValues(t *testing.T) {
100100
require.Equal(t,-1,preview.ID)
101101
require.Empty(t,preview.Diagnostics)
102102

103-
require.Len(t,preview.Parameters,1)
104-
require.Equal(t,"jetbrains_ide",preview.Parameters[0].Name)
105-
require.True(t,preview.Parameters[0].Value.Valid)
106-
require.Equal(t,"CL",preview.Parameters[0].Value.Value)
103+
require.Len(t,preview.Parameters,2)
104+
coderdtest.AssertParameter(t,"jetbrains_ide",preview.Parameters).
105+
Exists().Value("CL")
106+
coderdtest.AssertParameter(t,"region",preview.Parameters).
107+
Exists().Value("na")
107108
})
108109

109110
// OldProvisioners use the static parameters in the dynamic param flow
@@ -241,10 +242,11 @@ func TestDynamicParametersWithTerraformValues(t *testing.T) {
241242
require.Equal(t,-1,preview.ID)
242243
require.Empty(t,preview.Diagnostics)
243244

244-
require.Len(t,preview.Parameters,1)
245-
require.Equal(t,"jetbrains_ide",preview.Parameters[0].Name)
246-
require.True(t,preview.Parameters[0].Value.Valid)
247-
require.Equal(t,"CL",preview.Parameters[0].Value.Value)
245+
require.Len(t,preview.Parameters,2)
246+
coderdtest.AssertParameter(t,"jetbrains_ide",preview.Parameters).
247+
Exists().Value("CL")
248+
coderdtest.AssertParameter(t,"region",preview.Parameters).
249+
Exists().Value("na")
248250
_=stream.Close(websocket.StatusGoingAway)
249251

250252
wrk:=coderdtest.CreateWorkspace(t,setup.client,setup.template.ID,func(request*codersdk.CreateWorkspaceRequest) {
@@ -253,29 +255,35 @@ func TestDynamicParametersWithTerraformValues(t *testing.T) {
253255
Name:preview.Parameters[0].Name,
254256
Value:"GO",
255257
},
258+
{
259+
Name:preview.Parameters[1].Name,
260+
Value:"eu",
261+
},
256262
}
257263
request.EnableDynamicParameters=true
258264
})
259265
coderdtest.AwaitWorkspaceBuildJobCompleted(t,setup.client,wrk.LatestBuild.ID)
260266

261267
params,err:=setup.client.WorkspaceBuildParameters(ctx,wrk.LatestBuild.ID)
262268
require.NoError(t,err)
263-
require.Len(t,params,1)
264-
require.Equal(t,"jetbrains_ide",params[0].Name)
265-
require.Equal(t,"GO",params[0].Value)
269+
require.ElementsMatch(t, []codersdk.WorkspaceBuildParameter{
270+
{Name:"jetbrains_ide",Value:"GO"}, {Name:"region",Value:"eu"},
271+
},params)
272+
273+
regionOptions:= []string{"na","af","sa","as"}
266274

267275
// A helper function to assert params
268276
doTransition:=func(t*testing.T,trans codersdk.WorkspaceTransition) {
269277
t.Helper()
270278

271-
fooVal:=coderdtest.RandomUsername(t)
279+
regionVal:=regionOptions[0]
280+
regionOptions=regionOptions[1:]// Choose the next region on the next build
281+
272282
bld,err:=setup.client.CreateWorkspaceBuild(ctx,wrk.ID, codersdk.CreateWorkspaceBuildRequest{
273283
TemplateVersionID:setup.template.ActiveVersionID,
274284
Transition:trans,
275285
RichParameterValues: []codersdk.WorkspaceBuildParameter{
276-
// No validation, so this should work as is.
277-
// Overwrite the value on each transition
278-
{Name:"foo",Value:fooVal},
286+
{Name:"region",Value:regionVal},
279287
},
280288
EnableDynamicParameters:ptr.Ref(true),
281289
})
@@ -286,7 +294,7 @@ func TestDynamicParametersWithTerraformValues(t *testing.T) {
286294
require.NoError(t,err)
287295
require.ElementsMatch(t,latestParams, []codersdk.WorkspaceBuildParameter{
288296
{Name:"jetbrains_ide",Value:"GO"},
289-
{Name:"foo",Value:fooVal},
297+
{Name:"region",Value:regionVal},
290298
})
291299
}
292300

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp