Add retries/backoff when (only) reading IAM policies

Signed-off-by: Modular Magician <[email protected]>

Author: emilymye

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,41 @@ 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)
+
+	backoff := time.Second
+	prevBackoff := time.Duration(0)
+
+	for {
+		log.Printf("[DEBUG] Retrieving policy for %s\n", updater.DescribeResource())
+		p, err := updater.GetResourceIamPolicy()
+		if err == nil {
+			log.Printf("[DEBUG] Retrieved policy for %s: %+v\n", updater.DescribeResource(), p)
+			return p, nil
+		}
+
+		if !isGoogleApiErrorWithCode(err, 429) {
+			return nil, err
+		}
+
+		if backoff > time.Second*maxBackoffSeconds {
+			return nil, fmt.Errorf("Error applying IAM policy to %s: Waited too long for read.\n", updater.DescribeResource())
+		}
+
+		log.Printf("[DEBUG] 429 while attempting to read policy for %s, waiting %v before attempting again", updater.DescribeResource(), backoff)
+		time.Sleep(backoff)
+		// Fibonacci increase backoff
+		newBackoff := backoff + prevBackoff
+		prevBackoff = backoff
+		backoff = newBackoff
+	}
+}
+
 func iamPolicyReadModifyWrite(updater ResourceIamUpdater, modify iamPolicyModifyFunc) error {
 	mutexKey := updater.GetMutexKey()
 	mutexKV.Lock(mutexKey)
@@ -54,6 +91,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 +109,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)

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)
+	}
+}

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())

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)

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())

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())