allthingssecurity opened a new pull request, #27467: URL: https://github.com/apache/camel/pull/27467
# Description [CAMEL-25068](https://issues.apache.org/jira/browse/CAMEL-25068) item 2 Follow-up from the deep review of CAMEL-25048. A route created from a route template with the id of an existing route silently replaces that route. With a route `existing` (`from("direct:existing")`), `TemplatedRouteBuilder.builder(context, "myTemplate").routeId("existing")...add()` does not throw, and afterwards the only route `existing` consumes from the template's endpoint: the original route is gone. `doAddRouteFromTemplate` ends with `addRouteDefinition`, which first removes any route with the same id (that is how a plain route is updated). Kamelets go through the same method (`addRouteFromKamelet`), so two Kamelet endpoints with the same route id and different parameters (`kamelet:echo/same?prefix=a` and `kamelet:echo/same?prefix=b`) replace each other's route, and the first endpoint then sends to the second route. This change implements option (a) from the JIRA analysis: `doAddRouteFromTemplate` fails with `FailedToCreateRouteFromTemplateException` ("Route with id: existing already exists. Remove the existing route first or use another route id.") when a route definition or a route with that id already exists. It applies to every way of creating a route from a template (`TemplatedRouteBuilder`, `templatedRoute` in Java, XML and YAML, `addRouteFromTemplate`, route template parameters from properties) and to Kamelets, where the error is wrapped in the Kamelet's route creation failure. It is consistent with the existing "Duplicate id detected" check for node ids of a route from a template. The check runs before the template beans are bound and the route is added, so nothing is left behind; routes without an explicit id are not checked (they get a new id). Other options, for the review: - (b) log a WARN and keep replacing: no behaviour change, but still easy to miss; - (c) keep the behaviour and document it. Route reload: I checked what reload does with a file that has a `templatedRoute` (YAML probe on `main`). `RouteBuilder.updateRoutesToCamelContext`, which reload uses, removes and adds the plain routes of the file again, but does not add its templated routes again. So reload never goes through `doAddRouteFromTemplate` and cannot hit the new check, with `removeAllRoutes=false` or `true`. `RouteReloadTemplatedRouteTest` (camel-yaml-dsl) reloads such a file with `removeAllRoutes=false` and checks that the reload works and both routes run; it passes on `main` too, and guards the reload path against the new check. The probe also shows a separate, pre-existing problem, not changed here: with `removeAllRoutes=false` the templated route keeps its old parameters after a reload, and with the default `removeAllRoutes=true` it is gone after the reload. If that gets fixed (adding the templated routes again in `updateRoutesToCamelContext`), that change must remove their old routes first, as `pop ulateOrUpdateRoutes` does for plain routes; I can open a JIRA for it. Also fixed, in the same method: the existing "Duplicate id detected" error for node ids passed the `routeId` argument to `FailedToCreateRouteFromTemplateException`, whose constructor requires it to be non-null. For a route without an explicit id (two routes from a template with the same `prefixId` and a hardcoded node id, the case CAMEL-25048 made a duplicate), this threw `NullPointerException: routeId` instead. It now passes the id of the route definition (the generated one when none was given). Behaviour change: the 4.23 upgrade guide gets a "(Breaking change)" note under "Route templates": anyone who added a route from a template with the id of an existing route to replace it must now stop and remove the route first. The rule was checked with a small Lean 4 model of the route list (add by id with replacement as on `main`, add-or-fail with the change, and reload of a file). It proves that with the change an add from a template never removes a route and keeps the ids unique, and that it adds exactly what `main` adds when the id is not used. For reload, it models `main` (only plain routes are added again, and the templated route is lost with `removeAllRoutes=true`), and shows that a reload that adds templated routes again would fail with the change unless it removes their old routes first, and succeeds with the same result as `main` when it does. Tests: - `RouteTemplateExistingRouteIdTest` (camel-core): `TemplatedRouteBuilder` and `templatedRoute` (`addRouteFromTemplatedRoute`) with the id of a plain route, and two routes from the same template with the same id: each fails and the existing route keeps running unchanged; `duplicateNodeIdWithoutRouteId` for the `NullPointerException`; - `KameletExistingRouteIdTest` (camel-kamelet): two Kamelet endpoints with the same route id and other parameters; the second fails and the first keeps its route; - `RouteReloadTemplatedRouteTest` (camel-yaml-dsl): see above (passes on `main`, guards reload). The first two classes fail on `main` (two runs each): "Expecting code to raise a throwable", and for `duplicateNodeIdWithoutRouteId` a `NullPointerException: routeId` instead of `FailedToCreateRouteFromTemplateException`. With the change: - the camel-core suite passes (8091 tests, 0 failures, 45 skipped), and so do the suites of the core modules it is built with (9455 tests in total); - camel-kamelet (all 65 test classes, 88 tests, 0 failures, 3 skipped); - the route template, templated route, Kamelet and reload tests of camel-yaml-dsl (14 classes, 57 tests) and camel-xml-io-dsl (6 classes, 16 tests), 0 failures. # Target - [x] I checked that the commit is targeting the correct branch (Camel 4 uses the `main` branch) # Tracking - [x] If this is a large change, bug fix, or code improvement, I checked there is a [JIRA issue](https://issues.apache.org/jira/browse/CAMEL) filed for the change (usually before you start working on it). # Apache Camel coding standards and style - [x] 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 the core modules and the modules listed above, including the formatter and import-sort plugins. No generated files change. I did not run the full root build.) # AI-assisted contributions - [x] 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](https://claude.com/claude-code) -- 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]
