Skip to content

Commit b5f5e90

Browse files
dwahlerdwahler
authored
Return template parameters in consistent order (#2975)
* return parameters from Terraform provisioner in sorted order * persist parameter indices in database and return them in correct order from API * don't re-sort parameters by name when creating templates
1 parent b692b7e commit b5f5e90

File tree

12 files changed

+141
-26
lines changed

12 files changed

+141
-26
lines changed

‎cli/templatecreate.go

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ import (
44
"fmt"
55
"os"
66
"path/filepath"
7-
"sort"
87
"strings"
98
"time"
109

@@ -226,10 +225,6 @@ func createValidTemplateVersion(cmd *cobra.Command, args createValidTemplateVers
226225
valuesBySchemaID[parameterValue.SchemaID.String()] = parameterValue
227226
}
228227

229-
sort.Slice(parameterSchemas, func(i, j int) bool {
230-
return parameterSchemas[i].Name < parameterSchemas[j].Name
231-
})
232-
233228
// parameterMapFromFile can be nil if parameter file is not specified
234229
var parameterMapFromFile map[string]string
235230
if args.ParameterFile != "" {

‎coderd/database/databasefake/databasefake.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1027,6 +1027,9 @@ func (q *fakeQuerier) GetParameterSchemasByJobID(_ context.Context, jobID uuid.U
10271027
if len(parameters) == 0 {
10281028
return nil, sql.ErrNoRows
10291029
}
1030+
sort.Slice(parameters, func(i, j int) bool {
1031+
return parameters[i].Index < parameters[j].Index
1032+
})
10301033
return parameters, nil
10311034
}
10321035

@@ -1555,6 +1558,7 @@ func (q *fakeQuerier) InsertParameterSchema(_ context.Context, arg database.Inse
15551558
ValidationCondition: arg.ValidationCondition,
15561559
ValidationTypeSystem: arg.ValidationTypeSystem,
15571560
ValidationValueType: arg.ValidationValueType,
1561+
Index: arg.Index,
15581562
}
15591563
q.parameterSchemas = append(q.parameterSchemas, param)
15601564
return param, nil

‎coderd/database/dump.sql

Lines changed: 2 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/migrations/000029_variable_order.down.sql

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
ALTER TABLE parameter_schemas DROP COLUMN index;

‎coderd/database/migrations/000029_variable_order.up.sql

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
-- temporarily create index column as nullable
2+
ALTER TABLE parameter_schemas ADD COLUMN index integer;
3+
4+
-- initialize ordering for existing parameters
5+
WITH tmp AS (
6+
SELECT id, row_number() OVER (PARTITION BY job_id ORDER BY name) AS index
7+
FROM parameter_schemas
8+
)
9+
UPDATE parameter_schemas
10+
SET index = tmp.index
11+
FROM tmp
12+
WHERE parameter_schemas.id = tmp.id;
13+
14+
-- all rows should now be initialized
15+
ALTER TABLE parameter_schemas ALTER COLUMN index SET NOT NULL;

‎coderd/database/models.go

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/queries.sql.go

Lines changed: 14 additions & 5 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/queries/parameterschemas.sql

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,9 @@ SELECT
44
FROM
55
parameter_schemas
66
WHERE
7-
job_id = $1;
7+
job_id = $1
8+
ORDER BY
9+
index;
810

911
-- name: GetParameterSchemasCreatedAfter :many
1012
SELECT * FROM parameter_schemas WHERE created_at > $1;
@@ -27,7 +29,8 @@ INSERT INTO
2729
validation_error,
2830
validation_condition,
2931
validation_type_system,
30-
validation_value_type
32+
validation_value_type,
33+
index
3134
)
3235
VALUES
3336
(
@@ -46,5 +49,6 @@ VALUES
4649
$13,
4750
$14,
4851
$15,
49-
$16
52+
$16,
53+
$17
5054
) RETURNING *;

‎coderd/provisionerdaemons.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -405,7 +405,7 @@ func (server *provisionerdServer) UpdateJob(ctx context.Context, request *proto.
405405
}
406406

407407
if len(request.ParameterSchemas) > 0 {
408-
for _, protoParameter := range request.ParameterSchemas {
408+
for index, protoParameter := range request.ParameterSchemas {
409409
validationTypeSystem, err := convertValidationTypeSystem(protoParameter.ValidationTypeSystem)
410410
if err != nil {
411411
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.
428428

429429
AllowOverrideDestination: protoParameter.AllowOverrideDestination,
430430
AllowOverrideSource: protoParameter.AllowOverrideSource,
431+
432+
Index: int32(index),
431433
}
432434

433435
// It's possible a parameter doesn't define a default source!

‎coderd/templateversions_test.go

Lines changed: 24 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -244,17 +244,30 @@ func TestTemplateVersionParameters(t *testing.T) {
244244
Parse: []*proto.Parse_Response{{
245245
Type: &proto.Parse_Response_Complete{
246246
Complete: &proto.Parse_Complete{
247-
ParameterSchemas: []*proto.ParameterSchema{{
248-
Name: "example",
249-
RedisplayValue: true,
250-
DefaultSource: &proto.ParameterSource{
251-
Scheme: proto.ParameterSource_DATA,
252-
Value: "hello",
247+
ParameterSchemas: []*proto.ParameterSchema{
248+
{
249+
Name: "example",
250+
RedisplayValue: true,
251+
DefaultSource: &proto.ParameterSource{
252+
Scheme: proto.ParameterSource_DATA,
253+
Value: "hello",
254+
},
255+
DefaultDestination: &proto.ParameterDestination{
256+
Scheme: proto.ParameterDestination_PROVISIONER_VARIABLE,
257+
},
253258
},
254-
DefaultDestination: &proto.ParameterDestination{
255-
Scheme: proto.ParameterDestination_PROVISIONER_VARIABLE,
259+
{
260+
Name: "abcd",
261+
RedisplayValue: true,
262+
DefaultSource: &proto.ParameterSource{
263+
Scheme: proto.ParameterSource_DATA,
264+
Value: "world",
265+
},
266+
DefaultDestination: &proto.ParameterDestination{
267+
Scheme: proto.ParameterDestination_PROVISIONER_VARIABLE,
268+
},
256269
},
257-
}},
270+
},
258271
},
259272
},
260273
}},
@@ -264,8 +277,9 @@ func TestTemplateVersionParameters(t *testing.T) {
264277
params, err := client.TemplateVersionParameters(context.Background(), version.ID)
265278
require.NoError(t, err)
266279
require.NotNil(t, params)
267-
require.Len(t, params, 1)
280+
require.Len(t, params, 2)
268281
require.Equal(t, "hello", params[0].SourceValue)
282+
require.Equal(t, "world", params[1].SourceValue)
269283
})
270284
}
271285

0 commit comments

Comments
 (0)