Skip to content

fix: change precedence order between coder_workspace_tags and request_tags #16239

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jan 23, 2025
Merged

fix: change precedence order between coder_workspace_tags and request_tags #16239

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 3 additions & 5 deletions coderd/templateversions.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1597,11 +1597,9 @@ func (api *API) postTemplateVersionsByOrganization(rw http.ResponseWriter, r *ht
}

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

var templateVersion database.TemplateVersion
var provisionerJob database.ProvisionerJob
Expand Down
55 changes: 52 additions & 3 deletions coderd/templateversions_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -355,10 +355,11 @@ func TestPostTemplateVersionsByOrganization(t *testing.T) {
wantTags: map[string]string{"owner": "", "scope": "organization", "foo": "bar", "a": "1", "b": "2"},
},
{
name: "main.tf with workspace tags and request tags",
name: "main.tf with request tags not clobbering workspace tags",
files: map[string]string{
`main.tf`: `
// This file is the same as the above, except for this comment.
// This file is, once again, the same as the above, except
// for a slightly different comment.
variable "a" {
type = string
default = "1"
Expand All @@ -381,9 +382,57 @@ func TestPostTemplateVersionsByOrganization(t *testing.T) {
}
}`,
},
reqTags: map[string]string{"baz": "zap", "foo": "noclobber"},
reqTags: map[string]string{"baz": "zap"},
wantTags: map[string]string{"owner": "", "scope": "organization", "foo": "bar", "baz": "zap", "a": "1", "b": "2"},
},
{
name: "main.tf with request tags clobbering workspace tags",
files: map[string]string{
`main.tf`: `
// This file is the same as the above, except for this comment.
variable "a" {
type = string
default = "1"
}
data "coder_parameter" "b" {
type = string
default = "2"
}
data "coder_parameter" "unrelated" {
name = "unrelated"
type = "list(string)"
default = jsonencode(["a", "b"])
}
resource "null_resource" "test" {}
data "coder_workspace_tags" "tags" {
tags = {
"foo": "bar",
"a": var.a,
"b": data.coder_parameter.b.value,
}
}`,
},
reqTags: map[string]string{"baz": "zap", "foo": "clobbered"},
wantTags: map[string]string{"owner": "", "scope": "organization", "foo": "clobbered", "baz": "zap", "a": "1", "b": "2"},
},
// FIXME(cian): we should skip evaluating tags for which values have already been provided.
{
name: "main.tf with variable missing default value but value is passed in request",
files: map[string]string{
`main.tf`: `
variable "a" {
type = string
}
data "coder_workspace_tags" "tags" {
tags = {
"a": var.a,
}
}`,
},
reqTags: map[string]string{"a": "b"},
// wantTags: map[string]string{"owner": "", "scope": "organization", "a": "b"},
expectError: `provisioner tag "a" evaluated to an empty value`,
},
{
name: "main.tf with disallowed workspace tag value",
files: map[string]string{
Expand Down
26 changes: 16 additions & 10 deletions enterprise/coderd/workspaces_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1217,7 +1217,8 @@ func TestWorkspaceTagsTerraform(t *testing.T) {
createTemplateVersionRequestTags map[string]string
// the coder_workspace_tags bit of main.tf.
// you can add more stuff here if you need
tfWorkspaceTags string
tfWorkspaceTags string
skipCreateWorkspace bool
}{
{
name: "no tags",
Expand Down Expand Up @@ -1304,15 +1305,18 @@ func TestWorkspaceTagsTerraform(t *testing.T) {
}`,
},
{
name: "does not override static tag",
provisionerTags: map[string]string{"foo": "bar"},
name: "overrides static tag from request",
provisionerTags: map[string]string{"foo": "baz"},
createTemplateVersionRequestTags: map[string]string{"foo": "baz"},
tfWorkspaceTags: `
data "coder_workspace_tags" "tags" {
tags = {
"foo" = "bar"
}
}`,
// When we go to create the workspace, there won't be any provisioner
// matching tag foo=bar.
skipCreateWorkspace: true,
},
} {
tc := tc
Expand Down Expand Up @@ -1352,13 +1356,15 @@ func TestWorkspaceTagsTerraform(t *testing.T) {
coderdtest.AwaitTemplateVersionJobCompleted(t, templateAdmin, tv.ID)
tpl := coderdtest.CreateTemplate(t, templateAdmin, owner.OrganizationID, tv.ID)

// Creating a workspace as a non-privileged user must succeed
ws, err := member.CreateUserWorkspace(ctx, memberUser.Username, codersdk.CreateWorkspaceRequest{
TemplateID: tpl.ID,
Name: coderdtest.RandomUsername(t),
})
require.NoError(t, err, "failed to create workspace")
coderdtest.AwaitWorkspaceBuildJobCompleted(t, member, ws.LatestBuild.ID)
if !tc.skipCreateWorkspace {
// Creating a workspace as a non-privileged user must succeed
ws, err := member.CreateUserWorkspace(ctx, memberUser.Username, codersdk.CreateWorkspaceRequest{
TemplateID: tpl.ID,
Name: coderdtest.RandomUsername(t),
})
require.NoError(t, err, "failed to create workspace")
coderdtest.AwaitWorkspaceBuildJobCompleted(t, member, ws.LatestBuild.ID)
}
})
}
}
Expand Down
Loading