coder · dwahler · Jul 13, 2022 · Jul 13, 2022 · Jul 13, 2022 · Jul 13, 2022
diff --git a/cli/templatecreate.go b/cli/templatecreate.go
@@ -4,7 +4,6 @@ import (
 	"fmt"
 	"os"
 	"path/filepath"
-	"sort"
 	"strings"
 	"time"
 
@@ -226,10 +225,6 @@ func createValidTemplateVersion(cmd *cobra.Command, args createValidTemplateVers
 			valuesBySchemaID[parameterValue.SchemaID.String()] = parameterValue
 		}
 
-		sort.Slice(parameterSchemas, func(i, j int) bool {
-			return parameterSchemas[i].Name < parameterSchemas[j].Name
-		})
-
 		// parameterMapFromFile can be nil if parameter file is not specified
 		var parameterMapFromFile map[string]string
 		if args.ParameterFile != "" {

diff --git a/coderd/database/databasefake/databasefake.go b/coderd/database/databasefake/databasefake.go
@@ -1027,6 +1027,9 @@ func (q *fakeQuerier) GetParameterSchemasByJobID(_ context.Context, jobID uuid.U
 	if len(parameters) == 0 {
 		return nil, sql.ErrNoRows
 	}
+	sort.Slice(parameters, func(i, j int) bool {
+		return parameters[i].Index < parameters[j].Index
+	})
 	return parameters, nil
 }
 
@@ -1555,6 +1558,7 @@ func (q *fakeQuerier) InsertParameterSchema(_ context.Context, arg database.Inse
 		ValidationCondition:      arg.ValidationCondition,
 		ValidationTypeSystem:     arg.ValidationTypeSystem,
 		ValidationValueType:      arg.ValidationValueType,
+		Index:                    arg.Index,
 	}
 	q.parameterSchemas = append(q.parameterSchemas, param)
 	return param, nil

diff --git a/coderd/database/dump.sql b/coderd/database/dump.sql
diff --git a/coderd/database/migrations/000029_variable_order.down.sql b/coderd/database/migrations/000029_variable_order.down.sql
@@ -0,0 +1 @@
+ALTER TABLE parameter_schemas DROP COLUMN index;
diff --git a/coderd/database/migrations/000029_variable_order.up.sql b/coderd/database/migrations/000029_variable_order.up.sql
@@ -0,0 +1,15 @@
+-- temporarily create index column as nullable
+ALTER TABLE parameter_schemas ADD COLUMN index integer;
+
+-- initialize ordering for existing parameters
+WITH tmp AS (
+	SELECT id, row_number() OVER (PARTITION BY job_id ORDER BY name) AS index
+	FROM parameter_schemas
+)
+UPDATE parameter_schemas
+SET index = tmp.index
+FROM tmp
+WHERE parameter_schemas.id = tmp.id;
+
+-- all rows should now be initialized
+ALTER TABLE parameter_schemas ALTER COLUMN index SET NOT NULL;
diff --git a/coderd/database/models.go b/coderd/database/models.go
diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go
diff --git a/coderd/database/queries/parameterschemas.sql b/coderd/database/queries/parameterschemas.sql
@@ -4,7 +4,9 @@ SELECT
 FROM
 	parameter_schemas
 WHERE
-	job_id = $1;
+	job_id = $1
+ORDER BY
+	index;
 
 -- name: GetParameterSchemasCreatedAfter :many
 SELECT * FROM parameter_schemas WHERE created_at > $1;
@@ -27,7 +29,8 @@ INSERT INTO
 		validation_error,
 		validation_condition,
 		validation_type_system,
-		validation_value_type
+		validation_value_type,
+		index
 	)
 VALUES
 	(
@@ -46,5 +49,6 @@ VALUES
 		$13,
 		$14,
 		$15,
-		$16
+		$16,
+		$17
 	) RETURNING *;
diff --git a/coderd/provisionerdaemons.go b/coderd/provisionerdaemons.go
@@ -405,7 +405,7 @@ func (server *provisionerdServer) UpdateJob(ctx context.Context, request *proto.
 	}
 
 	if len(request.ParameterSchemas) > 0 {
-		for _, protoParameter := range request.ParameterSchemas {
+		for index, protoParameter := range request.ParameterSchemas {
 			validationTypeSystem, err := convertValidationTypeSystem(protoParameter.ValidationTypeSystem)
 			if err != nil {
 				return nil, xerrors.Errorf("convert validation type system for %q: %w", protoParameter.Name, err)
@@ -428,6 +428,8 @@ func (server *provisionerdServer) UpdateJob(ctx context.Context, request *proto.
 
 				AllowOverrideDestination: protoParameter.AllowOverrideDestination,
 				AllowOverrideSource:      protoParameter.AllowOverrideSource,
+
+				Index: int32(index),
 			}
 
 			// It's possible a parameter doesn't define a default source!

diff --git a/coderd/templateversions_test.go b/coderd/templateversions_test.go
@@ -244,17 +244,30 @@ func TestTemplateVersionParameters(t *testing.T) {
 			Parse: []*proto.Parse_Response{{
 				Type: &proto.Parse_Response_Complete{
 					Complete: &proto.Parse_Complete{
-						ParameterSchemas: []*proto.ParameterSchema{{
-							Name:           "example",
-							RedisplayValue: true,
-							DefaultSource: &proto.ParameterSource{
-								Scheme: proto.ParameterSource_DATA,
-								Value:  "hello",
+						ParameterSchemas: []*proto.ParameterSchema{
+							{
+								Name:           "example",
+								RedisplayValue: true,
+								DefaultSource: &proto.ParameterSource{
+									Scheme: proto.ParameterSource_DATA,
+									Value:  "hello",
+								},
+								DefaultDestination: &proto.ParameterDestination{
+									Scheme: proto.ParameterDestination_PROVISIONER_VARIABLE,
+								},
 							},
-							DefaultDestination: &proto.ParameterDestination{
-								Scheme: proto.ParameterDestination_PROVISIONER_VARIABLE,
+							{
+								Name:           "abcd",
+								RedisplayValue: true,
+								DefaultSource: &proto.ParameterSource{
+									Scheme: proto.ParameterSource_DATA,
+									Value:  "world",
+								},
+								DefaultDestination: &proto.ParameterDestination{
+									Scheme: proto.ParameterDestination_PROVISIONER_VARIABLE,
+								},
 							},
-						}},
+						},
 					},
 				},
 			}},
@@ -264,8 +277,9 @@ func TestTemplateVersionParameters(t *testing.T) {
 		params, err := client.TemplateVersionParameters(context.Background(), version.ID)
 		require.NoError(t, err)
 		require.NotNil(t, params)
-		require.Len(t, params, 1)
+		require.Len(t, params, 2)
 		require.Equal(t, "hello", params[0].SourceValue)
+		require.Equal(t, "world", params[1].SourceValue)
 	})
 }
 

diff --git a/provisioner/terraform/parse.go b/provisioner/terraform/parse.go
@@ -5,6 +5,7 @@ import (
 	"fmt"
 	"os"
 	"path/filepath"
+	"sort"
 	"strings"
 
 	"github.com/hashicorp/terraform-config-inspect/tfconfig"
@@ -22,8 +23,17 @@ func (*server) Parse(request *proto.Parse_Request, stream proto.DRPCProvisioner_
 		return xerrors.Errorf("load module: %s", formatDiagnostics(request.Directory, diags))
 	}
 
-	parameters := make([]*proto.ParameterSchema, 0, len(module.Variables))
+	// Sort variables by (filename, line) to make the ordering consistent
+	variables := make([]*tfconfig.Variable, 0, len(module.Variables))
 	for _, v := range module.Variables {
+		variables = append(variables, v)
+	}
+	sort.Slice(variables, func(i, j int) bool {
+		return compareSourcePos(variables[i].Pos, variables[j].Pos)
+	})
+
+	parameters := make([]*proto.ParameterSchema, 0, len(variables))
+	for _, v := range variables {
 		schema, err := convertVariableToParameter(v)
 		if err != nil {
 			return xerrors.Errorf("convert variable %q: %w", v.Name, err)
@@ -129,3 +139,10 @@ func formatDiagnostics(baseDir string, diags tfconfig.Diagnostics) string {
 
 	return spacer + strings.TrimSpace(msgs.String())
 }
+
+func compareSourcePos(x, y tfconfig.SourcePos) bool {
+	if x.Filename != y.Filename {
+		return x.Filename < y.Filename
+	}
+	return x.Line < y.Line
+}
diff --git a/provisioner/terraform/parse_test.go b/provisioner/terraform/parse_test.go
@@ -128,6 +128,58 @@ func TestParse(t *testing.T) {
 			},
 			ErrorContains: `The ";" character is not valid.`,
 		},
+		{
+			Name: "multiple-variables",
+			Files: map[string]string{
+				"main1.tf": `variable "foo" { }
+				variable "bar" { }`,
+				"main2.tf": `variable "baz" { }
+				variable "quux" { }`,
+			},
+			Response: &proto.Parse_Response{
+				Type: &proto.Parse_Response_Complete{
+					Complete: &proto.Parse_Complete{
+						ParameterSchemas: []*proto.ParameterSchema{
+							{
+								Name:                "foo",
+								RedisplayValue:      true,
+								AllowOverrideSource: true,
+								Description:         "",
+								DefaultDestination: &proto.ParameterDestination{
+									Scheme: proto.ParameterDestination_PROVISIONER_VARIABLE,
+								},
+							},
+							{
+								Name:                "bar",
+								RedisplayValue:      true,
+								AllowOverrideSource: true,
+								Description:         "",
+								DefaultDestination: &proto.ParameterDestination{
+									Scheme: proto.ParameterDestination_PROVISIONER_VARIABLE,
+								},
+							},
+							{
+								Name:                "baz",
+								RedisplayValue:      true,
+								AllowOverrideSource: true,
+								Description:         "",
+								DefaultDestination: &proto.ParameterDestination{
+									Scheme: proto.ParameterDestination_PROVISIONER_VARIABLE,
+								},
+							},
+							{
+								Name:                "quux",
+								RedisplayValue:      true,
+								AllowOverrideSource: true,
+								Description:         "",
+								DefaultDestination: &proto.ParameterDestination{
+									Scheme: proto.ParameterDestination_PROVISIONER_VARIABLE,
+								},
+							}},
+					},
+				},
+			},
+		},
 	}
 
 	for _, testCase := range testCases {
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1 @@
		ALTER TABLE parameter_schemas DROP COLUMN index;