diff --git a/application/core/settings.py b/application/core/settings.py index a4cf30c6..ce1b2db1 100644 --- a/application/core/settings.py +++ b/application/core/settings.py @@ -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 diff --git a/application/storage/s3.py b/application/storage/s3.py index b6c6a1b8..c2cb64b4 100644 --- a/application/storage/s3.py +++ b/application/storage/s3.py @@ -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: diff --git a/application/utils.py b/application/utils.py index f14007af..176f7720 100644 --- a/application/utils.py +++ b/application/utils.py @@ -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") diff --git a/docs/content/Deploying/DocsGPT-Settings.mdx b/docs/content/Deploying/DocsGPT-Settings.mdx index 098c2c42..c7e3595a 100644 --- a/docs/content/Deploying/DocsGPT-Settings.mdx +++ b/docs/content/Deploying/DocsGPT-Settings.mdx @@ -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://.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) diff --git a/tests/storage/test_s3_storage.py b/tests/storage/test_s3_storage.py index 967da1f5..aed2a9b2 100644 --- a/tests/storage/test_s3_storage.py +++ b/tests/storage/test_s3_storage.py @@ -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", diff --git a/tests/test_utils.py b/tests/test_utils.py index 0190b44c..40567289 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -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):