Skip to content

Commit 8187992

Browse files
mtojekmtojek
authored
fix: Validate template version name (#6804)
* WIP * Update * Validation
1 parent e0cc4ee commit 8187992

File tree

6 files changed

+105
-10
lines changed

6 files changed

+105
-10
lines changed

‎coderd/database/dbauthz/querier.go

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -859,11 +859,22 @@ func (q *querier) UpdateTemplateScheduleByID(ctx context.Context, arg database.U
859859
}
860860

861861
func (q *querier) UpdateTemplateVersionByID(ctx context.Context, arg database.UpdateTemplateVersionByIDParams) (database.TemplateVersion, error) {
862-
template, err := q.db.GetTemplateByID(ctx, arg.TemplateID.UUID)
862+
// An actor is allowed to update the template version if they are authorized to update the template.
863+
tv, err := q.db.GetTemplateVersionByID(ctx, arg.ID)
863864
if err != nil {
864865
return database.TemplateVersion{}, err
865866
}
866-
if err := q.authorizeContext(ctx, rbac.ActionUpdate, template); err != nil {
867+
var obj rbac.Objecter
868+
if !tv.TemplateID.Valid {
869+
obj = rbac.ResourceTemplate.InOrg(tv.OrganizationID)
870+
} else {
871+
tpl, err := q.db.GetTemplateByID(ctx, tv.TemplateID.UUID)
872+
if err != nil {
873+
return database.TemplateVersion{}, err
874+
}
875+
obj = tpl
876+
}
877+
if err := q.authorizeContext(ctx, rbac.ActionUpdate, obj); err != nil {
867878
return database.TemplateVersion{}, err
868879
}
869880
return q.db.UpdateTemplateVersionByID(ctx, arg)

‎coderd/httpapi/httpapi.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ func init() {
4444
valid := NameValid(str)
4545
return valid == nil
4646
}
47-
for _, tag := range []string{"username", "template_name", "workspace_name"} {
47+
for _, tag := range []string{"username", "template_name", "workspace_name", "template_version_name"} {
4848
err := Validate.RegisterValidation(tag, nameValidator)
4949
if err != nil {
5050
panic(err)

‎coderd/templateversions.go

Lines changed: 35 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -92,12 +92,43 @@ func (api *API) patchTemplateVersion(rw http.ResponseWriter, r *http.Request) {
9292
if params.Name != "" {
9393
updateParams.Name = params.Name
9494
}
95-
// It is not allowed to "patch" the template ID, and reassign it.
96-
updatedTemplateVersion, err := api.Database.UpdateTemplateVersionByID(ctx, updateParams)
95+
96+
errTemplateVersionNameConflict := xerrors.New("template version name must be unique for a template")
97+
98+
var updatedTemplateVersion database.TemplateVersion
99+
err := api.Database.InTx(func(tx database.Store) error {
100+
if templateVersion.TemplateID.Valid && templateVersion.Name != updateParams.Name {
101+
// User wants to rename the template version
102+
103+
_, err := tx.GetTemplateVersionByTemplateIDAndName(ctx, database.GetTemplateVersionByTemplateIDAndNameParams{
104+
TemplateID: templateVersion.TemplateID,
105+
Name: updateParams.Name,
106+
})
107+
if err != nil && !xerrors.Is(err, sql.ErrNoRows) {
108+
return xerrors.Errorf("error on retrieving conflicting template version: %v", err)
109+
}
110+
if err == nil {
111+
return errTemplateVersionNameConflict
112+
}
113+
}
114+
115+
// It is not allowed to "patch" the template ID, and reassign it.
116+
var err error
117+
updatedTemplateVersion, err = tx.UpdateTemplateVersionByID(ctx, updateParams)
118+
if err != nil {
119+
return xerrors.Errorf("error on patching template version: %v", err)
120+
}
121+
return nil
122+
}, nil)
123+
if errors.Is(err, errTemplateVersionNameConflict) {
124+
httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
125+
Message: err.Error(),
126+
})
127+
return
128+
}
97129
if err != nil {
98130
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
99-
Message: "Error on patching template version.",
100-
Detail: err.Error(),
131+
Message: err.Error(),
101132
})
102133
return
103134
}

‎coderd/templateversions_test.go

Lines changed: 54 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1347,7 +1347,7 @@ func TestTemplateVersionPatch(t *testing.T) {
13471347
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
13481348
defer cancel()
13491349

1350-
const newName = "new_name"
1350+
const newName = "new-name"
13511351
updatedVersion, err := client.UpdateTemplateVersion(ctx, version.ID, codersdk.PatchTemplateVersionRequest{
13521352
Name: newName,
13531353
})
@@ -1376,6 +1376,7 @@ func TestTemplateVersionPatch(t *testing.T) {
13761376
t.Parallel()
13771377
client := coderdtest.New(t, nil)
13781378
user := coderdtest.CreateFirstUser(t, client)
1379+
13791380
version1 := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil)
13801381
coderdtest.CreateTemplate(t, client, user.OrganizationID, version1.ID)
13811382
version2 := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil)
@@ -1398,4 +1399,56 @@ func TestTemplateVersionPatch(t *testing.T) {
13981399
assert.NotEqual(t, updatedVersion1.ID, updatedVersion2.ID)
13991400
assert.Equal(t, updatedVersion1.Name, updatedVersion2.Name)
14001401
})
1402+
1403+
t.Run("Use the same name for two versions for the same templates", func(t *testing.T) {
1404+
t.Parallel()
1405+
client := coderdtest.New(t, nil)
1406+
user := coderdtest.CreateFirstUser(t, client)
1407+
version1 := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil)
1408+
template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version1.ID)
1409+
1410+
version2 := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil, func(ctvr *codersdk.CreateTemplateVersionRequest) {
1411+
ctvr.TemplateID = template.ID
1412+
})
1413+
1414+
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
1415+
defer cancel()
1416+
_, err := client.UpdateTemplateVersion(ctx, version2.ID, codersdk.PatchTemplateVersionRequest{
1417+
Name: version1.Name,
1418+
})
1419+
require.Error(t, err)
1420+
})
1421+
1422+
t.Run("Rename the unassigned template", func(t *testing.T) {
1423+
t.Parallel()
1424+
client := coderdtest.New(t, nil)
1425+
user := coderdtest.CreateFirstUser(t, client)
1426+
version1 := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil)
1427+
1428+
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
1429+
defer cancel()
1430+
1431+
const commonTemplateVersionName = "common-template-version-name"
1432+
updatedVersion1, err := client.UpdateTemplateVersion(ctx, version1.ID, codersdk.PatchTemplateVersionRequest{
1433+
Name: commonTemplateVersionName,
1434+
})
1435+
require.NoError(t, err)
1436+
assert.Equal(t, commonTemplateVersionName, updatedVersion1.Name)
1437+
})
1438+
1439+
t.Run("Use incorrect template version name", func(t *testing.T) {
1440+
t.Parallel()
1441+
client := coderdtest.New(t, nil)
1442+
user := coderdtest.CreateFirstUser(t, client)
1443+
version1 := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil)
1444+
1445+
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
1446+
defer cancel()
1447+
1448+
const incorrectTemplateVersionName = "incorrect/name"
1449+
_, err := client.UpdateTemplateVersion(ctx, version1.ID, codersdk.PatchTemplateVersionRequest{
1450+
Name: incorrectTemplateVersionName,
1451+
})
1452+
require.Error(t, err)
1453+
})
14011454
}

‎codersdk/organizations.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ type OrganizationMember struct {
4242

4343
// CreateTemplateVersionRequest enables callers to create a new Template Version.
4444
type CreateTemplateVersionRequest struct {
45-
Name string `json:"name,omitempty" validate:"omitempty,template_name"`
45+
Name string `json:"name,omitempty" validate:"omitempty,template_version_name"`
4646
// TemplateID optionally associates a version with a template.
4747
TemplateID uuid.UUID `json:"template_id,omitempty" format:"uuid"`
4848
StorageMethod ProvisionerStorageMethod `json:"storage_method" validate:"oneof=file,required" enums:"file"`

‎codersdk/templateversions.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ type TemplateVersionVariable struct {
7777
}
7878

7979
type PatchTemplateVersionRequest struct {
80-
Name string `json:"name"`
80+
Name string `json:"name" validate:"omitempty,template_version_name"`
8181
}
8282

8383
// TemplateVersion returns a template version by ID.

0 commit comments

Comments
 (0)