CAMEL-18552: camel-jbang --openapi-ui with Swagger UI at /q/openapi - #25202
Open
atiaomar1978-hub wants to merge 7 commits into
Open
CAMEL-18552: camel-jbang --openapi-ui with Swagger UI at /q/openapi#25202atiaomar1978-hub wants to merge 7 commits into
atiaomar1978-hub wants to merge 7 commits into
Conversation
Contributor
Author
Review summary (Bugbot + Grok-style pass)Addressed in commit
Not changed (noted):
False positive / out of scope:
Tests re-run locally: 6 tests in AI-generated comment on behalf of atiaomar1978-hub |
Contributor
|
🌟 Thank you for your contribution to the Apache Camel project! 🌟 🐫 Apache Camel Committers, please review the following items:
|
Contributor
|
🧪 CI tested the following changed modules:
🔬 Scalpel shadow comparison — Scalpel: 575 tested, 25 compile-only — current: 76 all testedMaveniverse Scalpel detected 600 affected modules (current approach: 76).
|
gnodet
reviewed
Jul 30, 2026
gnodet
left a comment
gnodet
left a comment
Contributor
There was a problem hiding this comment.
Claude Code on behalf of Guillaume Nodet
Review: CAMEL-18552 — camel-jbang --openapi-ui with Swagger UI
Nice feature addition! The architecture is clean — bundling swagger-ui as a webjar, serving it from the management HTTP server, and gating it with @Metadata(security = "insecure:dev") follows Camel's established patterns for dev-only tooling. The port-alignment logic in prepareOpenApiUiServerOptions() is well thought out, and the test coverage across four classes is solid.
A few items worth addressing:
1. Duplicate configuration writes in Run.java
The if (serverOptions.openapiUi) block calls both main.addOverrideProperty(...) and writeSetting(main, profileProperties, ...) for the same 5 keys (camel.rest.apiContextPath, camel.rest.component, camel.management.openapiUiEnabled, camel.server.enabled, camel.management.enabled).
Other feature flags in the same method use one or the other — observe uses only addOverrideProperty, health/metrics/console use only writeSetting. Since addOverrideProperty always wins at runtime, the writeSetting calls are effectively redundant (though they do write to the settings file as a side effect). I'd suggest picking one pattern to stay consistent with the neighboring code.
2. Hardcoded swagger-ui version in two separate files
Version 5.21.0 appears in both:
components/camel-platform-http-main/pom.xml(Maven dependency)OpenApiUiSupport.java(SWAGGER_UI_WEBJAR_VERSIONconstant)
The Java constant even has a comment (must match pom.xml) — which acknowledges the coupling but provides no enforcement. If one is bumped without the other, the StaticHandler will look for resources in a classpath directory that doesn't exist, silently breaking asset loading.
Other dependencies in the same POM use Maven properties (${vertx-version}, ${rest-assured-version}). Consider either:
- Extracting a
<swagger-ui-version>Maven property and using resource filtering / a generated properties file - Reading the version at runtime from
META-INF/maven/org.webjars/swagger-ui/pom.properties
3. KameletMain unconditionally overrides rest.component and rest.apiContextPath
When openapiUi is true, KameletMain.java calls:
configure().rest().withComponent("platform-http");
configure().rest().withApiContextPath("/q/openapi.json");This clobbers any user-set camel.rest.component or camel.rest.apiContextPath. I notice other features (console, health) follow the same unconditional-override pattern in KameletMain, but those only toggle their own management features. Here the override affects shared REST DSL configuration that users may have deliberately set (e.g., servlet as their REST component, or a custom API doc path). Worth at least documenting that --openapi-ui will override these settings, or checking whether they're already configured before overriding.
4. Defense-in-depth: escape specPath in HTML template
In OpenApiUiSupport.buildIndexHtml(), specPath is interpolated directly into a <script> block via String.formatted() without JavaScript escaping. Since specPath comes entirely from trusted server-side configuration (openapiUiSpecPath property), this isn't exploitable by external attackers per Camel's security model. However, escaping quotes and angle brackets before embedding in JS would be a good defense-in-depth measure, especially since this is a <script> context.
5. Generated jbang metadata missing security annotation
CamelJBangConstants.java correctly declares @Metadata(security = "insecure:dev") on OPENAPI_UI, but the generated camel-jbang-configuration-metadata.json doesn't include the security field. Compare with camel.management.openapiUiEnabled in the management metadata which correctly shows "security": "insecure:dev". This is likely a jbang metadata generator limitation rather than a PR-specific issue, but worth being aware of — it means the prod-profile security enforcement may not catch this setting via the jbang path.
Minor suggestions
- Test for explicit
--management-port:prepareOpenApiUiServerOptions()has an important guard that preserves explicitly set management ports. Adding a test verifying--management-port=9999is preserved when combined with--openapi-uiwould strengthen coverage. - Upgrade guide entry: Per project conventions, new user-visible features like
--openapi-uiand thecamel.management.openapiUi*properties should get an entry in the 4.22 upgrade guide.
Overall this is well done — the security gating, port co-binding logic, and test suite all show careful work. 👍
Contributor
Author
|
AI-generated comment on behalf of atiaomar1978-hub Pushed
New/extended tests: explicit mvn test -pl components/camel-platform-http-main -Dtest=OpenApiUiHttpServerTest,OpenApiUiRestMainTest,OpenApiUiSupportTest
mvn test -pl dsl/camel-jbang/camel-jbang-core -Dtest=RunOpenApiUiOptionsTestAll green locally (8 + 3 tests). |
gnodet
approved these changes
Jul 31, 2026
gnodet
left a comment
gnodet
left a comment
Contributor
There was a problem hiding this comment.
Re-review after latest commits. All five items from the previous review are properly addressed:
Run.javaproperty handling — Now usesaddOverridePropertyfor runtime config andwriteSettingonly for the persisted jbang property. ✅- Swagger UI version management — Version is resolved at runtime from
META-INF/maven/org.webjars/swagger-ui/pom.propertiesand Maven uses a${swagger-ui-version}property. ✅ - Config override guard —
KameletMainnow checksisConfigured()before overridingcamel.rest.componentandcamel.rest.apiContextPath. ✅ - XSS prevention —
escapeForJavaScriptString()sanitizesspecPathbefore embedding in the HTML script block. ✅ - Security annotation —
PrepareCamelJBangMojonow reads and emits the security annotation from@Metadata. ✅
Additional quality improvements noted:
- Test for explicit
--management-portpreservation added (shouldPreserveExplicitManagementPortForOpenApiUi) - 4.22 upgrade guide entry written
- CI fully green on Java 17 and 25
Minor non-blocking suggestions for follow-up:
- The swagger-ui webjar could be marked
optionalincamel-platform-http-main/pom.xmlto reduce module size for non-OpenAPI-UI users. - Worth documenting that
--openapi-uitakes precedence over file-based REST configuration (e.g.camel.rest.component=servletinapplication.properties).
This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.
Claude Code on behalf of Guillaume Nodet
Expose bundled Swagger UI on the embedded management server and wire the JBang --openapi-ui flag to enable REST OpenAPI at /q/openapi.json. Co-authored-by: Cursor <cursoragent@cursor.com>
Align management and app HTTP ports for --openapi-ui, persist server enablement in Run, preserve absolute openapiUiSpecPath URLs, and extend tests. Co-authored-by: Cursor <cursoragent@cursor.com>
Commit generated camel-catalog files so CI sourcecheck passes after build. Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Remove stale timeoutPoolSize attribute so catalog matches post-build output and CI sourcecheck passes. Co-authored-by: Cursor <cursoragent@cursor.com>
Use addOverrideProperty only for openapi-ui runtime settings, resolve swagger-ui version from the webjar pom.properties, escape spec paths in Swagger UI HTML, preserve user REST settings in KameletMain, emit jbang security metadata, add management-port test, and document --openapi-ui in the 4.22 upgrade guide. Co-authored-by: Cursor <cursoragent@cursor.com>
atiaomar1978-hub
force-pushed
the
CAMEL-18552-jbang-openapi-ui
branch
from
July 31, 2026 23:58
3855e77 to
3068b9e
Compare
Contributor
Author
|
AI-generated comment on behalf of atiaomar1978-hub Rebased onto latest @gnodet review follow-ups (already in
|
| Item | Fix |
|---|---|
| Duplicate Run configuration | --openapi-ui runtime keys use addOverrideProperty only; camel.jbang.openapiUi persisted via writeSetting |
| Swagger UI version coupling | Version read from META-INF/maven/org.webjars/swagger-ui/pom.properties; Maven uses ${swagger-ui-version} |
| KameletMain REST overrides | camel.rest.component / camel.rest.apiContextPath applied only when not already configured |
| HTML defense-in-depth | OpenApiUiSupport.escapeForJavaScriptString() before embedding specPath |
| JBang metadata security | PrepareCamelJBangMojo emits security from @Metadata |
Explicit --management-port |
shouldPreserveExplicitManagementPortForOpenApiUi test added |
| Upgrade guide | 4.22 entry added |
Tests (all passed locally)
mvn test -pl components/camel-platform-http-main \
-Dtest=OpenApiUiHttpServerTest,OpenApiUiRestMainTest,OpenApiUiSupportTest
mvn test -pl dsl/camel-jbang/camel-jbang-core -Dtest=RunOpenApiUiOptionsTestThanks for the approval — ready for merge once CI is green.
gnodet
reviewed
Aug 2, 2026
gnodet
left a comment
gnodet
left a comment
Contributor
There was a problem hiding this comment.
Well-implemented feature adding Swagger UI for REST DSL OpenAPI to camel-jbang. Architecture follows established patterns, security is properly gated with insecure:dev, version management was improved to runtime resolution per prior review feedback, and test coverage across four test classes is solid. Already has committer approval with CI green.
Two observations:
-
Dependency scope — The swagger-ui webjar in
camel-platform-http-main/pom.xmluses default compile scope without<optional>true</optional>. Since this module is published in the Camel BOM, the ~4MB webjar will be transitively pulled into all downstream consumers even when the OpenAPI UI feature is not enabled. The feature is guarded at runtime byopenapiUiEnabled(default false), so marking the dependency as optional would work correctly — JBang already adds the dependency explicitly. -
Override behavior — When
--openapi-uiis set,Run.javaunconditionally callsaddOverrideProperty("camel.rest.component", "platform-http"). This will silently replace any user-configuredcamel.rest.component(e.g.,netty-http). While this follows the same pattern used elsewhere inRun.javaand is arguably intentional for the feature, documenting the override behavior in the upgrade guide entry would prevent surprises.
Positive notes:
- The PR has received a thorough two-round review from a committer with all initial findings addressed.
- The runtime version resolution from
pom.propertiesis a good improvement over the initial hardcoded constant. - Test quality is comprehensive: unit tests, HTTP server integration, full REST+Main integration, and CLI option parsing.
This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.
Claude Code on behalf of @gnodet
Address gnodet follow-up: swagger-ui webjar is optional in camel-platform-http-main, upgrade guide notes that --openapi-ui sets override properties for REST component/api path, and add test coverage for applyOpenApiUiRuntimeOptions(). Co-authored-by: Cursor <cursoragent@cursor.com>
Contributor
Author
|
AI-generated comment on behalf of atiaomar1978-hub Review follow-up (
|
gnodet
approved these changes
Aug 2, 2026
gnodet
left a comment
gnodet
left a comment
Contributor
There was a problem hiding this comment.
Re-review after latest commit (129a83f5). All prior feedback from three review rounds has been addressed:
- ✅ Swagger-ui webjar dependency marked
<optional>true</optional> - ✅ Upgrade guide expanded with REST override behavior documentation
- ✅
shouldApplyOpenApiUiOverridePropertiestest added - ✅ Earlier fixes: duplicate config writes, runtime swagger-ui version resolution from pom.properties, REST config override guarded via
isConfigured(), specPath JavaScript escaping, security annotation in JBang metadata, management-port preservation test
The implementation is solid with proper security gating, comprehensive tests, and complete documentation. The latest changes are minimal and low-risk (dependency scope, doc text, and one test).
Minor notes (informational, not blocking):
- The
isConfigured()check inKameletMaincovers override and initial properties but not user properties fromapplication.properties— acceptable for developer tooling. SWAGGER_UI_WEBJAR_VERSIONtriggersresolveSwaggerUiWebjarVersion()at class-load time, butsetup()is properly guarded byopenapiUiEnabled, so no unnecessary initialization occurs.
This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.
Claude Code on behalf of @gnodet
davsclaus
reviewed
Aug 3, 2026
| <properties> | ||
| <firstVersion>4.0.0</firstVersion> | ||
| <label>http</label> | ||
| <swagger-ui-version>5.21.0</swagger-ui-version> |
Contributor
There was a problem hiding this comment.
Move this to parent/pom.xml
davsclaus
requested changes
Aug 3, 2026
davsclaus
left a comment
davsclaus
left a comment
Contributor
There was a problem hiding this comment.
the version should be in parent/pom.xml
Contributor
|
@omarsmak here is a small change to do and also fix the merge conflict |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implements CAMEL-18552: Camel JBang can expose a Swagger UI for REST DSL OpenAPI (Hawtio-style developer tooling).
camel run … --openapi-ui(also ondev/debugviaRun).http://localhost:8080/q/openapi(bundledswagger-ui5.21.0 webjar on the embedded management server)./q/openapi.jsonvia REST DSL (camel.rest.apiContextPath).camel-platform-http-main+camel-openapi-java, enables HTTP/management servers, and aligns management port with--portso the UI and spec stay on the same listener (unless--management-portis set explicitly).camel.management.openapiUiEnabled=true(insecure:dev).Design
/q/openapi/q/openapi/webjars/*)/q/openapi.jsonFor split deployments,
camel.management.openapiUiSpecPathaccepts an absolutehttp(s)://…URL (not rewritten with a leading/).Review follow-ups (Bugbot + Grok-style review)
--portprepareOpenApiUiServerOptions()co-binds ports--openapi-uiafter server settings blockcamel.server.*writes; explicitcamel.server.enabled/camel.management.enabledin RunOpenApiUiSupport.normalizeSpecPath()camel.jbang.openapiUisecurity metadatasecurity = insecure:devon@MetadataTests
OpenApiUiHttpServerTest,OpenApiUiRestMainTest,OpenApiUiSupportTestRunOpenApiUiOptionsTest(CLI + port alignment)Test plan
camel run <rest.java> --openapi-ui --port=9090→ UI + JSON on 9090/q/openapiAI-generated PR description on behalf of atiaomar1978-hub