diff --git a/coderd/util/strings/strings.go b/coderd/util/strings/strings.go new file mode 100644 index 0000000000000..fda9f0e7c6ea6 --- /dev/null +++ b/coderd/util/strings/strings.go @@ -0,0 +1,19 @@ +package strings + +import ( + "fmt" + "strings" +) + +// JoinWithConjunction joins a slice of strings with commas except for the last +// two which are joined with "and". +func JoinWithConjunction(s []string) string { + last := len(s) - 1 + if last == 0 { + return s[last] + } + return fmt.Sprintf("%s and %s", + strings.Join(s[:last], ", "), + s[last], + ) +} diff --git a/coderd/util/strings/strings_test.go b/coderd/util/strings/strings_test.go new file mode 100644 index 0000000000000..a5db8ebbf8734 --- /dev/null +++ b/coderd/util/strings/strings_test.go @@ -0,0 +1,16 @@ +package strings_test + +import ( + "testing" + + "github.com/stretchr/testify/require" + + "github.com/coder/coder/coderd/util/strings" +) + +func TestJoinWithConjunction(t *testing.T) { + t.Parallel() + require.Equal(t, "foo", strings.JoinWithConjunction([]string{"foo"})) + require.Equal(t, "foo and bar", strings.JoinWithConjunction([]string{"foo", "bar"})) + require.Equal(t, "foo, bar and baz", strings.JoinWithConjunction([]string{"foo", "bar", "baz"})) +} diff --git a/provisioner/terraform/resources.go b/provisioner/terraform/resources.go index 8832c21ddc7d0..b208c25ce4689 100644 --- a/provisioner/terraform/resources.go +++ b/provisioner/terraform/resources.go @@ -1,6 +1,7 @@ package terraform import ( + "fmt" "strings" "github.com/awalterschulze/gographviz" @@ -10,6 +11,8 @@ import ( "github.com/coder/terraform-provider-coder/provider" + "github.com/coder/coder/coderd/util/slice" + stringutil "github.com/coder/coder/coderd/util/strings" "github.com/coder/coder/codersdk" "github.com/coder/coder/provisioner" "github.com/coder/coder/provisionersdk/proto" @@ -473,6 +476,7 @@ func ConvertState(modules []*tfjson.StateModule, rawGraph string, rawParameterNa } } + var duplicatedParamNames []string parameters := make([]*proto.RichParameter, 0) for _, resource := range orderedRichParametersResources(tfResourcesRichParameters, rawParameterNames) { var param provider.Parameter @@ -536,9 +540,31 @@ func ConvertState(modules []*tfjson.StateModule, rawGraph string, rawParameterNa }) } } + + // Check if this parameter duplicates an existing parameter. + formattedName := fmt.Sprintf("%q", protoParam.Name) + if !slice.Contains(duplicatedParamNames, formattedName) && + slice.ContainsCompare(parameters, protoParam, func(a, b *proto.RichParameter) bool { + return a.Name == b.Name + }) { + duplicatedParamNames = append(duplicatedParamNames, formattedName) + } + parameters = append(parameters, protoParam) } + // Enforce that parameters be uniquely named. + if len(duplicatedParamNames) > 0 { + s := "" + if len(duplicatedParamNames) == 1 { + s = "s" + } + return nil, xerrors.Errorf( + "coder_parameter names must be unique but %s appear%s multiple times", + stringutil.JoinWithConjunction(duplicatedParamNames), s, + ) + } + // A map is used to ensure we don't have duplicates! gitAuthProvidersMap := map[string]struct{}{} for _, tfResources := range tfResourcesByLabel { diff --git a/provisioner/terraform/resources_test.go b/provisioner/terraform/resources_test.go index ad96c0b0a0112..46a363dad35d7 100644 --- a/provisioner/terraform/resources_test.go +++ b/provisioner/terraform/resources_test.go @@ -2,6 +2,7 @@ package terraform_test import ( "encoding/json" + "fmt" "os" "path/filepath" "runtime" @@ -584,6 +585,69 @@ func TestAppSlugValidation(t *testing.T) { require.ErrorContains(t, err, "duplicate app slug") } +func TestParameterValidation(t *testing.T) { + t.Parallel() + + // nolint:dogsled + _, filename, _, _ := runtime.Caller(0) + + // Load the rich-parameters state file and edit it. + dir := filepath.Join(filepath.Dir(filename), "testdata", "rich-parameters") + tfPlanRaw, err := os.ReadFile(filepath.Join(dir, "rich-parameters.tfplan.json")) + require.NoError(t, err) + var tfPlan tfjson.Plan + err = json.Unmarshal(tfPlanRaw, &tfPlan) + require.NoError(t, err) + tfPlanGraph, err := os.ReadFile(filepath.Join(dir, "rich-parameters.tfplan.dot")) + require.NoError(t, err) + + // Change all names to be identical. + var names []string + for _, resource := range tfPlan.PriorState.Values.RootModule.Resources { + if resource.Type == "coder_parameter" { + resource.AttributeValues["name"] = "identical" + names = append(names, resource.Name) + } + } + + state, err := terraform.ConvertState([]*tfjson.StateModule{tfPlan.PriorState.Values.RootModule}, string(tfPlanGraph), names) + require.Nil(t, state) + require.Error(t, err) + require.ErrorContains(t, err, "coder_parameter names must be unique but \"identical\" appears multiple times") + + // Make two sets of identical names. + count := 0 + names = nil + for _, resource := range tfPlan.PriorState.Values.RootModule.Resources { + if resource.Type == "coder_parameter" { + resource.AttributeValues["name"] = fmt.Sprintf("identical-%d", count%2) + names = append(names, resource.Name) + count++ + } + } + + state, err = terraform.ConvertState([]*tfjson.StateModule{tfPlan.PriorState.Values.RootModule}, string(tfPlanGraph), names) + require.Nil(t, state) + require.Error(t, err) + require.ErrorContains(t, err, "coder_parameter names must be unique but \"identical-0\" and \"identical-1\" appear multiple times") + + // Once more with three sets. + count = 0 + names = nil + for _, resource := range tfPlan.PriorState.Values.RootModule.Resources { + if resource.Type == "coder_parameter" { + resource.AttributeValues["name"] = fmt.Sprintf("identical-%d", count%3) + names = append(names, resource.Name) + count++ + } + } + + state, err = terraform.ConvertState([]*tfjson.StateModule{tfPlan.PriorState.Values.RootModule}, string(tfPlanGraph), names) + require.Nil(t, state) + require.Error(t, err) + require.ErrorContains(t, err, "coder_parameter names must be unique but \"identical-0\", \"identical-1\" and \"identical-2\" appear multiple times") +} + func TestInstanceTypeAssociation(t *testing.T) { t.Parallel() type tc struct {