Skip to content

Commit

Permalink
Unignore empty values in the provider configuration block in 5.0.0 …
Browse files Browse the repository at this point in the history
…(#9014)

* Stop validating `""` as valid `credentials` value in the PF provider config

* Remove code that makes PF ignore empty values

* Update PF config code tests to match change in PF config code behaviour

* Update remaining PF config code tests

* Update test for the PF version of credentials validator

* Make not on SDK version of credentials validator that it cannot distinguish between `""` in config vs zero values

Signed-off-by: Modular Magician <magic-modules@google.com>
  • Loading branch information
modular-magician committed Sep 22, 2023
1 parent 82c389a commit 9c2b180
Show file tree
Hide file tree
Showing 20 changed files with 85 additions and 1,979 deletions.
112 changes: 55 additions & 57 deletions google-beta/fwtransport/framework_config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -336,63 +336,61 @@ func TestFrameworkProvider_LoadAndValidateFramework_credentials(t *testing.T) {
}
}

// 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
// func TestFrameworkProvider_LoadAndValidateFramework_credentials_unknown(t *testing.T) {
// // This test case is kept separate from other credentials tests, as it requires comparing
// // error messages returned by two different error states:
// // - When credentials = Null
// // - When credentials = Unknown

// t.Run("the same error is returned whether credentials is set as a null or unknown value (and access_token isn't set)", func(t *testing.T) {
// // Arrange
// acctest.UnsetTestProviderConfigEnvs(t)

// ctx := context.Background()
// tfVersion := "foobar"
// providerversion := "999"

// impersonateServiceAccountDelegates, _ := types.ListValue(types.StringType, []attr.Value{}) // empty list

// // Null data and error collection
// diagsNull := diag.Diagnostics{}
// dataNull := fwmodels.ProviderModel{
// Credentials: types.StringNull(),
// }
// dataNull.ImpersonateServiceAccountDelegates = impersonateServiceAccountDelegates

// // Unknown data and error collection
// diagsUnknown := diag.Diagnostics{}
// dataUnknown := fwmodels.ProviderModel{
// Credentials: types.StringUnknown(),
// }
// dataUnknown.ImpersonateServiceAccountDelegates = impersonateServiceAccountDelegates

// pNull := fwtransport.FrameworkProviderConfig{}
// pUnknown := fwtransport.FrameworkProviderConfig{}

// // Act
// pNull.LoadAndValidateFramework(ctx, &dataNull, tfVersion, &diagsNull, providerversion)
// pUnknown.LoadAndValidateFramework(ctx, &dataUnknown, tfVersion, &diagsUnknown, providerversion)

// // Assert
// if !diagsNull.HasError() {
// t.Fatalf("expect errors when credentials is null, but [%d] errors occurred", diagsNull.ErrorsCount())
// }
// if !diagsUnknown.HasError() {
// t.Fatalf("expect errors when credentials is unknown, but [%d] errors occurred", diagsUnknown.ErrorsCount())
// }

// errNull := diagsNull.Errors()
// errUnknown := diagsUnknown.Errors()
// for i := 0; i < len(errNull); i++ {
// if errNull[i] != errUnknown[i] {
// t.Fatalf("expect errors to be the same for null and unknown credentials values, instead got \nnull=`%s` \nunknown=%s", errNull[i], errUnknown[i])
// }
// }
// })
// }
func TestFrameworkProvider_LoadAndValidateFramework_credentials_unknown(t *testing.T) {
// This test case is kept separate from other credentials tests, as it requires comparing
// error messages returned by two different error states:
// - When credentials = Null
// - When credentials = Unknown

t.Run("the same error is returned whether credentials is set as a null or unknown value (and access_token isn't set)", func(t *testing.T) {

// Arrange
acctest.UnsetTestProviderConfigEnvs(t)

ctx := context.Background()
tfVersion := "foobar"
providerversion := "999"

impersonateServiceAccountDelegates, _ := types.ListValue(types.StringType, []attr.Value{}) // empty list

// Null data and error collection
diagsNull := diag.Diagnostics{}
dataNull := fwmodels.ProviderModel{
Credentials: types.StringNull(),
}
dataNull.ImpersonateServiceAccountDelegates = impersonateServiceAccountDelegates

// Unknown data and error collection
diagsUnknown := diag.Diagnostics{}
dataUnknown := fwmodels.ProviderModel{
Credentials: types.StringUnknown(),
}
dataUnknown.ImpersonateServiceAccountDelegates = impersonateServiceAccountDelegates

pNull := fwtransport.FrameworkProviderConfig{}
pUnknown := fwtransport.FrameworkProviderConfig{}

// Act
pNull.LoadAndValidateFramework(ctx, &dataNull, tfVersion, &diagsNull, providerversion)
pUnknown.LoadAndValidateFramework(ctx, &dataUnknown, tfVersion, &diagsUnknown, providerversion)

// Assert
if !diagsNull.HasError() {
t.Fatalf("expect errors when credentials is null, but [%d] errors occurred", diagsNull.ErrorsCount())
}
if !diagsUnknown.HasError() {
t.Fatalf("expect errors when credentials is unknown, but [%d] errors occurred", diagsUnknown.ErrorsCount())
}

errNull := diagsNull.Errors()
errUnknown := diagsUnknown.Errors()
for i := 0; i < len(errNull); i++ {
if errNull[i] != errUnknown[i] {
t.Fatalf("expect errors to be the same for null and unknown credentials values, instead got \nnull=`%s` \nunknown=%s", errNull[i], errUnknown[i])
}
}
})
}

func TestFrameworkProvider_LoadAndValidateFramework_billingProject(t *testing.T) {

Expand Down
6 changes: 2 additions & 4 deletions google-beta/provider/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -871,7 +871,6 @@ func DatasourceMapWithErrors() (map[string]*schema.Resource, error) {
"google_beyondcorp_app_gateway": beyondcorp.DataSourceGoogleBeyondcorpAppGateway(),
"google_billing_account": billing.DataSourceGoogleBillingAccount(),
"google_bigquery_default_service_account": bigquery.DataSourceGoogleBigqueryDefaultServiceAccount(),
"google_certificate_manager_certificate_map": certificatemanager.DataSourceGoogleCertificateManagerCertificateMap(),
"google_cloudbuild_trigger": cloudbuild.DataSourceGoogleCloudBuildTrigger(),
"google_cloudfunctions_function": cloudfunctions.DataSourceGoogleCloudFunctionsFunction(),
"google_cloudfunctions2_function": cloudfunctions2.DataSourceGoogleCloudFunctions2Function(),
Expand Down Expand Up @@ -1114,9 +1113,9 @@ func DatasourceMapWithErrors() (map[string]*schema.Resource, error) {
})
}

// Generated resources: 381
// Generated resources: 380
// Generated IAM resources: 237
// Total generated resources: 618
// Total generated resources: 617
func ResourceMap() map[string]*schema.Resource {
resourceMap, _ := ResourceMapWithErrors()
return resourceMap
Expand Down Expand Up @@ -1664,7 +1663,6 @@ func ResourceMapWithErrors() (map[string]*schema.Resource, error) {
"google_secret_manager_secret_version": secretmanager.ResourceSecretManagerSecretVersion(),
"google_scc_mute_config": securitycenter.ResourceSecurityCenterMuteConfig(),
"google_scc_notification_config": securitycenter.ResourceSecurityCenterNotificationConfig(),
"google_scc_project_custom_module": securitycenter.ResourceSecurityCenterProjectCustomModule(),
"google_scc_source": securitycenter.ResourceSecurityCenterSource(),
"google_scc_source_iam_binding": tpgiamresource.ResourceIamBinding(securitycenter.SecurityCenterSourceIamSchema, securitycenter.SecurityCenterSourceIamUpdaterProducer, securitycenter.SecurityCenterSourceIdParseFunc),
"google_scc_source_iam_member": tpgiamresource.ResourceIamMember(securitycenter.SecurityCenterSourceIamSchema, securitycenter.SecurityCenterSourceIamUpdaterProducer, securitycenter.SecurityCenterSourceIdParseFunc),
Expand Down

This file was deleted.

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,6 @@ func ResourceIdentityPlatformConfig() *schema.Resource {
},
"sign_in": {
Type: schema.TypeList,
Computed: true,
Optional: true,
Description: `Configuration related to local sign in methods.`,
MaxItems: 1,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,7 @@ resource "google_identity_platform_config" "default" {
}

func TestAccIdentityPlatformConfig_identityPlatformConfigMinimalExample(t *testing.T) {
acctest.SkipIfVcr(t)
t.Parallel()

context := map[string]interface{}{
Expand Down Expand Up @@ -163,10 +164,6 @@ resource "google_project_service" "identitytoolkit" {
resource "google_identity_platform_config" "default" {
project = google_project.default.project_id
depends_on = [
google_project_service.identitytoolkit
]
}
`, context)
}
Original file line number Diff line number Diff line change
Expand Up @@ -552,8 +552,7 @@ be executed directly, which will likely only succeed for scripts with shebang li
Type: schema.TypeString,
ValidateFunc: verify.ValidateEnum([]string{"CRITICAL", "SECURITY", "DEFINITION", "DRIVER", "FEATURE_PACK", "SERVICE_PACK", "TOOL", "UPDATE_ROLLUP", "UPDATE"}),
},
ConflictsWith: []string{"patch_config.0.windows_update.0.exclusive_patches"},
AtLeastOneOf: []string{"patch_config.0.windows_update.0.classifications", "patch_config.0.windows_update.0.excludes", "patch_config.0.windows_update.0.exclusive_patches"},
ExactlyOneOf: []string{"patch_config.0.windows_update.0.classifications", "patch_config.0.windows_update.0.excludes", "patch_config.0.windows_update.0.exclusive_patches"},
},
"excludes": {
Type: schema.TypeList,
Expand All @@ -563,8 +562,7 @@ be executed directly, which will likely only succeed for scripts with shebang li
Elem: &schema.Schema{
Type: schema.TypeString,
},
ConflictsWith: []string{"patch_config.0.windows_update.0.exclusive_patches"},
AtLeastOneOf: []string{"patch_config.0.windows_update.0.classifications", "patch_config.0.windows_update.0.excludes", "patch_config.0.windows_update.0.exclusive_patches"},
ExactlyOneOf: []string{"patch_config.0.windows_update.0.classifications", "patch_config.0.windows_update.0.excludes", "patch_config.0.windows_update.0.exclusive_patches"},
},
"exclusive_patches": {
Type: schema.TypeList,
Expand All @@ -575,8 +573,7 @@ This field must not be used with other patch configurations.`,
Elem: &schema.Schema{
Type: schema.TypeString,
},
ConflictsWith: []string{"patch_config.0.windows_update.0.classifications", "patch_config.0.windows_update.0.excludes"},
AtLeastOneOf: []string{"patch_config.0.windows_update.0.classifications", "patch_config.0.windows_update.0.excludes", "patch_config.0.windows_update.0.exclusive_patches"},
ExactlyOneOf: []string{"patch_config.0.windows_update.0.classifications", "patch_config.0.windows_update.0.excludes", "patch_config.0.windows_update.0.exclusive_patches"},
},
},
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -305,7 +305,7 @@ resource "google_os_config_patch_deployment" "patch" {
patch_config {
mig_instances_allowed = true
reboot_config = "ALWAYS"
apt {
Expand All @@ -329,7 +329,6 @@ resource "google_os_config_patch_deployment" "patch" {
windows_update {
classifications = ["CRITICAL", "SECURITY", "UPDATE"]
excludes = ["5012170"]
}
pre_step {
Expand Down
Loading

0 comments on commit 9c2b180

Please sign in to comment.