From 67fc4dd5a1f205148db9fbdf76d450ea2197f6a7 Mon Sep 17 00:00:00 2001 From: Kristin Laemmert Date: Fri, 6 Dec 2019 12:14:54 -0500 Subject: [PATCH] configs: move ProviderConfigCompact[Str] from addrs to configs The configs package is aware of provider name and type (which are the same thing today, but expected to be two different things in a future release), and should be the source of truth for a provider config address. --- addrs/module_instance.go | 2 +- addrs/provider_config.go | 78 ----------------------- addrs/provider_config_test.go | 62 ------------------- command/import.go | 2 +- configs/provider.go | 93 ++++++++++++++++++++++------ configs/provider_requirements.go | 32 ++++++++++ configs/provider_test.go | 68 ++++++++++++++++++++ states/statefile/version3_upgrade.go | 3 +- 8 files changed, 178 insertions(+), 162 deletions(-) create mode 100644 configs/provider_requirements.go diff --git a/addrs/module_instance.go b/addrs/module_instance.go index c81784e7a..d26aee277 100644 --- a/addrs/module_instance.go +++ b/addrs/module_instance.go @@ -57,7 +57,7 @@ func ParseModuleInstance(traversal hcl.Traversal) (ModuleInstance, tfdiags.Diagn // If a reference string is coming from a source that should be identified in // error messages then the caller should instead parse it directly using a // suitable function from the HCL API and pass the traversal itself to -// ParseProviderConfigCompact. +// ParseModuleInstance. // // Error diagnostics are returned if either the parsing fails or the analysis // of the traversal fails. There is no way for the caller to distinguish the diff --git a/addrs/provider_config.go b/addrs/provider_config.go index 6413ed9cf..965cd7d11 100644 --- a/addrs/provider_config.go +++ b/addrs/provider_config.go @@ -26,84 +26,6 @@ func NewDefaultProviderConfig(typeName string) ProviderConfig { } } -// ParseProviderConfigCompact parses the given absolute traversal as a relative -// provider address in compact form. The following are examples of traversals -// that can be successfully parsed as compact relative provider configuration -// addresses: -// -// aws -// aws.foo -// -// This function will panic if given a relative traversal. -// -// If the returned diagnostics contains errors then the result value is invalid -// and must not be used. -func ParseProviderConfigCompact(traversal hcl.Traversal) (ProviderConfig, tfdiags.Diagnostics) { - var diags tfdiags.Diagnostics - ret := ProviderConfig{ - Type: NewLegacyProvider(traversal.RootName()), - } - - if len(traversal) < 2 { - // Just a type name, then. - return ret, diags - } - - aliasStep := traversal[1] - switch ts := aliasStep.(type) { - case hcl.TraverseAttr: - ret.Alias = ts.Name - return ret, diags - default: - diags = diags.Append(&hcl.Diagnostic{ - Severity: hcl.DiagError, - Summary: "Invalid provider configuration address", - Detail: "The provider type name must either stand alone or be followed by an alias name separated with a dot.", - Subject: aliasStep.SourceRange().Ptr(), - }) - } - - if len(traversal) > 2 { - diags = diags.Append(&hcl.Diagnostic{ - Severity: hcl.DiagError, - Summary: "Invalid provider configuration address", - Detail: "Extraneous extra operators after provider configuration address.", - Subject: traversal[2:].SourceRange().Ptr(), - }) - } - - return ret, diags -} - -// ParseProviderConfigCompactStr is a helper wrapper around ParseProviderConfigCompact -// that takes a string and parses it with the HCL native syntax traversal parser -// before interpreting it. -// -// This should be used only in specialized situations since it will cause the -// created references to not have any meaningful source location information. -// If a reference string is coming from a source that should be identified in -// error messages then the caller should instead parse it directly using a -// suitable function from the HCL API and pass the traversal itself to -// ParseProviderConfigCompact. -// -// Error diagnostics are returned if either the parsing fails or the analysis -// of the traversal fails. There is no way for the caller to distinguish the -// two kinds of diagnostics programmatically. If error diagnostics are returned -// then the returned address is invalid. -func ParseProviderConfigCompactStr(str string) (ProviderConfig, tfdiags.Diagnostics) { - var diags tfdiags.Diagnostics - - traversal, parseDiags := hclsyntax.ParseTraversalAbs([]byte(str), "", hcl.Pos{Line: 1, Column: 1}) - diags = diags.Append(parseDiags) - if parseDiags.HasErrors() { - return ProviderConfig{}, diags - } - - addr, addrDiags := ParseProviderConfigCompact(traversal) - diags = diags.Append(addrDiags) - return addr, diags -} - // Absolute returns an AbsProviderConfig from the receiver and the given module // instance address. func (pc ProviderConfig) Absolute(module ModuleInstance) AbsProviderConfig { diff --git a/addrs/provider_config_test.go b/addrs/provider_config_test.go index 756fa0bb1..9b0eb90a4 100644 --- a/addrs/provider_config_test.go +++ b/addrs/provider_config_test.go @@ -9,68 +9,6 @@ import ( "github.com/hashicorp/hcl/v2/hclsyntax" ) -func TestParseProviderConfigCompact(t *testing.T) { - tests := []struct { - Input string - Want ProviderConfig - WantDiag string - }{ - { - `aws`, - ProviderConfig{ - Type: NewLegacyProvider("aws"), - }, - ``, - }, - { - `aws.foo`, - ProviderConfig{ - Type: NewLegacyProvider("aws"), - Alias: "foo", - }, - ``, - }, - { - `aws["foo"]`, - ProviderConfig{}, - `The provider type name must either stand alone or be followed by an alias name separated with a dot.`, - }, - } - - for _, test := range tests { - t.Run(test.Input, func(t *testing.T) { - traversal, parseDiags := hclsyntax.ParseTraversalAbs([]byte(test.Input), "", hcl.Pos{}) - if len(parseDiags) != 0 { - t.Errorf("unexpected diagnostics during parse") - for _, diag := range parseDiags { - t.Logf("- %s", diag) - } - return - } - - got, diags := ParseProviderConfigCompact(traversal) - - if test.WantDiag != "" { - if len(diags) != 1 { - t.Fatalf("got %d diagnostics; want 1", len(diags)) - } - gotDetail := diags[0].Description().Detail - if gotDetail != test.WantDiag { - t.Fatalf("wrong diagnostic detail\ngot: %s\nwant: %s", gotDetail, test.WantDiag) - } - return - } else { - if len(diags) != 0 { - t.Fatalf("got %d diagnostics; want 0", len(diags)) - } - } - - for _, problem := range deep.Equal(got, test.Want) { - t.Error(problem) - } - }) - } -} func TestParseAbsProviderConfig(t *testing.T) { tests := []struct { Input string diff --git a/command/import.go b/command/import.go index 5cdf446af..cc384405a 100644 --- a/command/import.go +++ b/command/import.go @@ -166,7 +166,7 @@ func (c *ImportCommand) Run(args []string) int { c.Ui.Info(importCommandInvalidAddressReference) return 1 } - relAddr, addrDiags := addrs.ParseProviderConfigCompact(traversal) + relAddr, addrDiags := configs.ParseProviderConfigCompact(traversal) diags = diags.Append(addrDiags) if addrDiags.HasErrors() { c.showDiagnostics(diags) diff --git a/configs/provider.go b/configs/provider.go index 169f897bf..6b57fc605 100644 --- a/configs/provider.go +++ b/configs/provider.go @@ -8,6 +8,7 @@ import ( "github.com/hashicorp/hcl/v2/hclsyntax" "github.com/hashicorp/terraform/addrs" + "github.com/hashicorp/terraform/tfdiags" ) // Provider represents a "provider" block in a module or file. A provider @@ -107,28 +108,82 @@ func (p *Provider) moduleUniqueKey() string { return p.Name } -// ProviderRequirement represents a declaration of a dependency on a particular -// provider version without actually configuring that provider. This is used in -// child modules that expect a provider to be passed in from their parent. -type ProviderRequirement struct { - Name string - Requirement VersionConstraint +// ParseProviderConfigCompact parses the given absolute traversal as a relative +// provider address in compact form. The following are examples of traversals +// that can be successfully parsed as compact relative provider configuration +// addresses: +// +// aws +// aws.foo +// +// This function will panic if given a relative traversal. +// +// If the returned diagnostics contains errors then the result value is invalid +// and must not be used. +func ParseProviderConfigCompact(traversal hcl.Traversal) (addrs.ProviderConfig, tfdiags.Diagnostics) { + var diags tfdiags.Diagnostics + ret := addrs.ProviderConfig{ + Type: addrs.NewLegacyProvider(traversal.RootName()), + } + + if len(traversal) < 2 { + // Just a type name, then. + return ret, diags + } + + aliasStep := traversal[1] + switch ts := aliasStep.(type) { + case hcl.TraverseAttr: + ret.Alias = ts.Name + return ret, diags + default: + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Invalid provider configuration address", + Detail: "The provider type name must either stand alone or be followed by an alias name separated with a dot.", + Subject: aliasStep.SourceRange().Ptr(), + }) + } + + if len(traversal) > 2 { + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Invalid provider configuration address", + Detail: "Extraneous extra operators after provider configuration address.", + Subject: traversal[2:].SourceRange().Ptr(), + }) + } + + return ret, diags } -func decodeRequiredProvidersBlock(block *hcl.Block) ([]*ProviderRequirement, hcl.Diagnostics) { - attrs, diags := block.Body.JustAttributes() - var reqs []*ProviderRequirement - for name, attr := range attrs { - req, reqDiags := decodeVersionConstraint(attr) - diags = append(diags, reqDiags...) - if !diags.HasErrors() { - reqs = append(reqs, &ProviderRequirement{ - Name: name, - Requirement: req, - }) - } +// ParseProviderConfigCompactStr is a helper wrapper around ParseProviderConfigCompact +// that takes a string and parses it with the HCL native syntax traversal parser +// before interpreting it. +// +// This should be used only in specialized situations since it will cause the +// created references to not have any meaningful source location information. +// If a reference string is coming from a source that should be identified in +// error messages then the caller should instead parse it directly using a +// suitable function from the HCL API and pass the traversal itself to +// ParseProviderConfigCompact. +// +// Error diagnostics are returned if either the parsing fails or the analysis +// of the traversal fails. There is no way for the caller to distinguish the +// two kinds of diagnostics programmatically. If error diagnostics are returned +// then the returned address is invalid. +func ParseProviderConfigCompactStr(str string) (addrs.ProviderConfig, tfdiags.Diagnostics) { + var diags tfdiags.Diagnostics + + traversal, parseDiags := hclsyntax.ParseTraversalAbs([]byte(str), "", hcl.Pos{Line: 1, Column: 1}) + diags = diags.Append(parseDiags) + if parseDiags.HasErrors() { + return addrs.ProviderConfig{}, diags } - return reqs, diags + + addr, addrDiags := ParseProviderConfigCompact(traversal) + diags = diags.Append(addrDiags) + return addr, diags } var providerBlockSchema = &hcl.BodySchema{ diff --git a/configs/provider_requirements.go b/configs/provider_requirements.go new file mode 100644 index 000000000..3890bbbcd --- /dev/null +++ b/configs/provider_requirements.go @@ -0,0 +1,32 @@ +package configs + +import ( + "github.com/hashicorp/hcl/v2" +) + +// ProviderRequirement represents a declaration of a dependency on a particular +// provider version without actually configuring that provider. This is used in +// child modules that expect a provider to be passed in from their parent. +// +// TODO: "Source" is a placeholder for an attribute that is not yet supported. +type ProviderRequirement struct { + Name string + Source string // TODO + Requirement VersionConstraint +} + +func decodeRequiredProvidersBlock(block *hcl.Block) ([]*ProviderRequirement, hcl.Diagnostics) { + attrs, diags := block.Body.JustAttributes() + var reqs []*ProviderRequirement + for name, attr := range attrs { + req, reqDiags := decodeVersionConstraint(attr) + diags = append(diags, reqDiags...) + if !diags.HasErrors() { + reqs = append(reqs, &ProviderRequirement{ + Name: name, + Requirement: req, + }) + } + } + return reqs, diags +} diff --git a/configs/provider_test.go b/configs/provider_test.go index 625108759..3458b9d2b 100644 --- a/configs/provider_test.go +++ b/configs/provider_test.go @@ -3,6 +3,11 @@ package configs import ( "io/ioutil" "testing" + + "github.com/go-test/deep" + "github.com/hashicorp/hcl/v2" + "github.com/hashicorp/hcl/v2/hclsyntax" + "github.com/hashicorp/terraform/addrs" ) func TestProviderReservedNames(t *testing.T) { @@ -24,3 +29,66 @@ func TestProviderReservedNames(t *testing.T) { `config.tf:13,3-9: Reserved argument name in provider block; The provider argument name "source" is reserved for use by Terraform in a future version.`, }) } + +func TestParseProviderConfigCompact(t *testing.T) { + tests := []struct { + Input string + Want addrs.ProviderConfig + WantDiag string + }{ + { + `aws`, + addrs.ProviderConfig{ + Type: addrs.NewLegacyProvider("aws"), + }, + ``, + }, + { + `aws.foo`, + addrs.ProviderConfig{ + Type: addrs.NewLegacyProvider("aws"), + Alias: "foo", + }, + ``, + }, + { + `aws["foo"]`, + addrs.ProviderConfig{}, + `The provider type name must either stand alone or be followed by an alias name separated with a dot.`, + }, + } + + for _, test := range tests { + t.Run(test.Input, func(t *testing.T) { + traversal, parseDiags := hclsyntax.ParseTraversalAbs([]byte(test.Input), "", hcl.Pos{}) + if len(parseDiags) != 0 { + t.Errorf("unexpected diagnostics during parse") + for _, diag := range parseDiags { + t.Logf("- %s", diag) + } + return + } + + got, diags := ParseProviderConfigCompact(traversal) + + if test.WantDiag != "" { + if len(diags) != 1 { + t.Fatalf("got %d diagnostics; want 1", len(diags)) + } + gotDetail := diags[0].Description().Detail + if gotDetail != test.WantDiag { + t.Fatalf("wrong diagnostic detail\ngot: %s\nwant: %s", gotDetail, test.WantDiag) + } + return + } else { + if len(diags) != 0 { + t.Fatalf("got %d diagnostics; want 0", len(diags)) + } + } + + for _, problem := range deep.Equal(got, test.Want) { + t.Error(problem) + } + }) + } +} diff --git a/states/statefile/version3_upgrade.go b/states/statefile/version3_upgrade.go index 753298ff0..4e38aca34 100644 --- a/states/statefile/version3_upgrade.go +++ b/states/statefile/version3_upgrade.go @@ -12,6 +12,7 @@ import ( ctyjson "github.com/zclconf/go-cty/cty/json" "github.com/hashicorp/terraform/addrs" + "github.com/hashicorp/terraform/configs" "github.com/hashicorp/terraform/states" "github.com/hashicorp/terraform/tfdiags" ) @@ -123,7 +124,7 @@ func upgradeStateV3ToV4(old *stateV3) (*stateV4, error) { // incorrect but it'll get fixed up next time any updates // are made to an instance. if oldProviderAddr != "" { - localAddr, diags := addrs.ParseProviderConfigCompactStr(oldProviderAddr) + localAddr, diags := configs.ParseProviderConfigCompactStr(oldProviderAddr) if diags.HasErrors() { if strings.Contains(oldProviderAddr, "${") { // There seems to be a common misconception that