fix(s3): use EmulatorConfig for FLOCI_HOSTNAME to fix native image crash#4
Open
fix(s3): use EmulatorConfig for FLOCI_HOSTNAME to fix native image crash#4
Conversation
Meemaw
force-pushed
the
fix/s3-virtual-host-emulator-config
branch
2 times, most recently
from
813a367 to
978e5bf
Compare
…tFilter Replace @ConfigProperty constructor injection with programmatic ConfigProvider.getConfig() lookup to fix native image crash when FLOCI_HOSTNAME is set at runtime. @PreMatching JAX-RS filters run before CDI beans are fully initialized, so neither @ConfigProperty nor @ConfigMapping beans are available during filter construction. Also make bucket extraction hostname-aware: only treat the first label as a bucket name when the remainder matches the configured base hostname (or a well-known AWS S3 domain). This prevents false positives when Floci sits behind a multi-label hostname like floci.svc.cluster.local.
Meemaw
force-pushed
the
fix/s3-virtual-host-emulator-config
branch
from
978e5bf to
ef5c36d
Compare
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
Comment on lines
+19
to
+25
| public S3VirtualHostFilter() { | ||
| var config = ConfigProvider.getConfig(); | ||
| String baseUrl = config | ||
| .getOptionalValue("floci.base-url", String.class) | ||
| .orElse("http://localhost:4566"); | ||
| Optional<String> hostname = config | ||
| .getOptionalValue("floci.hostname", String.class); |
There was a problem hiding this comment.
🔴 Violation of AGENT.md "Use constructor injection" rule: replaces @Inject with manual ConfigProvider
The PR removes @Inject constructor injection and replaces it with ConfigProvider.getConfig(), directly violating the mandatory AGENT.md Code Style rule at line 268: "Use constructor injection". This is the only file in the entire codebase that uses ConfigProvider.getConfig() — every other class that needs configuration uses CDI injection (typically via EmulatorConfig or @ConfigProperty), as seen in ServiceEnabledFilter.java:29-32, PreSignedUrlFilter.java:16-19, AwsJsonMessageBodyWriter.java:15-18, and dozens of service classes. This also violates the AGENT.md rule "Follow existing project patterns" (line 272).
Prompt for agents
The S3VirtualHostFilter constructor was changed from CDI constructor injection (@Inject + @ConfigProperty) to manual ConfigProvider.getConfig() usage. This violates the AGENT.md mandatory rules: 'Use constructor injection' and 'Follow existing project patterns'.
The entire rest of the codebase uses CDI injection for configuration, either via EmulatorConfig (the type-safe config interface) or @ConfigProperty annotations. This is the only class using ConfigProvider directly.
To fix this, restore the @Inject-based constructor injection. The original approach was:
@Inject
public S3VirtualHostFilter(
@ConfigProperty(name = "floci.base-url", defaultValue = "http://localhost:4566") String baseUrl,
@ConfigProperty(name = "floci.hostname") Optional<String> hostname) {
// ... same body
}
Alternatively, consider injecting EmulatorConfig instead of individual @ConfigProperty values, which is the more common pattern in this codebase (used by S3Service, LambdaService, etc.).
If there was a genuine technical issue with CDI injection in @PreMatching filters, it should be documented and an exception to the rule should be noted in a code comment.
Was this helpful? React with 👍 or 👎 to provide feedback.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
S3VirtualHostFilter used @ConfigProperty injection in its constructor, which gets baked into the GraalVM native binary at build time. Setting FLOCI_HOSTNAME at runtime via Docker env var caused an IllegalStateException because the runtime value differed from the build-time value (null).
Refactor to inject EmulatorConfig (which uses @ConfigMapping and is runtime-safe) instead of raw @ConfigProperty, following the established pattern used by StorageFactory, ServiceRegistry, RegionResolver, etc.
Also make bucket extraction hostname-aware: only treat the first label as a bucket name when the remainder matches the configured base hostname (or a well-known AWS S3 domain). This prevents false positives when Floci sits behind a multi-label hostname like floci.svc.cluster.local.
Summary
Fixes native image crash when
FLOCI_HOSTNAMEis set at runtime, and prevents false-positive virtual-host bucket extraction in multi-label hostname environments (e.g. Kubernetes).Based on #3
Reproduction:
Type of change
fix:)feat:)feat!:orfix!:)AWS Compatibility
Incorrect behavior:
FLOCI_HOSTNAMEenv var crashes the native image on startup. Without it, virtual-host bucket extraction has no hostname awareness, causing false positives in Docker Compose / Kubernetes setups.Checklist
./mvnw testpasses locally