Skip to content

fix(provisioner/terraform/tfparse): allow empty values in coder_workspace_tag defaults #16303

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 3 commits into from
Jan 28, 2025
Merged
Show file tree
Hide file tree
Changes from all 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
41 changes: 38 additions & 3 deletions coderd/templateversions_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -429,9 +429,8 @@ func TestPostTemplateVersionsByOrganization(t *testing.T) {
}
}`,
},
reqTags: map[string]string{"a": "b"},
// wantTags: map[string]string{"owner": "", "scope": "organization", "a": "b"},
expectError: `provisioner tag "a" evaluated to an empty value`,
reqTags: map[string]string{"a": "b"},
wantTags: map[string]string{"owner": "", "scope": "organization", "a": "b"},
},
{
name: "main.tf with disallowed workspace tag value",
Expand Down Expand Up @@ -568,6 +567,42 @@ func TestPostTemplateVersionsByOrganization(t *testing.T) {
},
wantTags: map[string]string{"owner": "", "scope": "organization"},
},
{
name: "main.tf with tags from parameter with default value from variable no default",
files: map[string]string{
`main.tf`: `
variable "provisioner" {
type = string
}
variable "default_provisioner" {
type = string
default = "" # intentionally blank, set on template creation
}
data "coder_parameter" "provisioner" {
name = "provisioner"
mutable = false
default = var.default_provisioner
dynamic "option" {
for_each = toset(split(",", var.provisioner))
content {
name = option.value
value = option.value
}
}
}
data "coder_workspace_tags" "tags" {
tags = {
"provisioner" : data.coder_parameter.provisioner.value
}
}`,
},
reqTags: map[string]string{
"provisioner": "alpha",
},
wantTags: map[string]string{
"provisioner": "alpha", "owner": "", "scope": "organization",
},
},
} {
tt := tt
t.Run(tt.name, func(t *testing.T) {
Expand Down
7 changes: 1 addition & 6 deletions docs/admin/templates/extending-templates/workspace-tags.md
Original file line number Diff line number Diff line change
Expand Up @@ -62,11 +62,6 @@ variables and parameters. This is illustrated in the table below:

## Constraints

### Default Values

All template variables and `coder_parameter` data sources **must** provide a
default value. Failure to do so will result in an error.

### Tagged provisioners

It is possible to choose tag combinations that no provisioner can handle. This
Expand Down Expand Up @@ -127,6 +122,6 @@ variables, and references to other resources.

#### Not supported

- Function calls: `try(var.foo, "default")`
- Function calls that reference files on disk: `abspath`, `file*`, `pathexpand`
- Resources: `compute_instance.dev.name`
- Data sources other than `coder_parameter`: `data.local_file.hostname.content`
2 changes: 1 addition & 1 deletion enterprise/coderd/coderdenttest/coderdenttest.go
Original file line number Diff line number Diff line change
Expand Up @@ -389,7 +389,7 @@ func newExternalProvisionerDaemon(t testing.TB, client *codersdk.Client, org uui
daemon := provisionerd.New(func(ctx context.Context) (provisionerdproto.DRPCProvisionerDaemonClient, error) {
return client.ServeProvisionerDaemon(ctx, codersdk.ServeProvisionerDaemonRequest{
ID: uuid.New(),
Name: t.Name(),
Name: testutil.GetRandomName(t),
Organization: org,
Provisioners: []codersdk.ProvisionerType{provisionerType},
Tags: tags,
Expand Down
2 changes: 1 addition & 1 deletion enterprise/coderd/provisionerdaemons_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -285,7 +285,7 @@ func TestProvisionerDaemonServe(t *testing.T) {
daemons, err := client.ProvisionerDaemons(context.Background())
assert.NoError(t, err, "failed to get provisioner daemons")
return len(daemons) > 0 &&
assert.Equal(t, t.Name(), daemons[0].Name) &&
assert.NotEmpty(t, daemons[0].Name) &&
assert.Equal(t, provisionersdk.ScopeUser, daemons[0].Tags[provisionersdk.TagScope]) &&
assert.Equal(t, user.UserID.String(), daemons[0].Tags[provisionersdk.TagOwner])
}, testutil.WaitShort, testutil.IntervalMedium)
Expand Down
55 changes: 46 additions & 9 deletions enterprise/coderd/workspaces_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1488,8 +1488,11 @@ 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
skipCreateWorkspace bool
tfWorkspaceTags string
templateImportUserVariableValues []codersdk.VariableValue
// if we need to set parameters on workspace build
workspaceBuildParameters []codersdk.WorkspaceBuildParameter
skipCreateWorkspace bool
}{
{
name: "no tags",
Expand Down Expand Up @@ -1589,6 +1592,38 @@ func TestWorkspaceTagsTerraform(t *testing.T) {
// matching tag foo=bar.
skipCreateWorkspace: true,
},
{
name: "overrides with dynamic option from var",
provisionerTags: map[string]string{"foo": "bar"},
createTemplateVersionRequestTags: map[string]string{"foo": "bar"},
templateImportUserVariableValues: []codersdk.VariableValue{{Name: "default_foo", Value: "baz"}, {Name: "foo", Value: "bar,baz"}},
workspaceBuildParameters: []codersdk.WorkspaceBuildParameter{{Name: "foo", Value: "bar"}},
tfWorkspaceTags: `
variable "default_foo" {
type = string
}
variable "foo" {
type = string
}
data "coder_parameter" "foo" {
name = "foo"
type = "string"
default = var.default_foo
mutable = false
dynamic "option" {
for_each = toset(split(",", var.foo))
content {
name = option.value
value = option.value
}
}
}
data "coder_workspace_tags" "tags" {
tags = {
"foo" = data.coder_parameter.foo.value
}
}`,
},
} {
tc := tc
t.Run(tc.name, func(t *testing.T) {
Expand Down Expand Up @@ -1617,11 +1652,12 @@ func TestWorkspaceTagsTerraform(t *testing.T) {
fi, err := templateAdmin.Upload(ctx, "application/x-tar", bytes.NewReader(tarBytes))
require.NoError(t, err, "failed to upload file")
tv, err := templateAdmin.CreateTemplateVersion(ctx, owner.OrganizationID, codersdk.CreateTemplateVersionRequest{
Name: testutil.GetRandomName(t),
FileID: fi.ID,
StorageMethod: codersdk.ProvisionerStorageMethodFile,
Provisioner: codersdk.ProvisionerTypeTerraform,
ProvisionerTags: tc.createTemplateVersionRequestTags,
Name: testutil.GetRandomName(t),
FileID: fi.ID,
StorageMethod: codersdk.ProvisionerStorageMethodFile,
Provisioner: codersdk.ProvisionerTypeTerraform,
ProvisionerTags: tc.createTemplateVersionRequestTags,
UserVariableValues: tc.templateImportUserVariableValues,
})
require.NoError(t, err, "failed to create template version")
coderdtest.AwaitTemplateVersionJobCompleted(t, templateAdmin, tv.ID)
Expand All @@ -1630,8 +1666,9 @@ func TestWorkspaceTagsTerraform(t *testing.T) {
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),
TemplateID: tpl.ID,
Name: coderdtest.RandomUsername(t),
RichParameterValues: tc.workspaceBuildParameters,
})
require.NoError(t, err, "failed to create workspace")
coderdtest.AwaitWorkspaceBuildJobCompleted(t, member, ws.LatestBuild.ID)
Expand Down
6 changes: 2 additions & 4 deletions examples/workspace-tags/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ icon: /icon/docker.png

## Overview

This Coder template presents use of [Workspace Tags](https://coder.com/docs/templates/workspace-tags) [Coder Parameters](https://coder.com/docs/templates/parameters).
This Coder template presents use of [Workspace Tags](https://coder.com/docs/admin/templates/extending-templates/workspace-tags) and [Coder Parameters](https://coder.com/docs/templates/parameters).

## Use case

Expand All @@ -18,10 +18,8 @@ By using `coder_workspace_tags` and `coder_parameter`s, template administrators
## Notes

- You will need to have an [external provisioner](https://coder.com/docs/admin/provisioners#external-provisioners) with the correct tagset running in order to import this template.
- When specifying values for the `coder_workspace_tags` data source, you are restricted to using a subset of Terraform's capabilities.
- You must specify default values for all data sources and variables referenced by the `coder_workspace_tags` data source.
- When specifying values for the `coder_workspace_tags` data source, you are restricted to using a subset of Terraform's capabilities. See [here](https://coder.com/docs/admin/templates/extending-templates/workspace-tags) for more details.

See [Workspace Tags](https://coder.com/docs/templates/workspace-tags) for more information.

## Development

Expand Down
7 changes: 0 additions & 7 deletions provisioner/terraform/tfparse/tfparse.go
Original file line number Diff line number Diff line change
Expand Up @@ -239,13 +239,6 @@ func (p *Parser) WorkspaceTagDefaults(ctx context.Context) (map[string]string, e
return nil, xerrors.Errorf("eval provisioner tags: %w", err)
}

// Ensure that none of the tag values are empty after evaluation.
for k, v := range evalTags {
if len(strings.TrimSpace(v)) > 0 {
continue
}
return nil, xerrors.Errorf("provisioner tag %q evaluated to an empty value, please set a default value", k)
}
return evalTags, nil
}

Expand Down
2 changes: 1 addition & 1 deletion provisioner/terraform/tfparse/tfparse_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,7 @@ func Test_WorkspaceTagDefaultsFromFile(t *testing.T) {
}
}`,
},
expectError: `provisioner tag "az" evaluated to an empty value, please set a default value`,
expectTags: map[string]string{"cluster": "developers", "az": "", "platform": "kubernetes", "region": "us"},
},
{
name: "main.tf with missing parameter default value outside workspace tags",
Expand Down
Loading