From 2c535d829d7b92007b83493b56b9ffbbe1f8b448 Mon Sep 17 00:00:00 2001 From: Martin Atkins Date: Tue, 21 Apr 2020 15:48:07 -0700 Subject: [PATCH] command/cliconfig: Decode provider_installation blocks This new CLI config block type allows explicitly specifying where Terraform should look to find provider plugins for installation. This is not used anywhere as of this commit, but in a future commit we'll change package main to treat the presence of a block of this type as a request to disable the default set of provider sources and use these explicitly- specified ones instead. --- command/cliconfig/cliconfig.go | 43 +++- command/cliconfig/cliconfig_test.go | 66 ++++- command/cliconfig/provider_installation.go | 236 ++++++++++++++++++ .../cliconfig/provider_installation_test.go | 66 +++++ .../cliconfig/testdata/provider-installation | 17 ++ .../testdata/provider-installation-errors | 11 + 6 files changed, 436 insertions(+), 3 deletions(-) create mode 100644 command/cliconfig/provider_installation.go create mode 100644 command/cliconfig/provider_installation_test.go create mode 100644 command/cliconfig/testdata/provider-installation create mode 100644 command/cliconfig/testdata/provider-installation-errors diff --git a/command/cliconfig/cliconfig.go b/command/cliconfig/cliconfig.go index ea0bf1e57..e36b11cc5 100644 --- a/command/cliconfig/cliconfig.go +++ b/command/cliconfig/cliconfig.go @@ -17,7 +17,7 @@ import ( "github.com/hashicorp/hcl" - "github.com/hashicorp/terraform-svchost" + svchost "github.com/hashicorp/terraform-svchost" "github.com/hashicorp/terraform/tfdiags" ) @@ -42,6 +42,12 @@ type Config struct { Credentials map[string]map[string]interface{} `hcl:"credentials"` CredentialsHelpers map[string]*ConfigCredentialsHelper `hcl:"credentials_helper"` + + // ProviderInstallation represents any provider_installation blocks + // in the configuration. Only one of these is allowed across the whole + // configuration, but we decode into a slice here so that we can handle + // that validation at validation time rather than initial decode time. + ProviderInstallation []*ProviderInstallation } // ConfigHost is the structure of the "host" nested block within the CLI @@ -57,6 +63,22 @@ type ConfigCredentialsHelper struct { Args []string `hcl:"args"` } +// ConfigProviderInstallationFilesystemMirror represents a "filesystem_mirror" +// block inside ConfigProviderInstallation. +type ConfigProviderInstallationFilesystemMirror struct { + Path string `hcl:"path"` + Include []string `hcl:"include"` + Exclude []string `hcl:"exclude"` +} + +// ConfigProviderInstallationNetworkMirror represents a "network_mirror" block +// inside ConfigProviderInstallation. +type ConfigProviderInstallationNetworkMirror struct { + Hostname string `hcl:"hostname"` + Include []string `hcl:"include"` + Exclude []string `hcl:"exclude"` +} + // BuiltinConfig is the built-in defaults for the configuration. These // can be overridden by user configurations. var BuiltinConfig Config @@ -136,6 +158,13 @@ func loadConfigFile(path string) (*Config, tfdiags.Diagnostics) { return result, diags } + // Deal with the provider_installation block, which is not handled using + // DecodeObject because its structure is not compatible with the + // limitations of that function. + providerInstBlocks, moreDiags := decodeProviderInstallationFromConfig(obj) + diags = diags.Append(moreDiags) + result.ProviderInstallation = providerInstBlocks + // Replace all env vars for k, v := range result.Providers { result.Providers[k] = os.ExpandEnv(v) @@ -242,6 +271,13 @@ func (c *Config) Validate() tfdiags.Diagnostics { ) } + // Should have zero or one "provider_installation" blocks + if len(c.ProviderInstallation) > 1 { + diags = diags.Append( + fmt.Errorf("No more than one provider_installation block may be specified"), + ) + } + return diags } @@ -310,6 +346,11 @@ func (c1 *Config) Merge(c2 *Config) *Config { } } + if (len(c1.ProviderInstallation) + len(c2.ProviderInstallation)) > 0 { + result.ProviderInstallation = append(result.ProviderInstallation, c1.ProviderInstallation...) + result.ProviderInstallation = append(result.ProviderInstallation, c2.ProviderInstallation...) + } + return &result } diff --git a/command/cliconfig/cliconfig_test.go b/command/cliconfig/cliconfig_test.go index 194296c4c..8ae79eb23 100644 --- a/command/cliconfig/cliconfig_test.go +++ b/command/cliconfig/cliconfig_test.go @@ -7,6 +7,7 @@ import ( "testing" "github.com/davecgh/go-spew/spew" + "github.com/google/go-cmp/cmp" ) // This is the directory where our test fixtures are. @@ -169,6 +170,29 @@ func TestConfigValidate(t *testing.T) { }, 1, // no more than one credentials_helper block allowed }, + "provider_installation good none": { + &Config{ + ProviderInstallation: nil, + }, + 0, + }, + "provider_installation good one": { + &Config{ + ProviderInstallation: []*ProviderInstallation{ + {}, + }, + }, + 0, + }, + "provider_installation too many": { + &Config{ + ProviderInstallation: []*ProviderInstallation{ + {}, + {}, + }, + }, + 1, // no more than one provider_installation block allowed + }, } for name, test := range tests { @@ -209,6 +233,19 @@ func TestConfig_Merge(t *testing.T) { CredentialsHelpers: map[string]*ConfigCredentialsHelper{ "buz": {}, }, + ProviderInstallation: []*ProviderInstallation{ + { + Sources: []*ProviderInstallationSource{ + {Location: ProviderInstallationFilesystemMirror("a")}, + {Location: ProviderInstallationFilesystemMirror("b")}, + }, + }, + { + Sources: []*ProviderInstallationSource{ + {Location: ProviderInstallationFilesystemMirror("c")}, + }, + }, + }, } c2 := &Config{ @@ -234,6 +271,13 @@ func TestConfig_Merge(t *testing.T) { CredentialsHelpers: map[string]*ConfigCredentialsHelper{ "biz": {}, }, + ProviderInstallation: []*ProviderInstallation{ + { + Sources: []*ProviderInstallationSource{ + {Location: ProviderInstallationFilesystemMirror("d")}, + }, + }, + }, } expected := &Config{ @@ -270,11 +314,29 @@ func TestConfig_Merge(t *testing.T) { "buz": {}, "biz": {}, }, + ProviderInstallation: []*ProviderInstallation{ + { + Sources: []*ProviderInstallationSource{ + {Location: ProviderInstallationFilesystemMirror("a")}, + {Location: ProviderInstallationFilesystemMirror("b")}, + }, + }, + { + Sources: []*ProviderInstallationSource{ + {Location: ProviderInstallationFilesystemMirror("c")}, + }, + }, + { + Sources: []*ProviderInstallationSource{ + {Location: ProviderInstallationFilesystemMirror("d")}, + }, + }, + }, } actual := c1.Merge(c2) - if !reflect.DeepEqual(actual, expected) { - t.Fatalf("bad: %#v", actual) + if diff := cmp.Diff(expected, actual); diff != "" { + t.Fatalf("wrong result\n%s", diff) } } diff --git a/command/cliconfig/provider_installation.go b/command/cliconfig/provider_installation.go new file mode 100644 index 000000000..10e8f4fc1 --- /dev/null +++ b/command/cliconfig/provider_installation.go @@ -0,0 +1,236 @@ +package cliconfig + +import ( + "fmt" + + "github.com/hashicorp/hcl" + hclast "github.com/hashicorp/hcl/hcl/ast" + "github.com/hashicorp/terraform/tfdiags" +) + +// ProviderInstallation is the structure of the "provider_installation" +// nested block within the CLI configuration. +type ProviderInstallation struct { + Sources []*ProviderInstallationSource +} + +// decodeProviderInstallationFromConfig uses the HCL AST API directly to +// decode "provider_installation" blocks from the given file. +// +// This uses the HCL AST directly, rather than HCL's decoder, because the +// intended configuration structure can't be represented using the HCL +// decoder's struct tags. This structure is intended as something that would +// be relatively easier to deal with in HCL 2 once we eventually migrate +// CLI config over to that, and so this function is stricter than HCL 1's +// decoder would be in terms of exactly what configuration shape it is +// expecting. +// +// Note that this function wants the top-level file object which might or +// might not contain provider_installation blocks, not a provider_installation +// block directly itself. +func decodeProviderInstallationFromConfig(hclFile *hclast.File) ([]*ProviderInstallation, tfdiags.Diagnostics) { + var ret []*ProviderInstallation + var diags tfdiags.Diagnostics + + root := hclFile.Node.(*hclast.ObjectList) + + // This is a rather odd hybrid: it's a HCL 2-like decode implemented using + // the HCL 1 AST API. That makes it a bit awkward in places, but it allows + // us to mimick the strictness of HCL 2 (making a later migration easier) + // and to support a block structure that the HCL 1 decoder can't represent. + for _, block := range root.Items { + if block.Keys[0].Token.Value() != "provider_installation" { + continue + } + if block.Assign.Line != 0 { + // Seems to be an attribute rather than a block + diags = diags.Append(tfdiags.Sourceless( + tfdiags.Error, + "Invalid provider_installation block", + fmt.Sprintf("The provider_installation block at %s must not be introduced with an equals sign.", block.Pos()), + )) + continue + } + if len(block.Keys) > 1 { + diags = diags.Append(tfdiags.Sourceless( + tfdiags.Error, + "Invalid provider_installation block", + fmt.Sprintf("The provider_installation block at %s must not have any labels.", block.Pos()), + )) + } + + pi := &ProviderInstallation{} + + // Because we checked block.Assign was unset above we can assume that + // we're reading something produced with block syntax and therefore + // it will always be an hclast.ObjectType. + body := block.Val.(*hclast.ObjectType) + + for _, sourceBlock := range body.List.Items { + if sourceBlock.Assign.Line != 0 { + // Seems to be an attribute rather than a block + diags = diags.Append(tfdiags.Sourceless( + tfdiags.Error, + "Invalid provider_installation source block", + fmt.Sprintf("The items inside the provider_installation block at %s must all be blocks.", block.Pos()), + )) + continue + } + if len(sourceBlock.Keys) > 1 { + diags = diags.Append(tfdiags.Sourceless( + tfdiags.Error, + "Invalid provider_installation source block", + fmt.Sprintf("The blocks inside the provider_installation block at %s may not have any labels.", block.Pos()), + )) + } + + sourceBody := sourceBlock.Val.(*hclast.ObjectType) + + sourceTypeStr := sourceBlock.Keys[0].Token.Value().(string) + var location ProviderInstallationSourceLocation + var include, exclude []string + var extraArgs []string + switch sourceTypeStr { + case "direct": + type BodyContent struct { + Include []string `hcl:"include"` + Exclude []string `hcl:"exclude"` + } + var bodyContent BodyContent + err := hcl.DecodeObject(&bodyContent, sourceBody) + 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), + )) + continue + } + location = ProviderInstallationDirect + include = bodyContent.Include + exclude = bodyContent.Exclude + case "filesystem_mirror": + type BodyContent struct { + Path string `hcl:"path"` + Include []string `hcl:"include"` + Exclude []string `hcl:"exclude"` + } + var bodyContent BodyContent + err := hcl.DecodeObject(&bodyContent, sourceBody) + 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), + )) + 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()), + )) + continue + } + location = ProviderInstallationFilesystemMirror(bodyContent.Path) + include = bodyContent.Include + exclude = bodyContent.Exclude + case "network_mirror": + type BodyContent struct { + Host string `hcl:"host"` + Include []string `hcl:"include"` + Exclude []string `hcl:"exclude"` + } + var bodyContent BodyContent + err := hcl.DecodeObject(&bodyContent, sourceBody) + 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), + )) + continue + } + if bodyContent.Host == "" { + diags = diags.Append(tfdiags.Sourceless( + tfdiags.Error, + "Invalid provider_installation source block", + fmt.Sprintf("Invalid %s block at %s: \"host\" argument is required.", sourceTypeStr, block.Pos()), + )) + continue + } + location = ProviderInstallationNetworkMirror(bodyContent.Host) + include = bodyContent.Include + exclude = bodyContent.Exclude + 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()), + )) + continue + } + + for _, argName := range extraArgs { + diags = diags.Append(tfdiags.Sourceless( + tfdiags.Error, + "Invalid provider_installation source block", + fmt.Sprintf("Invalid %s block at %s: this source type does not expect the argument %q.", sourceTypeStr, block.Pos(), argName), + )) + } + + pi.Sources = append(pi.Sources, &ProviderInstallationSource{ + Location: location, + Include: include, + Exclude: exclude, + }) + } + + ret = append(ret, pi) + } + + return ret, diags +} + +// ProviderInstallationSource represents an installation source block inside +// a provider_installation block. +type ProviderInstallationSource struct { + Location ProviderInstallationSourceLocation + Include []string `hcl:"include"` + Exclude []string `hcl:"exclude"` +} + +// ProviderInstallationSourceLocation is an interface type representing the +// different installation source 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 { + providerInstallationLocation() +} + +type configProviderInstallationDirect [0]byte + +func (i configProviderInstallationDirect) providerInstallationLocation() {} + +// ProviderInstallationDirect is a ProviderInstallationSourceLocation +// representing installation from a provider's origin registry. +var ProviderInstallationDirect ProviderInstallationSourceLocation = configProviderInstallationDirect{} + +// ProviderInstallationFilesystemMirror is a ProviderInstallationSourceLocation +// representing installation from a particular local filesystem mirror. The +// string value is the filesystem path to the mirror directory. +type ProviderInstallationFilesystemMirror string + +func (i ProviderInstallationFilesystemMirror) providerInstallationLocation() {} + +// ProviderInstallationNetworkMirror is a ProviderInstallationSourceLocation +// representing installation from a particular local network mirror. The +// string value is the hostname exactly as written in the configuration, without +// any normalization. +type ProviderInstallationNetworkMirror string + +func (i ProviderInstallationNetworkMirror) providerInstallationLocation() {} diff --git a/command/cliconfig/provider_installation_test.go b/command/cliconfig/provider_installation_test.go new file mode 100644 index 000000000..a9cb7e00c --- /dev/null +++ b/command/cliconfig/provider_installation_test.go @@ -0,0 +1,66 @@ +package cliconfig + +import ( + "path/filepath" + "testing" + + "github.com/google/go-cmp/cmp" +) + +func TestLoadConfig_providerInstallation(t *testing.T) { + got, diags := loadConfigFile(filepath.Join(fixtureDir, "provider-installation")) + if diags.HasErrors() { + t.Errorf("unexpected diagnostics: %s", diags.Err().Error()) + } + + want := &Config{ + ProviderInstallation: []*ProviderInstallation{ + { + Sources: []*ProviderInstallationSource{ + { + Location: ProviderInstallationFilesystemMirror("/tmp/example1"), + Include: []string{"example.com/*/*"}, + }, + { + Location: ProviderInstallationNetworkMirror("tf-Mirror.example.com"), + Include: []string{"registry.terraform.io/*/*"}, + Exclude: []string{"registry.Terraform.io/foobar/*"}, + }, + { + Location: ProviderInstallationFilesystemMirror("/tmp/example2"), + }, + { + Location: ProviderInstallationDirect, + Exclude: []string{"example.com/*/*"}, + }, + }, + }, + }, + } + + if diff := cmp.Diff(want, got); diff != "" { + t.Errorf("wrong result\n%s", diff) + } +} + +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: "host" 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 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.` + + // The above error messages include only line/column location information + // and not file location information because HCL 1 does not store + // information about the filename a location belongs to. (There is a field + // for it in token.Pos but it's always an empty string in practice.) + + if got := diags.Err().Error(); got != want { + t.Errorf("wrong diagnostics\ngot:\n%s\nwant:\n%s", got, want) + } +} diff --git a/command/cliconfig/testdata/provider-installation b/command/cliconfig/testdata/provider-installation new file mode 100644 index 000000000..ff2d5fbec --- /dev/null +++ b/command/cliconfig/testdata/provider-installation @@ -0,0 +1,17 @@ +provider_installation { + filesystem_mirror { + path = "/tmp/example1" + include = ["example.com/*/*"] + } + network_mirror { + host = "tf-Mirror.example.com" + include = ["registry.terraform.io/*/*"] + exclude = ["registry.Terraform.io/foobar/*"] + } + filesystem_mirror { + path = "/tmp/example2" + } + direct { + exclude = ["example.com/*/*"] + } +} diff --git a/command/cliconfig/testdata/provider-installation-errors b/command/cliconfig/testdata/provider-installation-errors new file mode 100644 index 000000000..8cf634e50 --- /dev/null +++ b/command/cliconfig/testdata/provider-installation-errors @@ -0,0 +1,11 @@ +provider_installation { + not_a_thing {} # unknown source type + filesystem_mirror {} # missing "path" argument + network_mirror {} # missing "host" argument + direct = {} # should be a block, not an argument + direct "what" {} # should not have a label +} + +provider_installation "what" {} # should not have a label + +provider_installation = {} # should be a block, not an argument