Merge pull request #121 from rlaakkol/add-auth-retry-option

Add option to retry on auth errors

Author: ross

CHANGELOG.md

@@ -1,3 +1,7 @@
+## v0.0.8 - 20??-??-?? - ???
+
+* Add support for optionally retrying requests that hit 403 errors
+
 ## v0.0.7 - 2024-08-20 - DS always come second
 
 * Create DS records after their sibling NS records to appease Cloudflare's

README.md

@@ -57,6 +57,9 @@ providers:
     # Optional. Default: 4. Number of times to retry if a 429 response
     # is received.
     #retry_count: 4
+    # Optional. Default: 0. Number of times to retry if a 403 response
+    # is received.
+    #auth_error_retry_count: 0
     # Optional. Default: 300. Number of seconds to wait before retrying.
     #retry_period: 300
     # Optional. Default: 50. Number of zones per page.

octodns_cloudflare/__init__.py

@@ -91,6 +91,7 @@ def __init__(
         pagerules=True,
         retry_count=4,
         retry_period=300,
+        auth_error_retry_count=0,
         zones_per_page=50,
         records_per_page=100,
         min_ttl=120,
@@ -124,6 +125,7 @@ def __init__(
         self.pagerules = pagerules
         self.retry_count = retry_count
         self.retry_period = retry_period
+        self.auth_error_retry_count = auth_error_retry_count
         self.zones_per_page = zones_per_page
         self.records_per_page = records_per_page
         self.min_ttl = min_ttl
@@ -140,6 +142,7 @@ def __init__(
 
     def _try_request(self, *args, **kwargs):
         tries = self.retry_count
+        auth_tries = self.auth_error_retry_count
         while True:  # We'll raise to break after our tries expire
             try:
                 return self._request(*args, **kwargs)
@@ -154,6 +157,17 @@ def _try_request(self, *args, **kwargs):
                     tries,
                 )
                 sleep(self.retry_period)
+            except CloudflareAuthenticationError:
+                if auth_tries <= 0:
+                    raise
+                auth_tries -= 1
+                self.log.warning(
+                    'authentication error encountered, pausing '
+                    'for %ds and trying again, %d remaining',
+                    self.retry_period,
+                    auth_tries,
+                )
+                sleep(self.retry_period)
 
     def _request(self, method, path, params=None, data=None):
         self.log.debug('_request: method=%s, path=%s', method, path)

tests/test_octodns_provider_cloudflare.py

@@ -17,7 +17,11 @@
 from octodns.record import Create, Delete, Record, Update
 from octodns.zone import Zone
 
-from octodns_cloudflare import CloudflareProvider, CloudflareRateLimitError
+from octodns_cloudflare import (
+    CloudflareAuthenticationError,
+    CloudflareProvider,
+    CloudflareRateLimitError,
+)
 
 
 def set_record_proxied_flag(record, proxied):
@@ -2177,7 +2181,11 @@ def test_emailless_auth(self):
 
     def test_retry_behavior(self):
         provider = CloudflareProvider(
-            'test', token='token 123', email='email 234', retry_period=0
+            'test',
+            token='token 123',
+            email='email 234',
+            retry_period=0,
+            auth_error_retry_count=2,  # Add auth retry config
         )
         result = {
             "success": True,
@@ -2198,7 +2206,7 @@ def test_retry_behavior(self):
             [call('GET', '/zones', params={'page': 1, 'per_page': 50})]
         )
 
-        # One retry required
+        # One rate limit retry required
         provider._zones = None
         provider._request.reset_mock()
         provider._request.side_effect = [CloudflareRateLimitError('{}'), result]
@@ -2207,20 +2215,32 @@ def test_retry_behavior(self):
             [call('GET', '/zones', params={'page': 1, 'per_page': 50})]
         )
 
-        # Two retries required
+        # One auth retry required
+        provider._zones = None
+        provider._request.reset_mock()
+        provider._request.side_effect = [
+            CloudflareAuthenticationError('{}'),
+            result,
+        ]
+        self.assertEqual([], provider.zone_records(zone))
+        provider._request.assert_has_calls(
+            [call('GET', '/zones', params={'page': 1, 'per_page': 50})]
+        )
+
+        # Two retries required - mixed rate limit and auth errors
         provider._zones = None
         provider._request.reset_mock()
         provider._request.side_effect = [
             CloudflareRateLimitError('{}'),
-            CloudflareRateLimitError('{}'),
+            CloudflareAuthenticationError('{}'),
             result,
         ]
         self.assertEqual([], provider.zone_records(zone))
         provider._request.assert_has_calls(
             [call('GET', '/zones', params={'page': 1, 'per_page': 50})]
         )
 
-        # # Exhaust our retries
+        # Exhaust rate limit retries
         provider._zones = None
         provider._request.reset_mock()
         provider._request.side_effect = [
@@ -2234,6 +2254,29 @@ def test_retry_behavior(self):
             provider.zone_records(zone)
             self.assertEqual('last', str(ctx.exception))
 
+        # Exhaust auth retries
+        provider._zones = None
+        provider._request.reset_mock()
+        provider._request.side_effect = [
+            CloudflareAuthenticationError({"errors": [{"message": "first"}]}),
+            CloudflareAuthenticationError({"errors": [{"message": "second"}]}),
+            CloudflareAuthenticationError({"errors": [{"message": "last"}]}),
+        ]
+        with self.assertRaises(CloudflareAuthenticationError) as ctx:
+            provider.zone_records(zone)
+            self.assertEqual('last', str(ctx.exception))
+
+        # Test with auth retries disabled (default behavior)
+        provider = CloudflareProvider(
+            'test', token='token 123', email='email 234', retry_period=0
+        )
+        provider._request = Mock()
+        provider._zones = None
+        provider._request.side_effect = [CloudflareAuthenticationError('{}')]
+        with self.assertRaises(CloudflareAuthenticationError):
+            provider.zone_records(zone)
+        self.assertEqual(1, provider._request.call_count)
+
     def test_ttl_mapping(self):
         provider = CloudflareProvider('test', 'email', 'token')