Skip to content

Commit 76adde9

Browse files
johnstcnjohnstcn
authored
fix(provisioner/terraform/tfparse): allow empty values in coder_workspace_tag defaults (coder#16303)
* chore(docs): update docs re workspace tag default values * chore(coderdenttest): use random name instead of t.Name() in newExternalProvisionerDaemon * fix(provisioner/terraform/tfparse): allow empty values in coder_workspace_tag defaults
1 parent f518669 commit 76adde9

File tree

8 files changed

+90
-32
lines changed

8 files changed

+90
-32
lines changed

‎coderd/templateversions_test.go

Lines changed: 38 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -429,9 +429,8 @@ func TestPostTemplateVersionsByOrganization(t *testing.T) {
429429
}
430430
}`,
431431
},
432-
reqTags: map[string]string{"a": "b"},
433-
// wantTags: map[string]string{"owner": "", "scope": "organization", "a": "b"},
434-
expectError: `provisioner tag "a" evaluated to an empty value`,
432+
reqTags: map[string]string{"a": "b"},
433+
wantTags: map[string]string{"owner": "", "scope": "organization", "a": "b"},
435434
},
436435
{
437436
name: "main.tf with disallowed workspace tag value",
@@ -568,6 +567,42 @@ func TestPostTemplateVersionsByOrganization(t *testing.T) {
568567
},
569568
wantTags: map[string]string{"owner": "", "scope": "organization"},
570569
},
570+
{
571+
name: "main.tf with tags from parameter with default value from variable no default",
572+
files: map[string]string{
573+
`main.tf`: `
574+
variable "provisioner" {
575+
type = string
576+
}
577+
variable "default_provisioner" {
578+
type = string
579+
default = "" # intentionally blank, set on template creation
580+
}
581+
data "coder_parameter" "provisioner" {
582+
name = "provisioner"
583+
mutable = false
584+
default = var.default_provisioner
585+
dynamic "option" {
586+
for_each = toset(split(",", var.provisioner))
587+
content {
588+
name = option.value
589+
value = option.value
590+
}
591+
}
592+
}
593+
data "coder_workspace_tags" "tags" {
594+
tags = {
595+
"provisioner" : data.coder_parameter.provisioner.value
596+
}
597+
}`,
598+
},
599+
reqTags: map[string]string{
600+
"provisioner": "alpha",
601+
},
602+
wantTags: map[string]string{
603+
"provisioner": "alpha", "owner": "", "scope": "organization",
604+
},
605+
},
571606
} {
572607
tt := tt
573608
t.Run(tt.name, func(t *testing.T) {

‎docs/admin/templates/extending-templates/workspace-tags.md

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -62,11 +62,6 @@ variables and parameters. This is illustrated in the table below:
6262

6363
## Constraints
6464

65-
### Default Values
66-
67-
All template variables and `coder_parameter` data sources **must** provide a
68-
default value. Failure to do so will result in an error.
69-
7065
### Tagged provisioners
7166

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

128123
#### Not supported
129124

130-
- Function calls: `try(var.foo, "default")`
125+
- Function calls that reference files on disk: `abspath`, `file*`, `pathexpand`
131126
- Resources: `compute_instance.dev.name`
132127
- Data sources other than `coder_parameter`: `data.local_file.hostname.content`

‎enterprise/coderd/coderdenttest/coderdenttest.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -389,7 +389,7 @@ func newExternalProvisionerDaemon(t testing.TB, client *codersdk.Client, org uui
389389
daemon := provisionerd.New(func(ctx context.Context) (provisionerdproto.DRPCProvisionerDaemonClient, error) {
390390
return client.ServeProvisionerDaemon(ctx, codersdk.ServeProvisionerDaemonRequest{
391391
ID: uuid.New(),
392-
Name: t.Name(),
392+
Name: testutil.GetRandomName(t),
393393
Organization: org,
394394
Provisioners: []codersdk.ProvisionerType{provisionerType},
395395
Tags: tags,

‎enterprise/coderd/provisionerdaemons_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -285,7 +285,7 @@ func TestProvisionerDaemonServe(t *testing.T) {
285285
daemons, err := client.ProvisionerDaemons(context.Background())
286286
assert.NoError(t, err, "failed to get provisioner daemons")
287287
return len(daemons) > 0 &&
288-
assert.Equal(t, t.Name(), daemons[0].Name) &&
288+
assert.NotEmpty(t, daemons[0].Name) &&
289289
assert.Equal(t, provisionersdk.ScopeUser, daemons[0].Tags[provisionersdk.TagScope]) &&
290290
assert.Equal(t, user.UserID.String(), daemons[0].Tags[provisionersdk.TagOwner])
291291
}, testutil.WaitShort, testutil.IntervalMedium)

‎enterprise/coderd/workspaces_test.go

Lines changed: 46 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1488,8 +1488,11 @@ func TestWorkspaceTagsTerraform(t *testing.T) {
14881488
createTemplateVersionRequestTags map[string]string
14891489
// the coder_workspace_tags bit of main.tf.
14901490
// you can add more stuff here if you need
1491-
tfWorkspaceTags string
1492-
skipCreateWorkspace bool
1491+
tfWorkspaceTags string
1492+
templateImportUserVariableValues []codersdk.VariableValue
1493+
// if we need to set parameters on workspace build
1494+
workspaceBuildParameters []codersdk.WorkspaceBuildParameter
1495+
skipCreateWorkspace bool
14931496
}{
14941497
{
14951498
name: "no tags",
@@ -1589,6 +1592,38 @@ func TestWorkspaceTagsTerraform(t *testing.T) {
15891592
// matching tag foo=bar.
15901593
skipCreateWorkspace: true,
15911594
},
1595+
{
1596+
name: "overrides with dynamic option from var",
1597+
provisionerTags: map[string]string{"foo": "bar"},
1598+
createTemplateVersionRequestTags: map[string]string{"foo": "bar"},
1599+
templateImportUserVariableValues: []codersdk.VariableValue{{Name: "default_foo", Value: "baz"}, {Name: "foo", Value: "bar,baz"}},
1600+
workspaceBuildParameters: []codersdk.WorkspaceBuildParameter{{Name: "foo", Value: "bar"}},
1601+
tfWorkspaceTags: `
1602+
variable "default_foo" {
1603+
type = string
1604+
}
1605+
variable "foo" {
1606+
type = string
1607+
}
1608+
data "coder_parameter" "foo" {
1609+
name = "foo"
1610+
type = "string"
1611+
default = var.default_foo
1612+
mutable = false
1613+
dynamic "option" {
1614+
for_each = toset(split(",", var.foo))
1615+
content {
1616+
name = option.value
1617+
value = option.value
1618+
}
1619+
}
1620+
}
1621+
data "coder_workspace_tags" "tags" {
1622+
tags = {
1623+
"foo" = data.coder_parameter.foo.value
1624+
}
1625+
}`,
1626+
},
15921627
} {
15931628
tc := tc
15941629
t.Run(tc.name, func(t *testing.T) {
@@ -1617,11 +1652,12 @@ func TestWorkspaceTagsTerraform(t *testing.T) {
16171652
fi, err := templateAdmin.Upload(ctx, "application/x-tar", bytes.NewReader(tarBytes))
16181653
require.NoError(t, err, "failed to upload file")
16191654
tv, err := templateAdmin.CreateTemplateVersion(ctx, owner.OrganizationID, codersdk.CreateTemplateVersionRequest{
1620-
Name: testutil.GetRandomName(t),
1621-
FileID: fi.ID,
1622-
StorageMethod: codersdk.ProvisionerStorageMethodFile,
1623-
Provisioner: codersdk.ProvisionerTypeTerraform,
1624-
ProvisionerTags: tc.createTemplateVersionRequestTags,
1655+
Name: testutil.GetRandomName(t),
1656+
FileID: fi.ID,
1657+
StorageMethod: codersdk.ProvisionerStorageMethodFile,
1658+
Provisioner: codersdk.ProvisionerTypeTerraform,
1659+
ProvisionerTags: tc.createTemplateVersionRequestTags,
1660+
UserVariableValues: tc.templateImportUserVariableValues,
16251661
})
16261662
require.NoError(t, err, "failed to create template version")
16271663
coderdtest.AwaitTemplateVersionJobCompleted(t, templateAdmin, tv.ID)
@@ -1630,8 +1666,9 @@ func TestWorkspaceTagsTerraform(t *testing.T) {
16301666
if !tc.skipCreateWorkspace {
16311667
// Creating a workspace as a non-privileged user must succeed
16321668
ws, err := member.CreateUserWorkspace(ctx, memberUser.Username, codersdk.CreateWorkspaceRequest{
1633-
TemplateID: tpl.ID,
1634-
Name: coderdtest.RandomUsername(t),
1669+
TemplateID: tpl.ID,
1670+
Name: coderdtest.RandomUsername(t),
1671+
RichParameterValues: tc.workspaceBuildParameters,
16351672
})
16361673
require.NoError(t, err, "failed to create workspace")
16371674
coderdtest.AwaitWorkspaceBuildJobCompleted(t, member, ws.LatestBuild.ID)

‎examples/workspace-tags/README.md

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ icon: /icon/docker.png
77

88
## Overview
99

10-
This Coder template presents use of [Workspace Tags](https://coder.com/docs/templates/workspace-tags) [Coder Parameters](https://coder.com/docs/templates/parameters).
10+
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).
1111

1212
## Use case
1313

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

2020
- 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.
21-
- When specifying values for the `coder_workspace_tags` data source, you are restricted to using a subset of Terraform's capabilities.
22-
- You must specify default values for all data sources and variables referenced by the `coder_workspace_tags` data source.
21+
- 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.
2322

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

2624
## Development
2725

‎provisioner/terraform/tfparse/tfparse.go

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -239,13 +239,6 @@ func (p *Parser) WorkspaceTagDefaults(ctx context.Context) (map[string]string, e
239239
return nil, xerrors.Errorf("eval provisioner tags: %w", err)
240240
}
241241

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

‎provisioner/terraform/tfparse/tfparse_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -268,7 +268,7 @@ func Test_WorkspaceTagDefaultsFromFile(t *testing.T) {
268268
}
269269
}`,
270270
},
271-
expectError: `provisioner tag "az" evaluated to an empty value, please set a default value`,
271+
expectTags: map[string]string{"cluster": "developers", "az": "", "platform": "kubernetes", "region": "us"},
272272
},
273273
{
274274
name: "main.tf with missing parameter default value outside workspace tags",

0 commit comments

Comments
 (0)