ammachado opened a new pull request, #1917:
URL: https://github.com/apache/camel-spring-boot/pull/1917
## 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 #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 #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._
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
