Stop reimplementing region resolution; let the boto3 session do it

Region was resolved by a hand-rolled os.getenv("AWS_REGION",
"AWS_DEFAULT_REGION") ladder in ~two places. That reimplements botocore's
own chain incompletely (it skips the active profile's ~/.aws/config), and
schema.py turned "no region configured" into a ValueError instead of the
loud NoRegionError boto3 raises.

Let the session resolve region: build the client with
region_name=config.region (an explicit override, or None to fall through
the chain) and read the concrete value back off client.meta.region_name
for the AMI lookup, SSM client, and ProvisionedInstance/SandboxInstanceInfo.
region stays instance data.

- schema.from_settings: drop the AWS_REGION ladder + the required-region
  ValueError; region is now an optional override (None -> session resolves).
- DefaultEc2InstanceProvider: region no longer a required config field;
  resolve it off the ec2 client in create_instance and find_sandbox_instances.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Author: art-dsit

README.md

@@ -83,7 +83,6 @@ and allow the end-user to specify the rest.
 The following environment variables must be set:
 
 ```bash
-INSPECT_EC2_SANDBOX_REGION=eu-west-1
 INSPECT_EC2_SANDBOX_VPC_ID=vpc-123456
 INSPECT_EC2_SANDBOX_SECURITY_GROUP_ID=sg-56781234
 INSPECT_EC2_SANDBOX_SUBNET_ID=subnet-654321
@@ -94,12 +93,17 @@ INSPECT_EC2_SANDBOX_S3_BUCKET=ec2sandboxstack-databucket123-456
 The following environment variables are optional:
 
 ```bash
+INSPECT_EC2_SANDBOX_REGION=eu-west-1
 INSPECT_EC2_SANDBOX_AMI_ID=ami-123456
 INSPECT_EC2_SANDBOX_INSTANCE_TYPE=t3a.small
 INSPECT_EC2_SANDBOX_S3_KEY_PREFIX=sandbox-comms
 INSPECT_EC2_SANDBOX_EXTRA_TAGS_STR='tagname1=tagvalue1;tagname2=tagvalue2'
 ```
 
+`INSPECT_EC2_SANDBOX_REGION` is only needed to override the region. When it
+is unset the boto3 session resolves the region the usual way (`AWS_REGION`,
+`AWS_DEFAULT_REGION`, then the active profile's `~/.aws/config`).
+
 ### Configuration
 
 As an alternative to the above environment variables you can specify the configuration directly in code, e.g

src/ec2sandbox/_instance_provider.py

@@ -15,7 +15,6 @@
 from __future__ import annotations
 
 import logging
-import os
 from dataclasses import dataclass
 from typing import TYPE_CHECKING, Any, ClassVar, Protocol, runtime_checkable
 
@@ -206,12 +205,12 @@ class DefaultEc2InstanceProvider:
     """Default :class:`Ec2InstanceProvider` using direct boto3 calls.
 
     Used by the EC2 sandbox when no custom provider has been registered.
-    Reads the infrastructure fields (``region``, ``security_group_id``,
+    Reads the infrastructure fields (``security_group_id``, ``subnet_id``,
     etc.) from the supplied :class:`Ec2SandboxEnvironmentConfig` at the
-    point they are needed — ``create_instance`` requires the full set,
-    while ``terminate_instance`` and ``find_sandbox_instances`` only
-    need a region (the instance's own region for terminate; the
-    configured region for find).
+    point they are needed. Region is never required in the config: each
+    method builds its client with ``region_name=config.region`` (an
+    explicit override, or ``None`` to let the session resolve it) and reads
+    the resolved region back off the client.
     """
 
     # Process-global Ubuntu 24.04 AMI cache keyed on region. Canonical's
@@ -249,7 +248,6 @@ async def create_instance(
     ) -> ProvisionedInstance:
         cfg = self._config
         required = {
-            "region": cfg.region,
             "security_group_id": cfg.security_group_id,
             "subnet_id": cfg.subnet_id,
             "instance_profile": cfg.instance_profile,
@@ -264,11 +262,17 @@ async def create_instance(
                 "register an Ec2InstanceProvider."
             )
 
+        # region_name=cfg.region is an explicit override when set; otherwise
+        # the session resolves it and raises NoRegionError if nothing is
+        # configured. Read the resolved value back off the client so the rest
+        # of the method (AMI lookup, SSM client, ProvisionedInstance) uses a
+        # concrete region rather than a possibly-None config field.
+        ec2_client = self._session.client("ec2", region_name=cfg.region)
+        region = ec2_client.meta.region_name
+
         if not ami_id:
-            assert cfg.region is not None  # validated above
-            ami_id = self._resolve_ubu24_ami(cfg.region)
+            ami_id = self._resolve_ubu24_ami(region)
 
-        ec2_client = self._session.client("ec2", region_name=cfg.region)
         instance_params: dict[str, Any] = {
             "ImageId": ami_id,
             "InstanceType": instance_type,
@@ -297,7 +301,7 @@ async def create_instance(
             waiter = ec2_client.get_waiter("instance_running")
             waiter.wait(InstanceIds=[instance_id])
 
-            ssm_client = self._session.client("ssm", region_name=cfg.region)
+            ssm_client = self._session.client("ssm", region_name=region)
             _wait_for_ssm(instance_id, ssm_client)
         except BaseException:
             try:
@@ -310,11 +314,10 @@ async def create_instance(
                 )
             raise
 
-        assert cfg.region is not None  # validated above
         assert cfg.s3_bucket is not None  # validated above
         return ProvisionedInstance(
             instance_id=instance_id,
-            region=cfg.region,
+            region=region,
             s3_bucket=cfg.s3_bucket,
             s3_key_prefix=cfg.s3_key_prefix,
         )
@@ -324,14 +327,11 @@ async def terminate_instance(self, instance_id: str, region: str) -> None:
         ec2.terminate_instances(InstanceIds=[instance_id])
 
     async def find_sandbox_instances(self) -> list[SandboxInstanceInfo]:
-        # cli_cleanup builds this provider with an empty config (no region),
-        # so fall back to AWS_REGION / AWS_DEFAULT_REGION as the session does.
-        region = (
-            self._config.region
-            or os.getenv("AWS_REGION")
-            or os.getenv("AWS_DEFAULT_REGION")
-        )
-        ec2 = self._session.client("ec2", region_name=region or None)
+        # cli_cleanup builds this provider with an empty config, so region is
+        # usually None here and the session resolves it. Read the resolved
+        # region back off the client to stamp on each SandboxInstanceInfo.
+        ec2 = self._session.client("ec2", region_name=self._config.region)
+        region = ec2.meta.region_name
         response = ec2.describe_instances(
             Filters=[
                 {"Name": f"tag:{MARKER_TAG_KEY}", "Values": ["true"]},
@@ -353,7 +353,7 @@ async def find_sandbox_instances(self) -> list[SandboxInstanceInfo]:
                     SandboxInstanceInfo(
                         instance_id=instance["InstanceId"],
                         name=name,
-                        region=region or "",
+                        region=region,
                     )
                 )
         return results

src/ec2sandbox/schema.py

@@ -5,7 +5,6 @@
  Inspect EC2 sandbox environments.
 """
 
-import os
 from typing import Optional, Tuple
 
 from pydantic import BaseModel, ConfigDict
@@ -66,10 +65,14 @@ class Ec2SandboxEnvironmentConfig(BaseModel):
     s3_key_prefix: str = ""
     volume_size: Optional[int] = None
 
+    # Optional explicit region override. None -> the boto3 session resolves
+    # the region (AWS_REGION / AWS_DEFAULT_REGION / ~/.aws/config) when it
+    # builds a client. A set value sits at the top of that chain.
+    region: Optional[str] = None
+
     # Direct-EC2-path fields — required when no Ec2InstanceProvider is
     # registered, ignored otherwise. ``sample_init`` validates these at
     # call time when the direct path is taken.
-    region: Optional[str] = None
     # TODO is vpc_id actually needed? We could just force a subnet ID.
     vpc_id: Optional[str] = None
     security_group_id: Optional[str] = None
@@ -102,15 +105,11 @@ def from_settings(cls, **kwargs):
         # Override with any provided kwargs
         params.update(kwargs)
 
-        region = params["region"]
-        if region is None:
-            region = os.getenv("AWS_REGION")
-        if not isinstance(region, str):
-            raise ValueError(
-                "Region must be specified either in settings,"
-                f" or as an environment variable {env_prefix}REGION or AWS_REGION."
-            )
-        params["region"] = region
+        # region is left as-is (may be None). Don't reimplement botocore's
+        # resolution chain: the boto3 session resolves the region (AWS_REGION
+        # / AWS_DEFAULT_REGION / ~/.aws/config) at client-construction time,
+        # and raises NoRegionError loudly if nothing is configured. An explicit
+        # INSPECT_EC2_SANDBOX_REGION (or region= kwarg) overrides that chain.
 
         # AMI resolution is deferred to DefaultEc2InstanceProvider.create_instance
         # so that callers who only need terminate/find don't pay for an SSM

tests/ec2sandboxtest/test_from_settings.py

@@ -36,23 +36,26 @@ def test_kwarg_overrides_env_var() -> None:
     assert config.instance_type == "t3a.xlarge"
 
 
-def test_region_falls_back_to_aws_region() -> None:
-    """AWS_REGION is used when INSPECT_EC2_SANDBOX_REGION is unset."""
-    env_vars = env_vars_all()
-    env_vars.pop("INSPECT_EC2_SANDBOX_REGION")
-    env_vars["AWS_REGION"] = "eu-west-1"
-    with mock.patch.dict(os.environ, env_vars, clear=True):
+def test_explicit_region_env_var_sets_region() -> None:
+    """INSPECT_EC2_SANDBOX_REGION is carried through as an explicit override."""
+    with mock.patch.dict(os.environ, env_vars_all(), clear=True):
         config = Ec2SandboxEnvironmentConfig.from_settings()
-    assert config.region == "eu-west-1"
+    assert config.region == "eu-west-2"
+
 
+def test_region_left_none_for_session_to_resolve() -> None:
+    """Without INSPECT_EC2_SANDBOX_REGION, from_settings leaves region None.
 
-def test_missing_region_raises_value_error() -> None:
-    """With no region from any source, from_settings raises a clear error."""
+    Region resolution is deferred to the boto3 session at client-construction
+    time, so from_settings must not reimplement the chain by reading AWS_REGION
+    itself — that would shadow ~/.aws/config and the rest of botocore's chain.
+    """
     env_vars = env_vars_all()
     env_vars.pop("INSPECT_EC2_SANDBOX_REGION")
+    env_vars["AWS_REGION"] = "eu-west-1"
     with mock.patch.dict(os.environ, env_vars, clear=True):
-        with pytest.raises(ValueError, match="Region must be specified"):
-            Ec2SandboxEnvironmentConfig.from_settings()
+        config = Ec2SandboxEnvironmentConfig.from_settings()
+    assert config.region is None
 
 
 def test_s3_key_prefix_leading_slash_raises() -> None:

tests/ec2sandboxtest/test_instance_provider.py

@@ -105,6 +105,29 @@ async def test_create_instance_resolves_and_caches_ami_when_empty():
     assert ssm_client.get_parameters.call_count == 1
 
 
+@pytest.mark.asyncio
+async def test_create_instance_stamps_session_resolved_region():
+    """With no region in config, it's resolved off the client and stamped.
+
+    The ec2 client is built with region_name=None so the session resolves the
+    region; ProvisionedInstance.region then carries the concrete value.
+    """
+    provider, ec2_client, _ = _make_provider_with_mocks(_make_config(region=None))
+    ec2_client.meta.region_name = "us-east-1"
+
+    result = await provider.create_instance(
+        instance_type="t3a.micro",
+        ami_id="ami-123",
+        tags=[("Name", "x")],
+    )
+
+    assert result.region == "us-east-1"
+    ec2_call = next(
+        c for c in provider._session.client.call_args_list if c.args[0] == "ec2"
+    )
+    assert ec2_call.kwargs["region_name"] is None
+
+
 @pytest.mark.asyncio
 async def test_create_instance_with_volume_size_sets_block_device_mappings():
     provider, ec2_client, _ = _make_provider_with_mocks(_make_config(volume_size=100))