From f5012c12dabf9f9b08d3ea75d3a349674d3d7c7f Mon Sep 17 00:00:00 2001 From: Martin Atkins Date: Wed, 22 Apr 2020 16:28:06 -0700 Subject: [PATCH] command/cliconfig: Installation methods, not installation sources Unfortunately in the user model the noun "source" is already used for the argument in the required_providers block to specify which provider to use, so it's confusing to use the same noun to also refer to the method used to obtain that provider. In the hope of mitigating that confusion, here we use the noun "method", as in "installation method", to talk about the decision between getting a provider directly from its origin registry or getting it from some mirror. This is distinct from the provider's "source", which is the location where a provider _originates_ (prior to mirroring). This noun is also not super awesome, but better than overloading an existing term in the same feature. --- command/cliconfig/cliconfig_test.go | 12 ++-- command/cliconfig/provider_installation.go | 66 +++++++++---------- .../cliconfig/provider_installation_test.go | 12 ++-- provider_source.go | 10 +-- 4 files changed, 50 insertions(+), 50 deletions(-) diff --git a/command/cliconfig/cliconfig_test.go b/command/cliconfig/cliconfig_test.go index 8ae79eb23..a84e825b3 100644 --- a/command/cliconfig/cliconfig_test.go +++ b/command/cliconfig/cliconfig_test.go @@ -235,13 +235,13 @@ func TestConfig_Merge(t *testing.T) { }, ProviderInstallation: []*ProviderInstallation{ { - Sources: []*ProviderInstallationSource{ + Methods: []*ProviderInstallationMethod{ {Location: ProviderInstallationFilesystemMirror("a")}, {Location: ProviderInstallationFilesystemMirror("b")}, }, }, { - Sources: []*ProviderInstallationSource{ + Methods: []*ProviderInstallationMethod{ {Location: ProviderInstallationFilesystemMirror("c")}, }, }, @@ -273,7 +273,7 @@ func TestConfig_Merge(t *testing.T) { }, ProviderInstallation: []*ProviderInstallation{ { - Sources: []*ProviderInstallationSource{ + Methods: []*ProviderInstallationMethod{ {Location: ProviderInstallationFilesystemMirror("d")}, }, }, @@ -316,18 +316,18 @@ func TestConfig_Merge(t *testing.T) { }, ProviderInstallation: []*ProviderInstallation{ { - Sources: []*ProviderInstallationSource{ + Methods: []*ProviderInstallationMethod{ {Location: ProviderInstallationFilesystemMirror("a")}, {Location: ProviderInstallationFilesystemMirror("b")}, }, }, { - Sources: []*ProviderInstallationSource{ + Methods: []*ProviderInstallationMethod{ {Location: ProviderInstallationFilesystemMirror("c")}, }, }, { - Sources: []*ProviderInstallationSource{ + Methods: []*ProviderInstallationMethod{ {Location: ProviderInstallationFilesystemMirror("d")}, }, }, diff --git a/command/cliconfig/provider_installation.go b/command/cliconfig/provider_installation.go index cca208222..28ca74b2c 100644 --- a/command/cliconfig/provider_installation.go +++ b/command/cliconfig/provider_installation.go @@ -11,7 +11,7 @@ import ( // ProviderInstallation is the structure of the "provider_installation" // nested block within the CLI configuration. type ProviderInstallation struct { - Sources []*ProviderInstallationSource + Methods []*ProviderInstallationMethod } // decodeProviderInstallationFromConfig uses the HCL AST API directly to @@ -66,42 +66,42 @@ func decodeProviderInstallationFromConfig(hclFile *hclast.File) ([]*ProviderInst // it will always be an hclast.ObjectType. body := block.Val.(*hclast.ObjectType) - for _, sourceBlock := range body.List.Items { - if sourceBlock.Assign.Line != 0 { + for _, methodBlock := range body.List.Items { + if methodBlock.Assign.Line != 0 { // Seems to be an attribute rather than a block diags = diags.Append(tfdiags.Sourceless( tfdiags.Error, - "Invalid provider_installation source block", + "Invalid provider_installation method block", fmt.Sprintf("The items inside the provider_installation block at %s must all be blocks.", block.Pos()), )) continue } - if len(sourceBlock.Keys) > 1 { + if len(methodBlock.Keys) > 1 { diags = diags.Append(tfdiags.Sourceless( tfdiags.Error, - "Invalid provider_installation source block", + "Invalid provider_installation method block", fmt.Sprintf("The blocks inside the provider_installation block at %s may not have any labels.", block.Pos()), )) } - sourceBody := sourceBlock.Val.(*hclast.ObjectType) + methodBody := methodBlock.Val.(*hclast.ObjectType) - sourceTypeStr := sourceBlock.Keys[0].Token.Value().(string) - var location ProviderInstallationSourceLocation + methodTypeStr := methodBlock.Keys[0].Token.Value().(string) + var location ProviderInstallationLocation var include, exclude []string - switch sourceTypeStr { + switch methodTypeStr { case "direct": type BodyContent struct { Include []string `hcl:"include"` Exclude []string `hcl:"exclude"` } var bodyContent BodyContent - err := hcl.DecodeObject(&bodyContent, sourceBody) + err := hcl.DecodeObject(&bodyContent, methodBody) if err != nil { diags = diags.Append(tfdiags.Sourceless( tfdiags.Error, - "Invalid provider_installation source block", - fmt.Sprintf("Invalid %s block at %s: %s.", sourceTypeStr, block.Pos(), err), + "Invalid provider_installation method block", + fmt.Sprintf("Invalid %s block at %s: %s.", methodTypeStr, block.Pos(), err), )) continue } @@ -115,20 +115,20 @@ func decodeProviderInstallationFromConfig(hclFile *hclast.File) ([]*ProviderInst Exclude []string `hcl:"exclude"` } var bodyContent BodyContent - err := hcl.DecodeObject(&bodyContent, sourceBody) + err := hcl.DecodeObject(&bodyContent, methodBody) if err != nil { diags = diags.Append(tfdiags.Sourceless( tfdiags.Error, - "Invalid provider_installation source block", - fmt.Sprintf("Invalid %s block at %s: %s.", sourceTypeStr, block.Pos(), err), + "Invalid provider_installation method block", + fmt.Sprintf("Invalid %s block at %s: %s.", methodTypeStr, block.Pos(), err), )) continue } if bodyContent.Path == "" { diags = diags.Append(tfdiags.Sourceless( tfdiags.Error, - "Invalid provider_installation source block", - fmt.Sprintf("Invalid %s block at %s: \"path\" argument is required.", sourceTypeStr, block.Pos()), + "Invalid provider_installation method block", + fmt.Sprintf("Invalid %s block at %s: \"path\" argument is required.", methodTypeStr, block.Pos()), )) continue } @@ -142,20 +142,20 @@ func decodeProviderInstallationFromConfig(hclFile *hclast.File) ([]*ProviderInst Exclude []string `hcl:"exclude"` } var bodyContent BodyContent - err := hcl.DecodeObject(&bodyContent, sourceBody) + err := hcl.DecodeObject(&bodyContent, methodBody) if err != nil { diags = diags.Append(tfdiags.Sourceless( tfdiags.Error, - "Invalid provider_installation source block", - fmt.Sprintf("Invalid %s block at %s: %s.", sourceTypeStr, block.Pos(), err), + "Invalid provider_installation method block", + fmt.Sprintf("Invalid %s block at %s: %s.", methodTypeStr, block.Pos(), err), )) continue } if bodyContent.URL == "" { diags = diags.Append(tfdiags.Sourceless( tfdiags.Error, - "Invalid provider_installation source block", - fmt.Sprintf("Invalid %s block at %s: \"url\" argument is required.", sourceTypeStr, block.Pos()), + "Invalid provider_installation method block", + fmt.Sprintf("Invalid %s block at %s: \"url\" argument is required.", methodTypeStr, block.Pos()), )) continue } @@ -165,13 +165,13 @@ func decodeProviderInstallationFromConfig(hclFile *hclast.File) ([]*ProviderInst default: diags = diags.Append(tfdiags.Sourceless( tfdiags.Error, - "Invalid provider_installation source block", - fmt.Sprintf("Unknown provider installation source type %q at %s.", sourceTypeStr, sourceBlock.Pos()), + "Invalid provider_installation method block", + fmt.Sprintf("Unknown provider installation method %q at %s.", methodTypeStr, methodBlock.Pos()), )) continue } - pi.Sources = append(pi.Sources, &ProviderInstallationSource{ + pi.Methods = append(pi.Methods, &ProviderInstallationMethod{ Location: location, Include: include, Exclude: exclude, @@ -184,22 +184,22 @@ func decodeProviderInstallationFromConfig(hclFile *hclast.File) ([]*ProviderInst return ret, diags } -// ProviderInstallationSource represents an installation source block inside +// ProviderInstallationMethod represents an installation method block inside // a provider_installation block. -type ProviderInstallationSource struct { - Location ProviderInstallationSourceLocation +type ProviderInstallationMethod struct { + Location ProviderInstallationLocation Include []string `hcl:"include"` Exclude []string `hcl:"exclude"` } -// ProviderInstallationSourceLocation is an interface type representing the -// different installation source types. The concrete implementations of +// ProviderInstallationLocation is an interface type representing the +// different installation location types. The concrete implementations of // this interface are: // // ProviderInstallationDirect: install from the provider's origin registry // ProviderInstallationFilesystemMirror(dir): install from a local filesystem mirror // ProviderInstallationNetworkMirror(host): install from a network mirror -type ProviderInstallationSourceLocation interface { +type ProviderInstallationLocation interface { providerInstallationLocation() } @@ -209,7 +209,7 @@ func (i configProviderInstallationDirect) providerInstallationLocation() {} // ProviderInstallationDirect is a ProviderInstallationSourceLocation // representing installation from a provider's origin registry. -var ProviderInstallationDirect ProviderInstallationSourceLocation = configProviderInstallationDirect{} +var ProviderInstallationDirect ProviderInstallationLocation = configProviderInstallationDirect{} // ProviderInstallationFilesystemMirror is a ProviderInstallationSourceLocation // representing installation from a particular local filesystem mirror. The diff --git a/command/cliconfig/provider_installation_test.go b/command/cliconfig/provider_installation_test.go index 553d0bc75..0712e0e9f 100644 --- a/command/cliconfig/provider_installation_test.go +++ b/command/cliconfig/provider_installation_test.go @@ -16,7 +16,7 @@ func TestLoadConfig_providerInstallation(t *testing.T) { want := &Config{ ProviderInstallation: []*ProviderInstallation{ { - Sources: []*ProviderInstallationSource{ + Methods: []*ProviderInstallationMethod{ { Location: ProviderInstallationFilesystemMirror("/tmp/example1"), Include: []string{"example.com/*/*"}, @@ -47,11 +47,11 @@ func TestLoadConfig_providerInstallationErrors(t *testing.T) { _, diags := loadConfigFile(filepath.Join(fixtureDir, "provider-installation-errors")) want := `7 problems: -- Invalid provider_installation source block: Unknown provider installation source type "not_a_thing" at 2:3. -- Invalid provider_installation source block: Invalid filesystem_mirror block at 1:1: "path" argument is required. -- Invalid provider_installation source block: Invalid network_mirror block at 1:1: "url" argument is required. -- Invalid provider_installation source block: The items inside the provider_installation block at 1:1 must all be blocks. -- Invalid provider_installation source block: The blocks inside the provider_installation block at 1:1 may not have any labels. +- Invalid provider_installation method block: Unknown provider installation method "not_a_thing" at 2:3. +- Invalid provider_installation method block: Invalid filesystem_mirror block at 1:1: "path" argument is required. +- Invalid provider_installation method block: Invalid network_mirror block at 1:1: "url" argument is required. +- Invalid provider_installation method block: The items inside the provider_installation block at 1:1 must all be blocks. +- Invalid provider_installation method block: The blocks inside the provider_installation block at 1:1 may not have any labels. - Invalid provider_installation block: The provider_installation block at 9:1 must not have any labels. - Invalid provider_installation block: The provider_installation block at 11:1 must not be introduced with an equals sign.` diff --git a/provider_source.go b/provider_source.go index 1d804342e..bdda08bd3 100644 --- a/provider_source.go +++ b/provider_source.go @@ -39,14 +39,14 @@ func explicitProviderSource(config *cliconfig.ProviderInstallation, services *di var diags tfdiags.Diagnostics var searchRules []getproviders.MultiSourceSelector - for _, sourceConfig := range config.Sources { - source, moreDiags := providerSourceForCLIConfigLocation(sourceConfig.Location, services) + for _, methodConfig := range config.Methods { + source, moreDiags := providerSourceForCLIConfigLocation(methodConfig.Location, services) diags = diags.Append(moreDiags) if moreDiags.HasErrors() { continue } - include, err := getproviders.ParseMultiSourceMatchingPatterns(sourceConfig.Include) + include, err := getproviders.ParseMultiSourceMatchingPatterns(methodConfig.Include) if err != nil { diags = diags.Append(tfdiags.Sourceless( tfdiags.Error, @@ -54,7 +54,7 @@ func explicitProviderSource(config *cliconfig.ProviderInstallation, services *di fmt.Sprintf("CLI config specifies invalid provider inclusion patterns: %s.", err), )) } - exclude, err := getproviders.ParseMultiSourceMatchingPatterns(sourceConfig.Include) + exclude, err := getproviders.ParseMultiSourceMatchingPatterns(methodConfig.Include) if err != nil { diags = diags.Append(tfdiags.Sourceless( tfdiags.Error, @@ -179,7 +179,7 @@ func implicitProviderSource(services *disco.Disco) getproviders.Source { return getproviders.MultiSource(searchRules) } -func providerSourceForCLIConfigLocation(loc cliconfig.ProviderInstallationSourceLocation, services *disco.Disco) (getproviders.Source, tfdiags.Diagnostics) { +func providerSourceForCLIConfigLocation(loc cliconfig.ProviderInstallationLocation, services *disco.Disco) (getproviders.Source, tfdiags.Diagnostics) { if loc == cliconfig.ProviderInstallationDirect { return getproviders.NewMemoizeSource( getproviders.NewRegistrySource(services),