Skip to content

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 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 2 commits into from
Jan 14, 2025

Conversation

johnstcn
Copy link
Member

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.

@johnstcn johnstcn self-assigned this Jan 13, 2025
@johnstcn johnstcn changed the title fix(coderd): swap order of precedence of coder_workspace_tags and tags from request fix(coderd): change the order of precedence between coder_workspace_tags and request tags Jan 13, 2025
@johnstcn johnstcn requested review from mafredri, mtojek and Emyrk January 14, 2025 11:19
@@ -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",
Copy link
Member

@mafredri mafredri Jan 14, 2025

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
Member Author

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.

Copy link
Member

@mtojek mtojek left a comment

Choose a reason for hiding this comment

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

Tricky edge case 👍

@johnstcn johnstcn merged commit 12991ff into main Jan 14, 2025
31 checks passed
@johnstcn johnstcn deleted the cj/templateversion-precedence branch January 14, 2025 16:43
@github-actions github-actions bot locked and limited conversation to collaborators Jan 14, 2025
@matifali
Copy link
Member

/cherry-pick release/2.18

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants