Skip to content

Commit

Permalink
Bootstrap required permissions for composer environment tests (Google…
Browse files Browse the repository at this point in the history
…CloudPlatform#7391)

* Bootstrap the required permissions

* Make BootstrapAllPSARoles actually work

Also adds some helper functions for debugging what the bootstrap
function does. It will now log the roles that were missing in the
policy.

* Add missing return, fix nits

* Fix typo in service agent name

* Account for newly exported test functions
  • Loading branch information
trodge authored and ericayyliu committed Jul 26, 2023
1 parent 5e5e693 commit 2a5027d
Show file tree
Hide file tree
Showing 4 changed files with 84 additions and 2 deletions.
4 changes: 2 additions & 2 deletions mmv1/products/healthcare/terraform.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions mmv1/third_party/terraform/utils/bootstrap_utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -570,7 +570,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 {
Expand Down
59 changes: 59 additions & 0 deletions mmv1/third_party/terraform/utils/iam.go.erb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down

0 comments on commit 2a5027d

Please sign in to comment.