mirror of
https://github.com/coder/coder.git
synced 2026-09-24 15:04:27 +08:00
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 posting a template version. Previously, user-specified tags in the request could not override those detected from our parsing of the template file. Now, they can do. This addresses a customer issue where were attempting to set a workspace tag via variable. Note: there is a possible follow-up item here where we could pass in the workspace tag values from the request into `tfparse` and let it take those user-specified values into account. This is covered in a separate test.
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -355,7 +355,38 @@ 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, once again, the same as the above, except
|
||||
// for a slightly different 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"},
|
||||
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.
|
||||
@@ -381,8 +412,26 @@ func TestPostTemplateVersionsByOrganization(t *testing.T) {
|
||||
}
|
||||
}`,
|
||||
},
|
||||
reqTags: map[string]string{"baz": "zap", "foo": "noclobber"},
|
||||
wantTags: map[string]string{"owner": "", "scope": "organization", "foo": "bar", "baz": "zap", "a": "1", "b": "2"},
|
||||
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",
|
||||
|
||||
Reference in New Issue
Block a user