Skip to content

Commit 9321b39

Browse files
fix: skip validating unknown version names (#123)
Same problem as seen in #115. This fixes a problem where having multiple template version names manually set using Terraform variables would always return a duplicate name error, as Terraform would mark the attribute, although it is required and already written in the config, as Unknown before calling our custom validator. This unknown value is only ever passed to the validator, never `Create` or `Update`. Knowing this, we see the same problem exists for all attributes we read in our custom validator. The only remaining one is `active`, where we check at least one version is marked as active. In this case, we also need to skip validating `active` if any of the booleans are unknown. We previously didn't have any tests that explicitly used Terraform variables, so this PR adds one, in case the behaviour of these variable set attributes is changed in the future.
1 parent 39ce21c commit 9321b39

File tree

2 files changed

+74
-13
lines changed

2 files changed

+74
-13
lines changed

internal/provider/template_resource.go

+18-13
Original file line numberDiff line numberDiff line change
@@ -896,9 +896,27 @@ func (a *activeVersionValidator) ValidateList(ctx context.Context, req validator
896896
return
897897
}
898898

899+
// Check all versions have unique names
900+
uniqueNames := make(map[string]struct{})
901+
for _, version := range data {
902+
if version.Name.IsNull() || version.Name.IsUnknown() {
903+
continue
904+
}
905+
if _, ok := uniqueNames[version.Name.ValueString()]; ok {
906+
resp.Diagnostics.AddError("Client Error", fmt.Sprintf("Template version names must be unique. `%s` appears twice.", version.Name.ValueString()))
907+
return
908+
}
909+
uniqueNames[version.Name.ValueString()] = struct{}{}
910+
}
911+
899912
// Check if only one item in Version has active set to true
900913
active := false
901914
for _, version := range data {
915+
// `active` is required, so if it's null or unknown, this is Terraform
916+
// requesting an early validation.
917+
if version.Active.IsNull() || version.Active.IsUnknown() {
918+
return
919+
}
902920
if version.Active.ValueBool() {
903921
if active {
904922
resp.Diagnostics.AddError("Client Error", "Only one template version can be active at a time.")
@@ -910,19 +928,6 @@ func (a *activeVersionValidator) ValidateList(ctx context.Context, req validator
910928
if !active {
911929
resp.Diagnostics.AddError("Client Error", "At least one template version must be active.")
912930
}
913-
914-
// Check all versions have unique names
915-
uniqueNames := make(map[string]struct{})
916-
for _, version := range data {
917-
if version.Name.IsNull() {
918-
continue
919-
}
920-
if _, ok := uniqueNames[version.Name.ValueString()]; ok {
921-
resp.Diagnostics.AddError("Client Error", "Template version names must be unique.")
922-
return
923-
}
924-
uniqueNames[version.Name.ValueString()] = struct{}{}
925-
}
926931
}
927932

928933
var _ validator.List = &activeVersionValidator{}

internal/provider/template_resource_test.go

+56
Original file line numberDiff line numberDiff line change
@@ -648,6 +648,62 @@ func TestAccTemplateResourceAGPL(t *testing.T) {
648648
})
649649
}
650650

651+
func TestAccTemplateResourceVariables(t *testing.T) {
652+
cfg := `
653+
provider coderd {
654+
url = "%s"
655+
token = "%s"
656+
}
657+
658+
data "coderd_organization" "default" {
659+
is_default = true
660+
}
661+
662+
variable "PRIOR_GIT_COMMIT_SHA" {
663+
default = "abcdef"
664+
}
665+
666+
variable "CURRENT_GIT_COMMIT_SHA" {
667+
default = "ghijkl"
668+
}
669+
670+
variable "ACTIVE" {
671+
default = true
672+
}
673+
674+
resource "coderd_template" "sample" {
675+
name = "example-template"
676+
versions = [
677+
{
678+
name = "${var.PRIOR_GIT_COMMIT_SHA}"
679+
directory = "../../integration/template-test/example-template"
680+
active = var.ACTIVE
681+
},
682+
{
683+
name = "${var.CURRENT_GIT_COMMIT_SHA}"
684+
directory = "../../integration/template-test/example-template"
685+
active = false
686+
}
687+
]
688+
}`
689+
690+
ctx := context.Background()
691+
client := integration.StartCoder(ctx, t, "template_acc", false)
692+
693+
cfg = fmt.Sprintf(cfg, client.URL.String(), client.SessionToken())
694+
695+
resource.Test(t, resource.TestCase{
696+
PreCheck: func() { testAccPreCheck(t) },
697+
IsUnitTest: true,
698+
ProtoV6ProviderFactories: testAccProtoV6ProviderFactories,
699+
Steps: []resource.TestStep{
700+
{
701+
Config: cfg,
702+
},
703+
},
704+
})
705+
}
706+
651707
type testAccTemplateResourceConfig struct {
652708
URL string
653709
Token string

0 commit comments

Comments
 (0)