feat(config): implement configuration management subsystem#60
Merged
Conversation
…endencies Bump the parent POM to 1.5.1 and add the YAML/JSON-Schema config parsing and validation dependencies required by the configuration-management feature (deliverable 1). Co-Authored-By: Claude <noreply@anthropic.com>
… tests Introduce the immutable Java record hierarchy under de.cuioss.sheriff.api.config.model (GatewayConfig, EndpointConfig with id/enabled/allowedMethods, the HttpMethod enum excluding TRACE/CONNECT, and the supporting gateway/route/upstream records) as the framework-agnostic binding target for the YAML loader, plus the ConfigModelContractTest value-object contract suite covering equals/hashCode/toString, builder equivalence, null-normalization, defensive copying, mandatory-field enforcement, and the enum contracts (deliverable 3). Co-Authored-By: Claude <noreply@anthropic.com>
Align multi-line record-component continuation lines and normalize import ordering in the configuration model records and ConfigModelContractTest as enforced by the pre-commit spotless profile. Co-Authored-By: Claude <noreply@anthropic.com>
Add the draft 2020-12 JSON Schemas that mirror the configuration reference field-for-field and reject unknown keys at every object level (additionalProperties:false), forming the structural-validation contract the YAML loader enforces at boot (deliverable 2). Co-Authored-By: Claude <noreply@anthropic.com>
Add the framework-agnostic boot loader under de.cuioss.sheriff.api.config.load:
ConfigLoader reads gateway.yaml and endpoints/*.yaml (sorted), validates each
document against the D2 JSON Schemas via networknt, resolves ${ENV_VAR} secret
references through EnvSecretResolver, and binds the valid trees to the immutable
D3 model records with Jackson (snake_case mapping, case-insensitive enums, and a
custom deserializer flattening the nested upstream_defaults block). Every schema,
secret, and binding problem is aggregated with file + JSON-pointer context and
raised together as ConfigLoadException — never failing on the first. Includes
ConfigError, ConfigLoadException, the loader/resolver unit tests, and valid /
invalid configuration fixtures (deliverable 4).
The endpoint-enablement, topology-alias, cross-cutting validation, and
route-table steps of the full boot sequence are layered on by later deliverables.
Co-Authored-By: Claude <noreply@anthropic.com>
Add the framework-agnostic boot-pipeline resolvers under de.cuioss.sheriff.api.config.topology: EndpointEnablementResolver computes each endpoint's effective enabled state (ENDPOINT_<ID>_ENABLED environment override over the file default) and drops disabled endpoints, and TopologyResolver resolves the topology aliases referenced by the surviving enabled endpoints (TOPOLOGY_<ALIAS> override over topology.properties), validates each URL, and decomposes it once into the immutable ResolvedUpstream (scheme/host/port/basePath, ADR-0004). Adds the ResolvedTopology / ResolvedUpstream model records and the resolver unit tests. A missing or malformed alias referenced by an enabled endpoint is a boot failure; disabled endpoints are exempt from alias resolution (deliverable 5). Co-Authored-By: Claude <noreply@anthropic.com>
…nd tests Co-Authored-By: Claude <noreply@anthropic.com>
…hods, retry/not-modified Co-Authored-By: Claude <noreply@anthropic.com>
…OADED LogRecords Co-Authored-By: Claude <noreply@anthropic.com>
…figuration The proxy edge now sources each route's upstream from the RouteTable assembled by the configuration subsystem instead of a static ProxyConfiguration bean, which is deleted. ProxyRouteTest and the @QuarkusTest boot config are rewired: the shared testboot config gains a /proxy endpoint whose UPSTREAM topology alias resolves to the MockWebServer port. Produce the immutable GatewayConfig and RouteTable records under the @singleton pseudo-scope: ArC cannot build a client proxy for a normal scope (@ApplicationScoped) over a final record type, which failed every @QuarkusTest container at boot once test-compile went green. Co-Authored-By: Claude <noreply@anthropic.com>
Replace the removed SHERIFF_PROXY_* env with a mounted sheriff-config directory (SHERIFF_CONFIG_DIR): gateway.yaml, endpoints/httpbin.yaml and topology.properties assemble the RouteTable whose UPSTREAM alias resolves to the go-httpbin echo backend. The benchmark overlay repoints the target via the TOPOLOGY_UPSTREAM alias override rather than a proxy env var. Add ConfigLoadedIntegrationIT (the mounted /proxy route forwards, unmatched paths deny by default, container boots healthy) and verify-invalid-config-fails.sh, a negative check that an invalid mounted config makes the container fail fast with a non-zero exit. Co-Authored-By: Claude <noreply@anthropic.com>
…g LogMessages Document the two bundled JSON Schemas (gateway/endpoint, Draft 2020-12, additionalProperties:false) and the upstream_defaults retry / not-modified toggles with their three-level wholesale-replace inheritance, matching RouteTableBuilder. Existing endpoint id/enabled/allowed_methods sections are left untouched. Catalogue the config LogRecords in doc/LogMessages.adoc — ApiSheriff-002 CONFIG_LOADED (INFO), ApiSheriff-200 CONFIG_VALIDATION_FAILED and ApiSheriff-201 CONFIG_STARTUP_ABORTED (ERROR) — matching ConfigLogMessages. Co-Authored-By: Claude <noreply@anthropic.com>
cui-http 2.1 is released: update every doc/plan cui-http:2.1-SNAPSHOT reference to cui-http:2.1 and drop the snapshot-risk caveats that no longer apply (token-sheriff stays 1.0.0-SNAPSHOT). Relocate the delivered Plan 02 spec out of doc/plan/ into the plan workspace and sweep all 5 link:02-configuration-management.adoc references across 4 files to plain-text 'Plan 02 (delivered)' notes, so no dangling link survives. Co-Authored-By: Claude <noreply@anthropic.com>
OpenRewrite pre-commit normalizations for the config subsystem: canonical import ordering in ConfigProducer, an import-group blank line in three config tests, and broadened throws Exception on the ConfigProducer/ConfigFailFast test methods. Committed so the quality gate is idempotent on these files. Co-Authored-By: Claude <noreply@anthropic.com>
Security-audit hardening: OidcConfig and its nested Session record used the
default Java-record toString(), which printed the resolved clientSecret,
encryptionKey, and previousKey verbatim. ConfigLoader substitutes ${ENV_VAR}
references with real secret values before binding, so an unredacted toString()
would leak them into any log line, exception message, or debugger view.
Added redacting toString() overrides that preserve presence without exposing
the values.
Co-Authored-By: Claude <noreply@anthropic.com>
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Sorry @cuioss-oliver, your pull request is larger than the review limit of 150000 diff characters
📝 WalkthroughWalkthroughChangesThe pull request adds a file-based configuration subsystem with immutable models, YAML/schema loading, environment secret resolution, topology and route resolution, semantic validation, Quarkus startup wiring, and route-table-driven proxy forwarding. It also updates integration fixtures, documentation, dependency management, native-image resources, and tests. Configuration contracts and loading
Startup and integration
Estimated code review effort: 4 (Complex) | ~60 minutes 🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements the complete configuration management subsystem for the API Sheriff gateway, introducing JSON schemas for global and endpoint configurations, an immutable configuration model, and a robust boot-time loading and validation pipeline. The interim proxy edge has been updated to dynamically source its routes and upstreams from the assembled RouteTable. Review feedback identifies several key areas for improvement: refining path prefix matching in RouteTable to prevent incorrect partial matches, updating header disjointness checks in RouteTableBuilder to respect the present attribute, trimming resolved topology URLs in TopologyResolver to prevent parsing failures, and ensuring timeout values are strictly positive during validation in ConfigValidator.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
api-sheriff/src/main/java/de/cuioss/sheriff/api/gateway/proxy/ProxyRoute.java (1)
92-113: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winAdd a per-request timeout to the upstream
HttpRequest.The
HttpClientsets onlyconnectTimeout(10s). Once connected,httpClient.send()(line 164) blocks the virtual thread indefinitely if the upstream hangs or sends data slowly. No server-side read timeout is configured inapplication.propertieseither, so a slow upstream will hold both the virtual thread and the client connection with no upper bound.⏱️ Proposed fix: add per-request timeout
HttpRequest.Builder builder = HttpRequest.newBuilder(URI.create(target)); + builder.timeout(Duration.ofSeconds(30)); request.headers().forEach(header -> {If you prefer a single configurable value, inject it via
@ConfigProperty:+ `@ConfigProperty`(name = "sheriff.proxy.upstream-timeout-seconds", defaultValue = "30") + int upstreamTimeoutSeconds; + // in buildUpstreamRequest: HttpRequest.Builder builder = HttpRequest.newBuilder(URI.create(target)); - // no timeout + builder.timeout(Duration.ofSeconds(upstreamTimeoutSeconds));🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@api-sheriff/src/main/java/de/cuioss/sheriff/api/gateway/proxy/ProxyRoute.java` around lines 92 - 113, Update the upstream request creation in ProxyRoute so each HttpRequest used by httpClient.send() has a bounded per-request timeout, covering response waits after connection and slow upstreams. Use the existing Duration/configuration approach, and preserve the current client connection timeout and request behavior.
🧹 Nitpick comments (4)
api-sheriff/src/main/java/de/cuioss/sheriff/api/config/model/OidcConfig.java (1)
175-184: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueCsrf is the only nested record without
@Builder.All other nested records in
OidcConfig(Logout,Session,Refresh,StepUp) are annotated with@Builder, butCsrfis not. If this is intentional (e.g., Csrf is always constructed inline), consider documenting why. Otherwise, adding@Builderkeeps the pattern consistent and allows builder-based construction in config loading or tests.♻️ Optional: add `@Builder` to Csrf for consistency
+@Builder public record Csrf(List<String> trustedOrigins) {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@api-sheriff/src/main/java/de/cuioss/sheriff/api/config/model/OidcConfig.java` around lines 175 - 184, Add Lombok `@Builder` to the OidcConfig.Csrf record, preserving its existing compact canonical constructor and trustedOrigins normalization. Keep the annotation consistent with the other nested records without changing construction behavior.api-sheriff/src/main/java/de/cuioss/sheriff/api/config/validation/ConfigValidator.java (1)
167-181: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider including the route ID in the error path for method membership violations.
validateWholeSecondTimeouts(line 200) uses"/endpoint/routes/%s/upstream/%s".formatted(route.id(), field)for precise error localization, butvalidateMethodMembershipuses the generic"/endpoint/routes"for all violations. Adding the route ID would make debugging easier when multiple routes in the same endpoint have method violations.♻️ Optional refactor for error path consistency
- errors.add(new ConfigError(endpointFile(endpoint), "/endpoint/routes", + errors.add(new ConfigError(endpointFile(endpoint), "/endpoint/routes/%s".formatted(route.id()), "route '%s' matches method %s outside the effective allowed_methods" .formatted(route.id(), method)));🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@api-sheriff/src/main/java/de/cuioss/sheriff/api/config/validation/ConfigValidator.java` around lines 167 - 181, Update validateMethodMembership so each method membership ConfigError uses a route-specific path containing route.id(), consistent with the precise route paths used by validateWholeSecondTimeouts; keep the existing endpoint file and message content unchanged.api-sheriff/src/main/resources/schema/endpoint.schema.json (1)
136-161: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd
minimumconstraints to integer fields in the schema.Several integer fields accept negative values through schema validation:
connect_timeout_ms,read_timeout_ms,max_attempts,failures,reset_ms, and the security-filter limits (max_header_count,max_header_value_length,max_query_params,max_param_value_length,max_body_bytes). While the semanticConfigValidatorlayer may catch these, schema-level constraints provide earlier, pointer-annotated error messages and serve as defense-in-depth.♻️ Proposed additions for the upstream and security-filter integer fields
"path": { "type": "string" }, - "connect_timeout_ms": { "type": "integer" }, - "read_timeout_ms": { "type": "integer" }, + "connect_timeout_ms": { "type": "integer", "minimum": 0 }, + "read_timeout_ms": { "type": "integer", "minimum": 0 }, "retry": { "type": "object", "additionalProperties": false, "properties": { "enabled": { "type": "boolean" }, - "max_attempts": { "type": "integer" }, + "max_attempts": { "type": "integer", "minimum": 0 }, "idempotent_only": { "type": "boolean" } } }, @@ circuit_breaker @@ "properties": { - "failures": { "type": "integer" }, - "reset_ms": { "type": "integer" } + "failures": { "type": "integer", "minimum": 0 }, + "reset_ms": { "type": "integer", "minimum": 0 } } @@ security_filter @@ - "max_header_count": { "type": "integer" }, - "max_header_value_length": { "type": "integer" }, - "max_query_params": { "type": "integer" }, - "max_param_value_length": { "type": "integer" }, - "max_body_bytes": { "type": "integer" }, + "max_header_count": { "type": "integer", "minimum": 0 }, + "max_header_value_length": { "type": "integer", "minimum": 0 }, + "max_query_params": { "type": "integer", "minimum": 0 }, + "max_param_value_length": { "type": "integer", "minimum": 0 }, + "max_body_bytes": { "type": "integer", "minimum": 0 },🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@api-sheriff/src/main/resources/schema/endpoint.schema.json` around lines 136 - 161, Add non-negative minimum constraints to the integer properties connect_timeout_ms, read_timeout_ms, retry.max_attempts, circuit_breaker.failures, circuit_breaker.reset_ms, and the security-filter limits max_header_count, max_header_value_length, max_query_params, max_param_value_length, and max_body_bytes in endpoint.schema.json, while preserving their existing integer types and structure.api-sheriff/src/test/java/de/cuioss/sheriff/api/config/RouteTableBuilderTest.java (1)
103-196: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd tests for header-based disjointness.
The
headersDistinguishmethod inRouteTableBuilder(lines 164-175) has non-trivial nested-loop logic (case-insensitive name matching, Optional value comparison) that is currently untested. The existing disjointness tests only cover method-based overlap. Consider adding tests for:
- Two same-prefix routes with the same header name but different values → should be accepted (distinguished).
- Two same-prefix routes with the same header name and same value → should be rejected (not distinguished).
🧪 Suggested test additions
`@Test` + `@DisplayName`("Should accept same-prefix routes distinguished by header value") + void shouldAcceptSamePrefixRoutesDistinguishedByHeader() { + MatchConfig matchA = MatchConfig.builder().pathPrefix("/x").methods(List.of(HttpMethod.GET)) + .headers(List.of(new MatchConfig.HeaderMatcher("X-Mode", Optional.of("read")))).build(); + MatchConfig matchB = MatchConfig.builder().pathPrefix("/x").methods(List.of(HttpMethod.GET)) + .headers(List.of(new MatchConfig.HeaderMatcher("X-Mode", Optional.of("write")))).build(); + EndpointConfig endpoint = endpoint("orders", "ORDERS") + .routes(List.of(RouteConfig.builder().id("reader").match(matchA).build(), + RouteConfig.builder().id("writer").match(matchB).build())) + .build(); + + RouteTable table = builder.build(gateway().build(), List.of(endpoint), topologyWith("ORDERS")); + + assertEquals(2, table.routes().size(), "header-distinguished same-prefix routes should both survive"); + } + + `@Test` + `@DisplayName`("Should reject same-prefix routes not distinguished by same header value") + void shouldRejectSamePrefixRoutesNotDistinguishedByHeader() { + MatchConfig matchA = MatchConfig.builder().pathPrefix("/x").methods(List.of(HttpMethod.GET)) + .headers(List.of(new MatchConfig.HeaderMatcher("X-Mode", Optional.of("read")))).build(); + MatchConfig matchB = MatchConfig.builder().pathPrefix("/x").methods(List.of(HttpMethod.GET)) + .headers(List.of(new MatchConfig.HeaderMatcher("X-Mode", Optional.of("read")))).build(); + EndpointConfig endpoint = endpoint("orders", "ORDERS") + .routes(List.of(RouteConfig.builder().id("first").match(matchA).build(), + RouteConfig.builder().id("second").match(matchB).build())) + .build(); + + assertThrows(RouteTableBuilder.RouteTableException.class, + () -> builder.build(gateway().build(), List.of(endpoint), topologyWith("ORDERS"))); + } } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@api-sheriff/src/test/java/de/cuioss/sheriff/api/config/RouteTableBuilderTest.java` around lines 103 - 196, Add tests in the MergeOrderDisjointness test group for header-based route disjointness, exercising RouteTableBuilder.headersDistinguish through builder.build. Verify same-prefix routes with the same header name but different values are accepted, and same-prefix routes with the same header name and identical values throw RouteTableBuilder.RouteTableException, using the existing route and endpoint test helpers.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@api-sheriff/src/main/java/de/cuioss/sheriff/api/config/model/RouteTable.java`:
- Around line 59-62: Update RouteTable.lookup to normalize the incoming path or
route prefix using the same trailing-slash convention as route registration
before applying startsWith. Ensure configured prefixes such as /api/ and lookup
paths such as /api resolve consistently, while preserving the existing null
validation and first-match behavior.
In `@api-sheriff/src/main/resources/schema/gateway.schema.json`:
- Around line 58-65: Add a required declaration to the mtls schema object so
enabled must be present whenever mtls is configured. Keep client_ca optional and
preserve the existing property types and additionalProperties restriction.
- Around line 134-142: Update the jwks.source enum in the gateway schema to
remove the unsupported "inline" value, keeping only sources representable by
IssuerConfig.Jwks; preserve the existing source, url, and file property
definitions.
---
Outside diff comments:
In
`@api-sheriff/src/main/java/de/cuioss/sheriff/api/gateway/proxy/ProxyRoute.java`:
- Around line 92-113: Update the upstream request creation in ProxyRoute so each
HttpRequest used by httpClient.send() has a bounded per-request timeout,
covering response waits after connection and slow upstreams. Use the existing
Duration/configuration approach, and preserve the current client connection
timeout and request behavior.
---
Nitpick comments:
In
`@api-sheriff/src/main/java/de/cuioss/sheriff/api/config/model/OidcConfig.java`:
- Around line 175-184: Add Lombok `@Builder` to the OidcConfig.Csrf record,
preserving its existing compact canonical constructor and trustedOrigins
normalization. Keep the annotation consistent with the other nested records
without changing construction behavior.
In
`@api-sheriff/src/main/java/de/cuioss/sheriff/api/config/validation/ConfigValidator.java`:
- Around line 167-181: Update validateMethodMembership so each method membership
ConfigError uses a route-specific path containing route.id(), consistent with
the precise route paths used by validateWholeSecondTimeouts; keep the existing
endpoint file and message content unchanged.
In `@api-sheriff/src/main/resources/schema/endpoint.schema.json`:
- Around line 136-161: Add non-negative minimum constraints to the integer
properties connect_timeout_ms, read_timeout_ms, retry.max_attempts,
circuit_breaker.failures, circuit_breaker.reset_ms, and the security-filter
limits max_header_count, max_header_value_length, max_query_params,
max_param_value_length, and max_body_bytes in endpoint.schema.json, while
preserving their existing integer types and structure.
In
`@api-sheriff/src/test/java/de/cuioss/sheriff/api/config/RouteTableBuilderTest.java`:
- Around line 103-196: Add tests in the MergeOrderDisjointness test group for
header-based route disjointness, exercising RouteTableBuilder.headersDistinguish
through builder.build. Verify same-prefix routes with the same header name but
different values are accepted, and same-prefix routes with the same header name
and identical values throw RouteTableBuilder.RouteTableException, using the
existing route and endpoint test helpers.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: cuioss/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 1c869bf1-f909-4ff9-bc34-cc1a1844c435
📒 Files selected for processing (82)
api-sheriff/pom.xmlapi-sheriff/src/main/java/de/cuioss/sheriff/api/config/ConfigLogMessages.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/config/RouteTableBuilder.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/config/load/ConfigError.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/config/load/ConfigLoadException.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/config/load/ConfigLoader.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/config/load/EnvSecretResolver.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/config/load/package-info.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/config/model/AuthConfig.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/config/model/EndpointConfig.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/config/model/ForwardConfig.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/config/model/ForwardedConfig.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/config/model/GatewayConfig.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/config/model/HttpMethod.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/config/model/IssuerConfig.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/config/model/MatchConfig.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/config/model/Metadata.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/config/model/OidcConfig.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/config/model/Protocol.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/config/model/RateLimitConfig.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/config/model/ResolvedRoute.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/config/model/ResolvedTopology.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/config/model/ResolvedUpstream.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/config/model/RouteConfig.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/config/model/RouteTable.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/config/model/SecurityDefaultsConfig.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/config/model/SecurityFilterConfig.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/config/model/SecurityHeadersConfig.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/config/model/TlsConfig.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/config/model/TokenValidationConfig.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/config/model/UpstreamConfig.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/config/model/UpstreamDefaultsConfig.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/config/model/package-info.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/config/package-info.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/config/topology/EndpointEnablementResolver.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/config/topology/TopologyResolver.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/config/topology/package-info.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/config/validation/ConfigValidator.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/config/validation/package-info.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/config/validation/rule/ValidationRule.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/config/validation/rule/package-info.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/gateway/proxy/ProxyConfiguration.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/gateway/proxy/ProxyRoute.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/gateway/proxy/package-info.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/quarkus/ConfigProducer.javaapi-sheriff/src/main/resources/application.propertiesapi-sheriff/src/main/resources/schema/endpoint.schema.jsonapi-sheriff/src/main/resources/schema/gateway.schema.jsonapi-sheriff/src/test/java/de/cuioss/sheriff/api/config/RouteTableBuilderTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/api/config/load/ConfigLoaderTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/api/config/load/EnvSecretResolverTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/api/config/model/ConfigModelContractTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/api/config/topology/EndpointEnablementResolverTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/api/config/topology/TopologyResolverTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/api/config/validation/ConfigValidatorTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/api/config/validation/rule/ValidationRuleTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/api/gateway/proxy/ProxyRouteTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/api/quarkus/ConfigFailFastTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/api/quarkus/ConfigProducerTest.javaapi-sheriff/src/test/resources/application.propertiesapi-sheriff/src/test/resources/config/broken/gateway.yamlapi-sheriff/src/test/resources/config/invalid/unknown-key.yamlapi-sheriff/src/test/resources/config/testboot/endpoints/proxy.yamlapi-sheriff/src/test/resources/config/testboot/gateway.yamlapi-sheriff/src/test/resources/config/testboot/topology.propertiesapi-sheriff/src/test/resources/config/valid/gateway.yamlapi-sheriff/src/test/resources/config/valid/topology.propertiesdoc/LogMessages.adocdoc/adr/0005-module-structure.adocdoc/configuration.adocdoc/plan/01-base-implementation.adocdoc/plan/02-configuration-management.adocdoc/plan/03-request-pipeline.adocdoc/plan/README.adocintegration-tests/docker-compose.benchmark.ymlintegration-tests/docker-compose.ymlintegration-tests/scripts/verify-invalid-config-fails.shintegration-tests/src/main/docker/sheriff-config/endpoints/httpbin.yamlintegration-tests/src/main/docker/sheriff-config/gateway.yamlintegration-tests/src/main/docker/sheriff-config/topology.propertiesintegration-tests/src/test/java/de/cuioss/sheriff/api/integration/ConfigLoadedIntegrationIT.javapom.xml
💤 Files with no reviewable changes (2)
- doc/plan/02-configuration-management.adoc
- api-sheriff/src/main/java/de/cuioss/sheriff/api/gateway/proxy/ProxyConfiguration.java
The native Quarkus app failed to boot because the file-based config subsystem relied on three JVM-only behaviors: - Bundled JSON schemas (schema/*.json) were excluded from the native image, so ConfigLoader.getResourceAsStream returned null. - The immutable config model records lacked reflection registration, so native Jackson databind into them failed. - ObjectMapper.findAndAddModules() discovers the Jdk8 module via the ServiceLoader, which does not resolve in a native image, leaving Optional-typed record components unbindable. Add schema/*.json to quarkus.native.resources.includes, consolidate reflection registration of the Jackson-bound config records, nested records, and enums in one ConfigModelReflection holder, and register the Jdk8 module explicitly. No config semantics change. Verified: native integration-tests suite boots and passes (10 ITs); api-sheriff JVM module tests pass (131). Co-Authored-By: Claude <noreply@anthropic.com>
headersDistinguish called HeaderMatcher.value() separately for the isPresent() guard and the get() access, so static dataflow analysis could not prove the value was present at the get() call (Sonar java:S3655, new-code reliability rating C). Bind each header value to a local Optional so the presence check and the access target the same instance. No behavior change. Co-Authored-By: Claude <noreply@anthropic.com>
… S2789) Co-Authored-By: Claude <noreply@anthropic.com>
…ss, url trim, timeout validation, mtls schema) Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
The three near-identical rejects* topology tests are consolidated into one
@ParameterizedTest over the malformed-input scenarios, clearing Sonar S5976
("replace these 3 tests with a single parameterized one"). Each scenario keeps
a descriptive display name; assertions are unchanged.
Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
api-sheriff/src/main/java/de/cuioss/sheriff/api/config/model/UpstreamConfig.java (1)
81-98: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider adding
@BuildertoNotModifiedfor consistency.
RetryandCircuitBreakercarry@Builder, butNotModifieddoes not. The test atRouteTableBuilderTest.javaline 96 constructs it directly withnew UpstreamConfig.NotModified(...), which works fine. Adding@Builderwould keep the pattern uniform across all nested records, though for a single-field record the direct constructor is arguably simpler.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@api-sheriff/src/main/java/de/cuioss/sheriff/api/config/model/UpstreamConfig.java` around lines 81 - 98, Update the nested UpstreamConfig.NotModified record to include the same `@Builder` annotation used by Retry and CircuitBreaker, while preserving its canonical constructor and enabled normalization behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In
`@api-sheriff/src/main/java/de/cuioss/sheriff/api/config/model/UpstreamConfig.java`:
- Around line 81-98: Update the nested UpstreamConfig.NotModified record to
include the same `@Builder` annotation used by Retry and CircuitBreaker, while
preserving its canonical constructor and enabled normalization behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: cuioss/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 0bfe1a14-4815-4969-92ca-2f33ea1f6176
📒 Files selected for processing (28)
api-sheriff/src/main/java/de/cuioss/sheriff/api/config/RouteTableBuilder.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/config/load/ConfigLoader.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/config/load/EnvSecretResolver.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/config/model/AuthConfig.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/config/model/EndpointConfig.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/config/model/ForwardedConfig.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/config/model/GatewayConfig.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/config/model/IssuerConfig.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/config/model/MatchConfig.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/config/model/Metadata.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/config/model/OidcConfig.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/config/model/RateLimitConfig.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/config/model/ResolvedRoute.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/config/model/ResolvedUpstream.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/config/model/RouteConfig.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/config/model/RouteTable.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/config/model/SecurityDefaultsConfig.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/config/model/SecurityFilterConfig.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/config/model/SecurityHeadersConfig.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/config/model/TlsConfig.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/config/model/UpstreamConfig.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/config/topology/EndpointEnablementResolver.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/config/topology/TopologyResolver.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/config/validation/ConfigValidator.javaapi-sheriff/src/main/resources/schema/gateway.schema.jsonapi-sheriff/src/test/java/de/cuioss/sheriff/api/config/RouteTableBuilderTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/api/config/topology/TopologyResolverTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/api/config/validation/ConfigValidatorTest.java
🚧 Files skipped from review as they are similar to previous changes (26)
- api-sheriff/src/main/java/de/cuioss/sheriff/api/config/model/SecurityDefaultsConfig.java
- api-sheriff/src/main/java/de/cuioss/sheriff/api/config/model/RateLimitConfig.java
- api-sheriff/src/main/java/de/cuioss/sheriff/api/config/model/AuthConfig.java
- api-sheriff/src/main/java/de/cuioss/sheriff/api/config/model/Metadata.java
- api-sheriff/src/main/java/de/cuioss/sheriff/api/config/model/SecurityHeadersConfig.java
- api-sheriff/src/main/java/de/cuioss/sheriff/api/config/model/RouteConfig.java
- api-sheriff/src/main/java/de/cuioss/sheriff/api/config/model/RouteTable.java
- api-sheriff/src/main/java/de/cuioss/sheriff/api/config/load/EnvSecretResolver.java
- api-sheriff/src/main/java/de/cuioss/sheriff/api/config/model/ForwardedConfig.java
- api-sheriff/src/main/java/de/cuioss/sheriff/api/config/model/TlsConfig.java
- api-sheriff/src/main/java/de/cuioss/sheriff/api/config/model/ResolvedUpstream.java
- api-sheriff/src/main/java/de/cuioss/sheriff/api/config/topology/EndpointEnablementResolver.java
- api-sheriff/src/main/java/de/cuioss/sheriff/api/config/model/EndpointConfig.java
- api-sheriff/src/main/java/de/cuioss/sheriff/api/config/model/GatewayConfig.java
- api-sheriff/src/main/java/de/cuioss/sheriff/api/config/model/MatchConfig.java
- api-sheriff/src/main/java/de/cuioss/sheriff/api/config/model/IssuerConfig.java
- api-sheriff/src/main/java/de/cuioss/sheriff/api/config/model/ResolvedRoute.java
- api-sheriff/src/main/resources/schema/gateway.schema.json
- api-sheriff/src/main/java/de/cuioss/sheriff/api/config/model/OidcConfig.java
- api-sheriff/src/main/java/de/cuioss/sheriff/api/config/validation/ConfigValidator.java
- api-sheriff/src/test/java/de/cuioss/sheriff/api/config/topology/TopologyResolverTest.java
- api-sheriff/src/test/java/de/cuioss/sheriff/api/config/validation/ConfigValidatorTest.java
- api-sheriff/src/main/java/de/cuioss/sheriff/api/config/RouteTableBuilder.java
- api-sheriff/src/main/java/de/cuioss/sheriff/api/config/model/SecurityFilterConfig.java
- api-sheriff/src/main/java/de/cuioss/sheriff/api/config/load/ConfigLoader.java
- api-sheriff/src/main/java/de/cuioss/sheriff/api/config/topology/TopologyResolver.java
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.
Summary
Implements the configuration-management subsystem described in
doc/plan/02-configuration-management.adoc: JSON Schemas forgateway.yamlandendpoints/*.yaml, the load → resolve → validate pipeline, an immutable configmodel, topology/route resolution into an immutable
RouteTable, fail-fast CDIwiring, and the associated unit and integration tests. Two aspects are folded in:
three-level
retry/ HTTP-304not_modifiedoverride plumbing (globalupstream_defaults→ endpointupstream_defaults→ per-routeupstream), andthe
cui-java-parent/cui-httpdoc version bumps.Changes
Config loading & resolution (
api-sheriff/src/main/java/de/cuioss/sheriff/api/config/)load/ConfigLoader.java,EnvSecretResolver.java,ConfigError.java,ConfigLoadException.java— YAML/properties loading with${ENV_VAR}secretresolution.
model/*— immutable config records (GatewayConfig,RouteConfig,OidcConfig,UpstreamConfig,UpstreamDefaultsConfig, resolved-topologytypes, etc.).
topology/TopologyResolver.java,EndpointEnablementResolver.javaandRouteTableBuilder.java— resolve endpoints/routes into an immutableRouteTable.validation/ConfigValidator.java+rule/ValidationRule.java— rule-basedvalidation.
quarkus/ConfigProducer.java— fail-fast CDI wiring (aggregates allviolations, refuses to start on any error).
ConfigLogMessages.java—LogRecordcatalogue.Schemas & resources
src/main/resources/schema/gateway.schema.json,endpoint.schema.json—allow-list-first (
additionalProperties: false) schemas including theupstream_defaultsretry / not-modified blocks.src/main/resources/application.properties.Security hardening (finalize security-audit)
model/OidcConfig.java— redactingtoString()overrides onOidcConfigand its nested
Sessionrecord so resolvedclientSecret/encryptionKey/
previousKeyvalues are not leaked via logs, exception messages, ordebugger views.
Tests — unit tests for loader, env-secret resolver, model contract,
topology/enablement resolvers, validator, proxy route, and fail-fast producer;
plus
integration-tests/.../ConfigLoadedIntegrationIT.javaand config fixtures.Docs & build
doc/configuration.adoc,doc/LogMessages.adoc,doc/adr/0005-module-structure.adoc,doc/plan/*(Plan 02 delivered;cui-http:2.1-SNAPSHOT→2.1references).pom.xml/api-sheriff/pom.xml—cui-java-parentbump and module deps.Test Plan
test -pl api-sheriff -amgreen (unit tests, incl. config subsystem)integration-tests(validated by CI on this PR)Related Issues
None — plan sourced from
doc/plan/02-configuration-management.adoc.Generated by plan-finalize skill
Summary by CodeRabbit
gateway.yaml,endpoints/*.yaml, andtopology.properties, including${ENV_VAR}substitution andTOPOLOGY_*/ENDPOINT_*overrides.404.upstream_defaultsbehavior.