diff --git a/mmv1/products/healthcare/terraform.yaml b/mmv1/products/healthcare/terraform.yaml index c7a2ea01d731..c47422024bb4 100644 --- a/mmv1/products/healthcare/terraform.yaml +++ b/mmv1/products/healthcare/terraform.yaml @@ -60,7 +60,7 @@ overrides: !ruby/object:Overrides::ResourceOverrides pubsub_topic: "fhir-notifications" bq_dataset_name: "bq_example_dataset" test_vars_overrides: - policyChanged: 'BootstrapPSARoles(t, "gsp-sa-healthcare", []string{"roles/bigquery.dataEditor", "roles/bigquery.jobUser"})' + policyChanged: 'BootstrapPSARoles(t, "gcp-sa-healthcare", []string{"roles/bigquery.dataEditor", "roles/bigquery.jobUser"})' - !ruby/object:Provider::Terraform::Examples name: "healthcare_fhir_store_notification_config" primary_resource_id: "default" @@ -110,7 +110,7 @@ overrides: !ruby/object:Overrides::ResourceOverrides bq_dataset_name: "dicom_bq_ds" bq_table_name: "dicom_bq_tb" test_vars_overrides: - policyChanged: 'BootstrapPSARoles(t, "gsp-sa-healthcare", []string{"roles/bigquery.dataEditor", "roles/bigquery.jobUser"})' + policyChanged: 'BootstrapPSARoles(t, "gcp-sa-healthcare", []string{"roles/bigquery.dataEditor", "roles/bigquery.jobUser"})' properties: creationTime: !ruby/object:Overrides::Terraform::PropertyOverride exclude: true diff --git a/mmv1/third_party/terraform/tests/resource_composer_environment_test.go.erb b/mmv1/third_party/terraform/tests/resource_composer_environment_test.go.erb index a5c2b2d57405..0b61d8d2592f 100644 --- a/mmv1/third_party/terraform/tests/resource_composer_environment_test.go.erb +++ b/mmv1/third_party/terraform/tests/resource_composer_environment_test.go.erb @@ -342,6 +342,7 @@ func TestAccComposerEnvironment_withEncryptionConfigComposer1(t *testing.T) { kms := BootstrapKMSKeyInLocation(t, "us-central1") pid := GetTestProjectFromEnv() + grantEncrypterDecrypterRoleToServiceAgents(t) envName := fmt.Sprintf("%s-%d", testComposerEnvironmentPrefix, RandInt(t)) network := fmt.Sprintf("%s-%d", testComposerNetworkPrefix, RandInt(t)) subnetwork := network + "-1" @@ -377,6 +378,7 @@ func TestAccComposerEnvironment_withEncryptionConfigComposer2(t *testing.T) { kms := BootstrapKMSKeyInLocation(t, "us-central1") pid := GetTestProjectFromEnv() + grantEncrypterDecrypterRoleToServiceAgents(t) envName := fmt.Sprintf("%s-%d", testComposerEnvironmentPrefix, RandInt(t)) network := fmt.Sprintf("%s-%d", testComposerNetworkPrefix, RandInt(t)) subnetwork := network + "-1" @@ -960,6 +962,22 @@ func TestAccComposerEnvironment_fixPyPiPackages(t *testing.T) { }) } +// This bootstraps the IAM roles needed for the service agents when using encryption. +func grantEncrypterDecrypterRoleToServiceAgents(t *testing.T) { + serviceAgents := []string{ + "cloudcomposer-accounts", + "compute-system", + "container-engine-robot", + "gcp-sa-artifactregistry", + "gcp-sa-pubsub", + } + role := "roles/cloudkms.cryptoKeyEncrypterDecrypter" + if policyChanged := BootstrapAllPSARole(t, serviceAgents, role); policyChanged { + // Fail this test run because the policy needs time to reconcile. + t.Fatalf("Granted crypto key encrypter/decrypter role for service agents %v in test project's IAM policy. Retry the test in a few minutes.", serviceAgents) + } +} + func testAccComposerEnvironmentDestroyProducer(t *testing.T) func(s *terraform.State) error { return func(s *terraform.State) error { config := GoogleProviderConfig(t) @@ -2325,6 +2343,7 @@ resource "google_project_iam_member" "composer-worker" { `, environment, network, subnetwork, serviceAccount) } + /** * CLEAN UP HELPER FUNCTIONS * Because the environments are flaky and bucket deletion rates can be diff --git a/mmv1/third_party/terraform/utils/bootstrap_utils_test.go b/mmv1/third_party/terraform/utils/bootstrap_utils_test.go index bcb464bb4d36..8571e7e2ce90 100644 --- a/mmv1/third_party/terraform/utils/bootstrap_utils_test.go +++ b/mmv1/third_party/terraform/utils/bootstrap_utils_test.go @@ -561,7 +561,11 @@ func BootstrapAllPSARoles(t *testing.T, agentNames, roles []string) bool { mergedBindings := MergeBindings(append(policy.Bindings, newBindings...)) if !compareBindings(policy.Bindings, mergedBindings) { + for _, missingBinding := range missingBindings(policy.Bindings, mergedBindings) { + log.Printf("[DEBUG] Missing binding: %v", missingBinding) + } // The policy must change. + policy.Bindings = mergedBindings setPolicyRequest := &cloudresourcemanager.SetIamPolicyRequest{Policy: policy} policy, err = client.Projects.SetIamPolicy(project.ProjectId, setPolicyRequest).Do() if err != nil { diff --git a/mmv1/third_party/terraform/utils/iam.go.erb b/mmv1/third_party/terraform/utils/iam.go.erb index 3b3bb45f45d9..8a08ce36b796 100644 --- a/mmv1/third_party/terraform/utils/iam.go.erb +++ b/mmv1/third_party/terraform/utils/iam.go.erb @@ -448,6 +448,65 @@ func compareBindings(a, b []*cloudresourcemanager.Binding) bool { return reflect.DeepEqual(aMap, bMap) } +// Returns a map representing iam bindings that are in one map but not the other. +func missingBindingsMap(aMap, bMap map[iamBindingKey]map[string]struct{}) map[iamBindingKey]map[string]struct{} { + results := make(map[iamBindingKey]map[string]struct{}) + for key, aMembers := range aMap { + if bMembers, ok := bMap[key]; ok { + // The key is in both maps. + resultMembers := make(map[string]struct{}) + + for aMember := range aMembers { + if _, ok := bMembers[aMember]; !ok { + // The member is in a but not in b. + resultMembers[aMember] = struct{}{} + } + } + for bMember := range bMembers { + if _, ok := aMembers[bMember]; !ok { + // The member is in b but not in a. + resultMembers[bMember] = struct{}{} + } + } + + if len(resultMembers) > 0 { + results[key] = resultMembers + } + } else { + // The key is in map a but not map b. + results[key] = aMembers + } + } + + for key, bMembers := range bMap { + if _, ok := aMap[key]; !ok { + // The key is in map b but not map a. + results[key] = bMembers + } + } + + return results +} + +// Returns the bindings that are in one set of bindings and not the other. +func missingBindings(a, b []*cloudresourcemanager.Binding) []*cloudresourcemanager.Binding { + aMap := createIamBindingsMap(a) + bMap := createIamBindingsMap(b) + + var results []*cloudresourcemanager.Binding + for key, membersSet := range missingBindingsMap(aMap, bMap) { + members := make([]string, 0, len(membersSet)) + for member := range membersSet { + members = append(members, member) + } + results = append(results, &cloudresourcemanager.Binding{ + Role: key.Role, + Members: members, + }) + } + return results +} + func compareAuditConfigs(a, b []*cloudresourcemanager.AuditConfig) bool { aMap := createIamAuditConfigsMap(a) bMap := createIamAuditConfigsMap(b)