Skip to content

Commit c7fb5f9

Browse files
mtojekmtojek
authored
feat: preserve original order of rich parameters (coder#6747)
* WIP * orderedParameters * fix * WIP * TestS
1 parent 9822745 commit c7fb5f9

File tree

5 files changed

+137
-17
lines changed

5 files changed

+137
-17
lines changed

‎provisioner/terraform/executor.go

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -259,7 +259,17 @@ func (e *executor) planResources(ctx, killCtx context.Context, planfilePath stri
259259
modules = append(modules, plan.PriorState.Values.RootModule)
260260
}
261261
modules = append(modules, plan.PlannedValues.RootModule)
262-
return ConvertState(modules, rawGraph)
262+
263+
rawParameterNames, err := rawRichParameterNames(e.workdir)
264+
if err != nil {
265+
return nil, xerrors.Errorf("raw rich parameter names: %w", err)
266+
}
267+
268+
state, err := ConvertState(modules, rawGraph, rawParameterNames)
269+
if err != nil {
270+
return nil, err
271+
}
272+
return state, nil
263273
}
264274

265275
// showPlan must only be called while the lock is held.
@@ -366,9 +376,14 @@ func (e *executor) stateResources(ctx, killCtx context.Context) (*State, error)
366376
}
367377
converted := &State{}
368378
if state.Values != nil {
379+
rawParameterNames, err := rawRichParameterNames(e.workdir)
380+
if err != nil {
381+
return nil, xerrors.Errorf("raw rich parameter names: %w", err)
382+
}
383+
369384
converted, err = ConvertState([]*tfjson.StateModule{
370385
state.Values.RootModule,
371-
}, rawGraph)
386+
}, rawGraph, rawParameterNames)
372387
if err != nil {
373388
return nil, err
374389
}

‎provisioner/terraform/parameters.go

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
package terraform
2+
3+
import (
4+
"fmt"
5+
"os"
6+
"path"
7+
"strings"
8+
9+
"github.com/hashicorp/hcl/v2"
10+
"github.com/hashicorp/hcl/v2/hclparse"
11+
)
12+
13+
var terraformWithCoderParametersSchema = &hcl.BodySchema{
14+
Blocks: []hcl.BlockHeaderSchema{
15+
{
16+
Type: "data",
17+
LabelNames: []string{"coder_parameter", "*"},
18+
},
19+
},
20+
}
21+
22+
func rawRichParameterNames(workdir string) ([]string, error) {
23+
entries, err := os.ReadDir(workdir)
24+
if err != nil {
25+
return nil, err
26+
}
27+
28+
var coderParameterNames []string
29+
for _, entry := range entries {
30+
if !strings.HasSuffix(entry.Name(), ".tf") {
31+
continue
32+
}
33+
34+
hclFilepath := path.Join(workdir, entry.Name())
35+
parser := hclparse.NewParser()
36+
parsedHCL, diags := parser.ParseHCLFile(hclFilepath)
37+
if diags.HasErrors() {
38+
return nil, hcl.Diagnostics{
39+
{
40+
Severity: hcl.DiagError,
41+
Summary: "Failed to parse HCL file",
42+
Detail: fmt.Sprintf("parser.ParseHCLFile can't parse %q file", hclFilepath),
43+
},
44+
}
45+
}
46+
47+
content, _, _ := parsedHCL.Body.PartialContent(terraformWithCoderParametersSchema)
48+
for _, block := range content.Blocks {
49+
if block.Type == "data" && block.Labels[0] == "coder_parameter" && len(block.Labels) == 2 {
50+
coderParameterNames = append(coderParameterNames, block.Labels[1])
51+
}
52+
}
53+
}
54+
return coderParameterNames, nil
55+
}

‎provisioner/terraform/provision_test.go

Lines changed: 32 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -328,11 +328,17 @@ func TestProvision(t *testing.T) {
328328
required_providers {
329329
coder = {
330330
source = "coder/coder"
331-
version = "0.6.6"
331+
version = "0.6.20"
332332
}
333333
}
334334
}
335335
336+
data "coder_parameter" "sample" {
337+
name = "Sample"
338+
type = "string"
339+
default = "foobaz"
340+
}
341+
336342
data "coder_parameter" "example" {
337343
name = "Example"
338344
type = "string"
@@ -347,6 +353,10 @@ func TestProvision(t *testing.T) {
347353
},
348354
Request: &proto.Provision_Plan{
349355
RichParameterValues: []*proto.RichParameterValue{
356+
{
357+
Name: "Sample",
358+
Value: "foofoo",
359+
},
350360
{
351361
Name: "Example",
352362
Value: "foobaz",
@@ -356,6 +366,18 @@ func TestProvision(t *testing.T) {
356366
Response: &proto.Provision_Response{
357367
Type: &proto.Provision_Response_Complete{
358368
Complete: &proto.Provision_Complete{
369+
Parameters: []*proto.RichParameter{
370+
{
371+
Name: "Sample",
372+
Type: "string",
373+
DefaultValue: "foobaz",
374+
},
375+
{
376+
Name: "Example",
377+
Type: "string",
378+
DefaultValue: "foobar",
379+
},
380+
},
359381
Resources: []*proto.Resource{{
360382
Name: "example",
361383
Type: "null_resource",
@@ -441,6 +463,7 @@ func TestProvision(t *testing.T) {
441463
planRequest.GetPlan().Config = &proto.Provision_Config{}
442464
}
443465
planRequest.GetPlan().ParameterValues = testCase.Request.ParameterValues
466+
planRequest.GetPlan().RichParameterValues = testCase.Request.RichParameterValues
444467
planRequest.GetPlan().GitAuthProviders = testCase.Request.GitAuthProviders
445468
if testCase.Request.Config != nil {
446469
planRequest.GetPlan().Config.State = testCase.Request.Config.State
@@ -499,15 +522,20 @@ func TestProvision(t *testing.T) {
499522
}
500523

501524
if testCase.Response != nil {
525+
require.Equal(t, testCase.Response.GetComplete().Error, msg.GetComplete().Error)
526+
502527
resourcesGot, err := json.Marshal(msg.GetComplete().Resources)
503528
require.NoError(t, err)
504-
505529
resourcesWant, err := json.Marshal(testCase.Response.GetComplete().Resources)
506530
require.NoError(t, err)
507531

508-
require.Equal(t, testCase.Response.GetComplete().Error, msg.GetComplete().Error)
509-
510532
require.Equal(t, string(resourcesWant), string(resourcesGot))
533+
534+
parametersGot, err := json.Marshal(msg.GetComplete().Parameters)
535+
require.NoError(t, err)
536+
parametersWant, err := json.Marshal(testCase.Response.GetComplete().Parameters)
537+
require.NoError(t, err)
538+
require.Equal(t, string(parametersWant), string(parametersGot))
511539
}
512540
break
513541
}

‎provisioner/terraform/resources.go

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ type State struct {
8383
// ConvertState consumes Terraform state and a GraphViz representation
8484
// produced by `terraform graph` to produce resources consumable by Coder.
8585
// nolint:gocyclo
86-
func ConvertState(modules []*tfjson.StateModule, rawGraph string) (*State, error) {
86+
func ConvertState(modules []*tfjson.StateModule, rawGraph string, rawParameterNames []string) (*State, error) {
8787
parsedGraph, err := gographviz.ParseString(rawGraph)
8888
if err != nil {
8989
return nil, xerrors.Errorf("parse graph: %w", err)
@@ -442,10 +442,7 @@ func ConvertState(modules []*tfjson.StateModule, rawGraph string) (*State, error
442442
}
443443

444444
parameters := make([]*proto.RichParameter, 0)
445-
for _, resource := range tfResourcesRichParameters {
446-
if resource.Type != "coder_parameter" {
447-
continue
448-
}
445+
for _, resource := range orderedRichParametersResources(tfResourcesRichParameters, rawParameterNames) {
449446
var param provider.Parameter
450447
err = mapstructure.Decode(resource.AttributeValues, &param)
451448
if err != nil {
@@ -629,3 +626,19 @@ func findResourcesInGraph(graph *gographviz.Graph, tfResourcesByLabel map[string
629626

630627
return graphResources
631628
}
629+
630+
func orderedRichParametersResources(tfResourcesRichParameters []*tfjson.StateResource, orderedNames []string) []*tfjson.StateResource {
631+
if len(orderedNames) == 0 {
632+
return tfResourcesRichParameters
633+
}
634+
635+
ordered := make([]*tfjson.StateResource, len(orderedNames))
636+
for i, name := range orderedNames {
637+
for _, resource := range tfResourcesRichParameters {
638+
if resource.Name == name {
639+
ordered[i] = resource
640+
}
641+
}
642+
}
643+
return ordered
644+
}

‎provisioner/terraform/resources_test.go

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"path/filepath"
77
"runtime"
88
"sort"
9+
"strings"
910
"testing"
1011

1112
protobuf "github.com/golang/protobuf/proto"
@@ -349,7 +350,7 @@ func TestConvertResources(t *testing.T) {
349350
// and that no errors occur!
350351
modules = append(modules, tfPlan.PlannedValues.RootModule)
351352
}
352-
state, err := terraform.ConvertState(modules, string(tfPlanGraph))
353+
state, err := terraform.ConvertState(modules, string(tfPlanGraph), richParameterResourceNames(expected.parameters))
353354
require.NoError(t, err)
354355
sortResources(state.Resources)
355356
sort.Strings(state.GitAuthProviders)
@@ -402,7 +403,7 @@ func TestConvertResources(t *testing.T) {
402403
tfStateGraph, err := os.ReadFile(filepath.Join(dir, folderName+".tfstate.dot"))
403404
require.NoError(t, err)
404405

405-
state, err := terraform.ConvertState([]*tfjson.StateModule{tfState.Values.RootModule}, string(tfStateGraph))
406+
state, err := terraform.ConvertState([]*tfjson.StateModule{tfState.Values.RootModule}, string(tfStateGraph), richParameterResourceNames(expected.parameters))
406407
require.NoError(t, err)
407408
sortResources(state.Resources)
408409
sort.Strings(state.GitAuthProviders)
@@ -461,7 +462,7 @@ func TestAppSlugValidation(t *testing.T) {
461462
}
462463
}
463464

464-
state, err := terraform.ConvertState([]*tfjson.StateModule{tfPlan.PlannedValues.RootModule}, string(tfPlanGraph))
465+
state, err := terraform.ConvertState([]*tfjson.StateModule{tfPlan.PlannedValues.RootModule}, string(tfPlanGraph), nil)
465466
require.Nil(t, state)
466467
require.Error(t, err)
467468
require.ErrorContains(t, err, "invalid app slug")
@@ -473,7 +474,7 @@ func TestAppSlugValidation(t *testing.T) {
473474
}
474475
}
475476

476-
state, err = terraform.ConvertState([]*tfjson.StateModule{tfPlan.PlannedValues.RootModule}, string(tfPlanGraph))
477+
state, err = terraform.ConvertState([]*tfjson.StateModule{tfPlan.PlannedValues.RootModule}, string(tfPlanGraph), nil)
477478
require.Nil(t, state)
478479
require.Error(t, err)
479480
require.ErrorContains(t, err, "duplicate app slug")
@@ -523,7 +524,7 @@ func TestInstanceTypeAssociation(t *testing.T) {
523524
subgraph "root" {
524525
"[root] `+tc.ResourceType+`.dev" [label = "`+tc.ResourceType+`.dev", shape = "box"]
525526
}
526-
}`)
527+
}`, nil)
527528
require.NoError(t, err)
528529
require.Len(t, state.Resources, 1)
529530
require.Equal(t, state.Resources[0].GetInstanceType(), instanceType)
@@ -594,7 +595,7 @@ func TestInstanceIDAssociation(t *testing.T) {
594595
"[root] `+tc.ResourceType+`.dev" -> "[root] coder_agent.dev"
595596
}
596597
}
597-
`)
598+
`, nil)
598599
require.NoError(t, err)
599600
require.Len(t, state.Resources, 1)
600601
require.Len(t, state.Resources[0].Agents, 1)
@@ -623,3 +624,11 @@ func sortResources(resources []*proto.Resource) {
623624
})
624625
}
625626
}
627+
628+
func richParameterResourceNames(parameters []*proto.RichParameter) []string {
629+
var names []string
630+
for _, p := range parameters {
631+
names = append(names, strings.ToLower(p.Name))
632+
}
633+
return names
634+
}

0 commit comments

Comments
 (0)