From 696dabbc943b9199f7b9f413d86005f502e00f75 Mon Sep 17 00:00:00 2001 From: Sarah French <15078782+SarahFrench@users.noreply.github.com> Date: Tue, 1 Oct 2024 10:25:15 +0100 Subject: [PATCH] Add acceptance tests for how provider handles `request_reason` argument (#11835) --- ...source_provider_config_plugin_framework.go | 2 +- .../framework_provider_request_reason_test.go | 144 ++++++++++++++++++ .../fwtransport/framework_config.go.tmpl | 3 + .../provider/provider_request_reason_test.go | 142 +++++++++++++++++ 4 files changed, 290 insertions(+), 1 deletion(-) create mode 100644 mmv1/third_party/terraform/fwprovider/framework_provider_request_reason_test.go create mode 100644 mmv1/third_party/terraform/provider/provider_request_reason_test.go diff --git a/mmv1/third_party/terraform/fwprovider/data_source_provider_config_plugin_framework.go b/mmv1/third_party/terraform/fwprovider/data_source_provider_config_plugin_framework.go index ab95da959c99..6d3b17785034 100644 --- a/mmv1/third_party/terraform/fwprovider/data_source_provider_config_plugin_framework.go +++ b/mmv1/third_party/terraform/fwprovider/data_source_provider_config_plugin_framework.go @@ -214,7 +214,7 @@ func (d *GoogleProviderConfigPluginFrameworkDataSource) Read(ctx context.Context data.UniverseDomain = d.providerConfig.UniverseDomain data.Scopes = d.providerConfig.Scopes data.UserProjectOverride = d.providerConfig.UserProjectOverride - // TODO(SarahFrench) - request_reason + data.RequestReason = d.providerConfig.RequestReason // TODO(SarahFrench) - request_timeout data.DefaultLabels = d.providerConfig.DefaultLabels // TODO(SarahFrench) - add_terraform_attribution_label diff --git a/mmv1/third_party/terraform/fwprovider/framework_provider_request_reason_test.go b/mmv1/third_party/terraform/fwprovider/framework_provider_request_reason_test.go new file mode 100644 index 000000000000..c2845d24b382 --- /dev/null +++ b/mmv1/third_party/terraform/fwprovider/framework_provider_request_reason_test.go @@ -0,0 +1,144 @@ +package fwprovider_test + +import ( + "testing" + + "github.com/hashicorp/terraform-plugin-testing/helper/resource" + "github.com/hashicorp/terraform-provider-google/google/acctest" +) + +// TestAccFwProvider_request_reason is a series of acc tests asserting how the SDK provider handles request_reason arguments +// It is SDK specific because the HCL used provisions SDK-implemented resources +// It is a counterpart to TestAccSdkProvider_request_reason +func TestAccFwProvider_request_reason(t *testing.T) { + testCases := map[string]func(t *testing.T){ + // Configuring the provider using inputs + "config takes precedence over environment variables": testAccFwProvider_request_reason_configPrecedenceOverEnvironmentVariables, + "when request_reason is unset in the config, environment variables are used in a given order": testAccFwProvider_request_reason_precedenceOrderEnvironmentVariables, // CLOUDSDK_CORE_REQUEST_REASON + + // Schema-level validation + // TODO: https://github.com/hashicorp/terraform-provider-google/issues/19643 + "when request_reason is set to an empty string in the config the value is not ignored, and there isn't any validation about this that raises an error": testAccFwProvider_request_reason_emptyStringValidation, + + // Usage + // We cannot test the impact of this field in an acc test, as it sets the X-Goog-Request-Reason value for audit logging purposes in GCP + // See: https://cloud.google.com/apis/docs/system-parameters#definitions + } + + for name, tc := range testCases { + // shadow the tc variable into scope so that when + // the loop continues, if t.Run hasn't executed tc(t) + // yet, we don't have a race condition + // see https://github.com/golang/go/wiki/CommonMistakes#using-goroutines-on-loop-iterator-variables + tc := tc + t.Run(name, func(t *testing.T) { + tc(t) + }) + } +} + +func testAccFwProvider_request_reason_configPrecedenceOverEnvironmentVariables(t *testing.T) { + acctest.SkipIfVcr(t) // Test doesn't interact with API + + envReason := "environment-variables" + + // ensure all possible request_reason env vars set; show they aren't used instead + t.Setenv("CLOUDSDK_CORE_REQUEST_REASON", envReason) + + providerReason := "provider-config" + + context := map[string]interface{}{ + "request_reason": providerReason, + } + + acctest.VcrTest(t, resource.TestCase{ + // No PreCheck for checking ENVs + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories(t), + Steps: []resource.TestStep{ + { + Config: testAccFwProvider_request_reason_inProviderBlock(context), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr("data.google_provider_config_plugin_framework.default", "request_reason", providerReason), + ), + }, + }, + }) +} + +func testAccFwProvider_request_reason_precedenceOrderEnvironmentVariables(t *testing.T) { + acctest.SkipIfVcr(t) // Test doesn't interact with API + /* + These are all the ENVs for request_reason + CLOUDSDK_CORE_REQUEST_REASON + */ + + CLOUDSDK_CORE_REQUEST_REASON := "CLOUDSDK_CORE_REQUEST_REASON" + + context := map[string]interface{}{} + + acctest.VcrTest(t, resource.TestCase{ + // No PreCheck for checking ENVs + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories(t), + Steps: []resource.TestStep{ + { + // CLOUDSDK_CORE_REQUEST_REASON is used if config doesn't provide a value + PreConfig: func() { + t.Setenv("CLOUDSDK_CORE_REQUEST_REASON", CLOUDSDK_CORE_REQUEST_REASON) //used + }, + Config: testAccFwProvider_request_reason_inEnvsOnly(context), + Check: resource.ComposeAggregateTestCheckFunc( + resource.TestCheckResourceAttr("data.google_provider_config_plugin_framework.default", "request_reason", CLOUDSDK_CORE_REQUEST_REASON), + ), + }, + }, + }) +} + +func testAccFwProvider_request_reason_emptyStringValidation(t *testing.T) { + acctest.SkipIfVcr(t) // Test doesn't interact with API + + envReason := "environment-variables" + + // ensure all request_reason env vars set + t.Setenv("CLOUDSDK_CORE_REQUEST_REASON", envReason) + + emptyString := "" + context := map[string]interface{}{ + "request_reason": emptyString, + } + + acctest.VcrTest(t, resource.TestCase{ + // No PreCheck for checking ENVs + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories(t), + Steps: []resource.TestStep{ + { + Config: testAccFwProvider_request_reason_inProviderBlock(context), + Check: resource.ComposeAggregateTestCheckFunc( + // Currently the PF provider uses empty strings, instead of providing validation feedback to users + // See: https://github.com/hashicorp/terraform-provider-google/issues/19643 + resource.TestCheckResourceAttr("data.google_provider_config_plugin_framework.default", "request_reason", emptyString), + ), + }, + }, + }) +} + +// testAccFwProvider_request_reason_inProviderBlock allows setting the request_reason argument in a provider block. +// This function uses data.google_provider_config_plugin_framework because it is implemented with the plugin-framework +func testAccFwProvider_request_reason_inProviderBlock(context map[string]interface{}) string { + return acctest.Nprintf(` +provider "google" { + request_reason = "%{request_reason}" +} + +data "google_provider_config_plugin_framework" "default" {} +`, context) +} + +// testAccFwProvider_request_reason_inEnvsOnly allows testing when the request_reason argument +// is only supplied via ENVs +func testAccFwProvider_request_reason_inEnvsOnly(context map[string]interface{}) string { + return acctest.Nprintf(` +data "google_provider_config_plugin_framework" "default" {} +`, context) +} diff --git a/mmv1/third_party/terraform/fwtransport/framework_config.go.tmpl b/mmv1/third_party/terraform/fwtransport/framework_config.go.tmpl index 659d802b6d40..f99a8dc0dfc8 100644 --- a/mmv1/third_party/terraform/fwtransport/framework_config.go.tmpl +++ b/mmv1/third_party/terraform/fwtransport/framework_config.go.tmpl @@ -38,6 +38,7 @@ type FrameworkProviderConfig struct { AccessToken types.String ImpersonateServiceAccount types.String ImpersonateServiceAccountDelegates types.List + RequestReason types.String // End temporary BillingProject types.String @@ -109,6 +110,8 @@ func (p *FrameworkProviderConfig) LoadAndValidateFramework(ctx context.Context, p.AccessToken = data.AccessToken p.ImpersonateServiceAccount = data.ImpersonateServiceAccount p.ImpersonateServiceAccountDelegates = data.ImpersonateServiceAccountDelegates + p.RequestReason = data.RequestReason + // End temporary // Copy values from the ProviderModel struct containing data about the provider configuration (present only when responsing to ConfigureProvider rpc calls) diff --git a/mmv1/third_party/terraform/provider/provider_request_reason_test.go b/mmv1/third_party/terraform/provider/provider_request_reason_test.go new file mode 100644 index 000000000000..e51a61c1396d --- /dev/null +++ b/mmv1/third_party/terraform/provider/provider_request_reason_test.go @@ -0,0 +1,142 @@ +package provider_test + +import ( + "testing" + + "github.com/hashicorp/terraform-plugin-testing/helper/resource" + "github.com/hashicorp/terraform-provider-google/google/acctest" +) + +// TestAccSdkProvider_request_reason is a series of acc tests asserting how the SDK provider handles request_reason arguments +// It is SDK specific because the HCL used provisions SDK-implemented resources +// It is a counterpart to TestAccFwProvider_request_reason +func TestAccSdkProvider_request_reason(t *testing.T) { + testCases := map[string]func(t *testing.T){ + // Configuring the provider using inputs + "config takes precedence over environment variables": testAccSdkProvider_request_reason_configPrecedenceOverEnvironmentVariables, + "when request_reason is unset in the config, environment variables are used in a given order": testAccSdkProvider_request_reason_precedenceOrderEnvironmentVariables, // CLOUDSDK_CORE_REQUEST_REASON + + // Schema-level validation + // TODO: https://github.com/hashicorp/terraform-provider-google/issues/19643 + "when request_reason is set to an empty string in the config the value IS ignored, allowing environment values to be used": testAccSdkProvider_request_reason_emptyStringValidation, + + // Usage + // We cannot test the impact of this field in an acc test, as it sets the X-Goog-Request-Reason value for audit logging purposes in GCP + // See: https://cloud.google.com/apis/docs/system-parameters#definitions + } + + for name, tc := range testCases { + // shadow the tc variable into scope so that when + // the loop continues, if t.Run hasn't executed tc(t) + // yet, we don't have a race condition + // see https://github.com/golang/go/wiki/CommonMistakes#using-goroutines-on-loop-iterator-variables + tc := tc + t.Run(name, func(t *testing.T) { + tc(t) + }) + } +} + +func testAccSdkProvider_request_reason_configPrecedenceOverEnvironmentVariables(t *testing.T) { + acctest.SkipIfVcr(t) // Test doesn't interact with API + + envReason := "environment-variables" + + // ensure all possible request_reason env vars set; show they aren't used instead + t.Setenv("CLOUDSDK_CORE_REQUEST_REASON", envReason) + + providerReason := "provider-config" + + context := map[string]interface{}{ + "request_reason": providerReason, + } + + acctest.VcrTest(t, resource.TestCase{ + // No PreCheck for checking ENVs + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories(t), + Steps: []resource.TestStep{ + { + Config: testAccSdkProvider_request_reason_inProviderBlock(context), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr("data.google_provider_config_sdk.default", "request_reason", providerReason), + ), + }, + }, + }) +} + +func testAccSdkProvider_request_reason_precedenceOrderEnvironmentVariables(t *testing.T) { + acctest.SkipIfVcr(t) // Test doesn't interact with API + /* + These are all the ENVs for request_reason + CLOUDSDK_CORE_REQUEST_REASON + */ + + CLOUDSDK_CORE_REQUEST_REASON := "CLOUDSDK_CORE_REQUEST_REASON" + + context := map[string]interface{}{} + + acctest.VcrTest(t, resource.TestCase{ + // No PreCheck for checking ENVs + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories(t), + Steps: []resource.TestStep{ + { + // CLOUDSDK_CORE_REQUEST_REASON is used if config doesn't provide a value + PreConfig: func() { + t.Setenv("CLOUDSDK_CORE_REQUEST_REASON", CLOUDSDK_CORE_REQUEST_REASON) //used + }, + Config: testAccSdkProvider_request_reason_inEnvsOnly(context), + Check: resource.ComposeAggregateTestCheckFunc( + resource.TestCheckResourceAttr("data.google_provider_config_sdk.default", "request_reason", CLOUDSDK_CORE_REQUEST_REASON), + ), + }, + }, + }) +} + +func testAccSdkProvider_request_reason_emptyStringValidation(t *testing.T) { + acctest.SkipIfVcr(t) // Test doesn't interact with API + + envReason := "environment-variables" + + // ensure all request_reason env vars set + t.Setenv("CLOUDSDK_CORE_REQUEST_REASON", envReason) + + context := map[string]interface{}{ + "request_reason": "", // empty string used + } + + acctest.VcrTest(t, resource.TestCase{ + // No PreCheck for checking ENVs + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories(t), + Steps: []resource.TestStep{ + { + Config: testAccSdkProvider_request_reason_inProviderBlock(context), + Check: resource.ComposeAggregateTestCheckFunc( + // Currently the SDK provider silently ignores empty strings, treating them similar to being unset + resource.TestCheckResourceAttr("data.google_provider_config_sdk.default", "request_reason", envReason), + ), + }, + }, + }) +} + +// testAccSdkProvider_request_reason_inProviderBlock allows setting the request_reason argument in a provider block. +// This function uses data.google_provider_config_sdk because it is implemented with the SDKv2 +func testAccSdkProvider_request_reason_inProviderBlock(context map[string]interface{}) string { + return acctest.Nprintf(` +provider "google" { + request_reason = "%{request_reason}" +} + +data "google_provider_config_sdk" "default" {} +`, context) +} + +// testAccSdkProvider_request_reason_inEnvsOnly allows testing when the request_reason argument +// is only supplied via ENVs +func testAccSdkProvider_request_reason_inEnvsOnly(context map[string]interface{}) string { + return acctest.Nprintf(` +data "google_provider_config_sdk" "default" {} +`, context) +}