From 0a5c90dc572dc23e1b895c98492771cec12ba235 Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Wed, 23 Aug 2023 11:07:14 +0200 Subject: [PATCH 1/3] fix: rewrite onlyDataResources --- provisioner/terraform/executor.go | 22 +++---- .../terraform/executor_internal_test.go | 58 +++++++++++++++++++ 2 files changed, 70 insertions(+), 10 deletions(-) diff --git a/provisioner/terraform/executor.go b/provisioner/terraform/executor.go index f006dcd149daa..cd847910e4c0f 100644 --- a/provisioner/terraform/executor.go +++ b/provisioner/terraform/executor.go @@ -264,18 +264,21 @@ func (e *executor) plan(ctx, killCtx context.Context, env, vars []string, logr l }, nil } -func onlyDataResources(sm *tfjson.StateModule) *tfjson.StateModule { - sm2 := *sm - sm2.Resources = make([]*tfjson.StateResource, 0, len(sm.Resources)) +func onlyDataResources(sm tfjson.StateModule) tfjson.StateModule { + filtered := sm + filtered.Resources = []*tfjson.StateResource{} for _, r := range sm.Resources { if r.Mode == "data" { - sm2.Resources = append(sm2.Resources, r) + filtered.Resources = append(filtered.Resources, r) } } + + filtered.ChildModules = []*tfjson.StateModule{} for _, c := range sm.ChildModules { - sm2.ChildModules = append(sm2.ChildModules, onlyDataResources(c)) + filteredChild := onlyDataResources(*c) + filtered.ChildModules = append(filtered.ChildModules, &filteredChild) } - return &sm2 + return filtered } // planResources must only be called while the lock is held. @@ -300,10 +303,9 @@ func (e *executor) planResources(ctx, killCtx context.Context, planfilePath stri // We don't want all prior resources, because Quotas (and // future features) would never know which resources are getting // deleted by a stop. - modules = append( - modules, - onlyDataResources(plan.PriorState.Values.RootModule), - ) + + filtered := onlyDataResources(*plan.PriorState.Values.RootModule) + modules = append(modules, &filtered) } modules = append(modules, plan.PlannedValues.RootModule) diff --git a/provisioner/terraform/executor_internal_test.go b/provisioner/terraform/executor_internal_test.go index 18f130f57bc3b..1a534478da596 100644 --- a/provisioner/terraform/executor_internal_test.go +++ b/provisioner/terraform/executor_internal_test.go @@ -3,6 +3,7 @@ package terraform import ( "testing" + tfjson "github.com/hashicorp/terraform-json" "github.com/stretchr/testify/require" "github.com/coder/coder/v2/provisionersdk/proto" @@ -41,3 +42,60 @@ From standing in the English rain`)) } require.Equal(t, expected, logr.logs) } + +func TestOnlyDataResources(t *testing.T) { + t.Parallel() + + sm := &tfjson.StateModule{ + Resources: []*tfjson.StateResource{ + { + Name: "cat", + Type: "coder_parameter", + Mode: "data", + Address: "cat-address", + }, + { + Name: "dog", + Type: "foobar", + Mode: "magic", + Address: "dog-address", + }, + { + Name: "cow", + Type: "foobaz", + Mode: "data", + Address: "cow-address", + }, + }, + ChildModules: []*tfjson.StateModule{ + { + Resources: []*tfjson.StateResource{ + { + Name: "child-cat", + Type: "coder_parameter", + Mode: "data", + Address: "child-cat-address", + }, + { + Name: "child-dog", + Type: "foobar", + Mode: "data", + Address: "child-dog-address", + }, + { + Name: "child-cow", + Mode: "data", + Type: "magic", + Address: "child-cow-address", + }, + }, + Address: "child-module-1", + }, + }, + Address: "fake-module", + } + + filtered := onlyDataResources(*sm) + require.Len(t, filtered.Resources, 2) + require.Len(t, filtered.ChildModules, 1) +} From ead334f094f6f0b24cf845764ed4620f4eae0159 Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Wed, 23 Aug 2023 11:23:49 +0200 Subject: [PATCH 2/3] Refactor --- provisioner/terraform/executor_internal_test.go | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/provisioner/terraform/executor_internal_test.go b/provisioner/terraform/executor_internal_test.go index 1a534478da596..5a53a18af0845 100644 --- a/provisioner/terraform/executor_internal_test.go +++ b/provisioner/terraform/executor_internal_test.go @@ -84,8 +84,8 @@ func TestOnlyDataResources(t *testing.T) { }, { Name: "child-cow", - Mode: "data", - Type: "magic", + Type: "foobaz", + Mode: "magic", Address: "child-cow-address", }, }, @@ -97,5 +97,10 @@ func TestOnlyDataResources(t *testing.T) { filtered := onlyDataResources(*sm) require.Len(t, filtered.Resources, 2) + require.Equal(t, sm.Resources[0], filtered.Resources[0]) + require.Equal(t, sm.Resources[2], filtered.Resources[1]) require.Len(t, filtered.ChildModules, 1) + require.Len(t, filtered.ChildModules[0].Resources, 2) + require.Equal(t, sm.ChildModules[0].Resources[0], filtered.ChildModules[0].Resources[0]) + require.Equal(t, sm.ChildModules[0].Resources[1], filtered.ChildModules[0].Resources[1]) } From 3a8db8d2199d57e5492332d09630bb1ccfa4c707 Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Wed, 23 Aug 2023 12:06:30 +0200 Subject: [PATCH 3/3] Use table test --- .../terraform/executor_internal_test.go | 153 +++++++++++++----- 1 file changed, 111 insertions(+), 42 deletions(-) diff --git a/provisioner/terraform/executor_internal_test.go b/provisioner/terraform/executor_internal_test.go index 5a53a18af0845..fd203c9b1ee5f 100644 --- a/provisioner/terraform/executor_internal_test.go +++ b/provisioner/terraform/executor_internal_test.go @@ -1,6 +1,7 @@ package terraform import ( + "encoding/json" "testing" tfjson "github.com/hashicorp/terraform-json" @@ -46,61 +47,129 @@ From standing in the English rain`)) func TestOnlyDataResources(t *testing.T) { t.Parallel() - sm := &tfjson.StateModule{ - Resources: []*tfjson.StateResource{ - { - Name: "cat", - Type: "coder_parameter", - Mode: "data", - Address: "cat-address", - }, - { - Name: "dog", - Type: "foobar", - Mode: "magic", - Address: "dog-address", + tests := []struct { + name string + stateMod *tfjson.StateModule + expected *tfjson.StateModule + }{ + { + name: "empty state module", + stateMod: &tfjson.StateModule{}, + expected: &tfjson.StateModule{}, + }, + { + name: "only data resources", + stateMod: &tfjson.StateModule{ + Resources: []*tfjson.StateResource{ + {Name: "cat", Type: "coder_parameter", Mode: "data", Address: "cat-address"}, + {Name: "cow", Type: "foobaz", Mode: "data", Address: "cow-address"}, + }, + ChildModules: []*tfjson.StateModule{ + { + Resources: []*tfjson.StateResource{ + {Name: "child-cat", Type: "coder_parameter", Mode: "data", Address: "child-cat-address"}, + {Name: "child-dog", Type: "foobar", Mode: "data", Address: "child-dog-address"}, + }, + Address: "child-module-1", + }, + }, + Address: "fake-module", }, - { - Name: "cow", - Type: "foobaz", - Mode: "data", - Address: "cow-address", + expected: &tfjson.StateModule{ + Resources: []*tfjson.StateResource{ + {Name: "cat", Type: "coder_parameter", Mode: "data", Address: "cat-address"}, + {Name: "cow", Type: "foobaz", Mode: "data", Address: "cow-address"}, + }, + ChildModules: []*tfjson.StateModule{ + { + Resources: []*tfjson.StateResource{ + {Name: "child-cat", Type: "coder_parameter", Mode: "data", Address: "child-cat-address"}, + {Name: "child-dog", Type: "foobar", Mode: "data", Address: "child-dog-address"}, + }, + Address: "child-module-1", + }, + }, + Address: "fake-module", }, }, - ChildModules: []*tfjson.StateModule{ - { + { + name: "only non-data resources", + stateMod: &tfjson.StateModule{ Resources: []*tfjson.StateResource{ + {Name: "cat", Type: "coder_parameter", Mode: "foobar", Address: "cat-address"}, + {Name: "cow", Type: "foobaz", Mode: "foo", Address: "cow-address"}, + }, + ChildModules: []*tfjson.StateModule{ { - Name: "child-cat", - Type: "coder_parameter", - Mode: "data", - Address: "child-cat-address", + Resources: []*tfjson.StateResource{ + {Name: "child-cat", Type: "coder_parameter", Mode: "foobar", Address: "child-cat-address"}, + {Name: "child-dog", Type: "foobar", Mode: "foobaz", Address: "child-dog-address"}, + }, + Address: "child-module-1", }, + }, + Address: "fake-module", + }, + expected: &tfjson.StateModule{ + Address: "fake-module", + ChildModules: []*tfjson.StateModule{ + {Address: "child-module-1"}, + }, + }, + }, + { + name: "mixed resources", + stateMod: &tfjson.StateModule{ + Resources: []*tfjson.StateResource{ + {Name: "cat", Type: "coder_parameter", Mode: "data", Address: "cat-address"}, + {Name: "dog", Type: "foobar", Mode: "magic", Address: "dog-address"}, + {Name: "cow", Type: "foobaz", Mode: "data", Address: "cow-address"}, + }, + ChildModules: []*tfjson.StateModule{ { - Name: "child-dog", - Type: "foobar", - Mode: "data", - Address: "child-dog-address", + Resources: []*tfjson.StateResource{ + {Name: "child-cat", Type: "coder_parameter", Mode: "data", Address: "child-cat-address"}, + {Name: "child-dog", Type: "foobar", Mode: "data", Address: "child-dog-address"}, + {Name: "child-cow", Type: "foobaz", Mode: "magic", Address: "child-cow-address"}, + }, + Address: "child-module-1", }, + }, + Address: "fake-module", + }, + expected: &tfjson.StateModule{ + Resources: []*tfjson.StateResource{ + {Name: "cat", Type: "coder_parameter", Mode: "data", Address: "cat-address"}, + {Name: "cow", Type: "foobaz", Mode: "data", Address: "cow-address"}, + }, + ChildModules: []*tfjson.StateModule{ { - Name: "child-cow", - Type: "foobaz", - Mode: "magic", - Address: "child-cow-address", + Resources: []*tfjson.StateResource{ + {Name: "child-cat", Type: "coder_parameter", Mode: "data", Address: "child-cat-address"}, + {Name: "child-dog", Type: "foobar", Mode: "data", Address: "child-dog-address"}, + }, + Address: "child-module-1", }, }, - Address: "child-module-1", + Address: "fake-module", }, }, - Address: "fake-module", } - filtered := onlyDataResources(*sm) - require.Len(t, filtered.Resources, 2) - require.Equal(t, sm.Resources[0], filtered.Resources[0]) - require.Equal(t, sm.Resources[2], filtered.Resources[1]) - require.Len(t, filtered.ChildModules, 1) - require.Len(t, filtered.ChildModules[0].Resources, 2) - require.Equal(t, sm.ChildModules[0].Resources[0], filtered.ChildModules[0].Resources[0]) - require.Equal(t, sm.ChildModules[0].Resources[1], filtered.ChildModules[0].Resources[1]) + for _, tt := range tests { + tt := tt + + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + filtered := onlyDataResources(*tt.stateMod) + + expected, err := json.Marshal(tt.expected) + require.NoError(t, err) + got, err := json.Marshal(filtered) + require.NoError(t, err) + + require.Equal(t, string(expected), string(got)) + }) + } }