From 7e8243e108402937d9a66631ec1dc032608fa60f Mon Sep 17 00:00:00 2001 From: Sarah French <15078782+SarahFrench@users.noreply.github.com> Date: Mon, 21 Oct 2024 20:00:49 +0100 Subject: [PATCH] Add acceptance tests for `request_timeout` (#11848) Co-authored-by: Mauricio Alvarez Leon <65101411+BBBmau@users.noreply.github.com> --- ...source_provider_config_plugin_framework.go | 2 +- ...amework_provider_access_token_test.go.tmpl | 2 +- ...framework_provider_request_timeout_test.go | 132 +++++++++++++ .../fwtransport/framework_config.go.tmpl | 4 +- .../provider/provider_request_timeout_test.go | 180 ++++++++++++++++++ 5 files changed, 317 insertions(+), 3 deletions(-) create mode 100644 mmv1/third_party/terraform/fwprovider/framework_provider_request_timeout_test.go create mode 100644 mmv1/third_party/terraform/provider/provider_request_timeout_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 98ec9011e6ed..315d2dd093fc 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 @@ -215,7 +215,7 @@ func (d *GoogleProviderConfigPluginFrameworkDataSource) Read(ctx context.Context data.Scopes = d.providerConfig.Scopes data.UserProjectOverride = d.providerConfig.UserProjectOverride data.RequestReason = d.providerConfig.RequestReason - // TODO(SarahFrench) - request_timeout + data.RequestTimeout = d.providerConfig.RequestTimeout data.DefaultLabels = d.providerConfig.DefaultLabels data.AddTerraformAttributionLabel = d.providerConfig.AddTerraformAttributionLabel data.TerraformAttributionLabelAdditionStrategy = d.providerConfig.TerraformAttributionLabelAdditionStrategy diff --git a/mmv1/third_party/terraform/fwprovider/framework_provider_access_token_test.go.tmpl b/mmv1/third_party/terraform/fwprovider/framework_provider_access_token_test.go.tmpl index c85685f99654..ccc304e8cad1 100644 --- a/mmv1/third_party/terraform/fwprovider/framework_provider_access_token_test.go.tmpl +++ b/mmv1/third_party/terraform/fwprovider/framework_provider_access_token_test.go.tmpl @@ -267,7 +267,7 @@ func testAccFwProvider_access_token_authInUse(t *testing.T) { {{- end }} // testAccFwProvider_access_tokenInProviderBlock allows setting the access_token argument in a provider block. -// This function uses data.google_provider_config_plugin_framework because it is implemented with the SDKv2 +// This function uses data.google_provider_config_plugin_framework because it is implemented with the PF func testAccFwProvider_access_tokenInProviderBlock(context map[string]interface{}) string { return acctest.Nprintf(` provider "google" { diff --git a/mmv1/third_party/terraform/fwprovider/framework_provider_request_timeout_test.go b/mmv1/third_party/terraform/fwprovider/framework_provider_request_timeout_test.go new file mode 100644 index 000000000000..aa22b6a496f5 --- /dev/null +++ b/mmv1/third_party/terraform/fwprovider/framework_provider_request_timeout_test.go @@ -0,0 +1,132 @@ +package fwprovider_test + +import ( + "regexp" + "testing" + + "github.com/hashicorp/terraform-plugin-testing/helper/resource" + "github.com/hashicorp/terraform-provider-google/google/acctest" +) + +// TestAccFwProvider_request_timeout is a series of acc tests asserting how the PF provider handles request_timeout arguments +// It is PF specific because the HCL used provisions PF-implemented resources +// It is a counterpart to TestAccSdkProvider_request_timeout +func TestAccFwProvider_request_timeout(t *testing.T) { + testCases := map[string]func(t *testing.T){ + // Configuring the provider using inputs + "a default value of 120s is used when there are no user inputs": testAccFwProvider_request_timeout_providerDefault, + "request_timeout can be set in config in different formats, are NOT normalized to full-length format": testAccFwProvider_request_timeout_setInConfig, + //no ENVs to test + + // Schema-level validation + "when request_timeout is set to an empty string in the config the value fails validation, as it is not a duration": testAccFwProvider_request_timeout_emptyStringValidation, + + // Usage + // We cannot test the impact of this field in an acc test until more resources/data sources are implemented with the plugin-framework + } + + 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_timeout_providerDefault(t *testing.T) { + acctest.SkipIfVcr(t) // Test doesn't interact with API + + defaultValue := "120s" + + acctest.VcrTest(t, resource.TestCase{ + // No PreCheck for checking ENVs + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories(t), + Steps: []resource.TestStep{ + { + Config: testAccFwProvider_request_timeout_unset(), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr("data.google_provider_config_plugin_framework.default", "request_timeout", defaultValue), + ), + }, + }, + }) +} + +func testAccFwProvider_request_timeout_setInConfig(t *testing.T) { + acctest.SkipIfVcr(t) // Test doesn't interact with API + + providerTimeout1 := "3m0s" + providerTimeout2 := "3m" + + // In the SDK version of the test expectedValue = "3m0s" only + expectedValue1 := "3m0s" + expectedValue2 := "3m" + + context1 := map[string]interface{}{ + "request_timeout": providerTimeout1, + } + context2 := map[string]interface{}{ + "request_timeout": providerTimeout2, + } + + acctest.VcrTest(t, resource.TestCase{ + // No PreCheck for checking ENVs + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories(t), + Steps: []resource.TestStep{ + { + Config: testAccFwProvider_request_timeout_inProviderBlock(context1), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr("data.google_provider_config_plugin_framework.default", "request_timeout", expectedValue1), + ), + }, + { + Config: testAccFwProvider_request_timeout_inProviderBlock(context2), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr("data.google_provider_config_plugin_framework.default", "request_timeout", expectedValue2), + ), + }, + }, + }) +} + +func testAccFwProvider_request_timeout_emptyStringValidation(t *testing.T) { + acctest.SkipIfVcr(t) // Test doesn't interact with API + + context := map[string]interface{}{ + "request_timeout": "", // empty string used + } + + acctest.VcrTest(t, resource.TestCase{ + // No PreCheck for checking ENVs + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories(t), + Steps: []resource.TestStep{ + { + Config: testAccFwProvider_request_timeout_inProviderBlock(context), + ExpectError: regexp.MustCompile("invalid duration"), + }, + }, + }) +} + +// testAccFwProvider_request_timeout_inProviderBlock allows setting the request_timeout argument in a provider block. +// This function uses data.google_provider_config_plugin_framework because it is implemented with the PF +func testAccFwProvider_request_timeout_inProviderBlock(context map[string]interface{}) string { + return acctest.Nprintf(` +provider "google" { + request_timeout = "%{request_timeout}" +} + +data "google_provider_config_plugin_framework" "default" {} +`, context) +} + +// testAccFwProvider_request_timeout_inEnvsOnly allows testing when the request_timeout argument is not set +func testAccFwProvider_request_timeout_unset() string { + return ` +data "google_provider_config_plugin_framework" "default" {} +` +} diff --git a/mmv1/third_party/terraform/fwtransport/framework_config.go.tmpl b/mmv1/third_party/terraform/fwtransport/framework_config.go.tmpl index 9107259edd08..25e715009913 100644 --- a/mmv1/third_party/terraform/fwtransport/framework_config.go.tmpl +++ b/mmv1/third_party/terraform/fwtransport/framework_config.go.tmpl @@ -38,7 +38,8 @@ type FrameworkProviderConfig struct { AccessToken types.String ImpersonateServiceAccount types.String ImpersonateServiceAccountDelegates types.List - RequestReason types.String + RequestReason types.String + RequestTimeout types.String AddTerraformAttributionLabel types.Bool TerraformAttributionLabelAdditionStrategy types.String // End temporary @@ -113,6 +114,7 @@ func (p *FrameworkProviderConfig) LoadAndValidateFramework(ctx context.Context, p.ImpersonateServiceAccount = data.ImpersonateServiceAccount p.ImpersonateServiceAccountDelegates = data.ImpersonateServiceAccountDelegates p.RequestReason = data.RequestReason + p.RequestTimeout = data.RequestTimeout p.AddTerraformAttributionLabel = data.AddTerraformAttributionLabel p.TerraformAttributionLabelAdditionStrategy = data.TerraformAttributionLabelAdditionStrategy // End temporary diff --git a/mmv1/third_party/terraform/provider/provider_request_timeout_test.go b/mmv1/third_party/terraform/provider/provider_request_timeout_test.go new file mode 100644 index 000000000000..cd57877a8bf4 --- /dev/null +++ b/mmv1/third_party/terraform/provider/provider_request_timeout_test.go @@ -0,0 +1,180 @@ +package provider_test + +import ( + "regexp" + "testing" + + "github.com/hashicorp/terraform-plugin-testing/helper/resource" + "github.com/hashicorp/terraform-provider-google/google/acctest" +) + +// TestAccSdkProvider_request_timeout is a series of acc tests asserting how the SDK provider handles request_timeout arguments +// It is SDK specific because the HCL used provisions SDK-implemented resources +// It is a counterpart to TestAccFwProvider_request_timeout +func TestAccSdkProvider_request_timeout(t *testing.T) { + testCases := map[string]func(t *testing.T){ + // Configuring the provider using inputs + "a default value of 0s is used when there are no user inputs (it is overridden downstream)": testAccSdkProvider_request_timeout_providerDefault, + "request_timeout can be set in config in different formats, are normalized to full-length format": testAccSdkProvider_request_timeout_setInConfig, + //no ENVs to test + + // Schema-level validation + "when request_timeout is set to an empty string in the config the value fails validation, as it is not a duration": testAccSdkProvider_request_timeout_emptyStringValidation, + + // Usage + "short timeouts impact provisioning resources": testAccSdkProvider_request_timeout_usage, + } + + 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) + }) + } +} + +// In the SDK version of the provider config code request_timeout has a zero value of "0s" and no default. +// The final 'effective' value is "120s", matching the default used in the plugin-framework version of the provider config code. +// See : https://github.com/hashicorp/terraform-provider-google/blob/09cb850ee64bcd78e4457df70905530c1ed75f19/google/transport/config.go#L1228-L1233 +func testAccSdkProvider_request_timeout_providerDefault(t *testing.T) { + acctest.SkipIfVcr(t) // Test doesn't interact with API + + acctest.VcrTest(t, resource.TestCase{ + // No PreCheck for checking ENVs + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories(t), + Steps: []resource.TestStep{ + { + Config: testAccSdkProvider_request_timeout_unset(), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr("data.google_provider_config_sdk.default", "request_timeout", "0s"), + ), + }, + }, + }) +} + +func testAccSdkProvider_request_timeout_setInConfig(t *testing.T) { + acctest.SkipIfVcr(t) // Test doesn't interact with API + + providerTimeout1 := "3m0s" + providerTimeout2 := "3m" + expectedValue := "3m0s" + + context1 := map[string]interface{}{ + "request_timeout": providerTimeout1, + } + context2 := map[string]interface{}{ + "request_timeout": providerTimeout2, + } + + acctest.VcrTest(t, resource.TestCase{ + // No PreCheck for checking ENVs + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories(t), + Steps: []resource.TestStep{ + { + Config: testAccSdkProvider_request_timeout_inProviderBlock(context1), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr("data.google_provider_config_sdk.default", "request_timeout", expectedValue), + ), + }, + { + Config: testAccSdkProvider_request_timeout_inProviderBlock(context2), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr("data.google_provider_config_sdk.default", "request_timeout", expectedValue), + ), + }, + }, + }) +} + +func testAccSdkProvider_request_timeout_emptyStringValidation(t *testing.T) { + acctest.SkipIfVcr(t) // Test doesn't interact with API + + context := map[string]interface{}{ + "request_timeout": "", // empty string used + } + + acctest.VcrTest(t, resource.TestCase{ + // No PreCheck for checking ENVs + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories(t), + Steps: []resource.TestStep{ + { + Config: testAccSdkProvider_request_timeout_inProviderBlock(context), + ExpectError: regexp.MustCompile("invalid duration"), + }, + }, + }) +} + +func testAccSdkProvider_request_timeout_usage(t *testing.T) { + acctest.SkipIfVcr(t) // Test doesn't interact with API + + shortTimeout := "10ms" // short time that will result in an error + longTimeout := "120s" + + randomString := acctest.RandString(t, 10) + context1 := map[string]interface{}{ + "request_timeout": shortTimeout, + "random_suffix": randomString, + } + context2 := map[string]interface{}{ + "request_timeout": longTimeout, + "random_suffix": randomString, + } + + acctest.VcrTest(t, resource.TestCase{ + // No PreCheck for checking ENVs + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories(t), + Steps: []resource.TestStep{ + { + Config: testAccSdkProvider_request_timeout_provisionWithTimeout(context1), + // Effect of request_timeout value + ExpectError: regexp.MustCompile("context deadline exceeded"), + }, + { + Config: testAccSdkProvider_request_timeout_provisionWithTimeout(context2), + // No error; everything is fine with an appropriate timeout value + }, + }, + }) +} + +// testAccSdkProvider_request_timeout_inProviderBlock allows setting the request_timeout argument in a provider block. +// This function uses data.google_provider_config_sdk because it is implemented with the SDKv2 +func testAccSdkProvider_request_timeout_inProviderBlock(context map[string]interface{}) string { + return acctest.Nprintf(` +provider "google" { + request_timeout = "%{request_timeout}" +} + +data "google_provider_config_sdk" "default" {} +`, context) +} + +// testAccSdkProvider_request_timeout_provisionWithTimeout allows testing the effects of request_timeout on +// provisioning a resource. +func testAccSdkProvider_request_timeout_provisionWithTimeout(context map[string]interface{}) string { + return acctest.Nprintf(` +provider "google" { + request_timeout = "%{request_timeout}" +} + +data "google_provider_config_sdk" "default" {} + +resource "google_service_account" "default" { + account_id = "tf-test-%{random_suffix}" + display_name = "AccTest Service Account" +} +`, context) +} + +// testAccSdkProvider_request_timeout_inEnvsOnly allows testing when the request_timeout argument is not set +func testAccSdkProvider_request_timeout_unset() string { + return ` +data "google_provider_config_sdk" "default" {} +` +}