synced/coder
1
0
Fork 0
mirror of https://github.com/coder/coder.git synced 2025-07-08 11:39:50 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: index template versions by template and name (#5993)

Browse Source 
* fix: index template versions by template and name

We were incorrectly returning template versions by name relative
to organizations. This could result in an incorrect version being
returned if multiple templates had versions with the same name.

* Fix auth referencing

* Fix route location

* Fix authorize route name

* Fix previous call

* Fix authorize route name
This commit is contained in:
Kyle Carberry
2023-02-02 15:47:53 -06:00
committed by GitHub
parent ea7e55fcf9
commit a5e8911d67
17 changed files with 451 additions and 417 deletions
Download Patch File Download Diff File Expand all files Collapse all files

182
coderd/apidoc/docs.go generated
View File

@ -1330,6 +1330,104 @@ const docTemplate = `{
}
}
},
"/organizations/{organization}/templates/{templatename}/versions/{templateversionname}": {
"get": {
"security": [
{
"CoderSessionToken": []
}
],
"produces": [
"application/json"
],
"tags": [
"Templates"
],
"summary": "Get template version by organization, template, and name",
"operationId": "get-template-version-by-organization-template-and-name",
"parameters": [
{
"type": "string",
"format": "uuid",
"description": "Organization ID",
"name": "organization",
"in": "path",
"required": true
},
{
"type": "string",
"description": "Template name",
"name": "templatename",
"in": "path",
"required": true
},
{
"type": "string",
"description": "Template version name",
"name": "templateversionname",
"in": "path",
"required": true
}
],
"responses": {
"200": {
"description": "OK",
"schema": {
"$ref": "#/definitions/codersdk.TemplateVersion"
}
}
}
}
},
"/organizations/{organization}/templates/{templatename}/versions/{templateversionname}/previous": {
"get": {
"security": [
{
"CoderSessionToken": []
}
],
"produces": [
"application/json"
],
"tags": [
"Templates"
],
"summary": "Get previous template version by organization, template, and name",
"operationId": "get-previous-template-version-by-organization-template-and-name",
"parameters": [
{
"type": "string",
"format": "uuid",
"description": "Organization ID",
"name": "organization",
"in": "path",
"required": true
},
{
"type": "string",
"description": "Template name",
"name": "templatename",
"in": "path",
"required": true
},
{
"type": "string",
"description": "Template version name",
"name": "templateversionname",
"in": "path",
"required": true
}
],
"responses": {
"200": {
"description": "OK",
"schema": {
"$ref": "#/definitions/codersdk.TemplateVersion"
}
}
}
}
},
"/organizations/{organization}/templateversions": {
"post": {
"security": [
@ -1377,90 +1475,6 @@ const docTemplate = `{
}
}
},
"/organizations/{organization}/templateversions/{templateversionname}": {
"get": {
"security": [
{
"CoderSessionToken": []
}
],
"produces": [
"application/json"
],
"tags": [
"Templates"
],
"summary": "Get template version by organization and name",
"operationId": "get-template-version-by-organization-and-name",
"parameters": [
{
"type": "string",
"format": "uuid",
"description": "Organization ID",
"name": "organization",
"in": "path",
"required": true
},
{
"type": "string",
"description": "Template version name",
"name": "templateversionname",
"in": "path",
"required": true
}
],
"responses": {
"200": {
"description": "OK",
"schema": {
"$ref": "#/definitions/codersdk.TemplateVersion"
}
}
}
}
},
"/organizations/{organization}/templateversions/{templateversionname}/previous": {
"get": {
"security": [
{
"CoderSessionToken": []
}
],
"produces": [
"application/json"
],
"tags": [
"Templates"
],
"summary": "Get previous template version by organization and name",
"operationId": "get-previous-template-version-by-organization-and-name",
"parameters": [
{
"type": "string",
"format": "uuid",
"description": "Organization ID",
"name": "organization",
"in": "path",
"required": true
},
{
"type": "string",
"description": "Template version name",
"name": "templateversionname",
"in": "path",
"required": true
}
],
"responses": {
"200": {
"description": "OK",
"schema": {
"$ref": "#/definitions/codersdk.TemplateVersion"
}
}
}
}
},
"/parameters/{scope}/{id}": {
"get": {
"security": [

166
coderd/apidoc/swagger.json generated
View File

@ -1160,6 +1160,96 @@
}
}
},
"/organizations/{organization}/templates/{templatename}/versions/{templateversionname}": {
"get": {
"security": [
{
"CoderSessionToken": []
}
],
"produces": ["application/json"],
"tags": ["Templates"],
"summary": "Get template version by organization, template, and name",
"operationId": "get-template-version-by-organization-template-and-name",
"parameters": [
{
"type": "string",
"format": "uuid",
"description": "Organization ID",
"name": "organization",
"in": "path",
"required": true
},
{
"type": "string",
"description": "Template name",
"name": "templatename",
"in": "path",
"required": true
},
{
"type": "string",
"description": "Template version name",
"name": "templateversionname",
"in": "path",
"required": true
}
],
"responses": {
"200": {
"description": "OK",
"schema": {
"$ref": "#/definitions/codersdk.TemplateVersion"
}
}
}
}
},
"/organizations/{organization}/templates/{templatename}/versions/{templateversionname}/previous": {
"get": {
"security": [
{
"CoderSessionToken": []
}
],
"produces": ["application/json"],
"tags": ["Templates"],
"summary": "Get previous template version by organization, template, and name",
"operationId": "get-previous-template-version-by-organization-template-and-name",
"parameters": [
{
"type": "string",
"format": "uuid",
"description": "Organization ID",
"name": "organization",
"in": "path",
"required": true
},
{
"type": "string",
"description": "Template name",
"name": "templatename",
"in": "path",
"required": true
},
{
"type": "string",
"description": "Template version name",
"name": "templateversionname",
"in": "path",
"required": true
}
],
"responses": {
"200": {
"description": "OK",
"schema": {
"$ref": "#/definitions/codersdk.TemplateVersion"
}
}
}
}
},
"/organizations/{organization}/templateversions": {
"post": {
"security": [
@ -1201,82 +1291,6 @@
}
}
},
"/organizations/{organization}/templateversions/{templateversionname}": {
"get": {
"security": [
{
"CoderSessionToken": []
}
],
"produces": ["application/json"],
"tags": ["Templates"],
"summary": "Get template version by organization and name",
"operationId": "get-template-version-by-organization-and-name",
"parameters": [
{
"type": "string",
"format": "uuid",
"description": "Organization ID",
"name": "organization",
"in": "path",
"required": true
},
{
"type": "string",
"description": "Template version name",
"name": "templateversionname",
"in": "path",
"required": true
}
],
"responses": {
"200": {
"description": "OK",
"schema": {
"$ref": "#/definitions/codersdk.TemplateVersion"
}
}
}
}
},
"/organizations/{organization}/templateversions/{templateversionname}/previous": {
"get": {
"security": [
{
"CoderSessionToken": []
}
],
"produces": ["application/json"],
"tags": ["Templates"],
"summary": "Get previous template version by organization and name",
"operationId": "get-previous-template-version-by-organization-and-name",
"parameters": [
{
"type": "string",
"format": "uuid",
"description": "Organization ID",
"name": "organization",
"in": "path",
"required": true
},
{
"type": "string",
"description": "Template version name",
"name": "templateversionname",
"in": "path",
"required": true
}
],
"responses": {
"200": {
"description": "OK",
"schema": {
"$ref": "#/definitions/codersdk.TemplateVersion"
}
}
}
}
},
"/parameters/{scope}/{id}": {
"get": {
"security": [

14
coderd/coderd.go
View File

@ -398,16 +398,18 @@ func New(options *Options) *API {
httpmw.ExtractOrganizationParam(options.Database),
)
r.Get("/", api.organization)
r.Route("/templateversions", func(r chi.Router) {
r.Post("/", api.postTemplateVersionsByOrganization)
r.Get("/{templateversionname}", api.templateVersionByOrganizationAndName)
r.Get("/{templateversionname}/previous", api.previousTemplateVersionByOrganizationAndName)
})
r.Post("/templateversions", api.postTemplateVersionsByOrganization)
r.Route("/templates", func(r chi.Router) {
r.Post("/", api.postTemplateByOrganization)
r.Get("/", api.templatesByOrganization)
r.Get("/{templatename}", api.templateByOrganizationAndName)
r.Get("/examples", api.templateExamples)
r.Route("/{templatename}", func(r chi.Router) {
r.Get("/", api.templateByOrganizationAndName)
r.Route("/versions/{templateversionname}", func(r chi.Router) {
r.Get("/", api.templateVersionByOrganizationTemplateAndName)
r.Get("/previous", api.previousTemplateVersionByOrganizationTemplateAndName)
})
})
})
r.Route("/members", func(r chi.Router) {
r.Get("/roles", api.assignableOrgRoles)

18
coderd/coderdtest/authorize.go
View File

@ -239,6 +239,14 @@ func AGPLRoutes(a *AuthTester) (map[string]string, map[string]RouteCheck) {
AssertAction: rbac.ActionRead,
AssertObject: templateObj,
},
"GET:/api/v2/organizations/{organization}/templates/{templatename}/versions/{templateversionname}": {
AssertAction: rbac.ActionRead,
AssertObject: templateObj,
},
"GET:/api/v2/organizations/{organization}/templates/{templatename}/versions/{templateversionname}/previous": {
AssertAction: rbac.ActionRead,
AssertObject: templateObj,
},
"POST:/api/v2/organizations/{organization}/members/{user}/workspaces": {
AssertAction: rbac.ActionCreate,
// No ID when creating
@ -252,12 +260,10 @@ func AGPLRoutes(a *AuthTester) (map[string]string, map[string]RouteCheck) {
"GET:/api/v2/applications/auth-redirect": {AssertAction: rbac.ActionCreate, AssertObject: rbac.ResourceAPIKey},
// These endpoints need payloads to get to the auth part. Payloads will be required
"PUT:/api/v2/users/{user}/roles": {StatusCode: http.StatusBadRequest, NoAuthorize: true},
"PUT:/api/v2/organizations/{organization}/members/{user}/roles": {NoAuthorize: true},
"POST:/api/v2/workspaces/{workspace}/builds": {StatusCode: http.StatusBadRequest, NoAuthorize: true},
"POST:/api/v2/organizations/{organization}/templateversions": {StatusCode: http.StatusBadRequest, NoAuthorize: true},
"GET:/api/v2/organizations/{organization}/templateversions/{templateversionname}": {StatusCode: http.StatusBadRequest, NoAuthorize: true},
"GET:/api/v2/organizations/{organization}/templateversions/{templateversionname}/previous": {StatusCode: http.StatusBadRequest, NoAuthorize: true},
"PUT:/api/v2/users/{user}/roles": {StatusCode: http.StatusBadRequest, NoAuthorize: true},
"PUT:/api/v2/organizations/{organization}/members/{user}/roles": {NoAuthorize: true},
"POST:/api/v2/workspaces/{workspace}/builds": {StatusCode: http.StatusBadRequest, NoAuthorize: true},
"POST:/api/v2/organizations/{organization}/templateversions": {StatusCode: http.StatusBadRequest, NoAuthorize: true},
// Endpoints that use the SQLQuery filter.
"GET:/api/v2/workspaces/": {

20
coderd/database/databasefake/databasefake.go
View File

@ -1797,26 +1797,6 @@ func (q *fakeQuerier) GetTemplateVersionParameters(_ context.Context, templateVe
return parameters, nil
}
func (q *fakeQuerier) GetTemplateVersionByOrganizationAndName(_ context.Context, arg database.GetTemplateVersionByOrganizationAndNameParams) (database.TemplateVersion, error) {
if err := validateDatabaseType(arg); err != nil {
return database.TemplateVersion{}, err
}
q.mutex.RLock()
defer q.mutex.RUnlock()
for _, templateVersion := range q.templateVersions {
if templateVersion.OrganizationID != arg.OrganizationID {
continue
}
if !strings.EqualFold(templateVersion.Name, arg.Name) {
continue
}
return templateVersion, nil
}
return database.TemplateVersion{}, sql.ErrNoRows
}
func (q *fakeQuerier) GetTemplateVersionByID(_ context.Context, templateVersionID uuid.UUID) (database.TemplateVersion, error) {
q.mutex.RLock()
defer q.mutex.RUnlock()

1
coderd/database/querier.go
View File

@ -84,7 +84,6 @@ type sqlcQuerier interface {
GetTemplateDAUs(ctx context.Context, templateID uuid.UUID) ([]GetTemplateDAUsRow, error)
GetTemplateVersionByID(ctx context.Context, id uuid.UUID) (TemplateVersion, error)
GetTemplateVersionByJobID(ctx context.Context, jobID uuid.UUID) (TemplateVersion, error)
GetTemplateVersionByOrganizationAndName(ctx context.Context, arg GetTemplateVersionByOrganizationAndNameParams) (TemplateVersion, error)
GetTemplateVersionByTemplateIDAndName(ctx context.Context, arg GetTemplateVersionByTemplateIDAndNameParams) (TemplateVersion, error)
GetTemplateVersionParameters(ctx context.Context, templateVersionID uuid.UUID) ([]TemplateVersionParameter, error)
GetTemplateVersionsByIDs(ctx context.Context, ids []uuid.UUID) ([]TemplateVersion, error)

32
coderd/database/queries.sql.go
View File

@ -3731,38 +3731,6 @@ func (q *sqlQuerier) GetTemplateVersionByJobID(ctx context.Context, jobID uuid.U
return i, err
}
const getTemplateVersionByOrganizationAndName = `-- name: GetTemplateVersionByOrganizationAndName :one
SELECT
id, template_id, organization_id, created_at, updated_at, name, readme, job_id, created_by
FROM
template_versions
WHERE
organization_id = $1
AND "name" = $2
`
type GetTemplateVersionByOrganizationAndNameParams struct {
OrganizationID uuid.UUID `db:"organization_id" json:"organization_id"`
Name string `db:"name" json:"name"`
}
func (q *sqlQuerier) GetTemplateVersionByOrganizationAndName(ctx context.Context, arg GetTemplateVersionByOrganizationAndNameParams) (TemplateVersion, error) {
row := q.db.QueryRowContext(ctx, getTemplateVersionByOrganizationAndName, arg.OrganizationID, 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,
)
return i, err
}
const getTemplateVersionByTemplateIDAndName = `-- name: GetTemplateVersionByTemplateIDAndName :one
SELECT
id, template_id, organization_id, created_at, updated_at, name, readme, job_id, created_by

9
coderd/database/queries/templateversions.sql
View File

@ -52,15 +52,6 @@ WHERE
template_id = $1
AND "name" = $2;
-- name: GetTemplateVersionByOrganizationAndName :one
SELECT
*
FROM
template_versions
WHERE
organization_id = $1
AND "name" = $2;
-- name: GetTemplateVersionByID :one
SELECT
*

83
coderd/templateversions.go
View File

@ -763,22 +763,50 @@ func (api *API) templateVersionByName(rw http.ResponseWriter, r *http.Request) {
httpapi.Write(ctx, rw, http.StatusOK, convertTemplateVersion(templateVersion, convertProvisionerJob(job), user))
}
// @Summary Get template version by organization and name
// @ID get-template-version-by-organization-and-name
// @Summary Get template version by organization, template, and name
// @ID get-template-version-by-organization-template-and-name
// @Security CoderSessionToken
// @Produce json
// @Tags Templates
// @Param organization path string true "Organization ID" format(uuid)
// @Param templatename path string true "Template name"
// @Param templateversionname path string true "Template version name"
// @Success 200 {object} codersdk.TemplateVersion
// @Router /organizations/{organization}/templateversions/{templateversionname} [get]
func (api *API) templateVersionByOrganizationAndName(rw http.ResponseWriter, r *http.Request) {
// @Router /organizations/{organization}/templates/{templatename}/versions/{templateversionname} [get]
func (api *API) templateVersionByOrganizationTemplateAndName(rw http.ResponseWriter, r *http.Request) {
ctx := r.Context()
organization := httpmw.OrganizationParam(r)
templateVersionName := chi.URLParam(r, "templateversionname")
templateVersion, err := api.Database.GetTemplateVersionByOrganizationAndName(ctx, database.GetTemplateVersionByOrganizationAndNameParams{
templateName := chi.URLParam(r, "templatename")
template, err := api.Database.GetTemplateByOrganizationAndName(ctx, database.GetTemplateByOrganizationAndNameParams{
OrganizationID: organization.ID,
Name: templateVersionName,
Name: templateName,
})
if err != nil {
if errors.Is(err, sql.ErrNoRows) {
httpapi.ResourceNotFound(rw)
return
}
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
Message: "Internal error fetching template.",
Detail: err.Error(),
})
return
}
if !api.Authorize(r, rbac.ActionRead, template) {
httpapi.ResourceNotFound(rw)
return
}
templateVersionName := chi.URLParam(r, "templateversionname")
templateVersion, err := api.Database.GetTemplateVersionByTemplateIDAndName(ctx, database.GetTemplateVersionByTemplateIDAndNameParams{
TemplateID: uuid.NullUUID{
UUID: template.ID,
Valid: true,
},
Name: templateVersionName,
})
if errors.Is(err, sql.ErrNoRows) {
httpapi.Write(ctx, rw, http.StatusNotFound, codersdk.Response{
@ -814,22 +842,49 @@ func (api *API) templateVersionByOrganizationAndName(rw http.ResponseWriter, r *
httpapi.Write(ctx, rw, http.StatusOK, convertTemplateVersion(templateVersion, convertProvisionerJob(job), user))
}
// @Summary Get previous template version by organization and name
// @ID get-previous-template-version-by-organization-and-name
// @Summary Get previous template version by organization, template, and name
// @ID get-previous-template-version-by-organization-template-and-name
// @Security CoderSessionToken
// @Produce json
// @Tags Templates
// @Param organization path string true "Organization ID" format(uuid)
// @Param templatename path string true "Template name"
// @Param templateversionname path string true "Template version name"
// @Success 200 {object} codersdk.TemplateVersion
// @Router /organizations/{organization}/templateversions/{templateversionname}/previous [get]
func (api *API) previousTemplateVersionByOrganizationAndName(rw http.ResponseWriter, r *http.Request) {
// @Router /organizations/{organization}/templates/{templatename}/versions/{templateversionname}/previous [get]
func (api *API) previousTemplateVersionByOrganizationTemplateAndName(rw http.ResponseWriter, r *http.Request) {
ctx := r.Context()
organization := httpmw.OrganizationParam(r)
templateVersionName := chi.URLParam(r, "templateversionname")
templateVersion, err := api.Database.GetTemplateVersionByOrganizationAndName(ctx, database.GetTemplateVersionByOrganizationAndNameParams{
templateName := chi.URLParam(r, "templatename")
template, err := api.Database.GetTemplateByOrganizationAndName(ctx, database.GetTemplateByOrganizationAndNameParams{
OrganizationID: organization.ID,
Name: templateVersionName,
Name: templateName,
})
if err != nil {
if errors.Is(err, sql.ErrNoRows) {
httpapi.ResourceNotFound(rw)
return
}
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
Message: "Internal error fetching template.",
Detail: err.Error(),
})
return
}
if !api.Authorize(r, rbac.ActionRead, template) {
httpapi.ResourceNotFound(rw)
return
}
templateVersionName := chi.URLParam(r, "templateversionname")
templateVersion, err := api.Database.GetTemplateVersionByTemplateIDAndName(ctx, database.GetTemplateVersionByTemplateIDAndNameParams{
TemplateID: uuid.NullUUID{
UUID: template.ID,
Valid: true,
},
Name: templateVersionName,
})
if err != nil {
if xerrors.Is(err, sql.ErrNoRows) {

14
coderd/templateversions_test.go
View File

@ -992,19 +992,19 @@ func TestPaginatedTemplateVersions(t *testing.T) {
}
}
func TestTemplateVersionByOrganizationAndName(t *testing.T) {
func TestTemplateVersionByOrganizationTemplateAndName(t *testing.T) {
t.Parallel()
t.Run("NotFound", func(t *testing.T) {
t.Parallel()
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)
template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID)
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
defer cancel()
_, err := client.TemplateVersionByOrganizationAndName(ctx, user.OrganizationID, "nothing")
_, err := client.TemplateVersionByOrganizationAndName(ctx, user.OrganizationID, template.Name, "nothing")
var apiErr *codersdk.Error
require.ErrorAs(t, err, &apiErr)
require.Equal(t, http.StatusNotFound, apiErr.StatusCode())
@ -1015,12 +1015,12 @@ func TestTemplateVersionByOrganizationAndName(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)
template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID)
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
defer cancel()
_, err := client.TemplateVersionByOrganizationAndName(ctx, user.OrganizationID, version.Name)
_, err := client.TemplateVersionByOrganizationAndName(ctx, user.OrganizationID, template.Name, version.Name)
require.NoError(t, err)
})
}
@ -1048,7 +1048,7 @@ func TestPreviousTemplateVersion(t *testing.T) {
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
defer cancel()
_, err := client.PreviousTemplateVersion(ctx, user.OrganizationID, templateBVersion1.Name)
_, err := client.PreviousTemplateVersion(ctx, user.OrganizationID, templateB.Name, templateBVersion1.Name)
var apiErr *codersdk.Error
require.ErrorAs(t, err, &apiErr)
require.Equal(t, http.StatusNotFound, apiErr.StatusCode())
@ -1075,7 +1075,7 @@ func TestPreviousTemplateVersion(t *testing.T) {
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
defer cancel()
result, err := client.PreviousTemplateVersion(ctx, user.OrganizationID, templateBVersion2.Name)
result, err := client.PreviousTemplateVersion(ctx, user.OrganizationID, templateB.Name, templateBVersion2.Name)
require.NoError(t, err)
require.Equal(t, templateBVersion1.ID, result.ID)
})