From ce50fe4166fd33e4bc2f6941b39ad30c4516a38e Mon Sep 17 00:00:00 2001 From: The Magician Date: Fri, 7 Feb 2025 14:47:29 -0800 Subject: [PATCH] Implemented exponential backoff for IAM bootstrapping (#13001) (#9270) [upstream:0a15a64f4dea8f148ce70cdc25b64ff97a1927b1] Signed-off-by: Modular Magician --- .changelog/13001.txt | 3 ++ .../acctest/bootstrap_iam_test_utils.go | 53 +++++++++++-------- 2 files changed, 35 insertions(+), 21 deletions(-) create mode 100644 .changelog/13001.txt diff --git a/.changelog/13001.txt b/.changelog/13001.txt new file mode 100644 index 0000000000..42b910df15 --- /dev/null +++ b/.changelog/13001.txt @@ -0,0 +1,3 @@ +```release-note:none + +``` \ No newline at end of file diff --git a/google-beta/acctest/bootstrap_iam_test_utils.go b/google-beta/acctest/bootstrap_iam_test_utils.go index be459dd725..dac7a7e0bf 100644 --- a/google-beta/acctest/bootstrap_iam_test_utils.go +++ b/google-beta/acctest/bootstrap_iam_test_utils.go @@ -3,8 +3,6 @@ package acctest import ( - "fmt" - "log" "strconv" "strings" "testing" @@ -12,6 +10,8 @@ import ( "github.com/hashicorp/terraform-provider-google-beta/google-beta/envvar" "github.com/hashicorp/terraform-provider-google-beta/google-beta/tpgiamresource" + "github.com/hashicorp/terraform-provider-google-beta/google-beta/tpgresource" + transport_tpg "github.com/hashicorp/terraform-provider-google-beta/google-beta/transport" cloudresourcemanager "google.golang.org/api/cloudresourcemanager/v1" ) @@ -36,12 +36,6 @@ func BootstrapIamMembers(t *testing.T, members []IamMember) { t.Fatalf("Error getting project with id %q: %s", project.ProjectId, err) } - getPolicyRequest := &cloudresourcemanager.GetIamPolicyRequest{} - policy, err := client.Projects.GetIamPolicy(project.ProjectId, getPolicyRequest).Do() - if err != nil { - t.Fatalf("Error getting project iam policy: %v", err) - } - // Create the bindings we need to add to the policy. var newBindings []*cloudresourcemanager.Binding for _, member := range members { @@ -51,26 +45,43 @@ func BootstrapIamMembers(t *testing.T, members []IamMember) { }) } - mergedBindings := tpgiamresource.MergeBindings(append(policy.Bindings, newBindings...)) + // Retry bootstrapping with exponential backoff for concurrent writes + backoff := time.Second + for { + getPolicyRequest := &cloudresourcemanager.GetIamPolicyRequest{} + policy, err := client.Projects.GetIamPolicy(project.ProjectId, getPolicyRequest).Do() + if transport_tpg.IsGoogleApiErrorWithCode(err, 429) { + t.Logf("[DEBUG] 429 while attempting to read policy for project %s, waiting %v before attempting again", project.ProjectId, backoff) + time.Sleep(backoff) + continue + } else if err != nil { + t.Fatalf("Error getting iam policy for project %s: %v\n", project.ProjectId, err) + } + + mergedBindings := tpgiamresource.MergeBindings(append(policy.Bindings, newBindings...)) - if !tpgiamresource.CompareBindings(policy.Bindings, mergedBindings) { - addedBindings := tpgiamresource.MissingBindings(policy.Bindings, mergedBindings) - for _, missingBinding := range addedBindings { - log.Printf("[DEBUG] Adding binding: %+v", missingBinding) + if tpgiamresource.CompareBindings(policy.Bindings, mergedBindings) { + t.Logf("[DEBUG] All bindings already present for project %s", project.ProjectId) + break } // The policy must change. policy.Bindings = mergedBindings setPolicyRequest := &cloudresourcemanager.SetIamPolicyRequest{Policy: policy} policy, err = client.Projects.SetIamPolicy(project.ProjectId, setPolicyRequest).Do() - if err != nil { - t.Fatalf("Error setting project iam policy: %v", err) + if err == nil { + t.Logf("[DEBUG] Waiting for IAM bootstrapping to propagate for project %s.", project.ProjectId) + time.Sleep(3 * time.Minute) + break } - msg := "Added the following bindings to the test project's IAM policy:\n" - for _, binding := range addedBindings { - msg += fmt.Sprintf("Members: %q, Role: %q\n", binding.Members, binding.Role) + if tpgresource.IsConflictError(err) { + t.Logf("[DEBUG]: Concurrent policy changes, restarting read-modify-write after %s", backoff) + time.Sleep(backoff) + backoff = backoff * 2 + if backoff > 30*time.Second { + t.Fatalf("Error applying IAM policy to %s: Too many conflicts. Latest error: %s", project.ProjectId, err) + } + continue } - msg += "Waiting for IAM to propagate." - t.Log(msg) - time.Sleep(3 * time.Minute) + t.Fatalf("Error setting project iam policy: %v", err) } }