From 21cdd01016aa49501c904f899d150da9ac8b4c1d Mon Sep 17 00:00:00 2001 From: TheMeinerLP Date: Wed, 14 Oct 2026 09:26:40 +0200 Subject: [PATCH 1/3] fix(storage): make cloudflare r2 work with account-id only Detect r2 from account-id with the built-in default endpoint or from an r2 host, force region auto and path-style, validate credentials, log resolved settings. Claude-Session: https://claude.ai/code/session_01CAJNYTHfbTcqGiWQnaecY7 --- .../bluemap/s3/storage/GenericS3Profile.java | 4 +- .../bluemap/s3/storage/ProviderProfile.java | 9 +- .../bluemap/s3/storage/R2Profile.java | 106 +++++++- .../bluemap/s3/storage/S3Configuration.java | 2 +- .../s3/storage/S3FileSystemFactory.java | 8 + .../s3/storage/S3StorageConfiguration.java | 43 +++- .../s3/storage/ProviderProfilesTest.java | 227 ++++++++++++++++++ .../bluemap/s3/storage/TestConfig.java | 121 ++++++++++ 8 files changed, 495 insertions(+), 25 deletions(-) create mode 100644 src/test/java/dev/themeinerlp/bluemap/s3/storage/ProviderProfilesTest.java create mode 100644 src/test/java/dev/themeinerlp/bluemap/s3/storage/TestConfig.java diff --git a/src/main/java/dev/themeinerlp/bluemap/s3/storage/GenericS3Profile.java b/src/main/java/dev/themeinerlp/bluemap/s3/storage/GenericS3Profile.java index ee690a2..bde269b 100644 --- a/src/main/java/dev/themeinerlp/bluemap/s3/storage/GenericS3Profile.java +++ b/src/main/java/dev/themeinerlp/bluemap/s3/storage/GenericS3Profile.java @@ -17,6 +17,8 @@ */ package dev.themeinerlp.bluemap.s3.storage; +import java.util.List; + /** * Fallback profile: passes the configured endpoint-url/force-path-style/region through * unchanged. Always applies, and {@link ProviderProfiles} tries it last so any provider-specific @@ -38,6 +40,6 @@ public boolean appliesTo(S3Configuration cfg) { @Override public ResolvedEndpoint resolve(S3Configuration cfg) { - return new ResolvedEndpoint(cfg.getEndpointUrl(), cfg.forcePathStyle(), cfg.getRegion()); + return new ResolvedEndpoint(ID, cfg.getEndpointUrl(), cfg.forcePathStyle(), cfg.getRegion(), List.of()); } } diff --git a/src/main/java/dev/themeinerlp/bluemap/s3/storage/ProviderProfile.java b/src/main/java/dev/themeinerlp/bluemap/s3/storage/ProviderProfile.java index 8ef5ae9..4540f46 100644 --- a/src/main/java/dev/themeinerlp/bluemap/s3/storage/ProviderProfile.java +++ b/src/main/java/dev/themeinerlp/bluemap/s3/storage/ProviderProfile.java @@ -17,6 +17,8 @@ */ package dev.themeinerlp.bluemap.s3.storage; +import java.util.List; + /** * Resolves a provider's own shorthand config (e.g. Cloudflare R2's account-id) into the plain * connection settings {@link S3FileSystemFactory} actually needs. Keeps provider-specific @@ -39,5 +41,10 @@ interface ProviderProfile { /** Only called once this profile has been selected, either explicitly or via {@link #appliesTo}. */ ResolvedEndpoint resolve(S3Configuration cfg); - record ResolvedEndpoint(String endpointUrl, boolean forcePathStyle, String region) {} + /** + * @param profileId id of the profile that produced this result + * @param warnings non-fatal notes the caller should log (never contains secrets) + */ + record ResolvedEndpoint( + String profileId, String endpointUrl, boolean forcePathStyle, String region, List warnings) {} } diff --git a/src/main/java/dev/themeinerlp/bluemap/s3/storage/R2Profile.java b/src/main/java/dev/themeinerlp/bluemap/s3/storage/R2Profile.java index 962a44e..a5a1b95 100644 --- a/src/main/java/dev/themeinerlp/bluemap/s3/storage/R2Profile.java +++ b/src/main/java/dev/themeinerlp/bluemap/s3/storage/R2Profile.java @@ -17,18 +17,24 @@ */ package dev.themeinerlp.bluemap.s3.storage; +import java.net.URI; +import java.util.List; +import java.util.Locale; + /** - * Cloudflare R2: derives the endpoint from account-id so users don't have to look up/type the - * full https://<account-id>.r2.cloudflarestorage.com URL themselves. Applies only when - * account-id is set and no explicit endpoint-url was given, so setting endpoint-url always - * still wins (e.g. for R2's jurisdictional/EU-restricted endpoints). + * Cloudflare R2. Applies when account-id is set and endpoint-url is blank or still the built-in + * default (the endpoint is then derived as https://<account-id>.r2.cloudflarestorage.com), + * or when endpoint-url itself points at an R2 host (e.g. the EU jurisdiction endpoint), in which + * case that endpoint is used as-is and account-id is not needed. * - *

R2 only supports path-style access, so this always forces it on regardless of - * force-path-style. + *

R2 only supports path-style access and signs with region "auto", so both are forced + * regardless of force-path-style/region (a differing region is reported as a warning). */ final class R2Profile implements ProviderProfile { static final String ID = "r2"; + static final String HOST_SUFFIX = ".r2.cloudflarestorage.com"; + static final String REGION = "auto"; @Override public String id() { @@ -37,13 +43,93 @@ public String id() { @Override public boolean appliesTo(S3Configuration cfg) { - boolean noExplicitEndpoint = cfg.getEndpointUrl() == null || cfg.getEndpointUrl().isBlank(); - return noExplicitEndpoint && cfg.getAccountId() != null && !cfg.getAccountId().isBlank(); + if (isR2Endpoint(cfg.getEndpointUrl())) { + return true; + } + return hasAccountId(cfg) && endpointUnset(cfg.getEndpointUrl()); } @Override public ResolvedEndpoint resolve(S3Configuration cfg) { - String endpoint = "https://" + cfg.getAccountId() + ".r2.cloudflarestorage.com"; - return new ResolvedEndpoint(endpoint, true, cfg.getRegion()); + String endpoint; + if (isR2Endpoint(cfg.getEndpointUrl())) { + endpoint = cfg.getEndpointUrl().trim(); + } else if (hasAccountId(cfg)) { + endpoint = "https://" + cfg.getAccountId().trim() + HOST_SUFFIX; + } else { + throw new IllegalArgumentException("Provider 'r2' needs either 'account-id' or an 'endpoint-url' " + + "ending in " + HOST_SUFFIX); + } + requireCredential("access-key-id", cfg.getAccessKeyId(), S3StorageConfiguration.DEFAULT_ACCESS_KEY_ID); + requireCredential( + "secret-access-key", cfg.getSecretAccessKey(), S3StorageConfiguration.DEFAULT_SECRET_ACCESS_KEY); + + List warnings = List.of(); + String region = cfg.getRegion(); + if (!isAutoAlias(region)) { + warnings = List.of("Cloudflare R2 only accepts region 'auto' (or the aliases 'us-east-1'/empty); " + + "ignoring configured region '" + region + "' and using 'auto'"); + } + return new ResolvedEndpoint(ID, endpoint, true, REGION, warnings); + } + + /** Replaces the account-id label of an R2 host with {@code ***} so logs don't leak it. */ + static String maskEndpoint(String endpoint) { + if (endpoint == null || endpoint.isBlank()) { + return ""; + } + String host = hostOf(endpoint); + if (host == null || !host.toLowerCase(Locale.ROOT).endsWith(HOST_SUFFIX)) { + return endpoint; + } + int dot = host.indexOf('.'); + if (dot <= 0) { + return endpoint; + } + int at = endpoint.indexOf(host); + return endpoint.substring(0, at) + "***" + endpoint.substring(at + dot); + } + + static boolean isR2Endpoint(String endpoint) { + String host = hostOf(endpoint); + return host != null && host.toLowerCase(Locale.ROOT).endsWith(HOST_SUFFIX); + } + + private static String hostOf(String endpoint) { + if (endpoint == null || endpoint.isBlank()) { + return null; + } + try { + return URI.create(endpoint.trim()).getHost(); + } catch (IllegalArgumentException e) { + return null; + } + } + + private static boolean hasAccountId(S3Configuration cfg) { + return cfg.getAccountId() != null && !cfg.getAccountId().isBlank(); + } + + private static boolean endpointUnset(String endpoint) { + return endpoint == null + || endpoint.isBlank() + || endpoint.trim().equals(S3StorageConfiguration.DEFAULT_ENDPOINT_URL); + } + + private static boolean isAutoAlias(String region) { + if (region == null || region.isBlank()) { + return true; + } + String r = region.trim(); + return r.equalsIgnoreCase(S3StorageConfiguration.DEFAULT_REGION) + || r.equalsIgnoreCase(REGION) + || r.equalsIgnoreCase("us-east-1"); + } + + private static void requireCredential(String key, String value, String placeholder) { + if (value == null || value.isBlank() || value.equals(placeholder)) { + throw new IllegalArgumentException("Cloudflare R2 needs a real '" + key + + "' (create an R2 API token); it is missing or still the built-in placeholder"); + } } } diff --git a/src/main/java/dev/themeinerlp/bluemap/s3/storage/S3Configuration.java b/src/main/java/dev/themeinerlp/bluemap/s3/storage/S3Configuration.java index 1803c57..3bb824f 100644 --- a/src/main/java/dev/themeinerlp/bluemap/s3/storage/S3Configuration.java +++ b/src/main/java/dev/themeinerlp/bluemap/s3/storage/S3Configuration.java @@ -17,7 +17,7 @@ */ package dev.themeinerlp.bluemap.s3.storage; -public sealed interface S3Configuration permits S3StorageConfiguration { +public interface S3Configuration { String getBucketName(); diff --git a/src/main/java/dev/themeinerlp/bluemap/s3/storage/S3FileSystemFactory.java b/src/main/java/dev/themeinerlp/bluemap/s3/storage/S3FileSystemFactory.java index 09bbe1c..1ac96d4 100644 --- a/src/main/java/dev/themeinerlp/bluemap/s3/storage/S3FileSystemFactory.java +++ b/src/main/java/dev/themeinerlp/bluemap/s3/storage/S3FileSystemFactory.java @@ -17,6 +17,7 @@ */ package dev.themeinerlp.bluemap.s3.storage; +import de.bluecolored.bluemap.core.logger.Logger; import java.net.URI; import java.nio.file.FileSystem; import java.util.Objects; @@ -40,6 +41,13 @@ public static S3Fs build(S3Configuration cfg) { } final ProviderProfile.ResolvedEndpoint resolved = ProviderProfiles.resolve(cfg); + resolved.warnings().forEach(Logger.global::logWarning); + Logger.global.logInfo("S3 storage: profile=" + resolved.profileId() + + ", endpoint=" + R2Profile.maskEndpoint(resolved.endpointUrl()) + + ", bucket=" + cfg.getBucketName() + + ", region=" + resolved.region() + + ", path-style=" + resolved.forcePathStyle() + + ", checksum-validation=" + cfg.getChecksumValidation()); final boolean thirdParty = resolved.endpointUrl() != null && !resolved.endpointUrl().isBlank(); try { final URI uri; diff --git a/src/main/java/dev/themeinerlp/bluemap/s3/storage/S3StorageConfiguration.java b/src/main/java/dev/themeinerlp/bluemap/s3/storage/S3StorageConfiguration.java index 61a0d54..4445903 100644 --- a/src/main/java/dev/themeinerlp/bluemap/s3/storage/S3StorageConfiguration.java +++ b/src/main/java/dev/themeinerlp/bluemap/s3/storage/S3StorageConfiguration.java @@ -29,23 +29,41 @@ @ConfigSerializable public final class S3StorageConfiguration extends StorageConfig implements S3Configuration { + + /** Built-in endpoint-url default (a local MinIO); R2 detection treats it as "not set". */ + static final String DEFAULT_ENDPOINT_URL = "http://localhost:9000"; + /** Built-in region default; R2 detection treats it as "not set". */ + static final String DEFAULT_REGION = "Minio"; + /** Built-in access-key-id placeholder; R2 refuses to start with it. */ + static final String DEFAULT_ACCESS_KEY_ID = "bluemap"; + /** Built-in secret-access-key placeholder; R2 refuses to start with it. */ + static final String DEFAULT_SECRET_ACCESS_KEY = "bluemap-secret"; @Comment("The name of the S3 bucket to use") private String bucketName = "bluemap-storage"; - @Comment("The AWS region where the bucket is located") - private String region = "Minio"; + @Comment(""" + The region used to sign requests. Generic S3: set the region of your bucket. + Cloudflare R2: leave unset - R2 always uses "auto" (the defaults "Minio", + "us-east-1" and an empty value are mapped to it, any other value is ignored with a warning). + """) + private String region = DEFAULT_REGION; @Comment("The AWS access key ID for authentication. Leave empty together with secret-access-key to use the default AWS credential chain (environment, profile, ECS task role, EC2 instance profile)") - private String accessKeyId = "bluemap"; + private String accessKeyId = DEFAULT_ACCESS_KEY_ID; @Comment("The AWS secret access key for authentication. Leave empty together with access-key-id to use the default AWS credential chain") @DebugDump(exclude = true) - private String secretAccessKey = "bluemap-secret"; + private String secretAccessKey = DEFAULT_SECRET_ACCESS_KEY; - @Comment("Optional: The endpoint URL for S3-compatible services (leave empty for AWS S3)") + @Comment(""" + The endpoint URL for S3-compatible services (leave empty for AWS S3). + Cloudflare R2: leave unset and set account-id instead, or set the full R2 endpoint + (e.g. https://.eu.r2.cloudflarestorage.com for the EU jurisdiction); + an endpoint ending in .r2.cloudflarestorage.com selects the R2 profile automatically. + """) @DebugDump(exclude = true) - private String endpointUrl = "http://localhost:9000"; + private String endpointUrl = DEFAULT_ENDPOINT_URL; @Comment("The compression type to use for storing data (default: gzip)") private String compression = "gzip"; // Default compression type @@ -68,17 +86,18 @@ public final class S3StorageConfiguration extends StorageConfig implements S3Con @Comment(""" Optional: explicitly picks which provider profile resolves this config, one of: - "r2", "generic". Leave empty to auto-detect: setting account-id below picks "r2" - automatically, otherwise "generic" (uses endpoint-url/force-path-style as-is). + "r2", "generic". Leave empty to auto-detect: setting account-id below, or an endpoint-url + ending in .r2.cloudflarestorage.com, picks "r2"; otherwise "generic" + (uses endpoint-url/region/force-path-style as-is). Only needed if you want to be explicit rather than rely on auto-detection. """) private String provider = ""; @Comment(""" - Optional: Cloudflare account ID. If set and endpoint-url is left empty, the - endpoint is derived automatically as https://.r2.cloudflarestorage.com - and path-style access is enabled (required by R2). Leave empty for AWS S3 or any - other S3-compatible provider - use endpoint-url directly for those instead. + Optional: Cloudflare account ID. If set and endpoint-url is empty or left at its + default, the R2 profile is used: the endpoint becomes + https://.r2.cloudflarestorage.com, path-style access is enabled and the + region is "auto". Leave empty for AWS S3 or any other S3-compatible provider. """) @DebugDump(exclude = true) private String accountId = ""; diff --git a/src/test/java/dev/themeinerlp/bluemap/s3/storage/ProviderProfilesTest.java b/src/test/java/dev/themeinerlp/bluemap/s3/storage/ProviderProfilesTest.java new file mode 100644 index 0000000..8fd48b6 --- /dev/null +++ b/src/test/java/dev/themeinerlp/bluemap/s3/storage/ProviderProfilesTest.java @@ -0,0 +1,227 @@ +/** + * A simple storage implementation for bluemap to save data into s3 storage solution. + * Copyright (C) 2026 TheMeinerLP and contributors + *

+ * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + *

+ * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + *

+ * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ +package dev.themeinerlp.bluemap.s3.storage; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import org.junit.jupiter.api.Test; + +class ProviderProfilesTest { + + private static final String EU_ENDPOINT = "https://acc123.eu.r2.cloudflarestorage.com"; + + private static ProviderProfile.ResolvedEndpoint resolve(TestConfig cfg) { + return ProviderProfiles.resolve(cfg); + } + + @Test + void accountIdOnlySelectsR2WithDerivedEndpointAndAutoRegion() { + var r = resolve(new TestConfig().accountId("acc123")); + assertEquals("r2", r.profileId()); + assertEquals("https://acc123.r2.cloudflarestorage.com", r.endpointUrl()); + assertEquals("auto", r.region()); + assertTrue(r.forcePathStyle(), "R2 requires path-style"); + assertTrue(r.warnings().isEmpty(), "defaults must not warn"); + } + + @Test + void accountIdWithBlankEndpointSelectsR2() { + var r = resolve(new TestConfig().accountId("acc123").endpointUrl(" ")); + assertEquals("r2", r.profileId()); + assertEquals("https://acc123.r2.cloudflarestorage.com", r.endpointUrl()); + } + + @Test + void accountIdWithNullEndpointSelectsR2() { + var r = resolve(new TestConfig().accountId("acc123").endpointUrl(null)); + assertEquals("r2", r.profileId()); + } + + @Test + void explicitEuEndpointWithoutAccountIdSelectsR2() { + var r = resolve(new TestConfig().endpointUrl(EU_ENDPOINT)); + assertEquals("r2", r.profileId()); + assertEquals(EU_ENDPOINT, r.endpointUrl()); + assertTrue(r.forcePathStyle(), "R2 requires path-style"); + assertEquals("auto", r.region()); + } + + @Test + void r2HostMatchIsCaseInsensitive() { + var r = resolve(new TestConfig().endpointUrl("https://ACC.EU.R2.CloudflareStorage.com")); + assertEquals("r2", r.profileId()); + assertEquals("https://ACC.EU.R2.CloudflareStorage.com", r.endpointUrl()); + } + + @Test + void explicitR2EndpointWinsOverDerivedEndpointWhenAccountIdAlsoSet() { + var r = resolve(new TestConfig().accountId("other").endpointUrl(EU_ENDPOINT)); + assertEquals(EU_ENDPOINT, r.endpointUrl()); + } + + @Test + void r2HostSuffixMustMatchHostNotPath() { + var r = resolve(new TestConfig().endpointUrl("http://minio.local:9000/.r2.cloudflarestorage.com")); + assertEquals("generic", r.profileId()); + } + + @Test + void defaultRegionMapsToAuto() { + assertEquals("auto", resolve(new TestConfig().accountId("a").region("Minio")).region()); + assertEquals("auto", resolve(new TestConfig().accountId("a").region("minio")).region()); + } + + @Test + void blankRegionMapsToAuto() { + assertEquals("auto", resolve(new TestConfig().accountId("a").region(" ")).region()); + assertEquals("auto", resolve(new TestConfig().accountId("a").region(null)).region()); + } + + @Test + void autoAndUsEast1RegionsMapToAutoWithoutWarning() { + var auto = resolve(new TestConfig().accountId("a").region("auto")); + var us = resolve(new TestConfig().accountId("a").region("us-east-1")); + assertEquals("auto", auto.region()); + assertEquals("auto", us.region()); + assertTrue(auto.warnings().isEmpty(), "auto is valid"); + assertTrue(us.warnings().isEmpty(), "us-east-1 is an accepted alias"); + } + + @Test + void otherRegionStillUsesAutoAndSignalsWarning() { + var r = resolve(new TestConfig().accountId("a").region("eu-west-1")); + assertEquals("auto", r.region()); + assertEquals(1, r.warnings().size(), "exactly one warning expected"); + assertTrue(r.warnings().get(0).contains("eu-west-1"), "warning names the ignored region"); + assertTrue(r.warnings().get(0).contains("auto"), "warning names the accepted value"); + } + + @Test + void genericEndpointStaysGenericAndUnchanged() { + var r = resolve(new TestConfig().endpointUrl("http://minio:9000").region("Minio").forcePathStyle(true)); + assertEquals("generic", r.profileId()); + assertEquals("http://minio:9000", r.endpointUrl()); + assertEquals("Minio", r.region()); + assertTrue(r.forcePathStyle()); + } + + @Test + void genericKeepsPathStyleFalseAsConfigured() { + var r = resolve(new TestConfig().endpointUrl("http://minio:9000")); + assertFalse(r.forcePathStyle()); + } + + @Test + void defaultsWithoutAccountIdStayGeneric() { + var r = resolve(new TestConfig()); + assertEquals("generic", r.profileId()); + assertEquals(S3StorageConfiguration.DEFAULT_ENDPOINT_URL, r.endpointUrl()); + } + + @Test + void explicitGenericOverridesAccountId() { + var r = resolve(new TestConfig().provider("generic").accountId("acc").region("Minio")); + assertEquals("generic", r.profileId()); + assertEquals(S3StorageConfiguration.DEFAULT_ENDPOINT_URL, r.endpointUrl()); + assertEquals("Minio", r.region()); + } + + @Test + void explicitGenericOverridesR2Endpoint() { + var r = resolve(new TestConfig().provider("generic").endpointUrl(EU_ENDPOINT).region("x")); + assertEquals("generic", r.profileId()); + assertEquals("x", r.region()); + } + + @Test + void explicitR2WithoutAccountIdOrR2EndpointFailsNamingBothKeys() { + var e = assertThrows(IllegalArgumentException.class, () -> resolve(new TestConfig().provider("r2"))); + assertTrue(e.getMessage().contains("account-id"), e.getMessage()); + assertTrue(e.getMessage().contains("endpoint-url"), e.getMessage()); + } + + @Test + void explicitR2WithAccountIdResolves() { + var r = resolve(new TestConfig().provider("R2").accountId("acc")); + assertEquals("https://acc.r2.cloudflarestorage.com", r.endpointUrl()); + } + + @Test + void placeholderAccessKeyWithR2FailsNamingKey() { + var e = assertThrows( + IllegalArgumentException.class, + () -> resolve(new TestConfig().accountId("a").accessKeyId("bluemap"))); + assertTrue(e.getMessage().contains("access-key-id"), e.getMessage()); + } + + @Test + void blankAccessKeyWithR2FailsNamingKey() { + var e = assertThrows( + IllegalArgumentException.class, + () -> resolve(new TestConfig().accountId("a").accessKeyId(" "))); + assertTrue(e.getMessage().contains("access-key-id"), e.getMessage()); + } + + @Test + void placeholderSecretWithR2FailsNamingKeyWithoutLeakingIt() { + var e = assertThrows( + IllegalArgumentException.class, + () -> resolve(new TestConfig().accountId("a").secretAccessKey("bluemap-secret"))); + assertTrue(e.getMessage().contains("secret-access-key"), e.getMessage()); + assertFalse(e.getMessage().contains("bluemap-secret"), "secret must not be printed"); + } + + @Test + void blankSecretWithR2FailsNamingKey() { + var e = assertThrows( + IllegalArgumentException.class, + () -> resolve(new TestConfig().accountId("a").secretAccessKey(""))); + assertTrue(e.getMessage().contains("secret-access-key"), e.getMessage()); + } + + @Test + void placeholderCredentialsAreAcceptedForGeneric() { + var r = resolve(new TestConfig().accessKeyId("bluemap").secretAccessKey("bluemap-secret")); + assertEquals("generic", r.profileId()); + } + + @Test + void maskEndpointHidesAccountIdOfR2Host() { + assertEquals("https://***.eu.r2.cloudflarestorage.com", R2Profile.maskEndpoint(EU_ENDPOINT)); + assertEquals( + "https://***.r2.cloudflarestorage.com", + R2Profile.maskEndpoint("https://acc123.r2.cloudflarestorage.com")); + } + + @Test + void maskEndpointKeepsPortAndLeavesOtherHostsUntouched() { + assertEquals("http://minio:9000", R2Profile.maskEndpoint("http://minio:9000")); + assertEquals( + "https://***.r2.cloudflarestorage.com:8443", + R2Profile.maskEndpoint("https://acc.r2.cloudflarestorage.com:8443")); + } + + @Test + void maskEndpointToleratesNullAndBlank() { + assertEquals("", R2Profile.maskEndpoint(null)); + assertEquals("", R2Profile.maskEndpoint(" ")); + } +} diff --git a/src/test/java/dev/themeinerlp/bluemap/s3/storage/TestConfig.java b/src/test/java/dev/themeinerlp/bluemap/s3/storage/TestConfig.java new file mode 100644 index 0000000..3b7478a --- /dev/null +++ b/src/test/java/dev/themeinerlp/bluemap/s3/storage/TestConfig.java @@ -0,0 +1,121 @@ +/** + * A simple storage implementation for bluemap to save data into s3 storage solution. + * Copyright (C) 2026 TheMeinerLP and contributors + *

+ * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + *

+ * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + *

+ * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ +package dev.themeinerlp.bluemap.s3.storage; + +/** Plain-value {@link S3Configuration} so tests need no BlueMap classes. Each test builds its own. */ +final class TestConfig implements S3Configuration { + + String bucketName = "bucket"; + String region = S3StorageConfiguration.DEFAULT_REGION; + String accessKeyId = "real-key"; + String secretAccessKey = "real-secret"; + String endpointUrl = S3StorageConfiguration.DEFAULT_ENDPOINT_URL; + boolean forcePathStyle = false; + String accountId = ""; + String provider = ""; + + TestConfig region(String v) { + region = v; + return this; + } + + TestConfig accessKeyId(String v) { + accessKeyId = v; + return this; + } + + TestConfig secretAccessKey(String v) { + secretAccessKey = v; + return this; + } + + TestConfig endpointUrl(String v) { + endpointUrl = v; + return this; + } + + TestConfig forcePathStyle(boolean v) { + forcePathStyle = v; + return this; + } + + TestConfig accountId(String v) { + accountId = v; + return this; + } + + TestConfig provider(String v) { + provider = v; + return this; + } + + @Override + public String getBucketName() { + return bucketName; + } + + @Override + public String getRegion() { + return region; + } + + @Override + public String getAccessKeyId() { + return accessKeyId; + } + + @Override + public String getSecretAccessKey() { + return secretAccessKey; + } + + @Override + public String getEndpointUrl() { + return endpointUrl; + } + + @Override + public String getRootPath() { + return "."; + } + + @Override + public boolean forcePathStyle() { + return forcePathStyle; + } + + @Override + public String getChecksumValidation() { + return "when_required"; + } + + @Override + public String getAccountId() { + return accountId; + } + + @Override + public int getListCacheTtlSeconds() { + return 0; + } + + @Override + public String getProvider() { + return provider; + } +} From 8c920fb88063c6b48525a6ca489a020dbf068219 Mon Sep 17 00:00:00 2001 From: TheMeinerLP Date: Wed, 14 Oct 2026 09:42:42 +0200 Subject: [PATCH 2/3] fix(storage): report r2 config errors as configuration errors Claude-Session: https://claude.ai/code/session_01CAJNYTHfbTcqGiWQnaecY7 --- docs/reference/configuration.md | 12 ++++++------ .../bluemap/s3/storage/S3StorageConfiguration.java | 12 +++++++++++- 2 files changed, 17 insertions(+), 7 deletions(-) diff --git a/docs/reference/configuration.md b/docs/reference/configuration.md index ec9eef8..a2124e0 100644 --- a/docs/reference/configuration.md +++ b/docs/reference/configuration.md @@ -41,14 +41,14 @@ The name of the S3 bucket to use - Type: `String` - Default: `"Minio"` -The AWS region where the bucket is located +The region used to sign requests. Generic S3: set the region of your bucket. Cloudflare R2: leave unset - R2 always uses "auto" (the defaults "Minio", "us-east-1" and an empty value are mapped to it, any other value is ignored with a warning). ## access-key-id - Type: `String` - Default: `"bluemap"` -The AWS access key ID for authentication +The AWS access key ID for authentication. Leave empty together with secret-access-key to use the default AWS credential chain (environment, profile, ECS task role, EC2 instance profile) ## secret-access-key @@ -56,7 +56,7 @@ The AWS access key ID for authentication - Default: `"bluemap-secret"` - Sensitive: excluded from BlueMap debug dumps (`@DebugDump(exclude = true)`) -The AWS secret access key for authentication +The AWS secret access key for authentication. Leave empty together with access-key-id to use the default AWS credential chain ## endpoint-url @@ -64,7 +64,7 @@ The AWS secret access key for authentication - Default: `"http://localhost:9000"` - Sensitive: excluded from BlueMap debug dumps (`@DebugDump(exclude = true)`) -Optional: The endpoint URL for S3-compatible services (leave empty for AWS S3) +The endpoint URL for S3-compatible services (leave empty for AWS S3). Cloudflare R2: leave unset and set account-id instead, or set the full R2 endpoint (e.g. https://.eu.r2.cloudflarestorage.com for the EU jurisdiction); an endpoint ending in .r2.cloudflarestorage.com selects the R2 profile automatically. ## compression @@ -99,7 +99,7 @@ Controls when the AWS SDK attaches/validates request and response checksums (aws - Type: `String` - Default: `""` -Optional: explicitly picks which provider profile resolves this config, one of: "r2", "generic". Leave empty to auto-detect: setting account-id below picks "r2" automatically, otherwise "generic" (uses endpoint-url/force-path-style as-is). Only needed if you want to be explicit rather than rely on auto-detection. +Optional: explicitly picks which provider profile resolves this config, one of: "r2", "generic". Leave empty to auto-detect: setting account-id below, or an endpoint-url ending in .r2.cloudflarestorage.com, picks "r2"; otherwise "generic" (uses endpoint-url/region/force-path-style as-is). Only needed if you want to be explicit rather than rely on auto-detection. ## account-id @@ -107,7 +107,7 @@ Optional: explicitly picks which provider profile resolves this config, one of: - Default: `""` - Sensitive: excluded from BlueMap debug dumps (`@DebugDump(exclude = true)`) -Optional: Cloudflare account ID. If set and endpoint-url is left empty, the endpoint is derived automatically as https://.r2.cloudflarestorage.com and path-style access is enabled (required by R2). Leave empty for AWS S3 or any other S3-compatible provider - use endpoint-url directly for those instead. +Optional: Cloudflare account ID. If set and endpoint-url is empty or left at its default, the R2 profile is used: the endpoint becomes https://.r2.cloudflarestorage.com, path-style access is enabled and the region is "auto". Leave empty for AWS S3 or any other S3-compatible provider. ## list-cache-ttl-seconds diff --git a/src/main/java/dev/themeinerlp/bluemap/s3/storage/S3StorageConfiguration.java b/src/main/java/dev/themeinerlp/bluemap/s3/storage/S3StorageConfiguration.java index 4445903..8fb0d6e 100644 --- a/src/main/java/dev/themeinerlp/bluemap/s3/storage/S3StorageConfiguration.java +++ b/src/main/java/dev/themeinerlp/bluemap/s3/storage/S3StorageConfiguration.java @@ -118,7 +118,17 @@ public Storage createStorage() throws ConfigurationException { throw new ConfigurationException("S3 bucket name is required"); } - if (region == null || region.isEmpty()) { + // Resolve the provider profile up front so R2 misconfiguration is reported as a clean + // ConfigurationException instead of a stack trace from S3FileSystemFactory. + final ProviderProfile.ResolvedEndpoint resolved; + try { + resolved = ProviderProfiles.resolve(this); + } catch (IllegalArgumentException e) { + throw new ConfigurationException(e.getMessage(), e); + } + + // R2 maps an empty region to "auto", so only the other profiles need one. + if (!R2Profile.ID.equals(resolved.profileId()) && (region == null || region.isEmpty())) { throw new ConfigurationException("AWS region is required"); } From 29fc9b265c1e335e273bbdb88fd9840cee4b16d6 Mon Sep 17 00:00:00 2001 From: TheMeinerLP Date: Wed, 14 Oct 2026 09:42:49 +0200 Subject: [PATCH 3/3] docs: align r2 docs with final error messages Claude-Session: https://claude.ai/code/session_01CAJNYTHfbTcqGiWQnaecY7 --- docs/reference/errors.md | 8 ++++++-- docs/reference/provider-detection.md | 3 ++- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/docs/reference/errors.md b/docs/reference/errors.md index 3a6be6d..2ab182c 100644 --- a/docs/reference/errors.md +++ b/docs/reference/errors.md @@ -54,9 +54,13 @@ Resolve with: [Put your map on Cloudflare R2](../tutorial/put-your-map-on-cloudf The `provider` key holds a value that is not a profile id. See [Provider detection](provider-detection.md). -## S3 bucket name is required / AWS region is required / AWS access key ID is required / AWS secret access key is required +## S3 bucket name is required / AWS region is required -A required key is empty. See the [Configuration reference](configuration.md). +A required key is empty. `AWS region is required` does not apply to the `r2` profile, which maps a blank region to `auto`. See the [Configuration reference](configuration.md). + +## Set both access-key-id and secret-access-key, or leave both empty to use the default AWS credential chain + +Only one of `access-key-id` and `secret-access-key` is set. Set both, or leave both empty to use the default AWS credential chain. See the [Configuration reference](configuration.md). ## Failed to build S3 FileSystem diff --git a/docs/reference/provider-detection.md b/docs/reference/provider-detection.md index ee58e31..9abee5a 100644 --- a/docs/reference/provider-detection.md +++ b/docs/reference/provider-detection.md @@ -29,7 +29,8 @@ The addon resolves every storage configuration through one of two provider profi - `provider: "r2"` without `account-id` and without an R2 `endpoint-url` fails at startup with `Provider 'r2' needs either 'account-id' or an 'endpoint-url' ending in .r2.cloudflarestorage.com`. - The `r2` profile with a blank or placeholder `access-key-id` or `secret-access-key` fails at startup with `Cloudflare R2 needs a real '' (create an R2 API token); it is missing or still the built-in placeholder`, where `` is the offending key. -- `bucket-name`, `region`, `access-key-id` and `secret-access-key` must be non-empty for every profile. +- `bucket-name` must be non-empty for every profile. `region` must be non-empty for every profile except `r2`, which maps a blank region to `auto`. +- `access-key-id` and `secret-access-key` must be both set or both empty; the message is `Set both access-key-id and secret-access-key, or leave both empty to use the default AWS credential chain`. Both empty is valid for the `generic` profile, where the AWS default credential chain applies, but not for `r2`, which has no default credential chain. ## Startup log line