Skip to content

Commit 48e2cb8

Browse files
code-ashercode-asher
committed
fix: validate that parameter names are unique
1 parent 7ec16cf commit 48e2cb8

File tree

4 files changed

+115
-0
lines changed

4 files changed

+115
-0
lines changed

‎coderd/util/slice/slice.go

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,10 @@
11
package slice
22

3+
import (
4+
"fmt"
5+
"strings"
6+
)
7+
38
// SameElements returns true if the 2 lists have the same elements in any
49
// order.
510
func SameElements[T comparable](a []T, b []T) bool {
@@ -67,3 +72,16 @@ func OverlapCompare[T any](a []T, b []T, equal func(a, b T) bool) bool {
6772
func New[T any](items ...T) []T {
6873
return items
6974
}
75+
76+
// JoinWithConjunction joins a slice of strings with commas except for the last
77+
// two which are joined with "and".
78+
func JoinWithConjunction(s []string) string {
79+
last := len(s) - 1
80+
if last == 0 {
81+
return s[last]
82+
}
83+
return fmt.Sprintf("%s and %s",
84+
strings.Join(s[:last], ", "),
85+
s[last],
86+
)
87+
}

‎coderd/util/slice/slice_test.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,13 @@ func TestOverlap(t *testing.T) {
8383
)
8484
}
8585

86+
func TestJoinWithConjunction(t *testing.T) {
87+
t.Parallel()
88+
require.Equal(t, "foo", slice.JoinWithConjunction([]string{"foo"}))
89+
require.Equal(t, "foo and bar", slice.JoinWithConjunction([]string{"foo", "bar"}))
90+
require.Equal(t, "foo, bar and baz", slice.JoinWithConjunction([]string{"foo", "bar", "baz"}))
91+
}
92+
8693
func assertSetOverlaps[T comparable](t *testing.T, overlap bool, a []T, b []T) {
8794
t.Helper()
8895
for _, e := range a {

‎provisioner/terraform/resources.go

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package terraform
22

33
import (
4+
"fmt"
45
"strings"
56

67
"github.com/awalterschulze/gographviz"
@@ -10,6 +11,7 @@ import (
1011

1112
"github.com/coder/terraform-provider-coder/provider"
1213

14+
"github.com/coder/coder/coderd/util/slice"
1315
"github.com/coder/coder/provisioner"
1416
"github.com/coder/coder/provisionersdk/proto"
1517
)
@@ -462,6 +464,7 @@ func ConvertState(modules []*tfjson.StateModule, rawGraph string, rawParameterNa
462464
}
463465
}
464466

467+
var duplicatedNames []string
465468
parameters := make([]*proto.RichParameter, 0)
466469
for _, resource := range orderedRichParametersResources(tfResourcesRichParameters, rawParameterNames) {
467470
var param provider.Parameter
@@ -525,9 +528,32 @@ func ConvertState(modules []*tfjson.StateModule, rawGraph string, rawParameterNa
525528
})
526529
}
527530
}
531+
532+
// Check if this parameter duplicates an existing parameter.
533+
formattedName := fmt.Sprintf("%q", protoParam.Name)
534+
if !slice.Contains(duplicatedNames, formattedName) &&
535+
slice.ContainsCompare(parameters, protoParam, func(a, b *proto.RichParameter) bool {
536+
return a.Name == b.Name
537+
}) {
538+
duplicatedNames = append(duplicatedNames, formattedName)
539+
}
540+
528541
parameters = append(parameters, protoParam)
529542
}
530543

544+
// Enforce that parameters must be uniquely named.
545+
if len(duplicatedNames) > 0 {
546+
s := ""
547+
if len(duplicatedNames) == 1 {
548+
s = "s"
549+
}
550+
return nil, xerrors.Errorf(
551+
"coder_parameter names must be unique but %s appear%s multiple times",
552+
slice.JoinWithConjunction(duplicatedNames),
553+
s,
554+
)
555+
}
556+
531557
// A map is used to ensure we don't have duplicates!
532558
gitAuthProvidersMap := map[string]struct{}{}
533559
for _, tfResources := range tfResourcesByLabel {

‎provisioner/terraform/resources_test.go

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package terraform_test
22

33
import (
44
"encoding/json"
5+
"fmt"
56
"os"
67
"path/filepath"
78
"runtime"
@@ -575,6 +576,69 @@ func TestAppSlugValidation(t *testing.T) {
575576
require.ErrorContains(t, err, "duplicate app slug")
576577
}
577578

579+
func TestParameterValidation(t *testing.T) {
580+
t.Parallel()
581+
582+
// nolint:dogsled
583+
_, filename, _, _ := runtime.Caller(0)
584+
585+
// Load the rich-parameters state file and edit it.
586+
dir := filepath.Join(filepath.Dir(filename), "testdata", "rich-parameters")
587+
tfPlanRaw, err := os.ReadFile(filepath.Join(dir, "rich-parameters.tfplan.json"))
588+
require.NoError(t, err)
589+
var tfPlan tfjson.Plan
590+
err = json.Unmarshal(tfPlanRaw, &tfPlan)
591+
require.NoError(t, err)
592+
tfPlanGraph, err := os.ReadFile(filepath.Join(dir, "rich-parameters.tfplan.dot"))
593+
require.NoError(t, err)
594+
595+
// Change all names to be identical.
596+
var names []string
597+
for _, resource := range tfPlan.PriorState.Values.RootModule.Resources {
598+
if resource.Type == "coder_parameter" {
599+
resource.AttributeValues["name"] = "identical"
600+
names = append(names, resource.Name)
601+
}
602+
}
603+
604+
state, err := terraform.ConvertState([]*tfjson.StateModule{tfPlan.PriorState.Values.RootModule}, string(tfPlanGraph), names)
605+
require.Nil(t, state)
606+
require.Error(t, err)
607+
require.ErrorContains(t, err, "coder_parameter names must be unique but \"identical\" appears multiple times")
608+
609+
// Make two sets of identical names.
610+
count := 0
611+
names = nil
612+
for _, resource := range tfPlan.PriorState.Values.RootModule.Resources {
613+
if resource.Type == "coder_parameter" {
614+
resource.AttributeValues["name"] = fmt.Sprintf("identical-%d", count%2)
615+
names = append(names, resource.Name)
616+
count++
617+
}
618+
}
619+
620+
state, err = terraform.ConvertState([]*tfjson.StateModule{tfPlan.PriorState.Values.RootModule}, string(tfPlanGraph), names)
621+
require.Nil(t, state)
622+
require.Error(t, err)
623+
require.ErrorContains(t, err, "coder_parameter names must be unique but \"identical-0\" and \"identical-1\" appear multiple times")
624+
625+
// Once more with three sets.
626+
count = 0
627+
names = nil
628+
for _, resource := range tfPlan.PriorState.Values.RootModule.Resources {
629+
if resource.Type == "coder_parameter" {
630+
resource.AttributeValues["name"] = fmt.Sprintf("identical-%d", count%3)
631+
names = append(names, resource.Name)
632+
count++
633+
}
634+
}
635+
636+
state, err = terraform.ConvertState([]*tfjson.StateModule{tfPlan.PriorState.Values.RootModule}, string(tfPlanGraph), names)
637+
require.Nil(t, state)
638+
require.Error(t, err)
639+
require.ErrorContains(t, err, "coder_parameter names must be unique but \"identical-0\", \"identical-1\" and \"identical-2\" appear multiple times")
640+
}
641+
578642
func TestInstanceTypeAssociation(t *testing.T) {
579643
t.Parallel()
580644
type tc struct {

0 commit comments

Comments
 (0)