Skip to content

feat!: extract provisioner tags from coder_workspace_tags data source #15578

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 20 commits into from
Nov 25, 2024
Merged

feat!: extract provisioner tags from coder_workspace_tags data source #15578

Changes from 1 commit
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
944eb3a
feat: extract provisioner tags from coder_workspace_tags data source …
johnstcn Nov 18, 2024
7bc2fc8
Merge remote-tracking branch 'origin/main' into cj/15087-extract-work…
johnstcn Nov 20, 2024
4653638
add initial framework for API tests
johnstcn Nov 20, 2024
fc8ff56
Merge remote-tracking branch 'origin/main' into cj/15087-extract-work…
johnstcn Nov 20, 2024
85c2c8f
update tests
johnstcn Nov 20, 2024
2e9526b
drop warn log
johnstcn Nov 20, 2024
01c8e99
var naming
johnstcn Nov 20, 2024
5a53d86
adjust control flow in error case
johnstcn Nov 20, 2024
c1e321d
show warning in templates push
johnstcn Nov 20, 2024
ed534bd
bump test timeout
johnstcn Nov 20, 2024
42f39a7
improve test coverage
johnstcn Nov 21, 2024
c6f33f7
fix tests
johnstcn Nov 21, 2024
b22e890
move to client-side checking for warnings
johnstcn Nov 21, 2024
a8a620f
make gen
johnstcn Nov 21, 2024
8ec672c
address indentation
johnstcn Nov 22, 2024
7b7aa40
use CacheDir for tfparse instead
johnstcn Nov 22, 2024
80dc6c2
feat: add codersdk.MatchedProvisioners
johnstcn Nov 22, 2024
5631577
update cli template push
johnstcn Nov 22, 2024
58fae97
cleanup and error handling
johnstcn Nov 22, 2024
90991bd
Apply suggestions from code review
johnstcn Nov 25, 2024
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
Prev Previous commit
Next Next commit
improve test coverage
  • Loading branch information
@johnstcn
johnstcn committed Nov 21, 2024
commit 42f39a761a9c9eb475e573182e7e3d7c34ec7b03
183 changes: 141 additions & 42 deletions coderd/templateversions_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,11 @@

t.Run("WorkspaceTags", func(t *testing.T) {
t.Parallel()
// This test ensures that when creating a template version from an archive continaining a coder_workspace_tags
// data source, we automatically assign some "reasonable" provisioner tag values to the resulting template
// import job.
// TODO(Cian): I'd also like to assert that the correct raw tag values are stored in the database,
// but in order to do this, we need to actually run the job! This isn't straightforward right now.

ctx := testutil.Context(t, testutil.WaitLong)
store, ps := dbtestutil.NewDB(t)
Expand All @@ -249,58 +254,136 @@
{
name: "main.tf with no tags",
files: map[string]string{
`main.tf`: `resource "null_resource" "test" {}`,
`main.tf`: `
variable "a" {
type = string
default = "1"
}
data "coder_parameter" "b" {
type = string
default = "2"
}
resource "null_resource" "test" {}`,
},
wantTags: map[string]string{"owner": "", "scope": "organization"},
},
{
name: "main.tf with empty workspace tags",
files: map[string]string{
`main.tf`: `resource "null_resource" "test" {}
data "coder_workspace_tags" "tags" {
tags = {}
}`,
`main.tf`: `
variable "a" {
type = string
default = "1"
}
data "coder_parameter" "b" {
type = string
default = "2"
}
resource "null_resource" "test" {}
data "coder_workspace_tags" "tags" {
tags = {}
}`,
},
wantTags: map[string]string{"owner": "", "scope": "organization"},
},
{
name: "main.tf with workspace tags",
files: map[string]string{
`main.tf`: `resource "null_resource" "test" {}
data "coder_workspace_tags" "tags" {
tags = {
"foo": "bar",
}
}`,
`main.tf`: `
variable "a" {
type = string
default = "1"
}
data "coder_parameter" "b" {
type = string
default = "2"
}
resource "null_resource" "test" {}
data "coder_workspace_tags" "tags" {
tags = {
"foo": "bar",
"a": var.a,
"b": data.coder_parameter.b.value,
}
}`,
},
wantTags: map[string]string{"owner": "", "scope": "organization", "foo": "bar"},
wantTags: map[string]string{"owner": "", "scope": "organization", "foo": "bar", "a": "1", "b": "2"},
},
{
name: "main.tf with workspace tags and request tags",
files: map[string]string{
`main.tf`: `resource "null_resource" "test" {}
data "coder_workspace_tags" "tags" {
tags = {
"foo": "bar",
}
}`,
`main.tf`: `
variable "a" {
type = string
default = "1"
}
data "coder_parameter" "b" {
type = string
default = "2"
}
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": "noclobber"},
wantTags: map[string]string{"owner": "", "scope": "organization", "foo": "bar", "baz": "zap"},
wantTags: map[string]string{"owner": "", "scope": "organization", "foo": "bar", "baz": "zap", "a": "1", "b": "2"},
},
{
name: "main.tf with disallowed workspace tag value",
files: map[string]string{
`main.tf`: `resource "null_resource" "test" {
name = "foo"
}
data "coder_workspace_tags" "tags" {
tags = {
"foo": null_resource.test.name,
}
}`,
`main.tf`: `
variable "a" {
type = string
default = "1"
}
data "coder_parameter" "b" {
type = string
default = "2"
}
resource "null_resource" "test" {
name = "foo"
}
data "coder_workspace_tags" "tags" {
tags = {
"foo": "bar",
"a": var.a,
"b": data.coder_parameter.b.value,
"test": null_resource.test.name,
}
}`,
},
expectError: ` Unknown variable; There is no variable named "null_resource".`,
expectError: `Unknown variable; There is no variable named "null_resource".`,
},
{
name: "main.tf with disallowed function in tag value",
files: map[string]string{
`main.tf`: `
variable "a" {
type = string
default = "1"
}
data "coder_parameter" "b" {
type = string
default = "2"
}
resource "null_resource" "test" {
name = "foo"
}
data "coder_workspace_tags" "tags" {
tags = {
"foo": "bar",
"a": var.a,
"b": data.coder_parameter.b.value,
"test": try(null_resource.test.name, "whatever"),
}
}`,
},
expectError: `Function calls not allowed; Functions may not be called here.`,
},
// We will allow coder_workspace_tags to set the scope on a template version import job
// BUT the user ID will be ultimately determined by the API key in the scope.
Expand All @@ -309,32 +392,47 @@
{
name: "main.tf with workspace tags that attempts to set user scope",
files: map[string]string{
`main.tf`: `resource "null_resource" "test" {}
data "coder_workspace_tags" "tags" {
tags = {
"scope": "user",
"owner": "12345678-1234-1234-1234-1234567890ab",
}
}`,
`main.tf`: `
resource "null_resource" "test" {}
data "coder_workspace_tags" "tags" {
tags = {
"scope": "user",
"owner": "12345678-1234-1234-1234-1234567890ab",
}
}`,
},
wantTags: map[string]string{"owner": templateAdminUser.ID.String(), "scope": "user"},
},
{
name: "main.tf with workspace tags that attempt to clobber org ID",
files: map[string]string{
`main.tf`: `resource "null_resource" "test" {}
data "coder_workspace_tags" "tags" {
tags = {
"scope": "organization",
"owner": "12345678-1234-1234-1234-1234567890ab",
}
}`,
`main.tf`: `
resource "null_resource" "test" {}
data "coder_workspace_tags" "tags" {
tags = {
"scope": "organization",
"owner": "12345678-1234-1234-1234-1234567890ab",
}
}`,
},
wantTags: map[string]string{"owner": "", "scope": "organization"},
Copy link
Member

Choose a reason for hiding this comment

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

Should this be an error? Silently replacing seems like a recipe for confusion.

Copy link
Member Author

Choose a reason for hiding this comment

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

I was debating that, but you've sold me. 👍

Copy link
Member Author

Choose a reason for hiding this comment

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

I might make this a follow-up, as the existing behaviour of provisionersdk.MutateTags is to overwrite provisionersdk.TagOwner

https://github.com/coder/coder/pull/15518/files#diff-01b662bbd7ba1a97489f36a381633c882cc2126dab8fb9ede81d3cb9239dd035R36-R40

},
{
name: "main.tf with workspace tags that set scope=user",
files: map[string]string{
`main.tf`: `
resource "null_resource" "test" {}
data "coder_workspace_tags" "tags" {
tags = {
"scope": "user",
}
}`,
},
wantTags: map[string]string{"owner": templateAdminUser.ID.String(), "scope": "user"},
},
} {
tt := tt
t.Run(tt.name, func(t *testing.T) {

Check failure on line 435 in coderd/templateversions_test.go

View workflow job for this annotation

GitHub Actions / lint 

empty-lines: extra empty line at the end of a block (revive)
t.Parallel()
// Create an archive from the files provided in the test case.
tarFile := testutil.CreateTar(t, tt.files)
Expand Down Expand Up @@ -363,6 +461,7 @@
} else {
require.ErrorContains(t, err, tt.expectError)
}

})
}
})
Expand Down
Loading