Skip to content

Commit 15daad1

Browse files
code-ashercode-asher
committed
fix: validate that parameter names are unique
1 parent 1288a83 commit 15daad1

File tree

4 files changed

+115
-0
lines changed

4 files changed

+115
-0
lines changed

‎coderd/util/slice/slice.go

+18
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

+7
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

+26
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/codersdk"
1416
"github.com/coder/coder/provisioner"
1517
"github.com/coder/coder/provisionersdk/proto"
@@ -473,6 +475,7 @@ func ConvertState(modules []*tfjson.StateModule, rawGraph string, rawParameterNa
473475
}
474476
}
475477

478+
var duplicatedNames []string
476479
parameters := make([]*proto.RichParameter, 0)
477480
for _, resource := range orderedRichParametersResources(tfResourcesRichParameters, rawParameterNames) {
478481
var param provider.Parameter
@@ -536,9 +539,32 @@ func ConvertState(modules []*tfjson.StateModule, rawGraph string, rawParameterNa
536539
})
537540
}
538541
}
542+
543+
// Check if this parameter duplicates an existing parameter.
544+
formattedName := fmt.Sprintf("%q", protoParam.Name)
545+
if !slice.Contains(duplicatedNames, formattedName) &&
546+
slice.ContainsCompare(parameters, protoParam, func(a, b *proto.RichParameter) bool {
547+
return a.Name == b.Name
548+
}) {
549+
duplicatedNames = append(duplicatedNames, formattedName)
550+
}
551+
539552
parameters = append(parameters, protoParam)
540553
}
541554

555+
// Enforce that parameters must be uniquely named.
556+
if len(duplicatedNames) > 0 {
557+
s := ""
558+
if len(duplicatedNames) == 1 {
559+
s = "s"
560+
}
561+
return nil, xerrors.Errorf(
562+
"coder_parameter names must be unique but %s appear%s multiple times",
563+
slice.JoinWithConjunction(duplicatedNames),
564+
s,
565+
)
566+
}
567+
542568
// A map is used to ensure we don't have duplicates!
543569
gitAuthProvidersMap := map[string]struct{}{}
544570
for _, tfResources := range tfResourcesByLabel {

‎provisioner/terraform/resources_test.go

+64
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"
@@ -584,6 +585,69 @@ func TestAppSlugValidation(t *testing.T) {
584585
require.ErrorContains(t, err, "duplicate app slug")
585586
}
586587

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

0 commit comments

Comments
 (0)