From e0cb718d02ab650003011347e46b5ea126579144 Mon Sep 17 00:00:00 2001 From: Radek Simko Date: Mon, 12 Apr 2021 07:46:23 +0100 Subject: [PATCH] Differentiate raw and inferred 1-part source string (#3) * Differentiate raw and inferred 1-part source string * Update provider.go Co-authored-by: Alisdair McDiarmid Co-authored-by: Alisdair McDiarmid --- README.md | 42 ++++++---- provider.go | 104 +++++++++++++++++-------- provider_test.go | 196 +++++++++++++++++++++++++++++++++++++++++++++-- 3 files changed, 288 insertions(+), 54 deletions(-) diff --git a/README.md b/README.md index d814df9..2ed8398 100644 --- a/README.md +++ b/README.md @@ -11,7 +11,7 @@ via [`hashicorp/terraform-exec`](https://github.com/hashicorp/terraform-exec). ## Example ```go -p, err := ParseProviderSourceString("hashicorp/aws") +p, err := ParseRawProviderSourceString("hashicorp/aws") if err != nil { // deal with error } @@ -23,20 +23,20 @@ if err != nil { // } ``` -Please note that the `ParseProviderSourceString` is **NOT** equipped -to deal with legacy addresses such as `aws`. Such address will be parsed -as if provider belongs to `hashicorp` namespace in the public Registry. - ## Legacy address A legacy address is by itself (without more context) ambiguous. For example `aws` may represent either the official `hashicorp/aws` or just any custom-built provider called `aws`. -If you know the address was produced by Terraform <=0.12 and/or that you're -dealing with a legacy address, the following sequence of steps should be taken. +Such ambiguous address can be produced by Terraform `<=0.12`. You can +just use `ImpliedProviderForUnqualifiedType` if you know for sure +the address was produced by an affected version. + +If you do not have that context you should parse the string via +`ParseRawProviderSourceString` and then check `addr.IsLegacy()`. -(optional) Parse such legacy address by `NewLegacyProvider(name)`. +### What to do with a legacy address? Ask the Registry API whether and where the provider was moved to @@ -44,20 +44,31 @@ Ask the Registry API whether and where the provider was moved to ```sh # grafana (redirected to its own namespace) -$ curl -s https://registry.terraform.io/v1/providers/-/grafana/versions | jq .moved_to +$ curl -s https://registry.terraform.io/v1/providers/-/grafana/versions | jq '(.id, .moved_to)' +"terraform-providers/grafana" "grafana/grafana" # aws (provider without redirection) -$ curl -s https://registry.terraform.io/v1/providers/-/aws/versions | jq .moved_to +$ curl -s https://registry.terraform.io/v1/providers/-/aws/versions | jq '(.id, .moved_to)' +"hashicorp/aws" null ``` Then: - - Use `ParseProviderSourceString` for the _new_ (`moved_to`) address of any _moved_ provider (e.g. `grafana/grafana`). - - Use `ImpliedProviderForUnqualifiedType` for any other provider (e.g. `aws`) - - Depending on context `terraform` may also be parsed by `ParseProviderSourceString`, - which assumes `hashicorp/terraform` provider. Read more about this provider below. + - Reparse the _new_ address (`moved_to`) of any _moved_ provider (e.g. `grafana/grafana`) via `ParseRawProviderSourceString` + - Reparse the full address (`id`) of any other provider (e.g. `hashicorp/aws`) + +Depending on context (legacy) `terraform` may need to be parsed separately. +Read more about this provider below. + +If for some reason you cannot ask the Registry API you may also use +`ParseAndInferProviderSourceString` which assumes that any legacy address +(including `terraform`) belongs to the `hashicorp` namespace. + +If you cache results (which you should), ensure you have invalidation +mechanism in place because target (migrated) namespace may change. +Hard-coding migrations anywhere in code is strongly discouraged. ### `terraform` provider @@ -74,5 +85,4 @@ Alternatively you may just treat the address as the builtin provider, i.e. assume all of its logic including schema is contained within Terraform Core. -In such case you should use `ImpliedProviderForUnqualifiedType(typeName)`, -as the function makes such assumption. +In such case you should just use `NewBuiltInProvider("terraform")`. diff --git a/provider.go b/provider.go index 11a8ebb..4dd0a5b 100644 --- a/provider.go +++ b/provider.go @@ -219,7 +219,7 @@ func (pt Provider) Equals(other Provider) bool { return pt == other } -// ParseProviderSourceString parses the source attribute and returns a provider. +// ParseRawProviderSourceString parses the source attribute and returns a provider. // This is intended primarily to parse the FQN-like strings returned by // terraform-config-inspect. // @@ -227,42 +227,22 @@ func (pt Provider) Equals(other Provider) bool { // name // namespace/name // hostname/namespace/name -func ParseProviderSourceString(str string) (Provider, error) { +// +// "name"-only format is parsed as -/name (i.e. legacy namespace) +// requiring further identification of the namespace via Registry API +func ParseRawProviderSourceString(str string) (Provider, error) { var ret Provider - - // split the source string into individual components - parts := strings.Split(str, "/") - if len(parts) == 0 || len(parts) > 3 { - return ret, &ParserError{ - Summary: "Invalid provider source string", - Detail: `The "source" attribute must be in the format "[hostname/][namespace/]name"`, - } - } - - // check for an invalid empty string in any part - for i := range parts { - if parts[i] == "" { - return ret, &ParserError{ - Summary: "Invalid provider source string", - Detail: `The "source" attribute must be in the format "[hostname/][namespace/]name"`, - } - } - } - - // check the 'name' portion, which is always the last part - givenName := parts[len(parts)-1] - name, err := ParseProviderPart(givenName) + parts, err := parseSourceStringParts(str) if err != nil { - return ret, &ParserError{ - Summary: "Invalid provider type", - Detail: fmt.Sprintf(`Invalid provider type %q in source %q: %s"`, givenName, str, err), - } + return ret, err } + + name := parts[len(parts)-1] ret.Type = name ret.Hostname = DefaultRegistryHost if len(parts) == 1 { - return NewDefaultProvider(parts[0]), nil + return NewLegacyProvider(name), nil } if len(parts) >= 2 { @@ -352,10 +332,68 @@ func ParseProviderSourceString(str string) (Provider, error) { return ret, nil } -// MustParseProviderSourceString is a wrapper around ParseProviderSourceString that panics if +// ParseAndInferProviderSourceString parses the source attribute and returns a provider. +// This is intended primarily to parse the FQN-like strings returned by +// terraform-config-inspect. +// +// The following are valid source string formats: +// name +// namespace/name +// hostname/namespace/name +// +// "name" format is assumed to be hashicorp/name +func ParseAndInferProviderSourceString(str string) (Provider, error) { + var ret Provider + parts, err := parseSourceStringParts(str) + if err != nil { + return ret, err + } + + if len(parts) == 1 { + return NewDefaultProvider(parts[0]), nil + } + + return ParseRawProviderSourceString(str) +} + +func parseSourceStringParts(str string) ([]string, error) { + // split the source string into individual components + parts := strings.Split(str, "/") + if len(parts) == 0 || len(parts) > 3 { + return nil, &ParserError{ + Summary: "Invalid provider source string", + Detail: `The "source" attribute must be in the format "[hostname/][namespace/]name"`, + } + } + + // check for an invalid empty string in any part + for i := range parts { + if parts[i] == "" { + return nil, &ParserError{ + Summary: "Invalid provider source string", + Detail: `The "source" attribute must be in the format "[hostname/][namespace/]name"`, + } + } + } + + // check the 'name' portion, which is always the last part + givenName := parts[len(parts)-1] + name, err := ParseProviderPart(givenName) + if err != nil { + return nil, &ParserError{ + Summary: "Invalid provider type", + Detail: fmt.Sprintf(`Invalid provider type %q in source %q: %s"`, givenName, str, err), + } + } + parts[len(parts)-1] = name + + return parts, nil +} + +// MustParseRawProviderSourceString is a wrapper around ParseRawProviderSourceString that panics if // it returns an error. -func MustParseProviderSourceString(str string) Provider { - result, err := ParseProviderSourceString(str) +func MustParseRawProviderSourceString(str string) Provider { + result, err := ParseRawProviderSourceString(str) if err != nil { panic(err) } diff --git a/provider_test.go b/provider_test.go index 1bf7ad1..39260cf 100644 --- a/provider_test.go +++ b/provider_test.go @@ -281,7 +281,193 @@ func TestProviderIsLegacy(t *testing.T) { } } -func TestParseProviderSourceStr(t *testing.T) { +func TestParseAndInferProviderSourceString(t *testing.T) { + tests := map[string]struct { + Want Provider + Err bool + }{ + "registry.terraform.io/hashicorp/aws": { + Provider{ + Type: "aws", + Namespace: "hashicorp", + Hostname: DefaultRegistryHost, + }, + false, + }, + "registry.Terraform.io/HashiCorp/AWS": { + Provider{ + Type: "aws", + Namespace: "hashicorp", + Hostname: DefaultRegistryHost, + }, + false, + }, + "terraform.io/builtin/terraform": { + Provider{ + Type: "terraform", + Namespace: BuiltInProviderNamespace, + Hostname: BuiltInProviderHost, + }, + false, + }, + "terraform": { + Provider{ + Type: "terraform", + Namespace: "hashicorp", + Hostname: DefaultRegistryHost, + }, + false, + }, + "hashicorp/aws": { + Provider{ + Type: "aws", + Namespace: "hashicorp", + Hostname: DefaultRegistryHost, + }, + false, + }, + "HashiCorp/AWS": { + Provider{ + Type: "aws", + Namespace: "hashicorp", + Hostname: DefaultRegistryHost, + }, + false, + }, + "aws": { + Provider{ + Type: "aws", + Namespace: "hashicorp", + Hostname: DefaultRegistryHost, + }, + false, + }, + "AWS": { + Provider{ + Type: "aws", + Namespace: "hashicorp", + Hostname: DefaultRegistryHost, + }, + false, + }, + "example.com/foo-bar/baz-boop": { + Provider{ + Type: "baz-boop", + Namespace: "foo-bar", + Hostname: svchost.Hostname("example.com"), + }, + false, + }, + "foo-bar/baz-boop": { + Provider{ + Type: "baz-boop", + Namespace: "foo-bar", + Hostname: DefaultRegistryHost, + }, + false, + }, + "localhost:8080/foo/bar": { + Provider{ + Type: "bar", + Namespace: "foo", + Hostname: svchost.Hostname("localhost:8080"), + }, + false, + }, + "example.com/too/many/parts/here": { + Provider{}, + true, + }, + "/too///many//slashes": { + Provider{}, + true, + }, + "///": { + Provider{}, + true, + }, + "/ / /": { // empty strings + Provider{}, + true, + }, + "badhost!/hashicorp/aws": { + Provider{}, + true, + }, + "example.com/badnamespace!/aws": { + Provider{}, + true, + }, + "example.com/bad--namespace/aws": { + Provider{}, + true, + }, + "example.com/-badnamespace/aws": { + Provider{}, + true, + }, + "example.com/badnamespace-/aws": { + Provider{}, + true, + }, + "example.com/bad.namespace/aws": { + Provider{}, + true, + }, + "example.com/hashicorp/badtype!": { + Provider{}, + true, + }, + "example.com/hashicorp/bad--type": { + Provider{}, + true, + }, + "example.com/hashicorp/-badtype": { + Provider{}, + true, + }, + "example.com/hashicorp/badtype-": { + Provider{}, + true, + }, + "example.com/hashicorp/bad.type": { + Provider{}, + true, + }, + + // We forbid the terraform- prefix both because it's redundant to + // include "terraform" in a Terraform provider name and because we use + // the longer prefix terraform-provider- to hint for users who might be + // accidentally using the git repository name or executable file name + // instead of the provider type. + "example.com/hashicorp/terraform-provider-bad": { + Provider{}, + true, + }, + "example.com/hashicorp/terraform-bad": { + Provider{}, + true, + }, + } + + for name, test := range tests { + got, err := ParseAndInferProviderSourceString(name) + if diff := cmp.Diff(test.Want, got); diff != "" { + t.Errorf("mismatch (%q): %s", name, diff) + } + if err != nil { + if test.Err == false { + t.Errorf("got error: %s, expected success", err) + } + } else { + if test.Err { + t.Errorf("got success, expected error") + } + } + } +} + +func TestParseRawProviderSourceString(t *testing.T) { tests := map[string]struct { Want Provider Err bool @@ -317,7 +503,7 @@ func TestParseProviderSourceStr(t *testing.T) { "terraform": { Provider{ Type: "terraform", - Namespace: "hashicorp", + Namespace: LegacyProviderNamespace, Hostname: DefaultRegistryHost, }, false, @@ -341,7 +527,7 @@ func TestParseProviderSourceStr(t *testing.T) { "aws": { Provider{ Type: "aws", - Namespace: "hashicorp", + Namespace: LegacyProviderNamespace, Hostname: DefaultRegistryHost, }, false, @@ -349,7 +535,7 @@ func TestParseProviderSourceStr(t *testing.T) { "AWS": { Provider{ Type: "aws", - Namespace: "hashicorp", + Namespace: LegacyProviderNamespace, Hostname: DefaultRegistryHost, }, false, @@ -455,7 +641,7 @@ func TestParseProviderSourceStr(t *testing.T) { } for name, test := range tests { - got, err := ParseProviderSourceString(name) + got, err := ParseRawProviderSourceString(name) if diff := cmp.Diff(test.Want, got); diff != "" { t.Errorf("mismatch (%q): %s", name, diff) }