TOPS-2500 - cap boto3 client timeouts & retries (env load hangs)

src/envars/aws_cloudformation.py

@@ -1,9 +1,11 @@
 import boto3
 
+from .aws_config import AWS_CLIENT_CONFIG
+
 
 class CloudFormationExports:
     def __init__(self, region_name: str | None = None):
-        self.client = boto3.client("cloudformation", region_name=region_name)
+        self.client = boto3.client("cloudformation", region_name=region_name, config=AWS_CLIENT_CONFIG)
         self._exports_cache: dict[str, str] | None = None
 
     def _populate_exports_cache(self):

src/envars/aws_config.py

@@ -0,0 +1,36 @@
+"""Shared botocore client configuration for envars' AWS calls.
+
+Without an explicit ``Config``, ``boto3.client(...)`` inherits the botocore defaults:
+``connect_timeout=60s``, ``read_timeout=60s`` and legacy retries (up to 5 attempts).
+A single stalled STS/KMS/SSM/CloudFormation endpoint can therefore hang for up to
+``5 x 60 = ~300s``, and a full resolve makes several such calls (STS for location
+auto-detect, one KMS decrypt per secret, plus any ``parameter_store:`` /
+``cloudformation_export:`` lookups). These bounds turn a silent multi-minute hang into
+a fast, legible failure while still tolerating a transient blip.
+
+Override per-environment via the ``ENVARS_AWS_*`` variables if the defaults are too tight.
+"""
+
+import os
+
+from botocore.config import Config
+
+
+def _int_env(name: str, default: int) -> int:
+    """Reads a positive int from the environment, falling back to ``default``."""
+    try:
+        value = int(os.environ[name])
+    except (KeyError, ValueError):
+        return default
+    return value if value > 0 else default
+
+
+# Bounded so a stalled endpoint fails in seconds, not minutes. Standard retry mode adds
+# one backoff retry for transient errors while capping the worst case far below botocore's
+# default of read_timeout(60) x legacy 5 attempts = ~300s. A fully-unreachable endpoint
+# means the whole resolve will fail regardless, so we fail fast rather than wait it out.
+AWS_CLIENT_CONFIG = Config(
+    connect_timeout=_int_env("ENVARS_AWS_CONNECT_TIMEOUT", 3),
+    read_timeout=_int_env("ENVARS_AWS_READ_TIMEOUT", 5),
+    retries={"max_attempts": _int_env("ENVARS_AWS_MAX_ATTEMPTS", 2), "mode": "standard"},
+)

src/envars/aws_kms.py

@@ -3,13 +3,15 @@
 import boto3
 from botocore.exceptions import ClientError
 
+from .aws_config import AWS_CLIENT_CONFIG
+
 
 class AWSKMSAgent:
     """A class to handle AWS KMS operations."""
 
     def __init__(self, region_name: str | None = None):
         """Initializes the KMS client."""
-        self.kms_client = boto3.client("kms", region_name=region_name)
+        self.kms_client = boto3.client("kms", region_name=region_name, config=AWS_CLIENT_CONFIG)
 
     def encrypt(self, data: str, key_id: str, encryption_context: dict[str, str]) -> str:
         """Encrypts data using the specified KMS key."""

src/envars/aws_ssm.py

@@ -1,9 +1,11 @@
 import boto3
 
+from .aws_config import AWS_CLIENT_CONFIG
+
 
 class SSMParameterStore:
     def __init__(self, region_name: str | None = None):
-        self.client = boto3.client("ssm", region_name=region_name)
+        self.client = boto3.client("ssm", region_name=region_name, config=AWS_CLIENT_CONFIG)
 
     def get_parameter(self, name: str, with_decryption: bool = True) -> str | None:
         try:

src/envars/cloud_utils.py

@@ -2,10 +2,12 @@
 import sys
 
 import boto3
-from botocore.exceptions import NoCredentialsError
+from botocore.exceptions import BotoCoreError, ClientError
 from google.auth import default as google_auth_default
 from google.auth.exceptions import DefaultCredentialsError
 
+from .aws_config import AWS_CLIENT_CONFIG
+
 
 def _debug(message):
     """Prints a debug message to stderr if ENVARS_DEBUG is set."""
@@ -16,11 +18,14 @@ def _debug(message):
 def get_aws_account_id() -> str | None:
     """Retrieves the AWS account ID from the current credentials."""
     try:
-        account_id = boto3.client("sts").get_caller_identity().get("Account")
+        account_id = boto3.client("sts", config=AWS_CLIENT_CONFIG).get_caller_identity().get("Account")
         _debug(f"Found AWS Account ID: {account_id}")
         return account_id
-    except NoCredentialsError:
-        _debug("No AWS credentials found.")
+    except (BotoCoreError, ClientError) as e:
+        # NoCredentialsError, connect/read timeouts and API errors all land here. This is
+        # best-effort location auto-detection, so degrade to "unknown" (the caller then asks
+        # for --loc) instead of hanging/crashing when STS is slow or unreachable.
+        _debug(f"Could not determine AWS account ID: {type(e).__name__}: {e}")
         return None
 
 

tests/test_aws_config.py

@@ -0,0 +1,68 @@
+"""Tests for the shared bounded AWS client configuration (TOPS-2500)."""
+
+from unittest.mock import MagicMock, patch
+
+import boto3
+from botocore.exceptions import ReadTimeoutError
+
+from src.envars import aws_config, cloud_utils
+from src.envars.aws_cloudformation import CloudFormationExports
+from src.envars.aws_kms import AWSKMSAgent
+from src.envars.aws_ssm import SSMParameterStore
+
+
+def test_default_config_is_bounded():
+    """The shared config caps timeouts and retries well below botocore's defaults (60s/60s/5)."""
+    cfg = aws_config.AWS_CLIENT_CONFIG
+    assert cfg.connect_timeout == 3
+    assert cfg.read_timeout == 5
+    assert cfg.retries == {"max_attempts": 2, "mode": "standard"}
+
+
+def test_int_env_parses_positive_override(monkeypatch):
+    """_int_env returns a valid positive override from the environment."""
+    monkeypatch.setenv("ENVARS_AWS_READ_TIMEOUT", "42")
+    assert aws_config._int_env("ENVARS_AWS_READ_TIMEOUT", 10) == 42
+
+
+def test_int_env_falls_back_when_unset_invalid_or_non_positive(monkeypatch):
+    """Unset, non-numeric, and zero/negative values all fall back to the default."""
+    monkeypatch.delenv("ENVARS_AWS_READ_TIMEOUT", raising=False)
+    assert aws_config._int_env("ENVARS_AWS_READ_TIMEOUT", 10) == 10  # unset
+
+    monkeypatch.setenv("ENVARS_AWS_READ_TIMEOUT", "not-an-int")
+    assert aws_config._int_env("ENVARS_AWS_READ_TIMEOUT", 10) == 10  # invalid
+
+    monkeypatch.setenv("ENVARS_AWS_READ_TIMEOUT", "0")
+    assert aws_config._int_env("ENVARS_AWS_READ_TIMEOUT", 10) == 10  # non-positive
+
+
+def test_kms_client_uses_bounded_config():
+    """AWSKMSAgent constructs its client with the shared bounded config."""
+    agent = AWSKMSAgent(region_name="eu-west-1")
+    assert agent.kms_client.meta.config.connect_timeout == 3
+    assert agent.kms_client.meta.config.read_timeout == 5
+
+
+def test_ssm_client_uses_bounded_config():
+    """SSMParameterStore constructs its client with the shared bounded config."""
+    store = SSMParameterStore(region_name="eu-west-1")
+    assert store.client.meta.config.read_timeout == 5
+
+
+def test_cloudformation_client_uses_bounded_config():
+    """CloudFormationExports constructs its client with the shared bounded config."""
+    exports = CloudFormationExports(region_name="eu-west-1")
+    assert exports.client.meta.config.read_timeout == 5
+
+
+def test_get_aws_account_id_returns_none_on_timeout():
+    """A stalled STS endpoint degrades to None instead of raising ReadTimeoutError.
+
+    Regression for the DSS-3428 crash: get_aws_account_id only caught NoCredentialsError,
+    so a stalled endpoint surfaced as an uncaught traceback out of `envars exec`.
+    """
+    stalled = MagicMock()
+    stalled.get_caller_identity.side_effect = ReadTimeoutError(endpoint_url="https://sts.eu-west-1.amazonaws.com/")
+    with patch.object(boto3, "client", return_value=stalled):
+        assert cloud_utils.get_aws_account_id() is None