oscerd commented on code in PR #26166:
URL: https://github.com/apache/camel/pull/26166#discussion_r3951969405
##########
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:
Good catch, fixed. You were right that the component branch was completely
uncovered - all three test classes went through the registry.
`CamelContextSecretRotationAwareTest` now adds a `MyRotationAwareComponent
extends DefaultComponent implements SecretRotationAware` via
`context.addComponent(...)`, with two new tests:
- `testComponentIsNotified` - the component branch itself.
- `testComponentAlsoInRegistryIsNotifiedOnce` - the same instance registered
both as a component and as a registry bean, which is what happens on Spring
Boot, must be notified exactly once. This pins down the `LinkedHashSet`
de-duplication in `notifySecretRotation`, which was also untested.
`testNotifiedOnEveryReload` and `testFailingListenerDoesNotBreakTheReload`
now assert on the component too, so the isolation guarantee is covered for
components and not just for registry beans.
I verified the new coverage is real by temporarily deleting the
`getComponentNames()` loop: 3 of the 6 tests fail (`expected: 1 but was: 0`).
Restored, all 6 green.
_Claude Code on behalf of oscerd_
##########
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:
Thanks - you found a real inaccuracy, though the underlying premise is only
half right, so I have rewritten the paragraph rather than applying the
suggestion as-is.
`BaseMainSupport.doAutoConfigurationFromProperties` with `reload=true` does
call `camelContext.removeComponent(name)` (which also **stops** the old
component) and then `getComponent(name)`. What comes back depends on how the
component was registered, because `AbstractCamelContext.initComponent` tries
the registry first and only then the resolver:
```java
component =
ResolverHelper.lookupComponentInRegistryWithFallback(getCamelContextReference(),
name);
if (component == null) {
component =
PluginHelper.getComponentResolver(camelContextExtension).resolveComponent(name,
...);
}
```
and `DefaultComponentResolver.resolveComponent` ends in
`context.getInjector().newInstance(type, false)`.
I checked this empirically rather than relying on reading, with a throwaway
test comparing identity hashes across a reload:
| component | registered via | before | after | same instance |
|---|---|---|---|---|
| `dummy` | `main.bind(...)` (registry) | 810629779 | 810629779 | yes |
| `seda` | classpath, `META-INF/services` | 933763177 | 906404427 | **no** |
So "re-creates" was wrong for registry-bound components, and "re-applies the
configured options to the component" would be wrong for classpath-resolved ones
- which is the common case for something like `camel-kafka` configured purely
through `camel.component.kafka.*`.
The guide now describes both paths, and adds the consequence that actually
matters to someone upgrading: the component is stopped either way, so one
holding a connection will close it on reload. I also changed the closing advice
from "must not be re-created" to "must not be stopped" for the same reason.
_Claude Code on behalf of oscerd_
##########
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:
Agreed, applied - "services ... are not updated". Thanks for catching it on
a line this PR was already touching.
_Claude Code on behalf of oscerd_
--
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]
