davsclaus commented on code in PR #26166:
URL: https://github.com/apache/camel/pull/26166#discussion_r3950034883
##########
core/camel-support/src/main/java/org/apache/camel/support/DefaultContextReloadStrategy.java:
##########
@@ -87,6 +99,68 @@ protected void reloadProperties(Object source) throws
Exception {
}
}
+ /**
+ * Re-applies the configuration properties whose value is a property
placeholder, so that components are
+ * re-configured with what those placeholders resolve to now.
+ * <p/>
+ * A component option such as
<tt>camel.component.kafka.saslJaasConfig</tt> has its placeholder resolved
once, when
+ * the component is configured, and the resolved value is what is stored
on the component. Reloading the routes
+ * rebuilds the endpoints from that same already-resolved value, so
without this step a rotated secret would never
+ * reach the component. Only <tt>camel.</tt> options whose value is a
placeholder are re-applied, as they are the
+ * only ones whose resolved value can change while the raw configuration
stays the same.
+ */
+ protected void reloadComponentProperties(Object source) throws Exception {
+ PropertiesReload pr =
getCamelContext().hasService(PropertiesReload.class);
+ if (pr == null) {
+ // component re-configuration is only supported when running with
Camel Main
+ return;
+ }
+
+ PropertiesComponent pc = getCamelContext().getPropertiesComponent();
+ Properties prop = pc.loadProperties();
+ // stringPropertyNames is a live view of the keys, so snapshot before
removing
+ Set<String> keys = new LinkedHashSet<>(prop.stringPropertyNames());
+ for (String key : keys) {
+ Object value = prop.get(key);
+ boolean placeholder = key.startsWith("camel.")
+ && value instanceof String str &&
str.contains(PropertiesComponent.PREFIX_TOKEN);
+ if (!placeholder) {
+ prop.remove(key);
+ }
+ }
+ if (!prop.isEmpty()) {
+ LOG.debug("Re-applying {} property placeholder based options to
components", prop.size());
+ pr.onReload(source != null ? source.toString() : "ContextReload",
prop);
+ }
+ }
+
+ /**
+ * Notifies every {@link SecretRotationAware} component and registry bean
that the secrets they captured may have
+ * been rotated, so they can re-authenticate before the routes are
restarted.
+ * <p/>
+ * A listener that throws is logged and skipped, so that one component
cannot prevent the others from being
+ * refreshed, nor fail the reload as a whole.
+ */
+ protected void notifySecretRotation(Object source) {
+ Set<SecretRotationAware> targets = new LinkedHashSet<>();
Review Comment:
**Missing test for the component path.**
The three test classes (`CamelContextSecretRotationAwareTest`,
`ContextReloadComponentPropertiesTest`, `MainContextReloadSecretRotationTest`)
all register `SecretRotationAware` implementations via the registry
(`context.getRegistry().bind(...)`). This loop — the component-level branch —
has no test coverage.
Since the `context-reload.adoc` documentation leads with components as the
primary use case, please add at least one test that registers a `Component`
implementing `SecretRotationAware` and verifies that `onSecretRotation` is
called on it during reload. For example, `CamelContextSecretRotationAwareTest`
could be extended with a minimal test component bound via
`context.addComponent(...)` and then assert it is notified alongside the
registry beans.
##########
docs/user-manual/modules/ROOT/pages/context-reload.adoc:
##########
@@ -6,12 +6,27 @@ upon an external triggered event.
For example, if you are using
xref:components::aws-secrets-manager-component.adoc[AWS Secrets], then
enabling context-reload would then reload Camel routes upon a secret is
updated in AWS.
-The context reload is limited to refresh the following on reload:
+The context reload refreshes the following on reload:
- xref:using-propertyplaceholder.adoc[property placeholders]
+- component options whose configured value is a property placeholder (requires
Camel Main, Camel Spring Boot or
+ Camel Quarkus)
+- all beans implementing `SecretRotationAware`, so they can re-authenticate in
place (see below)
- all existing xref:routes.adoc[routes] (no changes to structure of routes;
see xref:route-reload.adoc[]])
Review Comment:
Grammar nit (touched by this PR): "services … **is** not updated" should be
"services … **are** not updated".
```suggestion
Other general services in xref:camelcontext.adoc[CamelContext] and java
beans or Camel xref:processor.adoc[] are not
```
##########
docs/user-manual/modules/ROOT/pages/camel-4x-upgrade-guide-4_23.adoc:
##########
@@ -13,6 +13,20 @@ See the xref:camel-upgrade-recipes-tool.adoc[documentation]
page for details.
== Upgrading Camel 4.22 to 4.23
+=== Context reload now re-applies placeholder based component options
+
+When a context reload is triggered, for example by one of the vault components
detecting that a secret was rotated,
+Camel now also re-applies the `camel.component.`, `camel.dataformat.` and
`camel.language.` options whose configured
+value is a property placeholder, and notifies any bean implementing the new
+`org.apache.camel.spi.SecretRotationAware` SPI. Previously only the property
placeholders and the routes were
+reloaded, so a rotated secret never reached a component option that had been
resolved at bootstrap.
+
Review Comment:
**Wording may mislead: "re-creates the component" vs. "refreshes the
component fields".**
The PR body says the reload "refreshes the component fields", while this
line says it "re-creates the component". `MainPropertiesReload` sets new field
values on the existing component instance rather than constructing a new one.
"Re-creates" implies a new instance, which could mislead users who have state
they expect to persist (or, conversely, users who assume full
re-initialisation).
```suggestion
Re-applying such an option re-applies the configured options to the
component, as it already does when the option changes in a watched
```
--
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]
