From 8e18922170343ecd397ced79e38f3de03bffa077 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Mon, 21 Nov 2022 10:38:25 -0500 Subject: [PATCH] ProvidedBy should return the resolved provider Once the ProviderTransformer has resolved and set the exact provider, the ProvidedBy method should return that exact provider again. We can hoist the stored provider addr into the AbstractInstance and avoid the method duplication and slight differences between the implementations. --- internal/terraform/node_resource_abstract.go | 20 ++++++ .../node_resource_abstract_instance.go | 41 ------------ .../node_resource_abstract_instance_test.go | 5 +- .../terraform/node_resource_abstract_test.go | 67 +++++++++++++++++++ 4 files changed, 90 insertions(+), 43 deletions(-) diff --git a/internal/terraform/node_resource_abstract.go b/internal/terraform/node_resource_abstract.go index af9583a28d..5ad7901d03 100644 --- a/internal/terraform/node_resource_abstract.go +++ b/internal/terraform/node_resource_abstract.go @@ -68,6 +68,10 @@ type NodeAbstractResource struct { // The address of the provider this resource will use ResolvedProvider addrs.AbsProviderConfig + // storedProviderConfig is the provider address retrieved from the + // state. This is defined here for access within the ProvidedBy method, but + // will be set from the embedding instance type when the state is attached. + storedProviderConfig addrs.AbsProviderConfig // This resource may expand into instances which need to be imported. importTargets []*ImportTarget @@ -231,6 +235,11 @@ func (n *NodeAbstractResource) SetProvider(p addrs.AbsProviderConfig) { // GraphNodeProviderConsumer func (n *NodeAbstractResource) ProvidedBy() (addrs.ProviderConfig, bool) { + // Once the provider is fully resolved, we can return the known value. + if n.ResolvedProvider.Provider.Type != "" { + return n.ResolvedProvider, true + } + // If we have a config we prefer that above all else if n.Config != nil { relAddr := n.Config.ProviderConfigAddr() @@ -240,6 +249,14 @@ func (n *NodeAbstractResource) ProvidedBy() (addrs.ProviderConfig, bool) { }, false } + // See if we have a valid provider config from the state. + if n.storedProviderConfig.Provider.Type != "" { + // An address from the state must match exactly, since we must ensure + // we refresh/destroy a resource with the same provider configuration + // that created it. + return n.storedProviderConfig, true + } + // No provider configuration found; return a default address return addrs.AbsProviderConfig{ Provider: n.Provider(), @@ -252,6 +269,9 @@ func (n *NodeAbstractResource) Provider() addrs.Provider { if n.Config != nil { return n.Config.Provider } + if n.storedProviderConfig.Provider.Type != "" { + return n.storedProviderConfig.Provider + } return addrs.ImpliedProviderForUnqualifiedType(n.Addr.Resource.ImpliedProvider()) } diff --git a/internal/terraform/node_resource_abstract_instance.go b/internal/terraform/node_resource_abstract_instance.go index 5d6dcc43f3..8adf0ae3d6 100644 --- a/internal/terraform/node_resource_abstract_instance.go +++ b/internal/terraform/node_resource_abstract_instance.go @@ -31,10 +31,6 @@ type NodeAbstractResourceInstance struct { // These are set via the AttachState method. instanceState *states.ResourceInstance - // storedProviderConfig is the provider address retrieved from the - // state, but since it is only stored in the whole Resource rather than the - // ResourceInstance, we extract it out here. - storedProviderConfig addrs.AbsProviderConfig Dependencies []addrs.ConfigResource @@ -109,43 +105,6 @@ func (n *NodeAbstractResourceInstance) StateDependencies() []addrs.ConfigResourc return nil } -// GraphNodeProviderConsumer -func (n *NodeAbstractResourceInstance) ProvidedBy() (addrs.ProviderConfig, bool) { - // If we have a config we prefer that above all else - if n.Config != nil { - relAddr := n.Config.ProviderConfigAddr() - return addrs.LocalProviderConfig{ - LocalName: relAddr.LocalName, - Alias: relAddr.Alias, - }, false - } - - // See if we have a valid provider config from the state. - if n.storedProviderConfig.Provider.Type != "" { - // An address from the state must match exactly, since we must ensure - // we refresh/destroy a resource with the same provider configuration - // that created it. - return n.storedProviderConfig, true - } - - // No provider configuration found; return a default address - return addrs.AbsProviderConfig{ - Provider: n.Provider(), - Module: n.ModulePath(), - }, false -} - -// GraphNodeProviderConsumer -func (n *NodeAbstractResourceInstance) Provider() addrs.Provider { - if n.Config != nil { - return n.Config.Provider - } - if n.storedProviderConfig.Provider.Type != "" { - return n.storedProviderConfig.Provider - } - return addrs.ImpliedProviderForUnqualifiedType(n.Addr.Resource.ContainingResource().ImpliedProvider()) -} - // GraphNodeResourceInstance func (n *NodeAbstractResourceInstance) ResourceInstanceAddr() addrs.AbsResourceInstance { return n.Addr diff --git a/internal/terraform/node_resource_abstract_instance_test.go b/internal/terraform/node_resource_abstract_instance_test.go index dca1cb52f0..22a5b0b5f7 100644 --- a/internal/terraform/node_resource_abstract_instance_test.go +++ b/internal/terraform/node_resource_abstract_instance_test.go @@ -125,9 +125,10 @@ func TestNodeAbstractResourceInstanceProvider(t *testing.T) { // function. (This would not be valid for some other functions.) Addr: test.Addr, NodeAbstractResource: NodeAbstractResource{ - Config: test.Config, + Addr: test.Addr.ConfigResource(), + Config: test.Config, + storedProviderConfig: test.StoredProviderConfig, }, - storedProviderConfig: test.StoredProviderConfig, } got := node.Provider() if got != test.Want { diff --git a/internal/terraform/node_resource_abstract_test.go b/internal/terraform/node_resource_abstract_test.go index 352cc1bb6a..51ad8d9698 100644 --- a/internal/terraform/node_resource_abstract_test.go +++ b/internal/terraform/node_resource_abstract_test.go @@ -112,6 +112,73 @@ func TestNodeAbstractResourceProvider(t *testing.T) { } } +// Make sure ProvideBy returns the final resolved provider +func TestNodeAbstractResourceSetProvider(t *testing.T) { + node := &NodeAbstractResource{ + + // Just enough NodeAbstractResource for the Provider function. + // (This would not be valid for some other functions.) + Addr: addrs.Resource{ + Mode: addrs.DataResourceMode, + Type: "terraform_remote_state", + Name: "baz", + }.InModule(addrs.RootModule), + Config: &configs.Resource{ + Mode: addrs.ManagedResourceMode, + Type: "terraform_remote_state", + Name: "baz", + // Just enough configs.Resource for the Provider method. Not + // actually valid for general use. + Provider: addrs.Provider{ + Hostname: addrs.DefaultProviderRegistryHost, + Namespace: "awesomecorp", + Type: "happycloud", + }, + }, + } + + p, exact := node.ProvidedBy() + if exact { + t.Fatalf("no exact provider should be found from this confniguration, got %q\n", p) + } + + // the implied non-exact provider should be "terraform" + lpc, ok := p.(addrs.LocalProviderConfig) + if !ok { + t.Fatalf("expected LocalProviderConfig, got %#v\n", p) + } + + if lpc.LocalName != "terraform" { + t.Fatalf("expected non-exact provider of 'terraform', got %q", lpc.LocalName) + } + + // now set a resolved provider for the resource + resolved := addrs.AbsProviderConfig{ + Provider: addrs.Provider{ + Hostname: addrs.DefaultProviderRegistryHost, + Namespace: "awesomecorp", + Type: "happycloud", + }, + Module: addrs.RootModule, + Alias: "test", + } + + node.SetProvider(resolved) + p, exact = node.ProvidedBy() + if !exact { + t.Fatalf("exact provider should be found, got %q\n", p) + } + + apc, ok := p.(addrs.AbsProviderConfig) + if !ok { + t.Fatalf("expected AbsProviderConfig, got %#v\n", p) + } + + if apc.String() != resolved.String() { + t.Fatalf("incorrect resolved config: got %#v, wanted %#v\n", apc, resolved) + } +} + func TestNodeAbstractResource_ReadResourceInstanceState(t *testing.T) { mockProvider := mockProviderWithResourceTypeSchema("aws_instance", &configschema.Block{ Attributes: map[string]*configschema.Attribute{