Skip to content

Commit b4531c4

Browse files
Emyrkjaaydenh
Emyrk
and
jaaydenh
authored
feat: make dynamic parameters respect owner in form (#18013)
Closes #18012 --------- Co-authored-by: Jaayden Halko <[email protected]>
1 parent 5b9c404 commit b4531c4

File tree

13 files changed

+264
-189
lines changed

13 files changed

+264
-189
lines changed

‎coderd/apidoc/docs.go

Lines changed: 37 additions & 37 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/apidoc/swagger.json

Lines changed: 35 additions & 35 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/coderd.go

Lines changed: 8 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1122,6 +1122,7 @@ func New(options *Options) *API {
11221122
})
11231123
})
11241124
})
1125+
11251126
r.Route("/templateversions/{templateversion}", func(r chi.Router) {
11261127
r.Use(
11271128
apiKeyMiddleware,
@@ -1150,6 +1151,13 @@ func New(options *Options) *API {
11501151
r.Get("/{jobID}/matched-provisioners", api.templateVersionDryRunMatchedProvisioners)
11511152
r.Patch("/{jobID}/cancel", api.patchTemplateVersionDryRunCancel)
11521153
})
1154+
1155+
r.Group(func(r chi.Router) {
1156+
r.Use(
1157+
httpmw.RequireExperiment(api.Experiments, codersdk.ExperimentDynamicParameters),
1158+
)
1159+
r.Get("/dynamic-parameters", api.templateVersionDynamicParameters)
1160+
})
11531161
})
11541162
r.Route("/users", func(r chi.Router) {
11551163
r.Get("/first", api.firstUser)
@@ -1210,19 +1218,6 @@ func New(options *Options) *API {
12101218
r.Group(func(r chi.Router) {
12111219
r.Use(httpmw.ExtractUserParam(options.Database))
12121220

1213-
// Similarly to creating a workspace, evaluating parameters for a
1214-
// new workspace should also match the authz story of
1215-
// postWorkspacesByOrganization
1216-
// TODO: Do not require site wide read user permission. Make this work
1217-
// with org member permissions.
1218-
r.Route("/templateversions/{templateversion}", func(r chi.Router) {
1219-
r.Use(
1220-
httpmw.ExtractTemplateVersionParam(options.Database),
1221-
httpmw.RequireExperiment(api.Experiments, codersdk.ExperimentDynamicParameters),
1222-
)
1223-
r.Get("/parameters", api.templateVersionDynamicParameters)
1224-
})
1225-
12261221
r.Post("/convert-login", api.postConvertLoginType)
12271222
r.Delete("/", api.deleteUser)
12281223
r.Get("/", api.userByName)

‎coderd/parameters.go

Lines changed: 59 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ import (
3636
// @Param user path string true "Template version ID" format(uuid)
3737
// @Param templateversion path string true "Template version ID" format(uuid)
3838
// @Success 101
39-
// @Router /users/{user}/templateversions/{templateversion}/parameters [get]
39+
// @Router /templateversions/{templateversion}/dynamic-parameters [get]
4040
func (api *API) templateVersionDynamicParameters(rw http.ResponseWriter, r *http.Request) {
4141
ctx := r.Context()
4242
templateVersion := httpmw.TemplateVersionParam(r)
@@ -77,12 +77,12 @@ func (api *API) templateVersionDynamicParameters(rw http.ResponseWriter, r *http
7777
}
7878
}
7979

80-
type previewFunction func(ctx context.Context, values map[string]string) (*preview.Output, hcl.Diagnostics)
80+
type previewFunction func(ctx context.Context, ownerID uuid.UUID, values map[string]string) (*preview.Output, hcl.Diagnostics)
8181

8282
func (api *API) handleDynamicParameters(rw http.ResponseWriter, r *http.Request, tf database.TemplateVersionTerraformValue, templateVersion database.TemplateVersion) {
8383
var (
84-
ctx = r.Context()
85-
user = httpmw.UserParam(r)
84+
ctx = r.Context()
85+
apikey = httpmw.APIKey(r)
8686
)
8787

8888
// nolint:gocritic // We need to fetch the templates files for the Terraform
@@ -130,7 +130,7 @@ func (api *API) handleDynamicParameters(rw http.ResponseWriter, r *http.Request,
130130
templateFS = files.NewOverlayFS(templateFS, []files.Overlay{{Path: ".terraform/modules", FS: moduleFilesFS}})
131131
}
132132

133-
owner, err := getWorkspaceOwnerData(ctx, api.Database, user, templateVersion.OrganizationID)
133+
owner, err := getWorkspaceOwnerData(ctx, api.Database, apikey.UserID, templateVersion.OrganizationID)
134134
if err != nil {
135135
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
136136
Message: "Internal error fetching workspace owner.",
@@ -145,10 +145,46 @@ func (api *API) handleDynamicParameters(rw http.ResponseWriter, r *http.Request,
145145
Owner: owner,
146146
}
147147

148-
api.handleParameterWebsocket(rw, r, func(ctx context.Context, values map[string]string) (*preview.Output, hcl.Diagnostics) {
148+
// failedOwners keeps track of which owners failed to fetch from the database.
149+
// This prevents db spam on repeated requests for the same failed owner.
150+
failedOwners := make(map[uuid.UUID]error)
151+
failedOwnerDiag := hcl.Diagnostics{
152+
{
153+
Severity: hcl.DiagError,
154+
Summary: "Failed to fetch workspace owner",
155+
Detail: "Please check your permissions or the user may not exist.",
156+
Extra: previewtypes.DiagnosticExtra{
157+
Code: "owner_not_found",
158+
},
159+
},
160+
}
161+
162+
api.handleParameterWebsocket(rw, r, apikey.UserID, func(ctx context.Context, ownerID uuid.UUID, values map[string]string) (*preview.Output, hcl.Diagnostics) {
163+
if ownerID == uuid.Nil {
164+
// Default to the authenticated user
165+
// Nice for testing
166+
ownerID = apikey.UserID
167+
}
168+
169+
if _, ok := failedOwners[ownerID]; ok {
170+
// If it has failed once, assume it will fail always.
171+
// Re-open the websocket to try again.
172+
return nil, failedOwnerDiag
173+
}
174+
149175
// Update the input values with the new values.
150-
// The rest of the input is unchanged.
151176
input.ParameterValues = values
177+
178+
// Update the owner if there is a change
179+
if input.Owner.ID != ownerID.String() {
180+
owner, err = getWorkspaceOwnerData(ctx, api.Database, ownerID, templateVersion.OrganizationID)
181+
if err != nil {
182+
failedOwners[ownerID] = err
183+
return nil, failedOwnerDiag
184+
}
185+
input.Owner = owner
186+
}
187+
152188
return preview.Preview(ctx, input, templateFS)
153189
})
154190
}
@@ -239,7 +275,7 @@ func (api *API) handleStaticParameters(rw http.ResponseWriter, r *http.Request,
239275
params = append(params, param)
240276
}
241277

242-
api.handleParameterWebsocket(rw, r, func(_ context.Context, values map[string]string) (*preview.Output, hcl.Diagnostics) {
278+
api.handleParameterWebsocket(rw, r, uuid.Nil, func(_ context.Context, _ uuid.UUID, values map[string]string) (*preview.Output, hcl.Diagnostics) {
243279
for i := range params {
244280
param := &params[i]
245281
paramValue, ok := values[param.Name]
@@ -264,7 +300,7 @@ func (api *API) handleStaticParameters(rw http.ResponseWriter, r *http.Request,
264300
})
265301
}
266302

267-
func (api *API) handleParameterWebsocket(rw http.ResponseWriter, r *http.Request, render previewFunction) {
303+
func (api *API) handleParameterWebsocket(rw http.ResponseWriter, r *http.Request, ownerID uuid.UUID, render previewFunction) {
268304
ctx, cancel := context.WithTimeout(r.Context(), 30*time.Minute)
269305
defer cancel()
270306

@@ -284,7 +320,7 @@ func (api *API) handleParameterWebsocket(rw http.ResponseWriter, r *http.Request
284320
)
285321

286322
// Send an initial form state, computed without any user input.
287-
result, diagnostics := render(ctx, map[string]string{})
323+
result, diagnostics := render(ctx, ownerID, map[string]string{})
288324
response := codersdk.DynamicParametersResponse{
289325
ID: -1, // Always start with -1.
290326
Diagnostics: db2sdk.HCLDiagnostics(diagnostics),
@@ -312,7 +348,7 @@ func (api *API) handleParameterWebsocket(rw http.ResponseWriter, r *http.Request
312348
return
313349
}
314350

315-
result, diagnostics := render(ctx, update.Inputs)
351+
result, diagnostics := render(ctx, update.OwnerID, update.Inputs)
316352
response := codersdk.DynamicParametersResponse{
317353
ID: update.ID,
318354
Diagnostics: db2sdk.HCLDiagnostics(diagnostics),
@@ -332,17 +368,24 @@ func (api *API) handleParameterWebsocket(rw http.ResponseWriter, r *http.Request
332368
func getWorkspaceOwnerData(
333369
ctx context.Context,
334370
db database.Store,
335-
user database.User,
371+
ownerID uuid.UUID,
336372
organizationID uuid.UUID,
337373
) (previewtypes.WorkspaceOwner, error) {
338374
var g errgroup.Group
339375

376+
// TODO: @emyrk we should only need read access on the org member, not the
377+
// site wide user object. Figure out a better way to handle this.
378+
user, err := db.GetUserByID(ctx, ownerID)
379+
if err != nil {
380+
return previewtypes.WorkspaceOwner{}, xerrors.Errorf("fetch user: %w", err)
381+
}
382+
340383
var ownerRoles []previewtypes.WorkspaceOwnerRBACRole
341384
g.Go(func() error {
342385
// nolint:gocritic // This is kind of the wrong query to use here, but it
343386
// matches how the provisioner currently works. We should figure out
344387
// something that needs less escalation but has the correct behavior.
345-
row, err := db.GetAuthorizationUserRoles(dbauthz.AsSystemRestricted(ctx), user.ID)
388+
row, err := db.GetAuthorizationUserRoles(dbauthz.AsSystemRestricted(ctx), ownerID)
346389
if err != nil {
347390
return err
348391
}
@@ -372,7 +415,7 @@ func getWorkspaceOwnerData(
372415
// The correct public key has to be sent. This will not be leaked
373416
// unless the template leaks it.
374417
// nolint:gocritic
375-
key, err := db.GetGitSSHKey(dbauthz.AsSystemRestricted(ctx), user.ID)
418+
key, err := db.GetGitSSHKey(dbauthz.AsSystemRestricted(ctx), ownerID)
376419
if err != nil {
377420
return err
378421
}
@@ -388,7 +431,7 @@ func getWorkspaceOwnerData(
388431
// nolint:gocritic
389432
groups, err := db.GetGroups(dbauthz.AsSystemRestricted(ctx), database.GetGroupsParams{
390433
OrganizationID: organizationID,
391-
HasMemberID: user.ID,
434+
HasMemberID: ownerID,
392435
})
393436
if err != nil {
394437
return err
@@ -400,7 +443,7 @@ func getWorkspaceOwnerData(
400443
return nil
401444
})
402445

403-
err := g.Wait()
446+
err = g.Wait()
404447
if err != nil {
405448
return previewtypes.WorkspaceOwner{}, err
406449
}

0 commit comments

Comments
 (0)