Skip to content
Merged

Dev #112

Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/config/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -145,8 +145,8 @@ class Settings(BaseSettings):

# S3 Storage Configuration
s3_endpoint: str = Field(default="localhost:3900")
s3_access_key: str = Field(default="test-access-key", min_length=3)
s3_secret_key: str = Field(default="test-secret-key", min_length=8)
s3_access_key: Optional[str] = Field(default=None)
s3_secret_key: Optional[str] = Field(default=None)
s3_secure: bool = Field(default=False)
s3_bucket: str = Field(default="code-interpreter-files")
s3_region: str = Field(default="garage")
Expand Down
45 changes: 38 additions & 7 deletions src/config/s3.py
Original file line number Diff line number Diff line change
@@ -1,19 +1,25 @@
"""S3-compatible object storage configuration."""

from typing import Any, Optional

import boto3
from pydantic import Field
from pydantic_settings import BaseSettings


class S3Config(BaseSettings):
"""S3-compatible storage settings (Garage, AWS S3, etc.)."""
"""S3-compatible storage settings (Garage, AWS S3, etc.).

When ``access_key`` and ``secret_key`` are ``None`` (the default), boto3
uses its standard credential chain — environment variables,
``~/.aws/credentials``, and EC2/ECS instance metadata (IAM role). Set
them explicitly only when connecting to a non-AWS S3-compatible service
such as Garage or MinIO that requires static credentials.
"""

endpoint: str = Field(default="localhost:3900", alias="s3_endpoint")
access_key: str = Field(
default="test-access-key", min_length=3, alias="s3_access_key"
)
secret_key: str = Field(
default="test-secret-key", min_length=8, alias="s3_secret_key"
)
access_key: Optional[str] = Field(default=None, alias="s3_access_key")
secret_key: Optional[str] = Field(default=None, alias="s3_secret_key")
secure: bool = Field(default=False, alias="s3_secure")
bucket: str = Field(default="code-interpreter-files", alias="s3_bucket")
region: str = Field(default="garage", alias="s3_region")
Expand All @@ -24,6 +30,31 @@ def endpoint_url(self) -> str:
scheme = "https" if self.secure else "http"
return f"{scheme}://{self.endpoint}"

def make_client(self) -> Any:
"""Return a configured boto3 S3 client.

Credentials are passed explicitly only when both ``access_key`` and
``secret_key`` are set. When they are ``None``, boto3 falls through to
its default credential chain (env vars, ``~/.aws/credentials``, EC2/ECS
instance metadata).

Raises ``ValueError`` when exactly one of the pair is set — partial
static config is always a misconfiguration.
"""
if bool(self.access_key) != bool(self.secret_key):
raise ValueError(
"S3_ACCESS_KEY and S3_SECRET_KEY must both be set or both be unset. "
"Partial static credentials are not supported."
)
kwargs: dict[str, Any] = {
"endpoint_url": self.endpoint_url,
"region_name": self.region,
}
if self.access_key and self.secret_key:
kwargs["aws_access_key_id"] = self.access_key
kwargs["aws_secret_access_key"] = self.secret_key
return boto3.client("s3", **kwargs)

class Config:
env_prefix = ""
extra = "ignore"
9 changes: 1 addition & 8 deletions src/services/file.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
from typing import List, Optional, Tuple, Dict, Any

# Third-party imports
import boto3
import redis.asyncio as redis
import structlog
from botocore.exceptions import ClientError
Expand All @@ -25,13 +24,7 @@ class FileService(FileServiceInterface):

def __init__(self):
"""Initialize the file service with S3 and Redis clients."""
self.s3_client = boto3.client(
"s3",
endpoint_url=settings.s3.endpoint_url,
aws_access_key_id=settings.s3_access_key,
aws_secret_access_key=settings.s3_secret_key,
region_name=settings.s3_region,
)
self.s3_client = settings.s3.make_client()

# Initialize Redis client
self.redis_client = redis.from_url(
Expand Down
16 changes: 2 additions & 14 deletions src/services/health.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
from typing import Dict, Any, Optional

# Third-party imports
import boto3
import redis.asyncio as redis
import structlog
from botocore.exceptions import ClientError
Expand Down Expand Up @@ -224,21 +223,11 @@ async def check_s3(self) -> HealthCheckResult:

try:
if not self._s3_client:
self._s3_client = boto3.client(
"s3",
endpoint_url=settings.s3.endpoint_url,
aws_access_key_id=settings.s3_access_key,
aws_secret_access_key=settings.s3_secret_key,
region_name=settings.s3_region,
)
self._s3_client = settings.s3.make_client()

loop = asyncio.get_event_loop()
buckets_resp = await loop.run_in_executor(
None, self._s3_client.list_buckets
)
buckets = buckets_resp.get("Buckets", [])

# Check if our bucket exists
# Check if our bucket exists; create it if not
try:
await loop.run_in_executor(
None,
Expand Down Expand Up @@ -300,7 +289,6 @@ async def check_s3(self) -> HealthCheckResult:
"endpoint": settings.s3_endpoint,
"bucket": settings.s3_bucket,
"bucket_exists": bucket_exists,
"total_buckets": len(buckets),
"secure": settings.s3_secure,
}

Expand Down
9 changes: 1 addition & 8 deletions src/services/state_archival.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
from datetime import datetime, timezone
from typing import Optional, Dict, Any

import boto3
import structlog
from botocore.exceptions import ClientError

Expand Down Expand Up @@ -58,13 +57,7 @@ def __init__(
s3_client: Optional boto3 S3 client (creates new one if not provided)
"""
self.state_service = state_service or StateService()
self.s3_client = s3_client or boto3.client(
"s3",
endpoint_url=settings.s3.endpoint_url,
aws_access_key_id=settings.s3_access_key,
aws_secret_access_key=settings.s3_secret_key,
region_name=settings.s3_region,
)
self.s3_client = s3_client or settings.s3.make_client()
self.bucket_name = settings.s3_bucket
self._bucket_checked = False

Expand Down
33 changes: 12 additions & 21 deletions src/utils/config_validator.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
import shutil
from typing import List, Dict, Any
import redis
import boto3
from botocore.exceptions import ClientError

from ..config import settings
Expand Down Expand Up @@ -124,26 +123,18 @@ def _validate_redis_connection(self):
def _validate_s3_connection(self):
"""Validate S3 storage connection."""
try:
client = boto3.client(
"s3",
endpoint_url=settings.s3.endpoint_url,
aws_access_key_id=settings.s3_access_key,
aws_secret_access_key=settings.s3_secret_key,
region_name=settings.s3_region,
)

# Test connection by listing buckets
response = client.list_buckets()
buckets = response.get("Buckets", [])

# Check if our bucket exists
bucket_exists = any(
bucket["Name"] == settings.s3_bucket for bucket in buckets
)
if not bucket_exists:
self.warnings.append(
f"S3 bucket '{settings.s3_bucket}' does not exist - will be created"
)
client = settings.s3.make_client()

try:
client.head_bucket(Bucket=settings.s3_bucket)
except ClientError as e:
code = e.response["Error"]["Code"]
if code in ("404", "NoSuchBucket"):
self.warnings.append(
f"S3 bucket '{settings.s3_bucket}' does not exist"
)
else:
raise

except ClientError as e:
if settings.api_debug:
Expand Down
3 changes: 1 addition & 2 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -115,10 +115,9 @@ def execution_service(mock_sandbox_manager):
@pytest.fixture
def file_service(mock_s3_client, mock_redis):
"""Create FileService instance with mocked dependencies."""
with patch("src.services.file.boto3") as mock_boto3, patch(
with patch("src.config.s3.S3Config.make_client", return_value=mock_s3_client), patch(
"src.services.file.redis.from_url", return_value=mock_redis
):
mock_boto3.client.return_value = mock_s3_client
service = FileService()
yield service

Expand Down
3 changes: 1 addition & 2 deletions tests/unit/test_file_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,7 @@ def mock_redis_client():
@pytest.fixture
def file_service(mock_s3_client, mock_redis_client):
"""Create FileService with mocked clients."""
with patch("src.services.file.boto3") as mock_boto3:
mock_boto3.client.return_value = mock_s3_client
with patch("src.config.s3.S3Config.make_client", return_value=mock_s3_client):
with patch("src.services.file.redis.from_url") as mock_redis_from_url:
mock_redis_from_url.return_value = mock_redis_client
service = FileService()
Expand Down
Loading