From efeb85ed3c91dacc59e39b20b19d1572ebe2af89 Mon Sep 17 00:00:00 2001 From: Ammar Bandukwala Date: Sun, 21 Jan 2024 13:43:05 -0600 Subject: [PATCH 01/21] feat: add user-level parameter auto-fill --- coderd/apidoc/docs.go | 51 ++++++++++++++++++ coderd/apidoc/swagger.json | 47 ++++++++++++++++ coderd/coderd.go | 1 + coderd/database/dbauthz/dbauthz.go | 11 ++++ coderd/database/dbmem/dbmem.go | 36 +++++++++++++ coderd/database/dbmetrics/dbmetrics.go | 7 +++ coderd/database/dbmock/dbmock.go | 15 ++++++ coderd/database/querier.go | 1 + coderd/database/queries.sql.go | 54 +++++++++++++++++++ .../queries/workspacebuildparameters.sql | 25 +++++++++ coderd/users.go | 47 ++++++++++++++++ coderd/users_test.go | 47 ++++++++++++++++ codersdk/users.go | 22 ++++++++ docs/api/schemas.md | 18 +++++++ docs/api/users.md | 52 ++++++++++++++++++ site/src/api/typesGenerated.ts | 7 +++ 16 files changed, 441 insertions(+) diff --git a/coderd/apidoc/docs.go b/coderd/apidoc/docs.go index 06ed3e19dfe1c..1b5e5f29567b5 100644 --- a/coderd/apidoc/docs.go +++ b/coderd/apidoc/docs.go @@ -4555,6 +4555,43 @@ const docTemplate = `{ } } }, + "/users/{user}/parameters": { + "get": { + "security": [ + { + "CoderSessionToken": [] + } + ], + "produces": [ + "application/json" + ], + "tags": [ + "Users" + ], + "summary": "Get recent build parameters for user.", + "operationId": "get-user-params", + "parameters": [ + { + "type": "string", + "description": "User ID, username, or me", + "name": "user", + "in": "path", + "required": true + } + ], + "responses": { + "200": { + "description": "OK", + "schema": { + "type": "array", + "items": { + "$ref": "#/definitions/codersdk.UserParameter" + } + } + } + } + } + }, "/users/{user}/password": { "put": { "security": [ @@ -11779,6 +11816,20 @@ const docTemplate = `{ } } }, + "codersdk.UserParameter": { + "type": "object", + "properties": { + "display_name": { + "type": "string" + }, + "last_used": { + "type": "string" + }, + "name": { + "type": "string" + } + } + }, "codersdk.UserQuietHoursScheduleConfig": { "type": "object", "properties": { diff --git a/coderd/apidoc/swagger.json b/coderd/apidoc/swagger.json index 8982d4a4a781f..493fe3587888f 100644 --- a/coderd/apidoc/swagger.json +++ b/coderd/apidoc/swagger.json @@ -4007,6 +4007,39 @@ } } }, + "/users/{user}/parameters": { + "get": { + "security": [ + { + "CoderSessionToken": [] + } + ], + "produces": ["application/json"], + "tags": ["Users"], + "summary": "Get recent build parameters for user.", + "operationId": "get-user-params", + "parameters": [ + { + "type": "string", + "description": "User ID, username, or me", + "name": "user", + "in": "path", + "required": true + } + ], + "responses": { + "200": { + "description": "OK", + "schema": { + "type": "array", + "items": { + "$ref": "#/definitions/codersdk.UserParameter" + } + } + } + } + } + }, "/users/{user}/password": { "put": { "security": [ @@ -10676,6 +10709,20 @@ } } }, + "codersdk.UserParameter": { + "type": "object", + "properties": { + "display_name": { + "type": "string" + }, + "last_used": { + "type": "string" + }, + "name": { + "type": "string" + } + } + }, "codersdk.UserQuietHoursScheduleConfig": { "type": "object", "properties": { diff --git a/coderd/coderd.go b/coderd/coderd.go index e3c935971f3e3..630d284017ee5 100644 --- a/coderd/coderd.go +++ b/coderd/coderd.go @@ -823,6 +823,7 @@ func New(options *Options) *API { r.Post("/convert-login", api.postConvertLoginType) r.Delete("/", api.deleteUser) r.Get("/", api.userByName) + r.Get("/parameters", api.userParameters) r.Get("/login-type", api.userLoginType) r.Put("/profile", api.putUserProfile) r.Route("/status", func(r chi.Router) { diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go index a5b295e2e35eb..f52ab83377567 100644 --- a/coderd/database/dbauthz/dbauthz.go +++ b/coderd/database/dbauthz/dbauthz.go @@ -1754,6 +1754,17 @@ func (q *querier) GetUserLinksByUserID(ctx context.Context, userID uuid.UUID) ([ return q.db.GetUserLinksByUserID(ctx, userID) } +func (q *querier) GetUserWorkspaceBuildParameters(ctx context.Context, ownerID uuid.UUID) ([]database.GetUserWorkspaceBuildParametersRow, error) { + u, err := q.db.GetUserByID(ctx, ownerID) + if err != nil { + return nil, err + } + if err := q.authorizeContext(ctx, rbac.ActionUpdate, u.UserDataRBACObject()); err != nil { + return nil, err + } + return q.db.GetUserWorkspaceBuildParameters(ctx, ownerID) +} + func (q *querier) GetUsers(ctx context.Context, arg database.GetUsersParams) ([]database.GetUsersRow, error) { // This does the filtering in SQL. prep, err := prepareSQLFilter(ctx, q.auth, rbac.ActionRead, rbac.ResourceUser.Type) diff --git a/coderd/database/dbmem/dbmem.go b/coderd/database/dbmem/dbmem.go index 0800fb5dd0a54..0f8c64b634445 100644 --- a/coderd/database/dbmem/dbmem.go +++ b/coderd/database/dbmem/dbmem.go @@ -3758,6 +3758,42 @@ func (q *FakeQuerier) GetUserLinksByUserID(_ context.Context, userID uuid.UUID) return uls, nil } +func (q *FakeQuerier) GetUserWorkspaceBuildParameters(ctx context.Context, ownerID uuid.UUID) ([]database.GetUserWorkspaceBuildParametersRow, error) { + q.mutex.RLock() + defer q.mutex.RUnlock() + + userWorkspaceIDs := make(map[uuid.UUID]struct{}) + for _, ws := range q.workspaces { + if ws.OwnerID != ownerID { + continue + } + userWorkspaceIDs[ws.ID] = struct{}{} + } + + userWorkspaceBuilds := make(map[uuid.UUID]database.WorkspaceBuildTable) + for _, wb := range q.workspaceBuilds { + if _, ok := userWorkspaceIDs[wb.WorkspaceID]; !ok { + continue + } + userWorkspaceBuilds[wb.ID] = wb + } + + userWorkspaceBuildParameters := make([]database.GetUserWorkspaceBuildParametersRow, 0) + for _, wbp := range q.workspaceBuildParameters { + wb, ok := userWorkspaceBuilds[wbp.WorkspaceBuildID] + if !ok { + continue + } + userWorkspaceBuildParameters = append(userWorkspaceBuildParameters, database.GetUserWorkspaceBuildParametersRow{ + Name: wbp.Name, + Value: wbp.Value, + CreatedAt: wb.CreatedAt, + }) + } + + return userWorkspaceBuildParameters, nil +} + func (q *FakeQuerier) GetUsers(_ context.Context, params database.GetUsersParams) ([]database.GetUsersRow, error) { if err := validateDatabaseType(params); err != nil { return nil, err diff --git a/coderd/database/dbmetrics/dbmetrics.go b/coderd/database/dbmetrics/dbmetrics.go index 625871500dbeb..65f393c6e494b 100644 --- a/coderd/database/dbmetrics/dbmetrics.go +++ b/coderd/database/dbmetrics/dbmetrics.go @@ -998,6 +998,13 @@ func (m metricsStore) GetUserLinksByUserID(ctx context.Context, userID uuid.UUID return r0, r1 } +func (m metricsStore) GetUserWorkspaceBuildParameters(ctx context.Context, ownerID uuid.UUID) ([]database.GetUserWorkspaceBuildParametersRow, error) { + start := time.Now() + r0, r1 := m.s.GetUserWorkspaceBuildParameters(ctx, ownerID) + m.queryLatencies.WithLabelValues("GetUserWorkspaceBuildParameters").Observe(time.Since(start).Seconds()) + return r0, r1 +} + func (m metricsStore) GetUsers(ctx context.Context, arg database.GetUsersParams) ([]database.GetUsersRow, error) { start := time.Now() users, err := m.s.GetUsers(ctx, arg) diff --git a/coderd/database/dbmock/dbmock.go b/coderd/database/dbmock/dbmock.go index bfb93405f5524..991bf039f50b5 100644 --- a/coderd/database/dbmock/dbmock.go +++ b/coderd/database/dbmock/dbmock.go @@ -2075,6 +2075,21 @@ func (mr *MockStoreMockRecorder) GetUserLinksByUserID(arg0, arg1 any) *gomock.Ca return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetUserLinksByUserID", reflect.TypeOf((*MockStore)(nil).GetUserLinksByUserID), arg0, arg1) } +// GetUserWorkspaceBuildParameters mocks base method. +func (m *MockStore) GetUserWorkspaceBuildParameters(arg0 context.Context, arg1 uuid.UUID) ([]database.GetUserWorkspaceBuildParametersRow, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetUserWorkspaceBuildParameters", arg0, arg1) + ret0, _ := ret[0].([]database.GetUserWorkspaceBuildParametersRow) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetUserWorkspaceBuildParameters indicates an expected call of GetUserWorkspaceBuildParameters. +func (mr *MockStoreMockRecorder) GetUserWorkspaceBuildParameters(arg0, arg1 any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetUserWorkspaceBuildParameters", reflect.TypeOf((*MockStore)(nil).GetUserWorkspaceBuildParameters), arg0, arg1) +} + // GetUsers mocks base method. func (m *MockStore) GetUsers(arg0 context.Context, arg1 database.GetUsersParams) ([]database.GetUsersRow, error) { m.ctrl.T.Helper() diff --git a/coderd/database/querier.go b/coderd/database/querier.go index 8947ba185d14d..d5756f9d00816 100644 --- a/coderd/database/querier.go +++ b/coderd/database/querier.go @@ -209,6 +209,7 @@ type sqlcQuerier interface { GetUserLinkByLinkedID(ctx context.Context, linkedID string) (UserLink, error) GetUserLinkByUserIDLoginType(ctx context.Context, arg GetUserLinkByUserIDLoginTypeParams) (UserLink, error) GetUserLinksByUserID(ctx context.Context, userID uuid.UUID) ([]UserLink, error) + GetUserWorkspaceBuildParameters(ctx context.Context, ownerID uuid.UUID) ([]GetUserWorkspaceBuildParametersRow, error) // This will never return deleted users. GetUsers(ctx context.Context, arg GetUsersParams) ([]GetUsersRow, error) // This shouldn't check for deleted, because it's frequently used diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 4c4bfc6012e7b..e77c6f9d8ec22 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -9870,6 +9870,60 @@ func (q *sqlQuerier) InsertWorkspaceAppStats(ctx context.Context, arg InsertWork return err } +const getUserWorkspaceBuildParameters = `-- name: GetUserWorkspaceBuildParameters :many +SELECT + sub.name, + sub.value, + sub.created_at +FROM ( + SELECT + wbp.name, + wbp.value, + wb.created_at, + ROW_NUMBER() OVER (PARTITION BY wbp.name ORDER BY wb.created_at DESC) as rn + FROM + workspace_build_parameters wbp + JOIN + workspace_builds wb ON wb.id = wbp.workspace_build_id + JOIN + workspaces w ON w.id = wb.workspace_id + WHERE + w.owner_id = $1 + AND wb.transition = 'start' +) sub +WHERE + sub.rn = 1 +` + +type GetUserWorkspaceBuildParametersRow struct { + Name string `db:"name" json:"name"` + Value string `db:"value" json:"value"` + CreatedAt time.Time `db:"created_at" json:"created_at"` +} + +func (q *sqlQuerier) GetUserWorkspaceBuildParameters(ctx context.Context, ownerID uuid.UUID) ([]GetUserWorkspaceBuildParametersRow, error) { + rows, err := q.db.QueryContext(ctx, getUserWorkspaceBuildParameters, ownerID) + if err != nil { + return nil, err + } + defer rows.Close() + var items []GetUserWorkspaceBuildParametersRow + for rows.Next() { + var i GetUserWorkspaceBuildParametersRow + if err := rows.Scan(&i.Name, &i.Value, &i.CreatedAt); err != nil { + return nil, err + } + items = append(items, i) + } + if err := rows.Close(); err != nil { + return nil, err + } + if err := rows.Err(); err != nil { + return nil, err + } + return items, nil +} + const getWorkspaceBuildParameters = `-- name: GetWorkspaceBuildParameters :many SELECT workspace_build_id, name, value diff --git a/coderd/database/queries/workspacebuildparameters.sql b/coderd/database/queries/workspacebuildparameters.sql index 3b90673da7089..22ae7472c727c 100644 --- a/coderd/database/queries/workspacebuildparameters.sql +++ b/coderd/database/queries/workspacebuildparameters.sql @@ -14,3 +14,28 @@ FROM workspace_build_parameters WHERE workspace_build_id = $1; + +-- name: GetUserWorkspaceBuildParameters :many +SELECT + sub.name, + sub.value, + sub.created_at +FROM ( + SELECT + wbp.name, + wbp.value, + wb.created_at, + ROW_NUMBER() OVER (PARTITION BY wbp.name ORDER BY wb.created_at DESC) as rn + FROM + workspace_build_parameters wbp + JOIN + workspace_builds wb ON wb.id = wbp.workspace_build_id + JOIN + workspaces w ON w.id = wb.workspace_id + WHERE + w.owner_id = $1 + AND wb.transition = 'start' +) sub +WHERE + sub.rn = 1 +LIMIT 100; diff --git a/coderd/users.go b/coderd/users.go index 6cb8b03d37b50..e7beac21a0c1b 100644 --- a/coderd/users.go +++ b/coderd/users.go @@ -569,6 +569,53 @@ func (api *API) userByName(rw http.ResponseWriter, r *http.Request) { httpapi.Write(ctx, rw, http.StatusOK, db2sdk.User(user, organizationIDs)) } +// Returns recent build parameters for the signed-in user. +// +// @Summary Get recent build parameters for user. +// @ID get-user-params +// @Security CoderSessionToken +// @Produce json +// @Tags Users +// @Param user path string true "User ID, username, or me" +// @Success 200 {array} codersdk.UserParameter +// @Router /users/{user}/parameters [get] +func (api *API) userParameters(rw http.ResponseWriter, r *http.Request) { + var ( + apiKey = httpmw.APIKey(r) + user = httpmw.UserParam(r) + ) + + if apiKey.UserID != user.ID { + httpapi.Write(r.Context(), rw, http.StatusForbidden, codersdk.Response{ + Message: "You are not authorized to view this user's parameters.", + }) + return + } + + params, err := api.Database.GetUserWorkspaceBuildParameters( + r.Context(), + apiKey.UserID, + ) + if err != nil { + httpapi.Write(r.Context(), rw, http.StatusInternalServerError, codersdk.Response{ + Message: "Internal error fetching user's parameters.", + Detail: err.Error(), + }) + return + } + + var sdkParams []codersdk.UserParameter + for _, param := range params { + sdkParams = append(sdkParams, codersdk.UserParameter{ + Name: param.Name, + Value: param.Value, + LastUsedAt: param.CreatedAt, + }) + } + + httpapi.Write(r.Context(), rw, http.StatusOK, sdkParams) +} + // Returns the user's login type. This only works if the api key for authorization // and the requested user match. Eg: 'me' // diff --git a/coderd/users_test.go b/coderd/users_test.go index c73bd3014dc05..37ea7bc74efab 100644 --- a/coderd/users_test.go +++ b/coderd/users_test.go @@ -1721,6 +1721,53 @@ func TestSuspendedPagination(t *testing.T) { require.Equal(t, expected, page.Users, "expected page") } +func TestUserParameters(t *testing.T) { + t.Parallel() + t.Run("NotSelf", func(t *testing.T) { + t.Parallel() + client := coderdtest.New(t, nil) + + u1 := coderdtest.CreateFirstUser(t, client) + + _, u2 := coderdtest.CreateAnotherUser(t, client, u1.OrganizationID) + + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) + defer cancel() + + _, err := client.UserParameters( + ctx, + u2.ID.String(), + ) + + var apiErr *codersdk.Error + require.ErrorAs(t, err, &apiErr) + require.Equal(t, http.StatusForbidden, apiErr.StatusCode()) + }) + + t.Run("FindsParameters", func(t *testing.T) { + t.Parallel() + client, _, _ := coderdtest.NewWithAPI(t, &coderdtest.Options{IncludeProvisionerDaemon: true}) + + u1 := coderdtest.CreateFirstUser(t, client) + + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) + defer cancel() + + version := coderdtest.CreateTemplateVersion(t, client, u1.OrganizationID, nil) + coderdtest.AwaitTemplateVersionJobCompleted(t, client, version.ID) + template := coderdtest.CreateTemplate(t, client, u1.OrganizationID, version.ID) + coderdtest.CreateWorkspace(t, client, u1.OrganizationID, template.ID) + + params, err := client.UserParameters( + ctx, + u1.UserID.String(), + ) + require.NoError(t, err) + + require.Equal(t, 1, len(params)) + }) +} + // TestPaginatedUsers creates a list of users, then tries to paginate through // them using different page sizes. func TestPaginatedUsers(t *testing.T) { diff --git a/codersdk/users.go b/codersdk/users.go index a43b197c747f1..87a4d972a9555 100644 --- a/codersdk/users.go +++ b/codersdk/users.go @@ -221,6 +221,28 @@ type OIDCAuthMethod struct { IconURL string `json:"iconUrl"` } +type UserParameter struct { + Name string `json:"name"` + Value string `json:"display_name"` + LastUsedAt time.Time `json:"last_used"` +} + +// UserParameters returns all recently used parameters for the given user. +func (c *Client) UserParameters(ctx context.Context, user string) ([]UserParameter, error) { + res, err := c.Request(ctx, http.MethodGet, fmt.Sprintf("/api/v2/users/%s/parameters", user), nil) + if err != nil { + return nil, err + } + defer res.Body.Close() + + if res.StatusCode != http.StatusOK { + return nil, ReadBodyAsError(res) + } + + var params []UserParameter + return params, json.NewDecoder(res.Body).Decode(¶ms) +} + // HasFirstUser returns whether the first user has been created. func (c *Client) HasFirstUser(ctx context.Context) (bool, error) { res, err := c.Request(ctx, http.MethodGet, "/api/v2/users/first", nil) diff --git a/docs/api/schemas.md b/docs/api/schemas.md index a51b3bcdfd3df..151ea7268ceb8 100644 --- a/docs/api/schemas.md +++ b/docs/api/schemas.md @@ -5784,6 +5784,24 @@ If the schedule is empty, the user will be updated to use the default schedule.| | ------------ | ---------------------------------------- | -------- | ------------ | ----------- | | `login_type` | [codersdk.LoginType](#codersdklogintype) | false | | | +## codersdk.UserParameter + +```json +{ + "display_name": "string", + "last_used": "string", + "name": "string" +} +``` + +### Properties + +| Name | Type | Required | Restrictions | Description | +| -------------- | ------ | -------- | ------------ | ----------- | +| `display_name` | string | false | | | +| `last_used` | string | false | | | +| `name` | string | false | | | + ## codersdk.UserQuietHoursScheduleConfig ```json diff --git a/docs/api/users.md b/docs/api/users.md index 86869d1e8eb6a..295778a9a8d43 100644 --- a/docs/api/users.md +++ b/docs/api/users.md @@ -1009,6 +1009,58 @@ curl -X GET http://coder-server:8080/api/v2/users/{user}/organizations/{organiza To perform this operation, you must be authenticated. [Learn more](authentication.md). +## Get recent build parameters for user. + +### Code samples + +```shell +# Example request using curl +curl -X GET http://coder-server:8080/api/v2/users/{user}/parameters \ + -H 'Accept: application/json' \ + -H 'Coder-Session-Token: API_KEY' +``` + +`GET /users/{user}/parameters` + +### Parameters + +| Name | In | Type | Required | Description | +| ------ | ---- | ------ | -------- | ------------------------ | +| `user` | path | string | true | User ID, username, or me | + +### Example responses + +> 200 Response + +```json +[ + { + "display_name": "string", + "last_used": "string", + "name": "string" + } +] +``` + +### Responses + +| Status | Meaning | Description | Schema | +| ------ | ------------------------------------------------------- | ----------- | ------------------------------------------------------------------- | +| 200 | [OK](https://tools.ietf.org/html/rfc7231#section-6.3.1) | OK | array of [codersdk.UserParameter](schemas.md#codersdkuserparameter) | + +

Response Schema

+ +Status Code **200** + +| Name | Type | Required | Restrictions | Description | +| ---------------- | ------ | -------- | ------------ | ----------- | +| `[array item]` | array | false | | | +| `» display_name` | string | false | | | +| `» last_used` | string | false | | | +| `» name` | string | false | | | + +To perform this operation, you must be authenticated. [Learn more](authentication.md). + ## Update user password ### Code samples diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index 7127b5f72c114..9339d56d49f0f 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -1417,6 +1417,13 @@ export interface UserLoginType { readonly login_type: LoginType; } +// From codersdk/users.go +export interface UserParameter { + readonly name: string; + readonly display_name: string; + readonly last_used: string; +} + // From codersdk/deployment.go export interface UserQuietHoursScheduleConfig { readonly default_schedule: string; From 71bc50db2ffc3d5dcfd0e0f36d7682458fea6f0a Mon Sep 17 00:00:00 2001 From: Ammar Bandukwala Date: Sun, 21 Jan 2024 13:54:30 -0600 Subject: [PATCH 02/21] Get coderd tests passing --- coderd/users_test.go | 29 ++++++++++++++++++++++++----- 1 file changed, 24 insertions(+), 5 deletions(-) diff --git a/coderd/users_test.go b/coderd/users_test.go index 37ea7bc74efab..12e855171a489 100644 --- a/coderd/users_test.go +++ b/coderd/users_test.go @@ -23,6 +23,7 @@ import ( "github.com/coder/coder/v2/coderd/coderdtest" "github.com/coder/coder/v2/coderd/database" "github.com/coder/coder/v2/coderd/database/dbauthz" + "github.com/coder/coder/v2/coderd/database/dbfake" "github.com/coder/coder/v2/coderd/database/dbgen" "github.com/coder/coder/v2/coderd/database/dbtime" "github.com/coder/coder/v2/coderd/rbac" @@ -1746,17 +1747,32 @@ func TestUserParameters(t *testing.T) { t.Run("FindsParameters", func(t *testing.T) { t.Parallel() - client, _, _ := coderdtest.NewWithAPI(t, &coderdtest.Options{IncludeProvisionerDaemon: true}) + client, _, api := coderdtest.NewWithAPI(t, &coderdtest.Options{IncludeProvisionerDaemon: true}) u1 := coderdtest.CreateFirstUser(t, client) ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) defer cancel() - version := coderdtest.CreateTemplateVersion(t, client, u1.OrganizationID, nil) - coderdtest.AwaitTemplateVersionJobCompleted(t, client, version.ID) - template := coderdtest.CreateTemplate(t, client, u1.OrganizationID, version.ID) - coderdtest.CreateWorkspace(t, client, u1.OrganizationID, template.ID) + db := api.Database + + version := dbfake.TemplateVersion(t, db).Seed(database.TemplateVersion{ + CreatedBy: u1.UserID, + OrganizationID: u1.OrganizationID, + }).Params(database.TemplateVersionParameter{ + Name: "param", + Required: true, + }).Do() + + coderdtest.CreateWorkspace(t, client, u1.OrganizationID, version.Template.ID, + func(cwr *codersdk.CreateWorkspaceRequest) { + cwr.RichParameterValues = []codersdk.WorkspaceBuildParameter{ + { + Name: "param", + Value: "foo", + }, + } + }) params, err := client.UserParameters( ctx, @@ -1765,6 +1781,9 @@ func TestUserParameters(t *testing.T) { require.NoError(t, err) require.Equal(t, 1, len(params)) + + require.Equal(t, "param", params[0].Name) + require.Equal(t, "foo", params[0].Value) }) } From 73594f9df8b87f7239704802d9931ac7a492eff3 Mon Sep 17 00:00:00 2001 From: Ammar Bandukwala Date: Sun, 21 Jan 2024 14:39:23 -0600 Subject: [PATCH 03/21] FE works! --- coderd/apidoc/docs.go | 6 +-- coderd/apidoc/swagger.json | 6 +-- coderd/database/queries.sql.go | 1 + codersdk/users.go | 2 +- docs/api/schemas.md | 14 +++---- docs/api/users.md | 16 ++++---- site/src/api/api.ts | 7 ++++ site/src/api/queries/users.ts | 7 ++++ site/src/api/typesGenerated.ts | 2 +- .../RichParameterInput/RichParameterInput.tsx | 8 ++++ .../TemplateParameters/TemplateParameters.tsx | 11 +++++- .../CreateWorkspacePage.tsx | 37 ++++++++++++++++--- .../CreateWorkspacePageView.tsx | 20 +++++++++- 13 files changed, 106 insertions(+), 31 deletions(-) diff --git a/coderd/apidoc/docs.go b/coderd/apidoc/docs.go index 1b5e5f29567b5..e6b1eda0de266 100644 --- a/coderd/apidoc/docs.go +++ b/coderd/apidoc/docs.go @@ -11819,14 +11819,14 @@ const docTemplate = `{ "codersdk.UserParameter": { "type": "object", "properties": { - "display_name": { - "type": "string" - }, "last_used": { "type": "string" }, "name": { "type": "string" + }, + "value": { + "type": "string" } } }, diff --git a/coderd/apidoc/swagger.json b/coderd/apidoc/swagger.json index 493fe3587888f..bce886b517e12 100644 --- a/coderd/apidoc/swagger.json +++ b/coderd/apidoc/swagger.json @@ -10712,14 +10712,14 @@ "codersdk.UserParameter": { "type": "object", "properties": { - "display_name": { - "type": "string" - }, "last_used": { "type": "string" }, "name": { "type": "string" + }, + "value": { + "type": "string" } } }, diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index e77c6f9d8ec22..9df29f29db6d1 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -9893,6 +9893,7 @@ FROM ( ) sub WHERE sub.rn = 1 +LIMIT 100 ` type GetUserWorkspaceBuildParametersRow struct { diff --git a/codersdk/users.go b/codersdk/users.go index 87a4d972a9555..057de3a8e7cbd 100644 --- a/codersdk/users.go +++ b/codersdk/users.go @@ -223,7 +223,7 @@ type OIDCAuthMethod struct { type UserParameter struct { Name string `json:"name"` - Value string `json:"display_name"` + Value string `json:"value"` LastUsedAt time.Time `json:"last_used"` } diff --git a/docs/api/schemas.md b/docs/api/schemas.md index 151ea7268ceb8..f4f3be91bfaf6 100644 --- a/docs/api/schemas.md +++ b/docs/api/schemas.md @@ -5788,19 +5788,19 @@ If the schedule is empty, the user will be updated to use the default schedule.| ```json { - "display_name": "string", "last_used": "string", - "name": "string" + "name": "string", + "value": "string" } ``` ### Properties -| Name | Type | Required | Restrictions | Description | -| -------------- | ------ | -------- | ------------ | ----------- | -| `display_name` | string | false | | | -| `last_used` | string | false | | | -| `name` | string | false | | | +| Name | Type | Required | Restrictions | Description | +| ----------- | ------ | -------- | ------------ | ----------- | +| `last_used` | string | false | | | +| `name` | string | false | | | +| `value` | string | false | | | ## codersdk.UserQuietHoursScheduleConfig diff --git a/docs/api/users.md b/docs/api/users.md index 295778a9a8d43..6e20c46251231 100644 --- a/docs/api/users.md +++ b/docs/api/users.md @@ -1035,9 +1035,9 @@ curl -X GET http://coder-server:8080/api/v2/users/{user}/parameters \ ```json [ { - "display_name": "string", "last_used": "string", - "name": "string" + "name": "string", + "value": "string" } ] ``` @@ -1052,12 +1052,12 @@ curl -X GET http://coder-server:8080/api/v2/users/{user}/parameters \ Status Code **200** -| Name | Type | Required | Restrictions | Description | -| ---------------- | ------ | -------- | ------------ | ----------- | -| `[array item]` | array | false | | | -| `» display_name` | string | false | | | -| `» last_used` | string | false | | | -| `» name` | string | false | | | +| Name | Type | Required | Restrictions | Description | +| -------------- | ------ | -------- | ------------ | ----------- | +| `[array item]` | array | false | | | +| `» last_used` | string | false | | | +| `» name` | string | false | | | +| `» value` | string | false | | | To perform this operation, you must be authenticated. [Learn more](authentication.md). diff --git a/site/src/api/api.ts b/site/src/api/api.ts index 6814ad1b624a0..f99d9c18855a3 100644 --- a/site/src/api/api.ts +++ b/site/src/api/api.ts @@ -129,6 +129,13 @@ export const getAuthenticatedUser = async () => { return response.data; }; +export const getUserParameters = async () => { + const response = await axios.get( + "/api/v2/users/me/parameters", + ); + return response.data; +}; + export const getAuthMethods = async (): Promise => { const response = await axios.get( "/api/v2/users/authmethods", diff --git a/site/src/api/queries/users.ts b/site/src/api/queries/users.ts index 0249315071b13..a8a88f0e88d33 100644 --- a/site/src/api/queries/users.ts +++ b/site/src/api/queries/users.ts @@ -134,6 +134,13 @@ export const me = (): UseQueryOptions & { }; }; +export const userParameters = () => { + return { + queryKey: ["userParameters"], + queryFn: API.getUserParameters, + }; +}; + export const hasFirstUser = () => { return { queryKey: ["hasFirstUser"], diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index 9339d56d49f0f..3d16d8f080adf 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -1420,7 +1420,7 @@ export interface UserLoginType { // From codersdk/users.go export interface UserParameter { readonly name: string; - readonly display_name: string; + readonly value: string; readonly last_used: string; } diff --git a/site/src/components/RichParameterInput/RichParameterInput.tsx b/site/src/components/RichParameterInput/RichParameterInput.tsx index 331c67864ad65..aba1cf09029f0 100644 --- a/site/src/components/RichParameterInput/RichParameterInput.tsx +++ b/site/src/components/RichParameterInput/RichParameterInput.tsx @@ -137,6 +137,8 @@ export type RichParameterInputProps = Omit< "size" | "onChange" > & { parameter: TemplateVersionParameter; + // defaultReason is commentary on how the default value was determined. + defaultReason?: JSX.Element; onChange: (value: string) => void; size?: Size; }; @@ -144,6 +146,7 @@ export type RichParameterInputProps = Omit< export const RichParameterInput: FC = ({ parameter, size = "medium", + defaultReason, ...fieldProps }) => { return ( @@ -156,6 +159,11 @@ export const RichParameterInput: FC = ({
+ {defaultReason && ( +
+ 🪄 Autofilled {defaultReason} +
+ )}
); diff --git a/site/src/components/TemplateParameters/TemplateParameters.tsx b/site/src/components/TemplateParameters/TemplateParameters.tsx index 9befc7c74051f..42095dec534de 100644 --- a/site/src/components/TemplateParameters/TemplateParameters.tsx +++ b/site/src/components/TemplateParameters/TemplateParameters.tsx @@ -8,6 +8,7 @@ import { ComponentProps, FC } from "react"; export type TemplateParametersSectionProps = { templateParameters: TemplateVersionParameter[]; + defaultReasons?: Record; getInputProps: ( parameter: TemplateVersionParameter, index: number, @@ -16,7 +17,12 @@ export type TemplateParametersSectionProps = { export const MutableTemplateParametersSection: FC< TemplateParametersSectionProps -> = ({ templateParameters, getInputProps, ...formSectionProps }) => { +> = ({ + templateParameters, + getInputProps, + defaultReasons, + ...formSectionProps +}) => { const hasMutableParameters = templateParameters.filter((p) => p.mutable).length > 0; @@ -36,6 +42,9 @@ export const MutableTemplateParametersSection: FC< {...getInputProps(parameter, index)} key={parameter.name} parameter={parameter} + defaultReason={ + defaultReasons && defaultReasons[parameter.name] + } /> ), )} diff --git a/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.tsx b/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.tsx index 9a04103975d3d..56b1bed8f974f 100644 --- a/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.tsx +++ b/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.tsx @@ -1,5 +1,6 @@ import { TemplateVersionParameter, + UserParameter, Workspace, WorkspaceBuildParameter, } from "api/typesGenerated"; @@ -9,7 +10,10 @@ import { type FC, useCallback, useState, useEffect } from "react"; import { Helmet } from "react-helmet-async"; import { useNavigate, useParams, useSearchParams } from "react-router-dom"; import { pageTitle } from "utils/page"; -import { CreateWorkspacePageView } from "./CreateWorkspacePageView"; +import { + CreateWorkspacePageView, + DefaultBuildParameter, +} from "./CreateWorkspacePageView"; import { Loader } from "components/Loader/Loader"; import { ErrorAlert } from "components/Alert/ErrorAlert"; import { @@ -29,6 +33,7 @@ import { checkAuthorization } from "api/queries/authCheck"; import { CreateWSPermissions, createWorkspaceChecks } from "./permissions"; import { paramsUsedToCreateWorkspace } from "utils/workspace"; import { useEffectEvent } from "hooks/hookPolyfills"; +import { userParameters } from "api/queries/users"; export const createWorkspaceModes = ["form", "auto", "duplicate"] as const; export type CreateWorkspaceMode = (typeof createWorkspaceModes)[number]; @@ -41,7 +46,6 @@ const CreateWorkspacePage: FC = () => { const me = useMe(); const navigate = useNavigate(); const [searchParams, setSearchParams] = useSearchParams(); - const defaultBuildParameters = getDefaultBuildParameters(searchParams); const mode = getWorkspaceMode(searchParams); const customVersionId = searchParams.get("version") ?? undefined; const defaultName = getDefaultName(mode, searchParams); @@ -53,6 +57,8 @@ const CreateWorkspacePage: FC = () => { const createWorkspaceMutation = useMutation(createWorkspace(queryClient)); const templateQuery = useQuery(templateByName(organizationId, templateName)); + const userParametersQuery = useQuery(userParameters()); + const permissionsQuery = useQuery( checkAuthorization({ checks: createWorkspaceChecks(organizationId), @@ -89,6 +95,11 @@ const CreateWorkspacePage: FC = () => { [navigate], ); + const defaultBuildParameters = getDefaultBuildParameters( + searchParams, + userParametersQuery.data ? userParametersQuery.data : [], + ); + const automateWorkspaceCreation = useEffectEvent(async () => { try { const newWorkspace = await autoCreateWorkspaceMutation.mutateAsync({ @@ -210,15 +221,31 @@ const useExternalAuth = (versionId: string | undefined) => { const getDefaultBuildParameters = ( urlSearchParams: URLSearchParams, -): WorkspaceBuildParameter[] => { - const buildValues: WorkspaceBuildParameter[] = []; + userParameters: UserParameter[], +): DefaultBuildParameter[] => { + const userParamMap = userParameters.reduce((acc, param) => { + acc.set(param.name, param); + return acc; + }, new Map()); + + const buildValues: DefaultBuildParameter[] = []; Array.from(urlSearchParams.keys()) .filter((key) => key.startsWith("param.")) .forEach((key) => { const name = key.replace("param.", ""); const value = urlSearchParams.get(key) ?? ""; - buildValues.push({ name, value }); + // URL should take precedence over user parameters + userParamMap.delete(name); + buildValues.push({ name, value, reason: <>supplied by URL }); + }); + + userParamMap.forEach((param) => { + buildValues.push({ + name: param.name, + value: param.value, + reason: <>recently used value, }); + }); return buildValues; }; diff --git a/site/src/pages/CreateWorkspacePage/CreateWorkspacePageView.tsx b/site/src/pages/CreateWorkspacePage/CreateWorkspacePageView.tsx index 2c18de035b3dd..8386c0d7c4a62 100644 --- a/site/src/pages/CreateWorkspacePage/CreateWorkspacePageView.tsx +++ b/site/src/pages/CreateWorkspacePage/CreateWorkspacePageView.tsx @@ -4,7 +4,7 @@ import TextField from "@mui/material/TextField"; import type * as TypesGen from "api/typesGenerated"; import { UserAutocomplete } from "components/UserAutocomplete/UserAutocomplete"; import { FormikContextType, useFormik } from "formik"; -import { type FC, useEffect, useState } from "react"; +import { type FC, useEffect, useState, useMemo } from "react"; import { getFormHelpers, nameValidator, @@ -43,6 +43,10 @@ export const Language = { "Duplicating a workspace only copies its parameters. No state from the old workspace is copied over.", } as const; +export type DefaultBuildParameter = { + reason: JSX.Element; +} & TypesGen.WorkspaceBuildParameter; + export interface CreateWorkspacePageViewProps { mode: CreateWorkspaceMode; error: unknown; @@ -55,7 +59,7 @@ export interface CreateWorkspacePageViewProps { externalAuthPollingState: ExternalAuthPollingState; startPollingExternalAuth: () => void; parameters: TypesGen.TemplateVersionParameter[]; - defaultBuildParameters: TypesGen.WorkspaceBuildParameter[]; + defaultBuildParameters: DefaultBuildParameter[]; permissions: CreateWSPermissions; creatingWorkspace: boolean; onCancel: () => void; @@ -125,6 +129,16 @@ export const CreateWorkspacePageView: FC = ({ error, ); + const defaultReasons = useMemo(() => { + return defaultBuildParameters.reduce( + (acc, param) => { + acc[param.name] = param.reason; + return acc; + }, + {} as Record, + ); + }, [defaultBuildParameters]); + return ( @@ -211,6 +225,7 @@ export const CreateWorkspacePageView: FC = ({ {parameters && ( <> { return { @@ -231,6 +246,7 @@ export const CreateWorkspacePageView: FC = ({ }} /> Date: Sun, 21 Jan 2024 14:50:58 -0600 Subject: [PATCH 04/21] Fix lint and tests --- coderd/database/dbmem/dbmem.go | 2 +- .../components/RichParameterInput/RichParameterInput.tsx | 2 +- .../CreateWorkspacePage/CreateWorkspacePage.test.tsx | 8 ++++++-- .../src/pages/CreateWorkspacePage/CreateWorkspacePage.tsx | 7 +++++-- .../CreateWorkspacePageView.stories.tsx | 7 +++++++ 5 files changed, 20 insertions(+), 6 deletions(-) diff --git a/coderd/database/dbmem/dbmem.go b/coderd/database/dbmem/dbmem.go index 0f8c64b634445..06aef49d305d7 100644 --- a/coderd/database/dbmem/dbmem.go +++ b/coderd/database/dbmem/dbmem.go @@ -3758,7 +3758,7 @@ func (q *FakeQuerier) GetUserLinksByUserID(_ context.Context, userID uuid.UUID) return uls, nil } -func (q *FakeQuerier) GetUserWorkspaceBuildParameters(ctx context.Context, ownerID uuid.UUID) ([]database.GetUserWorkspaceBuildParametersRow, error) { +func (q *FakeQuerier) GetUserWorkspaceBuildParameters(_ context.Context, ownerID uuid.UUID) ([]database.GetUserWorkspaceBuildParametersRow, error) { q.mutex.RLock() defer q.mutex.RUnlock() diff --git a/site/src/components/RichParameterInput/RichParameterInput.tsx b/site/src/components/RichParameterInput/RichParameterInput.tsx index aba1cf09029f0..499611a483cc4 100644 --- a/site/src/components/RichParameterInput/RichParameterInput.tsx +++ b/site/src/components/RichParameterInput/RichParameterInput.tsx @@ -161,7 +161,7 @@ export const RichParameterInput: FC = ({ {defaultReason && (
- 🪄 Autofilled {defaultReason} + 🪄 Autofilled: {defaultReason}
)} diff --git a/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.test.tsx b/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.test.tsx index f4ffaecf5285c..fb79556774668 100644 --- a/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.test.tsx +++ b/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.test.tsx @@ -241,7 +241,9 @@ describe("CreateWorkspacePage", () => { "me", expect.objectContaining({ template_id: MockTemplate.id, - rich_parameter_values: [{ name: param, value: paramValue }], + rich_parameter_values: [ + { name: param, value: paramValue, reason: <>supplied by URL }, + ], }), ); }); @@ -266,7 +268,9 @@ describe("CreateWorkspacePage", () => { "me", expect.objectContaining({ template_version_id: MockTemplate.active_version_id, - rich_parameter_values: [{ name: param, value: paramValue }], + rich_parameter_values: [ + { name: param, value: paramValue, reason: <>supplied by URL }, + ], }), ); }); diff --git a/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.tsx b/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.tsx index 56b1bed8f974f..c21c9e7e1217a 100644 --- a/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.tsx +++ b/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.tsx @@ -2,7 +2,6 @@ import { TemplateVersionParameter, UserParameter, Workspace, - WorkspaceBuildParameter, } from "api/typesGenerated"; import { useMe } from "hooks/useMe"; import { useOrganizationId } from "hooks/useOrganizationId"; @@ -243,7 +242,11 @@ const getDefaultBuildParameters = ( buildValues.push({ name: param.name, value: param.value, - reason: <>recently used value, + reason: ( + <> + recently used {param.name} value. + + ), }); }); return buildValues; diff --git a/site/src/pages/CreateWorkspacePage/CreateWorkspacePageView.stories.tsx b/site/src/pages/CreateWorkspacePage/CreateWorkspacePageView.stories.tsx index 002b6c678a857..7d06fe785143a 100644 --- a/site/src/pages/CreateWorkspacePage/CreateWorkspacePageView.stories.tsx +++ b/site/src/pages/CreateWorkspacePage/CreateWorkspacePageView.stories.tsx @@ -86,6 +86,13 @@ export const Parameters: Story = { ephemeral: false, }, ], + defaultBuildParameters: [ + { + name: "first_parameter", + value: "It works!", + reason: <>recently used., + }, + ], }, }; From d11fb586c4a26dfe7a7bfdaa69e1e1433d1a34ca Mon Sep 17 00:00:00 2001 From: Ammar Bandukwala Date: Sun, 21 Jan 2024 14:56:29 -0600 Subject: [PATCH 05/21] Fix FE tests --- .../pages/CreateWorkspacePage/CreateWorkspacePage.test.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.test.tsx b/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.test.tsx index fb79556774668..fa751f73a6ba9 100644 --- a/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.test.tsx +++ b/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.test.tsx @@ -242,7 +242,7 @@ describe("CreateWorkspacePage", () => { expect.objectContaining({ template_id: MockTemplate.id, rich_parameter_values: [ - { name: param, value: paramValue, reason: <>supplied by URL }, + expect.objectContaining({ name: param, value: paramValue }), ], }), ); @@ -269,7 +269,7 @@ describe("CreateWorkspacePage", () => { expect.objectContaining({ template_version_id: MockTemplate.active_version_id, rich_parameter_values: [ - { name: param, value: paramValue, reason: <>supplied by URL }, + expect.objectContaining({ name: param, value: paramValue }), ], }), ); From 1c70e36930a0b6785ec34cdf96a2ec9107be3956 Mon Sep 17 00:00:00 2001 From: Ammar Bandukwala Date: Sun, 21 Jan 2024 15:02:29 -0600 Subject: [PATCH 06/21] Fix swagger ID --- coderd/users.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/coderd/users.go b/coderd/users.go index e7beac21a0c1b..bf73ff1a3f005 100644 --- a/coderd/users.go +++ b/coderd/users.go @@ -572,7 +572,7 @@ func (api *API) userByName(rw http.ResponseWriter, r *http.Request) { // Returns recent build parameters for the signed-in user. // // @Summary Get recent build parameters for user. -// @ID get-user-params +// @ID get-recent-build-parameters-for-user // @Security CoderSessionToken // @Produce json // @Tags Users From 2b0783f71670b0cb0453f7d72eafce22fea9a198 Mon Sep 17 00:00:00 2001 From: Ammar Bandukwala Date: Sun, 21 Jan 2024 15:08:51 -0600 Subject: [PATCH 07/21] make gen & docs --- coderd/apidoc/docs.go | 2 +- coderd/apidoc/swagger.json | 2 +- docs/templates/parameters.md | 7 +++++++ 3 files changed, 9 insertions(+), 2 deletions(-) diff --git a/coderd/apidoc/docs.go b/coderd/apidoc/docs.go index e6b1eda0de266..195d2ba527127 100644 --- a/coderd/apidoc/docs.go +++ b/coderd/apidoc/docs.go @@ -4569,7 +4569,7 @@ const docTemplate = `{ "Users" ], "summary": "Get recent build parameters for user.", - "operationId": "get-user-params", + "operationId": "get-recent-build-parameters-for-user", "parameters": [ { "type": "string", diff --git a/coderd/apidoc/swagger.json b/coderd/apidoc/swagger.json index bce886b517e12..f6adfaee89e53 100644 --- a/coderd/apidoc/swagger.json +++ b/coderd/apidoc/swagger.json @@ -4017,7 +4017,7 @@ "produces": ["application/json"], "tags": ["Users"], "summary": "Get recent build parameters for user.", - "operationId": "get-user-params", + "operationId": "get-recent-build-parameters-for-user", "parameters": [ { "type": "string", diff --git a/docs/templates/parameters.md b/docs/templates/parameters.md index a417157f2563a..74aca5b0e86e4 100644 --- a/docs/templates/parameters.md +++ b/docs/templates/parameters.md @@ -281,3 +281,10 @@ variable "CLOUD_API_KEY" { } ``` + +## Create Autofill + +When the template doesn't specify default values, Coder may still autofill parameters. + +1. Coder will look for URL query parameters with form `param.=`. This feature enables platform teams to create pre-filled template creation links. +2. Coder will populate recently used parameter key-value pairs for the user. This feature helps reduce repetition on common parameters such as `dotfiles_url` or `region`. From 1a28ef23942fa6a1d8d7e5e578094d1044c7a0e7 Mon Sep 17 00:00:00 2001 From: Ammar Bandukwala Date: Sun, 21 Jan 2024 15:09:25 -0600 Subject: [PATCH 08/21] make fmt --- docs/templates/parameters.md | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/docs/templates/parameters.md b/docs/templates/parameters.md index 74aca5b0e86e4..db15b50235b03 100644 --- a/docs/templates/parameters.md +++ b/docs/templates/parameters.md @@ -284,7 +284,12 @@ variable "CLOUD_API_KEY" { ## Create Autofill -When the template doesn't specify default values, Coder may still autofill parameters. - -1. Coder will look for URL query parameters with form `param.=`. This feature enables platform teams to create pre-filled template creation links. -2. Coder will populate recently used parameter key-value pairs for the user. This feature helps reduce repetition on common parameters such as `dotfiles_url` or `region`. +When the template doesn't specify default values, Coder may still autofill +parameters. + +1. Coder will look for URL query parameters with form `param.=`. + This feature enables platform teams to create pre-filled template creation + links. +2. Coder will populate recently used parameter key-value pairs for the user. + This feature helps reduce repetition on common parameters such as + `dotfiles_url` or `region`. From 15692236da7ea635500c234e6f20d02598856f5c Mon Sep 17 00:00:00 2001 From: Ammar Bandukwala Date: Sun, 21 Jan 2024 15:20:24 -0600 Subject: [PATCH 09/21] Slight docs edit --- docs/manifest.json | 10 +++++----- docs/templates/parameters.md | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/docs/manifest.json b/docs/manifest.json index e863e27572f10..7185a0b8090ff 100644 --- a/docs/manifest.json +++ b/docs/manifest.json @@ -182,14 +182,14 @@ "title": "Resource metadata", "description": "Show information in the workspace about template resources", "path": "./templates/resource-metadata.md" - }, - { - "title": "Parameters", - "description": "Prompt the user for additional information about a workspace", - "path": "./templates/parameters.md" } ] }, + { + "title": "Parameters", + "description": "Prompt the user for additional information about a workspace", + "path": "./templates/parameters.md" + }, { "title": "Administering templates", "description": "Configuration settings for template admins", diff --git a/docs/templates/parameters.md b/docs/templates/parameters.md index db15b50235b03..3dfc181cb4e9c 100644 --- a/docs/templates/parameters.md +++ b/docs/templates/parameters.md @@ -291,5 +291,5 @@ parameters. This feature enables platform teams to create pre-filled template creation links. 2. Coder will populate recently used parameter key-value pairs for the user. - This feature helps reduce repetition on common parameters such as + This feature helps reduce repetition when filling common parameters such as `dotfiles_url` or `region`. From 2d544d846db080562300e8c987c4b578c40faf89 Mon Sep 17 00:00:00 2001 From: Ammar Bandukwala Date: Sun, 21 Jan 2024 15:24:21 -0600 Subject: [PATCH 10/21] Fix go test --- coderd/database/dbauthz/dbauthz_test.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/coderd/database/dbauthz/dbauthz_test.go b/coderd/database/dbauthz/dbauthz_test.go index d9444278722e7..6126d7f134908 100644 --- a/coderd/database/dbauthz/dbauthz_test.go +++ b/coderd/database/dbauthz/dbauthz_test.go @@ -1052,6 +1052,12 @@ func (s *MethodTestSuite) TestUser() { UpdatedAt: u.UpdatedAt, }).Asserts(u.UserDataRBACObject(), rbac.ActionUpdate).Returns(u) })) + s.Run("GetUserWorkspaceBuildParameters", s.Subtest(func(db database.Store, check *expects) { + u := dbgen.User(s.T(), db, database.User{}) + check.Args(u.ID).Asserts(u.UserDataRBACObject(), rbac.ActionUpdate).Returns( + []database.GetUserWorkspaceBuildParametersRow{}, + ) + })) s.Run("UpdateUserAppearanceSettings", s.Subtest(func(db database.Store, check *expects) { u := dbgen.User(s.T(), db, database.User{}) check.Args(database.UpdateUserAppearanceSettingsParams{ From 93deee5e4417283059f8d975890080c13806d0d4 Mon Sep 17 00:00:00 2001 From: Ammar Bandukwala Date: Sun, 21 Jan 2024 15:58:47 -0600 Subject: [PATCH 11/21] site: use distinct coderd pprof port --- site/e2e/constants.ts | 5 ++++- site/e2e/helpers.ts | 4 ++-- site/e2e/playwright.config.ts | 3 ++- site/e2e/reporter.ts | 2 +- 4 files changed, 9 insertions(+), 5 deletions(-) diff --git a/site/e2e/constants.ts b/site/e2e/constants.ts index f75af482c7917..c7c6a474dc8dc 100644 --- a/site/e2e/constants.ts +++ b/site/e2e/constants.ts @@ -1,7 +1,10 @@ // Default port from the server export const defaultPort = 3000; export const prometheusPort = 2114; -export const pprofPort = 6061; + +// Use alternate ports in case we're running in a Coder Workspace. +export const agentPProfPort = 6061; +export const coderdPProfPort = 6062; // Credentials for the first user export const username = "admin"; diff --git a/site/e2e/helpers.ts b/site/e2e/helpers.ts index 77960b32234d0..ff3e7206df9e9 100644 --- a/site/e2e/helpers.ts +++ b/site/e2e/helpers.ts @@ -15,7 +15,7 @@ import { Resource, RichParameter, } from "./provisionerGenerated"; -import { prometheusPort, pprofPort } from "./constants"; +import { prometheusPort, agentPProfPort } from "./constants"; import { port } from "./playwright.config"; import * as ssh from "ssh2"; import { Duplex } from "stream"; @@ -306,7 +306,7 @@ export const startAgentWithCommand = async ( ...process.env, CODER_AGENT_URL: "http://localhost:" + port, CODER_AGENT_TOKEN: token, - CODER_AGENT_PPROF_ADDRESS: "127.0.0.1:" + pprofPort, + CODER_AGENT_PPROF_ADDRESS: "127.0.0.1:" + agentPProfPort, CODER_AGENT_PROMETHEUS_ADDRESS: "127.0.0.1:" + prometheusPort, }, }); diff --git a/site/e2e/playwright.config.ts b/site/e2e/playwright.config.ts index 78ebf4cc039f4..dd70350e7f280 100644 --- a/site/e2e/playwright.config.ts +++ b/site/e2e/playwright.config.ts @@ -1,6 +1,6 @@ import { defineConfig } from "@playwright/test"; import path from "path"; -import { defaultPort, gitAuth } from "./constants"; +import { defaultPort, coderdPProfPort, gitAuth } from "./constants"; export const port = process.env.CODER_E2E_PORT ? Number(process.env.CODER_E2E_PORT) @@ -103,6 +103,7 @@ export default defineConfig({ gitAuth.webPort, gitAuth.validatePath, ), + CODER_PPROF_ADDRESS: "127.0.0.1:" + coderdPProfPort, }, reuseExistingServer: false, }, diff --git a/site/e2e/reporter.ts b/site/e2e/reporter.ts index 6be742b02bb55..48a1c27ac14a1 100644 --- a/site/e2e/reporter.ts +++ b/site/e2e/reporter.ts @@ -119,7 +119,7 @@ const filteredServerLogLines = (chunk: string): string[] => const exportDebugPprof = async (outputFile: string) => { const response = await axios.get( - "http://127.0.0.1:6060/debug/pprof/goroutine?debug=1", + "http://127.0.0.1:6062/debug/pprof/goroutine?debug=1", ); if (response.status !== 200) { throw new Error(`Error: Received status code ${response.status}`); From 51cc1384b727ed3f44f46a3fc7b2581ce0ace0fc Mon Sep 17 00:00:00 2001 From: Ammar Bandukwala Date: Sun, 21 Jan 2024 16:22:46 -0600 Subject: [PATCH 12/21] Minor fixes --- coderd/database/dbauthz/dbauthz.go | 2 ++ codersdk/users.go | 2 +- site/src/api/typesGenerated.ts | 8 +++++--- 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go index f52ab83377567..626362a8f1f00 100644 --- a/coderd/database/dbauthz/dbauthz.go +++ b/coderd/database/dbauthz/dbauthz.go @@ -1759,6 +1759,8 @@ func (q *querier) GetUserWorkspaceBuildParameters(ctx context.Context, ownerID u if err != nil { return nil, err } + // The ability to update the user implies either the user themselves or someone + // with complete admin access to the user account. if err := q.authorizeContext(ctx, rbac.ActionUpdate, u.UserDataRBACObject()); err != nil { return nil, err } diff --git a/codersdk/users.go b/codersdk/users.go index 057de3a8e7cbd..2fb077ca01822 100644 --- a/codersdk/users.go +++ b/codersdk/users.go @@ -224,7 +224,7 @@ type OIDCAuthMethod struct { type UserParameter struct { Name string `json:"name"` Value string `json:"value"` - LastUsedAt time.Time `json:"last_used"` + LastUsedAt time.Time `json:"last_used_at" format:"date-time"` } // UserParameters returns all recently used parameters for the given user. diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index 3d16d8f080adf..d5b652beb35fa 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -1421,7 +1421,7 @@ export interface UserLoginType { export interface UserParameter { readonly name: string; readonly value: string; - readonly last_used: string; + readonly last_used_at: string; } // From codersdk/deployment.go @@ -2122,8 +2122,10 @@ export const WorkspaceAgentLifecycles: WorkspaceAgentLifecycle[] = [ // From codersdk/workspaceagents.go export type WorkspaceAgentStartupScriptBehavior = "blocking" | "non-blocking"; -export const WorkspaceAgentStartupScriptBehaviors: WorkspaceAgentStartupScriptBehavior[] = - ["blocking", "non-blocking"]; +export const WorkspaceAgentStartupScriptBehaviors: WorkspaceAgentStartupScriptBehavior[] = [ + "blocking", + "non-blocking", +]; // From codersdk/workspaceagents.go export type WorkspaceAgentStatus = From e61491ba5d1c7ec1a128a637304493345e85b9a1 Mon Sep 17 00:00:00 2001 From: Ammar Bandukwala Date: Sun, 21 Jan 2024 17:09:10 -0600 Subject: [PATCH 13/21] Fix one of the e2e tests --- site/e2e/parameters.ts | 6 +++ site/e2e/tests/createWorkspace.spec.ts | 13 ++++-- .../RichParameterInput/RichParameterInput.tsx | 16 ++++--- .../TemplateParameters/TemplateParameters.tsx | 5 ++- .../CreateWorkspacePage.tsx | 26 +++++------ .../CreateWorkspacePageView.stories.tsx | 6 +-- .../CreateWorkspacePageView.tsx | 26 +++++------ site/src/utils/richParameters.test.ts | 2 +- site/src/utils/richParameters.ts | 45 +++++++++++++------ 9 files changed, 87 insertions(+), 58 deletions(-) diff --git a/site/e2e/parameters.ts b/site/e2e/parameters.ts index c1477fad4c59e..715c96a784b6d 100644 --- a/site/e2e/parameters.ts +++ b/site/e2e/parameters.ts @@ -128,6 +128,12 @@ export const seventhParameter: RichParameter = { order: 1, }; +// randParamName returns a new parameter with a random name. +export const randParamName = (p: RichParameter): RichParameter => { + p.name += Math.random().toString(36).substring(7); + return p; +}; + // Build options export const firstBuildOption: RichParameter = { diff --git a/site/e2e/tests/createWorkspace.spec.ts b/site/e2e/tests/createWorkspace.spec.ts index d7cec29a90aa3..073686ae42621 100644 --- a/site/e2e/tests/createWorkspace.spec.ts +++ b/site/e2e/tests/createWorkspace.spec.ts @@ -1,3 +1,4 @@ +import { randParamName } from "./../parameters"; import { test, expect } from "@playwright/test"; import { createTemplate, @@ -101,10 +102,16 @@ test("create workspace with default and required parameters", async ({ }); test("create workspace and overwrite default parameters", async ({ page }) => { - const richParameters: RichParameter[] = [secondParameter, fourthParameter]; + // We use randParamName to prevent the new values from corrupting user_history + // and thus affecting other tests. + const richParameters: RichParameter[] = [ + randParamName(secondParameter), + randParamName(fourthParameter), + ]; + const buildParameters = [ - { name: secondParameter.name, value: "AAAAA" }, - { name: fourthParameter.name, value: "false" }, + { name: richParameters[0].name, value: "AAAAA" }, + { name: richParameters[1].name, value: "false" }, ]; const template = await createTemplate( page, diff --git a/site/src/components/RichParameterInput/RichParameterInput.tsx b/site/src/components/RichParameterInput/RichParameterInput.tsx index 499611a483cc4..f4b9ee62fe244 100644 --- a/site/src/components/RichParameterInput/RichParameterInput.tsx +++ b/site/src/components/RichParameterInput/RichParameterInput.tsx @@ -10,6 +10,7 @@ import { MemoizedMarkdown } from "components/Markdown/Markdown"; import { Stack } from "components/Stack/Stack"; import { MultiTextField } from "./MultiTextField"; import { ExternalImage } from "components/ExternalImage/ExternalImage"; +import { AutofillSource } from "utils/richParameters"; const isBoolean = (parameter: TemplateVersionParameter) => { return parameter.type === "bool"; @@ -137,8 +138,7 @@ export type RichParameterInputProps = Omit< "size" | "onChange" > & { parameter: TemplateVersionParameter; - // defaultReason is commentary on how the default value was determined. - defaultReason?: JSX.Element; + autofillSource?: AutofillSource; onChange: (value: string) => void; size?: Size; }; @@ -146,7 +146,7 @@ export type RichParameterInputProps = Omit< export const RichParameterInput: FC = ({ parameter, size = "medium", - defaultReason, + autofillSource, ...fieldProps }) => { return ( @@ -159,9 +159,15 @@ export const RichParameterInput: FC = ({
- {defaultReason && ( + {autofillSource && (
- 🪄 Autofilled: {defaultReason} + 🪄 Autofilled:{" "} + { + { + ["url"]: "supplied by URL.", + ["user_history"]: "recently used value.", + }[autofillSource] + }
)}
diff --git a/site/src/components/TemplateParameters/TemplateParameters.tsx b/site/src/components/TemplateParameters/TemplateParameters.tsx index 42095dec534de..37b04b5349c74 100644 --- a/site/src/components/TemplateParameters/TemplateParameters.tsx +++ b/site/src/components/TemplateParameters/TemplateParameters.tsx @@ -5,10 +5,11 @@ import { RichParameterInputProps, } from "components/RichParameterInput/RichParameterInput"; import { ComponentProps, FC } from "react"; +import { AutofillSource } from "utils/richParameters"; export type TemplateParametersSectionProps = { templateParameters: TemplateVersionParameter[]; - defaultReasons?: Record; + defaultReasons?: Record; getInputProps: ( parameter: TemplateVersionParameter, index: number, @@ -42,7 +43,7 @@ export const MutableTemplateParametersSection: FC< {...getInputProps(parameter, index)} key={parameter.name} parameter={parameter} - defaultReason={ + autofillSource={ defaultReasons && defaultReasons[parameter.name] } /> diff --git a/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.tsx b/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.tsx index c21c9e7e1217a..8ddea1fad408c 100644 --- a/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.tsx +++ b/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.tsx @@ -9,10 +9,7 @@ import { type FC, useCallback, useState, useEffect } from "react"; import { Helmet } from "react-helmet-async"; import { useNavigate, useParams, useSearchParams } from "react-router-dom"; import { pageTitle } from "utils/page"; -import { - CreateWorkspacePageView, - DefaultBuildParameter, -} from "./CreateWorkspacePageView"; +import { CreateWorkspacePageView } from "./CreateWorkspacePageView"; import { Loader } from "components/Loader/Loader"; import { ErrorAlert } from "components/Alert/ErrorAlert"; import { @@ -33,6 +30,7 @@ import { CreateWSPermissions, createWorkspaceChecks } from "./permissions"; import { paramsUsedToCreateWorkspace } from "utils/workspace"; import { useEffectEvent } from "hooks/hookPolyfills"; import { userParameters } from "api/queries/users"; +import { AutofillBuildParameter } from "utils/richParameters"; export const createWorkspaceModes = ["form", "auto", "duplicate"] as const; export type CreateWorkspaceMode = (typeof createWorkspaceModes)[number]; @@ -94,7 +92,7 @@ const CreateWorkspacePage: FC = () => { [navigate], ); - const defaultBuildParameters = getDefaultBuildParameters( + const autofillParameters = getAutofillParameters( searchParams, userParametersQuery.data ? userParametersQuery.data : [], ); @@ -104,7 +102,7 @@ const CreateWorkspacePage: FC = () => { const newWorkspace = await autoCreateWorkspaceMutation.mutateAsync({ templateName, organizationId, - defaultBuildParameters, + defaultBuildParameters: autofillParameters, defaultName, versionId: realizedVersionId, }); @@ -135,7 +133,7 @@ const CreateWorkspacePage: FC = () => { mode={mode} defaultName={defaultName} defaultOwner={me} - defaultBuildParameters={defaultBuildParameters} + autofillParameters={autofillParameters} error={createWorkspaceMutation.error} resetMutation={createWorkspaceMutation.reset} template={templateQuery.data!} @@ -218,16 +216,16 @@ const useExternalAuth = (versionId: string | undefined) => { }; }; -const getDefaultBuildParameters = ( +const getAutofillParameters = ( urlSearchParams: URLSearchParams, userParameters: UserParameter[], -): DefaultBuildParameter[] => { +): AutofillBuildParameter[] => { const userParamMap = userParameters.reduce((acc, param) => { acc.set(param.name, param); return acc; }, new Map()); - const buildValues: DefaultBuildParameter[] = []; + const buildValues: AutofillBuildParameter[] = []; Array.from(urlSearchParams.keys()) .filter((key) => key.startsWith("param.")) .forEach((key) => { @@ -235,18 +233,14 @@ const getDefaultBuildParameters = ( const value = urlSearchParams.get(key) ?? ""; // URL should take precedence over user parameters userParamMap.delete(name); - buildValues.push({ name, value, reason: <>supplied by URL }); + buildValues.push({ name, value, source: "url" }); }); userParamMap.forEach((param) => { buildValues.push({ name: param.name, value: param.value, - reason: ( - <> - recently used {param.name} value. - - ), + source: "user_history", }); }); return buildValues; diff --git a/site/src/pages/CreateWorkspacePage/CreateWorkspacePageView.stories.tsx b/site/src/pages/CreateWorkspacePage/CreateWorkspacePageView.stories.tsx index 7d06fe785143a..6820358bc79c0 100644 --- a/site/src/pages/CreateWorkspacePage/CreateWorkspacePageView.stories.tsx +++ b/site/src/pages/CreateWorkspacePage/CreateWorkspacePageView.stories.tsx @@ -17,7 +17,7 @@ const meta: Meta = { args: { defaultName: "", defaultOwner: MockUser, - defaultBuildParameters: [], + autofillParameters: [], template: MockTemplate, parameters: [], externalAuth: [], @@ -86,11 +86,11 @@ export const Parameters: Story = { ephemeral: false, }, ], - defaultBuildParameters: [ + autofillParameters: [ { name: "first_parameter", value: "It works!", - reason: <>recently used., + source: "user_history", }, ], }, diff --git a/site/src/pages/CreateWorkspacePage/CreateWorkspacePageView.tsx b/site/src/pages/CreateWorkspacePage/CreateWorkspacePageView.tsx index 8386c0d7c4a62..c403d64513e5d 100644 --- a/site/src/pages/CreateWorkspacePage/CreateWorkspacePageView.tsx +++ b/site/src/pages/CreateWorkspacePage/CreateWorkspacePageView.tsx @@ -20,6 +20,8 @@ import { HorizontalForm, } from "components/Form/Form"; import { + AutofillBuildParameter, + AutofillSource, getInitialRichParameterValues, useValidationSchemaForRichParameters, } from "utils/richParameters"; @@ -43,10 +45,6 @@ export const Language = { "Duplicating a workspace only copies its parameters. No state from the old workspace is copied over.", } as const; -export type DefaultBuildParameter = { - reason: JSX.Element; -} & TypesGen.WorkspaceBuildParameter; - export interface CreateWorkspacePageViewProps { mode: CreateWorkspaceMode; error: unknown; @@ -59,7 +57,7 @@ export interface CreateWorkspacePageViewProps { externalAuthPollingState: ExternalAuthPollingState; startPollingExternalAuth: () => void; parameters: TypesGen.TemplateVersionParameter[]; - defaultBuildParameters: DefaultBuildParameter[]; + autofillParameters: AutofillBuildParameter[]; permissions: CreateWSPermissions; creatingWorkspace: boolean; onCancel: () => void; @@ -81,7 +79,7 @@ export const CreateWorkspacePageView: FC = ({ externalAuthPollingState, startPollingExternalAuth, parameters, - defaultBuildParameters, + autofillParameters, permissions, creatingWorkspace, onSubmit, @@ -101,7 +99,7 @@ export const CreateWorkspacePageView: FC = ({ template_id: template.id, rich_parameter_values: getInitialRichParameterValues( parameters, - defaultBuildParameters, + autofillParameters, ), }, validationSchema: Yup.object({ @@ -129,15 +127,15 @@ export const CreateWorkspacePageView: FC = ({ error, ); - const defaultReasons = useMemo(() => { - return defaultBuildParameters.reduce( + const defaultSources = useMemo(() => { + return autofillParameters.reduce( (acc, param) => { - acc[param.name] = param.reason; + acc[param.name] = param.source; return acc; }, - {} as Record, + {} as Record, ); - }, [defaultBuildParameters]); + }, [autofillParameters]); return ( @@ -225,7 +223,7 @@ export const CreateWorkspacePageView: FC = ({ {parameters && ( <> { return { @@ -246,7 +244,7 @@ export const CreateWorkspacePageView: FC = ({ }} /> { - return templateParameters.map((parameter) => { - const existentBuildParameter = buildParameters?.find( + return templateParams.map((parameter) => { + const autofillParam = autofillParams?.find( (p) => p.name === parameter.name, ); - const shouldReturnTheDefaultValue = - !existentBuildParameter || - !isValidValue(parameter, existentBuildParameter) || - parameter.ephemeral; - if (shouldReturnTheDefaultValue) { - return { - name: parameter.name, - value: parameter.default_value, - }; + if (autofillParam !== undefined && isValidValue(parameter, autofillParam)) { + // URL takes precedence over all other sources. + if (autofillParam.source === "url") { + return { + name: parameter.name, + value: autofillParam.value, + }; + } + + // Need to decide whether user_history is more important than default value. + if (autofillParam.source === "user_history") { + return { + name: parameter.name, + value: autofillParam.value, + }; + } } - return existentBuildParameter; + + return { + name: parameter.name, + value: parameter.default_value, + }; }); }; From 1a1d0fac9f66ec6825f358a6b38e33a29f437918 Mon Sep 17 00:00:00 2001 From: Ammar Bandukwala Date: Sun, 21 Jan 2024 21:22:09 -0600 Subject: [PATCH 14/21] Fmt and fix e2e test --- site/e2e/parameters.ts | 4 ++-- site/e2e/tests/restartWorkspace.spec.ts | 19 ++++++++++----- site/e2e/tests/startWorkspace.spec.ts | 19 ++++++++++----- site/src/api/typesGenerated.ts | 6 ++--- .../RichParameterInput/RichParameterInput.tsx | 2 +- .../TemplateParameters/TemplateParameters.tsx | 4 ++-- .../CreateWorkspacePageView.tsx | 6 ++--- .../BuildParametersPopover.tsx | 14 ++++++++--- .../WorkspaceParametersForm.tsx | 7 +++--- .../WorkspaceParametersPage.tsx | 8 ++++++- site/src/utils/richParameters.ts | 23 ++++++------------- 11 files changed, 65 insertions(+), 47 deletions(-) diff --git a/site/e2e/parameters.ts b/site/e2e/parameters.ts index 715c96a784b6d..4f3cf48bdb2da 100644 --- a/site/e2e/parameters.ts +++ b/site/e2e/parameters.ts @@ -130,8 +130,8 @@ export const seventhParameter: RichParameter = { // randParamName returns a new parameter with a random name. export const randParamName = (p: RichParameter): RichParameter => { - p.name += Math.random().toString(36).substring(7); - return p; + const name = p.name + Math.random().toString(36).substring(7); + return { ...p, name: name }; }; // Build options diff --git a/site/e2e/tests/restartWorkspace.spec.ts b/site/e2e/tests/restartWorkspace.spec.ts index 3d8fb704b699d..24d40658f657f 100644 --- a/site/e2e/tests/restartWorkspace.spec.ts +++ b/site/e2e/tests/restartWorkspace.spec.ts @@ -7,14 +7,21 @@ import { verifyParameters, } from "../helpers"; -import { firstBuildOption, secondBuildOption } from "../parameters"; +import { + firstBuildOption, + randParamName, + secondBuildOption, +} from "../parameters"; import { RichParameter } from "../provisionerGenerated"; import { beforeCoderTest } from "../hooks"; test.beforeEach(async ({ page }) => await beforeCoderTest(page)); test("restart workspace with ephemeral parameters", async ({ page }) => { - const richParameters: RichParameter[] = [firstBuildOption, secondBuildOption]; + const richParameters: RichParameter[] = [ + randParamName(firstBuildOption), + randParamName(secondBuildOption), + ]; const template = await createTemplate( page, echoResponsesWithParameters(richParameters), @@ -23,14 +30,14 @@ test("restart workspace with ephemeral parameters", async ({ page }) => { // Verify that build options are default (not selected). await verifyParameters(page, workspaceName, richParameters, [ - { name: firstBuildOption.name, value: firstBuildOption.defaultValue }, - { name: secondBuildOption.name, value: secondBuildOption.defaultValue }, + { name: richParameters[0].name, value: firstBuildOption.defaultValue }, + { name: richParameters[1].name, value: secondBuildOption.defaultValue }, ]); // Now, restart the workspace with ephemeral parameters selected. const buildParameters = [ - { name: firstBuildOption.name, value: "AAAAA" }, - { name: secondBuildOption.name, value: "true" }, + { name: richParameters[0].name, value: "AAAAA" }, + { name: richParameters[1].name, value: "true" }, ]; await buildWorkspaceWithParameters( page, diff --git a/site/e2e/tests/startWorkspace.spec.ts b/site/e2e/tests/startWorkspace.spec.ts index ec22cda01d0c9..15d79d5c18f96 100644 --- a/site/e2e/tests/startWorkspace.spec.ts +++ b/site/e2e/tests/startWorkspace.spec.ts @@ -8,11 +8,18 @@ import { verifyParameters, } from "../helpers"; -import { firstBuildOption, secondBuildOption } from "../parameters"; +import { + firstBuildOption, + randParamName, + secondBuildOption, +} from "../parameters"; import { RichParameter } from "../provisionerGenerated"; test("start workspace with ephemeral parameters", async ({ page }) => { - const richParameters: RichParameter[] = [firstBuildOption, secondBuildOption]; + const richParameters: RichParameter[] = [ + randParamName(firstBuildOption), + randParamName(secondBuildOption), + ]; const template = await createTemplate( page, echoResponsesWithParameters(richParameters), @@ -21,8 +28,8 @@ test("start workspace with ephemeral parameters", async ({ page }) => { // Verify that build options are default (not selected). await verifyParameters(page, workspaceName, richParameters, [ - { name: firstBuildOption.name, value: firstBuildOption.defaultValue }, - { name: secondBuildOption.name, value: secondBuildOption.defaultValue }, + { name: richParameters[0].name, value: firstBuildOption.defaultValue }, + { name: richParameters[1].name, value: secondBuildOption.defaultValue }, ]); // Stop the workspace @@ -30,8 +37,8 @@ test("start workspace with ephemeral parameters", async ({ page }) => { // Now, start the workspace with ephemeral parameters selected. const buildParameters = [ - { name: firstBuildOption.name, value: "AAAAA" }, - { name: secondBuildOption.name, value: "true" }, + { name: richParameters[0].name, value: "AAAAA" }, + { name: richParameters[1].name, value: "true" }, ]; await buildWorkspaceWithParameters( diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index d5b652beb35fa..a0675d850bc81 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -2122,10 +2122,8 @@ export const WorkspaceAgentLifecycles: WorkspaceAgentLifecycle[] = [ // From codersdk/workspaceagents.go export type WorkspaceAgentStartupScriptBehavior = "blocking" | "non-blocking"; -export const WorkspaceAgentStartupScriptBehaviors: WorkspaceAgentStartupScriptBehavior[] = [ - "blocking", - "non-blocking", -]; +export const WorkspaceAgentStartupScriptBehaviors: WorkspaceAgentStartupScriptBehavior[] = + ["blocking", "non-blocking"]; // From codersdk/workspaceagents.go export type WorkspaceAgentStatus = diff --git a/site/src/components/RichParameterInput/RichParameterInput.tsx b/site/src/components/RichParameterInput/RichParameterInput.tsx index f4b9ee62fe244..25771a17f45c5 100644 --- a/site/src/components/RichParameterInput/RichParameterInput.tsx +++ b/site/src/components/RichParameterInput/RichParameterInput.tsx @@ -159,7 +159,7 @@ export const RichParameterInput: FC = ({
- {autofillSource && ( + {autofillSource && autofillSource !== "active_build" && (
🪄 Autofilled:{" "} { diff --git a/site/src/components/TemplateParameters/TemplateParameters.tsx b/site/src/components/TemplateParameters/TemplateParameters.tsx index 37b04b5349c74..fb29d42febca3 100644 --- a/site/src/components/TemplateParameters/TemplateParameters.tsx +++ b/site/src/components/TemplateParameters/TemplateParameters.tsx @@ -9,7 +9,7 @@ import { AutofillSource } from "utils/richParameters"; export type TemplateParametersSectionProps = { templateParameters: TemplateVersionParameter[]; - defaultReasons?: Record; + autofillSources?: Record; getInputProps: ( parameter: TemplateVersionParameter, index: number, @@ -21,7 +21,7 @@ export const MutableTemplateParametersSection: FC< > = ({ templateParameters, getInputProps, - defaultReasons, + autofillSources: defaultReasons, ...formSectionProps }) => { const hasMutableParameters = diff --git a/site/src/pages/CreateWorkspacePage/CreateWorkspacePageView.tsx b/site/src/pages/CreateWorkspacePage/CreateWorkspacePageView.tsx index c403d64513e5d..326dab01c754a 100644 --- a/site/src/pages/CreateWorkspacePage/CreateWorkspacePageView.tsx +++ b/site/src/pages/CreateWorkspacePage/CreateWorkspacePageView.tsx @@ -127,7 +127,7 @@ export const CreateWorkspacePageView: FC = ({ error, ); - const defaultSources = useMemo(() => { + const autofillSources = useMemo(() => { return autofillParameters.reduce( (acc, param) => { acc[param.name] = param.source; @@ -223,7 +223,7 @@ export const CreateWorkspacePageView: FC = ({ {parameters && ( <> { return { @@ -244,7 +244,7 @@ export const CreateWorkspacePageView: FC = ({ }} /> = ({ popover.setIsOpen(false); }} ephemeralParameters={ephemeralParameters} - buildParameters={buildParameters} + buildParameters={buildParameters.map( + (p): AutofillBuildParameter => ({ + ...p, + source: "active_build", + }), + )} />
@@ -147,7 +155,7 @@ const BuildParametersPopoverContent: FC = ({ interface FormProps { ephemeralParameters: TemplateVersionParameter[]; - buildParameters: WorkspaceBuildParameter[]; + buildParameters: AutofillBuildParameter[]; onSubmit: (buildParameters: WorkspaceBuildParameter[]) => void; } diff --git a/site/src/pages/WorkspaceSettingsPage/WorkspaceParametersPage/WorkspaceParametersForm.tsx b/site/src/pages/WorkspaceSettingsPage/WorkspaceParametersPage/WorkspaceParametersForm.tsx index 12ea287acfb84..0adbc4bfb9b04 100644 --- a/site/src/pages/WorkspaceSettingsPage/WorkspaceParametersPage/WorkspaceParametersForm.tsx +++ b/site/src/pages/WorkspaceSettingsPage/WorkspaceParametersPage/WorkspaceParametersForm.tsx @@ -10,6 +10,7 @@ import { } from "components/Form/Form"; import { RichParameterInput } from "components/RichParameterInput/RichParameterInput"; import { + AutofillBuildParameter, getInitialRichParameterValues, useValidationSchemaForRichParameters, } from "utils/richParameters"; @@ -27,7 +28,7 @@ export type WorkspaceParametersFormValues = { interface WorkspaceParameterFormProps { workspace: Workspace; templateVersionRichParameters: TemplateVersionParameter[]; - buildParameters: WorkspaceBuildParameter[]; + autofillParams: AutofillBuildParameter[]; isSubmitting: boolean; canChangeVersions: boolean; error: unknown; @@ -40,7 +41,7 @@ export const WorkspaceParametersForm: FC = ({ onCancel, onSubmit, templateVersionRichParameters, - buildParameters, + autofillParams, error, canChangeVersions, isSubmitting, @@ -50,7 +51,7 @@ export const WorkspaceParametersForm: FC = ({ initialValues: { rich_parameter_values: getInitialRichParameterValues( templateVersionRichParameters, - buildParameters, + autofillParams, ), }, validationSchema: Yup.object({ diff --git a/site/src/pages/WorkspaceSettingsPage/WorkspaceParametersPage/WorkspaceParametersPage.tsx b/site/src/pages/WorkspaceSettingsPage/WorkspaceParametersPage/WorkspaceParametersPage.tsx index 6d9f64dae5070..296bd6500b42f 100644 --- a/site/src/pages/WorkspaceSettingsPage/WorkspaceParametersPage/WorkspaceParametersPage.tsx +++ b/site/src/pages/WorkspaceSettingsPage/WorkspaceParametersPage/WorkspaceParametersPage.tsx @@ -24,6 +24,7 @@ import { EmptyState } from "components/EmptyState/EmptyState"; import Button from "@mui/material/Button"; import OpenInNewOutlined from "@mui/icons-material/OpenInNewOutlined"; import { docs } from "utils/docs"; +import { AutofillBuildParameter } from "utils/richParameters"; const WorkspaceParametersPage: FC = () => { const workspace = useWorkspaceSettings(); @@ -126,7 +127,12 @@ export const WorkspaceParametersPageView: FC< ({ + ...p, + source: "active_build", + }), + )} templateVersionRichParameters={data.templateVersionRichParameters} error={submitError} isSubmitting={isSubmitting} diff --git a/site/src/utils/richParameters.ts b/site/src/utils/richParameters.ts index f0c5deb649384..4277cd86041a0 100644 --- a/site/src/utils/richParameters.ts +++ b/site/src/utils/richParameters.ts @@ -4,8 +4,10 @@ import { } from "api/typesGenerated"; import * as Yup from "yup"; -export type AutofillSource = "user_history" | "url"; +export type AutofillSource = "user_history" | "url" | "active_build"; +// AutofillBuildParameter is a build parameter destined to a form, alongside +// its source so that the form can explain where the value comes from. export type AutofillBuildParameter = { source: AutofillSource; } & WorkspaceBuildParameter; @@ -19,21 +21,10 @@ export const getInitialRichParameterValues = ( (p) => p.name === parameter.name, ); if (autofillParam !== undefined && isValidValue(parameter, autofillParam)) { - // URL takes precedence over all other sources. - if (autofillParam.source === "url") { - return { - name: parameter.name, - value: autofillParam.value, - }; - } - - // Need to decide whether user_history is more important than default value. - if (autofillParam.source === "user_history") { - return { - name: parameter.name, - value: autofillParam.value, - }; - } + return { + name: parameter.name, + value: autofillParam.value, + }; } return { From 6cb0e29baa6dacc59ed5732e4f933df69777d21f Mon Sep 17 00:00:00 2001 From: Ammar Bandukwala Date: Sun, 21 Jan 2024 21:50:37 -0600 Subject: [PATCH 15/21] Fix ephemeral parameters --- site/e2e/parameters.ts | 4 +++- site/e2e/tests/restartWorkspace.spec.ts | 4 ++-- site/e2e/tests/startWorkspace.spec.ts | 4 ++-- site/src/utils/richParameters.ts | 10 ++++++++++ 4 files changed, 17 insertions(+), 5 deletions(-) diff --git a/site/e2e/parameters.ts b/site/e2e/parameters.ts index 4f3cf48bdb2da..47048d34688ef 100644 --- a/site/e2e/parameters.ts +++ b/site/e2e/parameters.ts @@ -129,8 +129,10 @@ export const seventhParameter: RichParameter = { }; // randParamName returns a new parameter with a random name. +// It helps to avoid cross-test interference when user-auto-fill triggers on +// the same parameter name. export const randParamName = (p: RichParameter): RichParameter => { - const name = p.name + Math.random().toString(36).substring(7); + const name = p.name + "_" + Math.random().toString(36).substring(7); return { ...p, name: name }; }; diff --git a/site/e2e/tests/restartWorkspace.spec.ts b/site/e2e/tests/restartWorkspace.spec.ts index 24d40658f657f..3ab099e687672 100644 --- a/site/e2e/tests/restartWorkspace.spec.ts +++ b/site/e2e/tests/restartWorkspace.spec.ts @@ -49,7 +49,7 @@ test("restart workspace with ephemeral parameters", async ({ page }) => { // Verify that build options are default (not selected). await verifyParameters(page, workspaceName, richParameters, [ - { name: firstBuildOption.name, value: firstBuildOption.defaultValue }, - { name: secondBuildOption.name, value: secondBuildOption.defaultValue }, + { name: richParameters[0].name, value: firstBuildOption.defaultValue }, + { name: richParameters[1].name, value: secondBuildOption.defaultValue }, ]); }); diff --git a/site/e2e/tests/startWorkspace.spec.ts b/site/e2e/tests/startWorkspace.spec.ts index 15d79d5c18f96..47aa83e59d0f6 100644 --- a/site/e2e/tests/startWorkspace.spec.ts +++ b/site/e2e/tests/startWorkspace.spec.ts @@ -50,7 +50,7 @@ test("start workspace with ephemeral parameters", async ({ page }) => { // Verify that build options are default (not selected). await verifyParameters(page, workspaceName, richParameters, [ - { name: firstBuildOption.name, value: firstBuildOption.defaultValue }, - { name: secondBuildOption.name, value: secondBuildOption.defaultValue }, + { name: richParameters[0].name, value: firstBuildOption.defaultValue }, + { name: richParameters[1].name, value: secondBuildOption.defaultValue }, ]); }); diff --git a/site/src/utils/richParameters.ts b/site/src/utils/richParameters.ts index 4277cd86041a0..3e3234085c055 100644 --- a/site/src/utils/richParameters.ts +++ b/site/src/utils/richParameters.ts @@ -17,9 +17,19 @@ export const getInitialRichParameterValues = ( autofillParams?: AutofillBuildParameter[], ): WorkspaceBuildParameter[] => { return templateParams.map((parameter) => { + // Short-circuit for ephemeral parameters, which are always reset to + // the template-defined default. + if (parameter.ephemeral) { + return { + name: parameter.name, + value: parameter.default_value, + }; + } + const autofillParam = autofillParams?.find( (p) => p.name === parameter.name, ); + if (autofillParam !== undefined && isValidValue(parameter, autofillParam)) { return { name: parameter.name, From 76e00186befd3d24dde11f74eee811f9aaab1554 Mon Sep 17 00:00:00 2001 From: Ammar Bandukwala Date: Sun, 21 Jan 2024 22:02:56 -0600 Subject: [PATCH 16/21] Minor fixes --- coderd/apidoc/docs.go | 5 +++-- coderd/apidoc/swagger.json | 5 +++-- docs/api/schemas.md | 12 ++++++------ docs/api/users.md | 14 +++++++------- site/e2e/tests/createWorkspace.spec.ts | 2 +- site/e2e/tests/restartWorkspace.spec.ts | 11 ++--------- site/e2e/tests/startWorkspace.spec.ts | 11 ++--------- .../TemplateParameters/TemplateParameters.tsx | 4 ++-- 8 files changed, 26 insertions(+), 38 deletions(-) diff --git a/coderd/apidoc/docs.go b/coderd/apidoc/docs.go index 195d2ba527127..00b01660a74e8 100644 --- a/coderd/apidoc/docs.go +++ b/coderd/apidoc/docs.go @@ -11819,8 +11819,9 @@ const docTemplate = `{ "codersdk.UserParameter": { "type": "object", "properties": { - "last_used": { - "type": "string" + "last_used_at": { + "type": "string", + "format": "date-time" }, "name": { "type": "string" diff --git a/coderd/apidoc/swagger.json b/coderd/apidoc/swagger.json index f6adfaee89e53..788dd3b135873 100644 --- a/coderd/apidoc/swagger.json +++ b/coderd/apidoc/swagger.json @@ -10712,8 +10712,9 @@ "codersdk.UserParameter": { "type": "object", "properties": { - "last_used": { - "type": "string" + "last_used_at": { + "type": "string", + "format": "date-time" }, "name": { "type": "string" diff --git a/docs/api/schemas.md b/docs/api/schemas.md index f4f3be91bfaf6..e9e585537263c 100644 --- a/docs/api/schemas.md +++ b/docs/api/schemas.md @@ -5788,7 +5788,7 @@ If the schedule is empty, the user will be updated to use the default schedule.| ```json { - "last_used": "string", + "last_used_at": "2019-08-24T14:15:22Z", "name": "string", "value": "string" } @@ -5796,11 +5796,11 @@ If the schedule is empty, the user will be updated to use the default schedule.| ### Properties -| Name | Type | Required | Restrictions | Description | -| ----------- | ------ | -------- | ------------ | ----------- | -| `last_used` | string | false | | | -| `name` | string | false | | | -| `value` | string | false | | | +| Name | Type | Required | Restrictions | Description | +| -------------- | ------ | -------- | ------------ | ----------- | +| `last_used_at` | string | false | | | +| `name` | string | false | | | +| `value` | string | false | | | ## codersdk.UserQuietHoursScheduleConfig diff --git a/docs/api/users.md b/docs/api/users.md index 6e20c46251231..4f94fae2790b2 100644 --- a/docs/api/users.md +++ b/docs/api/users.md @@ -1035,7 +1035,7 @@ curl -X GET http://coder-server:8080/api/v2/users/{user}/parameters \ ```json [ { - "last_used": "string", + "last_used_at": "2019-08-24T14:15:22Z", "name": "string", "value": "string" } @@ -1052,12 +1052,12 @@ curl -X GET http://coder-server:8080/api/v2/users/{user}/parameters \ Status Code **200** -| Name | Type | Required | Restrictions | Description | -| -------------- | ------ | -------- | ------------ | ----------- | -| `[array item]` | array | false | | | -| `» last_used` | string | false | | | -| `» name` | string | false | | | -| `» value` | string | false | | | +| Name | Type | Required | Restrictions | Description | +| ---------------- | ----------------- | -------- | ------------ | ----------- | +| `[array item]` | array | false | | | +| `» last_used_at` | string(date-time) | false | | | +| `» name` | string | false | | | +| `» value` | string | false | | | To perform this operation, you must be authenticated. [Learn more](authentication.md). diff --git a/site/e2e/tests/createWorkspace.spec.ts b/site/e2e/tests/createWorkspace.spec.ts index 073686ae42621..5e435340f3255 100644 --- a/site/e2e/tests/createWorkspace.spec.ts +++ b/site/e2e/tests/createWorkspace.spec.ts @@ -1,4 +1,3 @@ -import { randParamName } from "./../parameters"; import { test, expect } from "@playwright/test"; import { createTemplate, @@ -15,6 +14,7 @@ import { thirdParameter, seventhParameter, sixthParameter, + randParamName, } from "../parameters"; import { RichParameter } from "../provisionerGenerated"; import { beforeCoderTest } from "../hooks"; diff --git a/site/e2e/tests/restartWorkspace.spec.ts b/site/e2e/tests/restartWorkspace.spec.ts index 3ab099e687672..a0710e96fd58a 100644 --- a/site/e2e/tests/restartWorkspace.spec.ts +++ b/site/e2e/tests/restartWorkspace.spec.ts @@ -7,21 +7,14 @@ import { verifyParameters, } from "../helpers"; -import { - firstBuildOption, - randParamName, - secondBuildOption, -} from "../parameters"; +import { firstBuildOption, secondBuildOption } from "../parameters"; import { RichParameter } from "../provisionerGenerated"; import { beforeCoderTest } from "../hooks"; test.beforeEach(async ({ page }) => await beforeCoderTest(page)); test("restart workspace with ephemeral parameters", async ({ page }) => { - const richParameters: RichParameter[] = [ - randParamName(firstBuildOption), - randParamName(secondBuildOption), - ]; + const richParameters: RichParameter[] = [firstBuildOption, secondBuildOption]; const template = await createTemplate( page, echoResponsesWithParameters(richParameters), diff --git a/site/e2e/tests/startWorkspace.spec.ts b/site/e2e/tests/startWorkspace.spec.ts index 47aa83e59d0f6..be1cc5a5d78e1 100644 --- a/site/e2e/tests/startWorkspace.spec.ts +++ b/site/e2e/tests/startWorkspace.spec.ts @@ -8,18 +8,11 @@ import { verifyParameters, } from "../helpers"; -import { - firstBuildOption, - randParamName, - secondBuildOption, -} from "../parameters"; +import { firstBuildOption, secondBuildOption } from "../parameters"; import { RichParameter } from "../provisionerGenerated"; test("start workspace with ephemeral parameters", async ({ page }) => { - const richParameters: RichParameter[] = [ - randParamName(firstBuildOption), - randParamName(secondBuildOption), - ]; + const richParameters: RichParameter[] = [firstBuildOption, secondBuildOption]; const template = await createTemplate( page, echoResponsesWithParameters(richParameters), diff --git a/site/src/components/TemplateParameters/TemplateParameters.tsx b/site/src/components/TemplateParameters/TemplateParameters.tsx index fb29d42febca3..7f3ac3b7f457d 100644 --- a/site/src/components/TemplateParameters/TemplateParameters.tsx +++ b/site/src/components/TemplateParameters/TemplateParameters.tsx @@ -21,7 +21,7 @@ export const MutableTemplateParametersSection: FC< > = ({ templateParameters, getInputProps, - autofillSources: defaultReasons, + autofillSources, ...formSectionProps }) => { const hasMutableParameters = @@ -44,7 +44,7 @@ export const MutableTemplateParametersSection: FC< key={parameter.name} parameter={parameter} autofillSource={ - defaultReasons && defaultReasons[parameter.name] + autofillSources && autofillSources[parameter.name] } /> ), From 4b00f45da1347aa1d393b5b8793ca5e2ce1cf306 Mon Sep 17 00:00:00 2001 From: Ammar Bandukwala Date: Mon, 22 Jan 2024 13:21:52 -0600 Subject: [PATCH 17/21] Address marcin comments --- coderd/apidoc/docs.go | 85 ++++++++------- coderd/apidoc/swagger.json | 77 ++++++------- coderd/coderd.go | 2 +- coderd/database/dbauthz/dbauthz.go | 6 +- coderd/database/dbauthz/dbauthz_test.go | 7 +- coderd/database/dbmem/dbmem.go | 22 +--- coderd/database/dbmetrics/dbmetrics.go | 2 +- coderd/database/dbmock/dbmock.go | 2 +- coderd/database/querier.go | 4 +- coderd/database/queries.sql.go | 26 +++-- .../queries/workspacebuildparameters.sql | 10 +- coderd/users.go | 35 ++++-- coderd/users_test.go | 20 +++- codersdk/users.go | 11 +- docs/api/schemas.md | 10 +- docs/api/users.md | 103 +++++++++--------- site/src/api/api.ts | 4 +- site/src/api/queries/users.ts | 7 -- site/src/api/typesGenerated.ts | 1 - .../CreateWorkspacePage.tsx | 11 +- 20 files changed, 245 insertions(+), 200 deletions(-) diff --git a/coderd/apidoc/docs.go b/coderd/apidoc/docs.go index 00b01660a74e8..b8ffda6d5611c 100644 --- a/coderd/apidoc/docs.go +++ b/coderd/apidoc/docs.go @@ -4064,6 +4064,50 @@ const docTemplate = `{ } } }, + "/users/{user}/autofill-parameters": { + "get": { + "security": [ + { + "CoderSessionToken": [] + } + ], + "produces": [ + "application/json" + ], + "tags": [ + "Users" + ], + "summary": "Get autofill build parameters for user", + "operationId": "get-autofill-build-parameters-for-user", + "parameters": [ + { + "type": "string", + "description": "User ID, username, or me", + "name": "user", + "in": "path", + "required": true + }, + { + "type": "string", + "description": "Template ID", + "name": "template_id", + "in": "query", + "required": true + } + ], + "responses": { + "200": { + "description": "OK", + "schema": { + "type": "array", + "items": { + "$ref": "#/definitions/codersdk.UserParameter" + } + } + } + } + } + }, "/users/{user}/convert-login": { "post": { "security": [ @@ -4555,43 +4599,6 @@ const docTemplate = `{ } } }, - "/users/{user}/parameters": { - "get": { - "security": [ - { - "CoderSessionToken": [] - } - ], - "produces": [ - "application/json" - ], - "tags": [ - "Users" - ], - "summary": "Get recent build parameters for user.", - "operationId": "get-recent-build-parameters-for-user", - "parameters": [ - { - "type": "string", - "description": "User ID, username, or me", - "name": "user", - "in": "path", - "required": true - } - ], - "responses": { - "200": { - "description": "OK", - "schema": { - "type": "array", - "items": { - "$ref": "#/definitions/codersdk.UserParameter" - } - } - } - } - } - }, "/users/{user}/password": { "put": { "security": [ @@ -11819,10 +11826,6 @@ const docTemplate = `{ "codersdk.UserParameter": { "type": "object", "properties": { - "last_used_at": { - "type": "string", - "format": "date-time" - }, "name": { "type": "string" }, diff --git a/coderd/apidoc/swagger.json b/coderd/apidoc/swagger.json index 788dd3b135873..35db1b080bf03 100644 --- a/coderd/apidoc/swagger.json +++ b/coderd/apidoc/swagger.json @@ -3570,6 +3570,46 @@ } } }, + "/users/{user}/autofill-parameters": { + "get": { + "security": [ + { + "CoderSessionToken": [] + } + ], + "produces": ["application/json"], + "tags": ["Users"], + "summary": "Get autofill build parameters for user", + "operationId": "get-autofill-build-parameters-for-user", + "parameters": [ + { + "type": "string", + "description": "User ID, username, or me", + "name": "user", + "in": "path", + "required": true + }, + { + "type": "string", + "description": "Template ID", + "name": "template_id", + "in": "query", + "required": true + } + ], + "responses": { + "200": { + "description": "OK", + "schema": { + "type": "array", + "items": { + "$ref": "#/definitions/codersdk.UserParameter" + } + } + } + } + } + }, "/users/{user}/convert-login": { "post": { "security": [ @@ -4007,39 +4047,6 @@ } } }, - "/users/{user}/parameters": { - "get": { - "security": [ - { - "CoderSessionToken": [] - } - ], - "produces": ["application/json"], - "tags": ["Users"], - "summary": "Get recent build parameters for user.", - "operationId": "get-recent-build-parameters-for-user", - "parameters": [ - { - "type": "string", - "description": "User ID, username, or me", - "name": "user", - "in": "path", - "required": true - } - ], - "responses": { - "200": { - "description": "OK", - "schema": { - "type": "array", - "items": { - "$ref": "#/definitions/codersdk.UserParameter" - } - } - } - } - } - }, "/users/{user}/password": { "put": { "security": [ @@ -10712,10 +10719,6 @@ "codersdk.UserParameter": { "type": "object", "properties": { - "last_used_at": { - "type": "string", - "format": "date-time" - }, "name": { "type": "string" }, diff --git a/coderd/coderd.go b/coderd/coderd.go index 630d284017ee5..89d7f867c97b1 100644 --- a/coderd/coderd.go +++ b/coderd/coderd.go @@ -823,7 +823,7 @@ func New(options *Options) *API { r.Post("/convert-login", api.postConvertLoginType) r.Delete("/", api.deleteUser) r.Get("/", api.userByName) - r.Get("/parameters", api.userParameters) + r.Get("/autofill-parameters", api.userAutofillParameters) r.Get("/login-type", api.userLoginType) r.Put("/profile", api.putUserProfile) r.Route("/status", func(r chi.Router) { diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go index 626362a8f1f00..6c2b5607e2f55 100644 --- a/coderd/database/dbauthz/dbauthz.go +++ b/coderd/database/dbauthz/dbauthz.go @@ -1754,8 +1754,8 @@ func (q *querier) GetUserLinksByUserID(ctx context.Context, userID uuid.UUID) ([ return q.db.GetUserLinksByUserID(ctx, userID) } -func (q *querier) GetUserWorkspaceBuildParameters(ctx context.Context, ownerID uuid.UUID) ([]database.GetUserWorkspaceBuildParametersRow, error) { - u, err := q.db.GetUserByID(ctx, ownerID) +func (q *querier) GetUserWorkspaceBuildParameters(ctx context.Context, params database.GetUserWorkspaceBuildParametersParams) ([]database.GetUserWorkspaceBuildParametersRow, error) { + u, err := q.db.GetUserByID(ctx, params.OwnerID) if err != nil { return nil, err } @@ -1764,7 +1764,7 @@ func (q *querier) GetUserWorkspaceBuildParameters(ctx context.Context, ownerID u if err := q.authorizeContext(ctx, rbac.ActionUpdate, u.UserDataRBACObject()); err != nil { return nil, err } - return q.db.GetUserWorkspaceBuildParameters(ctx, ownerID) + return q.db.GetUserWorkspaceBuildParameters(ctx, params) } func (q *querier) GetUsers(ctx context.Context, arg database.GetUsersParams) ([]database.GetUsersRow, error) { diff --git a/coderd/database/dbauthz/dbauthz_test.go b/coderd/database/dbauthz/dbauthz_test.go index 6126d7f134908..dfea0eec110d7 100644 --- a/coderd/database/dbauthz/dbauthz_test.go +++ b/coderd/database/dbauthz/dbauthz_test.go @@ -1054,7 +1054,12 @@ func (s *MethodTestSuite) TestUser() { })) s.Run("GetUserWorkspaceBuildParameters", s.Subtest(func(db database.Store, check *expects) { u := dbgen.User(s.T(), db, database.User{}) - check.Args(u.ID).Asserts(u.UserDataRBACObject(), rbac.ActionUpdate).Returns( + check.Args( + database.GetUserWorkspaceBuildParametersParams{ + OwnerID: u.ID, + TemplateID: uuid.UUID{}, + }, + ).Asserts(u.UserDataRBACObject(), rbac.ActionUpdate).Returns( []database.GetUserWorkspaceBuildParametersRow{}, ) })) diff --git a/coderd/database/dbmem/dbmem.go b/coderd/database/dbmem/dbmem.go index 06aef49d305d7..409523d680034 100644 --- a/coderd/database/dbmem/dbmem.go +++ b/coderd/database/dbmem/dbmem.go @@ -3758,36 +3758,26 @@ func (q *FakeQuerier) GetUserLinksByUserID(_ context.Context, userID uuid.UUID) return uls, nil } -func (q *FakeQuerier) GetUserWorkspaceBuildParameters(_ context.Context, ownerID uuid.UUID) ([]database.GetUserWorkspaceBuildParametersRow, error) { +func (q *FakeQuerier) GetUserWorkspaceBuildParameters(_ context.Context, params database.GetUserWorkspaceBuildParametersParams) ([]database.GetUserWorkspaceBuildParametersRow, error) { q.mutex.RLock() defer q.mutex.RUnlock() userWorkspaceIDs := make(map[uuid.UUID]struct{}) for _, ws := range q.workspaces { - if ws.OwnerID != ownerID { + if ws.OwnerID != params.OwnerID { continue } - userWorkspaceIDs[ws.ID] = struct{}{} - } - - userWorkspaceBuilds := make(map[uuid.UUID]database.WorkspaceBuildTable) - for _, wb := range q.workspaceBuilds { - if _, ok := userWorkspaceIDs[wb.WorkspaceID]; !ok { + if ws.TemplateID != params.TemplateID { continue } - userWorkspaceBuilds[wb.ID] = wb + userWorkspaceIDs[ws.ID] = struct{}{} } userWorkspaceBuildParameters := make([]database.GetUserWorkspaceBuildParametersRow, 0) for _, wbp := range q.workspaceBuildParameters { - wb, ok := userWorkspaceBuilds[wbp.WorkspaceBuildID] - if !ok { - continue - } userWorkspaceBuildParameters = append(userWorkspaceBuildParameters, database.GetUserWorkspaceBuildParametersRow{ - Name: wbp.Name, - Value: wbp.Value, - CreatedAt: wb.CreatedAt, + Name: wbp.Name, + Value: wbp.Value, }) } diff --git a/coderd/database/dbmetrics/dbmetrics.go b/coderd/database/dbmetrics/dbmetrics.go index 65f393c6e494b..c5809f9f07d6d 100644 --- a/coderd/database/dbmetrics/dbmetrics.go +++ b/coderd/database/dbmetrics/dbmetrics.go @@ -998,7 +998,7 @@ func (m metricsStore) GetUserLinksByUserID(ctx context.Context, userID uuid.UUID return r0, r1 } -func (m metricsStore) GetUserWorkspaceBuildParameters(ctx context.Context, ownerID uuid.UUID) ([]database.GetUserWorkspaceBuildParametersRow, error) { +func (m metricsStore) GetUserWorkspaceBuildParameters(ctx context.Context, ownerID database.GetUserWorkspaceBuildParametersParams) ([]database.GetUserWorkspaceBuildParametersRow, error) { start := time.Now() r0, r1 := m.s.GetUserWorkspaceBuildParameters(ctx, ownerID) m.queryLatencies.WithLabelValues("GetUserWorkspaceBuildParameters").Observe(time.Since(start).Seconds()) diff --git a/coderd/database/dbmock/dbmock.go b/coderd/database/dbmock/dbmock.go index 991bf039f50b5..af2d6254026dc 100644 --- a/coderd/database/dbmock/dbmock.go +++ b/coderd/database/dbmock/dbmock.go @@ -2076,7 +2076,7 @@ func (mr *MockStoreMockRecorder) GetUserLinksByUserID(arg0, arg1 any) *gomock.Ca } // GetUserWorkspaceBuildParameters mocks base method. -func (m *MockStore) GetUserWorkspaceBuildParameters(arg0 context.Context, arg1 uuid.UUID) ([]database.GetUserWorkspaceBuildParametersRow, error) { +func (m *MockStore) GetUserWorkspaceBuildParameters(arg0 context.Context, arg1 database.GetUserWorkspaceBuildParametersParams) ([]database.GetUserWorkspaceBuildParametersRow, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "GetUserWorkspaceBuildParameters", arg0, arg1) ret0, _ := ret[0].([]database.GetUserWorkspaceBuildParametersRow) diff --git a/coderd/database/querier.go b/coderd/database/querier.go index d5756f9d00816..77b6fbe0f9a3e 100644 --- a/coderd/database/querier.go +++ b/coderd/database/querier.go @@ -209,7 +209,9 @@ type sqlcQuerier interface { GetUserLinkByLinkedID(ctx context.Context, linkedID string) (UserLink, error) GetUserLinkByUserIDLoginType(ctx context.Context, arg GetUserLinkByUserIDLoginTypeParams) (UserLink, error) GetUserLinksByUserID(ctx context.Context, userID uuid.UUID) ([]UserLink, error) - GetUserWorkspaceBuildParameters(ctx context.Context, ownerID uuid.UUID) ([]GetUserWorkspaceBuildParametersRow, error) + // If there are many distinct parameters, + // we only want the most recent ones. + GetUserWorkspaceBuildParameters(ctx context.Context, arg GetUserWorkspaceBuildParametersParams) ([]GetUserWorkspaceBuildParametersRow, error) // This will never return deleted users. GetUsers(ctx context.Context, arg GetUsersParams) ([]GetUsersRow, error) // This shouldn't check for deleted, because it's frequently used diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 9df29f29db6d1..0d30e4773249f 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -9873,8 +9873,7 @@ func (q *sqlQuerier) InsertWorkspaceAppStats(ctx context.Context, arg InsertWork const getUserWorkspaceBuildParameters = `-- name: GetUserWorkspaceBuildParameters :many SELECT sub.name, - sub.value, - sub.created_at + sub.value FROM ( SELECT wbp.name, @@ -9887,23 +9886,34 @@ FROM ( workspace_builds wb ON wb.id = wbp.workspace_build_id JOIN workspaces w ON w.id = wb.workspace_id + JOIN + template_version_parameters tvp ON tvp.template_version_id = wb.template_version_id WHERE w.owner_id = $1 AND wb.transition = 'start' + AND w.template_id = $2 + AND tvp.ephemeral = false ) sub WHERE sub.rn = 1 +ORDER BY sub.created_at DESC LIMIT 100 ` +type GetUserWorkspaceBuildParametersParams struct { + OwnerID uuid.UUID `db:"owner_id" json:"owner_id"` + TemplateID uuid.UUID `db:"template_id" json:"template_id"` +} + type GetUserWorkspaceBuildParametersRow struct { - Name string `db:"name" json:"name"` - Value string `db:"value" json:"value"` - CreatedAt time.Time `db:"created_at" json:"created_at"` + Name string `db:"name" json:"name"` + Value string `db:"value" json:"value"` } -func (q *sqlQuerier) GetUserWorkspaceBuildParameters(ctx context.Context, ownerID uuid.UUID) ([]GetUserWorkspaceBuildParametersRow, error) { - rows, err := q.db.QueryContext(ctx, getUserWorkspaceBuildParameters, ownerID) +// If there are many distinct parameters, +// we only want the most recent ones. +func (q *sqlQuerier) GetUserWorkspaceBuildParameters(ctx context.Context, arg GetUserWorkspaceBuildParametersParams) ([]GetUserWorkspaceBuildParametersRow, error) { + rows, err := q.db.QueryContext(ctx, getUserWorkspaceBuildParameters, arg.OwnerID, arg.TemplateID) if err != nil { return nil, err } @@ -9911,7 +9921,7 @@ func (q *sqlQuerier) GetUserWorkspaceBuildParameters(ctx context.Context, ownerI var items []GetUserWorkspaceBuildParametersRow for rows.Next() { var i GetUserWorkspaceBuildParametersRow - if err := rows.Scan(&i.Name, &i.Value, &i.CreatedAt); err != nil { + if err := rows.Scan(&i.Name, &i.Value); err != nil { return nil, err } items = append(items, i) diff --git a/coderd/database/queries/workspacebuildparameters.sql b/coderd/database/queries/workspacebuildparameters.sql index 22ae7472c727c..2de041b8f4d44 100644 --- a/coderd/database/queries/workspacebuildparameters.sql +++ b/coderd/database/queries/workspacebuildparameters.sql @@ -18,8 +18,7 @@ WHERE -- name: GetUserWorkspaceBuildParameters :many SELECT sub.name, - sub.value, - sub.created_at + sub.value FROM ( SELECT wbp.name, @@ -32,10 +31,17 @@ FROM ( workspace_builds wb ON wb.id = wbp.workspace_build_id JOIN workspaces w ON w.id = wb.workspace_id + JOIN + template_version_parameters tvp ON tvp.template_version_id = wb.template_version_id WHERE w.owner_id = $1 AND wb.transition = 'start' + AND w.template_id = $2 + AND tvp.ephemeral = false ) sub WHERE sub.rn = 1 +ORDER BY sub.created_at DESC +-- If there are many distinct parameters, +-- we only want the most recent ones. LIMIT 100; diff --git a/coderd/users.go b/coderd/users.go index bf73ff1a3f005..bf7dc2f7e2921 100644 --- a/coderd/users.go +++ b/coderd/users.go @@ -571,15 +571,16 @@ func (api *API) userByName(rw http.ResponseWriter, r *http.Request) { // Returns recent build parameters for the signed-in user. // -// @Summary Get recent build parameters for user. -// @ID get-recent-build-parameters-for-user +// @Summary Get autofill build parameters for user +// @ID get-autofill-build-parameters-for-user // @Security CoderSessionToken // @Produce json // @Tags Users // @Param user path string true "User ID, username, or me" +// @Param template_id query string true "Template ID" // @Success 200 {array} codersdk.UserParameter -// @Router /users/{user}/parameters [get] -func (api *API) userParameters(rw http.ResponseWriter, r *http.Request) { +// @Router /users/{user}/autofill-parameters [get] +func (api *API) userAutofillParameters(rw http.ResponseWriter, r *http.Request) { var ( apiKey = httpmw.APIKey(r) user = httpmw.UserParam(r) @@ -592,9 +593,28 @@ func (api *API) userParameters(rw http.ResponseWriter, r *http.Request) { return } + templateIDStr := r.URL.Query().Get("template_id") + if templateIDStr == "" { + httpapi.Write(r.Context(), rw, http.StatusBadRequest, codersdk.Response{ + Message: "template_id is required.", + }) + return + } + + templateID, err := uuid.Parse(templateIDStr) + if err != nil { + httpapi.Write(r.Context(), rw, http.StatusBadRequest, codersdk.Response{ + Message: "template_id must be a valid UUID.", + }) + return + } + params, err := api.Database.GetUserWorkspaceBuildParameters( r.Context(), - apiKey.UserID, + database.GetUserWorkspaceBuildParametersParams{ + OwnerID: user.ID, + TemplateID: templateID, + }, ) if err != nil { httpapi.Write(r.Context(), rw, http.StatusInternalServerError, codersdk.Response{ @@ -607,9 +627,8 @@ func (api *API) userParameters(rw http.ResponseWriter, r *http.Request) { var sdkParams []codersdk.UserParameter for _, param := range params { sdkParams = append(sdkParams, codersdk.UserParameter{ - Name: param.Name, - Value: param.Value, - LastUsedAt: param.CreatedAt, + Name: param.Name, + Value: param.Value, }) } diff --git a/coderd/users_test.go b/coderd/users_test.go index 12e855171a489..feb50dbc84cba 100644 --- a/coderd/users_test.go +++ b/coderd/users_test.go @@ -1722,11 +1722,11 @@ func TestSuspendedPagination(t *testing.T) { require.Equal(t, expected, page.Users, "expected page") } -func TestUserParameters(t *testing.T) { +func TestUserAutofillParameters(t *testing.T) { t.Parallel() t.Run("NotSelf", func(t *testing.T) { t.Parallel() - client := coderdtest.New(t, nil) + client, _, api := coderdtest.NewWithAPI(t, &coderdtest.Options{IncludeProvisionerDaemon: true}) u1 := coderdtest.CreateFirstUser(t, client) @@ -1735,9 +1735,20 @@ func TestUserParameters(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) defer cancel() - _, err := client.UserParameters( + db := api.Database + + version := dbfake.TemplateVersion(t, db).Seed(database.TemplateVersion{ + CreatedBy: u1.UserID, + OrganizationID: u1.OrganizationID, + }).Params(database.TemplateVersionParameter{ + Name: "param", + Required: true, + }).Do() + + _, err := client.UserAutofillParameters( ctx, u2.ID.String(), + version.Template.ID.String(), ) var apiErr *codersdk.Error @@ -1774,9 +1785,10 @@ func TestUserParameters(t *testing.T) { } }) - params, err := client.UserParameters( + params, err := client.UserAutofillParameters( ctx, u1.UserID.String(), + version.Template.ID.String(), ) require.NoError(t, err) diff --git a/codersdk/users.go b/codersdk/users.go index 2fb077ca01822..8f052e7d255b9 100644 --- a/codersdk/users.go +++ b/codersdk/users.go @@ -222,14 +222,13 @@ type OIDCAuthMethod struct { } type UserParameter struct { - Name string `json:"name"` - Value string `json:"value"` - LastUsedAt time.Time `json:"last_used_at" format:"date-time"` + Name string `json:"name"` + Value string `json:"value"` } -// UserParameters returns all recently used parameters for the given user. -func (c *Client) UserParameters(ctx context.Context, user string) ([]UserParameter, error) { - res, err := c.Request(ctx, http.MethodGet, fmt.Sprintf("/api/v2/users/%s/parameters", user), nil) +// UserAutofillParameters returns all recently used parameters for the given user. +func (c *Client) UserAutofillParameters(ctx context.Context, user string, templateID string) ([]UserParameter, error) { + res, err := c.Request(ctx, http.MethodGet, fmt.Sprintf("/api/v2/users/%s/autofill-parameters?template_id=%s", user, templateID), nil) if err != nil { return nil, err } diff --git a/docs/api/schemas.md b/docs/api/schemas.md index e9e585537263c..28b52a1e03a63 100644 --- a/docs/api/schemas.md +++ b/docs/api/schemas.md @@ -5788,7 +5788,6 @@ If the schedule is empty, the user will be updated to use the default schedule.| ```json { - "last_used_at": "2019-08-24T14:15:22Z", "name": "string", "value": "string" } @@ -5796,11 +5795,10 @@ If the schedule is empty, the user will be updated to use the default schedule.| ### Properties -| Name | Type | Required | Restrictions | Description | -| -------------- | ------ | -------- | ------------ | ----------- | -| `last_used_at` | string | false | | | -| `name` | string | false | | | -| `value` | string | false | | | +| Name | Type | Required | Restrictions | Description | +| ------- | ------ | -------- | ------------ | ----------- | +| `name` | string | false | | | +| `value` | string | false | | | ## codersdk.UserQuietHoursScheduleConfig diff --git a/docs/api/users.md b/docs/api/users.md index 4f94fae2790b2..e656ed6ac27a7 100644 --- a/docs/api/users.md +++ b/docs/api/users.md @@ -513,6 +513,57 @@ curl -X PUT http://coder-server:8080/api/v2/users/{user}/appearance \ To perform this operation, you must be authenticated. [Learn more](authentication.md). +## Get autofill build parameters for user + +### Code samples + +```shell +# Example request using curl +curl -X GET http://coder-server:8080/api/v2/users/{user}/autofill-parameters?template_id=string \ + -H 'Accept: application/json' \ + -H 'Coder-Session-Token: API_KEY' +``` + +`GET /users/{user}/autofill-parameters` + +### Parameters + +| Name | In | Type | Required | Description | +| ------------- | ----- | ------ | -------- | ------------------------ | +| `user` | path | string | true | User ID, username, or me | +| `template_id` | query | string | true | Template ID | + +### Example responses + +> 200 Response + +```json +[ + { + "name": "string", + "value": "string" + } +] +``` + +### Responses + +| Status | Meaning | Description | Schema | +| ------ | ------------------------------------------------------- | ----------- | ------------------------------------------------------------------- | +| 200 | [OK](https://tools.ietf.org/html/rfc7231#section-6.3.1) | OK | array of [codersdk.UserParameter](schemas.md#codersdkuserparameter) | + +

Response Schema

+ +Status Code **200** + +| Name | Type | Required | Restrictions | Description | +| -------------- | ------ | -------- | ------------ | ----------- | +| `[array item]` | array | false | | | +| `» name` | string | false | | | +| `» value` | string | false | | | + +To perform this operation, you must be authenticated. [Learn more](authentication.md). + ## Get user Git SSH key ### Code samples @@ -1009,58 +1060,6 @@ curl -X GET http://coder-server:8080/api/v2/users/{user}/organizations/{organiza To perform this operation, you must be authenticated. [Learn more](authentication.md). -## Get recent build parameters for user. - -### Code samples - -```shell -# Example request using curl -curl -X GET http://coder-server:8080/api/v2/users/{user}/parameters \ - -H 'Accept: application/json' \ - -H 'Coder-Session-Token: API_KEY' -``` - -`GET /users/{user}/parameters` - -### Parameters - -| Name | In | Type | Required | Description | -| ------ | ---- | ------ | -------- | ------------------------ | -| `user` | path | string | true | User ID, username, or me | - -### Example responses - -> 200 Response - -```json -[ - { - "last_used_at": "2019-08-24T14:15:22Z", - "name": "string", - "value": "string" - } -] -``` - -### Responses - -| Status | Meaning | Description | Schema | -| ------ | ------------------------------------------------------- | ----------- | ------------------------------------------------------------------- | -| 200 | [OK](https://tools.ietf.org/html/rfc7231#section-6.3.1) | OK | array of [codersdk.UserParameter](schemas.md#codersdkuserparameter) | - -

Response Schema

- -Status Code **200** - -| Name | Type | Required | Restrictions | Description | -| ---------------- | ----------------- | -------- | ------------ | ----------- | -| `[array item]` | array | false | | | -| `» last_used_at` | string(date-time) | false | | | -| `» name` | string | false | | | -| `» value` | string | false | | | - -To perform this operation, you must be authenticated. [Learn more](authentication.md). - ## Update user password ### Code samples diff --git a/site/src/api/api.ts b/site/src/api/api.ts index f99d9c18855a3..5e53b9fcd8820 100644 --- a/site/src/api/api.ts +++ b/site/src/api/api.ts @@ -129,9 +129,9 @@ export const getAuthenticatedUser = async () => { return response.data; }; -export const getUserParameters = async () => { +export const getUserParameters = async (templateID: string) => { const response = await axios.get( - "/api/v2/users/me/parameters", + "/api/v2/users/me/autofill-parameters?template_id=" + templateID, ); return response.data; }; diff --git a/site/src/api/queries/users.ts b/site/src/api/queries/users.ts index a8a88f0e88d33..0249315071b13 100644 --- a/site/src/api/queries/users.ts +++ b/site/src/api/queries/users.ts @@ -134,13 +134,6 @@ export const me = (): UseQueryOptions & { }; }; -export const userParameters = () => { - return { - queryKey: ["userParameters"], - queryFn: API.getUserParameters, - }; -}; - export const hasFirstUser = () => { return { queryKey: ["hasFirstUser"], diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index a0675d850bc81..e713402629d5f 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -1421,7 +1421,6 @@ export interface UserLoginType { export interface UserParameter { readonly name: string; readonly value: string; - readonly last_used_at: string; } // From codersdk/deployment.go diff --git a/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.tsx b/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.tsx index 07cbf883eeded..48fa44b4cb381 100644 --- a/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.tsx +++ b/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.tsx @@ -24,13 +24,13 @@ import { } from "api/queries/templates"; import { autoCreateWorkspace, createWorkspace } from "api/queries/workspaces"; import { useEffectEvent } from "hooks/hookPolyfills"; -import { userParameters } from "api/queries/users"; import { AutofillBuildParameter } from "utils/richParameters"; import { paramsUsedToCreateWorkspace } from "utils/workspace"; import { Loader } from "components/Loader/Loader"; import { ErrorAlert } from "components/Alert/ErrorAlert"; import { CreateWSPermissions, createWorkspaceChecks } from "./permissions"; import { CreateWorkspacePageView } from "./CreateWorkspacePageView"; +import { getUserParameters } from "api/api"; export const createWorkspaceModes = ["form", "auto", "duplicate"] as const; export type CreateWorkspaceMode = (typeof createWorkspaceModes)[number]; @@ -54,7 +54,14 @@ const CreateWorkspacePage: FC = () => { const createWorkspaceMutation = useMutation(createWorkspace(queryClient)); const templateQuery = useQuery(templateByName(organizationId, templateName)); - const userParametersQuery = useQuery(userParameters()); + + const userParametersQuery = useQuery( + ["userParameters"], + () => getUserParameters(templateQuery.data!.id), + { + enabled: templateQuery.isSuccess, + }, + ); const permissionsQuery = useQuery( checkAuthorization({ From 69ed7c0e3952aab85da94730d7bbfa3fdda53b39 Mon Sep 17 00:00:00 2001 From: Ammar Bandukwala Date: Tue, 23 Jan 2024 12:13:40 -0600 Subject: [PATCH 18/21] Address spike + mathias + cian comments --- coderd/database/dbauthz/dbauthz.go | 4 +- coderd/database/dbmem/dbmem.go | 41 ++++++++- coderd/database/modelmethods.go | 4 + .../queries/workspacebuildparameters.sql | 1 + coderd/rbac/object.go | 6 ++ coderd/rbac/object_gen.go | 1 + coderd/rbac/roles.go | 10 +- coderd/users.go | 31 ++----- coderd/users_test.go | 92 ++++++++++++++----- codersdk/rbacresources.go | 48 +++++----- codersdk/users.go | 2 +- .../RichParameterInput/RichParameterInput.tsx | 20 ++-- .../CreateWorkspacePage.tsx | 9 +- site/src/utils/richParameters.ts | 14 +-- 14 files changed, 178 insertions(+), 105 deletions(-) diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go index 6c2b5607e2f55..f5f34382e4494 100644 --- a/coderd/database/dbauthz/dbauthz.go +++ b/coderd/database/dbauthz/dbauthz.go @@ -1759,9 +1759,7 @@ func (q *querier) GetUserWorkspaceBuildParameters(ctx context.Context, params da if err != nil { return nil, err } - // The ability to update the user implies either the user themselves or someone - // with complete admin access to the user account. - if err := q.authorizeContext(ctx, rbac.ActionUpdate, u.UserDataRBACObject()); err != nil { + if err := q.authorizeContext(ctx, rbac.ActionRead, u.UserWorkspaceBuildParametersObject()); err != nil { return nil, err } return q.db.GetUserWorkspaceBuildParameters(ctx, params) diff --git a/coderd/database/dbmem/dbmem.go b/coderd/database/dbmem/dbmem.go index 409523d680034..edb4579006657 100644 --- a/coderd/database/dbmem/dbmem.go +++ b/coderd/database/dbmem/dbmem.go @@ -3773,15 +3773,48 @@ func (q *FakeQuerier) GetUserWorkspaceBuildParameters(_ context.Context, params userWorkspaceIDs[ws.ID] = struct{}{} } - userWorkspaceBuildParameters := make([]database.GetUserWorkspaceBuildParametersRow, 0) + userWorkspaceBuilds := make(map[uuid.UUID]struct{}) + for _, wb := range q.workspaceBuilds { + if _, ok := userWorkspaceIDs[wb.WorkspaceID]; !ok { + continue + } + userWorkspaceBuilds[wb.ID] = struct{}{} + } + + templateVersions := make(map[uuid.UUID]struct{}) + for _, tv := range q.templateVersions { + if tv.TemplateID.UUID != params.TemplateID { + continue + } + templateVersions[tv.ID] = struct{}{} + } + + tvps := make(map[string]struct{}) + for _, tvp := range q.templateVersionParameters { + if _, ok := templateVersions[tvp.TemplateVersionID]; !ok { + continue + } + + if _, ok := tvps[tvp.Name]; !ok && !tvp.Ephemeral { + tvps[tvp.Name] = struct{}{} + } + } + + userWorkspaceBuildParameters := make(map[string]database.GetUserWorkspaceBuildParametersRow) for _, wbp := range q.workspaceBuildParameters { - userWorkspaceBuildParameters = append(userWorkspaceBuildParameters, database.GetUserWorkspaceBuildParametersRow{ + if _, ok := userWorkspaceBuilds[wbp.WorkspaceBuildID]; !ok { + continue + } + if _, ok := tvps[wbp.Name]; !ok { + continue + } + userWorkspaceBuildParameters[wbp.Name] = database.GetUserWorkspaceBuildParametersRow{ Name: wbp.Name, Value: wbp.Value, - }) + } } - return userWorkspaceBuildParameters, nil + return maps.Values(userWorkspaceBuildParameters), nil } func (q *FakeQuerier) GetUsers(_ context.Context, params database.GetUsersParams) ([]database.GetUsersRow, error) { diff --git a/coderd/database/modelmethods.go b/coderd/database/modelmethods.go index 685c138c95288..15835a2a39639 100644 --- a/coderd/database/modelmethods.go +++ b/coderd/database/modelmethods.go @@ -255,6 +255,10 @@ func (u User) UserDataRBACObject() rbac.Object { return rbac.ResourceUserData.WithID(u.ID).WithOwner(u.ID.String()) } +func (u User) UserWorkspaceBuildParametersObject() rbac.Object { + return rbac.ResourceUserWorkspaceBuildParameters.WithID(u.ID).WithOwner(u.ID.String()) +} + func (u GetUsersRow) RBACObject() rbac.Object { return rbac.ResourceUserObject(u.ID) } diff --git a/coderd/database/queries/workspacebuildparameters.sql b/coderd/database/queries/workspacebuildparameters.sql index 2de041b8f4d44..362dc9ea72fad 100644 --- a/coderd/database/queries/workspacebuildparameters.sql +++ b/coderd/database/queries/workspacebuildparameters.sql @@ -38,6 +38,7 @@ FROM ( AND wb.transition = 'start' AND w.template_id = $2 AND tvp.ephemeral = false + AND tvp.name = wbp.name ) sub WHERE sub.rn = 1 diff --git a/coderd/rbac/object.go b/coderd/rbac/object.go index 373c5b6b2df17..ace060b3141d1 100644 --- a/coderd/rbac/object.go +++ b/coderd/rbac/object.go @@ -148,6 +148,12 @@ var ( Type: "user_data", } + // ResourceUserWorkspaceBuildParameters is the user's workspace build + // parameter history. + ResourceUserWorkspaceBuildParameters = Object{ + Type: "user_workspace_build_parameters", + } + // ResourceOrganizationMember is a user's membership in an organization. // Has ONLY an organization owner. // create/delete = Create/delete member from org. diff --git a/coderd/rbac/object_gen.go b/coderd/rbac/object_gen.go index aadf3fa1edcb7..4668f56b06700 100644 --- a/coderd/rbac/object_gen.go +++ b/coderd/rbac/object_gen.go @@ -25,6 +25,7 @@ func AllResources() []Object { ResourceTemplateInsights, ResourceUser, ResourceUserData, + ResourceUserWorkspaceBuildParameters, ResourceWildcard, ResourceWorkspace, ResourceWorkspaceApplicationConnect, diff --git a/coderd/rbac/roles.go b/coderd/rbac/roles.go index 7f8e0b2759547..d6a53d5b9b121 100644 --- a/coderd/rbac/roles.go +++ b/coderd/rbac/roles.go @@ -154,7 +154,8 @@ func ReloadBuiltinRoles(opts *RoleOptions) { Permissions(map[string][]Action{ // Users cannot do create/update/delete on themselves, but they // can read their own details. - ResourceUser.Type: {ActionRead}, + ResourceUser.Type: {ActionRead}, + ResourceUserWorkspaceBuildParameters.Type: {ActionRead}, // Users can create provisioner daemons scoped to themselves. ResourceProvisionerDaemon.Type: {ActionCreate, ActionRead, ActionUpdate}, })..., @@ -209,9 +210,10 @@ func ReloadBuiltinRoles(opts *RoleOptions) { Name: userAdmin, DisplayName: "User Admin", Site: Permissions(map[string][]Action{ - ResourceRoleAssignment.Type: {ActionCreate, ActionRead, ActionUpdate, ActionDelete}, - ResourceUser.Type: {ActionCreate, ActionRead, ActionUpdate, ActionDelete}, - ResourceUserData.Type: {ActionCreate, ActionRead, ActionUpdate, ActionDelete}, + ResourceRoleAssignment.Type: {ActionCreate, ActionRead, ActionUpdate, ActionDelete}, + ResourceUser.Type: {ActionCreate, ActionRead, ActionUpdate, ActionDelete}, + ResourceUserData.Type: {ActionCreate, ActionRead, ActionUpdate, ActionDelete}, + ResourceUserWorkspaceBuildParameters.Type: {ActionCreate, ActionRead, ActionUpdate, ActionDelete}, // Full perms to manage org members ResourceOrganizationMember.Type: {ActionCreate, ActionRead, ActionUpdate, ActionDelete}, ResourceGroup.Type: {ActionCreate, ActionRead, ActionUpdate, ActionDelete}, diff --git a/coderd/users.go b/coderd/users.go index bf7dc2f7e2921..e584576eed52f 100644 --- a/coderd/users.go +++ b/coderd/users.go @@ -581,30 +581,15 @@ func (api *API) userByName(rw http.ResponseWriter, r *http.Request) { // @Success 200 {array} codersdk.UserParameter // @Router /users/{user}/autofill-parameters [get] func (api *API) userAutofillParameters(rw http.ResponseWriter, r *http.Request) { - var ( - apiKey = httpmw.APIKey(r) - user = httpmw.UserParam(r) - ) - - if apiKey.UserID != user.ID { - httpapi.Write(r.Context(), rw, http.StatusForbidden, codersdk.Response{ - Message: "You are not authorized to view this user's parameters.", - }) - return - } - - templateIDStr := r.URL.Query().Get("template_id") - if templateIDStr == "" { - httpapi.Write(r.Context(), rw, http.StatusBadRequest, codersdk.Response{ - Message: "template_id is required.", - }) - return - } + user := httpmw.UserParam(r) - templateID, err := uuid.Parse(templateIDStr) - if err != nil { + p := httpapi.NewQueryParamParser().Required("template_id") + templateID := p.UUID(r.URL.Query(), uuid.UUID{}, "template_id") + p.ErrorExcessParams(r.URL.Query()) + if len(p.Errors) > 0 { httpapi.Write(r.Context(), rw, http.StatusBadRequest, codersdk.Response{ - Message: "template_id must be a valid UUID.", + Message: "Invalid query parameters.", + Validations: p.Errors, }) return } @@ -616,7 +601,7 @@ func (api *API) userAutofillParameters(rw http.ResponseWriter, r *http.Request) TemplateID: templateID, }, ) - if err != nil { + if err != nil && !errors.Is(err, sql.ErrNoRows) { httpapi.Write(r.Context(), rw, http.StatusInternalServerError, codersdk.Response{ Message: "Internal error fetching user's parameters.", Detail: err.Error(), diff --git a/coderd/users_test.go b/coderd/users_test.go index feb50dbc84cba..d726a37fb57ac 100644 --- a/coderd/users_test.go +++ b/coderd/users_test.go @@ -1726,11 +1726,11 @@ func TestUserAutofillParameters(t *testing.T) { t.Parallel() t.Run("NotSelf", func(t *testing.T) { t.Parallel() - client, _, api := coderdtest.NewWithAPI(t, &coderdtest.Options{IncludeProvisionerDaemon: true}) + client1, _, api := coderdtest.NewWithAPI(t, &coderdtest.Options{}) - u1 := coderdtest.CreateFirstUser(t, client) + u1 := coderdtest.CreateFirstUser(t, client1) - _, u2 := coderdtest.CreateAnotherUser(t, client, u1.OrganizationID) + client2, u2 := coderdtest.CreateAnotherUser(t, client1, u1.OrganizationID) ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) defer cancel() @@ -1745,25 +1745,32 @@ func TestUserAutofillParameters(t *testing.T) { Required: true, }).Do() - _, err := client.UserAutofillParameters( + _, err := client2.UserAutofillParameters( ctx, - u2.ID.String(), - version.Template.ID.String(), + u1.UserID.String(), + version.Template.ID, ) var apiErr *codersdk.Error require.ErrorAs(t, err, &apiErr) - require.Equal(t, http.StatusForbidden, apiErr.StatusCode()) + require.Equal(t, http.StatusBadRequest, apiErr.StatusCode()) + + // u1 should be able to read u2's parameters as u1 is site admin. + _, err = client1.UserAutofillParameters( + ctx, + u2.ID.String(), + version.Template.ID, + ) + require.NoError(t, err) }) t.Run("FindsParameters", func(t *testing.T) { t.Parallel() - client, _, api := coderdtest.NewWithAPI(t, &coderdtest.Options{IncludeProvisionerDaemon: true}) + client1, _, api := coderdtest.NewWithAPI(t, &coderdtest.Options{}) - u1 := coderdtest.CreateFirstUser(t, client) + u1 := coderdtest.CreateFirstUser(t, client1) - ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) - defer cancel() + client2, u2 := coderdtest.CreateAnotherUser(t, client1, u1.OrganizationID) db := api.Database @@ -1773,22 +1780,36 @@ func TestUserAutofillParameters(t *testing.T) { }).Params(database.TemplateVersionParameter{ Name: "param", Required: true, - }).Do() + }, + database.TemplateVersionParameter{ + Name: "param2", + Ephemeral: true, + }, + ).Do() + + dbfake.WorkspaceBuild(t, db, database.Workspace{ + OwnerID: u2.ID, + TemplateID: version.Template.ID, + }).Params( + database.WorkspaceBuildParameter{ + Name: "param", + Value: "foo", + }, + database.WorkspaceBuildParameter{ + Name: "param2", + Value: "bar", + }, + ).Do() - coderdtest.CreateWorkspace(t, client, u1.OrganizationID, version.Template.ID, - func(cwr *codersdk.CreateWorkspaceRequest) { - cwr.RichParameterValues = []codersdk.WorkspaceBuildParameter{ - { - Name: "param", - Value: "foo", - }, - } - }) + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) + defer cancel() - params, err := client.UserAutofillParameters( + // Use client2 since client1 is site admin, so + // we don't get good coverage on RBAC working. + params, err := client2.UserAutofillParameters( ctx, - u1.UserID.String(), - version.Template.ID.String(), + u2.ID.String(), + version.Template.ID, ) require.NoError(t, err) @@ -1796,6 +1817,29 @@ func TestUserAutofillParameters(t *testing.T) { require.Equal(t, "param", params[0].Name) require.Equal(t, "foo", params[0].Value) + + // Verify that latest parameter value is returned. + dbfake.WorkspaceBuild(t, db, database.Workspace{ + OwnerID: u2.ID, + TemplateID: version.Template.ID, + }).Params( + database.WorkspaceBuildParameter{ + Name: "param", + Value: "foo_new", + }, + ).Do() + + params, err = client2.UserAutofillParameters( + ctx, + u2.ID.String(), + version.Template.ID, + ) + require.NoError(t, err) + + require.Equal(t, 1, len(params)) + + require.Equal(t, "param", params[0].Name) + require.Equal(t, "foo_new", params[0].Value) }) } diff --git a/codersdk/rbacresources.go b/codersdk/rbacresources.go index 8854568525058..4b517e544e28f 100644 --- a/codersdk/rbacresources.go +++ b/codersdk/rbacresources.go @@ -3,29 +3,30 @@ package codersdk type RBACResource string const ( - ResourceWorkspace RBACResource = "workspace" - ResourceWorkspaceProxy RBACResource = "workspace_proxy" - ResourceWorkspaceExecution RBACResource = "workspace_execution" - ResourceWorkspaceApplicationConnect RBACResource = "application_connect" - ResourceAuditLog RBACResource = "audit_log" - ResourceTemplate RBACResource = "template" - ResourceGroup RBACResource = "group" - ResourceFile RBACResource = "file" - ResourceProvisionerDaemon RBACResource = "provisioner_daemon" - ResourceOrganization RBACResource = "organization" - ResourceRoleAssignment RBACResource = "assign_role" - ResourceOrgRoleAssignment RBACResource = "assign_org_role" - ResourceAPIKey RBACResource = "api_key" - ResourceUser RBACResource = "user" - ResourceUserData RBACResource = "user_data" - ResourceOrganizationMember RBACResource = "organization_member" - ResourceLicense RBACResource = "license" - ResourceDeploymentValues RBACResource = "deployment_config" - ResourceDeploymentStats RBACResource = "deployment_stats" - ResourceReplicas RBACResource = "replicas" - ResourceDebugInfo RBACResource = "debug_info" - ResourceSystem RBACResource = "system" - ResourceTemplateInsights RBACResource = "template_insights" + ResourceWorkspace RBACResource = "workspace" + ResourceWorkspaceProxy RBACResource = "workspace_proxy" + ResourceWorkspaceExecution RBACResource = "workspace_execution" + ResourceWorkspaceApplicationConnect RBACResource = "application_connect" + ResourceAuditLog RBACResource = "audit_log" + ResourceTemplate RBACResource = "template" + ResourceGroup RBACResource = "group" + ResourceFile RBACResource = "file" + ResourceProvisionerDaemon RBACResource = "provisioner_daemon" + ResourceOrganization RBACResource = "organization" + ResourceRoleAssignment RBACResource = "assign_role" + ResourceOrgRoleAssignment RBACResource = "assign_org_role" + ResourceAPIKey RBACResource = "api_key" + ResourceUser RBACResource = "user" + ResourceUserData RBACResource = "user_data" + ResourceUserWorkspaceBuildParameters RBACResource = "user_workspace_build_parameters" + ResourceOrganizationMember RBACResource = "organization_member" + ResourceLicense RBACResource = "license" + ResourceDeploymentValues RBACResource = "deployment_config" + ResourceDeploymentStats RBACResource = "deployment_stats" + ResourceReplicas RBACResource = "replicas" + ResourceDebugInfo RBACResource = "debug_info" + ResourceSystem RBACResource = "system" + ResourceTemplateInsights RBACResource = "template_insights" ) const ( @@ -52,6 +53,7 @@ var ( ResourceAPIKey, ResourceUser, ResourceUserData, + ResourceUserWorkspaceBuildParameters, ResourceOrganizationMember, ResourceLicense, ResourceDeploymentValues, diff --git a/codersdk/users.go b/codersdk/users.go index 8f052e7d255b9..1bd8852cf4dbe 100644 --- a/codersdk/users.go +++ b/codersdk/users.go @@ -227,7 +227,7 @@ type UserParameter struct { } // UserAutofillParameters returns all recently used parameters for the given user. -func (c *Client) UserAutofillParameters(ctx context.Context, user string, templateID string) ([]UserParameter, error) { +func (c *Client) UserAutofillParameters(ctx context.Context, user string, templateID uuid.UUID) ([]UserParameter, error) { res, err := c.Request(ctx, http.MethodGet, fmt.Sprintf("/api/v2/users/%s/autofill-parameters?template_id=%s", user, templateID), nil) if err != nil { return nil, err diff --git a/site/src/components/RichParameterInput/RichParameterInput.tsx b/site/src/components/RichParameterInput/RichParameterInput.tsx index ad71582c1f467..f21584abc5717 100644 --- a/site/src/components/RichParameterInput/RichParameterInput.tsx +++ b/site/src/components/RichParameterInput/RichParameterInput.tsx @@ -160,16 +160,16 @@ export const RichParameterInput: FC = ({
{autofillSource && autofillSource !== "active_build" && ( -
- 🪄 Autofilled:{" "} - { - { - ["url"]: "value supplied by URL.", - ["user_history"]: "recently used value.", - }[autofillSource] - } -
- )} +
+ 🪄 Autofilled:{" "} + {autofillSourceLabel[autofillSource]} +
+)} + +const autofillSourceLabel = { + url: "value supplied by URL.", + user_history: "recently used value.", +};
); diff --git a/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.tsx b/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.tsx index 48fa44b4cb381..0e2b5e308b7b4 100644 --- a/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.tsx +++ b/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.tsx @@ -232,15 +232,16 @@ const getAutofillParameters = ( return acc; }, new Map()); - const buildValues: AutofillBuildParameter[] = []; - Array.from(urlSearchParams.keys()) + const buildValues: AutofillBuildParameter[] = Array.from( + urlSearchParams.keys(), + ) .filter((key) => key.startsWith("param.")) - .forEach((key) => { + .map((key) => { const name = key.replace("param.", ""); const value = urlSearchParams.get(key) ?? ""; // URL should take precedence over user parameters userParamMap.delete(name); - buildValues.push({ name, value, source: "url" }); + return { name, value, source: "url" }; }); userParamMap.forEach((param) => { diff --git a/site/src/utils/richParameters.ts b/site/src/utils/richParameters.ts index 3e3234085c055..b09a70883cd98 100644 --- a/site/src/utils/richParameters.ts +++ b/site/src/utils/richParameters.ts @@ -27,19 +27,15 @@ export const getInitialRichParameterValues = ( } const autofillParam = autofillParams?.find( - (p) => p.name === parameter.name, + ({ name }) => name === parameter.name, ); - if (autofillParam !== undefined && isValidValue(parameter, autofillParam)) { - return { - name: parameter.name, - value: autofillParam.value, - }; - } - return { name: parameter.name, - value: parameter.default_value, + value: + autofillParam && isValidValue(parameter, autofillParam) + ? autofillParam.value + : parameter.default_value, }; }); }; From 72f1e88aebbe4cda44dd4c89edfe8644af23ed40 Mon Sep 17 00:00:00 2001 From: Ammar Bandukwala Date: Tue, 23 Jan 2024 14:58:57 -0600 Subject: [PATCH 19/21] Fix tests --- coderd/apidoc/docs.go | 2 + coderd/apidoc/swagger.json | 2 + coderd/database/dbauthz/dbauthz_test.go | 2 +- coderd/database/queries.sql.go | 1 + coderd/externalauth/externalauth.go | 2 +- coderd/users_test.go | 5 ++- docs/api/schemas.md | 51 +++++++++++++------------ site/src/api/typesGenerated.ts | 2 + 8 files changed, 38 insertions(+), 29 deletions(-) diff --git a/coderd/apidoc/docs.go b/coderd/apidoc/docs.go index 393626adb2bb4..05d8980dfae79 100644 --- a/coderd/apidoc/docs.go +++ b/coderd/apidoc/docs.go @@ -10416,6 +10416,7 @@ const docTemplate = `{ "api_key", "user", "user_data", + "user_workspace_build_parameters", "organization_member", "license", "deployment_config", @@ -10441,6 +10442,7 @@ const docTemplate = `{ "ResourceAPIKey", "ResourceUser", "ResourceUserData", + "ResourceUserWorkspaceBuildParameters", "ResourceOrganizationMember", "ResourceLicense", "ResourceDeploymentValues", diff --git a/coderd/apidoc/swagger.json b/coderd/apidoc/swagger.json index 9d917ffb9e0aa..acb97cbd5079c 100644 --- a/coderd/apidoc/swagger.json +++ b/coderd/apidoc/swagger.json @@ -9368,6 +9368,7 @@ "api_key", "user", "user_data", + "user_workspace_build_parameters", "organization_member", "license", "deployment_config", @@ -9393,6 +9394,7 @@ "ResourceAPIKey", "ResourceUser", "ResourceUserData", + "ResourceUserWorkspaceBuildParameters", "ResourceOrganizationMember", "ResourceLicense", "ResourceDeploymentValues", diff --git a/coderd/database/dbauthz/dbauthz_test.go b/coderd/database/dbauthz/dbauthz_test.go index dfea0eec110d7..a04407081acb6 100644 --- a/coderd/database/dbauthz/dbauthz_test.go +++ b/coderd/database/dbauthz/dbauthz_test.go @@ -1059,7 +1059,7 @@ func (s *MethodTestSuite) TestUser() { OwnerID: u.ID, TemplateID: uuid.UUID{}, }, - ).Asserts(u.UserDataRBACObject(), rbac.ActionUpdate).Returns( + ).Asserts(u.UserWorkspaceBuildParametersObject(), rbac.ActionRead).Returns( []database.GetUserWorkspaceBuildParametersRow{}, ) })) diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 1e024f3904cf4..90a5bb99197e2 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -9893,6 +9893,7 @@ FROM ( AND wb.transition = 'start' AND w.template_id = $2 AND tvp.ephemeral = false + AND tvp.name = wbp.name ) sub WHERE sub.rn = 1 diff --git a/coderd/externalauth/externalauth.go b/coderd/externalauth/externalauth.go index 0c936743a0df5..d4d9f060e65d8 100644 --- a/coderd/externalauth/externalauth.go +++ b/coderd/externalauth/externalauth.go @@ -347,7 +347,7 @@ func (c *DeviceAuth) AuthorizeDevice(ctx context.Context) (*codersdk.ExternalAut case mediaType == "application/x-www-form-urlencoded": return nil, xerrors.Errorf("status_code=%d, payload response is form-url encoded, expected a json payload", resp.StatusCode) default: - return nil, fmt.Errorf("status_code=%d, mediaType=%s: %w", resp.StatusCode, mediaType, err) + return nil, xerrors.Errorf("status_code=%d, mediaType=%s: %w", resp.StatusCode, mediaType, err) } } if r.ErrorDescription != "" { diff --git a/coderd/users_test.go b/coderd/users_test.go index d726a37fb57ac..cfbab40f2b54c 100644 --- a/coderd/users_test.go +++ b/coderd/users_test.go @@ -1788,8 +1788,9 @@ func TestUserAutofillParameters(t *testing.T) { ).Do() dbfake.WorkspaceBuild(t, db, database.Workspace{ - OwnerID: u2.ID, - TemplateID: version.Template.ID, + OwnerID: u2.ID, + TemplateID: version.Template.ID, + OrganizationID: u1.OrganizationID, }).Params( database.WorkspaceBuildParameter{ Name: "param", diff --git a/docs/api/schemas.md b/docs/api/schemas.md index e9264b54e9fa3..300ec6af5a8b4 100644 --- a/docs/api/schemas.md +++ b/docs/api/schemas.md @@ -4189,31 +4189,32 @@ AuthorizationObject can represent a "set" of objects, such as: all workspaces in #### Enumerated Values -| Value | -| --------------------- | -| `workspace` | -| `workspace_proxy` | -| `workspace_execution` | -| `application_connect` | -| `audit_log` | -| `template` | -| `group` | -| `file` | -| `provisioner_daemon` | -| `organization` | -| `assign_role` | -| `assign_org_role` | -| `api_key` | -| `user` | -| `user_data` | -| `organization_member` | -| `license` | -| `deployment_config` | -| `deployment_stats` | -| `replicas` | -| `debug_info` | -| `system` | -| `template_insights` | +| Value | +| --------------------------------- | +| `workspace` | +| `workspace_proxy` | +| `workspace_execution` | +| `application_connect` | +| `audit_log` | +| `template` | +| `group` | +| `file` | +| `provisioner_daemon` | +| `organization` | +| `assign_role` | +| `assign_org_role` | +| `api_key` | +| `user` | +| `user_data` | +| `user_workspace_build_parameters` | +| `organization_member` | +| `license` | +| `deployment_config` | +| `deployment_stats` | +| `replicas` | +| `debug_info` | +| `system` | +| `template_insights` | ## codersdk.RateLimitConfig diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index 53ea6a63cc5de..04b29751b344a 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -2004,6 +2004,7 @@ export type RBACResource = | "template_insights" | "user" | "user_data" + | "user_workspace_build_parameters" | "workspace" | "workspace_execution" | "workspace_proxy"; @@ -2028,6 +2029,7 @@ export const RBACResources: RBACResource[] = [ "template_insights", "user", "user_data", + "user_workspace_build_parameters", "workspace", "workspace_execution", "workspace_proxy", From b0eece835a82b3fb3b49c24fa7450b3c3514b91e Mon Sep 17 00:00:00 2001 From: Ammar Bandukwala Date: Tue, 30 Jan 2024 14:24:13 -0600 Subject: [PATCH 20/21] Optimize SQL per Mathias suggestion --- .../queries/workspacebuildparameters.sql | 48 ++++++++----------- coderd/users.go | 2 +- coderd/users_test.go | 5 +- 3 files changed, 24 insertions(+), 31 deletions(-) diff --git a/coderd/database/queries/workspacebuildparameters.sql b/coderd/database/queries/workspacebuildparameters.sql index 362dc9ea72fad..76bbbae1f3c1d 100644 --- a/coderd/database/queries/workspacebuildparameters.sql +++ b/coderd/database/queries/workspacebuildparameters.sql @@ -16,33 +16,25 @@ WHERE workspace_build_id = $1; -- name: GetUserWorkspaceBuildParameters :many -SELECT - sub.name, - sub.value -FROM ( - SELECT - wbp.name, - wbp.value, - wb.created_at, - ROW_NUMBER() OVER (PARTITION BY wbp.name ORDER BY wb.created_at DESC) as rn - FROM - workspace_build_parameters wbp - JOIN - workspace_builds wb ON wb.id = wbp.workspace_build_id - JOIN - workspaces w ON w.id = wb.workspace_id - JOIN - template_version_parameters tvp ON tvp.template_version_id = wb.template_version_id - WHERE - w.owner_id = $1 - AND wb.transition = 'start' - AND w.template_id = $2 - AND tvp.ephemeral = false - AND tvp.name = wbp.name -) sub +-- name: GetUserWorkspaceBuildParameters :many +SELECT DISTINCT ON (tvp.name) + tvp.name, + wbp.value +FROM + workspace_build_parameters wbp +JOIN + workspace_builds wb ON wb.id = wbp.workspace_build_id +JOIN + workspaces w ON w.id = wb.workspace_id +JOIN + template_version_parameters tvp ON tvp.template_version_id = wb.template_version_id WHERE - sub.rn = 1 -ORDER BY sub.created_at DESC --- If there are many distinct parameters, --- we only want the most recent ones. + w.owner_id = $1 + AND wb.transition = 'start' + AND w.template_id = $2 + AND tvp.ephemeral = false + AND tvp.name = wbp.name +ORDER BY + tvp.name, wb.created_at DESC LIMIT 100; + diff --git a/coderd/users.go b/coderd/users.go index e584576eed52f..b20051c0a14a3 100644 --- a/coderd/users.go +++ b/coderd/users.go @@ -609,7 +609,7 @@ func (api *API) userAutofillParameters(rw http.ResponseWriter, r *http.Request) return } - var sdkParams []codersdk.UserParameter + sdkParams := []codersdk.UserParameter{} for _, param := range params { sdkParams = append(sdkParams, codersdk.UserParameter{ Name: param.Name, diff --git a/coderd/users_test.go b/coderd/users_test.go index cfbab40f2b54c..a9b4fa8de72b3 100644 --- a/coderd/users_test.go +++ b/coderd/users_test.go @@ -1821,8 +1821,9 @@ func TestUserAutofillParameters(t *testing.T) { // Verify that latest parameter value is returned. dbfake.WorkspaceBuild(t, db, database.Workspace{ - OwnerID: u2.ID, - TemplateID: version.Template.ID, + OrganizationID: u1.OrganizationID, + OwnerID: u2.ID, + TemplateID: version.Template.ID, }).Params( database.WorkspaceBuildParameter{ Name: "param", From a6db979a2a451c1bbfa295a8f0c09048ee9e5c27 Mon Sep 17 00:00:00 2001 From: Ammar Bandukwala Date: Tue, 30 Jan 2024 15:06:21 -0600 Subject: [PATCH 21/21] make gen --- coderd/database/querier.go | 2 -- coderd/database/queries.sql.go | 48 ++++++++++++++-------------------- 2 files changed, 19 insertions(+), 31 deletions(-) diff --git a/coderd/database/querier.go b/coderd/database/querier.go index 237f301491578..7d9ec076497d0 100644 --- a/coderd/database/querier.go +++ b/coderd/database/querier.go @@ -211,8 +211,6 @@ type sqlcQuerier interface { GetUserLinkByLinkedID(ctx context.Context, linkedID string) (UserLink, error) GetUserLinkByUserIDLoginType(ctx context.Context, arg GetUserLinkByUserIDLoginTypeParams) (UserLink, error) GetUserLinksByUserID(ctx context.Context, userID uuid.UUID) ([]UserLink, error) - // If there are many distinct parameters, - // we only want the most recent ones. GetUserWorkspaceBuildParameters(ctx context.Context, arg GetUserWorkspaceBuildParametersParams) ([]GetUserWorkspaceBuildParametersRow, error) // This will never return deleted users. GetUsers(ctx context.Context, arg GetUsersParams) ([]GetUsersRow, error) diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 72780f803707a..9a4fa07ec5e54 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -9940,33 +9940,25 @@ func (q *sqlQuerier) InsertWorkspaceAppStats(ctx context.Context, arg InsertWork } const getUserWorkspaceBuildParameters = `-- name: GetUserWorkspaceBuildParameters :many -SELECT - sub.name, - sub.value -FROM ( - SELECT - wbp.name, - wbp.value, - wb.created_at, - ROW_NUMBER() OVER (PARTITION BY wbp.name ORDER BY wb.created_at DESC) as rn - FROM - workspace_build_parameters wbp - JOIN - workspace_builds wb ON wb.id = wbp.workspace_build_id - JOIN - workspaces w ON w.id = wb.workspace_id - JOIN - template_version_parameters tvp ON tvp.template_version_id = wb.template_version_id - WHERE - w.owner_id = $1 - AND wb.transition = 'start' - AND w.template_id = $2 - AND tvp.ephemeral = false - AND tvp.name = wbp.name -) sub -WHERE - sub.rn = 1 -ORDER BY sub.created_at DESC +SELECT DISTINCT ON (tvp.name) + tvp.name, + wbp.value +FROM + workspace_build_parameters wbp +JOIN + workspace_builds wb ON wb.id = wbp.workspace_build_id +JOIN + workspaces w ON w.id = wb.workspace_id +JOIN + template_version_parameters tvp ON tvp.template_version_id = wb.template_version_id +WHERE + w.owner_id = $1 + AND wb.transition = 'start' + AND w.template_id = $2 + AND tvp.ephemeral = false + AND tvp.name = wbp.name +ORDER BY + tvp.name, wb.created_at DESC LIMIT 100 ` @@ -9980,8 +9972,6 @@ type GetUserWorkspaceBuildParametersRow struct { Value string `db:"value" json:"value"` } -// If there are many distinct parameters, -// we only want the most recent ones. func (q *sqlQuerier) GetUserWorkspaceBuildParameters(ctx context.Context, arg GetUserWorkspaceBuildParametersParams) ([]GetUserWorkspaceBuildParametersRow, error) { rows, err := q.db.QueryContext(ctx, getUserWorkspaceBuildParameters, arg.OwnerID, arg.TemplateID) if err != nil {