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

fix(coderd): change the order of precedence between coder_workspace_tags and request tags#16119

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.

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

Already on GitHub?Sign in to your account

Merged
johnstcn merged 2 commits intomainfromcj/templateversion-precedence
Jan 14, 2025
Merged
Show file tree
Hide file tree
Changes fromall commits
Commits
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 deletionscoderd/templateversions.go
View file
Open in desktop
Original file line numberDiff line numberDiff 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 deletionscoderd/templateversions_test.go
View file
Open in desktop
Original file line numberDiff line numberDiff 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 withworkspace tagsand request tags",
name: "main.tf withrequest tagsnot clobbering workspace tags",
Copy link
Member

@mafredrimafredriJan 14, 2025•
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Is clobbering actually involved here? Since there's no two entries of the same tag the result is just a union.

Copy link
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

There's a test below that shows the actual clobbering behaviour. I guess "not clobbering" and "union" are similar enough in result but different enough in semantics to be confusing. But the tags are not a union, there is an order of precedence here that I'm trying to codify and demonstrate.

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 deletionsenterprise/coderd/workspaces_test.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -1433,7 +1433,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 DownExpand Up@@ -1520,15 +1521,18 @@ func TestWorkspaceTagsTerraform(t *testing.T) {
}`,
},
{
name: "does not overridestatic tag",
provisionerTags: map[string]string{"foo": "bar"},
name: "overridesstatic 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 DownExpand Up@@ -1570,13 +1574,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
[8]ページ先頭
©2009-2025 Movatter.jp