diff --git a/google/iam.go b/google/iam.go index 869bdb60149..6da3242a3ae 100644 --- a/google/iam.go +++ b/google/iam.go @@ -10,6 +10,8 @@ import ( "google.golang.org/api/cloudresourcemanager/v1" ) +const maxBackoffSeconds = 30 + // The ResourceIamUpdater interface is implemented for each GCP resource supporting IAM policy. // // Implementations should keep track of the resource identifier. @@ -44,6 +46,26 @@ type iamPolicyModifyFunc func(p *cloudresourcemanager.Policy) error type resourceIdParserFunc func(d *schema.ResourceData, config *Config) error +// Wrapper around updater.GetResourceIamPolicy() that handles Fibonacci backoff +// for reading policies from IAM +func iamPolicyReadWithRetry(updater ResourceIamUpdater) (*cloudresourcemanager.Policy, error) { + mutexKey := updater.GetMutexKey() + mutexKV.Lock(mutexKey) + defer mutexKV.Unlock(mutexKey) + + log.Printf("[DEBUG] Retrieving policy for %s\n", updater.DescribeResource()) + var policy *cloudresourcemanager.Policy + err := retryTime(func() (perr error) { + policy, perr = updater.GetResourceIamPolicy() + return perr + }, 10) + if err != nil { + return nil, err + } + log.Printf("[DEBUG] Retrieved policy for %s: %+v\n", updater.DescribeResource(), policy) + return policy, nil +} + func iamPolicyReadModifyWrite(updater ResourceIamUpdater, modify iamPolicyModifyFunc) error { mutexKey := updater.GetMutexKey() mutexKV.Lock(mutexKey) @@ -54,6 +76,7 @@ func iamPolicyReadModifyWrite(updater ResourceIamUpdater, modify iamPolicyModify log.Printf("[DEBUG]: Retrieving policy for %s\n", updater.DescribeResource()) p, err := updater.GetResourceIamPolicy() if isGoogleApiErrorWithCode(err, 429) { + log.Printf("[DEBUG] 429 while attempting to read policy for %s, waiting %v before attempting again", updater.DescribeResource(), backoff) time.Sleep(backoff) continue } else if err != nil { @@ -71,7 +94,7 @@ func iamPolicyReadModifyWrite(updater ResourceIamUpdater, modify iamPolicyModify if err == nil { fetchBackoff := 1 * time.Second for successfulFetches := 0; successfulFetches < 3; { - if fetchBackoff > 30*time.Second { + if fetchBackoff > maxBackoffSeconds*time.Second { return fmt.Errorf("Error applying IAM policy to %s: Waited too long for propagation.\n", updater.DescribeResource()) } time.Sleep(fetchBackoff) diff --git a/google/iam_test.go b/google/iam_test.go new file mode 100644 index 00000000000..72097dc6f3e --- /dev/null +++ b/google/iam_test.go @@ -0,0 +1,63 @@ +package google + +import ( + "google.golang.org/api/cloudresourcemanager/v1" + "google.golang.org/api/googleapi" + "testing" +) + +// Mock ResourceIamUpdater +type readTestUpdater struct { + errorCode int + returnAfterAttempts int + numAttempts int +} + +// Not being tested +func (u *readTestUpdater) SetResourceIamPolicy(p *cloudresourcemanager.Policy) error { return nil } +func (u *readTestUpdater) GetMutexKey() string { return "a-resource-key" } +func (u *readTestUpdater) GetResourceId() string { return "id" } +func (u *readTestUpdater) DescribeResource() string { return "mock updater" } + +// Fetch the existing IAM policy attached to a resource. +func (u *readTestUpdater) GetResourceIamPolicy() (*cloudresourcemanager.Policy, error) { + u.numAttempts++ + if u.numAttempts < u.returnAfterAttempts { + // Indicate retry + return nil, &googleapi.Error{Code: 429} + } + + // Was testing successes or was testing retryable + if u.errorCode == 200 { + return &cloudresourcemanager.Policy{}, nil + } + return nil, &googleapi.Error{Code: u.errorCode} +} + +func TestIamPolicyReadWithRetry_returnImmediately(t *testing.T) { + mockUpdater := &readTestUpdater{ + returnAfterAttempts: 1, + errorCode: 200, + } + p, err := iamPolicyReadWithRetry(mockUpdater) + if err != nil || p == nil { + t.Errorf("expected valid policy and no error, got nil policy and error %v", err) + } + if mockUpdater.numAttempts != 1 { + t.Errorf("expected GetResourceIamPolicy to have been called once") + } +} + +func TestIamPolicyReadWithRetry_retry(t *testing.T) { + mockUpdater := &readTestUpdater{ + returnAfterAttempts: 3, + errorCode: 404, + } + p, err := iamPolicyReadWithRetry(mockUpdater) + if err == nil || !isGoogleApiErrorWithCode(err, 404) { + t.Errorf("expected googleapi error 404, got policy %v, err %v", p, err) + } + if mockUpdater.numAttempts != mockUpdater.returnAfterAttempts { + t.Errorf("expected GetResourceIamPolicy to have been called %d times, was called %d", mockUpdater.numAttempts, mockUpdater.returnAfterAttempts) + } +} diff --git a/google/resource_iam_audit_config.go b/google/resource_iam_audit_config.go index 77efa7a2026..9f2010cc30e 100644 --- a/google/resource_iam_audit_config.go +++ b/google/resource_iam_audit_config.go @@ -86,7 +86,7 @@ func resourceIamAuditConfigRead(newUpdaterFunc newResourceIamUpdaterFunc) schema } eAuditConfig := getResourceIamAuditConfig(d) - p, err := updater.GetResourceIamPolicy() + p, err := iamPolicyReadWithRetry(updater) if err != nil { if isGoogleApiErrorWithCode(err, 404) { log.Printf("[DEBUG]: AuditConfig for service %q not found for non-existent resource %s, removing from state file.", eAuditConfig.Service, updater.DescribeResource()) diff --git a/google/resource_iam_binding.go b/google/resource_iam_binding.go index 5a8d072adfc..099bdf9f69a 100644 --- a/google/resource_iam_binding.go +++ b/google/resource_iam_binding.go @@ -77,7 +77,7 @@ func resourceIamBindingRead(newUpdaterFunc newResourceIamUpdaterFunc) schema.Rea } eBinding := getResourceIamBinding(d) - p, err := updater.GetResourceIamPolicy() + p, err := iamPolicyReadWithRetry(updater) if err != nil { if isGoogleApiErrorWithCode(err, 404) { log.Printf("[DEBUG]: Binding for role %q not found for non-existent resource %s, removing from state file.", updater.DescribeResource(), eBinding.Role) diff --git a/google/resource_iam_member.go b/google/resource_iam_member.go index 76f48bcd1da..d1a37544cae 100644 --- a/google/resource_iam_member.go +++ b/google/resource_iam_member.go @@ -112,7 +112,7 @@ func resourceIamMemberRead(newUpdaterFunc newResourceIamUpdaterFunc) schema.Read } eMember := getResourceIamMember(d) - p, err := updater.GetResourceIamPolicy() + p, err := iamPolicyReadWithRetry(updater) if err != nil { if isGoogleApiErrorWithCode(err, 404) { log.Printf("[DEBUG]: Binding of member %q with role %q does not exist for non-existent resource %s, removing from state.", eMember.Members[0], eMember.Role, updater.DescribeResource()) diff --git a/google/resource_iam_policy.go b/google/resource_iam_policy.go index 0b7408c9b9e..25d84c89a9a 100644 --- a/google/resource_iam_policy.go +++ b/google/resource_iam_policy.go @@ -81,7 +81,7 @@ func ResourceIamPolicyRead(newUpdaterFunc newResourceIamUpdaterFunc) schema.Read return err } - policy, err := updater.GetResourceIamPolicy() + policy, err := iamPolicyReadWithRetry(updater) if err != nil { if isGoogleApiErrorWithCode(err, 404) { log.Printf("[DEBUG]: Policy does not exist for non-existent resource %q", updater.GetResourceId())