From c5b0a2b0b8943e8fb024d671d1c484a8085a6169 Mon Sep 17 00:00:00 2001 From: Brandon Croft Date: Mon, 2 Oct 2023 17:16:29 -0600 Subject: [PATCH 1/2] Revert 'add client caching by config, extract client and logging packages' --- internal/client/client.go | 263 ----------- internal/client/config.go | 254 ----------- internal/client/config_test.go | 158 ------- internal/client/init.go | 19 - internal/{client => provider}/config_unix.go | 2 +- .../{client => provider}/config_windows.go | 2 +- internal/{logging => provider}/logging.go | 2 +- internal/provider/plugin_provider.go | 3 +- internal/provider/provider.go | 412 +++++++++++++++++- internal/provider/provider_next.go | 3 +- internal/provider/provider_test.go | 156 ++++++- 11 files changed, 567 insertions(+), 707 deletions(-) delete mode 100644 internal/client/client.go delete mode 100644 internal/client/config.go delete mode 100644 internal/client/config_test.go delete mode 100644 internal/client/init.go rename internal/{client => provider}/config_unix.go (98%) rename internal/{client => provider}/config_windows.go (98%) rename internal/{logging => provider}/logging.go (99%) diff --git a/internal/client/client.go b/internal/client/client.go deleted file mode 100644 index c058b4902..000000000 --- a/internal/client/client.go +++ /dev/null @@ -1,263 +0,0 @@ -// Copyright (c) HashiCorp, Inc. -// SPDX-License-Identifier: MPL-2.0 - -package client - -import ( - "errors" - "fmt" - "log" - "net/url" - "os" - "sort" - "strings" - "sync" - - tfe "github.com/hashicorp/go-tfe" - "github.com/hashicorp/go-version" - providerVersion "github.com/hashicorp/terraform-provider-tfe/version" - svchost "github.com/hashicorp/terraform-svchost" - "github.com/hashicorp/terraform-svchost/disco" -) - -const ( - DefaultHostname = "app.terraform.io" -) - -var ( - ErrMissingAuthToken = errors.New("required token could not be found. Please set the token using an input variable in the provider configuration block or by using the TFE_TOKEN environment variable") - tfeServiceIDs = []string{"tfe.v2.2"} -) - -type ClientConfigMap struct { - mu sync.Mutex - values map[string]*tfe.Client -} - -func (c *ClientConfigMap) GetByConfig(config *ClientConfiguration) *tfe.Client { - if c.mu.TryLock() { - defer c.Unlock() - } - - return c.values[config.Key()] -} - -func (c *ClientConfigMap) Lock() { - c.mu.Lock() -} - -func (c *ClientConfigMap) Unlock() { - c.mu.Unlock() -} - -func (c *ClientConfigMap) Set(client *tfe.Client, config *ClientConfiguration) { - if c.mu.TryLock() { - defer c.Unlock() - } - c.values[config.Key()] = client -} - -func getTokenFromEnv() string { - log.Printf("[DEBUG] TFE_TOKEN used for token value") - return os.Getenv("TFE_TOKEN") -} - -func getTokenFromCreds(services *disco.Disco, hostname svchost.Hostname) string { - log.Printf("[DEBUG] Attempting to fetch token from Terraform CLI configuration for hostname %s...", hostname) - creds, err := services.CredentialsForHost(hostname) - if err != nil { - log.Printf("[DEBUG] Failed to get credentials for %s: %s (ignoring)", hostname, err) - } - if creds != nil { - return creds.Token() - } - return "" -} - -// GetClient encapsulates the logic for configuring a go-tfe client instance for -// the provider, including fallback to values from environment variables. This -// is useful because we're muxing multiple provider servers together and each -// one needs an identically configured client. -// -// Internally, this function caches configured clients using the specified -// parameters -func GetClient(tfeHost, token string, insecure bool) (*tfe.Client, error) { - config, err := configure(tfeHost, token, insecure) - if err != nil { - return nil, err - } - - clientCache.Lock() - defer clientCache.Unlock() - - // Try to retrieve the client from cache - cached := clientCache.GetByConfig(config) - if cached != nil { - return cached, nil - } - - // Discover the Terraform Enterprise address. - host, err := config.Services.Discover(config.TFEHost) - if err != nil { - return nil, err - } - - // Get the full Terraform Enterprise service address. - var address *url.URL - var discoErr error - for _, tfeServiceID := range tfeServiceIDs { - service, err := host.ServiceURL(tfeServiceID) - if _, ok := err.(*disco.ErrVersionNotSupported); !ok && err != nil { - return nil, err - } - // If discoErr is nil we save the first error. When multiple services - // are checked and we found one that didn't give an error we need to - // reset the discoErr. So if err is nil, we assign it as well. - if discoErr == nil || err == nil { - discoErr = err - } - if service != nil { - address = service - break - } - } - - if providerVersion.ProviderVersion != "dev" { - // We purposefully ignore the error and return the previous error, as - // checking for version constraints is considered optional. - constraints, _ := host.VersionConstraints(tfeServiceIDs[0], "tfe-provider") - - // First check any constraints we might have received. - if constraints != nil { - if err := CheckConstraints(constraints); err != nil { - return nil, err - } - } - } - - // When we don't have any constraints errors, also check for discovery - // errors before we continue. - if discoErr != nil { - return nil, discoErr - } - - // Create a new TFE client. - client, err := tfe.NewClient(&tfe.Config{ - Address: address.String(), - Token: token, - HTTPClient: config.HTTPClient, - }) - if err != nil { - return nil, err - } - - client.RetryServerErrors(true) - clientCache.Set(client, config) - - return client, nil -} - -// CheckConstraints checks service version constrains against our own -// version and returns rich and informational diagnostics in case any -// incompatibilities are detected. -func CheckConstraints(c *disco.Constraints) error { - if c == nil || c.Minimum == "" || c.Maximum == "" { - return nil - } - - // Generate a parsable constraints string. - excluding := "" - if len(c.Excluding) > 0 { - excluding = fmt.Sprintf(", != %s", strings.Join(c.Excluding, ", != ")) - } - constStr := fmt.Sprintf(">= %s%s, <= %s", c.Minimum, excluding, c.Maximum) - - // Create the constraints to check against. - constraints, err := version.NewConstraint(constStr) - if err != nil { - return checkConstraintsWarning(err) - } - - // Create the version to check. - v, err := version.NewVersion(providerVersion.ProviderVersion) - if err != nil { - return checkConstraintsWarning(err) - } - - // Return if we satisfy all constraints. - if constraints.Check(v) { - return nil - } - - // Find out what action (upgrade/downgrade) we should advice. - minimum, err := version.NewVersion(c.Minimum) - if err != nil { - return checkConstraintsWarning(err) - } - - maximum, err := version.NewVersion(c.Maximum) - if err != nil { - return checkConstraintsWarning(err) - } - - var excludes []*version.Version - for _, exclude := range c.Excluding { - v, err := version.NewVersion(exclude) - if err != nil { - return checkConstraintsWarning(err) - } - excludes = append(excludes, v) - } - - // Sort all the excludes. - sort.Sort(version.Collection(excludes)) - - var action, toVersion string - switch { - case minimum.GreaterThan(v): - action = "upgrade" - toVersion = ">= " + minimum.String() - case maximum.LessThan(v): - action = "downgrade" - toVersion = "<= " + maximum.String() - case len(excludes) > 0: - // Get the latest excluded version. - action = "upgrade" - toVersion = "> " + excludes[len(excludes)-1].String() - } - - switch { - case len(excludes) == 1: - excluding = fmt.Sprintf(", excluding version %s", excludes[0].String()) - case len(excludes) > 1: - var vs []string - for _, v := range excludes { - vs = append(vs, v.String()) - } - excluding = fmt.Sprintf(", excluding versions %s", strings.Join(vs, ", ")) - default: - excluding = "" - } - - summary := fmt.Sprintf("Incompatible TFE provider version v%s", v.String()) - details := fmt.Sprintf( - "The configured Terraform Enterprise backend is compatible with TFE provider\n"+ - "versions >= %s, <= %s%s.", c.Minimum, c.Maximum, excluding, - ) - - if action != "" && toVersion != "" { - summary = fmt.Sprintf("Please %s the TFE provider to %s", action, toVersion) - } - - // Return the customized and informational error message. - return fmt.Errorf("%s\n\n%s", summary, details) -} - -func checkConstraintsWarning(err error) error { - return fmt.Errorf( - "failed to check version constraints: %v\n\n"+ - "checking version constraints is considered optional, but this is an\n"+ - "unexpected error which should be reported", - err, - ) -} diff --git a/internal/client/config.go b/internal/client/config.go deleted file mode 100644 index 855c949f5..000000000 --- a/internal/client/config.go +++ /dev/null @@ -1,254 +0,0 @@ -// Copyright (c) HashiCorp, Inc. -// SPDX-License-Identifier: MPL-2.0 - -package client - -import ( - "crypto/sha256" - "crypto/tls" - "fmt" - "log" - "net/http" - "os" - "strconv" - - tfe "github.com/hashicorp/go-tfe" - "github.com/hashicorp/hcl" - "github.com/hashicorp/terraform-provider-tfe/internal/logging" - providerVersion "github.com/hashicorp/terraform-provider-tfe/version" - svchost "github.com/hashicorp/terraform-svchost" - "github.com/hashicorp/terraform-svchost/auth" - "github.com/hashicorp/terraform-svchost/disco" -) - -var ( - // TFEUserAgent is the user agent string sent with all requests made by the provider - TFEUserAgent = fmt.Sprintf("terraform-provider-tfe/%s", providerVersion.ProviderVersion) -) - -type CredentialsMap map[string]map[string]interface{} - -// CLIHostConfig is the structure of the configuration for the Terraform CLI. -type CLIHostConfig struct { - Hosts map[string]*ConfigHost `hcl:"host"` - Credentials CredentialsMap `hcl:"credentials"` -} - -// ConfigHost is the structure of the "host" nested block within the CLI -// configuration, which can be used to override the default service host -// discovery behavior for a particular hostname. -type ConfigHost struct { - Services map[string]interface{} `hcl:"services"` -} - -// ClientConfiguration is the refined information needed to configure a tfe.Client -type ClientConfiguration struct { - Services *disco.Disco - HTTPClient *http.Client - TFEHost svchost.Hostname - Token string - Insecure bool -} - -// Key returns a string that is comparable to other ClientConfiguration values -func (c ClientConfiguration) Key() string { - return fmt.Sprintf("%x %s/%v", sha256.New().Sum([]byte(c.Token)), c.TFEHost, c.Insecure) -} - -// cliConfig tries to find and parse the configuration of the Terraform CLI. -// This is an optional step, so any errors are ignored. -func cliConfig() CLIHostConfig { - mainConfig := CLIHostConfig{} - credentialsConfig := CLIHostConfig{} - combinedConfig := CLIHostConfig{} - - // Main CLI config file; might contain manually-entered credentials, and/or - // some host service discovery objects. Location is configurable via - // environment variables. - configFilePath := locateConfigFile() - if configFilePath != "" { - mainConfig = readCliConfigFile(configFilePath) - } - - // Credentials file; might contain credentials auto-configured by terraform - // login. Location isn't configurable. - credentialsFilePath, err := credentialsFile() - if err != nil { - log.Printf("[ERROR] Error detecting default credentials file path: %s", err) - } else { - credentialsConfig = readCliConfigFile(credentialsFilePath) - } - - // Use host service discovery configs from main config file. - combinedConfig.Hosts = mainConfig.Hosts - - // Combine both sets of credentials. Per Terraform's own behavior, the main - // config file overrides the credentials file if they have any overlapping - // hostnames. - combinedConfig.Credentials = credentialsConfig.Credentials - if combinedConfig.Credentials == nil { - combinedConfig.Credentials = make(map[string]map[string]interface{}) - } - for host, creds := range mainConfig.Credentials { - combinedConfig.Credentials[host] = creds - } - - return combinedConfig -} - -func locateConfigFile() string { - // To find the main CLI config file, follow Terraform's own logic: try - // TF_CLI_CONFIG_FILE, then try TERRAFORM_CONFIG, then try the default - // location. - - if os.Getenv("TF_CLI_CONFIG_FILE") != "" { - return os.Getenv("TF_CLI_CONFIG_FILE") - } - - if os.Getenv("TERRAFORM_CONFIG") != "" { - return os.Getenv("TERRAFORM_CONFIG") - } - filePath, err := configFile() - if err != nil { - log.Printf("[ERROR] Error detecting default CLI config file path: %s", err) - return "" - } - - return filePath -} - -// All the errors returned by the helper methods called in this function get ignored (down the road we throw an error when all auth methods have failed.) We only use these errors to log warnings to the user. -func readCliConfigFile(configFilePath string) CLIHostConfig { - config := CLIHostConfig{} - - // Read the CLI config file content. - content, err := os.ReadFile(configFilePath) - if err != nil { - log.Printf("[WARN] Unable to read CLI config or credentials file %s: %v", configFilePath, err) - return config - } - - // Parse the CLI config file content. - obj, err := hcl.Parse(string(content)) - if err != nil { - log.Printf("[WARN] Unable to parse CLI config or credentials file %s: %v", configFilePath, err) - return config - } - - // Decode the CLI config file content. - if err := hcl.DecodeObject(&config, obj); err != nil { - log.Printf("[WARN] Unable to decode CLI config or credentials file %s: %v", configFilePath, err) - } - - return config -} - -func credentialsSource(credentials CredentialsMap) auth.CredentialsSource { - creds := auth.NoCredentials - - // Add all configured credentials to the credentials source. - if len(credentials) > 0 { - staticTable := map[svchost.Hostname]map[string]interface{}{} - for userHost, creds := range credentials { - host, err := svchost.ForComparison(userHost) - if err != nil { - // We expect the config was already validated by the time we get - // here, so we'll just ignore invalid hostnames. - continue - } - staticTable[host] = creds - } - creds = auth.StaticCredentialsSource(staticTable) - } - - return creds -} - -// configure accepts the provider-level configuration values and creates a -// clientConfiguration using fallback values from the environment or CLI configuration. -func configure(tfeHost, token string, insecure bool) (*ClientConfiguration, error) { - if tfeHost == "" { - if os.Getenv("TFE_HOSTNAME") != "" { - tfeHost = os.Getenv("TFE_HOSTNAME") - } else { - tfeHost = DefaultHostname - } - } - log.Printf("[DEBUG] Configuring client for host %q", tfeHost) - - // If ssl_skip_verify is false, it is either set that way in configuration or unset. Check - // the environment to see if it was set to true there. Strictly speaking, this means that - // the env var can override an explicit 'false' in configuration (which is not true of the - // other settings), but that's how it goes with a boolean zero value. - var err error - if !insecure && os.Getenv("TFE_SSL_SKIP_VERIFY") != "" { - v := os.Getenv("TFE_SSL_SKIP_VERIFY") - insecure, err = strconv.ParseBool(v) - if err != nil { - return nil, err - } - } - - // Configure the certificate verification options. - if insecure { - log.Printf("[DEBUG] Warning: Client configured to skip certificate verifications") - } - - // Parse the hostname for comparison, - hostname, err := svchost.ForComparison(tfeHost) - if err != nil { - return nil, err - } - - httpClient := tfe.DefaultConfig().HTTPClient - - // Make sure the transport has a TLS config. - transport := httpClient.Transport.(*http.Transport) - if transport.TLSClientConfig == nil { - transport.TLSClientConfig = &tls.Config{MinVersion: tls.VersionTLS12} - } - - transport.TLSClientConfig.InsecureSkipVerify = insecure - - // Get the Terraform CLI configuration. - config := cliConfig() - - // Create a new credential source and service discovery object. - credsSrc := credentialsSource(config.Credentials) - services := disco.NewWithCredentialsSource(credsSrc) - services.SetUserAgent(TFEUserAgent) - services.Transport = logging.NewLoggingTransport("TFE", transport) - - // Add any static host configurations service discovery object. - for userHost, hostConfig := range config.Hosts { - host, err := svchost.ForComparison(userHost) - if err != nil { - // ignore invalid hostnames. - continue - } - services.ForceHostServices(host, hostConfig.Services) - } - - // If a token wasn't set in the provider configuration block, try and fetch it - // from the environment or from Terraform's CLI configuration or configured credential helper. - if token == "" { - if os.Getenv("TFE_TOKEN") != "" { - token = getTokenFromEnv() - } else { - token = getTokenFromCreds(services, hostname) - } - } - - // If we still don't have a token at this point, we return an error. - if token == "" { - return nil, ErrMissingAuthToken - } - - return &ClientConfiguration{ - Services: services, - HTTPClient: httpClient, - TFEHost: hostname, - Token: token, - Insecure: insecure, - }, nil -} diff --git a/internal/client/config_test.go b/internal/client/config_test.go deleted file mode 100644 index e985bd2c7..000000000 --- a/internal/client/config_test.go +++ /dev/null @@ -1,158 +0,0 @@ -// Copyright (c) HashiCorp, Inc. -// SPDX-License-Identifier: MPL-2.0 - -package client - -import ( - "os" - "testing" -) - -func TestConfig_locateConfigFile(t *testing.T) { - originalHome := os.Getenv("HOME") - originalTfCliConfigFile := os.Getenv("TF_CLI_CONFIG_FILE") - originalTerraformConfig := os.Getenv("TERRAFORM_CONFIG") - reset := func() { - os.Setenv("HOME", originalHome) - if originalTfCliConfigFile != "" { - os.Setenv("TF_CLI_CONFIG_FILE", originalTfCliConfigFile) - } else { - os.Unsetenv("TF_CLI_CONFIG_FILE") - } - if originalTerraformConfig != "" { - os.Setenv("TERRAFORM_CONFIG", originalTerraformConfig) - } else { - os.Unsetenv("TERRAFORM_CONFIG") - } - } - defer reset() - - // Use a predictable value for $HOME - os.Setenv("HOME", "/Users/someone") - - setup := func(tfCliConfigFile, terraformConfig string) { - os.Setenv("TF_CLI_CONFIG_FILE", tfCliConfigFile) - os.Setenv("TERRAFORM_CONFIG", terraformConfig) - } - - cases := map[string]struct { - tfCliConfigFile string - terraformConfig string - result string - }{ - "has TF_CLI_CONFIG_FILE": { - tfCliConfigFile: "~/.terraform_alternate/terraformrc", - terraformConfig: "", - result: "~/.terraform_alternate/terraformrc", - }, - "has TERRAFORM_CONFIG": { - tfCliConfigFile: "", - terraformConfig: "~/.terraform_alternate_rc", - result: "~/.terraform_alternate_rc", - }, - "has both env vars": { - tfCliConfigFile: "~/.from_TF_CLI", - terraformConfig: "~/.from_TERRAFORM_CONFIG", - result: "~/.from_TF_CLI", - }, - "has neither env var": { - tfCliConfigFile: "", - terraformConfig: "", - result: "/Users/someone/.terraformrc", // expect tests run on unix - }, - } - - for name, tc := range cases { - setup(tc.tfCliConfigFile, tc.terraformConfig) - - fileResult := locateConfigFile() - if tc.result != fileResult { - t.Fatalf("%s: expected config file at %s, got %s", name, tc.result, fileResult) - } - } -} - -func TestConfig_cliConfig(t *testing.T) { - // This only tests the behavior of merging various combinations of - // (credentials file, .terraformrc file, absent). Locating the .terraformrc - // file is tested separately. - originalHome := os.Getenv("HOME") - originalTfCliConfigFile := os.Getenv("TF_CLI_CONFIG_FILE") - reset := func() { - os.Setenv("HOME", originalHome) - if originalTfCliConfigFile != "" { - os.Setenv("TF_CLI_CONFIG_FILE", originalTfCliConfigFile) - } else { - os.Unsetenv("TF_CLI_CONFIG_FILE") - } - } - defer reset() - - // Summary of fixtures: the credentials file and terraformrc file each have - // credentials for two hosts, they both have credentials for app.terraform.io, - // and the terraformrc also has one service discovery override. - hasCredentials := "test-fixtures/cli-config-files/home" - noCredentials := "test-fixtures/cli-config-files/no-credentials" - terraformrc := "test-fixtures/cli-config-files/terraformrc" - noTerraformrc := "test-fixtures/cli-config-files/no-terraformrc" - tokenFromTerraformrc := "something.atlasv1.prod_rc_file" - tokenFromCredentials := "something.atlasv1.prod_credentials_file" - - cases := map[string]struct { - home string - rcfile string - expectCount int - expectProdToken string - expectHostsCount int - }{ - "both main config and credentials JSON": { - home: hasCredentials, - rcfile: terraformrc, - expectCount: 3, - expectProdToken: tokenFromTerraformrc, - expectHostsCount: 1, - }, - "only main config": { - home: noCredentials, - rcfile: terraformrc, - expectCount: 2, - expectProdToken: tokenFromTerraformrc, - expectHostsCount: 1, - }, - "only credentials JSON": { - home: hasCredentials, - rcfile: noTerraformrc, - expectCount: 2, - expectProdToken: tokenFromCredentials, - expectHostsCount: 0, - }, - "neither file": { - home: noCredentials, - rcfile: noTerraformrc, - expectCount: 0, - expectProdToken: "", - expectHostsCount: 0, - }, - } - - for name, tc := range cases { - os.Setenv("HOME", tc.home) - os.Setenv("TF_CLI_CONFIG_FILE", tc.rcfile) - config := cliConfig() - credentialsCount := len(config.Credentials) - if credentialsCount != tc.expectCount { - t.Fatalf("%s: expected %d credentials, got %d", name, tc.expectCount, credentialsCount) - } - prodToken := "" - if config.Credentials["app.terraform.io"] != nil { - prodToken = config.Credentials["app.terraform.io"]["token"].(string) - } - if prodToken != tc.expectProdToken { - t.Fatalf("%s: expected %s as prod token, got %s", name, tc.expectProdToken, prodToken) - } - hostsCount := len(config.Hosts) - if hostsCount != tc.expectHostsCount { - t.Fatalf("%s: expected %d `host` blocks in the final config, got %d", name, tc.expectHostsCount, hostsCount) - } - } -} diff --git a/internal/client/init.go b/internal/client/init.go deleted file mode 100644 index dfb1392d1..000000000 --- a/internal/client/init.go +++ /dev/null @@ -1,19 +0,0 @@ -// Copyright (c) HashiCorp, Inc. -// SPDX-License-Identifier: MPL-2.0 - -package client - -import ( - "sync" - - tfe "github.com/hashicorp/go-tfe" -) - -var clientCache *ClientConfigMap - -func init() { - clientCache = &ClientConfigMap{ - values: make(map[string]*tfe.Client), - mu: sync.Mutex{}, - } -} diff --git a/internal/client/config_unix.go b/internal/provider/config_unix.go similarity index 98% rename from internal/client/config_unix.go rename to internal/provider/config_unix.go index 9f8cef07f..a2e7e55bb 100644 --- a/internal/client/config_unix.go +++ b/internal/provider/config_unix.go @@ -4,7 +4,7 @@ //go:build !windows // +build !windows -package client +package provider import ( "errors" diff --git a/internal/client/config_windows.go b/internal/provider/config_windows.go similarity index 98% rename from internal/client/config_windows.go rename to internal/provider/config_windows.go index 6ba3e0815..61f40d71f 100644 --- a/internal/client/config_windows.go +++ b/internal/provider/config_windows.go @@ -4,7 +4,7 @@ //go:build windows // +build windows -package client +package provider import ( "path/filepath" diff --git a/internal/logging/logging.go b/internal/provider/logging.go similarity index 99% rename from internal/logging/logging.go rename to internal/provider/logging.go index 07c23ffa2..44b404495 100644 --- a/internal/logging/logging.go +++ b/internal/provider/logging.go @@ -1,7 +1,7 @@ // Copyright (c) HashiCorp, Inc. // SPDX-License-Identifier: MPL-2.0 -package logging +package provider import ( "bytes" diff --git a/internal/provider/plugin_provider.go b/internal/provider/plugin_provider.go index c51141ba3..375609e72 100644 --- a/internal/provider/plugin_provider.go +++ b/internal/provider/plugin_provider.go @@ -11,7 +11,6 @@ import ( tfe "github.com/hashicorp/go-tfe" "github.com/hashicorp/terraform-plugin-go/tfprotov5" "github.com/hashicorp/terraform-plugin-go/tftypes" - "github.com/hashicorp/terraform-provider-tfe/internal/client" ) type pluginProviderServer struct { @@ -76,7 +75,7 @@ func (p *pluginProviderServer) ConfigureProvider(ctx context.Context, req *tfpro return resp, nil } - client, err := client.GetClient(meta.hostname, meta.token, meta.sslSkipVerify) + client, err := getClient(meta.hostname, meta.token, meta.sslSkipVerify) if err != nil { resp.Diagnostics = append(resp.Diagnostics, &tfprotov5.Diagnostic{ Severity: tfprotov5.DiagnosticSeverityError, diff --git a/internal/provider/provider.go b/internal/provider/provider.go index 9e70955c8..656155347 100644 --- a/internal/provider/provider.go +++ b/internal/provider/provider.go @@ -5,21 +5,50 @@ package provider import ( "context" + "crypto/tls" "errors" + "fmt" + "log" + "net/http" + "net/url" "os" + "sort" + "strconv" + "strings" tfe "github.com/hashicorp/go-tfe" + version "github.com/hashicorp/go-version" + "github.com/hashicorp/hcl" "github.com/hashicorp/terraform-plugin-sdk/v2/diag" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" - "github.com/hashicorp/terraform-provider-tfe/internal/client" + providerVersion "github.com/hashicorp/terraform-provider-tfe/version" + svchost "github.com/hashicorp/terraform-svchost" + "github.com/hashicorp/terraform-svchost/auth" + "github.com/hashicorp/terraform-svchost/disco" ) +const defaultHostname = "app.terraform.io" const defaultSSLSkipVerify = false var ( + tfeServiceIDs = []string{"tfe.v2.2"} + errMissingAuthToken = errors.New("required token could not be found. Please set the token using an input variable in the provider configuration block or by using the TFE_TOKEN environment variable") errMissingOrganization = errors.New("no organization was specified on the resource or provider") ) +// Config is the structure of the configuration for the Terraform CLI. +type Config struct { + Hosts map[string]*ConfigHost `hcl:"host"` + Credentials map[string]map[string]interface{} `hcl:"credentials"` +} + +// ConfigHost is the structure of the "host" nested block within the CLI +// configuration, which can be used to override the default service host +// discovery behavior for a particular hostname. +type ConfigHost struct { + Services map[string]interface{} `hcl:"services"` +} + // ConfiguredClient wraps the tfe.Client the provider uses, plus the default // organization name to be used by resources that need an organization but don't // specify one. @@ -173,7 +202,386 @@ func configureClient(d *schema.ResourceData) (*tfe.Client, error) { token := d.Get("token").(string) insecure := d.Get("ssl_skip_verify").(bool) - return client.GetClient(hostname, token, insecure) + return getClient(hostname, token, insecure) +} + +func getTokenFromEnv() string { + log.Printf("[DEBUG] TFE_TOKEN used for token value") + return os.Getenv("TFE_TOKEN") +} + +func getTokenFromCreds(services *disco.Disco, hostname svchost.Hostname) string { + log.Printf("[DEBUG] Attempting to fetch token from Terraform CLI configuration for hostname %s...", hostname) + creds, err := services.CredentialsForHost(hostname) + if err != nil { + log.Printf("[DEBUG] Failed to get credentials for %s: %s (ignoring)", hostname, err) + } + if creds != nil { + return creds.Token() + } + return "" +} + +// getClient encapsulates the logic for configuring a go-tfe client instance for +// the provider, including fallback to values from environment variables. This +// is useful because we're muxing multiple provider servers together and each +// one needs an identically configured client. +func getClient(tfeHost, token string, insecure bool) (*tfe.Client, error) { + h := tfeHost + if tfeHost == "" { + if os.Getenv("TFE_HOSTNAME") != "" { + h = os.Getenv("TFE_HOSTNAME") + } else { + h = defaultHostname + } + } + + log.Printf("[DEBUG] Configuring client for host %q", h) + + // Parse the hostname for comparison, + hostname, err := svchost.ForComparison(h) + if err != nil { + return nil, err + } + + providerUaString := fmt.Sprintf("terraform-provider-tfe/%s", providerVersion.ProviderVersion) + + httpClient := tfe.DefaultConfig().HTTPClient + + // Make sure the transport has a TLS config. + transport := httpClient.Transport.(*http.Transport) + if transport.TLSClientConfig == nil { + transport.TLSClientConfig = &tls.Config{MinVersion: tls.VersionTLS12} + } + + // If ssl_skip_verify is false, it is either set that way in configuration or unset. Check + // the environment to see if it was set to true there. Strictly speaking, this means that + // the env var can override an explicit 'false' in configuration (which is not true of the + // other settings), but that's how it goes with a boolean zero value. + if !insecure && os.Getenv("TFE_SSL_SKIP_VERIFY") != "" { + v := os.Getenv("TFE_SSL_SKIP_VERIFY") + insecure, err = strconv.ParseBool(v) + if err != nil { + return nil, err + } + } + + // Configure the certificate verification options. + if insecure { + log.Printf("[DEBUG] Warning: Client configured to skip certificate verifications") + } + transport.TLSClientConfig.InsecureSkipVerify = insecure + + // Get the Terraform CLI configuration. + config := cliConfig() + + // Create a new credential source and service discovery object. + credsSrc := credentialsSource(config) + services := disco.NewWithCredentialsSource(credsSrc) + services.SetUserAgent(providerUaString) + services.Transport = NewLoggingTransport("TFE Discovery", transport) + + // Add any static host configurations service discovery object. + for userHost, hostConfig := range config.Hosts { + host, err := svchost.ForComparison(userHost) + if err != nil { + // ignore invalid hostnames. + continue + } + services.ForceHostServices(host, hostConfig.Services) + } + + // Discover the Terraform Enterprise address. + host, err := services.Discover(hostname) + if err != nil { + return nil, err + } + + // Get the full Terraform Enterprise service address. + var address *url.URL + var discoErr error + for _, tfeServiceID := range tfeServiceIDs { + service, err := host.ServiceURL(tfeServiceID) + if _, ok := err.(*disco.ErrVersionNotSupported); !ok && err != nil { + return nil, err + } + // If discoErr is nil we save the first error. When multiple services + // are checked and we found one that didn't give an error we need to + // reset the discoErr. So if err is nil, we assign it as well. + if discoErr == nil || err == nil { + discoErr = err + } + if service != nil { + address = service + break + } + } + + if providerVersion.ProviderVersion != "dev" { + // We purposefully ignore the error and return the previous error, as + // checking for version constraints is considered optional. + constraints, _ := host.VersionConstraints(tfeServiceIDs[0], "tfe-provider") + + // First check any constraints we might have received. + if constraints != nil { + if err := checkConstraints(constraints); err != nil { + return nil, err + } + } + } + + // When we don't have any constraints errors, also check for discovery + // errors before we continue. + if discoErr != nil { + return nil, discoErr + } + + // If a token wasn't set in the provider configuration block, try and fetch it + // from the environment or from Terraform's CLI configuration or configured credential helper. + if token == "" { + if os.Getenv("TFE_TOKEN") != "" { + token = getTokenFromEnv() + } else { + token = getTokenFromCreds(services, hostname) + } + } + + // If we still don't have a token at this point, we return an error. + if token == "" { + return nil, errMissingAuthToken + } + + // Wrap the configured transport to enable logging. + httpClient.Transport = NewLoggingTransport("TFE", transport) + + // Create a new TFE client config + cfg := &tfe.Config{ + Address: address.String(), + Token: token, + HTTPClient: httpClient, + } + + // Create a new TFE client. + client, err := tfe.NewClient(cfg) + if err != nil { + return nil, err + } + + client.RetryServerErrors(true) + return client, nil +} + +// cliConfig tries to find and parse the configuration of the Terraform CLI. +// This is an optional step, so any errors are ignored. +func cliConfig() Config { + mainConfig := Config{} + credentialsConfig := Config{} + combinedConfig := Config{} + + // Main CLI config file; might contain manually-entered credentials, and/or + // some host service discovery objects. Location is configurable via + // environment variables. + configFilePath := locateConfigFile() + if configFilePath != "" { + mainConfig = readCliConfigFile(configFilePath) + } + + // Credentials file; might contain credentials auto-configured by terraform + // login. Location isn't configurable. + credentialsFilePath, err := credentialsFile() + if err != nil { + log.Printf("[ERROR] Error detecting default credentials file path: %s", err) + } else { + credentialsConfig = readCliConfigFile(credentialsFilePath) + } + + // Use host service discovery configs from main config file. + combinedConfig.Hosts = mainConfig.Hosts + + // Combine both sets of credentials. Per Terraform's own behavior, the main + // config file overrides the credentials file if they have any overlapping + // hostnames. + combinedConfig.Credentials = credentialsConfig.Credentials + if combinedConfig.Credentials == nil { + combinedConfig.Credentials = make(map[string]map[string]interface{}) + } + for host, creds := range mainConfig.Credentials { + combinedConfig.Credentials[host] = creds + } + + return combinedConfig +} + +func locateConfigFile() string { + // To find the main CLI config file, follow Terraform's own logic: try + // TF_CLI_CONFIG_FILE, then try TERRAFORM_CONFIG, then try the default + // location. + + if os.Getenv("TF_CLI_CONFIG_FILE") != "" { + return os.Getenv("TF_CLI_CONFIG_FILE") + } + + if os.Getenv("TERRAFORM_CONFIG") != "" { + return os.Getenv("TERRAFORM_CONFIG") + } + filePath, err := configFile() + if err != nil { + log.Printf("[ERROR] Error detecting default CLI config file path: %s", err) + return "" + } + + return filePath +} + +func readCliConfigFile(configFilePath string) Config { + config := Config{} + + // Read the CLI config file content. + content, err := os.ReadFile(configFilePath) + if err != nil { + log.Printf("[ERROR] Error reading CLI config or credentials file %s: %v", configFilePath, err) + return config + } + + // Parse the CLI config file content. + obj, err := hcl.Parse(string(content)) + if err != nil { + log.Printf("[ERROR] Error parsing CLI config or credentials file %s: %v", configFilePath, err) + return config + } + + // Decode the CLI config file content. + if err := hcl.DecodeObject(&config, obj); err != nil { + log.Printf("[ERROR] Error decoding CLI config or credentials file %s: %v", configFilePath, err) + } + + return config +} + +func credentialsSource(config Config) auth.CredentialsSource { + creds := auth.NoCredentials + + // Add all configured credentials to the credentials source. + if len(config.Credentials) > 0 { + staticTable := map[svchost.Hostname]map[string]interface{}{} + for userHost, creds := range config.Credentials { + host, err := svchost.ForComparison(userHost) + if err != nil { + // We expect the config was already validated by the time we get + // here, so we'll just ignore invalid hostnames. + continue + } + staticTable[host] = creds + } + creds = auth.StaticCredentialsSource(staticTable) + } + + return creds +} + +// checkConstraints checks service version constrains against our own +// version and returns rich and informational diagnostics in case any +// incompatibilities are detected. +func checkConstraints(c *disco.Constraints) error { + if c == nil || c.Minimum == "" || c.Maximum == "" { + return nil + } + + // Generate a parsable constraints string. + excluding := "" + if len(c.Excluding) > 0 { + excluding = fmt.Sprintf(", != %s", strings.Join(c.Excluding, ", != ")) + } + constStr := fmt.Sprintf(">= %s%s, <= %s", c.Minimum, excluding, c.Maximum) + + // Create the constraints to check against. + constraints, err := version.NewConstraint(constStr) + if err != nil { + return checkConstraintsWarning(err) + } + + // Create the version to check. + v, err := version.NewVersion(providerVersion.ProviderVersion) + if err != nil { + return checkConstraintsWarning(err) + } + + // Return if we satisfy all constraints. + if constraints.Check(v) { + return nil + } + + // Find out what action (upgrade/downgrade) we should advice. + minimum, err := version.NewVersion(c.Minimum) + if err != nil { + return checkConstraintsWarning(err) + } + + maximum, err := version.NewVersion(c.Maximum) + if err != nil { + return checkConstraintsWarning(err) + } + + var excludes []*version.Version + for _, exclude := range c.Excluding { + v, err := version.NewVersion(exclude) + if err != nil { + return checkConstraintsWarning(err) + } + excludes = append(excludes, v) + } + + // Sort all the excludes. + sort.Sort(version.Collection(excludes)) + + var action, toVersion string + switch { + case minimum.GreaterThan(v): + action = "upgrade" + toVersion = ">= " + minimum.String() + case maximum.LessThan(v): + action = "downgrade" + toVersion = "<= " + maximum.String() + case len(excludes) > 0: + // Get the latest excluded version. + action = "upgrade" + toVersion = "> " + excludes[len(excludes)-1].String() + } + + switch { + case len(excludes) == 1: + excluding = fmt.Sprintf(", excluding version %s", excludes[0].String()) + case len(excludes) > 1: + var vs []string + for _, v := range excludes { + vs = append(vs, v.String()) + } + excluding = fmt.Sprintf(", excluding versions %s", strings.Join(vs, ", ")) + default: + excluding = "" + } + + summary := fmt.Sprintf("Incompatible TFE provider version v%s", v.String()) + details := fmt.Sprintf( + "The configured Terraform Enterprise backend is compatible with TFE provider\n"+ + "versions >= %s, <= %s%s.", c.Minimum, c.Maximum, excluding, + ) + + if action != "" && toVersion != "" { + summary = fmt.Sprintf("Please %s the TFE provider to %s", action, toVersion) + } + + // Return the customized and informational error message. + return fmt.Errorf("%s\n\n%s", summary, details) +} + +func checkConstraintsWarning(err error) error { + return fmt.Errorf( + "failed to check version constraints: %v\n\n"+ + "checking version constraints is considered optional, but this is an\n"+ + "unexpected error which should be reported", + err, + ) } var descriptions = map[string]string{ diff --git a/internal/provider/provider_next.go b/internal/provider/provider_next.go index 6e42e3b22..4da56ff15 100644 --- a/internal/provider/provider_next.go +++ b/internal/provider/provider_next.go @@ -12,7 +12,6 @@ import ( "github.com/hashicorp/terraform-plugin-framework/provider/schema" "github.com/hashicorp/terraform-plugin-framework/resource" "github.com/hashicorp/terraform-plugin-framework/types" - "github.com/hashicorp/terraform-provider-tfe/internal/client" ) // frameworkProvider is a type that implements the terraform-plugin-framework @@ -95,7 +94,7 @@ func (p *frameworkProvider) Configure(ctx context.Context, req provider.Configur data.Organization = types.StringValue(os.Getenv("TFE_ORGANIZATION")) } - client, err := client.GetClient(data.Hostname.ValueString(), data.Token.ValueString(), data.SSLSkipVerify.ValueBool()) + client, err := getClient(data.Hostname.ValueString(), data.Token.ValueString(), data.SSLSkipVerify.ValueBool()) if err != nil { res.Diagnostics.AddError("Failed to initialize HTTP client", err.Error()) diff --git a/internal/provider/provider_test.go b/internal/provider/provider_test.go index f1e2af621..93e0444de 100644 --- a/internal/provider/provider_test.go +++ b/internal/provider/provider_test.go @@ -19,7 +19,6 @@ import ( "github.com/hashicorp/terraform-plugin-sdk/v2/diag" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/hashicorp/terraform-plugin-sdk/v2/terraform" - "github.com/hashicorp/terraform-provider-tfe/internal/client" "github.com/hashicorp/terraform-provider-tfe/version" "github.com/hashicorp/terraform-svchost/disco" ) @@ -84,13 +83,13 @@ func setupDefaultOrganization(t *testing.T) (string, int) { } func getClientUsingEnv() (*tfe.Client, error) { - hostname := client.DefaultHostname + hostname := defaultHostname if os.Getenv("TFE_HOSTNAME") != "" { hostname = os.Getenv("TFE_HOSTNAME") } token := os.Getenv("TFE_TOKEN") - client, err := client.GetClient(hostname, token, defaultSSLSkipVerify) + client, err := getClient(hostname, token, defaultSSLSkipVerify) if err != nil { return nil, fmt.Errorf("Error getting client: %w", err) } @@ -154,7 +153,7 @@ func TestProvider_versionConstraints(t *testing.T) { // Set the version for this test. version.ProviderVersion = tc.version - err := client.CheckConstraints(tc.constraints) + err := checkConstraints(tc.constraints) if err == nil && tc.result != "" { t.Fatalf("%s: expected error to contain %q, but got no error", name, tc.result) } @@ -167,6 +166,155 @@ func TestProvider_versionConstraints(t *testing.T) { } } +func TestProvider_locateConfigFile(t *testing.T) { + originalHome := os.Getenv("HOME") + originalTfCliConfigFile := os.Getenv("TF_CLI_CONFIG_FILE") + originalTerraformConfig := os.Getenv("TERRAFORM_CONFIG") + reset := func() { + os.Setenv("HOME", originalHome) + if originalTfCliConfigFile != "" { + os.Setenv("TF_CLI_CONFIG_FILE", originalTfCliConfigFile) + } else { + os.Unsetenv("TF_CLI_CONFIG_FILE") + } + if originalTerraformConfig != "" { + os.Setenv("TERRAFORM_CONFIG", originalTerraformConfig) + } else { + os.Unsetenv("TERRAFORM_CONFIG") + } + } + defer reset() + + // Use a predictable value for $HOME + os.Setenv("HOME", "/Users/someone") + + setup := func(tfCliConfigFile, terraformConfig string) { + os.Setenv("TF_CLI_CONFIG_FILE", tfCliConfigFile) + os.Setenv("TERRAFORM_CONFIG", terraformConfig) + } + + cases := map[string]struct { + tfCliConfigFile string + terraformConfig string + result string + }{ + "has TF_CLI_CONFIG_FILE": { + tfCliConfigFile: "~/.terraform_alternate/terraformrc", + terraformConfig: "", + result: "~/.terraform_alternate/terraformrc", + }, + "has TERRAFORM_CONFIG": { + tfCliConfigFile: "", + terraformConfig: "~/.terraform_alternate_rc", + result: "~/.terraform_alternate_rc", + }, + "has both env vars": { + tfCliConfigFile: "~/.from_TF_CLI", + terraformConfig: "~/.from_TERRAFORM_CONFIG", + result: "~/.from_TF_CLI", + }, + "has neither env var": { + tfCliConfigFile: "", + terraformConfig: "", + result: "/Users/someone/.terraformrc", // expect tests run on unix + }, + } + + for name, tc := range cases { + setup(tc.tfCliConfigFile, tc.terraformConfig) + + fileResult := locateConfigFile() + if tc.result != fileResult { + t.Fatalf("%s: expected config file at %s, got %s", name, tc.result, fileResult) + } + } +} + +func TestProvider_cliConfig(t *testing.T) { + // This only tests the behavior of merging various combinations of + // (credentials file, .terraformrc file, absent). Locating the .terraformrc + // file is tested separately. + originalHome := os.Getenv("HOME") + originalTfCliConfigFile := os.Getenv("TF_CLI_CONFIG_FILE") + reset := func() { + os.Setenv("HOME", originalHome) + if originalTfCliConfigFile != "" { + os.Setenv("TF_CLI_CONFIG_FILE", originalTfCliConfigFile) + } else { + os.Unsetenv("TF_CLI_CONFIG_FILE") + } + } + defer reset() + + // Summary of fixtures: the credentials file and terraformrc file each have + // credentials for two hosts, they both have credentials for app.terraform.io, + // and the terraformrc also has one service discovery override. + hasCredentials := "test-fixtures/cli-config-files/home" + noCredentials := "test-fixtures/cli-config-files/no-credentials" + terraformrc := "test-fixtures/cli-config-files/terraformrc" + noTerraformrc := "test-fixtures/cli-config-files/no-terraformrc" + tokenFromTerraformrc := "something.atlasv1.prod_rc_file" + tokenFromCredentials := "something.atlasv1.prod_credentials_file" + + cases := map[string]struct { + home string + rcfile string + expectCount int + expectProdToken string + expectHostsCount int + }{ + "both main config and credentials JSON": { + home: hasCredentials, + rcfile: terraformrc, + expectCount: 3, + expectProdToken: tokenFromTerraformrc, + expectHostsCount: 1, + }, + "only main config": { + home: noCredentials, + rcfile: terraformrc, + expectCount: 2, + expectProdToken: tokenFromTerraformrc, + expectHostsCount: 1, + }, + "only credentials JSON": { + home: hasCredentials, + rcfile: noTerraformrc, + expectCount: 2, + expectProdToken: tokenFromCredentials, + expectHostsCount: 0, + }, + "neither file": { + home: noCredentials, + rcfile: noTerraformrc, + expectCount: 0, + expectProdToken: "", + expectHostsCount: 0, + }, + } + + for name, tc := range cases { + os.Setenv("HOME", tc.home) + os.Setenv("TF_CLI_CONFIG_FILE", tc.rcfile) + config := cliConfig() + credentialsCount := len(config.Credentials) + if credentialsCount != tc.expectCount { + t.Fatalf("%s: expected %d credentials, got %d", name, tc.expectCount, credentialsCount) + } + prodToken := "" + if config.Credentials["app.terraform.io"] != nil { + prodToken = config.Credentials["app.terraform.io"]["token"].(string) + } + if prodToken != tc.expectProdToken { + t.Fatalf("%s: expected %s as prod token, got %s", name, tc.expectProdToken, prodToken) + } + hostsCount := len(config.Hosts) + if hostsCount != tc.expectHostsCount { + t.Fatalf("%s: expected %d `host` blocks in the final config, got %d", name, tc.expectHostsCount, hostsCount) + } + } +} + func testAccPreCheck(t *testing.T) { // The credentials must be provided by the CLI config file for testing. if diags := Provider().Configure(context.Background(), &terraform.ResourceConfig{}); diags.HasError() { From 006b4e30031ff99f8c5b06667bd27201b5481f3a Mon Sep 17 00:00:00 2001 From: Brandon Croft Date: Mon, 2 Oct 2023 17:27:24 -0600 Subject: [PATCH 2/2] Update CHANGELOG and bump release versions --- CHANGELOG.md | 5 +++++ README.md | 2 +- website/docs/index.html.markdown | 4 ++-- 3 files changed, 8 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 212cd3a02..6273c5d8d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,8 @@ +## v0.49.1 (October 2, 2023) + +BUG FIXES: +* Reverts previous bug fix for provider making two service discovery requests per provider config. This was an internal refactor that broke some token configuration, by @brandonc [1085](https://github.com/hashicorp/terraform-provider-tfe/pull/1085)" + ## v0.49.0 (October 2, 2023) BUG FIXES: diff --git a/README.md b/README.md index 9190b7629..e981ced19 100644 --- a/README.md +++ b/README.md @@ -29,7 +29,7 @@ Declare the provider in your configuration and `terraform init` will automatical terraform { required_providers { tfe = { - version = "~> 0.49.0" + version = "~> 0.49.1" } } } diff --git a/website/docs/index.html.markdown b/website/docs/index.html.markdown index 05777efd7..bf86b92f5 100644 --- a/website/docs/index.html.markdown +++ b/website/docs/index.html.markdown @@ -75,7 +75,7 @@ automatically installed by `terraform init` in the future: terraform { required_providers { tfe = { - version = "~> 0.49.0" + version = "~> 0.49.1" } } } @@ -88,7 +88,7 @@ The above snippet using `required_providers` is for Terraform 0.13+; if you are ```hcl provider "tfe" { - version = "~> 0.49.0" + version = "~> 0.49.1" ... } ```