CAMEL-16809: camel-aws2-s3 - support ACL grant headers on putObject and multipart upload - #25565
CAMEL-16809: camel-aws2-s3 - support ACL grant headers on putObject and multipart upload#25565oscerd wants to merge 1 commit into
Conversation
…nd multipart upload Adds the CamelAwsS3GrantFullControl, CamelAwsS3GrantRead, CamelAwsS3GrantReadACP and CamelAwsS3GrantWriteACP producer headers, wired to the grantFullControl / grantRead / grantReadACP / grantWriteACP options of PutObjectRequest and CreateMultipartUploadRequest. This complements the existing canned-ACL support (CamelAwsS3CannedAcl / CamelAwsS3Acl) so a route can set explicit grantee-based ACLs when uploading objects. Co-Authored-By: Claude Opus 4.8 <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:
|
|
🧪 CI tested the following changed modules:
🔬 Scalpel shadow comparison — Scalpel: 12 tested, 27 compile-only — current: 10 all testedMaveniverse Scalpel detected 39 affected modules (current approach: 10).
|
Review (Bugbot + Grok + manual)AI-generated review. Verdict: Looks good to merge with minor follow-ups. CI is green; implementation matches existing BugbotNo bugs found in grant header wiring for SecurityNo medium-or-higher findings. Grant headers use the Test coverage gaps
Documentation
Minor (non-blocking)
Nice work on closing the AWS SDK grant-header gap for put/multipart upload paths. |
gnodet
left a comment
There was a problem hiding this comment.
Clean feature addition of four ACL grant headers to the AWS2 S3 producer. The implementation correctly follows existing patterns (header read, null check, set on builder) and the test follows the established Mockito approach in this module.
One observation:
The test (AWS2S3ProducerAclGrantTest) only covers the doPutObject code path (small String body, isMultiPartUpload() defaults to false). However, the processMultiPart method is also modified with the same four grant-header blocks but has no test coverage. Since the PR title explicitly claims multipart upload support, adding a test that exercises the multipart path (with isMultiPartUpload() set to true, capturing CreateMultipartUploadRequest) would ensure both modified paths are verified and guard against future divergence.
Otherwise the implementation looks good — header documentation is automatically handled via @Metadata annotations, and the code changes are clean and consistent.
This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.
Claude Code on behalf of @gnodet
gnodet
left a comment
There was a problem hiding this comment.
Clean, well-structured feature addition of four ACL grant headers (CamelAwsS3GrantFullControl, CamelAwsS3GrantRead, CamelAwsS3GrantReadAcp, CamelAwsS3GrantWriteAcp) to the AWS2 S3 producer. Implementation correctly follows existing patterns for header handling, @Metadata annotations are properly configured, and the putObject path is well-tested.
Main observation (medium):
The test only covers the doPutObject code path (small String body, isMultiPartUpload() defaults to false). However, processMultiPart was also modified with the same four grant-header blocks and has zero test coverage. Since the PR title explicitly claims multipart upload support, a test that sets isMultiPartUpload() to true and captures the CreateMultipartUploadRequest would verify both modified paths and guard against future divergence.
Informational notes (pre-existing, not introduced by this PR):
AWS2S3StreamUploadProducerreadsCANNED_ACLandACLheaders but does not read the four new grant headers, creating a functional inconsistency across upload paths. Worth tracking as a follow-up.- The grant-header blocks are duplicated verbatim between
doPutObjectandprocessMultiPart, following the pre-existing pattern for all other headers in this class. A future refactoring could extract a helper method.
This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.
Claude Code on behalf of @gnodet
What
Adds support for explicit ACL grant headers on the aws2-s3 producer, complementing the existing canned-ACL support (
CamelAwsS3CannedAcl/CamelAwsS3Acl).Four new producer headers map to the S3
grant*request options onPutObjectandCreateMultipartUpload:CamelAwsS3GrantFullControlgrantFullControlCamelAwsS3GrantReadgrantReadCamelAwsS3GrantReadACPgrantReadACPCamelAwsS3GrantWriteACPgrantWriteACPEach value is an S3 grantee expression (
id=...,emailAddress=...oruri=...). This lets a route set explicit grantee-based ACLs when uploading objects — as requested in the issue (referencing aws/aws-sdk-java-v2#1703) — on both the singleputObjectand the multipart-upload paths.Testing
AWS2S3ProducerAclGrantTest(Mockito + AssertJ): verifies all four grant headers are applied to the capturedPutObjectRequest, and that the grant fields staynullwhen the headers are absent.mvn clean install -DskipTestspasses; catalog and endpoint-DSL regenerated and committed.Claude Code on behalf of oscerd