From f5bf8f9965b7cd8fc38d491e33fafecb59e080a3 Mon Sep 17 00:00:00 2001 From: Bruno Quaresma Date: Tue, 21 Mar 2023 17:11:56 +0000 Subject: [PATCH 01/11] feat(coder): Add PATCH /templateversions/:templateversion endpoint --- coderd/apidoc/docs.go | 33 +++++++++ coderd/apidoc/swagger.json | 29 ++++++++ coderd/database/dbauthz/querier.go | 6 +- coderd/database/dbfake/databasefake.go | 9 +-- coderd/database/querier.go | 2 +- coderd/database/queries.sql.go | 32 +++++++-- coderd/database/queries/templateversions.sql | 7 +- coderd/templates.go | 2 +- coderd/templateversions.go | 29 ++++++++ codersdk/deployment.go | 2 +- codersdk/templateversions.go | 16 +++++ docs/api/templates.md | 75 ++++++++++++++++++++ site/src/api/typesGenerated.ts | 5 ++ 13 files changed, 228 insertions(+), 19 deletions(-) diff --git a/coderd/apidoc/docs.go b/coderd/apidoc/docs.go index d3e5b48d58db0..40a48c9dda9be 100644 --- a/coderd/apidoc/docs.go +++ b/coderd/apidoc/docs.go @@ -2255,6 +2255,39 @@ const docTemplate = `{ } } } + }, + "patch": { + "security": [ + { + "CoderSessionToken": [] + } + ], + "produces": [ + "application/json" + ], + "tags": [ + "Templates" + ], + "summary": "Patch template version by ID", + "operationId": "patch-template-version-by-id", + "parameters": [ + { + "type": "string", + "format": "uuid", + "description": "Template version ID", + "name": "templateversion", + "in": "path", + "required": true + } + ], + "responses": { + "200": { + "description": "OK", + "schema": { + "$ref": "#/definitions/codersdk.TemplateVersion" + } + } + } } }, "/templateversions/{templateversion}/cancel": { diff --git a/coderd/apidoc/swagger.json b/coderd/apidoc/swagger.json index 913e02eb22c37..4e6bbfe3b3f29 100644 --- a/coderd/apidoc/swagger.json +++ b/coderd/apidoc/swagger.json @@ -1971,6 +1971,35 @@ } } } + }, + "patch": { + "security": [ + { + "CoderSessionToken": [] + } + ], + "produces": ["application/json"], + "tags": ["Templates"], + "summary": "Patch template version by ID", + "operationId": "patch-template-version-by-id", + "parameters": [ + { + "type": "string", + "format": "uuid", + "description": "Template version ID", + "name": "templateversion", + "in": "path", + "required": true + } + ], + "responses": { + "200": { + "description": "OK", + "schema": { + "$ref": "#/definitions/codersdk.TemplateVersion" + } + } + } } }, "/templateversions/{templateversion}/cancel": { diff --git a/coderd/database/dbauthz/querier.go b/coderd/database/dbauthz/querier.go index 867bfa36383d1..42b4cb87e81d6 100644 --- a/coderd/database/dbauthz/querier.go +++ b/coderd/database/dbauthz/querier.go @@ -850,13 +850,13 @@ func (q *querier) UpdateTemplateScheduleByID(ctx context.Context, arg database.U return updateWithReturn(q.log, q.auth, fetch, q.db.UpdateTemplateScheduleByID)(ctx, arg) } -func (q *querier) UpdateTemplateVersionByID(ctx context.Context, arg database.UpdateTemplateVersionByIDParams) error { +func (q *querier) UpdateTemplateVersionByID(ctx context.Context, arg database.UpdateTemplateVersionByIDParams) (database.TemplateVersion, error) { template, err := q.db.GetTemplateByID(ctx, arg.TemplateID.UUID) if err != nil { - return err + return database.TemplateVersion{}, err } if err := q.authorizeContext(ctx, rbac.ActionUpdate, template); err != nil { - return err + return database.TemplateVersion{}, err } return q.db.UpdateTemplateVersionByID(ctx, arg) } diff --git a/coderd/database/dbfake/databasefake.go b/coderd/database/dbfake/databasefake.go index f77b19f81d9e1..cfe67a729076f 100644 --- a/coderd/database/dbfake/databasefake.go +++ b/coderd/database/dbfake/databasefake.go @@ -3413,9 +3413,9 @@ func (q *fakeQuerier) UpdateTemplateACLByID(_ context.Context, arg database.Upda return database.Template{}, sql.ErrNoRows } -func (q *fakeQuerier) UpdateTemplateVersionByID(_ context.Context, arg database.UpdateTemplateVersionByIDParams) error { +func (q *fakeQuerier) UpdateTemplateVersionByID(_ context.Context, arg database.UpdateTemplateVersionByIDParams) (database.TemplateVersion, error) { if err := validateDatabaseType(arg); err != nil { - return err + return database.TemplateVersion{}, err } q.mutex.Lock() @@ -3427,10 +3427,11 @@ func (q *fakeQuerier) UpdateTemplateVersionByID(_ context.Context, arg database. } templateVersion.TemplateID = arg.TemplateID templateVersion.UpdatedAt = arg.UpdatedAt + templateVersion.Name = arg.Name q.templateVersions[index] = templateVersion - return nil + return templateVersion, nil } - return sql.ErrNoRows + return database.TemplateVersion{}, sql.ErrNoRows } func (q *fakeQuerier) UpdateTemplateVersionDescriptionByJobID(_ context.Context, arg database.UpdateTemplateVersionDescriptionByJobIDParams) error { diff --git a/coderd/database/querier.go b/coderd/database/querier.go index 174e5fcec1575..77ce2395d8424 100644 --- a/coderd/database/querier.go +++ b/coderd/database/querier.go @@ -210,7 +210,7 @@ type sqlcQuerier interface { UpdateTemplateDeletedByID(ctx context.Context, arg UpdateTemplateDeletedByIDParams) error UpdateTemplateMetaByID(ctx context.Context, arg UpdateTemplateMetaByIDParams) (Template, error) UpdateTemplateScheduleByID(ctx context.Context, arg UpdateTemplateScheduleByIDParams) (Template, error) - UpdateTemplateVersionByID(ctx context.Context, arg UpdateTemplateVersionByIDParams) error + UpdateTemplateVersionByID(ctx context.Context, arg UpdateTemplateVersionByIDParams) (TemplateVersion, error) UpdateTemplateVersionDescriptionByJobID(ctx context.Context, arg UpdateTemplateVersionDescriptionByJobIDParams) error UpdateTemplateVersionGitAuthProvidersByJobID(ctx context.Context, arg UpdateTemplateVersionGitAuthProvidersByJobIDParams) error UpdateUserDeletedByID(ctx context.Context, arg UpdateUserDeletedByIDParams) error diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 7d5539ba5b369..1ff1cb7c7c6cc 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -4095,25 +4095,45 @@ func (q *sqlQuerier) InsertTemplateVersion(ctx context.Context, arg InsertTempla return i, err } -const updateTemplateVersionByID = `-- name: UpdateTemplateVersionByID :exec +const updateTemplateVersionByID = `-- name: UpdateTemplateVersionByID :one UPDATE template_versions SET template_id = $2, - updated_at = $3 + updated_at = $3, + name = $4 WHERE - id = $1 + id = $1 RETURNING id, template_id, organization_id, created_at, updated_at, name, readme, job_id, created_by, git_auth_providers ` type UpdateTemplateVersionByIDParams struct { ID uuid.UUID `db:"id" json:"id"` TemplateID uuid.NullUUID `db:"template_id" json:"template_id"` UpdatedAt time.Time `db:"updated_at" json:"updated_at"` + Name string `db:"name" json:"name"` } -func (q *sqlQuerier) UpdateTemplateVersionByID(ctx context.Context, arg UpdateTemplateVersionByIDParams) error { - _, err := q.db.ExecContext(ctx, updateTemplateVersionByID, arg.ID, arg.TemplateID, arg.UpdatedAt) - return err +func (q *sqlQuerier) UpdateTemplateVersionByID(ctx context.Context, arg UpdateTemplateVersionByIDParams) (TemplateVersion, error) { + row := q.db.QueryRowContext(ctx, updateTemplateVersionByID, + arg.ID, + arg.TemplateID, + arg.UpdatedAt, + arg.Name, + ) + var i TemplateVersion + err := row.Scan( + &i.ID, + &i.TemplateID, + &i.OrganizationID, + &i.CreatedAt, + &i.UpdatedAt, + &i.Name, + &i.Readme, + &i.JobID, + &i.CreatedBy, + pq.Array(&i.GitAuthProviders), + ) + return i, err } const updateTemplateVersionDescriptionByJobID = `-- name: UpdateTemplateVersionDescriptionByJobID :exec diff --git a/coderd/database/queries/templateversions.sql b/coderd/database/queries/templateversions.sql index e4efc7f46ab90..89f56ef617bcd 100644 --- a/coderd/database/queries/templateversions.sql +++ b/coderd/database/queries/templateversions.sql @@ -84,14 +84,15 @@ INSERT INTO VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9) RETURNING *; --- name: UpdateTemplateVersionByID :exec +-- name: UpdateTemplateVersionByID :one UPDATE template_versions SET template_id = $2, - updated_at = $3 + updated_at = $3, + name = $4 WHERE - id = $1; + id = $1 RETURNING *; -- name: UpdateTemplateVersionDescriptionByJobID :exec UPDATE diff --git a/coderd/templates.go b/coderd/templates.go index 9bdc591cc6da1..9ebc661e45a4a 100644 --- a/coderd/templates.go +++ b/coderd/templates.go @@ -269,7 +269,7 @@ func (api *API) postTemplateByOrganization(rw http.ResponseWriter, r *http.Reque templateAudit.New = dbTemplate - err = tx.UpdateTemplateVersionByID(ctx, database.UpdateTemplateVersionByIDParams{ + _, err = tx.UpdateTemplateVersionByID(ctx, database.UpdateTemplateVersionByIDParams{ ID: templateVersion.ID, TemplateID: uuid.NullUUID{ UUID: dbTemplate.ID, diff --git a/coderd/templateversions.go b/coderd/templateversions.go index 59f42c8150df1..c8ed665aa1d4b 100644 --- a/coderd/templateversions.go +++ b/coderd/templateversions.go @@ -63,6 +63,35 @@ func (api *API) templateVersion(rw http.ResponseWriter, r *http.Request) { httpapi.Write(ctx, rw, http.StatusOK, convertTemplateVersion(templateVersion, convertProvisionerJob(job), user)) } +// @Summary Patch template version by ID +// @ID patch-template-version-by-id +// @Security CoderSessionToken +// @Produce json +// @Tags Templates +// @Param templateversion path string true "Template version ID" format(uuid) +// @Success 200 {object} codersdk.TemplateVersion +// @Router /templateversions/{templateversion} [patch] +func (api *API) patchTemplateVersion(rw http.ResponseWriter, r *http.Request) { + ctx := r.Context() + templateVersion := httpmw.TemplateVersionParam(r) + var params codersdk.PatchTemplateVersionRequest + if !httpapi.Read(ctx, rw, r, ¶ms) { + return + } + templateVersion, err := api.Database.UpdateTemplateVersionByID(ctx, database.UpdateTemplateVersionByIDParams{ + ID: templateVersion.ID, + Name: params.Name, + }) + if err != nil { + httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ + Message: "Error on patching template version.", + Detail: err.Error(), + }) + return + } + httpapi.Write(ctx, rw, http.StatusNoContent, templateVersion) +} + // @Summary Cancel template version by ID // @ID cancel-template-version-by-id // @Security CoderSessionToken diff --git a/codersdk/deployment.go b/codersdk/deployment.go index dba09776a87b1..bb4292e929f14 100644 --- a/codersdk/deployment.go +++ b/codersdk/deployment.go @@ -199,7 +199,7 @@ func ParseSSHConfigOption(opt string) (key string, value string, err error) { return r == ' ' || r == '=' }) if idx == -1 { - return "", "", fmt.Errorf("invalid config-ssh option %q", opt) + return "", "", xerrors.New(fmt.Sprintf("invalid config-ssh option %q", opt)) } return opt[:idx], opt[idx+1:], nil } diff --git a/codersdk/templateversions.go b/codersdk/templateversions.go index 80a1a14776440..e5dc1eaad7e14 100644 --- a/codersdk/templateversions.go +++ b/codersdk/templateversions.go @@ -76,6 +76,10 @@ type TemplateVersionVariable struct { Sensitive bool `json:"sensitive"` } +type PatchTemplateVersionRequest struct { + Name string `json:"name"` +} + // TemplateVersion returns a template version by ID. func (c *Client) TemplateVersion(ctx context.Context, id uuid.UUID) (TemplateVersion, error) { res, err := c.Request(ctx, http.MethodGet, fmt.Sprintf("/api/v2/templateversions/%s", id), nil) @@ -291,3 +295,15 @@ func (c *Client) PreviousTemplateVersion(ctx context.Context, organization uuid. var version TemplateVersion return version, json.NewDecoder(res.Body).Decode(&version) } + +func (c *Client) UpdateTemplateVersion(ctx context.Context, version, req PatchTemplateVersionRequest) error { + res, err := c.Request(ctx, http.MethodPatch, fmt.Sprintf("/api/v2/templateversions/%s", version), req) + if err != nil { + return err + } + defer res.Body.Close() + if res.StatusCode != http.StatusNoContent { + return ReadBodyAsError(res) + } + return nil +} diff --git a/docs/api/templates.md b/docs/api/templates.md index d3397e02d8209..38dcef4d39a0e 100644 --- a/docs/api/templates.md +++ b/docs/api/templates.md @@ -1250,6 +1250,81 @@ curl -X GET http://coder-server:8080/api/v2/templateversions/{templateversion} \ To perform this operation, you must be authenticated. [Learn more](authentication.md). +## Patch template version by ID + +### Code samples + +```shell +# Example request using curl +curl -X PATCH http://coder-server:8080/api/v2/templateversions/{templateversion} \ + -H 'Accept: application/json' \ + -H 'Coder-Session-Token: API_KEY' +``` + +`PATCH /templateversions/{templateversion}` + +### Parameters + +| Name | In | Type | Required | Description | +| ----------------- | ---- | ------------ | -------- | ------------------- | +| `templateversion` | path | string(uuid) | true | Template version ID | + +### Example responses + +> 200 Response + +```json +{ + "created_at": "2019-08-24T14:15:22Z", + "created_by": { + "avatar_url": "http://example.com", + "created_at": "2019-08-24T14:15:22Z", + "email": "user@example.com", + "id": "497f6eca-6276-4993-bfeb-53cbbbba6f08", + "last_seen_at": "2019-08-24T14:15:22Z", + "organization_ids": ["497f6eca-6276-4993-bfeb-53cbbbba6f08"], + "roles": [ + { + "display_name": "string", + "name": "string" + } + ], + "status": "active", + "username": "string" + }, + "id": "497f6eca-6276-4993-bfeb-53cbbbba6f08", + "job": { + "canceled_at": "2019-08-24T14:15:22Z", + "completed_at": "2019-08-24T14:15:22Z", + "created_at": "2019-08-24T14:15:22Z", + "error": "string", + "error_code": "MISSING_TEMPLATE_PARAMETER", + "file_id": "8a0cfb4f-ddc9-436d-91bb-75133c583767", + "id": "497f6eca-6276-4993-bfeb-53cbbbba6f08", + "started_at": "2019-08-24T14:15:22Z", + "status": "pending", + "tags": { + "property1": "string", + "property2": "string" + }, + "worker_id": "ae5fa6f7-c55b-40c1-b40a-b36ac467652b" + }, + "name": "string", + "organization_id": "7c60d51f-b44e-4682-87d6-449835ea4de6", + "readme": "string", + "template_id": "c6d67e98-83ea-49f0-8812-e4abae2b68bc", + "updated_at": "2019-08-24T14:15:22Z" +} +``` + +### Responses + +| Status | Meaning | Description | Schema | +| ------ | ------------------------------------------------------- | ----------- | -------------------------------------------------------------- | +| 200 | [OK](https://tools.ietf.org/html/rfc7231#section-6.3.1) | OK | [codersdk.TemplateVersion](schemas.md#codersdktemplateversion) | + +To perform this operation, you must be authenticated. [Learn more](authentication.md). + ## Cancel template version by ID ### Code samples diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index 4b31afb1f0ee0..c1d87bf1d1f53 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -577,6 +577,11 @@ export interface PatchGroupRequest { readonly quota_allowance?: number } +// From codersdk/templateversions.go +export interface PatchTemplateVersionRequest { + readonly name: string +} + // From codersdk/deployment.go export interface PprofConfig { readonly enable: boolean From 73aa2dd6bd728dfaf927504a5167770d94a4d7b2 Mon Sep 17 00:00:00 2001 From: Bruno Quaresma Date: Tue, 21 Mar 2023 17:22:57 +0000 Subject: [PATCH 02/11] Update metadata --- coderd/apidoc/docs.go | 17 +++++++++++++++++ coderd/apidoc/swagger.json | 17 +++++++++++++++++ coderd/coderd.go | 1 + coderd/templateversions.go | 1 + docs/api/schemas.md | 14 ++++++++++++++ docs/api/templates.md | 16 +++++++++++++--- 6 files changed, 63 insertions(+), 3 deletions(-) diff --git a/coderd/apidoc/docs.go b/coderd/apidoc/docs.go index 40a48c9dda9be..6fd8e4bc7522b 100644 --- a/coderd/apidoc/docs.go +++ b/coderd/apidoc/docs.go @@ -2278,6 +2278,15 @@ const docTemplate = `{ "name": "templateversion", "in": "path", "required": true + }, + { + "description": "Patch template version request", + "name": "request", + "in": "body", + "required": true, + "schema": { + "$ref": "#/definitions/codersdk.PatchTemplateVersionRequest" + } } ], "responses": { @@ -7427,6 +7436,14 @@ const docTemplate = `{ "ParameterSourceSchemeData" ] }, + "codersdk.PatchTemplateVersionRequest": { + "type": "object", + "properties": { + "name": { + "type": "string" + } + } + }, "codersdk.PprofConfig": { "type": "object", "properties": { diff --git a/coderd/apidoc/swagger.json b/coderd/apidoc/swagger.json index 4e6bbfe3b3f29..7510423a98742 100644 --- a/coderd/apidoc/swagger.json +++ b/coderd/apidoc/swagger.json @@ -1990,6 +1990,15 @@ "name": "templateversion", "in": "path", "required": true + }, + { + "description": "Patch template version request", + "name": "request", + "in": "body", + "required": true, + "schema": { + "$ref": "#/definitions/codersdk.PatchTemplateVersionRequest" + } } ], "responses": { @@ -6643,6 +6652,14 @@ "ParameterSourceSchemeData" ] }, + "codersdk.PatchTemplateVersionRequest": { + "type": "object", + "properties": { + "name": { + "type": "string" + } + } + }, "codersdk.PprofConfig": { "type": "object", "properties": { diff --git a/coderd/coderd.go b/coderd/coderd.go index ac277d057bb71..4e86a3f40ce17 100644 --- a/coderd/coderd.go +++ b/coderd/coderd.go @@ -495,6 +495,7 @@ func New(options *Options) *API { httpmw.ExtractTemplateVersionParam(options.Database), ) r.Get("/", api.templateVersion) + r.Patch("/", api.patchTemplateVersion) r.Patch("/cancel", api.patchCancelTemplateVersion) r.Get("/schema", api.templateVersionSchema) r.Get("/parameters", api.templateVersionParameters) diff --git a/coderd/templateversions.go b/coderd/templateversions.go index c8ed665aa1d4b..94b26524898e9 100644 --- a/coderd/templateversions.go +++ b/coderd/templateversions.go @@ -69,6 +69,7 @@ func (api *API) templateVersion(rw http.ResponseWriter, r *http.Request) { // @Produce json // @Tags Templates // @Param templateversion path string true "Template version ID" format(uuid) +// @Param request body codersdk.PatchTemplateVersionRequest true "Patch template version request" // @Success 200 {object} codersdk.TemplateVersion // @Router /templateversions/{templateversion} [patch] func (api *API) patchTemplateVersion(rw http.ResponseWriter, r *http.Request) { diff --git a/docs/api/schemas.md b/docs/api/schemas.md index 441feb2201b44..8c030a175aa6a 100644 --- a/docs/api/schemas.md +++ b/docs/api/schemas.md @@ -3002,6 +3002,20 @@ Parameter represents a set value for the scope. | `none` | | `data` | +## codersdk.PatchTemplateVersionRequest + +```json +{ + "name": "string" +} +``` + +### Properties + +| Name | Type | Required | Restrictions | Description | +| ------ | ------ | -------- | ------------ | ----------- | +| `name` | string | false | | | + ## codersdk.PprofConfig ```json diff --git a/docs/api/templates.md b/docs/api/templates.md index 38dcef4d39a0e..7953da485e25a 100644 --- a/docs/api/templates.md +++ b/docs/api/templates.md @@ -1257,17 +1257,27 @@ To perform this operation, you must be authenticated. [Learn more](authenticatio ```shell # Example request using curl curl -X PATCH http://coder-server:8080/api/v2/templateversions/{templateversion} \ + -H 'Content-Type: application/json' \ -H 'Accept: application/json' \ -H 'Coder-Session-Token: API_KEY' ``` `PATCH /templateversions/{templateversion}` +> Body parameter + +```json +{ + "name": "string" +} +``` + ### Parameters -| Name | In | Type | Required | Description | -| ----------------- | ---- | ------------ | -------- | ------------------- | -| `templateversion` | path | string(uuid) | true | Template version ID | +| Name | In | Type | Required | Description | +| ----------------- | ---- | -------------------------------------------------------------------------------------- | -------- | ------------------------------ | +| `templateversion` | path | string(uuid) | true | Template version ID | +| `body` | body | [codersdk.PatchTemplateVersionRequest](schemas.md#codersdkpatchtemplateversionrequest) | true | Patch template version request | ### Example responses From 7a39d3476f8bb891d9c03e509aaf4a2771329f8e Mon Sep 17 00:00:00 2001 From: Bruno Quaresma Date: Tue, 21 Mar 2023 19:23:17 +0000 Subject: [PATCH 03/11] Add tests --- coderd/templateversions.go | 3 +++ coderd/templateversions_test.go | 35 +++++++++++++++++++++++++++++++++ codersdk/deployment.go | 3 +-- codersdk/templateversions.go | 11 ++++++----- 4 files changed, 45 insertions(+), 7 deletions(-) diff --git a/coderd/templateversions.go b/coderd/templateversions.go index 94b26524898e9..f6f4fab756db7 100644 --- a/coderd/templateversions.go +++ b/coderd/templateversions.go @@ -79,6 +79,9 @@ func (api *API) patchTemplateVersion(rw http.ResponseWriter, r *http.Request) { if !httpapi.Read(ctx, rw, r, ¶ms) { return } + if params.Name == "" { + params.Name = templateVersion.Name + } templateVersion, err := api.Database.UpdateTemplateVersionByID(ctx, database.UpdateTemplateVersionByIDParams{ ID: templateVersion.ID, Name: params.Name, diff --git a/coderd/templateversions_test.go b/coderd/templateversions_test.go index 176c338f5ed1c..0d8d3050de6a3 100644 --- a/coderd/templateversions_test.go +++ b/coderd/templateversions_test.go @@ -1334,3 +1334,38 @@ func TestTemplateVersionVariables(t *testing.T) { require.Equal(t, "*redacted*", actualVariables[0].Value) }) } + +func TestTemplateVersionPatch(t *testing.T) { + t.Parallel() + t.Run("Update the name", func(t *testing.T) { + t.Parallel() + client := coderdtest.New(t, nil) + user := coderdtest.CreateFirstUser(t, client) + version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil) + + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) + defer cancel() + + updatedVersion, err := client.UpdateTemplateVersion(ctx, version.ID, codersdk.PatchTemplateVersionRequest{ + Name: "new name", + }) + + require.NoError(t, err) + require.Equal(t, updatedVersion.Name, "new name") + }) + + t.Run("Use the same name if a new name is not passed", func(t *testing.T) { + t.Parallel() + client := coderdtest.New(t, nil) + user := coderdtest.CreateFirstUser(t, client) + version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil) + + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) + defer cancel() + + updatedVersion, err := client.UpdateTemplateVersion(ctx, version.ID, codersdk.PatchTemplateVersionRequest{}) + + require.NoError(t, err) + require.Equal(t, updatedVersion.Name, version.Name) + }) +} diff --git a/codersdk/deployment.go b/codersdk/deployment.go index bb4292e929f14..7bffe59656de9 100644 --- a/codersdk/deployment.go +++ b/codersdk/deployment.go @@ -4,7 +4,6 @@ import ( "context" "encoding/json" "flag" - "fmt" "math" "net/http" "os" @@ -199,7 +198,7 @@ func ParseSSHConfigOption(opt string) (key string, value string, err error) { return r == ' ' || r == '=' }) if idx == -1 { - return "", "", xerrors.New(fmt.Sprintf("invalid config-ssh option %q", opt)) + return "", "", xerrors.Errorf("invalid config-ssh option %q", opt) } return opt[:idx], opt[idx+1:], nil } diff --git a/codersdk/templateversions.go b/codersdk/templateversions.go index e5dc1eaad7e14..2db999cbd5022 100644 --- a/codersdk/templateversions.go +++ b/codersdk/templateversions.go @@ -296,14 +296,15 @@ func (c *Client) PreviousTemplateVersion(ctx context.Context, organization uuid. return version, json.NewDecoder(res.Body).Decode(&version) } -func (c *Client) UpdateTemplateVersion(ctx context.Context, version, req PatchTemplateVersionRequest) error { - res, err := c.Request(ctx, http.MethodPatch, fmt.Sprintf("/api/v2/templateversions/%s", version), req) +func (c *Client) UpdateTemplateVersion(ctx context.Context, versionId uuid.UUID, req PatchTemplateVersionRequest) (TemplateVersion, error) { + res, err := c.Request(ctx, http.MethodPatch, fmt.Sprintf("/api/v2/templateversions/%s", versionId), req) if err != nil { - return err + return TemplateVersion{}, err } defer res.Body.Close() if res.StatusCode != http.StatusNoContent { - return ReadBodyAsError(res) + return TemplateVersion{}, ReadBodyAsError(res) } - return nil + var version TemplateVersion + return version, json.NewDecoder(res.Body).Decode(&version) } From 7be61cdc93ccf92a39672e5e8446b16d70265b2b Mon Sep 17 00:00:00 2001 From: Bruno Quaresma Date: Tue, 21 Mar 2023 19:44:34 +0000 Subject: [PATCH 04/11] Fix a few tests --- coderd/database/dbauthz/querier_test.go | 2 +- coderd/templateversions.go | 1 + codersdk/templateversions.go | 4 ++-- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/coderd/database/dbauthz/querier_test.go b/coderd/database/dbauthz/querier_test.go index cb746cb81d514..649a3e6ba559f 100644 --- a/coderd/database/dbauthz/querier_test.go +++ b/coderd/database/dbauthz/querier_test.go @@ -721,7 +721,7 @@ func (s *MethodTestSuite) TestTemplate() { check.Args(database.UpdateTemplateVersionByIDParams{ ID: tv.ID, TemplateID: uuid.NullUUID{UUID: t1.ID, Valid: true}, - }).Asserts(t1, rbac.ActionUpdate).Returns() + }).Asserts(t1, rbac.ActionUpdate).Returns(tv) })) s.Run("UpdateTemplateVersionDescriptionByJobID", s.Subtest(func(db database.Store, check *expects) { jobID := uuid.New() diff --git a/coderd/templateversions.go b/coderd/templateversions.go index f6f4fab756db7..aa9dcf722e36f 100644 --- a/coderd/templateversions.go +++ b/coderd/templateversions.go @@ -66,6 +66,7 @@ func (api *API) templateVersion(rw http.ResponseWriter, r *http.Request) { // @Summary Patch template version by ID // @ID patch-template-version-by-id // @Security CoderSessionToken +// @Accept json // @Produce json // @Tags Templates // @Param templateversion path string true "Template version ID" format(uuid) diff --git a/codersdk/templateversions.go b/codersdk/templateversions.go index 2db999cbd5022..d0de12bedc45b 100644 --- a/codersdk/templateversions.go +++ b/codersdk/templateversions.go @@ -296,8 +296,8 @@ func (c *Client) PreviousTemplateVersion(ctx context.Context, organization uuid. return version, json.NewDecoder(res.Body).Decode(&version) } -func (c *Client) UpdateTemplateVersion(ctx context.Context, versionId uuid.UUID, req PatchTemplateVersionRequest) (TemplateVersion, error) { - res, err := c.Request(ctx, http.MethodPatch, fmt.Sprintf("/api/v2/templateversions/%s", versionId), req) +func (c *Client) UpdateTemplateVersion(ctx context.Context, versionID uuid.UUID, req PatchTemplateVersionRequest) (TemplateVersion, error) { + res, err := c.Request(ctx, http.MethodPatch, fmt.Sprintf("/api/v2/templateversions/%s", versionID), req) if err != nil { return TemplateVersion{}, err } From 1d349423fc6f58d4768c061034c9e13357b5b610 Mon Sep 17 00:00:00 2001 From: Bruno Quaresma Date: Tue, 21 Mar 2023 19:52:03 +0000 Subject: [PATCH 05/11] Fix gen --- coderd/apidoc/docs.go | 3 +++ coderd/apidoc/swagger.json | 1 + 2 files changed, 4 insertions(+) diff --git a/coderd/apidoc/docs.go b/coderd/apidoc/docs.go index 6fd8e4bc7522b..da1e7b6430818 100644 --- a/coderd/apidoc/docs.go +++ b/coderd/apidoc/docs.go @@ -2262,6 +2262,9 @@ const docTemplate = `{ "CoderSessionToken": [] } ], + "consumes": [ + "application/json" + ], "produces": [ "application/json" ], diff --git a/coderd/apidoc/swagger.json b/coderd/apidoc/swagger.json index 7510423a98742..5198b54c77662 100644 --- a/coderd/apidoc/swagger.json +++ b/coderd/apidoc/swagger.json @@ -1978,6 +1978,7 @@ "CoderSessionToken": [] } ], + "consumes": ["application/json"], "produces": ["application/json"], "tags": ["Templates"], "summary": "Patch template version by ID", From 7d0b0c96961078da670162dd9ec203d1298ce4b6 Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Wed, 22 Mar 2023 15:16:30 +0100 Subject: [PATCH 06/11] WIP --- coderd/database/dbauthz/querier.go | 7 +++-- coderd/database/dbauthz/querier_test.go | 2 ++ coderd/templateversions.go | 39 ++++++++++++++++++++----- coderd/templateversions_test.go | 11 ++++--- codersdk/templateversions.go | 2 +- 5 files changed, 47 insertions(+), 14 deletions(-) diff --git a/coderd/database/dbauthz/querier.go b/coderd/database/dbauthz/querier.go index 42b4cb87e81d6..019e4f9aefb7d 100644 --- a/coderd/database/dbauthz/querier.go +++ b/coderd/database/dbauthz/querier.go @@ -851,11 +851,14 @@ func (q *querier) UpdateTemplateScheduleByID(ctx context.Context, arg database.U } func (q *querier) UpdateTemplateVersionByID(ctx context.Context, arg database.UpdateTemplateVersionByIDParams) (database.TemplateVersion, error) { - template, err := q.db.GetTemplateByID(ctx, arg.TemplateID.UUID) + // Must do an authorized fetch to prevent leaking template ids this way. + tpl, err := q.GetTemplateByID(ctx, arg.TemplateID.UUID) if err != nil { return database.TemplateVersion{}, err } - if err := q.authorizeContext(ctx, rbac.ActionUpdate, template); err != nil { + // Check the create permission on the template. + err = q.authorizeContext(ctx, rbac.ActionUpdate, tpl) + if err != nil { return database.TemplateVersion{}, err } return q.db.UpdateTemplateVersionByID(ctx, arg) diff --git a/coderd/database/dbauthz/querier_test.go b/coderd/database/dbauthz/querier_test.go index 649a3e6ba559f..4596c09693eae 100644 --- a/coderd/database/dbauthz/querier_test.go +++ b/coderd/database/dbauthz/querier_test.go @@ -721,6 +721,8 @@ func (s *MethodTestSuite) TestTemplate() { check.Args(database.UpdateTemplateVersionByIDParams{ ID: tv.ID, TemplateID: uuid.NullUUID{UUID: t1.ID, Valid: true}, + Name: tv.Name, + UpdatedAt: tv.UpdatedAt, }).Asserts(t1, rbac.ActionUpdate).Returns(tv) })) s.Run("UpdateTemplateVersionDescriptionByJobID", s.Subtest(func(db database.Store, check *expects) { diff --git a/coderd/templateversions.go b/coderd/templateversions.go index aa9dcf722e36f..39e27361aec04 100644 --- a/coderd/templateversions.go +++ b/coderd/templateversions.go @@ -80,13 +80,19 @@ func (api *API) patchTemplateVersion(rw http.ResponseWriter, r *http.Request) { if !httpapi.Read(ctx, rw, r, ¶ms) { return } - if params.Name == "" { - params.Name = templateVersion.Name + + updateParams := database.UpdateTemplateVersionByIDParams{ + ID: templateVersion.ID, + TemplateID: templateVersion.TemplateID, + UpdatedAt: database.Now(), + Name: templateVersion.Name, } - templateVersion, err := api.Database.UpdateTemplateVersionByID(ctx, database.UpdateTemplateVersionByIDParams{ - ID: templateVersion.ID, - Name: params.Name, - }) + + if params.Name != "" { + updateParams.Name = params.Name + } + // It is not allowed to "patch" the template ID, and reassign it. + updatedTemplateVersion, err := api.Database.UpdateTemplateVersionByID(ctx, updateParams) if err != nil { httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ Message: "Error on patching template version.", @@ -94,7 +100,26 @@ func (api *API) patchTemplateVersion(rw http.ResponseWriter, r *http.Request) { }) return } - httpapi.Write(ctx, rw, http.StatusNoContent, templateVersion) + + job, err := api.Database.GetProvisionerJobByID(ctx, templateVersion.JobID) + if err != nil { + httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ + Message: "Internal error fetching provisioner job.", + Detail: err.Error(), + }) + return + } + + user, err := api.Database.GetUserByID(ctx, templateVersion.CreatedBy) + if err != nil { + httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ + Message: "Internal error on fetching user.", + Detail: err.Error(), + }) + return + } + + httpapi.Write(ctx, rw, http.StatusOK, convertTemplateVersion(updatedTemplateVersion, convertProvisionerJob(job), user)) } // @Summary Cancel template version by ID diff --git a/coderd/templateversions_test.go b/coderd/templateversions_test.go index 0d8d3050de6a3..25bcaac125893 100644 --- a/coderd/templateversions_test.go +++ b/coderd/templateversions_test.go @@ -1342,16 +1342,19 @@ func TestTemplateVersionPatch(t *testing.T) { client := coderdtest.New(t, nil) user := coderdtest.CreateFirstUser(t, client) version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil) + coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID) ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) defer cancel() + const newName = "new_name" updatedVersion, err := client.UpdateTemplateVersion(ctx, version.ID, codersdk.PatchTemplateVersionRequest{ - Name: "new name", + Name: newName, }) require.NoError(t, err) - require.Equal(t, updatedVersion.Name, "new name") + assert.Equal(t, newName, updatedVersion.Name) + assert.NotEqual(t, updatedVersion.Name, version.Name) }) t.Run("Use the same name if a new name is not passed", func(t *testing.T) { @@ -1359,13 +1362,13 @@ func TestTemplateVersionPatch(t *testing.T) { client := coderdtest.New(t, nil) user := coderdtest.CreateFirstUser(t, client) version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil) + coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID) ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) defer cancel() updatedVersion, err := client.UpdateTemplateVersion(ctx, version.ID, codersdk.PatchTemplateVersionRequest{}) - require.NoError(t, err) - require.Equal(t, updatedVersion.Name, version.Name) + assert.Equal(t, version.Name, updatedVersion.Name) }) } diff --git a/codersdk/templateversions.go b/codersdk/templateversions.go index d0de12bedc45b..d82f86a2c38b2 100644 --- a/codersdk/templateversions.go +++ b/codersdk/templateversions.go @@ -302,7 +302,7 @@ func (c *Client) UpdateTemplateVersion(ctx context.Context, versionID uuid.UUID, return TemplateVersion{}, err } defer res.Body.Close() - if res.StatusCode != http.StatusNoContent { + if res.StatusCode != http.StatusOK { return TemplateVersion{}, ReadBodyAsError(res) } var version TemplateVersion From a480655e19250e733a0290158888d9dde48fe960 Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Wed, 22 Mar 2023 15:32:48 +0100 Subject: [PATCH 07/11] WIP --- coderd/templateversions.go | 1 + 1 file changed, 1 insertion(+) diff --git a/coderd/templateversions.go b/coderd/templateversions.go index 39e27361aec04..023e185907349 100644 --- a/coderd/templateversions.go +++ b/coderd/templateversions.go @@ -76,6 +76,7 @@ func (api *API) templateVersion(rw http.ResponseWriter, r *http.Request) { func (api *API) patchTemplateVersion(rw http.ResponseWriter, r *http.Request) { ctx := r.Context() templateVersion := httpmw.TemplateVersionParam(r) + var params codersdk.PatchTemplateVersionRequest if !httpapi.Read(ctx, rw, r, ¶ms) { return From 9b351429c2839921a201a1264fede3d324aa5f1e Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Wed, 22 Mar 2023 16:18:02 +0100 Subject: [PATCH 08/11] Fix: newTemplateVersion --- coderd/templates.go | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/coderd/templates.go b/coderd/templates.go index 9ebc661e45a4a..f17df9c1f907f 100644 --- a/coderd/templates.go +++ b/coderd/templates.go @@ -269,22 +269,18 @@ func (api *API) postTemplateByOrganization(rw http.ResponseWriter, r *http.Reque templateAudit.New = dbTemplate - _, err = tx.UpdateTemplateVersionByID(ctx, database.UpdateTemplateVersionByIDParams{ + newTemplateVersion, err := tx.UpdateTemplateVersionByID(ctx, database.UpdateTemplateVersionByIDParams{ ID: templateVersion.ID, TemplateID: uuid.NullUUID{ UUID: dbTemplate.ID, Valid: true, }, UpdatedAt: database.Now(), + Name: templateVersion.Name, }) if err != nil { return xerrors.Errorf("insert template version: %s", err) } - newTemplateVersion := templateVersion - newTemplateVersion.TemplateID = uuid.NullUUID{ - UUID: dbTemplate.ID, - Valid: true, - } templateVersionAudit.New = newTemplateVersion for _, parameterValue := range createTemplate.ParameterValues { From e5d6642771cda5c8993c225dcdbac6bda6920677 Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Wed, 22 Mar 2023 16:22:47 +0100 Subject: [PATCH 09/11] Fix --- coderd/templates.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/coderd/templates.go b/coderd/templates.go index f17df9c1f907f..ac28814eda9dc 100644 --- a/coderd/templates.go +++ b/coderd/templates.go @@ -269,7 +269,7 @@ func (api *API) postTemplateByOrganization(rw http.ResponseWriter, r *http.Reque templateAudit.New = dbTemplate - newTemplateVersion, err := tx.UpdateTemplateVersionByID(ctx, database.UpdateTemplateVersionByIDParams{ + _, err = tx.UpdateTemplateVersionByID(ctx, database.UpdateTemplateVersionByIDParams{ ID: templateVersion.ID, TemplateID: uuid.NullUUID{ UUID: dbTemplate.ID, @@ -281,6 +281,11 @@ func (api *API) postTemplateByOrganization(rw http.ResponseWriter, r *http.Reque if err != nil { return xerrors.Errorf("insert template version: %s", err) } + newTemplateVersion := templateVersion + newTemplateVersion.TemplateID = uuid.NullUUID{ + UUID: dbTemplate.ID, + Valid: true, + } templateVersionAudit.New = newTemplateVersion for _, parameterValue := range createTemplate.ParameterValues { From 41bc1dd0a484f41e12ddc6b87b78a47a355f123e Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Wed, 22 Mar 2023 16:29:42 +0100 Subject: [PATCH 10/11] Fix --- coderd/templateversions_test.go | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/coderd/templateversions_test.go b/coderd/templateversions_test.go index 25bcaac125893..6e3742df23c1b 100644 --- a/coderd/templateversions_test.go +++ b/coderd/templateversions_test.go @@ -1371,4 +1371,31 @@ func TestTemplateVersionPatch(t *testing.T) { require.NoError(t, err) assert.Equal(t, version.Name, updatedVersion.Name) }) + + t.Run("Use the same name for two different templates", func(t *testing.T) { + t.Parallel() + client := coderdtest.New(t, nil) + user := coderdtest.CreateFirstUser(t, client) + version1 := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil) + coderdtest.CreateTemplate(t, client, user.OrganizationID, version1.ID) + version2 := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil) + coderdtest.CreateTemplate(t, client, user.OrganizationID, version2.ID) + + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) + defer cancel() + + const commonTemplateVersionName = "common-template-version-name" + updatedVersion1, err := client.UpdateTemplateVersion(ctx, version1.ID, codersdk.PatchTemplateVersionRequest{ + Name: commonTemplateVersionName, + }) + require.NoError(t, err) + + updatedVersion2, err := client.UpdateTemplateVersion(ctx, version2.ID, codersdk.PatchTemplateVersionRequest{ + Name: commonTemplateVersionName, + }) + require.NoError(t, err) + + assert.NotEqual(t, updatedVersion1.ID, updatedVersion2.ID) + assert.Equal(t, updatedVersion1.Name, updatedVersion2.Name) + }) } From c792dca29369882d1a2b7170c7edd3b5615d0e0e Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Wed, 22 Mar 2023 16:37:03 +0100 Subject: [PATCH 11/11] Fix: RBAC --- coderd/database/dbauthz/querier.go | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/coderd/database/dbauthz/querier.go b/coderd/database/dbauthz/querier.go index 019e4f9aefb7d..42b4cb87e81d6 100644 --- a/coderd/database/dbauthz/querier.go +++ b/coderd/database/dbauthz/querier.go @@ -851,14 +851,11 @@ func (q *querier) UpdateTemplateScheduleByID(ctx context.Context, arg database.U } func (q *querier) UpdateTemplateVersionByID(ctx context.Context, arg database.UpdateTemplateVersionByIDParams) (database.TemplateVersion, error) { - // Must do an authorized fetch to prevent leaking template ids this way. - tpl, err := q.GetTemplateByID(ctx, arg.TemplateID.UUID) + template, err := q.db.GetTemplateByID(ctx, arg.TemplateID.UUID) if err != nil { return database.TemplateVersion{}, err } - // Check the create permission on the template. - err = q.authorizeContext(ctx, rbac.ActionUpdate, tpl) - if err != nil { + if err := q.authorizeContext(ctx, rbac.ActionUpdate, template); err != nil { return database.TemplateVersion{}, err } return q.db.UpdateTemplateVersionByID(ctx, arg)