CAMEL-24532: Preserve property-source precedence during early vault resolution

[atiaomar1978-hub]

Summary

Fixes CAMEL-24532.

Early-resolution listeners (vault, jasypt, spring-cloud-config) iterate Spring PropertySources in precedence order (highest first) and collect resolved values into a flat Properties map that is injected with addFirst. Using Properties.put meant duplicate keys were overwritten by lower-precedence sources, so the wrong value became effective after addFirst.

This change introduces EarlyResolutionPropertySources in core/camel-spring-boot with putIfAbsent and asString helpers, updates all early-resolution parsers to retain the first (highest-precedence) resolved value, and adds regression tests.

Test plan

• mvn -pl core/camel-spring-boot -Dtest=EarlyResolutionPropertySourcesTest test
• CI build on PR

AI-generated PR description on behalf of atiaomar1978-hub

[davsclaus]

Reviewed against this repo's rule files (.oss-ai-helper-rules/). Good fix — the early-resolution listeners (vault providers, jasypt, spring-cloud-config) iterate PropertySources highest-precedence-first but used Properties.put, so a lower-precedence source's value silently won after addFirst. Extracting asString()/putIfAbsent() into a shared EarlyResolutionPropertySources helper and applying it consistently across all seven listeners is a clean way to fix this without duplicating the same logic seven times.

Verified locally: checked out the branch, installed core/camel-spring-boot (new EarlyResolutionPropertySources + 3 unit tests, all passing), then built camel-cyberark-vault-starter, camel-jasypt-starter, and camel-hashicorp-vault-starter against it — all compiled and passed their tests, including the Hashicorp starter's Testcontainers-backed EarlyResolvedPropertiesTest, which exercised the real fix end-to-end. Also checked git log/git blame on the touched parsers — this extends rather than conflicts with prior work (the OriginTrackedValue handling being refactored here came from separate, earlier hardening).

Found an import-ordering issue in two of the seven modified parsers and pushed a fix commit directly to this branch (maintainer-edit is enabled, confirmed with the branch owner) rather than a suggestion block, since it was purely mechanical:

• SpringBootCyberArkVaultPropertiesParser.java: import org.apache.camel.spring.boot.EarlyResolutionPropertySources; was inserted before org.apache.camel.component.cyberark.vault.client.ConjurClient/ConjurClientFactory, which sort earlier alphabetically (component < spring).
• SpringBootJasyptPropertiesParser.java: same pattern — EarlyResolutionPropertySources was inserted before org.apache.camel.component.properties.PropertiesParser.

Rebuilt and retested both starters after the fix — still green.

Minor, non-blocking: the branch name (cursor/CAMEL-24532-...) doesn't follow this repo's fix/<ISSUE_ID> convention, though the commit message itself is correctly formatted (CAMEL-24532: <description>). Not something to hold up the PR over.

This review does not replace CodeRabbit/Sourcery/SonarCloud or a full static analysis pass.

This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.

[Croway]

I reproduced a property-precedence bug on current main. Each parser resolves a matching entry before putIfAbsent, so a shadowed lower-precedence placeholder is still contacted. With the fail-closed behavior from CAMEL-24508, a missing lower-precedence secret aborts startup even when a higher-precedence source defines the key; a successful lower resolution can also become effective because the resolved property source is added first.

Please track every key while iterating Spring property sources (highest precedence first), skip resolution for later duplicates in all eight early-resolution parsers, and log the skipped key and property-source name at DEBUG without logging the value. Add a regression with a higher-precedence plain property and a lower-precedence missing Hashicorp placeholder. It fails before the guard and passes afterward; the core and all eight affected starter suites pass locally.

AI-generated review comment on behalf of Federico Mariani.

[Croway]

Apply the shared higher-precedence check before resolving a placeholder. The DEBUG message identifies only the property key and source name; it does not log the value.

AI-generated review suggestions on behalf of Federico Mariani.

[Croway]

Add the shared precedence helper used by the parser suggestions. Only concrete MapPropertySource entries are compared, avoiding Spring Boot synthetic aggregate property sources.

AI-generated review suggestions on behalf of Federico Mariani.

[atiaomar1978-hub]

Review feedback addressed

Thanks @Croway and @davsclaus for the detailed review on CAMEL-24532.

This commit addresses @Croway's follow-up feedback:

1. EarlyResolutionPropertySources — added hasHigherPrecedenceProperty() and shouldSkipBecauseHigherPrecedenceDefines() (DEBUG log includes property key and source name only, never the value). Only concrete MapPropertySource entries are compared.
2. All eight early-resolution parsers — guard runs before contacting vault/jasypt/spring-cloud-config backends, skipping resolution when a higher-precedence source already defines the same key.
3. Regression tests — core unit tests cover the higher-precedence plain value + lower-precedence placeholder scenario; SpringBootHashicorpVaultPropertiesParserPrecedenceTest exercises the Hashicorp parser directly (Testcontainers-backed when Docker is available).

Commit: 4cf1c53 — please re-review when CI is green.

AI-generated on behalf of atiaomar1978-hub

[davsclaus]

Thanks for tracking down CAMEL-24532 and centralizing the precedence logic in EarlyResolutionPropertySources — that's a much better shape than duplicating the fix across all 8 parsers, and the shouldSkipBecauseHigherPrecedenceDefines short-circuit is a nice touch since it avoids pointless vault/network calls for placeholders that would be discarded anyway.

I verified the JIRA (CAMEL-24532) matches this PR's description, and the core precedence logic itself (putIfAbsent + the reference-equality guard in hasHigherPrecedenceProperty) looks correct. A couple of things worth a look before merge, plus a couple of open questions — nothing blocking:

1. Test coverage gap for the actual reported scenario: the JIRA specifically asks for "a regression test specifically covering duplicate keys across property sources" (i.e., the same placeholder resolvable in two precedence-ordered sources). See inline comment on EarlyResolutionPropertySourcesTest for details — the closest test to this exercises a hand-rolled simulation rather than a real parser's onApplicationEvent.
2. Only hashicorp-vault gets a dedicated parser-level regression test; the other 7 modified parsers (aws-secrets-manager, azure-key-vault, cyberark-vault, google-secret-manager, ibm-secrets-manager, jasypt, spring-cloud-config) get the identical code change with no parser-specific test, relying solely on the shared helper's unit tests.
3. See inline comments on the Jasypt parser and the helper class for smaller questions.

This review does not replace CodeRabbit/Sourcery/SonarCloud — it's a check against this project's own contribution rules and conventions.

This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.

[davsclaus]

Behavior change beyond the stated precedence fix, worth confirming intent: before this PR, the Jasypt parser only matched OriginTrackedValue-wrapped values (there was no else if (value instanceof String) branch here, unlike the other vault parsers). Routing through EarlyResolutionPropertySources.asString() now also matches plain String values, so Jasypt will start early-decrypting values it previously ignored. Probably a reasonable consistency fix, but it's a second, unannounced behavior change riding along with the precedence fix — might be worth calling out explicitly in the PR description, or splitting out.

[davsclaus]

This only inspects MapPropertySource instances, matching the pre-existing scanning loop in each parser, so a higher-precedence non-map source (e.g. command-line args via SimpleCommandLinePropertySource, which is an EnumerablePropertySource but not a MapPropertySource) defining the same key won't be detected here. Not a new bug introduced by this PR, but now that this logic lives in a shared helper named for general precedence handling, it might be worth a one-line doc note on the limitation so a future caller doesn't assume it covers all PropertySource types.

[davsclaus]

This is the test closest to the scenario the JIRA asks for ("duplicate keys across property sources"), but it goes through collectResolved/collectResolvedWithPrecedenceGuard, a hand-rolled loop defined in this test file that mimics the parsers' onApplicationEvent logic — it doesn't exercise any of the actual parser implementations. None of the 8 modified parsers has a test proving that two matching placeholders for the same key across precedence-ordered sources resolve to the highest-precedence one (the SpringBootHashicorpVaultPropertiesParserPrecedenceTest added in this PR covers a different case: a higher-precedence plain value suppressing a lower-precedence unresolved placeholder). Would be good to add at least one test that drives a real parser's onApplicationEvent with two placeholders for the same key to close that gap, matching what the ticket explicitly requested.