From d51921f08557644bcb61acf3c46bbd78a3ea3c11 Mon Sep 17 00:00:00 2001 From: Martin Atkins Date: Tue, 31 Aug 2021 10:16:44 -0700 Subject: [PATCH] core: Provider transformers don't use the set of all available providers In earlier incarnations of these transformers we used the set of all available providers for tasks such as generating implied provider configuration nodes. However, in modern Terraform we can extract all of the information we need from the configuration itself, and so these transformers weren't actually using this set of provider addresses. These also ended up getting left behind as sets of string rather than sets of addrs.Provider in our earlier refactoring work, which didn't really matter because the result wasn't used anywhere anyway. Rather than updating these to use addrs.Provider instead, I've just removed the unused arguments entirely in the hope of making it easier to see what inputs these transformers use to make their decisions. --- internal/terraform/graph_builder_apply.go | 2 +- .../terraform/graph_builder_destroy_plan.go | 2 +- internal/terraform/graph_builder_eval.go | 2 +- internal/terraform/graph_builder_import.go | 2 +- internal/terraform/graph_builder_plan.go | 2 +- internal/terraform/transform_provider.go | 18 +++++--------- internal/terraform/transform_provider_test.go | 24 +++++++++---------- internal/terraform/transform_root_test.go | 4 +--- 8 files changed, 24 insertions(+), 32 deletions(-) diff --git a/internal/terraform/graph_builder_apply.go b/internal/terraform/graph_builder_apply.go index 683353cf9..bf213d1d0 100644 --- a/internal/terraform/graph_builder_apply.go +++ b/internal/terraform/graph_builder_apply.go @@ -116,7 +116,7 @@ func (b *ApplyGraphBuilder) Steps() []GraphTransformer { &AttachResourceConfigTransformer{Config: b.Config}, // add providers - TransformProviders(b.Components.ResourceProviders(), concreteProvider, b.Config), + transformProviders(concreteProvider, b.Config), // Remove modules no longer present in the config &RemovedModuleTransformer{Config: b.Config, State: b.State}, diff --git a/internal/terraform/graph_builder_destroy_plan.go b/internal/terraform/graph_builder_destroy_plan.go index 9962442af..1973237b4 100644 --- a/internal/terraform/graph_builder_destroy_plan.go +++ b/internal/terraform/graph_builder_destroy_plan.go @@ -94,7 +94,7 @@ func (b *DestroyPlanGraphBuilder) Steps() []GraphTransformer { // Attach the configuration to any resources &AttachResourceConfigTransformer{Config: b.Config}, - TransformProviders(b.Components.ResourceProviders(), concreteProvider, b.Config), + transformProviders(concreteProvider, b.Config), // Destruction ordering. We require this only so that // targeting below will prune the correct things. diff --git a/internal/terraform/graph_builder_eval.go b/internal/terraform/graph_builder_eval.go index 18b6d5199..65e663550 100644 --- a/internal/terraform/graph_builder_eval.go +++ b/internal/terraform/graph_builder_eval.go @@ -75,7 +75,7 @@ func (b *EvalGraphBuilder) Steps() []GraphTransformer { // Attach the state &AttachStateTransformer{State: b.State}, - TransformProviders(b.Components.ResourceProviders(), concreteProvider, b.Config), + transformProviders(concreteProvider, b.Config), // Must attach schemas before ReferenceTransformer so that we can // analyze the configuration to find references. diff --git a/internal/terraform/graph_builder_import.go b/internal/terraform/graph_builder_import.go index af5df1403..bbef67713 100644 --- a/internal/terraform/graph_builder_import.go +++ b/internal/terraform/graph_builder_import.go @@ -67,7 +67,7 @@ func (b *ImportGraphBuilder) Steps() []GraphTransformer { // Add the import steps &ImportStateTransformer{Targets: b.ImportTargets, Config: b.Config}, - TransformProviders(b.Components.ResourceProviders(), concreteProvider, config), + transformProviders(concreteProvider, config), // Must attach schemas before ReferenceTransformer so that we can // analyze the configuration to find references. diff --git a/internal/terraform/graph_builder_plan.go b/internal/terraform/graph_builder_plan.go index 84884c3d4..8d6801624 100644 --- a/internal/terraform/graph_builder_plan.go +++ b/internal/terraform/graph_builder_plan.go @@ -130,7 +130,7 @@ func (b *PlanGraphBuilder) Steps() []GraphTransformer { &AttachResourceConfigTransformer{Config: b.Config}, // add providers - TransformProviders(b.Components.ResourceProviders(), b.ConcreteProvider, b.Config), + transformProviders(b.ConcreteProvider, b.Config), // Remove modules no longer present in the config &RemovedModuleTransformer{Config: b.Config, State: b.State}, diff --git a/internal/terraform/transform_provider.go b/internal/terraform/transform_provider.go index ee5087eaf..3e459e7a7 100644 --- a/internal/terraform/transform_provider.go +++ b/internal/terraform/transform_provider.go @@ -11,19 +11,17 @@ import ( "github.com/hashicorp/terraform/internal/tfdiags" ) -func TransformProviders(providers []string, concrete ConcreteProviderNodeFunc, config *configs.Config) GraphTransformer { +func transformProviders(concrete ConcreteProviderNodeFunc, config *configs.Config) GraphTransformer { return GraphTransformMulti( // Add providers from the config &ProviderConfigTransformer{ - Config: config, - Providers: providers, - Concrete: concrete, + Config: config, + Concrete: concrete, }, // Add any remaining missing providers &MissingProviderTransformer{ - Config: config, - Providers: providers, - Concrete: concrete, + Config: config, + Concrete: concrete, }, // Connect the providers &ProviderTransformer{ @@ -298,9 +296,6 @@ func (t *CloseProviderTransformer) Transform(g *Graph) error { // PruneProviderTransformer can then remove these once ProviderTransformer // has resolved all of the inheritence, etc. type MissingProviderTransformer struct { - // Providers is the list of providers we support. - Providers []string - // MissingProviderTransformer needs the config to rule out _implied_ default providers Config *configs.Config @@ -478,8 +473,7 @@ func (n *graphNodeProxyProvider) Target() GraphNodeProvider { // ProviderConfigTransformer adds all provider nodes from the configuration and // attaches the configs. type ProviderConfigTransformer struct { - Providers []string - Concrete ConcreteProviderNodeFunc + Concrete ConcreteProviderNodeFunc // each provider node is stored here so that the proxy nodes can look up // their targets by name. diff --git a/internal/terraform/transform_provider_test.go b/internal/terraform/transform_provider_test.go index 596a39735..0436fc032 100644 --- a/internal/terraform/transform_provider_test.go +++ b/internal/terraform/transform_provider_test.go @@ -31,7 +31,7 @@ func TestProviderTransformer(t *testing.T) { g := testProviderTransformerGraph(t, mod) { - transform := &MissingProviderTransformer{Providers: []string{"aws"}} + transform := &MissingProviderTransformer{} if err := transform.Transform(g); err != nil { t.Fatalf("err: %s", err) } @@ -79,7 +79,7 @@ func TestProviderTransformer_ImportModuleChild(t *testing.T) { } { - tf := &MissingProviderTransformer{Providers: []string{"foo", "bar"}} + tf := &MissingProviderTransformer{} if err := tf.Transform(g); err != nil { t.Fatalf("err: %s", err) } @@ -108,7 +108,7 @@ func TestProviderTransformer_fqns(t *testing.T) { g := testProviderTransformerGraph(t, mod) { - transform := &MissingProviderTransformer{Providers: []string{"aws"}, Config: mod} + transform := &MissingProviderTransformer{Config: mod} if err := transform.Transform(g); err != nil { t.Fatalf("err: %s", err) } @@ -132,7 +132,7 @@ func TestCloseProviderTransformer(t *testing.T) { g := testProviderTransformerGraph(t, mod) { - transform := &MissingProviderTransformer{Providers: []string{"aws"}} + transform := &MissingProviderTransformer{} if err := transform.Transform(g); err != nil { t.Fatalf("err: %s", err) } @@ -164,7 +164,7 @@ func TestCloseProviderTransformer_withTargets(t *testing.T) { g := testProviderTransformerGraph(t, mod) transforms := []GraphTransformer{ - &MissingProviderTransformer{Providers: []string{"aws"}}, + &MissingProviderTransformer{}, &ProviderTransformer{}, &CloseProviderTransformer{}, &TargetsTransformer{ @@ -194,7 +194,7 @@ func TestMissingProviderTransformer(t *testing.T) { g := testProviderTransformerGraph(t, mod) { - transform := &MissingProviderTransformer{Providers: []string{"aws", "foo", "bar"}} + transform := &MissingProviderTransformer{} if err := transform.Transform(g); err != nil { t.Fatalf("err: %s", err) } @@ -228,7 +228,7 @@ func TestMissingProviderTransformer_grandchildMissing(t *testing.T) { g := testProviderTransformerGraph(t, mod) { - transform := TransformProviders([]string{"aws", "foo", "bar"}, concrete, mod) + transform := transformProviders(concrete, mod) if err := transform.Transform(g); err != nil { t.Fatalf("err: %s", err) } @@ -252,7 +252,7 @@ func TestPruneProviderTransformer(t *testing.T) { g := testProviderTransformerGraph(t, mod) { - transform := &MissingProviderTransformer{Providers: []string{"foo"}} + transform := &MissingProviderTransformer{} if err := transform.Transform(g); err != nil { t.Fatalf("err: %s", err) } @@ -293,7 +293,7 @@ func TestProviderConfigTransformer_parentProviders(t *testing.T) { g := testProviderTransformerGraph(t, mod) { - tf := TransformProviders([]string{"aws"}, concrete, mod) + tf := transformProviders(concrete, mod) if err := tf.Transform(g); err != nil { t.Fatalf("err: %s", err) } @@ -313,7 +313,7 @@ func TestProviderConfigTransformer_grandparentProviders(t *testing.T) { g := testProviderTransformerGraph(t, mod) { - tf := TransformProviders([]string{"aws"}, concrete, mod) + tf := transformProviders(concrete, mod) if err := tf.Transform(g); err != nil { t.Fatalf("err: %s", err) } @@ -347,7 +347,7 @@ resource "test_object" "a" { g := testProviderTransformerGraph(t, mod) { - tf := TransformProviders([]string{"registry.terraform.io/hashicorp/test"}, concrete, mod) + tf := transformProviders(concrete, mod) if err := tf.Transform(g); err != nil { t.Fatalf("err: %s", err) } @@ -425,7 +425,7 @@ resource "test_object" "a" { g := testProviderTransformerGraph(t, mod) { - tf := TransformProviders([]string{"registry.terraform.io/hashicorp/test"}, concrete, mod) + tf := transformProviders(concrete, mod) if err := tf.Transform(g); err != nil { t.Fatalf("err: %s", err) } diff --git a/internal/terraform/transform_root_test.go b/internal/terraform/transform_root_test.go index 04b8659de..4a426b5e7 100644 --- a/internal/terraform/transform_root_test.go +++ b/internal/terraform/transform_root_test.go @@ -19,9 +19,7 @@ func TestRootTransformer(t *testing.T) { } { - transform := &MissingProviderTransformer{ - Providers: []string{"aws", "do"}, - } + transform := &MissingProviderTransformer{} if err := transform.Transform(&g); err != nil { t.Fatalf("err: %s", err) }