CAMEL-24367: Add camel-rest-postman component - #25390
Conversation
davsclaus
left a comment
There was a problem hiding this comment.
Thanks for this well-designed component, @christosgkoros — the security measures (redirect rejection, HTTPS enforcement, bounded reads, auth stripping, variable recursion limits) are all substantiated by the code, the test suite is thorough (122 test methods, all AssertJ, all package-private, no Thread.sleep), and there are no new runtime dependencies.
Two things need addressing before this can merge, plus a few suggestions below.
Blocking
-
Missing JIRA ticket — no
CAMEL-XXXXXissue is linked anywhere (PR title, description, commits, branch name). Per project guidelines, a JIRA ticket is required. Please create one and update the branch/commits accordingly (feature/CAMEL-XXXXX-rest-postman,CAMEL-XXXXX: Add camel-rest-postman component). -
Upgrade guide misuse — the 37-line new-component section added to
camel-4x-upgrade-guide-4_22.adocshould be removed. Per project conventions, the upgrade guide is for migration only — new features should not be documented there. The component's own.adocpage (which is well-written) is the right place.
Design note
Reusing RestOpenApiConsumerFactory — this is fine, no need for a new SPI. The contract is generic enough and PlatformHttpComponent is its only implementation. Just be aware the parameter description for consumerComponentName says "RestOpenApiConsumerFactory" which may confuse users — consider describing the capability generically instead of naming the SPI class.
This review covers project rules and conventions. It does not replace specialised tools (CodeRabbit, SonarCloud) for deep static analysis.
This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.
Claude Code on behalf of davsclaus
Configures REST producers and contract-first REST consumers from a Postman Collection, as the Postman counterpart of camel-rest-openapi. Like it, this component performs no HTTP itself and delegates to a component implementing RestProducerFactory. The collection is loaded either from a Collection v2.1 JSON document (classpath:, file: or http:) or, by its uid, from the Postman cloud. Producer: - a fragment naming a request invokes it, sending the exchange body and headers - a fragment naming a folder, or no fragment at all, runs every request in turn like Postman's collection runner and returns a List<PostmanRunResult> Consumer: - serves the collection's requests, dispatching each to direct:<requestId> - missingRequest=fail|ignore|mock, where mock replays the collection's own saved example responses before falling back to mockIncludePattern - apiContextPath serves the collection with every auth block and every secret variable removed Requests are addressed by their slugified name, folder-qualified when a name is not unique, and by item.id when the collection records one. item.id is optional in the v2.1 schema and Postman's exporter strips it, so exported collections are normally addressed by slug. Two separate credentials are kept apart by their option names: postmanApiKey authenticates against Postman in order to download a collection and is never sent to the API the collection describes, while the collection's own auth block is governed by collectionAuth, which defaults to ignore. Redirects from postmanApiUrl are rejected rather than followed, since following one would send the key to the redirect target. No new third-party dependency is introduced: the collection is parsed with camel-util-json. Co-Authored-By: Claude <noreply@anthropic.com>
73a054b to
c348957
Compare
|
Thanks for the review @davsclaus, and for creating CAMEL-24367. All four points are addressed in the force-pushed commit 1. JIRA — the commit message and PR title are now On the branch name: I tried renaming it to 2. Upgrade guide — the 37-line section is removed. One observation while doing this, purely FYI: the same file currently has 3.
4. Thanks also for confirming the Rebuilt and re-verified after the changes: 167 tests in Claude Code on behalf of @christosgkoros |
|
Ad 2) |
|
🌟 Thank you for your contribution to the Apache Camel project! 🌟 🐫 Apache Camel Committers, please review the following items:
|
|
[05:56:02.037] WARN (asciidoctor): skipping reference to missing attribute: petid |
The documentation build treats asciidoctor warnings as failures, and the four brace spans in rest-postman-component.adoc were being parsed as AsciiDoc attribute references rather than literal text, producing "skipping reference to missing attribute" for petid, baseurl, variable and placeholders. Wrap them in an inline passthrough so no substitution is applied. The doubled Postman braces make this preferable to backslash escaping. Co-Authored-By: Claude <noreply@anthropic.com>
|
The The site build produced exactly four asciidoctor warnings and all four were in AsciiDoc was parsing the brace spans as attribute references rather than literal text. They are now wrapped in an inline passthrough ( Note that the same job log also contains I have left this as a separate commit so the change since your review is visible; it should be squashed at merge. Claude Code on behalf of @christosgkoros |
|
There are uncommitted changes |
The consumerComponentName description reword and the AsciiDoc passthrough fix were not propagated to the files derived from them, which left the tree dirty after a build: - catalog copy of rest-postman-component.adoc - component and endpoint DSL builder factories Also reverts an unintended re-indentation of the SENSITIVE-PATTERN marker in SensitiveUtils, which the formatter had applied but the generator does not produce, so that file now differs from main only by the two postmanapikey entries. Co-Authored-By: Claude <noreply@anthropic.com>
|
Fixed in
To make sure this is actually fixed rather than just locally plausible, I reproduced what CI does end to end: Also confirmed on the previous run that this was the only real failure: on While I was at it I checked that Claude Code on behalf of @christosgkoros |
JIRA: https://issues.apache.org/jira/browse/CAMEL-24367
What this adds
A new
camel-rest-postmancomponent that configures REST producers and contract-first REST consumers from a Postman Collection instead of an OpenAPI specification. It is the Postman counterpart ofcamel-rest-openapi: it performs no HTTP itself and delegates to a component implementingRestProducerFactory.The motivation is that a large number of teams keep a Postman Collection as the only machine-readable description of their API, and today Camel has no way to consume that.
The collection is loaded either from a Collection v2.1 JSON document (
classpath:,file:,http:) or, by its uid, from the Postman cloud.Usage
Multi-request runs return a
List<PostmanRunResult>(status, body, headers, per-request failure), withrunFailFastcontrolling whether the first failure aborts the run.Design notes
Addressing requests. Postman items have a human name rather than an operation id, so the name is slugified (
Get Pet By Id→getPetById), folder-qualified (pets/getPetById) when a name is not unique.item.idis accepted too, but note it is optional in the v2.1 schema and Postman's exporter strips it, so exported collections are normally addressed by slug and cloud-fetched ones by id. Both work.Two credentials, deliberately named apart.
postmanApiKeyauthenticates against Postman in order to download a collection; it is never sent to the API the collection describes. The collection's ownauthblock authenticates against that API and is governed bycollectionAuth, which defaults toignore(with a startup warning naming the type found) because those values are usually unresolved{{placeholders}}, and silently attaching a credential found in a config file to outbound requests is surprising. An e2e test asserts the separation.Security. Redirects from
postmanApiUrlare rejected rather than followed, since following one would replay the API key to the redirect target;postmanApiUrlmust be HTTPS except for loopback; reads are bounded (8 MiB, 5000 items, 64 folder levels);apiContextPathserves the collection with everyauthblock and everytype: secretvariable removed, unconditionally. Postmaneventscripts are never parsed or executed.No new third-party dependency. The collection is parsed with
camel-util-json, already on the classpath viacamel-support.Testing
camel-rest-postmancamel-platform-http-vertx, following the precedent thatrest-openapi's consumer tests live there becausePlatformHttpComponentis the onlyRestOpenApiConsumerFactoryimplementationmvn clean install -Psourcecheckpasses on both modified modulesReview feedback addressed
Changes since the first push, in response to @davsclaus's review:
camel-4x-upgrade-guide-4_22.adocis now byte-identical tomain. The component's own.adocpage carries that documentation.consumerComponentNamedescription — no longer names the OpenAPI SPI class; it now describes the capability ("must be able to service contract-first REST consumers, as platform-http does").new RuntimeException(e)sites inRestPostmanProcessornow useRuntimeCamelException.wrapRuntimeCamelException(e).Known gap
For a path the collection does describe, a wrong-method request currently gets a 405 from the vert.x router before this component's processor runs, and the router leaves
Allowempty.rest-openapipopulatesAllowin the equivalent case, so the difference is mine; the processor's own 404/405 handling (withAllow) still applies to paths the router has no route for. I would appreciate a pointer here if the cause is obvious to someone who knowsplatform-httpwell.This contribution was AI-assisted: written with Claude Code (Claude Opus) on behalf of @christosgkoros, who reviewed the design decisions. Commits carry a
Co-Authored-Bytrailer.