fix: s3 storage configured flag fails on minimum setup (#6573)

This commit is contained in:
Matti Nannt
2025-09-22 09:11:58 +02:00
committed by GitHub
parent 8b3c0f1547
commit 1c5244e030
4 changed files with 54 additions and 15 deletions

View File

@@ -117,7 +117,7 @@ export const MAX_FILE_UPLOAD_SIZES = {
// Storage is considered configured if we have the minimum required settings:
// - S3_REGION and S3_BUCKET_NAME are always required
// - S3_ACCESS_KEY and S3_SECRET_KEY are optional (for IAM role-based authentication)
export const IS_STORAGE_CONFIGURED = Boolean(S3_REGION && S3_BUCKET_NAME);
export const IS_STORAGE_CONFIGURED = Boolean(S3_BUCKET_NAME);
// Colors for Survey Bg
export const SURVEY_BG_COLORS = [

View File

@@ -133,11 +133,13 @@ REDIS_URL=redis://your-redis-host:6379
Configure S3 storage by adding the following environment variables to your instances:
```sh env
# Required for file uploads in serverless environments
# Required
S3_BUCKET_NAME=your-bucket-name
# Optional - if not provided, AWS SDK will use defaults (us-east-1) or auto-detect
S3_ACCESS_KEY=your-access-key
S3_SECRET_KEY=your-secret-key
S3_REGION=your-region
S3_BUCKET_NAME=your-bucket-name
# For S3-compatible storage (e.g., StorJ, MinIO)
# Leave empty for Amazon S3

View File

@@ -227,8 +227,8 @@ describe("client.ts", () => {
}
});
test("should return error when region is missing", async () => {
// Mock constants with missing region
test("should create S3 client when region is missing (uses AWS SDK defaults)", async () => {
// Mock constants with missing region - should still work
vi.doMock("./constants", () => ({
...mockConstants,
S3_REGION: undefined,
@@ -238,9 +238,44 @@ describe("client.ts", () => {
const result = createS3ClientFromEnv();
expect(result.ok).toBe(false);
if (!result.ok) {
expect(result.error.code).toBe("s3_credentials_error");
expect(mockS3Client).toHaveBeenCalledWith({
credentials: {
accessKeyId: mockConstants.S3_ACCESS_KEY,
secretAccessKey: mockConstants.S3_SECRET_KEY,
},
endpoint: mockConstants.S3_ENDPOINT_URL,
forcePathStyle: mockConstants.S3_FORCE_PATH_STYLE,
});
expect(result.ok).toBe(true);
if (result.ok) {
expect(result.data).toBeDefined();
}
});
test("should create S3 client with only bucket name (minimal config for IAM roles)", async () => {
// Mock constants with only bucket name - minimal required config
vi.doMock("./constants", () => ({
S3_ACCESS_KEY: undefined,
S3_SECRET_KEY: undefined,
S3_REGION: undefined,
S3_BUCKET_NAME: "test-bucket",
S3_ENDPOINT_URL: undefined,
S3_FORCE_PATH_STYLE: false,
}));
const { createS3ClientFromEnv } = await import("./client");
const result = createS3ClientFromEnv();
expect(mockS3Client).toHaveBeenCalledWith({
endpoint: undefined,
forcePathStyle: false,
});
expect(result.ok).toBe(true);
if (result.ok) {
expect(result.data).toBeDefined();
}
});
@@ -318,10 +353,9 @@ describe("client.ts", () => {
});
test("should return undefined when creating from env fails and no client provided", async () => {
// Mock constants with missing required fields (region and bucket)
// Mock constants with missing required field (bucket name only)
vi.doMock("./constants", () => ({
...mockConstants,
S3_REGION: undefined,
S3_BUCKET_NAME: undefined,
}));
@@ -354,7 +388,6 @@ describe("client.ts", () => {
test("returns undefined when env is invalid and does not construct client", async () => {
vi.doMock("./constants", () => ({
...mockConstants,
S3_REGION: undefined,
S3_BUCKET_NAME: undefined,
}));

View File

@@ -19,9 +19,9 @@ let cachedS3Client: S3Client | undefined;
*/
export const createS3ClientFromEnv = (): Result<S3Client, StorageError> => {
try {
// S3_REGION and S3_BUCKET_NAME are always required
if (!S3_BUCKET_NAME || !S3_REGION) {
logger.error("S3 Client: S3_REGION and S3_BUCKET_NAME are required");
// Only S3_BUCKET_NAME is required - S3_REGION is optional and will default to AWS SDK defaults
if (!S3_BUCKET_NAME) {
logger.error("S3 Client: S3_BUCKET_NAME is required");
return err({
code: StorageErrorCode.S3CredentialsError,
});
@@ -29,11 +29,15 @@ export const createS3ClientFromEnv = (): Result<S3Client, StorageError> => {
// Build S3 client configuration
const s3Config: S3ClientConfig = {
region: S3_REGION,
endpoint: S3_ENDPOINT_URL,
forcePathStyle: S3_FORCE_PATH_STYLE,
};
// Only set region if it's provided, otherwise let AWS SDK use its defaults
if (S3_REGION) {
s3Config.region = S3_REGION;
}
// Only add credentials if both access key and secret key are provided
// This allows the AWS SDK to use IAM roles, instance profiles, or other credential providers
if (S3_ACCESS_KEY && S3_SECRET_KEY) {