Skip to content

Commit ef38b73

Browse files
author
Damien
committed
merge: integrate origin/main into feat/runtime-only (S3 IAM role auth)
Brings in 5 commits from origin/main (PR usnavy13#111/usnavy13#112 from usnavy13): - bf97b64 Support IAM role / default credential chain for S3 auth - 64474ae fix: address PR review — head_bucket and partial-creds validation Touches src/config/{__init__.py,s3.py}, src/services/{file,health,state_archival}.py, src/utils/config_validator.py, tests/{conftest.py,unit/test_file_service.py}. No overlap with runtime-only changes — clean merge expected.
2 parents f4cbff6 + 9301fc4 commit ef38b73

8 files changed

Lines changed: 58 additions & 64 deletions

File tree

src/config/__init__.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -145,8 +145,8 @@ class Settings(BaseSettings):
145145

146146
# S3 Storage Configuration
147147
s3_endpoint: str = Field(default="localhost:3900")
148-
s3_access_key: str = Field(default="test-access-key", min_length=3)
149-
s3_secret_key: str = Field(default="test-secret-key", min_length=8)
148+
s3_access_key: Optional[str] = Field(default=None)
149+
s3_secret_key: Optional[str] = Field(default=None)
150150
s3_secure: bool = Field(default=False)
151151
s3_bucket: str = Field(default="code-interpreter-files")
152152
s3_region: str = Field(default="garage")

src/config/s3.py

Lines changed: 38 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,25 @@
11
"""S3-compatible object storage configuration."""
22

3+
from typing import Any, Optional
4+
5+
import boto3
36
from pydantic import Field
47
from pydantic_settings import BaseSettings
58

69

710
class S3Config(BaseSettings):
8-
"""S3-compatible storage settings (Garage, AWS S3, etc.)."""
11+
"""S3-compatible storage settings (Garage, AWS S3, etc.).
12+
13+
When ``access_key`` and ``secret_key`` are ``None`` (the default), boto3
14+
uses its standard credential chain — environment variables,
15+
``~/.aws/credentials``, and EC2/ECS instance metadata (IAM role). Set
16+
them explicitly only when connecting to a non-AWS S3-compatible service
17+
such as Garage or MinIO that requires static credentials.
18+
"""
919

1020
endpoint: str = Field(default="localhost:3900", alias="s3_endpoint")
11-
access_key: str = Field(
12-
default="test-access-key", min_length=3, alias="s3_access_key"
13-
)
14-
secret_key: str = Field(
15-
default="test-secret-key", min_length=8, alias="s3_secret_key"
16-
)
21+
access_key: Optional[str] = Field(default=None, alias="s3_access_key")
22+
secret_key: Optional[str] = Field(default=None, alias="s3_secret_key")
1723
secure: bool = Field(default=False, alias="s3_secure")
1824
bucket: str = Field(default="code-interpreter-files", alias="s3_bucket")
1925
region: str = Field(default="garage", alias="s3_region")
@@ -24,6 +30,31 @@ def endpoint_url(self) -> str:
2430
scheme = "https" if self.secure else "http"
2531
return f"{scheme}://{self.endpoint}"
2632

33+
def make_client(self) -> Any:
34+
"""Return a configured boto3 S3 client.
35+
36+
Credentials are passed explicitly only when both ``access_key`` and
37+
``secret_key`` are set. When they are ``None``, boto3 falls through to
38+
its default credential chain (env vars, ``~/.aws/credentials``, EC2/ECS
39+
instance metadata).
40+
41+
Raises ``ValueError`` when exactly one of the pair is set — partial
42+
static config is always a misconfiguration.
43+
"""
44+
if bool(self.access_key) != bool(self.secret_key):
45+
raise ValueError(
46+
"S3_ACCESS_KEY and S3_SECRET_KEY must both be set or both be unset. "
47+
"Partial static credentials are not supported."
48+
)
49+
kwargs: dict[str, Any] = {
50+
"endpoint_url": self.endpoint_url,
51+
"region_name": self.region,
52+
}
53+
if self.access_key and self.secret_key:
54+
kwargs["aws_access_key_id"] = self.access_key
55+
kwargs["aws_secret_access_key"] = self.secret_key
56+
return boto3.client("s3", **kwargs)
57+
2758
class Config:
2859
env_prefix = ""
2960
extra = "ignore"

src/services/file.py

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66
from typing import List, Optional, Tuple, Dict, Any
77

88
# Third-party imports
9-
import boto3
109
import redis.asyncio as redis
1110
import structlog
1211
from botocore.exceptions import ClientError
@@ -25,13 +24,7 @@ class FileService(FileServiceInterface):
2524

2625
def __init__(self):
2726
"""Initialize the file service with S3 and Redis clients."""
28-
self.s3_client = boto3.client(
29-
"s3",
30-
endpoint_url=settings.s3.endpoint_url,
31-
aws_access_key_id=settings.s3_access_key,
32-
aws_secret_access_key=settings.s3_secret_key,
33-
region_name=settings.s3_region,
34-
)
27+
self.s3_client = settings.s3.make_client()
3528

3629
# Initialize Redis client
3730
self.redis_client = redis.from_url(

src/services/health.py

Lines changed: 2 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@
1111
from typing import Dict, Any, Optional
1212

1313
# Third-party imports
14-
import boto3
1514
import redis.asyncio as redis
1615
import structlog
1716
from botocore.exceptions import ClientError
@@ -224,21 +223,11 @@ async def check_s3(self) -> HealthCheckResult:
224223

225224
try:
226225
if not self._s3_client:
227-
self._s3_client = boto3.client(
228-
"s3",
229-
endpoint_url=settings.s3.endpoint_url,
230-
aws_access_key_id=settings.s3_access_key,
231-
aws_secret_access_key=settings.s3_secret_key,
232-
region_name=settings.s3_region,
233-
)
226+
self._s3_client = settings.s3.make_client()
234227

235228
loop = asyncio.get_event_loop()
236-
buckets_resp = await loop.run_in_executor(
237-
None, self._s3_client.list_buckets
238-
)
239-
buckets = buckets_resp.get("Buckets", [])
240229

241-
# Check if our bucket exists
230+
# Check if our bucket exists; create it if not
242231
try:
243232
await loop.run_in_executor(
244233
None,
@@ -300,7 +289,6 @@ async def check_s3(self) -> HealthCheckResult:
300289
"endpoint": settings.s3_endpoint,
301290
"bucket": settings.s3_bucket,
302291
"bucket_exists": bucket_exists,
303-
"total_buckets": len(buckets),
304292
"secure": settings.s3_secure,
305293
}
306294

src/services/state_archival.py

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@
2222
from datetime import datetime, timezone
2323
from typing import Optional, Dict, Any
2424

25-
import boto3
2625
import structlog
2726
from botocore.exceptions import ClientError
2827

@@ -58,13 +57,7 @@ def __init__(
5857
s3_client: Optional boto3 S3 client (creates new one if not provided)
5958
"""
6059
self.state_service = state_service or StateService()
61-
self.s3_client = s3_client or boto3.client(
62-
"s3",
63-
endpoint_url=settings.s3.endpoint_url,
64-
aws_access_key_id=settings.s3_access_key,
65-
aws_secret_access_key=settings.s3_secret_key,
66-
region_name=settings.s3_region,
67-
)
60+
self.s3_client = s3_client or settings.s3.make_client()
6861
self.bucket_name = settings.s3_bucket
6962
self._bucket_checked = False
7063

src/utils/config_validator.py

Lines changed: 12 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44
import shutil
55
from typing import List, Dict, Any
66
import redis
7-
import boto3
87
from botocore.exceptions import ClientError
98

109
from ..config import settings
@@ -124,26 +123,18 @@ def _validate_redis_connection(self):
124123
def _validate_s3_connection(self):
125124
"""Validate S3 storage connection."""
126125
try:
127-
client = boto3.client(
128-
"s3",
129-
endpoint_url=settings.s3.endpoint_url,
130-
aws_access_key_id=settings.s3_access_key,
131-
aws_secret_access_key=settings.s3_secret_key,
132-
region_name=settings.s3_region,
133-
)
134-
135-
# Test connection by listing buckets
136-
response = client.list_buckets()
137-
buckets = response.get("Buckets", [])
138-
139-
# Check if our bucket exists
140-
bucket_exists = any(
141-
bucket["Name"] == settings.s3_bucket for bucket in buckets
142-
)
143-
if not bucket_exists:
144-
self.warnings.append(
145-
f"S3 bucket '{settings.s3_bucket}' does not exist - will be created"
146-
)
126+
client = settings.s3.make_client()
127+
128+
try:
129+
client.head_bucket(Bucket=settings.s3_bucket)
130+
except ClientError as e:
131+
code = e.response["Error"]["Code"]
132+
if code in ("404", "NoSuchBucket"):
133+
self.warnings.append(
134+
f"S3 bucket '{settings.s3_bucket}' does not exist"
135+
)
136+
else:
137+
raise
147138

148139
except ClientError as e:
149140
if settings.api_debug:

tests/conftest.py

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -115,10 +115,9 @@ def execution_service(mock_sandbox_manager):
115115
@pytest.fixture
116116
def file_service(mock_s3_client, mock_redis):
117117
"""Create FileService instance with mocked dependencies."""
118-
with patch("src.services.file.boto3") as mock_boto3, patch(
118+
with patch("src.config.s3.S3Config.make_client", return_value=mock_s3_client), patch(
119119
"src.services.file.redis.from_url", return_value=mock_redis
120120
):
121-
mock_boto3.client.return_value = mock_s3_client
122121
service = FileService()
123122
yield service
124123

tests/unit/test_file_service.py

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,8 +39,7 @@ def mock_redis_client():
3939
@pytest.fixture
4040
def file_service(mock_s3_client, mock_redis_client):
4141
"""Create FileService with mocked clients."""
42-
with patch("src.services.file.boto3") as mock_boto3:
43-
mock_boto3.client.return_value = mock_s3_client
42+
with patch("src.config.s3.S3Config.make_client", return_value=mock_s3_client):
4443
with patch("src.services.file.redis.from_url") as mock_redis_from_url:
4544
mock_redis_from_url.return_value = mock_redis_client
4645
service = FileService()

0 commit comments

Comments
 (0)