Skip to content

CAMEL-24531: extract a shared early-resolution properties parser - #1917

Open
ammachado wants to merge 12 commits into
apache:mainfrom
ammachado:CAMEL-24531-shared-early-resolution-parser
Open

CAMEL-24531: extract a shared early-resolution properties parser#1917
ammachado wants to merge 12 commits into
apache:mainfrom
ammachado:CAMEL-24531-shared-early-resolution-parser

Conversation

@ammachado

Copy link
Copy Markdown
Contributor

What this does

Seven vault and secrets starters each carried a near-identical ApplicationListener<ApplicationEnvironmentPreparedEvent> that resolved {{<prefix>:...}} placeholders before the ApplicationContext exists. The bodies were roughly 108 lines each and differed only in the guard property, the override property source name, and how the PropertiesFunction is constructed.

This extracts the shared lifecycle into AbstractEarlyResolutionPropertiesParser in core/camel-spring-boot and reduces each starter to three small overrides.

Starter Listener Guard property
camel-aws-secrets-manager SpringBootAwsSecretsManagerPropertiesParser camel.component.aws-secrets-manager.early-resolve-properties
camel-azure-key-vault SpringBootAzureKeyVaultPropertiesParser camel.component.azure-key-vault.early-resolve-properties
camel-cyberark-vault SpringBootCyberArkVaultPropertiesParser camel.component.cyberark-vault.early-resolve-properties
camel-google-secret-manager SpringBootGoogleSecretManagerPropertiesParser camel.component.google-secret-manager.early-resolve-properties
camel-hashicorp-vault SpringBootHashicorpVaultPropertiesParser camel.component.hashicorp-vault.early-resolve-properties
camel-ibm-secrets-manager IBMSecretsManagerVaultPropertiesParser camel.component.ibm-secrets-manager.early-resolve-properties
camel-spring-cloud-config SpringBootCloudConfigPropertiesParser camel.component.spring-cloud-config.early-resolve-properties

The base class declares three abstract methods (getEarlyResolutionProperty(), getOverridePropertySourceName(), createPropertiesFunction(ConfigurableEnvironment)), one overridable getSourceDescription(), and a final onApplicationEvent.

Every override property source name is preserved byte for byte, including camel-ibm-secrets-manager-starter's asymmetric overridden-ibm-secrets-manager-properties, which lacks the camel- segment the other six use. That asymmetry is pre-existing and renaming it would break anyone looking that source up by name.

Behaviour changes

Most of this is a pure extraction, but three things do change. They are called out here so they can be reviewed as decisions rather than discovered as surprises.

1. Exception normalization. Two starters previously threw exception types that signal a programming defect rather than an operator configuration error, and that slip past any catch (RuntimeCamelException) handler.

Path Before After
Google client creation RuntimeException wrapping IOException RuntimeCamelException, cause chained
Hashicorp missing token, host, port, scheme NullPointerException via Objects.requireNonNull RuntimeCamelException, original message plus the property key
Hashicorp non-numeric port NumberFormatException naming no property RuntimeCamelException naming camel.vault.hashicorp.port, cause chained

No message became less specific. Objects.requireNonNull's messages were already good (for example "Hashicorp Vault token is required") and they survive verbatim with the property key appended. The port case is the one that was genuinely unhelpful before, since Integer.parseInt reports For input string: "..." without ever naming the property the operator got wrong.

2. Blank values are now rejected for the four Hashicorp settings. The new required() helper uses ObjectHelper.isEmpty, so camel.vault.hashicorp.port= now fails with "port is required (set camel.vault.hashicorp.port)" instead of reaching Integer.parseInt and dying on an empty string. An empty scheme now fails in the parser rather than deeper inside VaultEndpoint.

3. Placeholder unwrapping is now exact. The old code did value.replace("{{aws:", "").replace("}}", ""), a global replace that would corrupt a secret path legitimately containing }}. The shared version uses substring against the known delimiter lengths. There is a test pinning this: {{test:a}}b}} must yield the remainder a}}b, which the old code turned into the empty string.

A related simplification: the placeholder prefix is no longer hardcoded per starter. PropertiesFunction.getName() already returns exactly the prefix token (aws, azure, gcp, and so on), so the base class derives "{{" + fn.getName() + ":" itself. The prefix can no longer drift out of sync with the function that resolves it.

Deliberately out of scope

  • Property source precedence (CAMEL-24532). Iteration order and last-write-wins semantics are preserved exactly. That defect is tracked separately and assigned to another contributor.
  • Concatenated placeholders. A value such as uri={{aws:user}}:{{aws:pass}} satisfies both startsWith and endsWith, so it is unwrapped into a garbage remainder and fails. This behaviour is unchanged from before this PR and from before CAMEL-24508: vault and secrets starters - fail closed when early property resolution fails #1900, so it is not a regression, but the class javadoc now documents the limitation instead of claiming such values are left alone. Worth its own ticket.
  • Environment variables and relaxed binding. SystemEnvironmentPropertySource extends MapPropertySource, so a placeholder in MY_SECRET is resolved and stored under the literal key MY_SECRET, while Spring's relaxed binding then resolves my.secret from the environment source rather than the exact-match override source. The unresolved placeholder stays effective with no error. This arrives from CAMEL-24508: vault and secrets starters - fail closed when early property resolution fails #1900 unchanged by this refactor and fixing it means changing property source semantics. Flagging it because it is security relevant and deserves its own ticket.

Testing

mvn -o verify passes in all 8 affected modules with zero failures and zero errors.

  • core/camel-spring-boot: 156 tests plus 2 integration tests. 11 of those are new, covering the shared class: the guard, flag-read ordering, whole-value matching, OriginTrackedValue unwrapping, exact delimiter stripping, failure aggregation with causes attached via addSuppressed, no override source registered on failure, tolerant mode, and a null resolution result.
  • Each starter gains a contract test asserting its guard property and override property source name as exact literals. These seven tests are intentionally near-identical rather than factored into a shared helper: a wrong guard key fails open, meaning early resolution silently never runs and the placeholder stays in the property value as the effective secret. A parameterized helper would compare a typo against itself.
  • camel-hashicorp-vault-starter's EarlyResolvedPropertiesTest ran end to end against a real Testcontainers Vault and passed.

Two coverage gaps worth stating plainly rather than leaving to be found:

  • The Google IOException to RuntimeCamelException change has no test. Forcing SecretManagerServiceClient.create to throw requires credential environment manipulation, and the alternatives drag live GCP into a unit test. Verified by inspection.
  • The remaining EarlyResolvedPropertiesTest classes are guarded by @EnabledIfSystemProperty and @EnabledIfEnvironmentVariable and skip without live cloud credentials. That is pre-existing. The behavioural safety net for this refactor is the shared class's own test suite in core/camel-spring-boot, which runs everywhere.

No new dependencies, no POM changes, and therefore no regenerated code. 16 files changed.

Note on sequencing

This originally stacked on #1900. That PR has since merged, and this branch has been rebased directly onto main, so it no longer depends on anything unmerged.


Claude Code on behalf of Adriano Machado.

@ammachado
ammachado marked this pull request as ready for review August 30, 2026 22:34

@davsclaus davsclaus left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice extraction. I diffed the migrated SpringBootAwsSecretsManagerPropertiesParser, SpringBootGoogleSecretManagerPropertiesParser, SpringBootHashicorpVaultPropertiesParser, and IBMSecretsManagerVaultPropertiesParser line-by-line against their pre-refactor versions: the shared AbstractEarlyResolutionPropertiesParser faithfully reproduces the original per-parser control flow, and the three called-out behavior changes (exception normalization, blank-value rejection for the Hashicorp settings, exact delimiter stripping instead of a global replace) are each covered by a dedicated test in AbstractEarlyResolutionPropertiesParserTest. The asymmetric overridden-ibm-secrets-manager-properties name is preserved exactly as claimed. CI is green across all jobs.

One thing worth flagging before merge, not a defect in this PR by itself: PR #1907 (CAMEL-24532) is open concurrently and touches the same 7 parser files to fix the property-source-precedence bug (duplicate keys across sources currently resolve last-write-wins instead of highest-precedence-wins). This PR is built on the pre-#1907 version of those files and, per its own description, intentionally keeps the old last-write-wins semantics (props.put(key, resolved) in the new shared base class) so that CAMEL-24532 can be handled separately — that's a sensible division per the linked JIRA tickets, but it means whichever of the two PRs merges second will need real rework rather than a routine rebase: if this PR merges first, #1907's fix has no per-parser loop left to attach its helper calls to and will need to move its precedence check into AbstractEarlyResolutionPropertiesParser's shared loop; if #1907 merges first, this branch would silently reintroduce the last-write-wins bug into the new shared class unless the precedence fix is carried forward manually. Worth deciding merge order and making sure whichever merges second reconciles with the other explicitly.

This review does not replace CodeRabbit/Sourcery/SonarCloud.

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

@davsclaus
davsclaus force-pushed the CAMEL-24531-shared-early-resolution-parser branch from 06b9c7d to 0bdcb78 Compare August 31, 2026 05:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants