From b233aa39e9d00243bc74bc381237ff1f3f1e2ba4 Mon Sep 17 00:00:00 2001 From: Alisdair McDiarmid Date: Fri, 3 Apr 2020 08:37:40 -0400 Subject: [PATCH] addrs: Simplify presentation of provider FQNs The provider fully-qualified name string used in configuration is very long, and since most providers are hosted in the public registry, most of that length is redundant. This commit adds and uses a `ForDisplay` method, which simplifies the presentation of provider FQNs. If the hostname is the default hostname, we now display only the namespace and type. This is only used in UI, but should still be unambiguous, as it matches the FQN string parsing behaviour. --- addrs/provider.go | 15 +++++++- addrs/provider_test.go | 78 ++++++++++++++++++++++++++++++++++++++++++ command/init.go | 20 +++++------ 3 files changed, 102 insertions(+), 11 deletions(-) diff --git a/addrs/provider.go b/addrs/provider.go index 462194e166..9e13957e28 100644 --- a/addrs/provider.go +++ b/addrs/provider.go @@ -44,7 +44,7 @@ const BuiltInProviderNamespace = "builtin" // FQN. const LegacyProviderNamespace = "-" -// String returns an FQN string, indended for use in output. +// String returns an FQN string, indended for use in machine-readable output. func (pt Provider) String() string { if pt.IsZero() { panic("called String on zero-value addrs.Provider") @@ -52,6 +52,19 @@ func (pt Provider) String() string { return pt.Hostname.ForDisplay() + "/" + pt.Namespace + "/" + pt.Type } +// ForDisplay returns a user-friendly FQN string, simplified for readability. If +// the provider is using the default hostname, the hostname is omitted. +func (pt Provider) ForDisplay() string { + if pt.IsZero() { + panic("called ForDisplay on zero-value addrs.Provider") + } + + if pt.Hostname == DefaultRegistryHost { + return pt.Namespace + "/" + pt.Type + } + return pt.Hostname.ForDisplay() + "/" + pt.Namespace + "/" + pt.Type +} + // NewProvider constructs a provider address from its parts, and normalizes // the namespace and type parts to lowercase using unicode case folding rules // so that resulting addrs.Provider values can be compared using standard diff --git a/addrs/provider_test.go b/addrs/provider_test.go index ba2309e7b2..3abc96e111 100644 --- a/addrs/provider_test.go +++ b/addrs/provider_test.go @@ -7,6 +7,84 @@ import ( svchost "github.com/hashicorp/terraform-svchost" ) +func TestProviderString(t *testing.T) { + tests := []struct { + Input Provider + Want string + }{ + { + Provider{ + Type: "test", + Hostname: DefaultRegistryHost, + Namespace: "hashicorp", + }, + DefaultRegistryHost.ForDisplay() + "/hashicorp/test", + }, + { + Provider{ + Type: "test", + Hostname: "registry.terraform.com", + Namespace: "hashicorp", + }, + "registry.terraform.com/hashicorp/test", + }, + { + Provider{ + Type: "test", + Hostname: DefaultRegistryHost, + Namespace: "othercorp", + }, + DefaultRegistryHost.ForDisplay() + "/othercorp/test", + }, + } + + for _, test := range tests { + got := test.Input.String() + if got != test.Want { + t.Errorf("wrong result for %s\n", test.Input.String()) + } + } +} + +func TestProviderDisplay(t *testing.T) { + tests := []struct { + Input Provider + Want string + }{ + { + Provider{ + Type: "test", + Hostname: DefaultRegistryHost, + Namespace: "hashicorp", + }, + "hashicorp/test", + }, + { + Provider{ + Type: "test", + Hostname: "registry.terraform.com", + Namespace: "hashicorp", + }, + "registry.terraform.com/hashicorp/test", + }, + { + Provider{ + Type: "test", + Hostname: DefaultRegistryHost, + Namespace: "othercorp", + }, + "othercorp/test", + }, + } + + for _, test := range tests { + got := test.Input.ForDisplay() + if got != test.Want { + t.Errorf("wrong result for %s\n", test.Input.String()) + } + } +} + func TestProviderIsDefault(t *testing.T) { tests := []struct { Input Provider diff --git a/command/init.go b/command/init.go index a6f1b436d0..6edf1fd7c3 100644 --- a/command/init.go +++ b/command/init.go @@ -461,50 +461,50 @@ func (c *InitCommand) getProviders(earlyConfig *earlyconfig.Config, state *state )) }, ProviderAlreadyInstalled: func(provider addrs.Provider, selectedVersion getproviders.Version) { - c.Ui.Info(fmt.Sprintf("- Using previously-installed %s v%s", provider, selectedVersion)) + c.Ui.Info(fmt.Sprintf("- Using previously-installed %s v%s", provider.ForDisplay(), selectedVersion)) }, BuiltInProviderAvailable: func(provider addrs.Provider) { - c.Ui.Info(fmt.Sprintf("- %s is built in to Terraform", provider)) + c.Ui.Info(fmt.Sprintf("- %s is built in to Terraform", provider.ForDisplay())) }, BuiltInProviderFailure: func(provider addrs.Provider, err error) { diags = diags.Append(tfdiags.Sourceless( tfdiags.Error, "Invalid dependency on built-in provider", - fmt.Sprintf("Cannot use %s: %s.", provider, err), + fmt.Sprintf("Cannot use %s: %s.", provider.ForDisplay(), err), )) }, QueryPackagesBegin: func(provider addrs.Provider, versionConstraints getproviders.VersionConstraints) { if len(versionConstraints) > 0 { - c.Ui.Info(fmt.Sprintf("- Finding %s versions matching %q...", provider, getproviders.VersionConstraintsString(versionConstraints))) + c.Ui.Info(fmt.Sprintf("- Finding %s versions matching %q...", provider.ForDisplay(), getproviders.VersionConstraintsString(versionConstraints))) } else { - c.Ui.Info(fmt.Sprintf("- Finding latest version of %s...", provider)) + c.Ui.Info(fmt.Sprintf("- Finding latest version of %s...", provider.ForDisplay())) } }, LinkFromCacheBegin: func(provider addrs.Provider, version getproviders.Version, cacheRoot string) { - c.Ui.Info(fmt.Sprintf("- Using %s v%s from the shared cache directory", provider, version)) + c.Ui.Info(fmt.Sprintf("- Using %s v%s from the shared cache directory", provider.ForDisplay(), version)) }, FetchPackageBegin: func(provider addrs.Provider, version getproviders.Version, location getproviders.PackageLocation) { - c.Ui.Info(fmt.Sprintf("- Installing %s v%s...", provider, version)) + c.Ui.Info(fmt.Sprintf("- Installing %s v%s...", provider.ForDisplay(), version)) }, QueryPackagesFailure: func(provider addrs.Provider, err error) { diags = diags.Append(tfdiags.Sourceless( tfdiags.Error, "Failed to query available provider packages", - fmt.Sprintf("Could not retrieve the list of available versions for provider %s: %s.", provider, err), + fmt.Sprintf("Could not retrieve the list of available versions for provider %s: %s.", provider.ForDisplay(), err), )) }, LinkFromCacheFailure: func(provider addrs.Provider, version getproviders.Version, err error) { diags = diags.Append(tfdiags.Sourceless( tfdiags.Error, "Failed to install provider from shared cache", - fmt.Sprintf("Error while importing %s v%s from the shared cache directory: %s.", provider, version, err), + fmt.Sprintf("Error while importing %s v%s from the shared cache directory: %s.", provider.ForDisplay(), version, err), )) }, FetchPackageFailure: func(provider addrs.Provider, version getproviders.Version, err error) { diags = diags.Append(tfdiags.Sourceless( tfdiags.Error, "Failed to install provider", - fmt.Sprintf("Error while installing %s v%s: %s.", provider, version, err), + fmt.Sprintf("Error while installing %s v%s: %s.", provider.ForDisplay(), version, err), )) }, }