CAMEL-25518: camel-hashicorp-vault - Secret refresh detects updates in any engine and before the first check by allthingssecurity · Pull Request #27669 · apache/camel

allthingssecurity · 2026-10-11T00:03:50Z

Description

With camel.vault.hashicorp.refreshEnabled=true, the refresh task also tracks the secrets the hashicorp: properties function resolved (the documented behaviour when camel.vault.hashicorp.secrets is not set). Two problems:

The function recorded hashicorp:kv:db#password as db. The task reads a name without an engine from the secret engine, so it polled secret/metadata/db, got nothing, and an update of a secret in any other engine never triggered a reload.
The task's first run only records the version it sees. It runs one refresh period (60 s by default) after startup, so an update in that window became the baseline and the application kept the old value.

This change records the secret in the format of camel.vault.hashicorp.secrets, which the task already parses: engine:secret, or only secret for the default secret engine (so the names of those secrets do not change). It also remembers the version the function resolved (from data.metadata.version, not when a specific @version was requested), and the task starts from that version for a secret it has not seen yet. A lookup without engine is recorded like a secret of the default engine, which is how the task reads a name without engine. A version that cannot be parsed is logged at DEBUG. The recorded set and the new version map are concurrent collections, as the task reads them from its own thread. New public method: HashicorpVaultPropertiesFunction.getSecretVersion(String). The component docs (and their catalog copy) and the 4.23 upgrade guide describe the change, including the new names of secrets of other engines in getSecrets() and in the task's updates.

Tests:

New HashicorpVaultReloadTriggerTaskTest with a small in-process fake of the Vault KV v2 HTTP API (JDK HttpServer, no container): an update in the kv engine, an update before the first check, the recorded names, a secret resolved at a specific version (@1) that must not be compared with that version, and an unchanged secret that must not trigger a reload (the last two pass without the change too; they guard against a reload caused by the remembered version), and a lookup without engine (hashicorp:api on a function that has no engine yet), which is recorded as api like a secret of the default engine, not as null:api.
Without the change: the refresh task must read the metadata of the engine the secret was resolved from, but read [secret/metadata/db, secret/metadata/db], the application resolved version 1 but version 2 was not detected, updates: {}, and the recorded names are db and api instead of kv:db and api. The lookup without engine was recorded as null:api before the review follow-up (expected: <[api]> but was: <[null:api]>).
With the change, the camel-hashicorp-vault unit tests pass (9). The ITs need a Vault container; I did not run them.

Found with a TLA+ model of startup resolution, secret updates and the refresh runs: "after a complete check run with no update since it started, the application uses the current version of every secret" is violated for a kv secret (12 steps) and for an update before the first check (8 steps). I then reproduced both with the real classes.

Affected: 4.17.0 and later (the refresh was added by CAMEL-22676).

Target

I checked that the commit is targeting the correct branch (Camel 4 uses the main branch)

Tracking

If this is a large change, bug fix, or code improvement, I checked there is a JIRA issue filed for the change (usually before you start working on it).

Apache Camel coding standards and style

I checked that each commit in the pull request has a meaningful subject line and body.
I have run mvn clean install -DskipTests locally from root folder and I have committed all auto-generated changes.
(I built and tested camel-hashicorp-vault, including the formatter and import-sort plugins, and updated the catalog copy of the component page. I did not run the full root build.)

AI-assisted contributions

If this PR includes AI-generated code, commits have proper co-authorship attribution (e.g., Co-authored-by trailers) and the PR description identifies the AI tool used.
This PR was prepared with Claude Code (Claude Opus 5.5). The commit carries a Co-Authored-By trailer.

Claude Code on behalf of allthingssecurity

🤖 Generated with Claude Code

…n any engine and before the first check HashicorpVaultPropertiesFunction recorded a secret used as hashicorp:engine:secret by its name only. The refresh task reads a name without an engine from the secret engine, so for a secret in any other engine it read secret/metadata/<name>, got nothing, and never triggered a reload (the documented auto-detection only worked for the secret engine). The task also only recorded the version it saw in its first run, so a secret updated between the startup and the first check (one refresh period, 60 s by default) was taken as the baseline and the application kept using the old value. The properties function now records the secret in the format of camel.vault.hashicorp.secrets, which the task already understands (engine:secret, or only the secret for the default secret engine), and remembers the version it resolved (unless a specific version was requested). The task starts from that version for a secret it has not seen before. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

gnodet

Solid bug fix with good test coverage — the fake Vault HTTP server approach in the test is clever and makes the scenarios self-contained. Two items worth addressing.

This review was generated by an AI agent, Hermès on behalf of @gnodet.

…ithout engine as the default engine, log unparsable versions Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

gnodet

Thanks for the PR! One minor API hygiene note below.

This review was generated by an AI agent, Hermès on behalf of @gnodet.

…ly view of the tracked secrets Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

gnodet

All prior review findings are addressed.

trackedSecretName null engine bug: ✅ fixed — null engine correctly maps to the key without prefix
DEBUG logging for NumberFormatException in captureVersion: ✅ added with secret name, version value, and exception
@param/@return Javadoc on getSecretVersion: ✅ added
getSecrets() returning mutable view: ✅ wrapped in Collections.unmodifiableSet(secrets) in this commit

The trackedSecretName ↔ buildMetadataPath alignment is correct. The captureVersion version guard (ObjectHelper.isEmpty(version)) correctly excludes specific-version lookups. Thread-safety is sound (ConcurrentHashMap throughout). Test suite covers all key scenarios with a clean fake HTTP server approach.

This review was generated by an AI agent, Hermès on behalf of @gnodet.

github-actions · 2026-10-11T05:47:51Z

🌟 Thank you for your contribution to the Apache Camel project! 🌟
🤖 CI automation will test this PR automatically.

🐫 Apache Camel Committers, please review the following items:

First-time contributors require MANUAL approval for the GitHub Actions to run
You can use the command /component-test (camel-)component-name1 (camel-)component-name2.. to request a test from the test bot although they are normally detected and executed by CI.
You can label PRs using skip-tests and test-dependents to fine-tune the checks executed by this PR.
Build and test logs are available in the summary page. Only Apache Camel committers have access to the summary.

⚠️ Be careful when sharing logs. Review their contents before sharing them publicly.

davsclaus

Thanks, both fixes look right: secrets in non-default engines are now tracked as engine:key, and remembering the resolved version means an update before the first check triggers a reload without causing a spurious reload at startup. Good test coverage with the in-process server, and no secret values are logged.

One optional suggestion inline.

Existing issues on main (not introduced here, worth a separate JIRA): engine is a shared mutable field set inside apply(), so concurrent lookups can mix engines, and a lookup without an engine reuses the previous call's engine (or null). Also, rawSecret.getData() NPEs when client.read returns null and a subkey is given.

Claude Code on behalf of davsclaus. This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying. It is a static review against the project conventions and does not replace static analysis or specialized review tools.

…resolved version of a secret for the first refresh check A secret resolved again after an update, but before the first check of the refresh task, no longer hides that update from the task. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

github-actions · 2026-10-11T06:15:34Z

🧪 CI tested the following changed modules:

catalog/camel-catalog
components/camel-hashicorp-vault
docs

🔬 Scalpel shadow comparison — Scalpel: 9 of 704 tested, 25 compile-only — current: 9 all tested

Maveniverse Scalpel detected 9 affected modules (current approach: 9).

Skip-tests mode would test 9 modules (3 direct + 8 downstream), skip tests for 25 (generated code, meta-modules)

Modules Scalpel would test (9)

camel-hashicorp-vault ← components/camel-hashicorp-vault/src/main/docs/hashicorp-vault-component.adoc, components/camel-hashicorp-vault/src/main/java/org/apache/camel/component/hashicorp/vault/HashicorpVaultPropertiesFunction.java, components/camel-hashicorp-vault/src/main/java/org/apache/camel/component/hashicorp/vault/vault/HashicorpVaultReloadTriggerTask.java, components/camel-hashicorp-vault/src/test/java/org/apache/camel/component/hashicorp/vault/HashicorpVaultReloadTriggerTaskTest.java
camel-jbang-mcp ← downstream of org.apache.camel:camel-catalog
camel-jbang-plugin-mcp ← downstream of org.apache.camel:camel-jbang-core
camel-jbang-plugin-route-parser ← downstream of org.apache.camel:camel-route-parser
camel-jbang-plugin-tui ← downstream of org.apache.camel:camel-catalog
camel-jbang-plugin-validate ← downstream of org.apache.camel:camel-yaml-dsl-validator
camel-launcher-container ← downstream of org.apache.camel:camel-launcher
camel-yaml-dsl-validator ← downstream of org.apache.camel:camel-catalog
camel-yaml-dsl-validator-maven-plugin ← downstream of org.apache.camel:camel-yaml-dsl-validator

Modules with tests skipped (25)

apache-camel
camel-allcomponents
camel-catalog-console
camel-catalog-maven
camel-catalog-suggest
camel-componentdsl
camel-endpointdsl
camel-endpointdsl-support
camel-itest
camel-jbang-core
camel-jbang-it
camel-jbang-main
camel-jbang-plugin-edit
camel-jbang-plugin-generate
camel-jbang-plugin-kubernetes
camel-jbang-plugin-test
camel-kamelet-main
camel-launcher
camel-report-maven-plugin
camel-route-parser
camel-yaml-dsl
camel-yaml-dsl-deserializers
camel-yaml-dsl-maven-plugin
coverage
dummy-component

ℹ️ Shadow mode — Scalpel observes but does not affect test execution. Learn more

All tested modules (36 modules, 5m 13s total)

Total reactor time: 5m 13s

Module	Duration	Status
Camel :: Launcher	45.7s	SUCCESS
Camel :: JBang :: MCP	39.1s	SUCCESS
Camel :: Catalog :: Camel Catalog	22.7s	SUCCESS
Camel :: HashiCorp :: Key Vault	20.4s	SUCCESS
Camel :: YAML DSL	19.2s	SUCCESS
Camel :: YAML DSL :: Validator	19.1s	SUCCESS
Camel :: Component DSL	18.9s	SUCCESS
Camel :: JBang :: Plugin :: Kubernetes	16.6s	SUCCESS
Camel :: JBang :: Plugin :: Validate	16.5s	SUCCESS
Camel :: Docs	15.0s	SUCCESS
Camel :: JBang :: Plugin :: Testing	12.9s	SUCCESS
Camel :: Kamelet Main	12.5s	SUCCESS
Camel :: YAML DSL :: Deserializers	8.1s	SUCCESS
Camel :: Catalog :: Camel Route Parser	8.0s	SUCCESS
Camel :: Catalog :: Camel Report Maven Plugin	6.7s	SUCCESS
Camel :: All Components Sync point	5.2s	SUCCESS
Camel :: YAML DSL :: Validator Maven Plugin	4.1s	SUCCESS
Camel :: YAML DSL :: Maven Plugins	3.7s	SUCCESS
Camel :: Catalog :: Suggest (deprecated)	3.1s	SUCCESS
Camel :: Catalog :: Maven	2.6s	SUCCESS
Camel :: JBang :: Plugin :: MCP	1.5s	SUCCESS
Camel :: Assembly	1.5s	SUCCESS
Camel :: JBang :: Integration tests	1.4s	SUCCESS
Camel :: JBang :: Plugin :: Edit	1.3s	SUCCESS
Camel :: Coverage	1.2s	SUCCESS
Camel :: JBang :: Plugin :: Generate	1.1s	SUCCESS
Camel :: Catalog :: Dummy Component	1.1s	SUCCESS
Camel :: Endpoint DSL :: Support	0.9s	SUCCESS
Camel :: JBang :: Main	0.8s	SUCCESS
Camel :: Catalog :: Console	0.8s	SUCCESS
Camel :: Launcher :: Container	0.7s	SUCCESS
Camel :: JBang :: Plugin :: Route Parser	0.5s	SUCCESS
Camel :: Endpoint DSL	n/a
Camel :: Integration Tests	n/a
Camel :: JBang :: Core	n/a
Camel :: JBang :: Plugin :: TUI	n/a

Top 20 slowest modules:

Camel :: Launcher (45.7s)
Camel :: JBang :: MCP (39.1s)
Camel :: Catalog :: Camel Catalog (22.7s)
Camel :: HashiCorp :: Key Vault (20.4s)
Camel :: YAML DSL (19.2s)
Camel :: YAML DSL :: Validator (19.1s)
Camel :: Component DSL (18.9s)
Camel :: JBang :: Plugin :: Kubernetes (16.6s)
Camel :: JBang :: Plugin :: Validate (16.5s)
Camel :: Docs (15.0s)
Camel :: JBang :: Plugin :: Testing (12.9s)
Camel :: Kamelet Main (12.5s)
Camel :: YAML DSL :: Deserializers (8.1s)
Camel :: Catalog :: Camel Route Parser (8.0s)
Camel :: Catalog :: Camel Report Maven Plugin (6.7s)
Camel :: All Components Sync point (5.2s)
Camel :: YAML DSL :: Validator Maven Plugin (4.1s)
Camel :: YAML DSL :: Maven Plugins (3.7s)
Camel :: Catalog :: Suggest (deprecated) (3.1s)
Camel :: Catalog :: Maven (2.6s)

⚙️ View full build and test results

gnodet

All prior review findings are addressed in the current HEAD (67fb082):

trackedSecretName null engine bug: ✅ fixed — null engine correctly maps to the key without prefix
DEBUG logging for NumberFormatException in captureVersion: ✅ added with secret name, version value, and exception
@param/@return Javadoc on getSecretVersion: ✅ added
getSecrets() returning mutable view: ✅ wrapped in Collections.unmodifiableSet(secrets) in this commit
Keep oldest resolved version via Math::min (davsclaus): ✅ applied

The trackedSecretName ↔ buildMetadataPath alignment is correct: null or "secret" engine maps to just the key; other engines map to engine:key. The versionsMap.put(secretName, lastKnownVersion) before the change-detection check is correct — it seeds the baseline so subsequent runs compare against the resolved version rather than discarding it. Pre-fix test validation by static trace confirms both rotationInNonDefaultEngineIsDetected and rotationBeforeFirstCheckIsDetected would fail without the fix.

This review was generated by an AI agent, Hermès on behalf of @gnodet.

gnodet reviewed Oct 11, 2026

View reviewed changes

gnodet added the lts label Oct 11, 2026

CAMEL-25518: camel-hashicorp-vault - Address review: track a lookup w…

86a401a

…ithout engine as the default engine, log unparsable versions Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

gnodet reviewed Oct 11, 2026

View reviewed changes

Comment thread ...c/main/java/org/apache/camel/component/hashicorp/vault/HashicorpVaultPropertiesFunction.java Outdated

CAMEL-25518: camel-hashicorp-vault - Address review: return a read-on…

1a9153a

…ly view of the tracked secrets Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

gnodet reviewed Oct 11, 2026

View reviewed changes

github-actions Bot added components catalog docs and removed lts labels Oct 11, 2026

davsclaus approved these changes Oct 11, 2026

View reviewed changes

Comment thread ...c/main/java/org/apache/camel/component/hashicorp/vault/HashicorpVaultPropertiesFunction.java Outdated

This was referenced Oct 11, 2026

CAMEL-25552: camel-hashicorp-vault - the properties function no longer shares the secret engine between lookups #27679

Open

CAMEL-25553: vault properties functions - make the shared state thread safe #27680

Merged

gnodet reviewed Oct 11, 2026

View reviewed changes

davsclaus added this to the 4.23.0 milestone Oct 11, 2026

davsclaus assigned allthingssecurity Oct 11, 2026

davsclaus added the bug Something isn't working label Oct 11, 2026

davsclaus merged commit fa153a8 into apache:main Oct 11, 2026
6 checks passed

allthingssecurity deleted the camel-hashicorp-vault-refresh-all-engines branch October 11, 2026 07:08

Navigation Menu

Repository navigation

CAMEL-25518: camel-hashicorp-vault - Secret refresh detects updates in any engine and before the first check - #27669

CAMEL-25518: camel-hashicorp-vault - Secret refresh detects updates in any engine and before the first check#27669
davsclaus merged 4 commits into
apache:mainfrom
allthingssecurity:camel-hashicorp-vault-refresh-all-engines

allthingssecurity commented Oct 11, 2026 •

edited

Loading

Uh oh!

gnodet left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gnodet left a comment

Uh oh!

Uh oh!

gnodet left a comment

Uh oh!

github-actions Bot commented Oct 11, 2026

Uh oh!

davsclaus left a comment

Uh oh!

Uh oh!

github-actions Bot commented Oct 11, 2026 •

edited

Loading

Uh oh!

gnodet left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

allthingssecurity commented Oct 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Target

Tracking

Apache Camel coding standards and style

AI-assisted contributions

Uh oh!

gnodet left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gnodet left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

gnodet left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Oct 11, 2026

Uh oh!

davsclaus left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions Bot commented Oct 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gnodet left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

allthingssecurity commented Oct 11, 2026 •

edited

Loading

github-actions Bot commented Oct 11, 2026 •

edited

Loading