Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit12991ff

Browse files
authored
fix(coderd): change the order of precedence between coder_workspace_tags and request tags (#16119)
This PR switches the order of precedence of workspace tags when postinga template version.Previously, user-specified tags in the request could not override thosedetected from our parsing of the template file. Now, they can do.This addresses a customer issue where were attempting to set a workspacetag via variable.Note: there is a possible follow-up item here where we could pass in theworkspace tag values from the request into `tfparse` and let it takethose user-specified values into account. This is covered in a separatetest.
1 parent071bb26 commit12991ff

File tree

3 files changed

+71
-18
lines changed

3 files changed

+71
-18
lines changed

‎coderd/templateversions.go‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1597,11 +1597,9 @@ func (api *API) postTemplateVersionsByOrganization(rw http.ResponseWriter, r *ht
15971597
}
15981598

15991599
// Ensure the "owner" tag is properly applied in addition to request tags and coder_workspace_tags.
1600-
// Tag order precedence:
1601-
// 1) User-specified tags in the request
1602-
// 2) Tags parsed from coder_workspace_tags data source in template file
1603-
// 2 may clobber 1.
1604-
tags:=provisionersdk.MutateTags(apiKey.UserID,req.ProvisionerTags,parsedTags)
1600+
// User-specified tags in the request will take precedence over tags parsed from `coder_workspace_tags`
1601+
// data sources defined in the template file.
1602+
tags:=provisionersdk.MutateTags(apiKey.UserID,parsedTags,req.ProvisionerTags)
16051603

16061604
vartemplateVersion database.TemplateVersion
16071605
varprovisionerJob database.ProvisionerJob

‎coderd/templateversions_test.go‎

Lines changed: 52 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -355,10 +355,11 @@ func TestPostTemplateVersionsByOrganization(t *testing.T) {
355355
wantTags:map[string]string{"owner":"","scope":"organization","foo":"bar","a":"1","b":"2"},
356356
},
357357
{
358-
name:"main.tf withworkspace tagsand request tags",
358+
name:"main.tf withrequest tagsnot clobbering workspace tags",
359359
files:map[string]string{
360360
`main.tf`:`
361-
// This file is the same as the above, except for this comment.
361+
// This file is, once again, the same as the above, except
362+
// for a slightly different comment.
362363
variable "a" {
363364
type = string
364365
default = "1"
@@ -381,9 +382,57 @@ func TestPostTemplateVersionsByOrganization(t *testing.T) {
381382
}
382383
}`,
383384
},
384-
reqTags:map[string]string{"baz":"zap","foo":"noclobber"},
385+
reqTags:map[string]string{"baz":"zap"},
385386
wantTags:map[string]string{"owner":"","scope":"organization","foo":"bar","baz":"zap","a":"1","b":"2"},
386387
},
388+
{
389+
name:"main.tf with request tags clobbering workspace tags",
390+
files:map[string]string{
391+
`main.tf`:`
392+
// This file is the same as the above, except for this comment.
393+
variable "a" {
394+
type = string
395+
default = "1"
396+
}
397+
data "coder_parameter" "b" {
398+
type = string
399+
default = "2"
400+
}
401+
data "coder_parameter" "unrelated" {
402+
name = "unrelated"
403+
type = "list(string)"
404+
default = jsonencode(["a", "b"])
405+
}
406+
resource "null_resource" "test" {}
407+
data "coder_workspace_tags" "tags" {
408+
tags = {
409+
"foo": "bar",
410+
"a": var.a,
411+
"b": data.coder_parameter.b.value,
412+
}
413+
}`,
414+
},
415+
reqTags:map[string]string{"baz":"zap","foo":"clobbered"},
416+
wantTags:map[string]string{"owner":"","scope":"organization","foo":"clobbered","baz":"zap","a":"1","b":"2"},
417+
},
418+
// FIXME(cian): we should skip evaluating tags for which values have already been provided.
419+
{
420+
name:"main.tf with variable missing default value but value is passed in request",
421+
files:map[string]string{
422+
`main.tf`:`
423+
variable "a" {
424+
type = string
425+
}
426+
data "coder_workspace_tags" "tags" {
427+
tags = {
428+
"a": var.a,
429+
}
430+
}`,
431+
},
432+
reqTags:map[string]string{"a":"b"},
433+
// wantTags: map[string]string{"owner": "", "scope": "organization", "a": "b"},
434+
expectError:`provisioner tag "a" evaluated to an empty value`,
435+
},
387436
{
388437
name:"main.tf with disallowed workspace tag value",
389438
files:map[string]string{

‎enterprise/coderd/workspaces_test.go‎

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1433,7 +1433,8 @@ func TestWorkspaceTagsTerraform(t *testing.T) {
14331433
createTemplateVersionRequestTagsmap[string]string
14341434
// the coder_workspace_tags bit of main.tf.
14351435
// you can add more stuff here if you need
1436-
tfWorkspaceTagsstring
1436+
tfWorkspaceTagsstring
1437+
skipCreateWorkspacebool
14371438
}{
14381439
{
14391440
name:"no tags",
@@ -1520,15 +1521,18 @@ func TestWorkspaceTagsTerraform(t *testing.T) {
15201521
}`,
15211522
},
15221523
{
1523-
name:"does not overridestatic tag",
1524-
provisionerTags:map[string]string{"foo":"bar"},
1524+
name:"overridesstatic tag from request",
1525+
provisionerTags:map[string]string{"foo":"baz"},
15251526
createTemplateVersionRequestTags:map[string]string{"foo":"baz"},
15261527
tfWorkspaceTags:`
15271528
data "coder_workspace_tags" "tags" {
15281529
tags = {
15291530
"foo" = "bar"
15301531
}
15311532
}`,
1533+
// When we go to create the workspace, there won't be any provisioner
1534+
// matching tag foo=bar.
1535+
skipCreateWorkspace:true,
15321536
},
15331537
} {
15341538
tc:=tc
@@ -1570,13 +1574,15 @@ func TestWorkspaceTagsTerraform(t *testing.T) {
15701574
coderdtest.AwaitTemplateVersionJobCompleted(t,templateAdmin,tv.ID)
15711575
tpl:=coderdtest.CreateTemplate(t,templateAdmin,owner.OrganizationID,tv.ID)
15721576

1573-
// Creating a workspace as a non-privileged user must succeed
1574-
ws,err:=member.CreateUserWorkspace(ctx,memberUser.Username, codersdk.CreateWorkspaceRequest{
1575-
TemplateID:tpl.ID,
1576-
Name:coderdtest.RandomUsername(t),
1577-
})
1578-
require.NoError(t,err,"failed to create workspace")
1579-
coderdtest.AwaitWorkspaceBuildJobCompleted(t,member,ws.LatestBuild.ID)
1577+
if!tc.skipCreateWorkspace {
1578+
// Creating a workspace as a non-privileged user must succeed
1579+
ws,err:=member.CreateUserWorkspace(ctx,memberUser.Username, codersdk.CreateWorkspaceRequest{
1580+
TemplateID:tpl.ID,
1581+
Name:coderdtest.RandomUsername(t),
1582+
})
1583+
require.NoError(t,err,"failed to create workspace")
1584+
coderdtest.AwaitWorkspaceBuildJobCompleted(t,member,ws.LatestBuild.ID)
1585+
}
15801586
})
15811587
}
15821588
}

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp