From 5ce0565d077433fed6883b243e2dc4449278baac Mon Sep 17 00:00:00 2001 From: Modular Magician Date: Thu, 14 Sep 2023 18:02:33 +0000 Subject: [PATCH] Make plugin-framework provider configuration code treat empty values like the SDK (#8798) * Make PF config code change `project = ""` to null value before checking ENVs * Make plugin framework config code change all empty strings in config to null values * Fix defect in unit test for `project`, uncomment * Uncomment empty string unit tests * Fix defect in test; expeceted value should be null list * Add handling of empty lists in `HandleZeroValues` * Fix typo in comment * Add test case asserting `impersonate_service_account` empty strings are overridden by an ENV * Update SDK `batching` tests: rename test cases, make it clear values are arbitary * Update `HandleZeroValues` to handle `batching` argument * Uncomment empty string test * Change test inputs from 123s to 45s This is because 123s is transformed to 2m3s, but 45s remains 45s * Protect against Batching being null/unknown in `HandleZeroValues` * Add non-VCR acceptance test that shows provider behaviour when `credentials=""` and `GOOGLE_CREDENTIALS` interact Signed-off-by: Modular Magician --- .changelog/8798.txt | 3 + google/fwtransport/framework_config.go | 81 ++++++ google/fwtransport/framework_config_test.go | 284 ++++++++++---------- google/provider/provider_internal_test.go | 20 +- google/provider/provider_test.go | 69 +++++ 5 files changed, 310 insertions(+), 147 deletions(-) create mode 100644 .changelog/8798.txt diff --git a/.changelog/8798.txt b/.changelog/8798.txt new file mode 100644 index 00000000000..21d03751d30 --- /dev/null +++ b/.changelog/8798.txt @@ -0,0 +1,3 @@ +```release-note:bug +provider: fixed the provider so it resumes ignoring empty strings set in the `provider` block +``` diff --git a/google/fwtransport/framework_config.go b/google/fwtransport/framework_config.go index 6f2ff38a2c2..c98c9724b16 100644 --- a/google/fwtransport/framework_config.go +++ b/google/fwtransport/framework_config.go @@ -18,6 +18,7 @@ import ( "google.golang.org/grpc" "github.com/hashicorp/go-cleanhttp" + "github.com/hashicorp/terraform-plugin-framework/attr" "github.com/hashicorp/terraform-plugin-framework/diag" "github.com/hashicorp/terraform-plugin-framework/types" "github.com/hashicorp/terraform-plugin-log/tflog" @@ -151,6 +152,15 @@ type FrameworkProviderConfig struct { // LoadAndValidateFramework handles the bulk of configuring the provider // it is pulled out so that we can manually call this from our testing provider as well func (p *FrameworkProviderConfig) LoadAndValidateFramework(ctx context.Context, data *fwmodels.ProviderModel, tfVersion string, diags *diag.Diagnostics, providerversion string) { + + // Make the plugin framwork code behave like the SDK by ignoring zero values. This means re-setting zero values to null. + // This is added to fix https://github.com/hashicorp/terraform-provider-google/issues/14255 in a v4.x.x release + // TODO(SarahFrench) remove as part of https://github.com/hashicorp/terraform-provider-google/issues/14447 in 5.0.0 + p.HandleZeroValues(ctx, data, diags) + if diags.HasError() { + return + } + // Set defaults if needed p.HandleDefaults(ctx, data, diags) if diags.HasError() { @@ -295,6 +305,77 @@ func (p *FrameworkProviderConfig) LoadAndValidateFramework(ctx context.Context, p.RequestBatcherIam = transport_tpg.NewRequestBatcher("IAM", ctx, batchingConfig) } +// HandleZeroValues will make the plugin framework act like the SDK; zero value, particularly empty strings, are converted to null. +// This causes the plugin framework to treat the field as unset, just like how the SDK ignores empty strings. +func (p *FrameworkProviderConfig) HandleZeroValues(ctx context.Context, data *fwmodels.ProviderModel, diags *diag.Diagnostics) { + + // Change empty strings to null values + if data.AccessToken.Equal(types.StringValue("")) { + data.AccessToken = types.StringNull() + } + if data.BillingProject.Equal(types.StringValue("")) { + data.BillingProject = types.StringNull() + } + if data.Credentials.Equal(types.StringValue("")) { + data.Credentials = types.StringNull() + } + if data.ImpersonateServiceAccount.Equal(types.StringValue("")) { + data.ImpersonateServiceAccount = types.StringNull() + } + if data.Project.Equal(types.StringValue("")) { + data.Project = types.StringNull() + } + if data.Region.Equal(types.StringValue("")) { + data.Region = types.StringNull() + } + if data.RequestReason.Equal(types.StringValue("")) { + data.RequestReason = types.StringNull() + } + if data.RequestTimeout.Equal(types.StringValue("")) { + data.RequestTimeout = types.StringNull() + } + if data.Zone.Equal(types.StringValue("")) { + data.Zone = types.StringNull() + } + + // Change lists that aren't null or unknown with length of zero to null lists + if !data.Scopes.IsNull() && !data.Scopes.IsUnknown() && (len(data.Scopes.Elements()) == 0) { + data.Scopes = types.ListNull(types.StringType) + } + if !data.ImpersonateServiceAccountDelegates.IsNull() && !data.ImpersonateServiceAccountDelegates.IsUnknown() && (len(data.ImpersonateServiceAccountDelegates.Elements()) == 0) { + data.ImpersonateServiceAccountDelegates = types.ListNull(types.StringType) + } + + // Batching implementation will change in future, but this code will be removed in 5.0.0 so may be unaffected + if !data.Batching.IsNull() && !data.Batching.IsUnknown() && (len(data.Batching.Elements()) > 0) { + var pbConfigs []fwmodels.ProviderBatching + d := data.Batching.ElementsAs(ctx, &pbConfigs, true) + diags.Append(d...) + if diags.HasError() { + return + } + if pbConfigs[0].SendAfter.Equal(types.StringValue("")) { + pbConfigs[0].SendAfter = types.StringNull() // Convert empty string to null + } + b, _ := types.ObjectValue( + map[string]attr.Type{ + "enable_batching": types.BoolType, + "send_after": types.StringType, + }, + map[string]attr.Value{ + "enable_batching": pbConfigs[0].EnableBatching, + "send_after": pbConfigs[0].SendAfter, + }, + ) + newBatching, d := types.ListValue(types.ObjectType{}.WithAttributeTypes(fwmodels.ProviderBatchingAttributes), []attr.Value{b}) + diags.Append(d...) + if diags.HasError() { + return + } + data.Batching = newBatching + } +} + // HandleDefaults will handle all the defaults necessary in the provider func (p *FrameworkProviderConfig) HandleDefaults(ctx context.Context, data *fwmodels.ProviderModel, diags *diag.Diagnostics) { if data.AccessToken.IsNull() && data.Credentials.IsNull() { diff --git a/google/fwtransport/framework_config_test.go b/google/fwtransport/framework_config_test.go index 54a5a0f5775..8d8e95a7757 100644 --- a/google/fwtransport/framework_config_test.go +++ b/google/fwtransport/framework_config_test.go @@ -103,24 +103,23 @@ func TestFrameworkProvider_LoadAndValidateFramework_project(t *testing.T) { ExpectedConfigStructValue: types.StringNull(), }, // Handling empty strings in config - // TODO(SarahFrench) make these tests pass to address: https://github.com/hashicorp/terraform-provider-google/issues/14255 - // "when project is set as an empty string the field is treated as if it's unset, without error": { - // ConfigValues: fwmodels.ProviderModel{ - // Project: types.StringValue(""), - // }, - // ExpectedDataModelValue: types.StringNull(), - // ExpectedConfigStructValue: types.StringNull(), - // }, - // "when project is set as an empty string an environment variable will be used": { - // ConfigValues: fwmodels.ProviderModel{ - // Project: types.StringValue(""), - // }, - // EnvVariables: map[string]string{ - // "GOOGLE_PROJECT": "project-from-GOOGLE_PROJECT", - // }, - // ExpectedDataModelValue: types.StringNull(), - // ExpectedConfigStructValue: types.StringValue("project-from-GOOGLE_PROJECT"), - // }, + "when project is set as an empty string the field is treated as if it's unset, without error": { + ConfigValues: fwmodels.ProviderModel{ + Project: types.StringValue(""), + }, + ExpectedDataModelValue: types.StringNull(), + ExpectedConfigStructValue: types.StringNull(), + }, + "when project is set as an empty string an environment variable will be used": { + ConfigValues: fwmodels.ProviderModel{ + Project: types.StringValue(""), + }, + EnvVariables: map[string]string{ + "GOOGLE_PROJECT": "project-from-GOOGLE_PROJECT", + }, + ExpectedDataModelValue: types.StringValue("project-from-GOOGLE_PROJECT"), + ExpectedConfigStructValue: types.StringValue("project-from-GOOGLE_PROJECT"), + }, // Handling unknown values // TODO(SarahFrench) make these tests pass to address: https://github.com/hashicorp/terraform-provider-google/issues/14444 // "when project is an unknown value, the provider treats it as if it's unset (align to SDK behaviour)": { @@ -265,16 +264,15 @@ func TestFrameworkProvider_LoadAndValidateFramework_credentials(t *testing.T) { ExpectedDataModelValue: types.StringNull(), }, // Handling empty strings in config - // TODO(SarahFrench) make these tests pass to address: https://github.com/hashicorp/terraform-provider-google/issues/14255 - // "when credentials is set to an empty string in the config (and access_token unset), GOOGLE_APPLICATION_CREDENTIALS is used": { - // ConfigValues: fwmodels.ProviderModel{ - // Credentials: types.StringValue(""), - // }, - // EnvVariables: map[string]string{ - // "GOOGLE_APPLICATION_CREDENTIALS": transport_tpg.TestFakeCredentialsPath, // needs to be a path to a file when used by code - // }, - // ExpectedDataModelValue: types.StringNull(), - // }, + "when credentials is set to an empty string in the config (and access_token unset), GOOGLE_APPLICATION_CREDENTIALS is used": { + ConfigValues: fwmodels.ProviderModel{ + Credentials: types.StringValue(""), + }, + EnvVariables: map[string]string{ + "GOOGLE_APPLICATION_CREDENTIALS": transport_tpg.TestFakeCredentialsPath, // needs to be a path to a file when used by code + }, + ExpectedDataModelValue: types.StringNull(), + }, // NOTE: these tests can't run in Cloud Build due to ADC locating credentials despite `GOOGLE_APPLICATION_CREDENTIALS` being unset // See https://cloud.google.com/docs/authentication/application-default-credentials#search_order // Also, when running these tests locally you need to run `gcloud auth application-default revoke` to ensure your machine isn't supplying ADCs @@ -435,23 +433,23 @@ func TestFrameworkProvider_LoadAndValidateFramework_billingProject(t *testing.T) ExpectedConfigStructValue: types.StringNull(), }, // Handling empty strings in config - // "when billing_project is set as an empty string the field is treated as if it's unset, without error": { - // ConfigValues: fwmodels.ProviderModel{ - // BillingProject: types.StringValue(""), - // }, - // ExpectedDataModelValue: types.StringNull(), - // ExpectedConfigStructValue: types.StringNull(), - // }, - // "when billing_project is set as an empty string an environment variable will be used": { - // ConfigValues: fwmodels.ProviderModel{ - // BillingProject: types.StringValue(""), - // }, - // EnvVariables: map[string]string{ - // "GOOGLE_BILLING_PROJECT": "billing-project-from-env", - // }, - // ExpectedDataModelValue: types.StringValue("billing-project-from-env"), - // ExpectedConfigStructValue: types.StringValue("billing-project-from-env"), - // }, + "when billing_project is set as an empty string the field is treated as if it's unset, without error": { + ConfigValues: fwmodels.ProviderModel{ + BillingProject: types.StringValue(""), + }, + ExpectedDataModelValue: types.StringNull(), + ExpectedConfigStructValue: types.StringNull(), + }, + "when billing_project is set as an empty string an environment variable will be used": { + ConfigValues: fwmodels.ProviderModel{ + BillingProject: types.StringValue(""), + }, + EnvVariables: map[string]string{ + "GOOGLE_BILLING_PROJECT": "billing-project-from-env", + }, + ExpectedDataModelValue: types.StringValue("billing-project-from-env"), + ExpectedConfigStructValue: types.StringValue("billing-project-from-env"), + }, } for tn, tc := range cases { @@ -549,24 +547,23 @@ func TestFrameworkProvider_LoadAndValidateFramework_region(t *testing.T) { ExpectedConfigStructValue: types.StringNull(), }, // Handling empty strings in config - // TODO(SarahFrench) make these tests pass to address: https://github.com/hashicorp/terraform-provider-google/issues/14255 - // "when region is set as an empty string the field is treated as if it's unset, without error": { - // ConfigValues: fwmodels.ProviderModel{ - // Region: types.StringValue(""), - // }, - // ExpectedDataModelValue: types.StringNull(), - // ExpectedConfigStructValue: types.StringNull(), - // }, - // "when region is set as an empty string an environment variable will be used": { - // ConfigValues: fwmodels.ProviderModel{ - // Region: types.StringValue(""), - // }, - // EnvVariables: map[string]string{ - // "GOOGLE_REGION": "region-from-env", - // }, - // ExpectedDataModelValue: types.StringValue("region-from-env"), - // ExpectedConfigStructValue: types.StringValue("region-from-env"), - // }, + "when region is set as an empty string the field is treated as if it's unset, without error": { + ConfigValues: fwmodels.ProviderModel{ + Region: types.StringValue(""), + }, + ExpectedDataModelValue: types.StringNull(), + ExpectedConfigStructValue: types.StringNull(), + }, + "when region is set as an empty string an environment variable will be used": { + ConfigValues: fwmodels.ProviderModel{ + Region: types.StringValue(""), + }, + EnvVariables: map[string]string{ + "GOOGLE_REGION": "region-from-env", + }, + ExpectedDataModelValue: types.StringValue("region-from-env"), + ExpectedConfigStructValue: types.StringValue("region-from-env"), + }, // Handling unknown values // TODO(SarahFrench) make these tests pass to address: https://github.com/hashicorp/terraform-provider-google/issues/14444 // "when region is an unknown value, the provider treats it as if it's unset (align to SDK behaviour)": { @@ -698,24 +695,23 @@ func TestFrameworkProvider_LoadAndValidateFramework_zone(t *testing.T) { ExpectedConfigStructValue: types.StringNull(), }, // Handling empty strings in config - // TODO(SarahFrench) make these tests pass to address: https://github.com/hashicorp/terraform-provider-google/issues/14255 - // "when zone is set as an empty string the field is treated as if it's unset, without error": { - // ConfigValues: fwmodels.ProviderModel{ - // Zone: types.StringValue(""), - // }, - // ExpectedDataModelValue: types.StringNull(), - // ExpectedConfigStructValue: types.StringNull(), - // }, - // "when zone is set as an empty string an environment variable will be used": { - // ConfigValues: fwmodels.ProviderModel{ - // Zone: types.StringValue(""), - // }, - // EnvVariables: map[string]string{ - // "GOOGLE_ZONE": "zone-from-env", - // }, - // ExpectedDataModelValue: types.StringValue("zone-from-env"), - // ExpectedConfigStructValue: types.StringValue("zone-from-env"), - // }, + "when zone is set as an empty string the field is treated as if it's unset, without error": { + ConfigValues: fwmodels.ProviderModel{ + Zone: types.StringValue(""), + }, + ExpectedDataModelValue: types.StringNull(), + ExpectedConfigStructValue: types.StringNull(), + }, + "when zone is set as an empty string an environment variable will be used": { + ConfigValues: fwmodels.ProviderModel{ + Zone: types.StringValue(""), + }, + EnvVariables: map[string]string{ + "GOOGLE_ZONE": "zone-from-env", + }, + ExpectedDataModelValue: types.StringValue("zone-from-env"), + ExpectedConfigStructValue: types.StringValue("zone-from-env"), + }, // Handling unknown values // TODO(SarahFrench) make these tests pass to address: https://github.com/hashicorp/terraform-provider-google/issues/14444 // "when zone is an unknown value, the provider treats it as if it's unset (align to SDK behaviour)": { @@ -814,23 +810,22 @@ func TestFrameworkProvider_LoadAndValidateFramework_accessToken(t *testing.T) { ExpectedDataModelValue: types.StringNull(), }, // Handling empty strings in config - // TODO(SarahFrench) make these tests pass to address: https://github.com/hashicorp/terraform-provider-google/issues/14255 - // "when access_token is set as an empty string the field is treated as if it's unset, without error (as long as credentials supplied in its absence)": { - // ConfigValues: fwmodels.ProviderModel{ - // AccessToken: types.StringValue(""), - // Credentials: types.StringValue(transport_tpg.TestFakeCredentialsPath), - // }, - // ExpectedDataModelValue: types.StringNull(), - // }, - // "when access_token is set as an empty string in the config, an environment variable is used": { - // ConfigValues: fwmodels.ProviderModel{ - // AccessToken: types.StringValue(""), - // }, - // EnvVariables: map[string]string{ - // "GOOGLE_OAUTH_ACCESS_TOKEN": "value-from-GOOGLE_OAUTH_ACCESS_TOKEN", - // }, - // ExpectedDataModelValue: types.StringValue("value-from-GOOGLE_OAUTH_ACCESS_TOKEN"), - // }, + "when access_token is set as an empty string the field is treated as if it's unset, without error (as long as credentials supplied in its absence)": { + ConfigValues: fwmodels.ProviderModel{ + AccessToken: types.StringValue(""), + Credentials: types.StringValue(transport_tpg.TestFakeCredentialsPath), + }, + ExpectedDataModelValue: types.StringNull(), + }, + "when access_token is set as an empty string in the config, an environment variable is used": { + ConfigValues: fwmodels.ProviderModel{ + AccessToken: types.StringValue(""), + }, + EnvVariables: map[string]string{ + "GOOGLE_OAUTH_ACCESS_TOKEN": "value-from-GOOGLE_OAUTH_ACCESS_TOKEN", + }, + ExpectedDataModelValue: types.StringValue("value-from-GOOGLE_OAUTH_ACCESS_TOKEN"), + }, // Handling unknown values // TODO(SarahFrench) make these tests pass to address: https://github.com/hashicorp/terraform-provider-google/issues/14444 // "when access_token is an unknown value, the provider treats it as if it's unset (align to SDK behaviour)": { @@ -1054,13 +1049,21 @@ func TestFrameworkProvider_LoadAndValidateFramework_impersonateServiceAccount(t ExpectedDataModelValue: types.StringNull(), }, // Handling empty strings in config - // TODO(SarahFrench) make these tests pass to address: https://github.com/hashicorp/terraform-provider-google/issues/14255 - // "when impersonate_service_account is set as an empty array the field is treated as if it's unset, without error": { - // ConfigValues: fwmodels.ProviderModel{ - // ImpersonateServiceAccount: types.StringValue(""), - // }, - // ExpectedDataModelValue: types.StringNull(), - // }, + "when impersonate_service_account is set as an empty string the field is treated as if it's unset, without error": { + ConfigValues: fwmodels.ProviderModel{ + ImpersonateServiceAccount: types.StringValue(""), + }, + ExpectedDataModelValue: types.StringNull(), + }, + "when impersonate_service_account is set as an empty string in the config, an environment variable is used": { + ConfigValues: fwmodels.ProviderModel{ + ImpersonateServiceAccount: types.StringValue(""), + }, + EnvVariables: map[string]string{ + "GOOGLE_IMPERSONATE_SERVICE_ACCOUNT": "value-from-env@example.com", + }, + ExpectedDataModelValue: types.StringValue("value-from-env@example.com"), + }, // Handling unknown values // TODO(SarahFrench) make these tests pass to address: https://github.com/hashicorp/terraform-provider-google/issues/14444 // "when impersonate_service_account is an unknown value, the provider treats it as if it's unset (align to SDK behaviour)": { @@ -1148,7 +1151,7 @@ func TestFrameworkProvider_LoadAndValidateFramework_impersonateServiceAccountDel // Handling empty values in config "when impersonate_service_account_delegates is set as an empty array the field is treated as if it's unset, without error": { ImpersonateServiceAccountDelegatesValue: []string{}, - ExpectedDataModelValue: []string{}, + ExpectedDataModelValue: nil, }, // Handling unknown values // TODO(SarahFrench) make these tests pass to address: https://github.com/hashicorp/terraform-provider-google/issues/14444 @@ -1347,22 +1350,21 @@ func TestFrameworkProvider_LoadAndValidateFramework_requestReason(t *testing.T) ExpectedDataModelValue: types.StringNull(), }, // Handling empty strings in config - // TODO(SarahFrench) make these tests pass to address: https://github.com/hashicorp/terraform-provider-google/issues/14255 - // "when request_reason is set as an empty string in the config it is overridden by environment variables": { - // ConfigValues: fwmodels.ProviderModel{ - // RequestReason: types.StringValue(""), - // }, - // EnvVariables: map[string]string{ - // "CLOUDSDK_CORE_REQUEST_REASON": "foo", - // }, - // ExpectedDataModelValue: types.StringValue("foo"), - // }, - // "when request_reason is set as an empty string in the config the field is treated as if it's unset, without error": { - // ConfigValues: fwmodels.ProviderModel{ - // RequestReason: types.StringValue(""), - // }, - // ExpectedDataModelValue: types.StringNull(), - // }, + "when request_reason is set as an empty string in the config it is overridden by environment variables": { + ConfigValues: fwmodels.ProviderModel{ + RequestReason: types.StringValue(""), + }, + EnvVariables: map[string]string{ + "CLOUDSDK_CORE_REQUEST_REASON": "foo", + }, + ExpectedDataModelValue: types.StringValue("foo"), + }, + "when request_reason is set as an empty string in the config the field is treated as if it's unset, without error": { + ConfigValues: fwmodels.ProviderModel{ + RequestReason: types.StringValue(""), + }, + ExpectedDataModelValue: types.StringNull(), + }, // Handling unknown values // TODO(SarahFrench) make these tests pass to address: https://github.com/hashicorp/terraform-provider-google/issues/14444 // "when request_timeout is an unknown value, the provider treats it as if it's unset (align to SDK behaviour)": { @@ -1451,13 +1453,12 @@ func TestFrameworkProvider_LoadAndValidateFramework_requestTimeout(t *testing.T) ExpectedDataModelValue: types.StringValue("120s"), }, // Handling empty strings in config - // TODO(SarahFrench) make these tests pass to address: https://github.com/hashicorp/terraform-provider-google/issues/14255 - // "when request_timeout is set as an empty string, the default value is 120s.": { - // ConfigValues: fwmodels.ProviderModel{ - // RequestTimeout: types.StringValue(""), - // }, - // ExpectedDataModelValue: types.StringValue("120s"), - // }, + "when request_timeout is set as an empty string, the default value is 120s.": { + ConfigValues: fwmodels.ProviderModel{ + RequestTimeout: types.StringValue(""), + }, + ExpectedDataModelValue: types.StringValue("120s"), + }, // Handling unknown values // TODO(SarahFrench) make these tests pass to address: https://github.com/hashicorp/terraform-provider-google/issues/14444 // "when request_timeout is an unknown value, the provider treats it as if it's unset (align to SDK behaviour)": { @@ -1534,9 +1535,9 @@ func TestFrameworkProvider_LoadAndValidateFramework_batching(t *testing.T) { }{ "batching can be configured with values for enable_batching and send_after": { EnableBatchingValue: types.BoolValue(true), - SendAfterValue: types.StringValue("123s"), + SendAfterValue: types.StringValue("45s"), ExpectEnableBatchingValue: types.BoolValue(true), - ExpectSendAfterValue: types.StringValue("123s"), + ExpectSendAfterValue: types.StringValue("45s"), }, "if batching is an empty block, it will set the default values for enable_batching and send_after": { // In this test, we try to create a list containing only null values @@ -1553,18 +1554,17 @@ func TestFrameworkProvider_LoadAndValidateFramework_batching(t *testing.T) { }, "when batching is configured with only send_after, enable_batching will be set to a default value": { EnableBatchingValue: types.BoolNull(), - SendAfterValue: types.StringValue("123s"), + SendAfterValue: types.StringValue("45s"), ExpectEnableBatchingValue: types.BoolValue(true), - ExpectSendAfterValue: types.StringValue("123s"), + ExpectSendAfterValue: types.StringValue("45s"), }, // Handling empty strings in config - // TODO(SarahFrench) make these tests pass to address: https://github.com/hashicorp/terraform-provider-google/issues/14255 - // "when batching is configured with send_after as an empty string, send_after will be set to a default value": { - // EnableBatchingValue: types.BoolValue(true), - // SendAfterValue: types.StringValue(""), - // ExpectEnableBatchingValue: types.BoolValue(true), - // ExpectSendAfterValue: types.StringValue("10s"), - // }, + "when batching is configured with send_after as an empty string, send_after will be set to a default value": { + EnableBatchingValue: types.BoolValue(true), + SendAfterValue: types.StringValue(""), + ExpectEnableBatchingValue: types.BoolValue(true), + ExpectSendAfterValue: types.StringValue("10s"), // When batching block is present but has missing arguments inside, default is 10s + }, // Handling unknown values // TODO(SarahFrench) make these tests pass to address: https://github.com/hashicorp/terraform-provider-google/issues/14444 // "when batching is an unknown value, the provider treats it as if it's unset (align to SDK behaviour)": { @@ -1580,9 +1580,9 @@ func TestFrameworkProvider_LoadAndValidateFramework_batching(t *testing.T) { // }, // "when batching is configured with enable_batching as an unknown value, the provider treats it as if it's unset (align to SDK behaviour)": { // EnableBatchingValue: types.BoolNull(), - // SendAfterValue: types.StringValue("123s"), + // SendAfterValue: types.StringValue("45s"), // ExpectEnableBatchingValue: types.BoolValue(true), - // ExpectSendAfterValue: types.StringValue("123s"), + // ExpectSendAfterValue: types.StringValue("45s"), // }, // Error states "if batching is configured with send_after as an invalid value, there's an error": { diff --git a/google/provider/provider_internal_test.go b/google/provider/provider_internal_test.go index e672548b9e8..5f43794eea1 100644 --- a/google/provider/provider_internal_test.go +++ b/google/provider/provider_internal_test.go @@ -405,7 +405,7 @@ func TestProvider_ProviderConfigure_impersonateServiceAccount(t *testing.T) { ExpectedValue: "", }, // Handling empty strings in config - "when impersonate_service_account is set as an empty array the field is treated as if it's unset, without error": { + "when impersonate_service_account is set as an empty string the field is treated as if it's unset, without error": { ConfigValues: map[string]interface{}{ "impersonate_service_account": "", "credentials": transport_tpg.TestFakeCredentialsPath, @@ -414,6 +414,16 @@ func TestProvider_ProviderConfigure_impersonateServiceAccount(t *testing.T) { ExpectFieldUnset: true, ExpectedValue: "", }, + "when impersonate_service_account is set as an empty string in the config, an environment variable is used": { + ConfigValues: map[string]interface{}{ + "impersonate_service_account": "", + "credentials": transport_tpg.TestFakeCredentialsPath, + }, + EnvVariables: map[string]string{ + "GOOGLE_IMPERSONATE_SERVICE_ACCOUNT": "value-from-env@example.com", + }, + ExpectedValue: "value-from-env@example.com", + }, } for tn, tc := range cases { @@ -1573,12 +1583,12 @@ func TestProvider_ProviderConfigure_batching(t *testing.T) { "batching": []interface{}{ map[string]interface{}{ "enable_batching": true, - "send_after": "123s", + "send_after": "45s", }, }, }, ExpectedEnableBatchingValue: true, - ExpectedSendAfterValue: "123s", + ExpectedSendAfterValue: "45s", }, "if batching is an empty block, it will set the default values for enable_batching and send_after": { ConfigValues: map[string]interface{}{ @@ -1609,12 +1619,12 @@ func TestProvider_ProviderConfigure_batching(t *testing.T) { "credentials": transport_tpg.TestFakeCredentialsPath, "batching": []interface{}{ map[string]interface{}{ - "send_after": "123s", + "send_after": "45s", }, }, }, ExpectedEnableBatchingValue: false, - ExpectedSendAfterValue: "123s", + ExpectedSendAfterValue: "45s", }, // Error states "if batching is configured with send_after as an invalid value, there's an error": { diff --git a/google/provider/provider_test.go b/google/provider/provider_test.go index 1930ed281f2..7f4de2d0f44 100644 --- a/google/provider/provider_test.go +++ b/google/provider/provider_test.go @@ -181,6 +181,49 @@ func TestAccProviderIndirectUserProjectOverride(t *testing.T) { }) } +func TestAccProviderCredentialsEmptyString(t *testing.T) { + // Test is not parallel because ENVs are set. + // Need to skip VCR as this test downloads providers from the Terraform Registry + acctest.SkipIfVcr(t) + + creds := envvar.GetTestCredsFromEnv() + project := envvar.GetTestProjectFromEnv() + t.Setenv("GOOGLE_CREDENTIALS", creds) + t.Setenv("GOOGLE_PROJECT", project) + + pid := "tf-test-" + acctest.RandString(t, 10) + + acctest.VcrTest(t, resource.TestCase{ + PreCheck: func() { acctest.AccTestPreCheck(t) }, + // No TestDestroy since that's not really the point of this test + Steps: []resource.TestStep{ + { + Config: testAccProviderCredentials_actWithCredsFromEnv(pid), + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories(t), + }, + { + // Assert that errors are expected with credentials when + // - GOOGLE_CREDENTIALS is set + // - provider block has credentials = "" + // - TPG v4.60.2 is used + Config: testAccProviderCredentials_actWithCredsFromEnv_emptyString(pid), + ExternalProviders: map[string]resource.ExternalProvider{ + "google": { + VersionConstraint: "4.60.2", + Source: "hashicorp/google", + }, + }, + ExpectError: regexp.MustCompile(`unexpected end of JSON input`), + }, + { + // Errors are not expected when using the latest 4.x.x version of the provider + Config: testAccProviderCredentials_actWithCredsFromEnv_emptyString(pid), + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories(t), + }, + }, + }) +} + func testAccProviderBasePath_setBasePath(endpoint, name string) string { return fmt.Sprintf(` provider "google" { @@ -322,3 +365,29 @@ func testAccCheckComputeAddressDestroyProducer(t *testing.T) func(s *terraform.S return nil } } + +func testAccProviderCredentials_actWithCredsFromEnv(name string) string { + return fmt.Sprintf(` +provider "google" { + alias = "testing_credentials" + +} + +resource "google_compute_address" "default" { + provider = google.testing_credentials + name = "%s" +}`, name) +} + +func testAccProviderCredentials_actWithCredsFromEnv_emptyString(name string) string { + return fmt.Sprintf(` +provider "google" { + alias = "testing_credentials" + credentials = "" +} + +resource "google_compute_address" "default" { + provider = google.testing_credentials + name = "%s" +}`, name) +}