diff --git a/internal/client/client.go b/internal/client/client.go index 24164d960..ad83802f3 100644 --- a/internal/client/client.go +++ b/internal/client/client.go @@ -74,6 +74,22 @@ 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 @@ -81,7 +97,7 @@ func getTokenFromCreds(services *disco.Disco, hostname svchost.Hostname) string // // Internally, this function caches configured clients using the specified // parameters -func GetClient(tfeHost, token string, insecure bool) (*tfe.Client, error) { +func GetClient(tfeHost, token string, insecure bool) (*ProviderClient, error) { config, err := configure(tfeHost, token, insecure) if err != nil { return nil, err @@ -93,7 +109,7 @@ func GetClient(tfeHost, token string, insecure bool) (*tfe.Client, error) { // Try to retrieve the client from cache cached := clientCache.GetByConfig(config) if cached != nil { - return cached, nil + return &ProviderClient{cached, config.tokenSource}, nil } // Discover the Terraform Enterprise address. @@ -157,7 +173,7 @@ func GetClient(tfeHost, token string, insecure bool) (*tfe.Client, error) { client.RetryServerErrors(true) clientCache.Set(client, config) - return client, nil + return &ProviderClient{client, config.tokenSource}, nil } // CheckConstraints checks service version constrains against our own diff --git a/internal/client/client_test.go b/internal/client/client_test.go index f75b1a22c..949c274ac 100644 --- a/internal/client/client_test.go +++ b/internal/client/client_test.go @@ -94,27 +94,31 @@ credentials "%s" { hostname string token string expectMissingAuth bool + expectTokenSource tokenSource }{ "everything from env": { env: map[string]string{ "TFE_HOSTNAME": serverURL.Host, "TFE_TOKEN": testToken, }, + expectTokenSource: environmentVariable, }, "token from env": { env: map[string]string{ "TFE_HOSTNAME": serverURL.Host, "TFE_TOKEN": "", }, - token: testToken, + token: testToken, + expectTokenSource: providerArgument, }, "everything from provider config": { env: map[string]string{ "TFE_HOSTNAME": "", "TFE_TOKEN": "", }, - hostname: serverURL.Host, - token: testToken, + hostname: serverURL.Host, + token: testToken, + expectTokenSource: providerArgument, }, "token missing": { env: map[string]string{ @@ -129,7 +133,8 @@ credentials "%s" { "TFE_TOKEN": "", "TF_CLI_CONFIG_FILE": cliConfig.Name(), }, - hostname: serverURL.Host, + hostname: serverURL.Host, + expectTokenSource: credentialFiles, }, } @@ -138,7 +143,8 @@ credentials "%s" { t.Setenv(k, v) } // Must always skip SSL verification for this test server - client, err := GetClient(c.hostname, c.token, true) + providerClient, err := GetClient(c.hostname, c.token, true) + if c.expectMissingAuth { if !errors.Is(err, ErrMissingAuthToken) { t.Errorf("Expected ErrMissingAuthToken, got %v", err) @@ -150,13 +156,73 @@ credentials "%s" { t.Errorf("Unexpected error when getting client: %q", err) } + client := providerClient.TfeClient if client == nil { t.Fatal("Unexpected client was nil") } + tokenSource := providerClient.tokenSource + if tokenSource != c.expectTokenSource { + t.Fatalf("Expected token source %d, got %d", c.expectTokenSource, tokenSource) + } + _, err = client.Organizations.List(context.Background(), &tfe.OrganizationListOptions{}) if err != nil { t.Errorf("Unexpected error from using client: %q", err) } } } + +func TestClient_sendAuthenticationWarning(t *testing.T) { + // This tests that the SendAuthenticationWarning function returns true when the + // token source is credentialFiles and the TFE_AGENT_VERSION env var is set + origTfcAgentVersion := os.Getenv("TFC_AGENT_VERSION") + reset := func() { + if origTfcAgentVersion != "" { + os.Setenv("TFC_AGENT_VERSION", origTfcAgentVersion) + } else { + os.Unsetenv("TFC_AGENT_VERSION") + } + } + defer reset() + + cases := map[string]struct { + tokenSource tokenSource + tfcAgentVersionEnvVariableSet bool + expectResult bool + }{ + "token from credentials files and TFC_AGENT_VERSION is set": { + tokenSource: credentialFiles, + tfcAgentVersionEnvVariableSet: true, + expectResult: true, + }, + "token from credentials files but TFC_AGENT_VERSION not set": { + tokenSource: credentialFiles, + tfcAgentVersionEnvVariableSet: false, + expectResult: false, + }, + "TFC_AGENT_VERSION is set but token not from credentials files": { + tokenSource: providerArgument, + tfcAgentVersionEnvVariableSet: true, + expectResult: false, + }, + } + + for name, tc := range cases { + if tc.tfcAgentVersionEnvVariableSet { + os.Setenv("TFC_AGENT_VERSION", "1.0") + } else { + os.Unsetenv("TFC_AGENT_VERSION") + } + + providerClient := ProviderClient{ + TfeClient: nil, + tokenSource: tc.tokenSource, + } + + result := providerClient.SendAuthenticationWarning() + if result != tc.expectResult { + t.Fatalf("%s: SendAuthenticationWarning() expected result: %t, got %t", name, tc.expectResult, result) + } + } +} diff --git a/internal/client/config.go b/internal/client/config.go index 22fe4d3c8..78f7512b5 100644 --- a/internal/client/config.go +++ b/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 @@ -232,11 +241,15 @@ func configure(tfeHost, token string, insecure bool) (*ClientConfiguration, erro // 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 { token = getTokenFromCreds(services, hostname) + tokenSource = credentialFiles } } @@ -246,10 +259,11 @@ func configure(tfeHost, token string, insecure bool) (*ClientConfiguration, erro } return &ClientConfiguration{ - Services: services, - HTTPClient: httpClient, - TFEHost: hostname, - Token: token, - Insecure: insecure, + Services: services, + HTTPClient: httpClient, + TFEHost: hostname, + Token: token, + tokenSource: tokenSource, + Insecure: insecure, }, nil } diff --git a/internal/client/config_test.go b/internal/client/config_test.go index 93bef1090..63f509555 100644 --- a/internal/client/config_test.go +++ b/internal/client/config_test.go @@ -156,3 +156,52 @@ func TestConfig_cliConfig(t *testing.T) { } } } + +func TestConfig_configureSetsTokenSource(t *testing.T) { + // This tests that the tokenSource is set properly when calling the configure function + reset := func() { + os.Unsetenv("TFE_TOKEN") + } + defer reset() + + cases := map[string]struct { + tokenProviderArgument string + tokenEnvVariable string + + expectTokenSource tokenSource + }{ + "has token from provider argument": { + tokenProviderArgument: "testtoken", + tokenEnvVariable: "", + expectTokenSource: providerArgument, + }, + "has token from environment variable": { + tokenProviderArgument: "", + tokenEnvVariable: "testtoken", + expectTokenSource: environmentVariable, + }, + "has token from both provider argument and environment variable": { + tokenProviderArgument: "testtoken", + tokenEnvVariable: "testtoken", + expectTokenSource: providerArgument, + }, + "has token from neither provider argument and environment variable": { + tokenProviderArgument: "", + tokenEnvVariable: "", + expectTokenSource: credentialFiles, + }, + } + + for name, tc := range cases { + os.Setenv("TFE_TOKEN", tc.tokenEnvVariable) + + config, err := configure("app.terraform.io", tc.tokenProviderArgument, true) + + if err != nil { + t.Fatalf("%s: received error while configuring client: %s", name, err.Error()) + } + if config.tokenSource != tc.expectTokenSource { + t.Fatalf("%s: expected token source %d, got %d", name, tc.expectTokenSource, config.tokenSource) + } + } +} diff --git a/internal/provider/provider.go b/internal/provider/provider.go index 7160567a4..241000963 100644 --- a/internal/provider/provider.go +++ b/internal/provider/provider.go @@ -153,19 +153,31 @@ func configure() schema.ConfigureContextFunc { providerOrganization = os.Getenv("TFE_ORGANIZATION") } - tfeClient, err := configureClient(rd) + providerClient, err := configureClient(rd) if err != nil { return nil, diag.FromErr(err) } + var diagnosticWarnings diag.Diagnostics = nil + + if providerClient.SendAuthenticationWarning() { + diagnosticWarnings = diag.Diagnostics{ + diag.Diagnostic{ + Severity: diag.Warning, + Summary: "Authentication with configuration files 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, - }, nil + }, diagnosticWarnings } } -func configureClient(d *schema.ResourceData) (*tfe.Client, 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) diff --git a/internal/provider/provider_next.go b/internal/provider/provider_next.go index f04cce9e5..0f5447141 100644 --- a/internal/provider/provider_next.go +++ b/internal/provider/provider_next.go @@ -111,15 +111,19 @@ func (p *frameworkProvider) Configure(ctx context.Context, req provider.Configur } } - tfeClient, 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 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(), } diff --git a/internal/provider/provider_test.go b/internal/provider/provider_test.go index 296b25131..223165e83 100644 --- a/internal/provider/provider_test.go +++ b/internal/provider/provider_test.go @@ -150,11 +150,11 @@ func getClientUsingEnv() (*tfe.Client, error) { } token := os.Getenv("TFE_TOKEN") - tfeClient, err := client.GetClient(hostname, token, defaultSSLSkipVerify) + providerClient, err := client.GetClient(hostname, token, defaultSSLSkipVerify) if err != nil { return nil, fmt.Errorf("Error getting client: %w", err) } - return tfeClient, nil + return providerClient.TfeClient, nil } func TestProvider(t *testing.T) { @@ -268,6 +268,62 @@ func TestConfigureEnvOrganization(t *testing.T) { } } +func TestConfigureEnvOnCloudUsingConfigFiles(t *testing.T) { + // tests that the provider sends a warning when running on cloud (checked using TFE_AGENT_VERSION) + // and using a token from configuration files + envToken := os.Getenv("TFE_TOKEN") + envHostname := os.Getenv("TFE_HOSTNAME") + origTfcAgentVersion := os.Getenv("TFC_AGENT_VERSION") + origTfCliConfigFile := os.Getenv("TF_CLI_CONFIG_FILE") + + reset := func() { + os.Setenv("TFE_TOKEN", envToken) + os.Setenv("TFE_HOSTNAME", envHostname) + + if origTfcAgentVersion != "" { + os.Setenv("TFC_AGENT_VERSION", origTfcAgentVersion) + } else { + os.Unsetenv("TFC_AGENT_VERSION") + } + + if origTfCliConfigFile != "" { + os.Setenv("TF_CLI_CONFIG_FILE", origTfCliConfigFile) + } else { + os.Unsetenv("TF_CLI_CONFIG_FILE") + } + } + defer reset() + + // temporarily removes TFE_TOKEN so token will be from configuration files + os.Unsetenv("TFE_TOKEN") + os.Setenv("TFC_AGENT_VERSION", "1.0") + os.Setenv("TFE_HOSTNAME", "app.terraform.io") + os.Setenv("TF_CLI_CONFIG_FILE", "test-fixtures/cli-config-files/terraformrc") + + provider := Provider() + diags := provider.Configure(context.Background(), &sdkTerraform.ResourceConfig{}) + + if len(diags) != 1 { + t.Fatalf("Expected 1 diagnostic, received %d", len(diags)) + } + expectedSeverity := diag.Warning + expectedSummary := "Authentication with configuration files is invalid for TFE Provider running on HCP Terraform or Terraform Enterprise" + expectedDetail := "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." + + onlyDiag := diags[0] + t.Logf("Want to see if this shows up in Datadog flaky test") + + if onlyDiag.Severity != expectedSeverity { + t.Fatalf("Expected Diagnostic to have Severity %d, got %d. Also got summary: %s. And detail: %s", expectedSeverity, onlyDiag.Severity, onlyDiag.Summary, onlyDiag.Detail) + } + if onlyDiag.Summary != expectedSummary { + t.Fatalf("Expected Diagnostic to have Summary %s, got %s. Also got detail %s", expectedSummary, onlyDiag.Summary, onlyDiag.Detail) + } + if onlyDiag.Detail != expectedDetail { + t.Fatalf("Expected Diagnostic to have Detail %s, got %s. Also got summary %s.", expectedDetail, onlyDiag.Detail, onlyDiag.Summary) + } +} + // The TFE Provider tests use these environment variables, which are set in the // GitHub Action workflow file .github/workflows/ci.yml. func testAccGithubPreCheck(t *testing.T) {