gnodet commented on code in PR #27669:
URL: https://github.com/apache/camel/pull/27669#discussion_r4239685071
##########
components/camel-hashicorp-vault/src/main/java/org/apache/camel/component/hashicorp/vault/HashicorpVaultPropertiesFunction.java:
##########
@@ -257,6 +262,24 @@ private String getSecretFromSource(String key, String
subkey, String defaultValu
return returnValue;
}
+ /**
+ * The name of a secret as the refresh task tracks it: engine:secret, or
only the secret for the default secret
+ * engine
+ */
+ private static String trackedSecretName(String secretEngine, String key) {
+ return "secret".equals(secretEngine) ? key : secretEngine + ":" + key;
Review Comment:
⚠️ **Bug: `trackedSecretName` produces `"null:key"` when `engine` is null**
If the `engine` field is still `null` (it's initialized lazily and can
remain null), `"secret".equals(null)` returns `false`, so the method falls
through to `secretEngine + ":" + key` — which Java evaluates as `"null:key"`.
That mangled key then propagates into `secrets` and `secretVersions`, and the
refresh task will never match a real Vault secret.
```suggestion
return engine == null || "secret".equals(secretEngine) ? key :
secretEngine + ":" + key;
```
##########
components/camel-hashicorp-vault/src/main/java/org/apache/camel/component/hashicorp/vault/HashicorpVaultPropertiesFunction.java:
##########
@@ -268,9 +291,17 @@ public CamelContext getCamelContext() {
}
/**
- * Ids of the secrets in use
+ * Ids of the secrets in use, as engine:secret, or only the secret for the
default secret engine
*/
public Set<String> getSecrets() {
return secrets;
}
+
+ /**
+ * The version of the secret (named as in {@link #getSecrets()}) that was
last resolved, or null if not known (such
+ * as when a specific version was requested).
+ */
+ public Integer getSecretVersion(String secretName) {
Review Comment:
ℹ️ **Minor: `getSecretVersion` Javadoc is missing `@param` and `@return`
tags**
The description is clear, but the method's `@param secretName` and `@return`
are absent. No action required — just flagging for completeness.
##########
components/camel-hashicorp-vault/src/main/java/org/apache/camel/component/hashicorp/vault/HashicorpVaultPropertiesFunction.java:
##########
@@ -257,6 +262,24 @@ private String getSecretFromSource(String key, String
subkey, String defaultValu
return returnValue;
}
+ /**
+ * The name of a secret as the refresh task tracks it: engine:secret, or
only the secret for the default secret
+ * engine
+ */
+ private static String trackedSecretName(String secretEngine, String key) {
+ return "secret".equals(secretEngine) ? key : secretEngine + ":" + key;
+ }
+
+ private void captureVersion(String secretName, VaultResponse rawSecret) {
+ if (rawSecret.getData().get("metadata") instanceof Map<?, ?> metadata
&& metadata.get("version") != null) {
+ try {
+ secretVersions.put(secretName,
Integer.valueOf(metadata.get("version").toString()));
+ } catch (NumberFormatException e) {
+ // ignore, the refresh task then uses the version it reads
first
+ }
Review Comment:
💡 **Suggestion: log at DEBUG instead of silently swallowing
`NumberFormatException`**
The inline comment explains the intent, but a `LOG.debug()` call here would
make it observable in production — especially useful when debugging a "secret
never triggers reload" scenario. The class already has a logger, so just add a
debug line.
```suggestion
} catch (NumberFormatException e) {
LOG.debug("Could not parse secret version for {}: {}",
secretName, metadata.get("version"), e);
// refresh task will use the version it reads first
}
```
--
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]
