oscerd commented on code in PR #26845: URL: https://github.com/apache/camel/pull/26845#discussion_r4193884534
########## core/camel-support/src/main/java/org/apache/camel/support/DataSourceHelper.java: ########## @@ -0,0 +1,170 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.camel.support; + +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; +import java.util.Collections; +import java.util.IdentityHashMap; +import java.util.Set; +import java.util.function.Function; + +import javax.sql.DataSource; + +import org.apache.camel.Component; +import org.apache.camel.Endpoint; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * Utility methods for working with JDBC {@link DataSource} instances. + */ +public final class DataSourceHelper { + + private static final Logger LOG = LoggerFactory.getLogger(DataSourceHelper.class); + + private DataSourceHelper() { + } + + /** + * Collects and evicts stale connections from all DataSources used by a component: the component's own + * {@code dataSource} field plus any DataSources held by the component's active endpoints. + * <p/> + * Identity-based deduplication ensures each DataSource is evicted at most once, even if the same instance is shared + * between the component and one or more endpoints. + * + * @param componentDataSource the component-level DataSource (may be null) + * @param endpoints the component's active endpoints (from {@code getCamelContext().getEndpoints()}) + * @param componentInstance the component instance, used to filter endpoints + * ({@code endpoint.getComponent() == this}) + * @param endpointDsExtractor a function to extract the DataSource from an endpoint (may return null) + * @param source an opaque label for the rotation event (used in log messages only) + */ + public static void evictComponentDataSources( + DataSource componentDataSource, + Iterable<Endpoint> endpoints, + Component componentInstance, + Function<Endpoint, DataSource> endpointDsExtractor, + Object source) { + + Set<DataSource> dataSources = Collections.newSetFromMap(new IdentityHashMap<>()); + if (componentDataSource != null) { + dataSources.add(componentDataSource); + } + if (endpoints != null) { + for (Endpoint ep : endpoints) { + if (ep.getComponent() == componentInstance) { Review Comment: **Endpoint DataSources are silently skipped when the reload re-created the component, and for intercepted endpoints** The `ep.getComponent() == componentInstance` check does not hold in a supported reload configuration. `DefaultContextReloadStrategy.onReload()` runs `reloadComponentProperties()` before `notifySecretRotation()`, and with Camel Main `doAutoConfigurationFromProperties(..., reload=true)` calls `camelContext.removeComponent(name)` and resolves the component again for every `camel.component.<name>.*` option whose value is a placeholder (the 4.23 upgrade guide documents that such a component "is replaced by a new instance"). `notifySecretRotation()` then notifies the new instance, which passes `this` here, while every endpoint still in the registry references the old, stopped instance, because the routes are only reloaded afterwards. They are all filtered out and nothing is logged: for `JdbcComponent` (`jdbc:myDs` style) the callback becomes a complete no-op, and for `SqlComponent` only an autowired component DataSource is evicted, never `dataSource=#myDs` endpoints. A second miss in the same loop: with `interceptSendToEndpoint("jdbc:*")` / `("sql:*")` the registry holds a `DefaultInterceptSendToEndpoint` proxy whose `getComponent()` delegates (so the identity check passes), but the extractors' `instanceof JdbcEndpoint` / `instanceof DefaultSqlEndpoint` is false, so that DataSource is skipped too. Unwrapping `InterceptSendToEndpoint#getOriginalEndpoint()` first and matching on the endpoint type (the extractors already do the `instanceof`) instead of on instance identity would cover both; at worst a second component instance sharing the pool evicts it twice, which you already showed is harmless: ```java Endpoint target = ep instanceof InterceptSendToEndpoint ise ? ise.getOriginalEndpoint() : ep; DataSource ds = endpointDsExtractor.apply(target); ``` A unit test that calls `context.removeComponent("jdbc")` / `context.getComponent("jdbc")` before `onSecretRotation` on the new instance would pin it. ########## core/camel-support/src/main/java/org/apache/camel/support/DataSourceHelper.java: ########## @@ -0,0 +1,170 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.camel.support; + +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; +import java.util.Collections; +import java.util.IdentityHashMap; +import java.util.Set; +import java.util.function.Function; + +import javax.sql.DataSource; + +import org.apache.camel.Component; +import org.apache.camel.Endpoint; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * Utility methods for working with JDBC {@link DataSource} instances. + */ +public final class DataSourceHelper { + + private static final Logger LOG = LoggerFactory.getLogger(DataSourceHelper.class); + + private DataSourceHelper() { + } + + /** + * Collects and evicts stale connections from all DataSources used by a component: the component's own + * {@code dataSource} field plus any DataSources held by the component's active endpoints. + * <p/> + * Identity-based deduplication ensures each DataSource is evicted at most once, even if the same instance is shared + * between the component and one or more endpoints. + * + * @param componentDataSource the component-level DataSource (may be null) + * @param endpoints the component's active endpoints (from {@code getCamelContext().getEndpoints()}) + * @param componentInstance the component instance, used to filter endpoints + * ({@code endpoint.getComponent() == this}) + * @param endpointDsExtractor a function to extract the DataSource from an endpoint (may return null) + * @param source an opaque label for the rotation event (used in log messages only) + */ + public static void evictComponentDataSources( + DataSource componentDataSource, + Iterable<Endpoint> endpoints, + Component componentInstance, + Function<Endpoint, DataSource> endpointDsExtractor, + Object source) { + + Set<DataSource> dataSources = Collections.newSetFromMap(new IdentityHashMap<>()); + if (componentDataSource != null) { + dataSources.add(componentDataSource); + } + if (endpoints != null) { + for (Endpoint ep : endpoints) { + if (ep.getComponent() == componentInstance) { + DataSource ds = endpointDsExtractor.apply(ep); + if (ds != null) { + dataSources.add(ds); + } + } + } + } + for (DataSource ds : dataSources) { + evictDataSourceConnections(ds, source); + } + } + + /** + * Evicts stale connections from the given DataSource's connection pool. + * <p/> + * The following pool implementations are supported (detected via reflection — no compile-time dependency required): + * <ul> + * <li><b>HikariCP</b>: {@code softEvictConnections()} is called via {@code HikariPoolMXBean}. Idle connections are + * evicted immediately; borrowed connections are evicted when returned.</li> + * <li><b>Agroal</b> (Quarkus default): {@code flush(GRACEFUL)} is called on {@code AgroalDataSource}. Idle + * connections are closed immediately; active connections are closed when returned to the pool.</li> + * </ul> + * Any DataSource not recognised as one of the above is left untouched — a log message is emitted and the pool will + * naturally replace connections as they expire or are validated. + * <p/> + * <b>Important:</b> this method only evicts existing connections from the pool. It does <em>not</em> update the + * pool's credentials. For pools configured with a static password (e.g. Spring Boot + * {@code spring.datasource.password}), the pool will re-open connections using the <em>old</em> credentials unless + * the credentials are resolved dynamically (e.g. {@code HikariCredentialsProvider}, the AWS JDBC wrapper secrets + * plugin, or a custom {@code DataSource} that fetches credentials from a vault at connect time). + * + * @param ds the DataSource whose connections should be evicted + * @param source an opaque label for the rotation event (used in log messages only) + */ + public static void evictDataSourceConnections(DataSource ds, Object source) { + // HikariCP: softEvictConnections() is defined on HikariPoolMXBean, not on HikariDataSource directly. + // We retrieve the MXBean via getHikariPoolMXBean() (a public method on HikariDataSource) using reflection + // so that the caller does not need a compile-time dependency on HikariCP. + try { + Method getPoolMXBean = ds.getClass().getMethod("getHikariPoolMXBean"); Review Comment: **Pools behind a java.sql.Wrapper (Spring proxies, tracing wrappers) are never recognised** Detection only inspects the concrete class (`getHikariPoolMXBean` on `ds.getClass()`, `agroalDsClass.isInstance(ds)`), so a HikariCP or Agroal pool exposed through a delegating wrapper, such as Spring's `TransactionAwareDataSourceProxy`/`LazyConnectionDataSourceProxy` or a JDBC tracing proxy, falls through to "not a recognised connection pool". `DataSource` extends `java.sql.Wrapper`, and both Spring's `DelegatingDataSource` and `HikariDataSource` implement `isWrapperFor`/`unwrap`, so loading the pool type by name (as the Agroal branch already does) and trying `ds.isWrapperFor(type)` → `ds.unwrap(type)` before falling back would recognise them. ########## components/camel-sql/src/main/java/org/apache/camel/component/sql/SqlComponent.java: ########## @@ -151,6 +153,16 @@ protected Endpoint createEndpoint(String uri, String remaining, Map<String, Obje return endpoint; } + @Override + public void onSecretRotation(Object source) throws Exception { Review Comment: **sql-stored (same module, same DataSource pattern) is not covered** `SqlStoredComponent` (`sql-stored:`) has the same autowired component `dataSource` plus an endpoint-level `dataSource` that takes precedence, but does not implement `SecretRotationAware`, so pools used only by `sql-stored:` endpoints are never evicted (`spring-jdbc` is covered implicitly, since `SpringJdbcComponent extends JdbcComponent`). The same one-liner with `SqlStoredEndpoint` would close the gap; otherwise the docs could say sql-stored is not covered. ########## components/camel-jdbc/src/main/docs/jdbc-component.adoc: ########## @@ -336,3 +336,29 @@ from("timer://MoveNewCustomersEveryHour?period=3600000") .setBody(simple("insert into processed_customer values('${body[ID]}','${body[NAME]}')")) .to("jdbc:testdb"); ---- + +== Secret Rotation + +The JDBC component implements `SecretRotationAware`. When a secret rotation event is triggered Review Comment: **Docs: eviction runs on every context reload, not only on vault-detected rotation** `DefaultContextReloadStrategy.onReload()` calls `notifySecretRotation()` unconditionally, so the pools are soft-evicted on every context reload, whatever changed: besides the vault refresh tasks, that includes the JMX `onReload` operation, `camel cmd reload` and load-on-demand reloads. These pools are often shared with non-Camel code, so it would help to say so here (and in sql-component.adoc), and a line in the 4.23 upgrade-guide section "Context reload now re-applies placeholder based component options" would fit the rule for behaviour changes for users who already have context reload enabled. ########## components/camel-jdbc/src/main/docs/jdbc-component.adoc: ########## @@ -336,3 +336,29 @@ from("timer://MoveNewCustomersEveryHour?period=3600000") .setBody(simple("insert into processed_customer values('${body[ID]}','${body[NAME]}')")) .to("jdbc:testdb"); ---- + +== Secret Rotation + +The JDBC component implements `SecretRotationAware`. When a secret rotation event is triggered +(e.g. by a vault provider), the component evicts stale connections from its connection pools. +This covers the component-level DataSource as well as any DataSources resolved by active endpoints +(e.g. `jdbc:myDs`), with identity-based deduplication so each pool is evicted at most once. + +Currently HikariCP and Agroal (Quarkus default) pools are supported for active eviction. +HikariCP pools are evicted via `softEvictConnections()` called through `getHikariPoolMXBean()` +via reflection — JMX registration (`registerMbeans=true`) is *not* required. +Agroal pools are evicted via `flush(GRACEFUL)` called on `AgroalDataSource` via reflection. +Other pool implementations are not actively evicted — existing connections +will be replaced as they expire or are validated by the pool. + +NOTE: The pool eviction uses reflection to avoid compile-time dependencies on pool libraries. +In GraalVM native mode (e.g. Quarkus native), the reflected classes and methods need to be registered +for reflection: `HikariDataSource.getHikariPoolMXBean()`, `HikariPoolMXBean.softEvictConnections()` +for HikariCP, and `AgroalDataSource.flush(FlushMode)` for Agroal. + +IMPORTANT: The eviction only closes existing connections — it does *not* update the pool's credentials. +For pools configured with a static password (e.g. Spring Boot `spring.datasource.password`), +the pool will re-open connections using the _old_ credentials. +This feature works out of the box only with pools that resolve credentials dynamically, +such as `HikariCredentialsProvider`, the AWS JDBC wrapper secrets plugin, Review Comment: **Docs: HikariCredentialsProvider only exists from HikariCP 7.0** `com.zaxxer.hikari.HikariCredentialsProvider` only exists from HikariCP 7.0, while Camel manages `hikaricp-version` 6.3.0 (6.x only offers the runtime `HikariConfigMXBean#setPassword`/`setCredentials` setters). Qualifying the reference saves 6.x users from looking for a type that is not there (the same line is in sql-component.adoc and the catalog copies): ```suggestion such as a `HikariCredentialsProvider` (HikariCP 7.0 or later), the AWS JDBC wrapper secrets plugin, ``` -- 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]
