Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Continue pushing the Path calls out of the Resource and Provider types #24346

Merged
merged 4 commits into from
Mar 11, 2020
Merged

Continue pushing the Path calls out of the Resource and Provider types #24346

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
fae5f99
remove GraphNodeModuleInstance from Resource types
jbardin Mar 9, 2020
8497adc
AbsProviderConfig to use addrs.Module
jbardin Mar 9, 2020
98cfb51
convert /terraform to use new provider config
jbardin Mar 11, 2020
e13eecb
finish provider ModuleInstance replacement
jbardin Mar 11, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 44 additions & 18 deletions addrs/provider_config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package addrs

import (
"fmt"
"strings"

"github.com/hashicorp/terraform/tfdiags"
"github.com/zclconf/go-cty/cty"
Expand Down Expand Up @@ -86,7 +87,7 @@ func (pc LocalProviderConfig) StringCompact() string {
// AbsProviderConfig is the absolute address of a provider configuration
// within a particular module instance.
type AbsProviderConfig struct {
Module ModuleInstance
Module Module
Provider Provider
Alias string
}
Expand All @@ -101,17 +102,30 @@ var _ ProviderConfig = AbsProviderConfig{}
// provider["registry.terraform.io/hashicorp/aws"].foo
// module.bar.provider["registry.terraform.io/hashicorp/aws"]
// module.bar.module.baz.provider["registry.terraform.io/hashicorp/aws"].foo
// module.foo[1].provider["registry.terraform.io/hashicorp/aws"].foo
//
// This type of address is used, for example, to record the relationships
// between resources and provider configurations in the state structure.
// This type of address is not generally used in the UI, except in error
// messages that refer to provider configurations.
func ParseAbsProviderConfig(traversal hcl.Traversal) (AbsProviderConfig, tfdiags.Diagnostics) {
modInst, remain, diags := parseModuleInstancePrefix(traversal)
ret := AbsProviderConfig{
Module: modInst,
var ret AbsProviderConfig

// Providers cannot resolve within module instances, so verify that there
// are no instance keys in the module path before converting to a Module.
for _, step := range modInst {
if step.InstanceKey != NoKey {
diags = diags.Append(&hcl.Diagnostic{
Severity: hcl.DiagError,
Summary: "Invalid provider configuration address",
Detail: "Provider address cannot contain module indexes",
Subject: remain.SourceRange().Ptr(),
})
return ret, diags
}
}
ret.Module = modInst.Module()

if len(remain) < 2 || remain.RootName() != "provider" {
diags = diags.Append(&hcl.Diagnostic{
Severity: hcl.DiagError,
Expand Down Expand Up @@ -223,16 +237,28 @@ func ParseLegacyAbsProviderConfigStr(str string) (AbsProviderConfig, tfdiags.Dia
// provider.aws.foo
// module.bar.provider.aws
// module.bar.module.baz.provider.aws.foo
// module.foo[1].provider.aws.foo
//
// This type of address is used in legacy state and may appear in state v4 if
// the provider config addresses have not been normalized to include provider
// FQN.
func ParseLegacyAbsProviderConfig(traversal hcl.Traversal) (AbsProviderConfig, tfdiags.Diagnostics) {
modInst, remain, diags := parseModuleInstancePrefix(traversal)
ret := AbsProviderConfig{
Module: modInst,
var ret AbsProviderConfig

// Providers cannot resolve within module instances, so verify that there
// are no instance keys in the module path before converting to a Module.
for _, step := range modInst {
if step.InstanceKey != NoKey {
diags = diags.Append(&hcl.Diagnostic{
Severity: hcl.DiagError,
Summary: "Invalid provider configuration address",
Detail: "Provider address cannot contain module indexes",
Subject: remain.SourceRange().Ptr(),
})
return ret, diags
}
}
ret.Module = modInst.Module()

if len(remain) < 2 || remain.RootName() != "provider" {
diags = diags.Append(&hcl.Diagnostic{
Expand Down Expand Up @@ -287,7 +313,7 @@ func ParseLegacyAbsProviderConfig(traversal hcl.Traversal) (AbsProviderConfig, t
// the given type inside the recieving module instance.
func (m ModuleInstance) ProviderConfigDefault(provider Provider) AbsProviderConfig {
return AbsProviderConfig{
Module: m,
Module: m.Module(),
Provider: provider,
}
}
Expand All @@ -296,7 +322,7 @@ func (m ModuleInstance) ProviderConfigDefault(provider Provider) AbsProviderConf
// the given type and alias inside the recieving module instance.
func (m ModuleInstance) ProviderConfigAliased(provider Provider, alias string) AbsProviderConfig {
return AbsProviderConfig{
Module: m,
Module: m.Module(),
Provider: provider,
Alias: alias,
}
Expand Down Expand Up @@ -359,16 +385,16 @@ func (pc AbsProviderConfig) LegacyString() string {
// module.module-name.provider["example.com/namespace/name"]
// module.module-name.provider["example.com/namespace/name"].alias
func (pc AbsProviderConfig) String() string {
if pc.Alias != "" {
if len(pc.Module) == 0 {
return fmt.Sprintf("%s[%q].%s", "provider", pc.Provider.String(), pc.Alias)
} else {
return fmt.Sprintf("%s.%s[%q].%s", pc.Module.String(), "provider", pc.Provider.String(), pc.Alias)
}
var parts []string
if len(pc.Module) > 0 {
parts = append(parts, pc.Module.String())
}
if len(pc.Module) == 0 {
return fmt.Sprintf("%s[%q]", "provider", pc.Provider.String())

parts = append(parts, fmt.Sprintf("provider[%q]", pc.Provider))

if pc.Alias != "" {
parts = append(parts, pc.Alias)
}

return fmt.Sprintf("%s.%s[%q]", pc.Module.String(), "provider", pc.Provider.String())
return strings.Join(parts, ".")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what a lovely refactor, thank you! That's MUCH nicer to read ❤️

}
83 changes: 18 additions & 65 deletions addrs/provider_config_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ func TestParseAbsProviderConfig(t *testing.T) {
{
`provider["registry.terraform.io/hashicorp/aws"]`,
AbsProviderConfig{
Module: RootModuleInstance,
Module: RootModule,
Provider: Provider{
Type: "aws",
Namespace: "hashicorp",
Expand All @@ -30,7 +30,7 @@ func TestParseAbsProviderConfig(t *testing.T) {
{
`provider["registry.terraform.io/hashicorp/aws"].foo`,
AbsProviderConfig{
Module: RootModuleInstance,
Module: RootModule,
Provider: Provider{
Type: "aws",
Namespace: "hashicorp",
Expand All @@ -43,11 +43,7 @@ func TestParseAbsProviderConfig(t *testing.T) {
{
`module.baz.provider["registry.terraform.io/hashicorp/aws"]`,
AbsProviderConfig{
Module: ModuleInstance{
{
Name: "baz",
},
},
Module: Module{"baz"},
Provider: Provider{
Type: "aws",
Namespace: "hashicorp",
Expand All @@ -59,11 +55,7 @@ func TestParseAbsProviderConfig(t *testing.T) {
{
`module.baz.provider["registry.terraform.io/hashicorp/aws"].foo`,
AbsProviderConfig{
Module: ModuleInstance{
{
Name: "baz",
},
},
Module: Module{"baz"},
Provider: Provider{
Type: "aws",
Namespace: "hashicorp",
Expand All @@ -75,57 +67,18 @@ func TestParseAbsProviderConfig(t *testing.T) {
},
{
`module.baz["foo"].provider["registry.terraform.io/hashicorp/aws"]`,
AbsProviderConfig{
Module: ModuleInstance{
{
Name: "baz",
InstanceKey: StringKey("foo"),
},
},
Provider: Provider{
Type: "aws",
Namespace: "hashicorp",
Hostname: "registry.terraform.io",
},
},
``,
AbsProviderConfig{},
`Provider address cannot contain module indexes`,
},
{
`module.baz[1].provider["registry.terraform.io/hashicorp/aws"]`,
AbsProviderConfig{
Module: ModuleInstance{
{
Name: "baz",
InstanceKey: IntKey(1),
},
},
Provider: Provider{
Type: "aws",
Namespace: "hashicorp",
Hostname: "registry.terraform.io",
},
},
``,
AbsProviderConfig{},
`Provider address cannot contain module indexes`,
},
{
`module.baz[1].module.bar.provider["registry.terraform.io/hashicorp/aws"]`,
AbsProviderConfig{
Module: ModuleInstance{
{
Name: "baz",
InstanceKey: IntKey(1),
},
{
Name: "bar",
},
},
Provider: Provider{
Type: "aws",
Namespace: "hashicorp",
Hostname: "registry.terraform.io",
},
},
``,
AbsProviderConfig{},
`Provider address cannot contain module indexes`,
},
{
`aws`,
Expand Down Expand Up @@ -206,29 +159,29 @@ func TestAbsProviderConfigString(t *testing.T) {
}{
{
AbsProviderConfig{
Module: RootModuleInstance,
Module: RootModule,
Provider: NewLegacyProvider("foo"),
},
`provider["registry.terraform.io/-/foo"]`,
},
{
AbsProviderConfig{
Module: RootModuleInstance.Child("child_module", NoKey),
Module: RootModule.Child("child_module"),
Provider: NewLegacyProvider("foo"),
},
`module.child_module.provider["registry.terraform.io/-/foo"]`,
},
{
AbsProviderConfig{
Module: RootModuleInstance,
Module: RootModule,
Alias: "bar",
Provider: NewLegacyProvider("foo"),
},
`provider["registry.terraform.io/-/foo"].bar`,
},
{
AbsProviderConfig{
Module: RootModuleInstance.Child("child_module", NoKey),
Module: RootModule.Child("child_module"),
Alias: "bar",
Provider: NewLegacyProvider("foo"),
},
Expand All @@ -251,29 +204,29 @@ func TestAbsProviderConfigLegacyString(t *testing.T) {
}{
{
AbsProviderConfig{
Module: RootModuleInstance,
Module: RootModule,
Provider: NewLegacyProvider("foo"),
},
`provider.foo`,
},
{
AbsProviderConfig{
Module: RootModuleInstance.Child("child_module", NoKey),
Module: RootModule.Child("child_module"),
Provider: NewLegacyProvider("foo"),
},
`module.child_module.provider.foo`,
},
{
AbsProviderConfig{
Module: RootModuleInstance,
Module: RootModule,
Alias: "bar",
Provider: NewLegacyProvider("foo"),
},
`provider.foo.bar`,
},
{
AbsProviderConfig{
Module: RootModuleInstance.Child("child_module", NoKey),
Module: RootModule.Child("child_module"),
Alias: "bar",
Provider: NewLegacyProvider("foo"),
},
Expand Down
10 changes: 5 additions & 5 deletions backend/local/backend_plan_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ func TestLocal_planDeposedOnly(t *testing.T) {
},
addrs.AbsProviderConfig{
Provider: addrs.NewLegacyProvider("test"),
Module: addrs.RootModuleInstance,
Module: addrs.RootModule,
},
)
}))
Expand Down Expand Up @@ -661,7 +661,7 @@ func testPlanState() *states.State {
},
addrs.AbsProviderConfig{
Provider: addrs.NewLegacyProvider("test"),
Module: addrs.RootModuleInstance,
Module: addrs.RootModule,
},
)
return state
Expand All @@ -688,7 +688,7 @@ func testPlanState_withDataSource() *states.State {
},
addrs.AbsProviderConfig{
Provider: addrs.NewLegacyProvider("test"),
Module: addrs.RootModuleInstance,
Module: addrs.RootModule,
},
)
rootModule.SetResourceInstanceCurrent(
Expand All @@ -705,7 +705,7 @@ func testPlanState_withDataSource() *states.State {
},
addrs.AbsProviderConfig{
Provider: addrs.NewLegacyProvider("test"),
Module: addrs.RootModuleInstance,
Module: addrs.RootModule,
},
)
return state
Expand All @@ -732,7 +732,7 @@ func testPlanState_tainted() *states.State {
},
addrs.AbsProviderConfig{
Provider: addrs.NewLegacyProvider("test"),
Module: addrs.RootModuleInstance,
Module: addrs.RootModule,
},
)
return state
Expand Down
2 changes: 1 addition & 1 deletion backend/testing.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ func TestBackendStates(t *testing.T, b Backend) {
},
addrs.AbsProviderConfig{
Provider: addrs.NewLegacyProvider("test"),
Module: addrs.RootModuleInstance,
Module: addrs.RootModule,
},
)

Expand Down
8 changes: 4 additions & 4 deletions command/apply_destroy_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ func TestApply_destroy(t *testing.T) {
},
addrs.AbsProviderConfig{
Provider: addrs.NewLegacyProvider("test"),
Module: addrs.RootModuleInstance,
Module: addrs.RootModule,
},
)
})
Expand Down Expand Up @@ -127,7 +127,7 @@ func TestApply_destroyLockedState(t *testing.T) {
},
addrs.AbsProviderConfig{
Provider: addrs.NewLegacyProvider("test"),
Module: addrs.RootModuleInstance,
Module: addrs.RootModule,
},
)
})
Expand Down Expand Up @@ -202,7 +202,7 @@ func TestApply_destroyTargeted(t *testing.T) {
},
addrs.AbsProviderConfig{
Provider: addrs.NewLegacyProvider("test"),
Module: addrs.RootModuleInstance,
Module: addrs.RootModule,
},
)
s.SetResourceInstanceCurrent(
Expand All @@ -218,7 +218,7 @@ func TestApply_destroyTargeted(t *testing.T) {
},
addrs.AbsProviderConfig{
Provider: addrs.NewLegacyProvider("test"),
Module: addrs.RootModuleInstance,
Module: addrs.RootModule,
},
)
})
Expand Down
Loading