fix: better s3 bucket var names

This commit is contained in:
Alex committed 2026-06-16 18:08:04 +01:00
1 parent 7e7b92aa5e
commit 2923f85ce3
6 files changed
+196 -41

No files matched your search

+12
View File
@@ -193,6 +193,18 @@ class Settings(BaseSettings):
FLASK_DEBUG_MODE: bool = False
STORAGE_TYPE: str = "local" # local or s3
# S3-compatible object storage (used when STORAGE_TYPE=s3). Works with AWS
# S3 and any S3-compatible service (MinIO, Cloudflare R2, Backblaze B2,
# DigitalOcean Spaces, ...). For non-AWS services, set S3_ENDPOINT_URL and
# usually S3_PATH_STYLE=true. The SAGEMAKER_* credentials are still read as
# a deprecated fallback for backward compatibility.
S3_BUCKET_NAME: str = "docsgpt-test-bucket"
S3_ENDPOINT_URL: Optional[str] = None # custom endpoint for S3-compatible services; omit for AWS
S3_ACCESS_KEY_ID: Optional[str] = None
S3_SECRET_ACCESS_KEY: Optional[str] = None
S3_REGION: Optional[str] = None # AWS region; use "auto" for Cloudflare R2
S3_PATH_STYLE: bool = False # path-style addressing (required by most non-AWS services)
# Anonymous startup version check for security issues.
VERSION_CHECK: bool = True
URL_STRATEGY: str = "backend" # backend or s3
+54 -18
View File
@@ -1,19 +1,55 @@
"""S3 storage implementation."""
import io
import logging
import os
import posixpath
from typing import BinaryIO, Callable, List
from typing import BinaryIO, Callable, List, Optional, Tuple
import boto3
from application.core.settings import settings
from application.storage.base import BaseStorage
from botocore.config import Config
from botocore.exceptions import ClientError
logger = logging.getLogger(__name__)
class S3Storage(BaseStorage):
"""AWS S3 storage implementation."""
"""S3-compatible object storage (AWS S3, MinIO, Cloudflare R2, etc.)."""
@staticmethod
def _resolve_credentials() -> Tuple[Optional[str], Optional[str], Optional[str]]:
"""Resolve S3 credentials, falling back to deprecated SAGEMAKER_* vars.
Returns:
Tuple of (access_key_id, secret_access_key, region).
"""
access_key = settings.S3_ACCESS_KEY_ID
secret_key = settings.S3_SECRET_ACCESS_KEY
region = settings.S3_REGION
legacy_access = getattr(settings, "SAGEMAKER_ACCESS_KEY", None)
legacy_secret = getattr(settings, "SAGEMAKER_SECRET_KEY", None)
legacy_region = getattr(settings, "SAGEMAKER_REGION", None)
used_legacy = (
(not access_key and legacy_access)
or (not secret_key and legacy_secret)
or (not region and legacy_region)
)
if used_legacy:
logger.warning(
"Using SAGEMAKER_* credentials for S3 storage is deprecated; "
"set S3_ACCESS_KEY_ID, S3_SECRET_ACCESS_KEY, and S3_REGION instead."
)
return (
access_key or legacy_access,
secret_key or legacy_secret,
region or legacy_region,
)
@staticmethod
def _validate_path(path: str) -> str:
@@ -36,22 +72,24 @@ class S3Storage(BaseStorage):
Args:
bucket_name: S3 bucket name (optional, defaults to settings)
"""
self.bucket_name = bucket_name or getattr(
settings, "S3_BUCKET_NAME", "docsgpt-test-bucket"
)
self.bucket_name = bucket_name or settings.S3_BUCKET_NAME
# Get credentials from settings
aws_access_key_id, aws_secret_access_key, region_name = self._resolve_credentials()
self.region = region_name
aws_access_key_id = getattr(settings, "SAGEMAKER_ACCESS_KEY", None)
aws_secret_access_key = getattr(settings, "SAGEMAKER_SECRET_KEY", None)
region_name = getattr(settings, "SAGEMAKER_REGION", None)
client_kwargs = {
"aws_access_key_id": aws_access_key_id,
"aws_secret_access_key": aws_secret_access_key,
"region_name": region_name,
}
# Custom endpoint for S3-compatible services (MinIO, R2, B2, Spaces, ...).
if settings.S3_ENDPOINT_URL:
client_kwargs["endpoint_url"] = settings.S3_ENDPOINT_URL
# Most non-AWS services require path-style addressing.
if settings.S3_PATH_STYLE:
client_kwargs["config"] = Config(s3={"addressing_style": "path"})
self.s3 = boto3.client(
"s3",
aws_access_key_id=aws_access_key_id,
aws_secret_access_key=aws_secret_access_key,
region_name=region_name,
)
self.s3 = boto3.client("s3", **client_kwargs)
def save_file(
self,
@@ -66,13 +104,11 @@ class S3Storage(BaseStorage):
file_data, self.bucket_name, path, ExtraArgs={"StorageClass": storage_class}
)
region = getattr(settings, "SAGEMAKER_REGION", None)
return {
"storage_type": "s3",
"bucket_name": self.bucket_name,
"uri": f"s3://{self.bucket_name}/{path}",
"region": region,
"region": self.region,
}
def get_file(self, path: str) -> BinaryIO:
+15 -2
View File
@@ -6,6 +6,7 @@ import os
import re
import uuid
from typing import List
from urllib.parse import urlparse
import tiktoken
from flask import jsonify, make_response
@@ -197,8 +198,20 @@ def generate_image_url(image_path):
return image_path
strategy = getattr(settings, "URL_STRATEGY", "backend")
if strategy == "s3":
bucket_name = getattr(settings, "S3_BUCKET_NAME", "docsgpt-test-bucket")
region_name = getattr(settings, "SAGEMAKER_REGION", "eu-central-1")
bucket_name = settings.S3_BUCKET_NAME
endpoint_url = settings.S3_ENDPOINT_URL
if endpoint_url:
# S3-compatible service (MinIO, R2, B2, Spaces, ...).
base = endpoint_url.rstrip("/")
if settings.S3_PATH_STYLE:
return f"{base}/{bucket_name}/{image_path}"
parsed = urlparse(base)
return f"{parsed.scheme}://{bucket_name}.{parsed.netloc}/{image_path}"
region_name = (
settings.S3_REGION
or getattr(settings, "SAGEMAKER_REGION", None)
or "eu-central-1"
)
return f"https://{bucket_name}.s3.{region_name}.amazonaws.com/{image_path}"
else:
base_url = getattr(settings, "API_URL", "http://localhost:7091")
+29 -11
View File
@@ -306,28 +306,46 @@ If you have configured `AUTH_TYPE=simple_jwt`, the DocsGPT frontend will prompt
## S3 Storage Backend
By default DocsGPT stores files locally. Set `STORAGE_TYPE=s3` to use Amazon S3 instead.
By default DocsGPT stores files locally. Set `STORAGE_TYPE=s3` to use Amazon S3 — or any S3-compatible service (MinIO, Cloudflare R2, Backblaze B2, DigitalOcean Spaces, …) — instead.
| Setting | Description | Default |
| --- | --- | --- |
| `STORAGE_TYPE` | `local` or `s3` | `local` |
| `S3_BUCKET_NAME` | S3 bucket name | `docsgpt-test-bucket` |
| `SAGEMAKER_ACCESS_KEY` | AWS access key ID | — |
| `SAGEMAKER_SECRET_KEY` | AWS secret access key | — |
| `SAGEMAKER_REGION` | AWS region | — |
| `URL_STRATEGY` | `backend` (proxy through API) or `s3` (direct S3 URLs) | `backend` |
| `S3_BUCKET_NAME` | Bucket name | `docsgpt-test-bucket` |
| `S3_ACCESS_KEY_ID` | Access key ID | — |
| `S3_SECRET_ACCESS_KEY` | Secret access key | — |
| `S3_REGION` | Region (use `auto` for Cloudflare R2) | — |
| `S3_ENDPOINT_URL` | Custom endpoint for S3-compatible services; leave unset for AWS S3 | — |
| `S3_PATH_STYLE` | Use path-style addressing (required by most non-AWS services) | `false` |
| `URL_STRATEGY` | `backend` (proxy through API) or `s3` (direct object URLs) | `backend` |
The S3 credentials use `SAGEMAKER_*` variable names because they are shared with the SageMaker integration.
### AWS S3
```env
STORAGE_TYPE=s3
S3_BUCKET_NAME=your-bucket-name
SAGEMAKER_ACCESS_KEY=your-aws-access-key-id
SAGEMAKER_SECRET_KEY=your-aws-secret-access-key
SAGEMAKER_REGION=us-east-1
S3_ACCESS_KEY_ID=your-access-key-id
S3_SECRET_ACCESS_KEY=your-secret-access-key
S3_REGION=us-east-1
```
Your IAM user needs these permissions on the bucket: `s3:PutObject`, `s3:GetObject`, `s3:DeleteObject`, `s3:ListBucket`, `s3:HeadObject`.
### S3-compatible services (MinIO, Cloudflare R2, …)
Set `S3_ENDPOINT_URL` and usually `S3_PATH_STYLE=true`:
```env
STORAGE_TYPE=s3
S3_BUCKET_NAME=your-bucket-name
S3_ACCESS_KEY_ID=your-access-key-id
S3_SECRET_ACCESS_KEY=your-secret-access-key
S3_REGION=auto
S3_ENDPOINT_URL=https://<account>.r2.cloudflarestorage.com
S3_PATH_STYLE=true
```
Your credentials need these permissions on the bucket: `s3:PutObject`, `s3:GetObject`, `s3:DeleteObject`, `s3:ListBucket`, `s3:HeadObject`.
> **Deprecated:** earlier versions reused the `SAGEMAKER_ACCESS_KEY`, `SAGEMAKER_SECRET_KEY`, and `SAGEMAKER_REGION` variables for S3 credentials. These are still honored as a fallback (with a deprecation warning) but you should migrate to the `S3_*` variables above.
## User-Data Storage (Postgres)
+63 -8
View File
@@ -42,15 +42,21 @@ class TestS3StorageInitialization:
assert storage.bucket_name == "custom-bucket"
@pytest.mark.unit
def test_init_creates_boto3_client(self):
"""Should create boto3 S3 client with credentials from settings."""
def test_init_creates_boto3_client_with_s3_credentials(self):
"""Should create boto3 S3 client with dedicated S3_* credentials."""
with patch("boto3.client") as mock_client, patch(
"application.storage.s3.settings"
) as mock_settings:
mock_settings.SAGEMAKER_ACCESS_KEY = "test-key"
mock_settings.SAGEMAKER_SECRET_KEY = "test-secret"
mock_settings.SAGEMAKER_REGION = "us-west-2"
mock_settings.S3_BUCKET_NAME = "docsgpt-test-bucket"
mock_settings.S3_ACCESS_KEY_ID = "test-key"
mock_settings.S3_SECRET_ACCESS_KEY = "test-secret"
mock_settings.S3_REGION = "us-west-2"
mock_settings.S3_ENDPOINT_URL = None
mock_settings.S3_PATH_STYLE = False
mock_settings.SAGEMAKER_ACCESS_KEY = None
mock_settings.SAGEMAKER_SECRET_KEY = None
mock_settings.SAGEMAKER_REGION = None
S3Storage()
@@ -61,6 +67,56 @@ class TestS3StorageInitialization:
region_name="us-west-2",
)
@pytest.mark.unit
def test_init_falls_back_to_sagemaker_credentials(self):
"""Should fall back to deprecated SAGEMAKER_* credentials when S3_* unset."""
with patch("boto3.client") as mock_client, patch(
"application.storage.s3.settings"
) as mock_settings, patch("application.storage.s3.logger") as mock_logger:
mock_settings.S3_BUCKET_NAME = "docsgpt-test-bucket"
mock_settings.S3_ACCESS_KEY_ID = None
mock_settings.S3_SECRET_ACCESS_KEY = None
mock_settings.S3_REGION = None
mock_settings.S3_ENDPOINT_URL = None
mock_settings.S3_PATH_STYLE = False
mock_settings.SAGEMAKER_ACCESS_KEY = "legacy-key"
mock_settings.SAGEMAKER_SECRET_KEY = "legacy-secret"
mock_settings.SAGEMAKER_REGION = "eu-central-1"
S3Storage()
mock_client.assert_called_once_with(
"s3",
aws_access_key_id="legacy-key",
aws_secret_access_key="legacy-secret",
region_name="eu-central-1",
)
mock_logger.warning.assert_called_once()
@pytest.mark.unit
def test_init_with_custom_endpoint_and_path_style(self):
"""Should pass endpoint_url and path-style config for S3-compatible services."""
with patch("boto3.client") as mock_client, patch(
"application.storage.s3.settings"
) as mock_settings:
mock_settings.S3_BUCKET_NAME = "my-bucket"
mock_settings.S3_ACCESS_KEY_ID = "key"
mock_settings.S3_SECRET_ACCESS_KEY = "secret"
mock_settings.S3_REGION = "auto"
mock_settings.S3_ENDPOINT_URL = "https://account.r2.cloudflarestorage.com"
mock_settings.S3_PATH_STYLE = True
mock_settings.SAGEMAKER_ACCESS_KEY = None
mock_settings.SAGEMAKER_SECRET_KEY = None
mock_settings.SAGEMAKER_REGION = None
S3Storage()
_, kwargs = mock_client.call_args
assert kwargs["endpoint_url"] == "https://account.r2.cloudflarestorage.com"
assert kwargs["config"].s3 == {"addressing_style": "path"}
class TestS3StorageSaveFile:
"""Test file saving functionality."""
@@ -71,9 +127,8 @@ class TestS3StorageSaveFile:
file_data = io.BytesIO(b"test content")
path = "documents/test.txt"
with patch("application.storage.s3.settings") as mock_settings:
mock_settings.SAGEMAKER_REGION = "us-east-1"
result = s3_storage.save_file(file_data, path)
s3_storage.region = "us-east-1"
result = s3_storage.save_file(file_data, path)
mock_boto3_client.upload_fileobj.assert_called_once_with(
file_data,
"test-bucket",
+23 -2
View File
@@ -320,9 +320,30 @@ class TestGenerateImageUrl:
with patch("application.utils.settings") as s:
s.URL_STRATEGY = "s3"
s.S3_BUCKET_NAME = "my-bucket"
s.SAGEMAKER_REGION = "us-west-2"
s.S3_ENDPOINT_URL = None
s.S3_REGION = "us-west-2"
result = generate_image_url("path/to/img.png")
assert "my-bucket.s3.us-west-2" in result
assert result == "https://my-bucket.s3.us-west-2.amazonaws.com/path/to/img.png"
@pytest.mark.unit
def test_s3_strategy_custom_endpoint_path_style(self):
with patch("application.utils.settings") as s:
s.URL_STRATEGY = "s3"
s.S3_BUCKET_NAME = "my-bucket"
s.S3_ENDPOINT_URL = "https://account.r2.cloudflarestorage.com"
s.S3_PATH_STYLE = True
result = generate_image_url("path/to/img.png")
assert result == "https://account.r2.cloudflarestorage.com/my-bucket/path/to/img.png"
@pytest.mark.unit
def test_s3_strategy_custom_endpoint_virtual_host(self):
with patch("application.utils.settings") as s:
s.URL_STRATEGY = "s3"
s.S3_BUCKET_NAME = "my-bucket"
s.S3_ENDPOINT_URL = "https://minio.example.com"
s.S3_PATH_STYLE = False
result = generate_image_url("path/to/img.png")
assert result == "https://my-bucket.minio.example.com/path/to/img.png"
@pytest.mark.unit
def test_backend_strategy(self):