From 857f1585787124b2179b74f942d50c1f7a4b6178 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Fri, 23 May 2025 10:42:22 -0500 Subject: [PATCH 01/10] feat: handle workspace owner in dynamic param form --- coderd/coderd.go | 21 ++++------ coderd/parameters.go | 75 ++++++++++++++++++++++++++-------- codersdk/parameters.go | 5 ++- site/src/api/typesGenerated.ts | 1 + 4 files changed, 71 insertions(+), 31 deletions(-) diff --git a/coderd/coderd.go b/coderd/coderd.go index 3989f8a87ea1b..20a8a49f7bb61 100644 --- a/coderd/coderd.go +++ b/coderd/coderd.go @@ -1122,6 +1122,7 @@ func New(options *Options) *API { }) }) }) + r.Route("/templateversions/{templateversion}", func(r chi.Router) { r.Use( apiKeyMiddleware, @@ -1150,6 +1151,13 @@ func New(options *Options) *API { r.Get("/{jobID}/matched-provisioners", api.templateVersionDryRunMatchedProvisioners) r.Patch("/{jobID}/cancel", api.patchTemplateVersionDryRunCancel) }) + + r.Group(func(r chi.Router) { + r.Use( + httpmw.RequireExperiment(api.Experiments, codersdk.ExperimentDynamicParameters), + ) + r.Get("/parameters", api.templateVersionDynamicParameters) + }) }) r.Route("/users", func(r chi.Router) { r.Get("/first", api.firstUser) @@ -1210,19 +1218,6 @@ func New(options *Options) *API { r.Group(func(r chi.Router) { r.Use(httpmw.ExtractUserParam(options.Database)) - // Similarly to creating a workspace, evaluating parameters for a - // new workspace should also match the authz story of - // postWorkspacesByOrganization - // TODO: Do not require site wide read user permission. Make this work - // with org member permissions. - r.Route("/templateversions/{templateversion}", func(r chi.Router) { - r.Use( - httpmw.ExtractTemplateVersionParam(options.Database), - httpmw.RequireExperiment(api.Experiments, codersdk.ExperimentDynamicParameters), - ) - r.Get("/parameters", api.templateVersionDynamicParameters) - }) - r.Post("/convert-login", api.postConvertLoginType) r.Delete("/", api.deleteUser) r.Get("/", api.userByName) diff --git a/coderd/parameters.go b/coderd/parameters.go index 1a0c1f92ddbf9..02bb4c05aa44e 100644 --- a/coderd/parameters.go +++ b/coderd/parameters.go @@ -36,7 +36,7 @@ import ( // @Param user path string true "Template version ID" format(uuid) // @Param templateversion path string true "Template version ID" format(uuid) // @Success 101 -// @Router /users/{user}/templateversions/{templateversion}/parameters [get] +// @Router /templateversions/{templateversion}/parameters [get] func (api *API) templateVersionDynamicParameters(rw http.ResponseWriter, r *http.Request) { ctx := r.Context() templateVersion := httpmw.TemplateVersionParam(r) @@ -77,12 +77,12 @@ func (api *API) templateVersionDynamicParameters(rw http.ResponseWriter, r *http } } -type previewFunction func(ctx context.Context, values map[string]string) (*preview.Output, hcl.Diagnostics) +type previewFunction func(ctx context.Context, ownerID uuid.UUID, values map[string]string) (*preview.Output, hcl.Diagnostics) func (api *API) handleDynamicParameters(rw http.ResponseWriter, r *http.Request, tf database.TemplateVersionTerraformValue, templateVersion database.TemplateVersion) { var ( - ctx = r.Context() - user = httpmw.UserParam(r) + ctx = r.Context() + apikey = httpmw.APIKey(r) ) // 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, templateFS = files.NewOverlayFS(templateFS, []files.Overlay{{Path: ".terraform/modules", FS: moduleFilesFS}}) } - owner, err := getWorkspaceOwnerData(ctx, api.Database, user, templateVersion.OrganizationID) + owner, err := getWorkspaceOwnerData(ctx, api.Database, apikey.UserID, templateVersion.OrganizationID) if err != nil { httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ Message: "Internal error fetching workspace owner.", @@ -145,10 +145,46 @@ func (api *API) handleDynamicParameters(rw http.ResponseWriter, r *http.Request, Owner: owner, } - api.handleParameterWebsocket(rw, r, func(ctx context.Context, values map[string]string) (*preview.Output, hcl.Diagnostics) { + // failedOwners keeps track of which owners failed to fetch from the database. + // This prevents db spam on repeated requests for the same failed owner. + failedOwners := make(map[uuid.UUID]error) + failedOwnerDiag := hcl.Diagnostics{ + { + Severity: hcl.DiagError, + Summary: "Failed to fetch workspace owner", + Detail: "Please check your permissions or the user may not exist.", + Extra: previewtypes.DiagnosticExtra{ + Code: "owner_not_found", + }, + }, + } + + api.handleParameterWebsocket(rw, r, apikey.UserID, func(ctx context.Context, ownerID uuid.UUID, values map[string]string) (*preview.Output, hcl.Diagnostics) { + if ownerID == uuid.Nil { + // Default to the authenticated user + // Nice for testing + ownerID = apikey.UserID + } + + if _, ok := failedOwners[ownerID]; ok { + // If it has failed once, assume it will fail always. + // Re-open the websocket to try again. + return nil, failedOwnerDiag + } + // Update the input values with the new values. - // The rest of the input is unchanged. input.ParameterValues = values + + // Update the owner if there is a change + if input.Owner.ID != ownerID.String() { + owner, err = getWorkspaceOwnerData(ctx, api.Database, ownerID, templateVersion.OrganizationID) + if err != nil { + failedOwners[ownerID] = err + return nil, failedOwnerDiag + } + input.Owner = owner + } + return preview.Preview(ctx, input, templateFS) }) } @@ -239,7 +275,7 @@ func (api *API) handleStaticParameters(rw http.ResponseWriter, r *http.Request, params = append(params, param) } - api.handleParameterWebsocket(rw, r, func(_ context.Context, values map[string]string) (*preview.Output, hcl.Diagnostics) { + api.handleParameterWebsocket(rw, r, uuid.Nil, func(_ context.Context, _ uuid.UUID, values map[string]string) (*preview.Output, hcl.Diagnostics) { for i := range params { param := ¶ms[i] paramValue, ok := values[param.Name] @@ -264,7 +300,7 @@ func (api *API) handleStaticParameters(rw http.ResponseWriter, r *http.Request, }) } -func (api *API) handleParameterWebsocket(rw http.ResponseWriter, r *http.Request, render previewFunction) { +func (api *API) handleParameterWebsocket(rw http.ResponseWriter, r *http.Request, ownerID uuid.UUID, render previewFunction) { ctx, cancel := context.WithTimeout(r.Context(), 30*time.Minute) defer cancel() @@ -284,7 +320,7 @@ func (api *API) handleParameterWebsocket(rw http.ResponseWriter, r *http.Request ) // Send an initial form state, computed without any user input. - result, diagnostics := render(ctx, map[string]string{}) + result, diagnostics := render(ctx, ownerID, map[string]string{}) response := codersdk.DynamicParametersResponse{ ID: -1, // Always start with -1. Diagnostics: db2sdk.HCLDiagnostics(diagnostics), @@ -312,7 +348,7 @@ func (api *API) handleParameterWebsocket(rw http.ResponseWriter, r *http.Request return } - result, diagnostics := render(ctx, update.Inputs) + result, diagnostics := render(ctx, update.OwnerID, update.Inputs) response := codersdk.DynamicParametersResponse{ ID: update.ID, Diagnostics: db2sdk.HCLDiagnostics(diagnostics), @@ -332,17 +368,24 @@ func (api *API) handleParameterWebsocket(rw http.ResponseWriter, r *http.Request func getWorkspaceOwnerData( ctx context.Context, db database.Store, - user database.User, + ownerID uuid.UUID, organizationID uuid.UUID, ) (previewtypes.WorkspaceOwner, error) { var g errgroup.Group + // TODO: @emyrk we should only need read access on the org member, not the + // site wide user object. Figure out a better way to handle this. + user, err := db.GetUserByID(ctx, ownerID) + if err != nil { + return previewtypes.WorkspaceOwner{}, xerrors.Errorf("fetch user: %w", err) + } + var ownerRoles []previewtypes.WorkspaceOwnerRBACRole g.Go(func() error { // nolint:gocritic // This is kind of the wrong query to use here, but it // matches how the provisioner currently works. We should figure out // something that needs less escalation but has the correct behavior. - row, err := db.GetAuthorizationUserRoles(dbauthz.AsSystemRestricted(ctx), user.ID) + row, err := db.GetAuthorizationUserRoles(dbauthz.AsSystemRestricted(ctx), ownerID) if err != nil { return err } @@ -372,7 +415,7 @@ func getWorkspaceOwnerData( // The correct public key has to be sent. This will not be leaked // unless the template leaks it. // nolint:gocritic - key, err := db.GetGitSSHKey(dbauthz.AsSystemRestricted(ctx), user.ID) + key, err := db.GetGitSSHKey(dbauthz.AsSystemRestricted(ctx), ownerID) if err != nil { return err } @@ -388,7 +431,7 @@ func getWorkspaceOwnerData( // nolint:gocritic groups, err := db.GetGroups(dbauthz.AsSystemRestricted(ctx), database.GetGroupsParams{ OrganizationID: organizationID, - HasMemberID: user.ID, + HasMemberID: ownerID, }) if err != nil { return err @@ -400,7 +443,7 @@ func getWorkspaceOwnerData( return nil }) - err := g.Wait() + err = g.Wait() if err != nil { return previewtypes.WorkspaceOwner{}, err } diff --git a/codersdk/parameters.go b/codersdk/parameters.go index d81dc7cf55ca0..4ef725f062496 100644 --- a/codersdk/parameters.go +++ b/codersdk/parameters.go @@ -112,8 +112,9 @@ type PreviewParameterValidation struct { type DynamicParametersRequest struct { // ID identifies the request. The response contains the same // ID so that the client can match it to the request. - ID int `json:"id"` - Inputs map[string]string `json:"inputs"` + ID int `json:"id"` + Inputs map[string]string `json:"inputs"` + OwnerID uuid.UUID `json:"owner_id"` } type DynamicParametersResponse struct { diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index 74631c2be32fd..27bbbcd9061ab 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -774,6 +774,7 @@ export const DisplayApps: DisplayApp[] = [ export interface DynamicParametersRequest { readonly id: number; readonly inputs: Record; + readonly owner_id: string; } // From codersdk/parameters.go From b460e7369b7ca6dd602d29f418d8c2c52725a608 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Fri, 23 May 2025 10:44:03 -0500 Subject: [PATCH 02/10] fixup codersdk --- codersdk/parameters.go | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/codersdk/parameters.go b/codersdk/parameters.go index 4ef725f062496..c15132b39b32b 100644 --- a/codersdk/parameters.go +++ b/codersdk/parameters.go @@ -112,9 +112,10 @@ type PreviewParameterValidation struct { type DynamicParametersRequest struct { // ID identifies the request. The response contains the same // ID so that the client can match it to the request. - ID int `json:"id"` - Inputs map[string]string `json:"inputs"` - OwnerID uuid.UUID `json:"owner_id"` + ID int `json:"id"` + Inputs map[string]string `json:"inputs"` + // OwnerID if uuid.Nil, it defaults to `codersdk.Me` + OwnerID uuid.UUID `json:"owner_id"` } type DynamicParametersResponse struct { @@ -125,7 +126,7 @@ type DynamicParametersResponse struct { } func (c *Client) TemplateVersionDynamicParameters(ctx context.Context, userID, version uuid.UUID) (*wsjson.Stream[DynamicParametersResponse, DynamicParametersRequest], error) { - conn, err := c.Dial(ctx, fmt.Sprintf("/api/v2/users/%s/templateversions/%s/parameters", userID, version), nil) + conn, err := c.Dial(ctx, fmt.Sprintf("/api/v2/templateversions/%s/parameters", userID, version), nil) if err != nil { return nil, err } From 42dc0306276379048bbef56c7da7186cbf8e811b Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Fri, 23 May 2025 16:37:16 +0000 Subject: [PATCH 03/10] make gen --- coderd/apidoc/docs.go | 53 ++++++++------------------------- coderd/apidoc/swagger.json | 51 ++++++++----------------------- docs/reference/api/templates.md | 36 ++++------------------ 3 files changed, 29 insertions(+), 111 deletions(-) diff --git a/coderd/apidoc/docs.go b/coderd/apidoc/docs.go index 7cee63e183e7e..97a1e5881cb03 100644 --- a/coderd/apidoc/docs.go +++ b/coderd/apidoc/docs.go @@ -5984,9 +5984,17 @@ const docTemplate = `{ "tags": [ "Templates" ], - "summary": "Removed: Get parameters by template version", - "operationId": "removed-get-parameters-by-template-version", + "summary": "Open dynamic parameters WebSocket by template version", + "operationId": "open-dynamic-parameters-websocket-by-template-version", "parameters": [ + { + "type": "string", + "format": "uuid", + "description": "Template version ID", + "name": "user", + "in": "path", + "required": true + }, { "type": "string", "format": "uuid", @@ -5997,8 +6005,8 @@ const docTemplate = `{ } ], "responses": { - "200": { - "description": "OK" + "101": { + "description": "Switching Protocols" } } } @@ -7735,43 +7743,6 @@ const docTemplate = `{ } } }, - "/users/{user}/templateversions/{templateversion}/parameters": { - "get": { - "security": [ - { - "CoderSessionToken": [] - } - ], - "tags": [ - "Templates" - ], - "summary": "Open dynamic parameters WebSocket by template version", - "operationId": "open-dynamic-parameters-websocket-by-template-version", - "parameters": [ - { - "type": "string", - "format": "uuid", - "description": "Template version ID", - "name": "user", - "in": "path", - "required": true - }, - { - "type": "string", - "format": "uuid", - "description": "Template version ID", - "name": "templateversion", - "in": "path", - "required": true - } - ], - "responses": { - "101": { - "description": "Switching Protocols" - } - } - } - }, "/users/{user}/webpush/subscription": { "post": { "security": [ diff --git a/coderd/apidoc/swagger.json b/coderd/apidoc/swagger.json index 89a582091496f..dfe2bbb5b9d55 100644 --- a/coderd/apidoc/swagger.json +++ b/coderd/apidoc/swagger.json @@ -5291,9 +5291,17 @@ } ], "tags": ["Templates"], - "summary": "Removed: Get parameters by template version", - "operationId": "removed-get-parameters-by-template-version", + "summary": "Open dynamic parameters WebSocket by template version", + "operationId": "open-dynamic-parameters-websocket-by-template-version", "parameters": [ + { + "type": "string", + "format": "uuid", + "description": "Template version ID", + "name": "user", + "in": "path", + "required": true + }, { "type": "string", "format": "uuid", @@ -5304,8 +5312,8 @@ } ], "responses": { - "200": { - "description": "OK" + "101": { + "description": "Switching Protocols" } } } @@ -6834,41 +6842,6 @@ } } }, - "/users/{user}/templateversions/{templateversion}/parameters": { - "get": { - "security": [ - { - "CoderSessionToken": [] - } - ], - "tags": ["Templates"], - "summary": "Open dynamic parameters WebSocket by template version", - "operationId": "open-dynamic-parameters-websocket-by-template-version", - "parameters": [ - { - "type": "string", - "format": "uuid", - "description": "Template version ID", - "name": "user", - "in": "path", - "required": true - }, - { - "type": "string", - "format": "uuid", - "description": "Template version ID", - "name": "templateversion", - "in": "path", - "required": true - } - ], - "responses": { - "101": { - "description": "Switching Protocols" - } - } - } - }, "/users/{user}/webpush/subscription": { "post": { "security": [ diff --git a/docs/reference/api/templates.md b/docs/reference/api/templates.md index 09fc555c7d39c..c50f4bbfa4f44 100644 --- a/docs/reference/api/templates.md +++ b/docs/reference/api/templates.md @@ -2708,7 +2708,7 @@ Status Code **200** To perform this operation, you must be authenticated. [Learn more](authentication.md). -## Removed: Get parameters by template version +## Open dynamic parameters WebSocket by template version ### Code samples @@ -2724,13 +2724,14 @@ curl -X GET http://coder-server:8080/api/v2/templateversions/{templateversion}/p | Name | In | Type | Required | Description | |-------------------|------|--------------|----------|---------------------| +| `user` | path | string(uuid) | true | Template version ID | | `templateversion` | path | string(uuid) | true | Template version ID | ### Responses -| Status | Meaning | Description | Schema | -|--------|---------------------------------------------------------|-------------|--------| -| 200 | [OK](https://tools.ietf.org/html/rfc7231#section-6.3.1) | OK | | +| Status | Meaning | Description | Schema | +|--------|--------------------------------------------------------------------------|---------------------|--------| +| 101 | [Switching Protocols](https://tools.ietf.org/html/rfc7231#section-6.2.2) | Switching Protocols | | To perform this operation, you must be authenticated. [Learn more](authentication.md). @@ -3340,30 +3341,3 @@ Status Code **200** | `type` | `bool` | To perform this operation, you must be authenticated. [Learn more](authentication.md). - -## Open dynamic parameters WebSocket by template version - -### Code samples - -```shell -# Example request using curl -curl -X GET http://coder-server:8080/api/v2/users/{user}/templateversions/{templateversion}/parameters \ - -H 'Coder-Session-Token: API_KEY' -``` - -`GET /users/{user}/templateversions/{templateversion}/parameters` - -### Parameters - -| Name | In | Type | Required | Description | -|-------------------|------|--------------|----------|---------------------| -| `user` | path | string(uuid) | true | Template version ID | -| `templateversion` | path | string(uuid) | true | Template version ID | - -### Responses - -| Status | Meaning | Description | Schema | -|--------|--------------------------------------------------------------------------|---------------------|--------| -| 101 | [Switching Protocols](https://tools.ietf.org/html/rfc7231#section-6.2.2) | Switching Protocols | | - -To perform this operation, you must be authenticated. [Learn more](authentication.md). From d3f45caca5866d146d7a610b01608247f69b3841 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Fri, 23 May 2025 13:28:01 -0500 Subject: [PATCH 04/10] move route to /dynamic-parameters --- coderd/parameters.go | 2 +- coderd/parameters_test.go | 6 +++--- codersdk/parameters.go | 4 ++-- enterprise/coderd/parameters_test.go | 2 +- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/coderd/parameters.go b/coderd/parameters.go index 02bb4c05aa44e..d1e989c8ad032 100644 --- a/coderd/parameters.go +++ b/coderd/parameters.go @@ -36,7 +36,7 @@ import ( // @Param user path string true "Template version ID" format(uuid) // @Param templateversion path string true "Template version ID" format(uuid) // @Success 101 -// @Router /templateversions/{templateversion}/parameters [get] +// @Router /templateversions/{templateversion}/dynamic-parameters [get] func (api *API) templateVersionDynamicParameters(rw http.ResponseWriter, r *http.Request) { ctx := r.Context() templateVersion := httpmw.TemplateVersionParam(r) diff --git a/coderd/parameters_test.go b/coderd/parameters_test.go index 8edadc9b7e797..821b769ede530 100644 --- a/coderd/parameters_test.go +++ b/coderd/parameters_test.go @@ -57,7 +57,7 @@ func TestDynamicParametersOwnerSSHPublicKey(t *testing.T) { _ = coderdtest.CreateTemplate(t, templateAdmin, owner.OrganizationID, version.ID) ctx := testutil.Context(t, testutil.WaitShort) - stream, err := templateAdmin.TemplateVersionDynamicParameters(ctx, templateAdminUser.ID, version.ID) + stream, err := templateAdmin.TemplateVersionDynamicParameters(ctx, version.ID) require.NoError(t, err) defer stream.Close(websocket.StatusGoingAway) @@ -242,7 +242,7 @@ func setupDynamicParamsTest(t *testing.T, args setupDynamicParamsTestParams) dyn }) owner := coderdtest.CreateFirstUser(t, ownerClient) - templateAdmin, templateAdminUser := coderdtest.CreateAnotherUser(t, ownerClient, owner.OrganizationID, rbac.RoleTemplateAdmin()) + templateAdmin, _ := coderdtest.CreateAnotherUser(t, ownerClient, owner.OrganizationID, rbac.RoleTemplateAdmin()) files := echo.WithExtraFiles(map[string][]byte{ "main.tf": args.mainTF, @@ -262,7 +262,7 @@ func setupDynamicParamsTest(t *testing.T, args setupDynamicParamsTestParams) dyn _ = coderdtest.CreateTemplate(t, templateAdmin, owner.OrganizationID, version.ID) ctx := testutil.Context(t, testutil.WaitShort) - stream, err := templateAdmin.TemplateVersionDynamicParameters(ctx, templateAdminUser.ID, version.ID) + stream, err := templateAdmin.TemplateVersionDynamicParameters(ctx, version.ID) if args.expectWebsocketError { require.Errorf(t, err, "expected error forming websocket") } else { diff --git a/codersdk/parameters.go b/codersdk/parameters.go index c15132b39b32b..5b6779fcdaec7 100644 --- a/codersdk/parameters.go +++ b/codersdk/parameters.go @@ -125,8 +125,8 @@ type DynamicParametersResponse struct { // TODO: Workspace tags } -func (c *Client) TemplateVersionDynamicParameters(ctx context.Context, userID, version uuid.UUID) (*wsjson.Stream[DynamicParametersResponse, DynamicParametersRequest], error) { - conn, err := c.Dial(ctx, fmt.Sprintf("/api/v2/templateversions/%s/parameters", userID, version), nil) +func (c *Client) TemplateVersionDynamicParameters(ctx context.Context, version uuid.UUID) (*wsjson.Stream[DynamicParametersResponse, DynamicParametersRequest], error) { + conn, err := c.Dial(ctx, fmt.Sprintf("/api/v2/templateversions/%s/dynamic-parameters", version), nil) if err != nil { return nil, err } diff --git a/enterprise/coderd/parameters_test.go b/enterprise/coderd/parameters_test.go index 76bd5a1eafdbb..605385430e779 100644 --- a/enterprise/coderd/parameters_test.go +++ b/enterprise/coderd/parameters_test.go @@ -59,7 +59,7 @@ func TestDynamicParametersOwnerGroups(t *testing.T) { _ = coderdtest.CreateTemplate(t, templateAdmin, owner.OrganizationID, version.ID) ctx := testutil.Context(t, testutil.WaitShort) - stream, err := templateAdmin.TemplateVersionDynamicParameters(ctx, templateAdminUser.ID, version.ID) + stream, err := templateAdmin.TemplateVersionDynamicParameters(ctx, version.ID) require.NoError(t, err) defer stream.Close(websocket.StatusGoingAway) From 98e25b4ec5688e4852bfe3483c392fd6163421ee Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Fri, 23 May 2025 13:33:04 -0500 Subject: [PATCH 05/10] fixup! move route to /dynamic-parameters --- coderd/coderd.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/coderd/coderd.go b/coderd/coderd.go index 20a8a49f7bb61..37e7d22a6d080 100644 --- a/coderd/coderd.go +++ b/coderd/coderd.go @@ -1156,7 +1156,7 @@ func New(options *Options) *API { r.Use( httpmw.RequireExperiment(api.Experiments, codersdk.ExperimentDynamicParameters), ) - r.Get("/parameters", api.templateVersionDynamicParameters) + r.Get("/dynamic-parameters", api.templateVersionDynamicParameters) }) }) r.Route("/users", func(r chi.Router) { From 2956aa6e2bf2608c06967a6c512aaea327a398ee Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Fri, 23 May 2025 13:41:47 -0500 Subject: [PATCH 06/10] formatting --- coderd/parameters_test.go | 2 +- codersdk/parameters.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/coderd/parameters_test.go b/coderd/parameters_test.go index 821b769ede530..c04f3b4a4ce6c 100644 --- a/coderd/parameters_test.go +++ b/coderd/parameters_test.go @@ -32,7 +32,7 @@ func TestDynamicParametersOwnerSSHPublicKey(t *testing.T) { cfg.Experiments = []string{string(codersdk.ExperimentDynamicParameters)} ownerClient := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerDaemon: true, DeploymentValues: cfg}) owner := coderdtest.CreateFirstUser(t, ownerClient) - templateAdmin, templateAdminUser := coderdtest.CreateAnotherUser(t, ownerClient, owner.OrganizationID, rbac.RoleTemplateAdmin()) + templateAdmin, _ := coderdtest.CreateAnotherUser(t, ownerClient, owner.OrganizationID, rbac.RoleTemplateAdmin()) dynamicParametersTerraformSource, err := os.ReadFile("testdata/parameters/public_key/main.tf") require.NoError(t, err) diff --git a/codersdk/parameters.go b/codersdk/parameters.go index 5b6779fcdaec7..aa24731f28ccb 100644 --- a/codersdk/parameters.go +++ b/codersdk/parameters.go @@ -115,7 +115,7 @@ type DynamicParametersRequest struct { ID int `json:"id"` Inputs map[string]string `json:"inputs"` // OwnerID if uuid.Nil, it defaults to `codersdk.Me` - OwnerID uuid.UUID `json:"owner_id"` + OwnerID uuid.UUID `json:"owner_id" format:"uuid"` } type DynamicParametersResponse struct { From 89b321357c67e73231e5ab4a04c2b6251e556bdb Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Fri, 23 May 2025 18:44:18 +0000 Subject: [PATCH 07/10] make gen --- coderd/apidoc/docs.go | 53 +++++++++++++++++++++++++-------- coderd/apidoc/swagger.json | 51 +++++++++++++++++++++++-------- docs/reference/api/templates.md | 36 ++++++++++++++++++---- 3 files changed, 111 insertions(+), 29 deletions(-) diff --git a/coderd/apidoc/docs.go b/coderd/apidoc/docs.go index 97a1e5881cb03..567cfa8eed442 100644 --- a/coderd/apidoc/docs.go +++ b/coderd/apidoc/docs.go @@ -5880,6 +5880,43 @@ const docTemplate = `{ } } }, + "/templateversions/{templateversion}/dynamic-parameters": { + "get": { + "security": [ + { + "CoderSessionToken": [] + } + ], + "tags": [ + "Templates" + ], + "summary": "Open dynamic parameters WebSocket by template version", + "operationId": "open-dynamic-parameters-websocket-by-template-version", + "parameters": [ + { + "type": "string", + "format": "uuid", + "description": "Template version ID", + "name": "user", + "in": "path", + "required": true + }, + { + "type": "string", + "format": "uuid", + "description": "Template version ID", + "name": "templateversion", + "in": "path", + "required": true + } + ], + "responses": { + "101": { + "description": "Switching Protocols" + } + } + } + }, "/templateversions/{templateversion}/external-auth": { "get": { "security": [ @@ -5984,17 +6021,9 @@ const docTemplate = `{ "tags": [ "Templates" ], - "summary": "Open dynamic parameters WebSocket by template version", - "operationId": "open-dynamic-parameters-websocket-by-template-version", + "summary": "Removed: Get parameters by template version", + "operationId": "removed-get-parameters-by-template-version", "parameters": [ - { - "type": "string", - "format": "uuid", - "description": "Template version ID", - "name": "user", - "in": "path", - "required": true - }, { "type": "string", "format": "uuid", @@ -6005,8 +6034,8 @@ const docTemplate = `{ } ], "responses": { - "101": { - "description": "Switching Protocols" + "200": { + "description": "OK" } } } diff --git a/coderd/apidoc/swagger.json b/coderd/apidoc/swagger.json index dfe2bbb5b9d55..b1906025132dd 100644 --- a/coderd/apidoc/swagger.json +++ b/coderd/apidoc/swagger.json @@ -5197,6 +5197,41 @@ } } }, + "/templateversions/{templateversion}/dynamic-parameters": { + "get": { + "security": [ + { + "CoderSessionToken": [] + } + ], + "tags": ["Templates"], + "summary": "Open dynamic parameters WebSocket by template version", + "operationId": "open-dynamic-parameters-websocket-by-template-version", + "parameters": [ + { + "type": "string", + "format": "uuid", + "description": "Template version ID", + "name": "user", + "in": "path", + "required": true + }, + { + "type": "string", + "format": "uuid", + "description": "Template version ID", + "name": "templateversion", + "in": "path", + "required": true + } + ], + "responses": { + "101": { + "description": "Switching Protocols" + } + } + } + }, "/templateversions/{templateversion}/external-auth": { "get": { "security": [ @@ -5291,17 +5326,9 @@ } ], "tags": ["Templates"], - "summary": "Open dynamic parameters WebSocket by template version", - "operationId": "open-dynamic-parameters-websocket-by-template-version", + "summary": "Removed: Get parameters by template version", + "operationId": "removed-get-parameters-by-template-version", "parameters": [ - { - "type": "string", - "format": "uuid", - "description": "Template version ID", - "name": "user", - "in": "path", - "required": true - }, { "type": "string", "format": "uuid", @@ -5312,8 +5339,8 @@ } ], "responses": { - "101": { - "description": "Switching Protocols" + "200": { + "description": "OK" } } } diff --git a/docs/reference/api/templates.md b/docs/reference/api/templates.md index c50f4bbfa4f44..8eb322208d358 100644 --- a/docs/reference/api/templates.md +++ b/docs/reference/api/templates.md @@ -2575,6 +2575,33 @@ Status Code **200** To perform this operation, you must be authenticated. [Learn more](authentication.md). +## Open dynamic parameters WebSocket by template version + +### Code samples + +```shell +# Example request using curl +curl -X GET http://coder-server:8080/api/v2/templateversions/{templateversion}/dynamic-parameters \ + -H 'Coder-Session-Token: API_KEY' +``` + +`GET /templateversions/{templateversion}/dynamic-parameters` + +### Parameters + +| Name | In | Type | Required | Description | +|-------------------|------|--------------|----------|---------------------| +| `user` | path | string(uuid) | true | Template version ID | +| `templateversion` | path | string(uuid) | true | Template version ID | + +### Responses + +| Status | Meaning | Description | Schema | +|--------|--------------------------------------------------------------------------|---------------------|--------| +| 101 | [Switching Protocols](https://tools.ietf.org/html/rfc7231#section-6.2.2) | Switching Protocols | | + +To perform this operation, you must be authenticated. [Learn more](authentication.md). + ## Get external auth by template version ### Code samples @@ -2708,7 +2735,7 @@ Status Code **200** To perform this operation, you must be authenticated. [Learn more](authentication.md). -## Open dynamic parameters WebSocket by template version +## Removed: Get parameters by template version ### Code samples @@ -2724,14 +2751,13 @@ curl -X GET http://coder-server:8080/api/v2/templateversions/{templateversion}/p | Name | In | Type | Required | Description | |-------------------|------|--------------|----------|---------------------| -| `user` | path | string(uuid) | true | Template version ID | | `templateversion` | path | string(uuid) | true | Template version ID | ### Responses -| Status | Meaning | Description | Schema | -|--------|--------------------------------------------------------------------------|---------------------|--------| -| 101 | [Switching Protocols](https://tools.ietf.org/html/rfc7231#section-6.2.2) | Switching Protocols | | +| Status | Meaning | Description | Schema | +|--------|---------------------------------------------------------|-------------|--------| +| 200 | [OK](https://tools.ietf.org/html/rfc7231#section-6.3.1) | OK | | To perform this operation, you must be authenticated. [Learn more](authentication.md). From db938dec43f6177e34280bdd289e6955751eb3d6 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Tue, 27 May 2025 11:23:51 -0500 Subject: [PATCH 08/10] allow undefined as owner input --- codersdk/parameters.go | 2 +- site/src/api/typesGenerated.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/codersdk/parameters.go b/codersdk/parameters.go index aa24731f28ccb..035537d34259e 100644 --- a/codersdk/parameters.go +++ b/codersdk/parameters.go @@ -115,7 +115,7 @@ type DynamicParametersRequest struct { ID int `json:"id"` Inputs map[string]string `json:"inputs"` // OwnerID if uuid.Nil, it defaults to `codersdk.Me` - OwnerID uuid.UUID `json:"owner_id" format:"uuid"` + OwnerID uuid.UUID `json:"owner_id,omitempty" format:"uuid"` } type DynamicParametersResponse struct { diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index 27bbbcd9061ab..de9014b600cd8 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -774,7 +774,7 @@ export const DisplayApps: DisplayApp[] = [ export interface DynamicParametersRequest { readonly id: number; readonly inputs: Record; - readonly owner_id: string; + readonly owner_id?: string; } // From codersdk/parameters.go From f069ba10740c2d97e288639bc5a4f5a9cebc52ee Mon Sep 17 00:00:00 2001 From: Jaayden Halko Date: Tue, 27 May 2025 11:46:05 -0500 Subject: [PATCH 09/10] fix: update websocket creation and requests to handle changing owner id (#18020) --- site/src/api/api.ts | 3 +- .../CreateWorkspacePageExperimental.tsx | 63 +++++++++---------- .../CreateWorkspacePageViewExperimental.tsx | 14 +++-- .../WorkspaceParametersPageExperimental.tsx | 21 ++----- 4 files changed, 47 insertions(+), 54 deletions(-) diff --git a/site/src/api/api.ts b/site/src/api/api.ts index 9e579c3706de6..81931c003c99d 100644 --- a/site/src/api/api.ts +++ b/site/src/api/api.ts @@ -1000,7 +1000,6 @@ class ApiMethods { }; templateVersionDynamicParameters = ( - userId: string, versionId: string, { onMessage, @@ -1013,7 +1012,7 @@ class ApiMethods { }, ): WebSocket => { const socket = createWebSocket( - `/api/v2/users/${userId}/templateversions/${versionId}/parameters`, + `/api/v2/templateversions/${versionId}/dynamic-parameters`, ); socket.addEventListener("message", (event) => diff --git a/site/src/pages/CreateWorkspacePage/CreateWorkspacePageExperimental.tsx b/site/src/pages/CreateWorkspacePage/CreateWorkspacePageExperimental.tsx index fbb35c61ee047..0a5ccb93d083a 100644 --- a/site/src/pages/CreateWorkspacePage/CreateWorkspacePageExperimental.tsx +++ b/site/src/pages/CreateWorkspacePage/CreateWorkspacePageExperimental.tsx @@ -90,16 +90,19 @@ const CreateWorkspacePageExperimental: FC = () => { const autofillParameters = getAutofillParameters(searchParams); - const sendMessage = useCallback((formValues: Record) => { - const request: DynamicParametersRequest = { - id: wsResponseId.current + 1, - inputs: formValues, - }; - if (ws.current && ws.current.readyState === WebSocket.OPEN) { - ws.current.send(JSON.stringify(request)); - wsResponseId.current = wsResponseId.current + 1; - } - }, []); + const sendMessage = useEffectEvent( + (formValues: Record, ownerId?: string) => { + const request: DynamicParametersRequest = { + id: wsResponseId.current + 1, + owner_id: ownerId ?? owner.id, + inputs: formValues, + }; + if (ws.current && ws.current.readyState === WebSocket.OPEN) { + ws.current.send(JSON.stringify(request)); + wsResponseId.current = wsResponseId.current + 1; + } + }, + ); // On page load, sends all initial parameter values to the websocket // (including defaults and autofilled from the url) @@ -147,35 +150,31 @@ const CreateWorkspacePageExperimental: FC = () => { useEffect(() => { if (!realizedVersionId) return; - const socket = API.templateVersionDynamicParameters( - owner.id, - realizedVersionId, - { - onMessage, - onError: (error) => { - if (ws.current === socket) { - setWsError(error); - } - }, - onClose: () => { - if (ws.current === socket) { - setWsError( - new DetailedError( - "Websocket connection for dynamic parameters unexpectedly closed.", - "Refresh the page to reset the form.", - ), - ); - } - }, + const socket = API.templateVersionDynamicParameters(realizedVersionId, { + onMessage, + onError: (error) => { + if (ws.current === socket) { + setWsError(error); + } }, - ); + onClose: () => { + if (ws.current === socket) { + setWsError( + new DetailedError( + "Websocket connection for dynamic parameters unexpectedly closed.", + "Refresh the page to reset the form.", + ), + ); + } + }, + }); ws.current = socket; return () => { socket.close(); }; - }, [owner.id, realizedVersionId, onMessage]); + }, [realizedVersionId, onMessage]); const organizationId = templateQuery.data?.organization_id; diff --git a/site/src/pages/CreateWorkspacePage/CreateWorkspacePageViewExperimental.tsx b/site/src/pages/CreateWorkspacePage/CreateWorkspacePageViewExperimental.tsx index db629e813415e..4f990ef9de695 100644 --- a/site/src/pages/CreateWorkspacePage/CreateWorkspacePageViewExperimental.tsx +++ b/site/src/pages/CreateWorkspacePage/CreateWorkspacePageViewExperimental.tsx @@ -70,7 +70,7 @@ export interface CreateWorkspacePageViewExperimentalProps { owner: TypesGen.User, ) => void; resetMutation: () => void; - sendMessage: (message: Record) => void; + sendMessage: (message: Record, ownerId?: string) => void; startPollingExternalAuth: () => void; owner: TypesGen.User; setOwner: (user: TypesGen.User) => void; @@ -271,9 +271,10 @@ export const CreateWorkspacePageViewExperimental: FC< form.values.rich_parameter_values, ]); - // send the last user modified parameter and all touched parameters to the websocket + // include any modified parameters and all touched parameters to the websocket request const sendDynamicParamsRequest = ( parameters: Array<{ parameter: PreviewParameter; value: string }>, + ownerId?: string, ) => { const formInputs: Record = {}; const formParameters = form.values.rich_parameter_values ?? []; @@ -294,7 +295,12 @@ export const CreateWorkspacePageViewExperimental: FC< } } - sendMessage(formInputs); + sendMessage(formInputs, ownerId); + }; + + const handleOwnerChange = (user: TypesGen.User) => { + setOwner(user); + sendDynamicParamsRequest([], user.id); }; const handleChange = async ( @@ -449,7 +455,7 @@ export const CreateWorkspacePageViewExperimental: FC< { - setOwner(user ?? defaultOwner); + handleOwnerChange(user ?? defaultOwner); }} size="medium" /> diff --git a/site/src/pages/WorkspaceSettingsPage/WorkspaceParametersPage/WorkspaceParametersPageExperimental.tsx b/site/src/pages/WorkspaceSettingsPage/WorkspaceParametersPage/WorkspaceParametersPageExperimental.tsx index 156298be26e13..55c4422fd7095 100644 --- a/site/src/pages/WorkspaceSettingsPage/WorkspaceParametersPage/WorkspaceParametersPageExperimental.tsx +++ b/site/src/pages/WorkspaceSettingsPage/WorkspaceParametersPage/WorkspaceParametersPageExperimental.tsx @@ -14,14 +14,7 @@ import { Link } from "components/Link/Link"; import { Loader } from "components/Loader/Loader"; import { useEffectEvent } from "hooks/hookPolyfills"; import type { FC } from "react"; -import { - useCallback, - useContext, - useEffect, - useMemo, - useRef, - useState, -} from "react"; +import { useContext, useEffect, useMemo, useRef, useState } from "react"; import { Helmet } from "react-helmet-async"; import { useMutation, useQuery } from "react-query"; import { useNavigate } from "react-router-dom"; @@ -46,16 +39,17 @@ const WorkspaceParametersPageExperimental: FC = () => { const ws = useRef(null); const [wsError, setWsError] = useState(null); - const sendMessage = useCallback((formValues: Record) => { + const sendMessage = useEffectEvent((formValues: Record) => { const request: DynamicParametersRequest = { id: wsResponseId.current + 1, + owner_id: workspace.owner_id, inputs: formValues, }; if (ws.current && ws.current.readyState === WebSocket.OPEN) { ws.current.send(JSON.stringify(request)); wsResponseId.current = wsResponseId.current + 1; } - }, []); + }); const onMessage = useEffectEvent((response: DynamicParametersResponse) => { if (latestResponse && latestResponse?.id >= response.id) { @@ -69,7 +63,6 @@ const WorkspaceParametersPageExperimental: FC = () => { if (!workspace.latest_build.template_version_id) return; const socket = API.templateVersionDynamicParameters( - workspace.owner_id, workspace.latest_build.template_version_id, { onMessage, @@ -96,11 +89,7 @@ const WorkspaceParametersPageExperimental: FC = () => { return () => { socket.close(); }; - }, [ - workspace.owner_id, - workspace.latest_build.template_version_id, - onMessage, - ]); + }, [workspace.latest_build.template_version_id, onMessage]); const updateParameters = useMutation({ mutationFn: (buildParameters: WorkspaceBuildParameter[]) => From 5ad3ce92f1f7f3708649e15f0b0b3ec38b66391b Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Tue, 27 May 2025 11:56:48 -0500 Subject: [PATCH 10/10] add unit test for bad owner --- coderd/parameters_test.go | 41 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/coderd/parameters_test.go b/coderd/parameters_test.go index c04f3b4a4ce6c..91809d3a037d6 100644 --- a/coderd/parameters_test.go +++ b/coderd/parameters_test.go @@ -210,6 +210,47 @@ func TestDynamicParametersWithTerraformValues(t *testing.T) { // test to make it obvious what this test is doing. require.Zero(t, setup.api.FileCache.Count()) }) + + t.Run("BadOwner", func(t *testing.T) { + t.Parallel() + + dynamicParametersTerraformSource, err := os.ReadFile("testdata/parameters/modules/main.tf") + require.NoError(t, err) + + modulesArchive, err := terraform.GetModulesArchive(os.DirFS("testdata/parameters/modules")) + require.NoError(t, err) + + setup := setupDynamicParamsTest(t, setupDynamicParamsTestParams{ + provisionerDaemonVersion: provProto.CurrentVersion.String(), + mainTF: dynamicParametersTerraformSource, + modulesArchive: modulesArchive, + plan: nil, + static: nil, + }) + + ctx := testutil.Context(t, testutil.WaitShort) + stream := setup.stream + previews := stream.Chan() + + // Should see the output of the module represented + preview := testutil.RequireReceive(ctx, t, previews) + require.Equal(t, -1, preview.ID) + require.Empty(t, preview.Diagnostics) + + err = stream.Send(codersdk.DynamicParametersRequest{ + ID: 1, + Inputs: map[string]string{ + "jetbrains_ide": "GO", + }, + OwnerID: uuid.New(), + }) + require.NoError(t, err) + + preview = testutil.RequireReceive(ctx, t, previews) + require.Equal(t, 1, preview.ID) + require.Len(t, preview.Diagnostics, 1) + require.Equal(t, preview.Diagnostics[0].Extra.Code, "owner_not_found") + }) } type setupDynamicParamsTestParams struct {