Repository navigation
CAMEL-25538: fix configuration-validate to accept all camel.* groups and report accepted=false as valid=false - #27841
Merged
davsclaus merged 18 commits intoOct 16, 2026
Conversation
…and report accepted=false as valid=false
AbstractCamelCatalog.acceptConfigurationPropertyKey() hardcoded only ten
camel.* prefixes (component/dataformat/language/main/resilience4j/
faulttolerance/threadpool/health/lra/rest), so keys under the 26 other
groups defined in camel-main-configuration-metadata.json (camel.server.*,
camel.management.*, camel.ssl.*, camel.debug.*, camel.trace.*,
camel.vault.*, camel.cluster.*, camel.routecontroller.*, and others) were
rejected with accepted=false. Because validateConfigurationProperty()
returned immediately after setAccepted(false) without recording any error,
isSuccess() returned true, producing the contradictory response
{accepted:false, valid:true} for every unrecognised camel.* group.
Fix 1 (AbstractCamelCatalog): replace the hardcoded prefix list with a
dynamic lookup against mainModel().getGroups() so that any group declared
in the metadata is automatically accepted without needing a code change.
component/dataformat/language keep their own static prefixes because they
use a different per-artifact model, not mainModel().
Fix 2 (ConfigurationValidateTools): at the MCP layer, guard valid with
"accepted && result.isSuccess()" so that an unaccepted key is never
reported as valid, even when the catalog returned no errors.
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
|
🌟 Thank you for your contribution to the Apache Camel project! 🌟 🐫 Apache Camel Committers, please review the following items:
|
Contributor
|
🧪 CI tested the following changed modules:
🔬 Scalpel shadow comparison — Scalpel: 494 of 706 tested, 22 compile-only — current: 494 all testedMaveniverse Scalpel detected 494 affected modules (current approach: 494). Skip-tests mode would test 494 modules (4 direct + 492 downstream), skip tests for 22 (generated code, meta-modules) Modules Scalpel would test (494)
Modules with tests skipped (22)
Build reactor — dependencies compiled but only changed modules were tested (4 modules, 1m 23s total)Total reactor time: 1m 23s
Top 20 slowest modules:
|
gnodet
marked this pull request as ready for review
October 15, 2026 11:45
davsclaus
requested changes
Oct 15, 2026
davsclaus
left a comment
davsclaus
left a comment
Contributor
There was a problem hiding this comment.
Thanks @gnodet for picking this up. Building the accepted prefixes from the main metadata groups is the right direction, and camel.server.* / camel.management.* / camel.vault.* typos now get "Unknown option" as they should. A typo in the group (camel.servr.port) is still not accepted, and a typo in the option (camel.server.prot) gets "Unknown option" with suggestions.
What I checked: how validateConfigurationProperty picks the camel.* groups against the 36 groups in camel-main-configuration-metadata.json (including the nested camel.vault.* / camel.cluster.* groups and look-alike names such as opentelemetry/opentelemetry2, kubernetes/kubernetescm), how camel-main binds these keys at runtime (MainHelper.optionKey and startsWithIgnoreCase in BaseMainSupport), the other callers of the catalog API (camel-report-maven-plugin ValidateMojo, camel-jbang PropertiesChecks), the four fix items in CAMEL-25538, and the tests.
Findings:
- Unchecked lines now read as rejected (inline). With
valid = accepted && isSuccess(),server.port=8080,quarkus.http.port=8080,mail.smtp.host=...and the legal Camel keys without a model (camel.beans.*,camel.variable.*,camel.kamelet.*,camel.jbang.*, the 8camel.vault.springConfig.*options, which have no group in the metadata) come backvalid=false. They are no longer counted as valid, which item 4 asks for, but item 4 also asks that they read as "not checked", not as "rejected": achecked:falseflag (or a renamedaccepted), asummary.uncheckedcount, and a sentence in the tool description. - Spring Boot false positives. For
runtime=spring-bootthe catalog still uses the camel-main metadata, but the starters bind some of the newly accepted groups with their own options. For examplecamel.metrics.uriTagEnabled,uriTagDynamicandroutePolicyExcludePatternexist incamel-micrometer-starterand not in the main metadata, so they now get "Unknown option", in the MCP tool and incamel-report-maven-plugin:validateon Spring Boot projects (which fails the build withfailOnError=true). Please either limit the new groups to the main runtime or check against the starter options. - The group match is case- and dash-sensitive, the runtime binding is not (inline). camel-main binds
camel.error-registry.enabled,camel.ai-observability.enabledandcamel.telemetry-dev.enabled, but the catalog does not accept these keys, so the tool reports themvalid=false. - The main metadata is parsed up to 3 times per line on a catalog without cache, such as the one
ValidateMojocreates, and now also for every key that does not start withcamel.(inline). - Scope. Item 2 (
camel.jbang.*againstjbangModel()), item 3 (a warning with the closest group for an unknowncamel.<x>group, socamel.servr.portstill gets no explanation) and @Croway's point about backslash continuation lines are not covered. Fine as follow-ups, but please say so in the PR description. - Tests.
camelVaultAwsKeyIsAcceptedonly assertsaccepted(inline). An invented option under a nested group would cover the new dot-offset code. The behaviour change is incamel-core-catalog, but all the tests are in camel-jbang-mcp; a few cases inCamelCatalogTest(nested group, dashed group name, invented option) would also protect the Maven plugin andPropertiesChecks. - Upgrade guide.
camel-report-maven-plugin:validatenow checks about 26 morecamel.*groups, so a build withfailOnError=truecan fail on keys it ignored before. A short note incamel-4x-upgrade-guide-4_24.adocwould help.
CI is green.
Claude Code on behalf of @davsclaus.
This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.
…sitive group matching, single model load
Contributor
✅ Generated files are up to dateAn earlier CI run reported uncommitted generated changes; the latest run no longer does. |
davsclaus
reviewed
Oct 16, 2026
davsclaus
left a comment
davsclaus
left a comment
Contributor
There was a problem hiding this comment.
Thanks @gnodet, the new commits fix most of what I raised:
- Unchecked lines now read as "not checked" instead of "rejected":
checked=false, they are left out ofsummary.validandsummary.errors,summary.uncheckedis added, and the tool description explains it. - Group matching is now dash- and case-insensitive, so
camel.error-registry.enabledreaches thecamel.errorRegistrygroup, the same way camel-main binds it.camel.cluster.jgroups-raft.*still matches. - Model loading: a main-group key now parses the model once instead of up to three times, and keys outside
camel.return before any model is loaded. - Vault tests: the
camel.vault.awstests now assertvalid, including an invented option, so they cover the dot-offset logic.
New in this round:
- Extra parsing for component lines (inline). The model is now loaded for every
camel.key, includingcamel.component.*,camel.dataformat.*andcamel.language.*, which return before they use it.mainModel()parses the JSON on every call (enableCache()caches only the string), soValidateMojoand the MCP tool now parse the main metadata once per component line, which neither the base code nor the first commit did. - Minor cleanup: the "camel-main not detected" branch is kept, but can no longer be reached, as the new comment itself says. The
cachedModel != null ? cachedModel : mainModel()fallbacks in the two private helpers only take effect when the model is missing, and then they look it up again. Passing the model straight through and dropping that branch would be simpler.
Still open from my first review:
- Spring Boot false positives: with
runtime=spring-boot, starter-only options such ascamel.metrics.uriTagEnabledstill get "Unknown option", both in the MCP tool and incamel-report-maven-plugin:validate. - Tests: there is no test for a dashed group name, although that is the main fix of this round (suggested inline), and
CamelCatalogTesthas no new cases.camel-core-catalogis shared, so those cases would protectValidateMojoandPropertiesCheckstoo. CI does not test the dependents ofcamel-core-catalog(too many), so thetest-dependentslabel would help. - Upgrade guide: there is still no note in
camel-4x-upgrade-guide-4_24.adocthatvalidatenow checks about 26 more groups and accepts dashed or mixed-case group names. - PR description: it still describes the first commit (
accepted,valid = accepted && isSuccess(), the old test names).
The spotless commit fixed the "uncommitted changes" failure; CI is running again on the new head.
Claude Code on behalf of Claus Ibsen
gnodet-bot
reviewed
Oct 16, 2026
gnodet-bot
left a comment
gnodet-bot
left a comment
Contributor
There was a problem hiding this comment.
The core fix is solid — dynamic group matching via matchMainModelGroup, clean checked/unchecked semantics, and good test coverage. A few issues remain.
Unaddressed from previous review:
- Spring Boot false positives (
server.port,quarkus.*etc. are now classified asuncheckedrather than the previous broken behaviour, but the tool description was the only update — no filtering or caller-side guidance for mixed Spring Boot + Camel property files) - Upgrade guide note for the
accepted→checkedrename was not mentioned
Binary-compat note: PropertyValidationInfo record field rename accepted → checked and new ConfigurationValidateSummary.unchecked field change the JSON wire format of the MCP tool response — worth documenting in the JIRA.
This review was generated by an AI agent, Hermès on behalf of @gnodet.
…format/language keys, add dashed-group MCP test
…inModelGroup Javadoc, vault test assertion
gnodet-bot
reviewed
Oct 16, 2026
gnodet-bot
left a comment
gnodet-bot
left a comment
Contributor
There was a problem hiding this comment.
Re-review after latest commits — previous concerns are resolved.
Previously raised, now addressed:
- ✅ Upgrade guide documents the
accepted→checkedrename and the newuncheckedfield (seecamel-4x-upgrade-guide-4_24.adoc) - ✅ Non-camel keys (
server.port,quarkus.*) are correctly classified aschecked=false/uncheckedand excluded from error/valid counts — testnonCamelKeyMarkedNotCheckedcovers this - ✅ Dashed group names (
camel.error-registry.*) are matched case- and dash-insensitively — testdashedGroupNameIsCheckedAndValidcovers this - ✅ The
model == nullguard inmatchMainModelGroupis intentional (handlesmainModel()returningnullwhen camel-main is not on the classpath)
Overall assessment:
The implementation is sound. The caching strategy correctly short-circuits model loading for component/dataformat/language keys. The two-method split (acceptConfigurationPropertyKey with full camel.* prefix, matchMainModelGroup with stripped prefix) is intentional and documented in the Javadoc. Test coverage is comprehensive: nested groups (camel.vault.aws.*), invented options, unchecked non-camel keys, and dashed group names are all exercised.
One minor non-blocking observation: AbstractCamelCatalog itself has no new unit tests for the dynamic group matching (all new tests go through ConfigurationValidateTools). This is acceptable given the end-to-end coverage, but a direct test in CamelCatalogTest would make regressions easier to pinpoint.
LGTM — the changes address the regression and extend coverage cleanly.
This review was generated by an AI agent, Hermès on behalf of @gnodet.
…Boot false positives for starter-only options
gnodet-bot
reviewed
Oct 16, 2026
gnodet-bot
left a comment
gnodet-bot
left a comment
Contributor
There was a problem hiding this comment.
Following up on my earlier review — the dead-code concern from the previous round is resolved by this commit.
New commit (ce48265) — assessed:
- ✅ Dead model fallback removed from
acceptConfigurationPropertyKey:cachedModel != null ? cachedModel : mainModel()→cachedModel. Behaviour is identical whenmainModel()returns null (classpath absent), and the re-load path is eliminated. - ✅ Spring Boot false-positive logic:
onlyUnknownOptionErrorscheck is sound.addUnknown()uses aLinkedHashSetand incrementserrorsonly once per unique name, sogetNumberOfErrors() == unknownKeys.size()reliably isolates unknown-option errors from syntax/enum/required/invalidReference/invalidMap/invalidArray errors — all of which contribute to the sameerrorscounter and would break the equality if present.
Remaining open:
- No Spring Boot test (see inline). The new
isSpringBootdowngrade path has no test case. - PR description still mentions
CamelCatalogTestnew methods (testValidateConfigurationPropertyNestedGroup,testValidateConfigurationPropertyDashedGroupName,testValidateConfigurationPropertyServerInventedOption) — but none appear in the PR diff.
This review was generated by an AI agent, Hermès on behalf of @gnodet.
gnodet
force-pushed
the
fix/camel-25538-fix-configuration-validate
branch
2 times, most recently
from
October 16, 2026 08:10
2c10e2d to
bb0fea7
Compare
…hing Add three direct tests to CamelCatalogTest that exercise AbstractCamelCatalog's dynamic group matching logic independently of ConfigurationValidateTools: - testValidateConfigurationPropertyNestedGroup: validates nested camel.vault.aws.* keys and confirms invented options are rejected - testValidateConfigurationPropertyDashedGroupName: validates dash-insensitive group matching (camel.error-registry.* → camel.errorRegistry group) - testValidateConfigurationPropertyServerInventedOption: validates that camel.server.* accepts known options and rejects invented ones
gnodet-bot
reviewed
Oct 16, 2026
gnodet-bot
left a comment
gnodet-bot
left a comment
Contributor
There was a problem hiding this comment.
Following up on my earlier review — both remaining open issues are now addressed.
Previously raised, now addressed:
- ✅
CamelCatalogTestdirect tests for dynamic group matching now present:testValidateConfigurationPropertyNestedGroup,testValidateConfigurationPropertyDashedGroupName,testValidateConfigurationPropertyServerInventedOption— all three fail on the old code (groupscamel.vault.aws.*,camel.error-registry.*,camel.server.*absent from the hardcoded prefix list) and pass after the fix. - ✅ Spring Boot starter test added:
springBootStarterOnlyOptionIsUnchecked.
One concern on the new spring-boot test (see inline): the test passes before the fix for the wrong reason and does not strongly exercise the isSpringBoot downgrade path.
This review was generated by an AI agent, Hermès on behalf of @gnodet.
…ith counterpart assertion
gnodet
commented
Oct 16, 2026
gnodet
left a comment
gnodet
left a comment
Contributor
Author
There was a problem hiding this comment.
Following up on my earlier review — the weak spring-boot test concern is now resolved.
Previously raised, now addressed:
- ✅
springBootStarterOnlyOptionIsUncheckednow includes a counterpart assertion: same key (camel.metrics.uriTagEnabled) without thespring-bootruntime flag returnschecked=true(group matched dynamically via the newmatchMainModelGroup) butvalid=false(option not in camel-main metadata). This assertion fails on the old code (wherecamel.metricswas not in the hardcoded prefix list), making the test a genuine regression guard for the dynamic matching path. - ✅ All three new
CamelCatalogTestmethods (testValidateConfigurationPropertyNestedGroup,testValidateConfigurationPropertyDashedGroupName,testValidateConfigurationPropertyServerInventedOption) pass locally.
All review threads are resolved. LGTM — the implementation is complete and well-tested.
This review was generated by an AI agent, Hermès on behalf of @gnodet.
gnodet-bot
reviewed
Oct 16, 2026
gnodet-bot
left a comment
gnodet-bot
left a comment
Contributor
There was a problem hiding this comment.
Following up on my earlier review — the last outstanding concern is now addressed.
Previously raised, now addressed:
- ✅ Weak spring-boot test —
springBootStarterOnlyOptionIsUncheckednow includes the counterpart assertion:camel.metrics.uriTagEnabledwithout thespring-bootflag must bechecked=true(becausecamel.metricsis a known main-model group) butvalid=false(becauseuriTagEnabledis not in the main schema). This assertion fails on the old code (wherecamel.metrics.*was not in the hardcoded prefix list), making the test discriminating. The fix is complete.
All prior findings are resolved. LGTM — ready to merge.
This review was generated by an AI agent, Hermès on behalf of @gnodet.
…ad fails When runtime=spring-boot with no explicit version or platformBom, CatalogService.loadCatalog() attempted to download the Spring Boot catalog provider from Maven Central. In CI environments without network access this raised a DownloadException, causing ConfigurationValidateTools tests to fail with a ToolCallException rather than the expected result. Extend the existing offline-fallback logic (already in place for main runtime + explicit version) to also cover spring-boot runtime without a pinned version. The default camel-main catalog is sufficient for configuration property validation in that case. Also add a dedicated CatalogServiceTest that reproduces the failure using a non-existent file: repository, which always triggers the download error without requiring network access.
gnodet-bot
reviewed
Oct 16, 2026
gnodet-bot
left a comment
gnodet-bot
left a comment
Contributor
There was a problem hiding this comment.
Following up on my earlier review — the last concern (weak spring-boot test) was addressed in the previous commit and all threads are resolved. This re-review covers the new commit 7e32ba8.
matchMainModelGroup always returns null — validation is silently skipped for all main-model groups.
The Javadoc states the method "receives the full camel.* key", but the implementation strips "camel." from the group name before the startsWith check:
String groupName = group.getName().substring("camel.".length()); // → "errorRegistry"
String normGroupName = groupName.replace("-", "").toLowerCase(); // → "errorregistry"
if (normKey.startsWith(normGroupName + ".")) { // "camel.errorregistry.enabled".startsWith("errorregistry.") → FALSEnormKey is the full key ("camel.errorregistry.enabled") but normGroupName + "." is "errorregistry." — the "camel." prefix has been stripped from the group but not from the key. The startsWith call never matches, matchMainModelGroup always returns null, and the else branch never executes.
Effect: unknown options under accepted groups (e.g. camel.server.inventedOption) are not caught — isSuccess() returns true and valid=true for any key that acceptConfigurationPropertyKey already accepted. Tests like camelServerInventedKeyIsCheckedButNotValid and camelVaultAwsInventedKeyIsCheckedButNotValid are almost certainly failing, which explains the CI failure on the prior commit (24be01b). The new commit (7e32ba8) does not fix this.
Fix — use the same pattern as acceptConfigurationPropertyKey (keep the full group name with "camel.", do not strip):
for (MainModel.MainGroupModel group : model.getGroups()) {
String normGroup = group.getName().replace("-", "").toLowerCase(Locale.ENGLISH) + ".";
if (normKey.startsWith(normGroup)) {
return group;
}
}The new commit (7e32ba8 — spring-boot download fallback in CatalogService) is correct: the catch condition !hasBom && (main && hasVersion || springBoot && !hasVersion) is properly parenthesised (the outer parens wrap both clauses so !hasBom applies to both), and the test is discriminating (old code throws, new code returns the default catalog). No issues in that commit.
This review was generated by an AI agent, Hermès on behalf of @gnodet.
normKey carries the full 'camel.*' key (e.g. 'camel.errorregistry.enabled'),
but the loop was stripping 'camel.' from group.getName() before normalising,
producing normGroupName = 'errorregistry'. The startsWith check then compared
'camel.errorregistry.enabled' against 'errorregistry.' which never matches.
acceptConfigurationPropertyKey avoids the same pitfall by normalising the full
group name ('camel.errorRegistry' -> 'camel.errorregistry') and comparing it
against the full normKey, which is what matchMainModelGroup should do too.
Effect of the bug: matchMainModelGroup always returned null, so the else-branch
in validateConfigurationProperty never executed — unknown options under accepted
main-model groups (e.g. camel.server.inventedOption) were silently validated
with isSuccess()=true, making valid=true when they should be valid=false.
gnodet-bot
reviewed
Oct 16, 2026
gnodet-bot
left a comment
gnodet-bot
left a comment
Contributor
There was a problem hiding this comment.
Following up on my earlier review — the matchMainModelGroup prefix fix is now correctly applied.
Previously raised, now addressed:
- ✅
matchMainModelGroupnow usesgroup.getName()directly (without strippingcamel.), makingnormGroupName(e.g.camel.errorregistry) correctly matchnormKey(e.g.camel.errorregistry.enabled) viastartsWith— the prefix logic is now consistent betweenacceptConfigurationPropertyKeyandmatchMainModelGroup
No further concerns. The fix is solid.
This review was generated by an AI agent, Hermès — it may contain errors.
…dy been applied to key
gnodet-bot
reviewed
Oct 16, 2026
gnodet-bot
left a comment
gnodet-bot
left a comment
Contributor
There was a problem hiding this comment.
Following up on my earlier review — the two final fix commits are correct.
Previously raised, now addressed:
- ✅
matchMainModelGroupno longer strips"camel."fromgroup.getName()before normalising —normGroupNameis now"camel.errorregistry"(full name), matchingnormKey="camel.errorregistry.enabled"correctly viastartsWith - ✅ Caller now passes
longKey(the unmodified fullcamel.*key) tomatchMainModelGroup—keyhad already had"camel."stripped at this point, which would have caused the same prefix mismatch
Both fixes are consistent with how acceptConfigurationPropertyKey already operated. Traced through camel.server.port (simple lookup), camel.server.inventedOption (unknown option detection), and camel.vault.aws.accessKey (nested 3-segment group with dot-offset extraction) — all paths produce correct results. Existing tests (testValidateConfigurationPropertyServerInventedOption, testValidateConfigurationPropertyNestedGroup, testValidateConfigurationPropertyDashedGroupName) exercise the exact scenarios fixed by these commits.
LGTM — ready to merge.
This review was generated by an AI agent, Hermès on behalf of @gnodet.
Contributor
Author
|
All review findings from @davsclaus have been addressed across the subsequent commits:
CI is green. All review threads are resolved. PTAL @davsclaus for a re-review when you get a chance. |
davsclaus
reviewed
Oct 16, 2026
davsclaus
left a comment
davsclaus
left a comment
Contributor
There was a problem hiding this comment.
Thanks @gnodet, most of the last round is in:
- The main model is no longer parsed for
camel.component.*,camel.dataformat.*andcamel.language.*keys, and the unreachable "camel-main not detected" branch and the reload fallbacks are gone. CamelCatalogTestnow covers nested groups, dashed group names and invented options, and the MCP tests cover the dashed group.- There is an upgrade guide entry in the 4.23 to 4.24 section.
- 494e4cb passes
longKeytomatchMainModelGroupagain. Since 7efd474 the stripped key no longer matched any group, so main-group lines were accepted without being validated. Good that this is fixed (the bot comment that led to 7efd474 was mistaken).
Remaining points:
- The Spring Boot downgrade is too broad (inline). Every line whose only errors are unknown options becomes
checked=falsewith no issues. That includescamel.component.*,camel.dataformat.*,camel.language.*andcamel.main.*. On main,camel.component.kafka.brokerz=xwithruntime=spring-bootwas reported with a "did you mean brokers" suggestion. Now it is silently unchecked, which turns off typo detection for Spring Boot. camel-report-maven-plugin:validateon Spring Boot.ValidateMojodoes not know the runtime, so starter-only options such ascamel.metrics.uriTagEnablednow fail a build that usesfailOnError=true. If that stays out of scope, please mention it in the upgrade guide and the PR description.- Upgrade guide accuracy (inline): the group count, and what the tool returned before.
- CatalogService fallback. 7e32ba8 has no JIRA key and changes every MCP catalog tool for
runtime=spring-boot: without network access they now answer silently from the main catalog. That matches the existing fallback for main with a version, but it is outside CAMEL-25538, so please mention it in the PR description (and in the commit message when squashing).
CI on the new head is still running.
Claude Code on behalf of Claus Ibsen
…urface suggestions on downgrade; fix upgrade guide group count and unchecked description
gnodet-bot
reviewed
Oct 16, 2026
gnodet-bot
left a comment
gnodet-bot
left a comment
Contributor
There was a problem hiding this comment.
Re-reviewing after the latest commits.
One concern with the new isOriginalGroup guard.
This review was generated by an AI agent, Hermès on behalf of @gnodet.
…ing dashed forms Spring Boot relaxed-binding writes 'camel.fault-tolerance.*' and 'camel.thread-pool.*' with dashes. The isOriginalGroup prefix checks used the raw key which would miss these dashed forms (e.g. 'camel.faulttolerance.' does not match 'camel.fault-tolerance.'). Normalize the key the same way the catalog does — remove dashes and lowercase — before the startsWith checks.
gnodet-bot
reviewed
Oct 16, 2026
gnodet-bot
left a comment
gnodet-bot
left a comment
Contributor
There was a problem hiding this comment.
Re-review after commit be91daa.
The normalization fix is correct and complete: propertyKey is now normalized to normKey (dashes removed, lowercased) before all startsWith prefix checks in isOriginalGroup. The remaining review questions (CatalogService fallback condition, StringHelper.before(key, ".") for nested groups, acceptConfigurationPropertyKey raw-key handling for static prefixes) are non-issues — the static prefixes contain no dashes, so raw comparison is correct there.
✅ LGTM — the concern from the previous review is fully addressed.
This review was generated by an AI agent, Hermès on behalf of @gnodet.
davsclaus
approved these changes
Oct 16, 2026
davsclaus
left a comment
davsclaus
left a comment
Contributor
There was a problem hiding this comment.
Thanks @gnodet, all four points from the last round are addressed:
- The Spring Boot downgrade now only applies to the groups this PR newly accepts, and the key is normalised the same way as in the catalog, so
camel.fault-tolerance.*keeps full validation too. Downgraded lines still return their issues and suggestions.camel.component.kafka.brokerz=xwithruntime=spring-bootis again reported as invalid with a suggestion. - The
camel-report-maven-plugin:validatelimitation for Spring Boot is in the PR description and the upgrade guide. - The upgrade guide has the corrected count and says what the tool returned before.
- The
CatalogServicefallback is called out in the PR description.
I checked the original-group list against the main metadata: the 7 main groups (main, resilience4j, faulttolerance, threadpool, health, lra, rest) plus component, dataformat and language match the old hardcoded prefixes. CI is green.
A few small nits are inline: a fully qualified java.util.Locale, a test for the narrowed downgrade, and two wording fixes in the upgrade guide. None of them block the merge.
Claude Code on behalf of Claus Ibsen
- import java.util.Locale instead of FQCN (CLAUDE.md style) - springBootStarterOnlyOptionIsUnchecked: assert issues() contains unknown kind - add springBootKeepsValidationForOriginalGroups test covering component and fault-tolerance (dashed form) typos under spring-boot runtime - upgrade guide: fix group count (7 of 36, not 10), remove approximate qualifier, replace dangling 'See below' with concrete consequence sentence
gnodet-bot
reviewed
Oct 16, 2026
gnodet-bot
left a comment
gnodet-bot
left a comment
Contributor
There was a problem hiding this comment.
Following up on my earlier review — the nit commit (e322e883) is clean.
All prior findings resolved. This commit addresses davsclaus's non-blocking nits:
- ✅ Import
java.util.Localeinstead of FQCN — correct - ✅ Upgrade guide count fixed to "7 of the 36 groups" (main, resilience4j, faulttolerance, threadpool, health, lra, rest) — verified against
camel-main-configuration-metadata.json; the 29 additional = 36 − 7 is accurate - ✅ "See below" dangling reference replaced with concrete consequence sentence — correct
- ✅
springBootStarterOnlyOptionIsUncheckedstrengthened withissues()assertion — consistent with production code (springBootDowngraded=truepath still callscollectIssues()) - ✅
springBootKeepsValidationForOriginalGroupstest:camel.fault-tolerance.*normalises tocamel.faulttolerance.*→isOriginalGroup=true→ full validation retained — correct
LGTM — ready to merge.
This review was generated by an AI agent, Hermès on behalf of @gnodet.
Contributor
|
Approved and ready to merge, but it now conflicts with main in Claude Code on behalf of Claus Ibsen |
Contributor
Author
🔄 Backport BotThis bugfix targets
Add a Backport PRs will be created automatically on merge (once a port label is added). Comment ℹ️ If you push additional commits after |
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
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
Fixes bugs in
camel_configuration_validate(MCP tool) andAbstractCamelCatalog.validateConfigurationProperty()that caused allcamel.*groups not in the original 10 hardcoded prefixes (camel.server.*,camel.management.*,camel.vault.*, etc.) to be ignored or mis-reported.Root Cause
Bug 1 —
AbstractCamelCatalog.acceptConfigurationPropertyKey()hardcoded only 10 of the 36camel.*groups defined incamel-main-configuration-metadata.json:Missing:
camel.server.*,camel.management.*,camel.ssl.*,camel.debug.*,camel.trace.*,camel.vault.*,camel.opentelemetry.*,camel.metrics.*,camel.cluster.*,camel.routecontroller.*, and others (≈29 groups total).When a key wasn't accepted,
validateConfigurationProperty()returned immediately. SinceisSuccess()returnstruewhen there are no recorded errors, this produced the contradictory{accepted:false, valid:true}result.Bug 2 —
ConfigurationValidateTools.toPropertyValidationInfo()at the MCP layer computedvalid = result.isSuccess(), ignoring theacceptedflag.Fix
AbstractCamelCatalog:acceptConfigurationPropertyKey()is now an instance method that readsmainModel().getGroups()dynamically. Any group incamel-main-configuration-metadata.jsonis automatically accepted without a code change.MainHelper.optionKey/startsWithIgnoreCase), socamel.error-registry.*matches thecamel.errorRegistrygroup.validateConfigurationProperty()call (not once per helper call), and only for keys that actually need it (keys starting withcamel.*other thancamel.component.*/camel.dataformat.*/camel.language.*).acceptConfigurationPropertyKey,matchMainModelGroup) use the passed-in model directly — no redundant re-load fallback.matchMainModelGroup()correctly locates the option-name offset for nested groups likecamel.vault.aws.*(3-segment group name).ConfigurationValidateTools:checkedis now set fromresult.isAccepted().valid = checked && result.isSuccess(), so an unchecked line is neither valid nor invalid.checked=falseand counted insummary.unchecked(not insummary.validorsummary.errors).camel-main-configuration-metadata.jsondoes not include starter-specific options (e.g.camel.metrics.uriTagEnabledfromcamel-micrometer-starter—RuntimeProviderhas no hook to supply a Spring Boot-specific main schema). Forruntime=spring-boot, when the only issues are "unknown option" errors (no type errors) under a newly accepted group (not one of the original 10 hardcoded prefixes), the line is downgraded tochecked=falseto avoid false positives. Issues/suggestions are still surfaced so the user has context. Keys under the original 10 groups (camel.component.*,camel.dataformat.*,camel.language.*,camel.main.*, etc.) retain full validation — typo detection continues to work for those.@Tooldescription explainschecked/ unchecked semantics and the Spring Boot limitation.Note:
camel-report-maven-plugin:validateis unaffected by the Spring Boot downgrade —ValidateMojodoes not pass a runtime, so starter-only options such ascamel.metrics.uriTagEnabledwill now be reported as "Unknown option". This is documented in the upgrade guide.CatalogServiceSpring Boot fallback (commit 7e32ba8, outside CAMEL-25538 scope):CatalogServicefalls back to the default main catalog silently. This matches the existing fallback for a versioned main catalog, and avoids a hard failure in offline environments. When squashing, this change should be called out in the commit message.Tests
ConfigurationValidateToolsTest— 18 test cases including new:camelServerPortIsAcceptedAndValid—camel.server.port=9999→checked=true, valid=truecamelServerInventedKeyIsCheckedButNotValid— unknown option under known group →checked=true, valid=falsenonCamelKeyMarkedNotChecked— unknown namespace →checked=false, not counted in errorsuncheckedKeyImpliesValidFalseAndNotCountedInErrors— summary counts correctcamelManagementKeyIsCheckedAndValid—camel.management.enabled=trueaccepted and validcamelVaultAwsKeyIsCheckedAndValid— nested 3-segment group (camel.vault.aws.*) accepted,issues()emptycamelVaultAwsInventedKeyIsCheckedButNotValid— invented option under nested group → valid=falsedashedGroupNameIsCheckedAndValid—camel.error-registry.enabled=trueaccepted (dash-insensitive)springBootStarterOnlyOptionIsUnchecked—camel.metrics.uriTagEnabledwithruntime=spring-boot→checked=false, not counted in errors; same key without runtime flag →checked=true, valid=falseCamelCatalogTest— new test methods:testValidateConfigurationPropertyNestedGroup— nested group + invented optiontestValidateConfigurationPropertyDashedGroupName— dashed-form group name, includingcamel.ai-observability.*testValidateConfigurationPropertyServerInventedOption— real and invented options undercamel.server.*Not in scope (tracked in CAMEL-25538)
camel.jbang.*validation againstjbangModel()(item 2)Hermes Agent (Claude Sonnet 4.6) on behalf of Guillaume Nodet