aws: retries, timeouts and SDK errors, taken over from the #603 client

module/sources/aws/client.py

@@ -19,15 +19,21 @@
 """
 from contextlib import contextmanager
 from dataclasses import dataclass
+from functools import wraps
 from typing import Optional
 
 try:
     import boto3
-    from botocore.exceptions import ClientError
+    from botocore.config import Config
+    from botocore.exceptions import BotoCoreError, ClientError
 except ImportError:  # pragma: no cover - exercised via monkeypatching boto3 = None
     boto3 = None
+    Config = None
     ClientError = Exception
 
+    class BotoCoreError(Exception):
+        pass
+
 from module.common.logging import get_logger
 
 log = get_logger()
@@ -43,11 +49,29 @@
 
 MAX_ITEMS_PER_PAGE = 20
 
+# Every client retries throttled and transient failures with backoff and gives up on an
+# unreachable endpoint after 10 seconds instead of the botocore default of 60.
+BOTO_CONFIG = Config(
+    retries={"mode": "standard", "max_attempts": 5}, connect_timeout=10, read_timeout=60
+) if Config is not None else None
+
 
 class AWSDiscoveryError(Exception):
     """A fatal AWS discovery failure. The caller must not write to NetBox."""
 
 
+def fatal_on_sdk_error(func):
+    """Turn an SDK level failure (no route to the endpoint, timeout, missing credentials, ...)
+    into the AWSDiscoveryError the handler knows how to deal with."""
+    @wraps(func)
+    def wrapper(*args, **kwargs):
+        try:
+            return func(*args, **kwargs)
+        except BotoCoreError as exc:
+            raise AWSDiscoveryError(f"Incomplete AWS discovery: {exc}") from exc
+    return wrapper
+
+
 @dataclass(frozen=True)
 class AWSAccessCheck:
     account: Optional[str]
@@ -71,7 +95,10 @@ def __init__(self, profile=None):
                 "the 'boto3' package is required for the AWS source; "
                 "install it with 'pip install -r requirements-aws.txt'"
             )
-        self.session = boto3.Session(profile_name=profile) if profile else boto3.Session()
+        try:
+            self.session = boto3.Session(profile_name=profile) if profile else boto3.Session()
+        except BotoCoreError as exc:
+            raise AWSDiscoveryError(f"Unable to initialize AWS credentials: {exc}") from exc
         self.access_denied = []
         self.access_checks = []
         self.account_names = {}
@@ -168,7 +195,7 @@ def _bootstrap_region(self, regions, discovery_region):
         return regions[0]
 
     def _get_identity(self, bootstrap_region, account, sts=None):
-        sts = sts or self.session.client("sts", region_name=bootstrap_region)
+        sts = sts or self.session.client("sts", region_name=bootstrap_region, config=BOTO_CONFIG)
         identity_account = None
         with self.tolerate_access_denied(None, None, "account"):
             response = sts.get_caller_identity()
@@ -183,7 +210,7 @@ def _get_identity(self, bootstrap_region, account, sts=None):
         return identity_account
 
     def _enabled_regions(self, bootstrap_region, account):
-        ec2 = self.session.client("ec2", region_name=bootstrap_region)
+        ec2 = self.session.client("ec2", region_name=bootstrap_region, config=BOTO_CONFIG)
         enabled = set()
         denied_scope = (account, None, "ec2")
         with self.tolerate_access_denied(*denied_scope):
@@ -199,6 +226,7 @@ def _enabled_regions(self, bootstrap_region, account):
 
     # -- single-account discovery --------------------------------------------
 
+    @fatal_on_sdk_error
     def discover(self, regions, account=None, discovery_region=None, services=()):
         # Starts a fresh access_denied/access_checks run so a prior denial does
         # not linger forever once access is restored. Member clients created by
@@ -221,7 +249,7 @@ def discover(self, regions, account=None, discovery_region=None, services=()):
         return identity_account, snapshot
 
     def _discover_region(self, region, account, services):
-        ec2 = self.session.client("ec2", region_name=region)
+        ec2 = self.session.client("ec2", region_name=region, config=BOTO_CONFIG)
         data = {
             "instances": [], "InstanceTypes": {}, "NetworkInterfaces": {},
             "Volumes": {}, "Subnets": {}, "Vpcs": {},
@@ -248,23 +276,23 @@ def _discover_region(self, region, account, services):
         if "s3" in services:
             buckets = self._tolerated_list(self.discover_s3, account, region, "s3", ec2.meta.region_name
                                             if False else None,
-                                            args=(self.session.client("s3", region_name=region), region, account))
+                                            args=(self.session.client("s3", region_name=region, config=BOTO_CONFIG), region, account))
             if buckets is None:
                 inaccessible.append("s3")
                 data["Buckets"] = []
             else:
                 data["Buckets"] = buckets
         if "lambda" in services:
             functions = self._tolerated_list(self.discover_lambda, account, region, "lambda", None,
-                                             args=(self.session.client("lambda", region_name=region), region, account))
+                                             args=(self.session.client("lambda", region_name=region, config=BOTO_CONFIG), region, account))
             if functions is None:
                 inaccessible.append("lambda")
                 data["Functions"] = []
             else:
                 data["Functions"] = functions
         if "eks" in services:
             clusters = self._tolerated_list(self.discover_eks, account, region, "eks", None,
-                                            args=(self.session.client("eks", region_name=region), region, account))
+                                            args=(self.session.client("eks", region_name=region, config=BOTO_CONFIG), region, account))
             if clusters is None:
                 inaccessible.append("eks")
                 data["EKSClusters"] = []
@@ -474,17 +502,18 @@ def _paginate_token(self, func, key, token_key):
 
     # -- organization ---------------------------------------------------------
 
+    @fatal_on_sdk_error
     def discover_organization(self, regions, role_name, expected_account=None, discovery_region=None,
                                include_management=False, external_id=None, account_ids=None, services=()):
         self.access_denied = []
         self.access_checks = []
         bootstrap_region = self._bootstrap_region(regions, discovery_region)
-        sts = self.session.client("sts", region_name=bootstrap_region)
+        sts = self.session.client("sts", region_name=bootstrap_region, config=BOTO_CONFIG)
         management_account = self._get_identity(bootstrap_region, expected_account, sts=sts)
         if management_account is None:
             return {}
 
-        organizations = self.session.client("organizations", region_name=bootstrap_region)
+        organizations = self.session.client("organizations", region_name=bootstrap_region, config=BOTO_CONFIG)
         org_info = None
         with self.tolerate_access_denied(management_account, None, "organizations"):
             org_info = organizations.describe_organization()["Organization"]

tests/test_aws_client_config.py

@@ -0,0 +1,65 @@
+# -*- coding: utf-8 -*-
+#  Copyright (c) 2020 - 2026 netbox-sync team. All rights reserved.
+#
+#  netbox-sync.py
+#
+#  This work is licensed under the terms of the MIT license.
+#  For a copy, see file LICENSE.txt included in this
+#  repository or visit: <https://opensource.org/licenses/MIT>.
+
+"""Every boto3 client of the AWS source retries with backoff and has a short connect timeout, and
+failures of the SDK itself end up as AWSDiscoveryError like every other discovery failure."""
+
+import ast
+from pathlib import Path
+from unittest.mock import Mock
+
+import pytest
+from botocore.exceptions import EndpointConnectionError, ProfileNotFound
+
+import module.sources.aws.client as aws_client
+from module.sources.aws.client import AWSClient, AWSDiscoveryError
+
+
+def test_boto_config_values():
+    config = aws_client.BOTO_CONFIG
+
+    assert config.retries == {"mode": "standard", "max_attempts": 5}
+    assert config.connect_timeout == 10
+    assert config.read_timeout == 60
+
+
+def test_every_client_is_created_with_the_config():
+    tree = ast.parse(Path(aws_client.__file__).read_text(encoding="utf-8"))
+
+    calls = [
+        node for node in ast.walk(tree)
+        if isinstance(node, ast.Call)
+        and isinstance(node.func, ast.Attribute) and node.func.attr == "client"
+        and isinstance(node.func.value, ast.Attribute) and node.func.value.attr == "session"
+    ]
+
+    assert calls, "no session.client() calls found, the check needs updating"
+    without_config = [call.lineno for call in calls if "config" not in {kw.arg for kw in call.keywords}]
+    assert not without_config, f"session.client() without config= at lines {without_config}"
+
+
+@pytest.mark.parametrize("method", ["discover", "discover_organization"])
+def test_sdk_error_during_discovery_is_a_discovery_error(monkeypatch, method):
+    session = Mock(region_name="us-west-2")
+    session.client.side_effect = EndpointConnectionError(endpoint_url="https://sts.us-west-2.amazonaws.com")
+    monkeypatch.setattr(aws_client.boto3, "Session", Mock(return_value=session))
+    client = AWSClient()
+
+    with pytest.raises(AWSDiscoveryError, match="Incomplete AWS discovery"):
+        if method == "discover":
+            client.discover(["us-west-2"])
+        else:
+            client.discover_organization(["us-west-2"], "OrganizationAccountAccessRole")
+
+
+def test_unknown_profile_is_a_discovery_error(monkeypatch):
+    monkeypatch.setattr(aws_client.boto3, "Session", Mock(side_effect=ProfileNotFound(profile="nope")))
+
+    with pytest.raises(AWSDiscoveryError, match="Unable to initialize AWS credentials"):
+        AWSClient(profile="nope")