Refactor based on review comments

Author: Christian Doucette

internal/client/client.go

@@ -74,17 +74,35 @@ func getTokenFromCreds(services *disco.Disco, hostname svchost.Hostname) string
 	return ""
 }
 
+// TFE Client along with other necessary information for the provider to run it
+type ProviderClient struct {
+	TfeClient   *tfe.Client
+	tokenSource TokenSource
+}
+
+// Using presence of TFC_AGENT_VERSION to determine if this provider is running on HCP Terraform / enterprise
+func providerRunningInCloud() bool {
+	_, envVariablePresent := os.LookupEnv("TFC_AGENT_VERSION")
+	return envVariablePresent
+}
+
+func (pc *ProviderClient) SendAuthenticationWarning() bool {
+	return pc.tokenSource == CredentialFiles && providerRunningInCloud()
+
+}
+
 // 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, bool, error) {
-	config, sendCredentialDeprecationWarning, err := configure(tfeHost, token, insecure)
+func GetClient(tfeHost, token string, insecure bool) (*ProviderClient, error) {
+	config, err := configure(tfeHost, token, insecure)
+
 	if err != nil {
-		return nil, sendCredentialDeprecationWarning, err
+		return nil, err
 	}
 
 	clientCache.Lock()
@@ -93,13 +111,13 @@ func GetClient(tfeHost, token string, insecure bool) (*tfe.Client, bool, error)
 	// Try to retrieve the client from cache
 	cached := clientCache.GetByConfig(config)
 	if cached != nil {
-		return cached, sendCredentialDeprecationWarning, nil
+		return &ProviderClient{cached, config.TokenSource}, nil
 	}
 
 	// Discover the Terraform Enterprise address.
 	host, err := config.Services.Discover(config.TFEHost)
 	if err != nil {
-		return nil, sendCredentialDeprecationWarning, fmt.Errorf("failed to create client: %w", err)
+		return nil, fmt.Errorf("failed to create client: %w", err)
 	}
 
 	// Get the full Terraform Enterprise service address.
@@ -109,7 +127,7 @@ func GetClient(tfeHost, token string, insecure bool) (*tfe.Client, bool, error)
 		service, err := host.ServiceURL(tfeServiceID)
 		target := &disco.ErrVersionNotSupported{}
 		if err != nil && !errors.As(err, &target) {
-			return nil, sendCredentialDeprecationWarning, fmt.Errorf("failed to create client: %w", err)
+			return nil, fmt.Errorf("failed to create client: %w", err)
 		}
 
 		// If discoErr is nil we save the first error. When multiple services
@@ -133,15 +151,15 @@ func GetClient(tfeHost, token string, insecure bool) (*tfe.Client, bool, error)
 		// First check any constraints we might have received.
 		if constraints != nil {
 			if err := CheckConstraints(constraints); err != nil {
-				return nil, sendCredentialDeprecationWarning, err
+				return nil, err
 			}
 		}
 	}
 
 	// When we don't have any constraints errors, also check for discovery
 	// errors before we continue.
 	if discoErr != nil {
-		return nil, sendCredentialDeprecationWarning, discoErr
+		return nil, discoErr
 	}
 
 	// Create a new TFE client.
@@ -151,13 +169,13 @@ func GetClient(tfeHost, token string, insecure bool) (*tfe.Client, bool, error)
 		HTTPClient: config.HTTPClient,
 	})
 	if err != nil {
-		return nil, sendCredentialDeprecationWarning, fmt.Errorf("failed to create client: %w", err)
+		return nil, fmt.Errorf("failed to create client: %w", err)
 	}
 
 	client.RetryServerErrors(true)
 	clientCache.Set(client, config)
 
-	return client, sendCredentialDeprecationWarning, nil
+	return &ProviderClient{client, config.TokenSource}, nil
 }
 
 // CheckConstraints checks service version constrains against our own

internal/client/config.go

@@ -41,13 +41,22 @@ type ConfigHost struct {
 	Services map[string]interface{} `hcl:"services"`
 }
 
-// ClientConfiguration is the refined information needed to configure a tfe.Client
+type TokenSource int
+
+const (
+	ProviderArgument TokenSource = iota
+	EnvironmentVariable
+	CredentialFiles
+)
+
+// ClientConfiguration is the refined information needed to configureClient a tfe.Client
 type ClientConfiguration struct {
-	Services   *disco.Disco
-	HTTPClient *http.Client
-	TFEHost    svchost.Hostname
-	Token      string
-	Insecure   bool
+	Services    *disco.Disco
+	HTTPClient  *http.Client
+	TFEHost     svchost.Hostname
+	Token       string
+	TokenSource TokenSource
+	Insecure    bool
 }
 
 // Key returns a string that is comparable to other ClientConfiguration values
@@ -165,17 +174,9 @@ func credentialsSource(credentials CredentialsMap) auth.CredentialsSource {
 	return creds
 }
 
-// Using presence of TFC_AGENT_VERSION to determine if this provider is running on HCP Terraform / enterprise
-func providerRunningInCloud() bool {
-	_, present := os.LookupEnv("TFC_AGENT_VERSION")
-	return present
-}
-
 // 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, bool, error) {
-	sendCredentialDeprecationWarning := false
-
+func configure(tfeHost, token string, insecure bool) (*ClientConfiguration, error) {
 	if tfeHost == "" {
 		if os.Getenv("TFE_HOSTNAME") != "" {
 			tfeHost = os.Getenv("TFE_HOSTNAME")
@@ -194,7 +195,7 @@ func configure(tfeHost, token string, insecure bool) (*ClientConfiguration, bool
 		v := os.Getenv("TFE_SSL_SKIP_VERIFY")
 		insecure, err = strconv.ParseBool(v)
 		if err != nil {
-			return nil, sendCredentialDeprecationWarning, fmt.Errorf("TFE_SSL_SKIP_VERIFY has unrecognized value %q", v)
+			return nil, fmt.Errorf("TFE_SSL_SKIP_VERIFY has unrecognized value %q", v)
 		}
 	}
 
@@ -206,7 +207,7 @@ func configure(tfeHost, token string, insecure bool) (*ClientConfiguration, bool
 	// Parse the hostname for comparison,
 	hostname, err := svchost.ForComparison(tfeHost)
 	if err != nil {
-		return nil, sendCredentialDeprecationWarning, fmt.Errorf("invalid hostname %q: %w", tfeHost, err)
+		return nil, fmt.Errorf("invalid hostname %q: %w", tfeHost, err)
 	}
 
 	httpClient := tfe.DefaultConfig().HTTPClient
@@ -241,25 +242,28 @@ func configure(tfeHost, token string, insecure bool) (*ClientConfiguration, bool
 	// 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.
 
+	tokenSource := ProviderArgument
 	if token == "" {
 		if os.Getenv("TFE_TOKEN") != "" {
 			token = getTokenFromEnv()
+			tokenSource = EnvironmentVariable
 		} else {
-			sendCredentialDeprecationWarning = providerRunningInCloud()
 			token = getTokenFromCreds(services, hostname)
+			tokenSource = CredentialFiles
 		}
 	}
 
 	// If we still don't have a token at this point, we return an error.
 	if token == "" {
-		return nil, sendCredentialDeprecationWarning, ErrMissingAuthToken
+		return nil, ErrMissingAuthToken
 	}
 
 	return &ClientConfiguration{
-		Services:   services,
-		HTTPClient: httpClient,
-		TFEHost:    hostname,
-		Token:      token,
-		Insecure:   insecure,
-	}, sendCredentialDeprecationWarning, nil
+		Services:    services,
+		HTTPClient:  httpClient,
+		TFEHost:     hostname,
+		Token:       token,
+		TokenSource: tokenSource,
+		Insecure:    insecure,
+	}, nil
 }

internal/provider/provider.go

@@ -153,30 +153,31 @@ func configure() schema.ConfigureContextFunc {
 			providerOrganization = os.Getenv("TFE_ORGANIZATION")
 		}
 
-		tfeClient, sendCredentialDeprecationWarning, err := configureClient(rd)
+		providerClient, err := configureClient(rd)
 		if err != nil {
 			return nil, diag.FromErr(err)
 		}
 
 		var diagnosticWarnings diag.Diagnostics = nil
-		if sendCredentialDeprecationWarning {
+
+		if providerClient.SendAuthenticationWarning() {
 			diagnosticWarnings = diag.Diagnostics{
 				diag.Diagnostic{
 					Severity: diag.Warning,
-					Summary:  "Authentication method invalid for TFE Provider with HCP Terraform and Terraform Enterprise",
+					Summary:  "Authentication with configuration file is invalid for TFE Provider running on HCP Terraform or Terraform Enterprise",
 					Detail:   "Use a TFE_TOKEN variable in the workspace or the token argument for the provider. This authentication method will be deprecated in a future version.",
 				},
 			}
 		}
 
 		return ConfiguredClient{
-			tfeClient,
+			providerClient.TfeClient,
 			providerOrganization,
 		}, diagnosticWarnings
 	}
 }
 
-func configureClient(d *schema.ResourceData) (*tfe.Client, bool, error) {
+func configureClient(d *schema.ResourceData) (*client.ProviderClient, error) {
 	hostname := d.Get("hostname").(string)
 	token := d.Get("token").(string)
 	insecure := d.Get("ssl_skip_verify").(bool)

internal/provider/provider_next.go

@@ -111,19 +111,19 @@ func (p *frameworkProvider) Configure(ctx context.Context, req provider.Configur
 		}
 	}
 
-	tfeClient, sendCredentialDeprecationWarning, err := client.GetClient(data.Hostname.ValueString(), data.Token.ValueString(), data.SSLSkipVerify.ValueBool())
+	providerClient, err := client.GetClient(data.Hostname.ValueString(), data.Token.ValueString(), data.SSLSkipVerify.ValueBool())
 
 	if err != nil {
 		res.Diagnostics.AddError("Failed to initialize HTTP client", err.Error())
 		return
 	}
 
-	if sendCredentialDeprecationWarning {
-		res.Diagnostics.AddWarning("Authentication method invalid for TFE Provider with HCP Terraform and Terraform Enterprise", "Use a TFE_TOKEN variable in the workspace or the token argument for the provider. This authentication method will be deprecated in a future version.")
+	if providerClient.SendAuthenticationWarning() {
+		res.Diagnostics.AddWarning("Authentication with configuration files is invalid for TFE Provider running on HCP Terraform or Terraform Enterprise", "Use a TFE_TOKEN variable in the workspace or the token argument for the provider. This authentication method will be deprecated in a future version.")
 	}
 
 	configuredClient := ConfiguredClient{
-		Client:       tfeClient,
+		Client:       providerClient.TfeClient,
 		Organization: data.Organization.ValueString(),
 	}