CAMEL-24203: camel-aws-security-hub - implement the declared getFindingAggregator operation#24940
CAMEL-24203: camel-aws-security-hub - implement the declared getFindingAggregator operation#24940oscerd wants to merge 1 commit into
Conversation
…ngAggregator operation SecurityHubOperations declared getFindingAggregator but SecurityHubProducer had no case for it, so selecting that operation always failed with "Unsupported operation" even though the catalog advertised it. Implements the operation with the existing executeOperation helper (POJO GetFindingAggregatorRequest plus a header-driven path) and adds the CamelAwsSecurityHubFindingAggregatorArn header it requires. Adds SecurityHubGetFindingAggregatorTest asserting the operation runs and passes the ARN header through to the request. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Andrea Cosentino <ancosen@gmail.com>
|
🌟 Thank you for your contribution to the Apache Camel project! 🌟 🐫 Apache Camel Committers, please review the following items:
|
gnodet
left a comment
There was a problem hiding this comment.
Clean fix — the getFindingAggregator enum value was declared in SecurityHubOperations but had no case in the producer's switch, so the operation always threw IllegalArgumentException. This PR implements it properly.
What I checked:
- Pattern consistency: the new
getFindingAggregatormethod follows the exact sameexecuteOperationhelper pattern asgetFindingHistoryand other operations — POJO body support plus a header-driven fallback path. - Header constant:
FINDING_AGGREGATOR_ARNfollows the established naming convention (CamelAwsSecurityHub*) and is correctly annotated with@Metadata. - Validation: the ARN is validated as non-empty before use, consistent with how other required headers are validated (e.g.,
getFindingHistoryvalidatesfindingIdandproductArn). - Test:
SecurityHubGetFindingAggregatorTestuses a Mockito mock forSecurityHubClient, verifies the ARN header is correctly propagated to theGetFindingAggregatorRequestviaArgumentCaptor. Good coverage for the fix. - Generated files: catalog JSON and endpoint DSL factory correctly include the new header at index 21.
- API compatibility: implementing the existing enum value (rather than removing it) is the right call — removing a published operation would be a breaking change.
LGTM ✅
Claude Code on behalf of gnodet — AI-generated review
davsclaus
left a comment
There was a problem hiding this comment.
Clean fix for a declared-but-unimplemented operation. The getFindingAggregator enum value has existed since the component was introduced (CAMEL-22919), but the producer switch had no case for it — this PR fills that gap.
What looks good:
- Implementation follows the exact same
executeOperationhelper pattern used by all other operations inSecurityHubProducer - New
FINDING_AGGREGATOR_ARNheader constant properly annotated with@Metadata - Test uses Mockito to mock the
SecurityHubClient, drives the operation through a Camel route, and asserts the ARN header reaches the AWS request — effectively proving the fix - Catalog, component JSON, and endpoint-DSL metadata regenerated correctly
mockito-junit-jupitertest dependency already used by several othercamel-awsmodules
LGTM.
This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.
|
🧪 CI tested the following changed modules:
🔬 Scalpel shadow comparison — Scalpel: 11 tested, 27 compile-only — current: 9 all testedMaveniverse Scalpel detected 38 affected modules (current approach: 9).
|
Description
Fixes CAMEL-24203.
SecurityHubOperationsdeclaresgetFindingAggregator, butSecurityHubProducer.processhad nocasefor it, so the switch fell through to:Selecting
operation=getFindingAggregatortherefore always failed, even though the enum and catalog advertise it.Fix
executeOperationhelper — POJOGetFindingAggregatorRequestsupport plus a header-driven path.CamelAwsSecurityHubFindingAggregatorArnheader constant the AWSGetFindingAggregatorAPI requires (it is mandatory, so the header path fails fast with a clear message when absent).Chose to implement rather than drop the enum value, since removing a published operation would be a breaking API change.
Tests
Adds
SecurityHubGetFindingAggregatorTest— drives the operation through a route with a mockedSecurityHubClientand asserts the ARN header reaches the request. Against the old code the exchange fails with "Unsupported operation".Regenerated component/catalog/endpoint-DSL metadata for the new header is included.
Backport
Same code on
camel-4.18.xandcamel-4.14.x; will be backported after merge (fixVersions 4.22.0 / 4.18.4 / 4.14.9).Claude Code on behalf of oscerd