You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by "turcsanyip (via GitHub)" <gi...@apache.org> on 2025/10/23 16:38:08 UTC
[PR] NIFI-13192 Upgraded AWS S3 Processors to use AWS 2.x libraries [nifi]
turcsanyip opened a new pull request, #10460:
URL: https://github.com/apache/nifi/pull/10460
# Summary
[NIFI-13192](https://issues.apache.org/jira/browse/NIFI-13192)
# Tracking
Please complete the following tracking steps prior to pull request creation.
### Issue Tracking
- [ ] [Apache NiFi Jira](https://issues.apache.org/jira/browse/NIFI) issue created
### Pull Request Tracking
- [ ] Pull Request title starts with Apache NiFi Jira issue number, such as `NIFI-00000`
- [ ] Pull Request commit message starts with Apache NiFi Jira issue number, as such `NIFI-00000`
### Pull Request Formatting
- [ ] Pull Request based on current revision of the `main` branch
- [ ] Pull Request refers to a feature branch with one commit containing changes
# Verification
Please indicate the verification steps performed prior to pull request creation.
### Build
- [ ] Build completed using `./mvnw clean install -P contrib-check`
- [ ] JDK 21
- [ ] JDK 25
### Licensing
- [ ] New dependencies are compatible with the [Apache License 2.0](https://apache.org/licenses/LICENSE-2.0) according to the [License Policy](https://www.apache.org/legal/resolved.html)
- [ ] New dependencies are documented in applicable `LICENSE` and `NOTICE` files
### Documentation
- [ ] Documentation formatting appears as expected in rendered files
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NIFI-13192 Upgraded AWS S3 Processors to use AWS 2.x libraries [nifi]
Posted by "sfc-gh-jwitt (via GitHub)" <gi...@apache.org>.
sfc-gh-jwitt commented on PR #10460:
URL: https://github.com/apache/nifi/pull/10460#issuecomment-3438083479
AWESOME to see this @turcsanyip ! Thank you
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NIFI-13192 Upgraded AWS S3 Processors to use AWS 2.x libraries [nifi]
Posted by "pvillard31 (via GitHub)" <gi...@apache.org>.
pvillard31 commented on code in PR #10460:
URL: https://github.com/apache/nifi/pull/10460#discussion_r2461299356
##########
nifi-extension-bundles/nifi-aws-bundle/nifi-aws-abstract-processors/src/main/java/org/apache/nifi/processors/aws/s3/AbstractS3Processor.java:
##########
@@ -207,230 +147,120 @@ public abstract class AbstractS3Processor extends AbstractAWSCredentialsProvider
.defaultValue("false")
.build();
- /**
- * Create client using credentials provider. This is the preferred way for creating clients
- */
- @Override
- protected AmazonS3Client createClient(final ProcessContext context, final AWSCredentialsProvider credentialsProvider, final Region region,
- final ClientConfiguration config, final AwsClientBuilder.EndpointConfiguration endpointConfiguration) {
- getLogger().info("Creating client with credentials provider");
- initializeSignerOverride(context, config);
- AmazonS3EncryptionService encryptionService = context.getProperty(ENCRYPTION_SERVICE).asControllerService(AmazonS3EncryptionService.class);
-
- final Consumer<AmazonS3Builder<?, ?>> clientBuilder = builder -> {
- if (endpointConfiguration == null) {
- builder.withRegion(region.getName());
- } else {
- builder.withEndpointConfiguration(endpointConfiguration);
- }
- builder.withClientConfiguration(config);
- builder.withCredentials(credentialsProvider);
-
- final Boolean useChunkedEncoding = context.getProperty(USE_CHUNKED_ENCODING).asBoolean();
- if (useChunkedEncoding == Boolean.FALSE) {
- builder.disableChunkedEncoding();
- }
-
- final Boolean usePathStyleAccess = context.getProperty(USE_PATH_STYLE_ACCESS).asBoolean();
- final boolean endpointOverrideSet = !StringUtils.trimToEmpty(context.getProperty(ENDPOINT_OVERRIDE).evaluateAttributeExpressions().getValue()).isEmpty();
- if (usePathStyleAccess == Boolean.TRUE || endpointOverrideSet) {
- builder.withPathStyleAccessEnabled(true);
- }
- };
-
- AmazonS3 s3Client = null;
- if (encryptionService != null) {
- s3Client = encryptionService.createEncryptionClient(clientBuilder);
- }
- if (s3Client == null) {
- final AmazonS3ClientBuilder builder = AmazonS3Client.builder();
- clientBuilder.accept(builder);
- s3Client = builder.build();
- }
-
- return (AmazonS3Client) s3Client;
- }
-
+ // maps AWS SDK v1 CannedAccessControlList to v2 ObjectCannedACL
+ private static final Map<String, ObjectCannedACL> CANNED_ACL_MAPPING = Map.of(
+ "Private", ObjectCannedACL.PRIVATE,
+ "PublicRead", ObjectCannedACL.PUBLIC_READ,
+ "PublicReadWrite", ObjectCannedACL.PUBLIC_READ_WRITE,
+ "AuthenticatedRead", ObjectCannedACL.AUTHENTICATED_READ,
+ "LogDeliveryWrite", ObjectCannedACL.UNKNOWN_TO_SDK_VERSION,
Review Comment:
When `CANNED_ACL` is set to `LogDeliveryWrite`, we now map it to `ObjectCannedACL.UNKNOWN_TO_SDK_VERSION`. In the SDK v2 generated models, that would serialize to an empty value, so the `x-amz-acl` header would never be sent I believe.
Previously we sent `LogDeliveryWrite`, which is required when configuring the bucket to receive S3 server access logs. With this change, the request reverts to the default (effectively private) and S3 would reject the logging configuration.
We need to send log-delivery-write explicitly instead of `UNKNOWN_TO_SDK_VERSION`.
```suggestion
"LogDeliveryWrite", ObjectCannedACL.LOG_DELIVERY_WRITE,
```
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NIFI-13192 Upgraded AWS S3 Processors to use AWS 2.x libraries [nifi]
Posted by "pvillard31 (via GitHub)" <gi...@apache.org>.
pvillard31 commented on code in PR #10460:
URL: https://github.com/apache/nifi/pull/10460#discussion_r2463750533
##########
nifi-extension-bundles/nifi-aws-bundle/nifi-aws-abstract-processors/src/main/java/org/apache/nifi/processors/aws/s3/AbstractS3Processor.java:
##########
@@ -207,230 +147,120 @@ public abstract class AbstractS3Processor extends AbstractAWSCredentialsProvider
.defaultValue("false")
.build();
- /**
- * Create client using credentials provider. This is the preferred way for creating clients
- */
- @Override
- protected AmazonS3Client createClient(final ProcessContext context, final AWSCredentialsProvider credentialsProvider, final Region region,
- final ClientConfiguration config, final AwsClientBuilder.EndpointConfiguration endpointConfiguration) {
- getLogger().info("Creating client with credentials provider");
- initializeSignerOverride(context, config);
- AmazonS3EncryptionService encryptionService = context.getProperty(ENCRYPTION_SERVICE).asControllerService(AmazonS3EncryptionService.class);
-
- final Consumer<AmazonS3Builder<?, ?>> clientBuilder = builder -> {
- if (endpointConfiguration == null) {
- builder.withRegion(region.getName());
- } else {
- builder.withEndpointConfiguration(endpointConfiguration);
- }
- builder.withClientConfiguration(config);
- builder.withCredentials(credentialsProvider);
-
- final Boolean useChunkedEncoding = context.getProperty(USE_CHUNKED_ENCODING).asBoolean();
- if (useChunkedEncoding == Boolean.FALSE) {
- builder.disableChunkedEncoding();
- }
-
- final Boolean usePathStyleAccess = context.getProperty(USE_PATH_STYLE_ACCESS).asBoolean();
- final boolean endpointOverrideSet = !StringUtils.trimToEmpty(context.getProperty(ENDPOINT_OVERRIDE).evaluateAttributeExpressions().getValue()).isEmpty();
- if (usePathStyleAccess == Boolean.TRUE || endpointOverrideSet) {
- builder.withPathStyleAccessEnabled(true);
- }
- };
-
- AmazonS3 s3Client = null;
- if (encryptionService != null) {
- s3Client = encryptionService.createEncryptionClient(clientBuilder);
- }
- if (s3Client == null) {
- final AmazonS3ClientBuilder builder = AmazonS3Client.builder();
- clientBuilder.accept(builder);
- s3Client = builder.build();
- }
-
- return (AmazonS3Client) s3Client;
- }
-
+ // maps AWS SDK v1 CannedAccessControlList to v2 ObjectCannedACL
+ private static final Map<String, ObjectCannedACL> CANNED_ACL_MAPPING = Map.of(
+ "Private", ObjectCannedACL.PRIVATE,
+ "PublicRead", ObjectCannedACL.PUBLIC_READ,
+ "PublicReadWrite", ObjectCannedACL.PUBLIC_READ_WRITE,
+ "AuthenticatedRead", ObjectCannedACL.AUTHENTICATED_READ,
+ "LogDeliveryWrite", ObjectCannedACL.UNKNOWN_TO_SDK_VERSION,
Review Comment:
OK, I removed my previous comment on this line because I didn't look carefully enough the first time. I made some additional testing, and having this would break the flow for users where the property `Canned ACL` would be set to `LogDeliveryWrite`.
If we leave the mapping as `ObjectCannedACL.UNKNOWN_TO_SDK_VERSION`, the v2 builder serializes that enum by calling `toString()`. Because `UNKNOWN_TO_SDK_VERSION` holds a `null` literal, the resulting header becomes `x-amz-acl: null`. AWS treats that as an invalid canned ACL value, so the upload/copy call fails with a 400 response. Any existing NiFi flow that currently sets the property to `LogDeliveryWrite` (and has been working with the v1 client) would start erroring on every request, like the below
```
PutS3Object[id=d8070466-ec04-33b9-1413-166da0aabeab] Failed to put
FlowFile[filename=cdca61ce-a65b-4dc1-8f60-8f6f6e631edf] to Amazon S3 due
to software.amazon.awssdk.services.s3.model.S3Exception: (Service: S3, Status
Code: 400, Request ID: Q5BWX1WG7X2EZQ9Y, Extended Request ID: ...)
(SDK Attempt Count: 1): {}
```
I see two options:
- we are ok with this breaking change and we document it - I have no idea how frequently this feature is used and I'd assume this is not used that much if it's removed in v2
- we change the code to have a `Map<String, String>` instead of `Map<String, ObjectCannedACL>` and we would have something like:
```java
// maps AWS SDK v1 CannedAccessControlList names to the string value expected by the S3 v2 client
private static final Map<String, String> CANNED_ACL_MAPPING = Map.of(
"Private", ObjectCannedACL.PRIVATE.toString(),
"PublicRead", ObjectCannedACL.PUBLIC_READ.toString(),
"PublicReadWrite", ObjectCannedACL.PUBLIC_READ_WRITE.toString(),
"AuthenticatedRead", ObjectCannedACL.AUTHENTICATED_READ.toString(),
"LogDeliveryWrite", "log-delivery-write",
"BucketOwnerRead", ObjectCannedACL.BUCKET_OWNER_READ.toString(),
"BucketOwnerFullControl", ObjectCannedACL.BUCKET_OWNER_FULL_CONTROL.toString(),
"AwsExecRead", ObjectCannedACL.AWS_EXEC_READ.toString());
```
The required associated changes are minimal so this is also a valid option if we want to retain backward compatibility.
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NIFI-13192 Upgraded AWS S3 Processors to use AWS 2.x libraries [nifi]
Posted by "pvillard31 (via GitHub)" <gi...@apache.org>.
pvillard31 commented on code in PR #10460:
URL: https://github.com/apache/nifi/pull/10460#discussion_r2463877948
##########
nifi-extension-bundles/nifi-aws-bundle/nifi-aws-abstract-processors/src/main/java/org/apache/nifi/processors/aws/s3/AbstractS3Processor.java:
##########
@@ -207,230 +147,120 @@ public abstract class AbstractS3Processor extends AbstractAWSCredentialsProvider
.defaultValue("false")
.build();
- /**
- * Create client using credentials provider. This is the preferred way for creating clients
- */
- @Override
- protected AmazonS3Client createClient(final ProcessContext context, final AWSCredentialsProvider credentialsProvider, final Region region,
- final ClientConfiguration config, final AwsClientBuilder.EndpointConfiguration endpointConfiguration) {
- getLogger().info("Creating client with credentials provider");
- initializeSignerOverride(context, config);
- AmazonS3EncryptionService encryptionService = context.getProperty(ENCRYPTION_SERVICE).asControllerService(AmazonS3EncryptionService.class);
-
- final Consumer<AmazonS3Builder<?, ?>> clientBuilder = builder -> {
- if (endpointConfiguration == null) {
- builder.withRegion(region.getName());
- } else {
- builder.withEndpointConfiguration(endpointConfiguration);
- }
- builder.withClientConfiguration(config);
- builder.withCredentials(credentialsProvider);
-
- final Boolean useChunkedEncoding = context.getProperty(USE_CHUNKED_ENCODING).asBoolean();
- if (useChunkedEncoding == Boolean.FALSE) {
- builder.disableChunkedEncoding();
- }
-
- final Boolean usePathStyleAccess = context.getProperty(USE_PATH_STYLE_ACCESS).asBoolean();
- final boolean endpointOverrideSet = !StringUtils.trimToEmpty(context.getProperty(ENDPOINT_OVERRIDE).evaluateAttributeExpressions().getValue()).isEmpty();
- if (usePathStyleAccess == Boolean.TRUE || endpointOverrideSet) {
- builder.withPathStyleAccessEnabled(true);
- }
- };
-
- AmazonS3 s3Client = null;
- if (encryptionService != null) {
- s3Client = encryptionService.createEncryptionClient(clientBuilder);
- }
- if (s3Client == null) {
- final AmazonS3ClientBuilder builder = AmazonS3Client.builder();
- clientBuilder.accept(builder);
- s3Client = builder.build();
- }
-
- return (AmazonS3Client) s3Client;
- }
-
+ // maps AWS SDK v1 CannedAccessControlList to v2 ObjectCannedACL
+ private static final Map<String, ObjectCannedACL> CANNED_ACL_MAPPING = Map.of(
+ "Private", ObjectCannedACL.PRIVATE,
+ "PublicRead", ObjectCannedACL.PUBLIC_READ,
+ "PublicReadWrite", ObjectCannedACL.PUBLIC_READ_WRITE,
+ "AuthenticatedRead", ObjectCannedACL.AUTHENTICATED_READ,
+ "LogDeliveryWrite", ObjectCannedACL.UNKNOWN_TO_SDK_VERSION,
Review Comment:
Yeah I agree with you, I think this is the best approach
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NIFI-13192 Upgraded AWS S3 Processors to use AWS 2.x libraries [nifi]
Posted by "turcsanyip (via GitHub)" <gi...@apache.org>.
turcsanyip commented on PR #10460:
URL: https://github.com/apache/nifi/pull/10460#issuecomment-3451065980
> Reviewing the v1 code you removed in `AssumeRoleCredentialsStrategy`, I noticed that we may not be using `MAX_SESSION_TIME` correctly.
>
> Instead of
>
> ```java
> builder.refreshRequest(roleRequestBuilder
> .build())
> .stsClient(stsClient)
> .staleTime(Duration.ofSeconds(maxSessionTime));
> ```
>
> I think we want
>
> ```java
> builder.refreshRequest(roleRequestBuilder
> .durationSeconds(maxSessionTime)
> .build())
> .stsClient(stsClient)
> .staleTime(Duration.ofSeconds(maxSessionTime));
> ```
I also removed `staleTime()`. According to its [javadoc](https://github.com/aws/aws-sdk-java-v2/blob/85f098a0232a7cabf6a75986c9e638216765bcc6/services/sts/src/main/java/software/amazon/awssdk/services/sts/auth/StsCredentialsProvider.java#L118-L124):
> _Configure the amount of time, relative to STS token expiration, that the cached credentials are considered stale and must be updated._
So if it is set to the same duration as the session time, then the credentials would be considered expired immediately. The default 5 minutes before session expiration seems to fine so no explicit configuration is necessary.
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NIFI-13192 Upgraded AWS S3 Processors to use AWS 2.x libraries [nifi]
Posted by "pvillard31 (via GitHub)" <gi...@apache.org>.
pvillard31 commented on code in PR #10460:
URL: https://github.com/apache/nifi/pull/10460#discussion_r2463755401
##########
nifi-extension-bundles/nifi-aws-bundle/nifi-aws-processors/src/main/java/org/apache/nifi/processors/aws/s3/CopyS3Object.java:
##########
@@ -173,63 +184,73 @@ public void onTrigger(final ProcessContext context, final ProcessSession session
* Sections of this code were derived from example code from the official AWS S3 documentation. Specifically this example:
* https://github.com/awsdocs/aws-doc-sdk-examples/blob/df606a664bf2f7cfe3abc76c187e024451d0279c/java/example_code/s3/src/main/java/aws/example/s3/LowLevelMultipartCopy.java
*/
- private void copyMultipart(final AmazonS3Client s3, final AccessControlList acl, final CannedAccessControlList cannedAccessControlList,
+ private void copyMultipart(final S3Client client, final ProcessContext context, final FlowFile flowFile,
final String sourceBucket, final String sourceKey,
final String destinationBucket, final String destinationKey, final AtomicReference<String> multipartIdRef,
final long contentLength) {
- InitiateMultipartUploadRequest initRequest = new InitiateMultipartUploadRequest(destinationBucket,
- destinationKey);
- if (acl != null) {
- initRequest.setAccessControlList(acl);
- }
- if (cannedAccessControlList != null) {
- initRequest.setCannedACL(cannedAccessControlList);
- }
+ final CreateMultipartUploadRequest createRequest = CreateMultipartUploadRequest.builder()
+ .bucket(destinationBucket)
+ .key(destinationKey)
+ .grantFullControl(getFullControlGranteeSpec(context, flowFile))
+ .grantRead(getReadGranteeSpec(context, flowFile))
+ .grantReadACP(getReadACPGranteeSpec(context, flowFile))
+ .grantWriteACP(getWriteACPGranteeSpec(context, flowFile))
+ .acl(createCannedACL(context, flowFile))
+ .build();
- final InitiateMultipartUploadResult initResult = s3.initiateMultipartUpload(initRequest);
+ final CreateMultipartUploadResponse createResponse = client.createMultipartUpload(createRequest);
- multipartIdRef.set(initResult.getUploadId());
+ multipartIdRef.set(createResponse.uploadId());
long bytePosition = 0;
int partNumber = 1;
- final List<CopyPartResult> copyPartResults = new ArrayList<>();
+ final List<UploadPartCopyResponse> copyResponses = new ArrayList<>();
while (bytePosition < contentLength) {
long lastByte = Math.min(bytePosition + MULTIPART_THRESHOLD - 1, contentLength - 1);
- final CopyPartRequest copyPartRequest = new CopyPartRequest()
- .withSourceBucketName(sourceBucket)
- .withSourceKey(sourceKey)
- .withDestinationBucketName(destinationBucket)
- .withDestinationKey(destinationKey)
- .withUploadId(initResult.getUploadId())
- .withFirstByte(bytePosition)
- .withLastByte(lastByte)
- .withPartNumber(partNumber++);
+ final UploadPartCopyRequest copyRequest = UploadPartCopyRequest.builder()
+ .sourceBucket(sourceBucket)
+ .sourceKey(sourceKey)
+ .destinationBucket(destinationBucket)
+ .destinationBucket(destinationKey)
Review Comment:
```suggestion
.destinationKey(destinationKey)
```
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NIFI-13192 Upgraded AWS S3 Processors to use AWS 2.x libraries [nifi]
Posted by "exceptionfactory (via GitHub)" <gi...@apache.org>.
exceptionfactory merged PR #10460:
URL: https://github.com/apache/nifi/pull/10460
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NIFI-13192 Upgraded AWS S3 Processors to use AWS 2.x libraries [nifi]
Posted by "turcsanyip (via GitHub)" <gi...@apache.org>.
turcsanyip commented on PR #10460:
URL: https://github.com/apache/nifi/pull/10460#issuecomment-3451032862
@pvillard31 Updated the PR based on your suggestions.
Thanks for bringing up the parallel multipart upload topic. It is definitely something we should investigate and implement if feasible. I would do it in a separate jira (created [NIFI-15144](https://issues.apache.org/jira/browse/NIFI-15144)) because it does not seem to be trivial to add the feature in the Put processor and also because I would focus on the backward compatibility and feature parity during the v2 upgrade.
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NIFI-13192 Upgraded AWS S3 Processors to use AWS 2.x libraries [nifi]
Posted by "pvillard31 (via GitHub)" <gi...@apache.org>.
pvillard31 commented on code in PR #10460:
URL: https://github.com/apache/nifi/pull/10460#discussion_r2461299356
##########
nifi-extension-bundles/nifi-aws-bundle/nifi-aws-abstract-processors/src/main/java/org/apache/nifi/processors/aws/s3/AbstractS3Processor.java:
##########
@@ -207,230 +147,120 @@ public abstract class AbstractS3Processor extends AbstractAWSCredentialsProvider
.defaultValue("false")
.build();
- /**
- * Create client using credentials provider. This is the preferred way for creating clients
- */
- @Override
- protected AmazonS3Client createClient(final ProcessContext context, final AWSCredentialsProvider credentialsProvider, final Region region,
- final ClientConfiguration config, final AwsClientBuilder.EndpointConfiguration endpointConfiguration) {
- getLogger().info("Creating client with credentials provider");
- initializeSignerOverride(context, config);
- AmazonS3EncryptionService encryptionService = context.getProperty(ENCRYPTION_SERVICE).asControllerService(AmazonS3EncryptionService.class);
-
- final Consumer<AmazonS3Builder<?, ?>> clientBuilder = builder -> {
- if (endpointConfiguration == null) {
- builder.withRegion(region.getName());
- } else {
- builder.withEndpointConfiguration(endpointConfiguration);
- }
- builder.withClientConfiguration(config);
- builder.withCredentials(credentialsProvider);
-
- final Boolean useChunkedEncoding = context.getProperty(USE_CHUNKED_ENCODING).asBoolean();
- if (useChunkedEncoding == Boolean.FALSE) {
- builder.disableChunkedEncoding();
- }
-
- final Boolean usePathStyleAccess = context.getProperty(USE_PATH_STYLE_ACCESS).asBoolean();
- final boolean endpointOverrideSet = !StringUtils.trimToEmpty(context.getProperty(ENDPOINT_OVERRIDE).evaluateAttributeExpressions().getValue()).isEmpty();
- if (usePathStyleAccess == Boolean.TRUE || endpointOverrideSet) {
- builder.withPathStyleAccessEnabled(true);
- }
- };
-
- AmazonS3 s3Client = null;
- if (encryptionService != null) {
- s3Client = encryptionService.createEncryptionClient(clientBuilder);
- }
- if (s3Client == null) {
- final AmazonS3ClientBuilder builder = AmazonS3Client.builder();
- clientBuilder.accept(builder);
- s3Client = builder.build();
- }
-
- return (AmazonS3Client) s3Client;
- }
-
+ // maps AWS SDK v1 CannedAccessControlList to v2 ObjectCannedACL
+ private static final Map<String, ObjectCannedACL> CANNED_ACL_MAPPING = Map.of(
+ "Private", ObjectCannedACL.PRIVATE,
+ "PublicRead", ObjectCannedACL.PUBLIC_READ,
+ "PublicReadWrite", ObjectCannedACL.PUBLIC_READ_WRITE,
+ "AuthenticatedRead", ObjectCannedACL.AUTHENTICATED_READ,
+ "LogDeliveryWrite", ObjectCannedACL.UNKNOWN_TO_SDK_VERSION,
Review Comment:
When `CANNED_ACL` is set to `LogDeliveryWrite`, we now map it to `ObjectCannedACL.UNKNOWN_TO_SDK_VERSION`. In the SDK v2 generated models, that would serialize to an empty value, so the `x-amz-acl` header would never be sent I believe.
Previously we sent `LogDeliveryWrite`, which is required when configuring the bucket to receive S3 server access logs. With this change, the request reverts to the default (effectively private) and S3 would reject the logging configuration.
We need to send log-delivery-write explicitly instead of `UNKNOWN_TO_SDK_VERSION`.
```suggestion
"LogDeliveryWrite", ObjectCannedACL.LOG_DELIVERY_WRITE,
```
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NIFI-13192 Upgraded AWS S3 Processors to use AWS 2.x libraries [nifi]
Posted by "pvillard31 (via GitHub)" <gi...@apache.org>.
pvillard31 commented on PR #10460:
URL: https://github.com/apache/nifi/pull/10460#issuecomment-3444229349
I'll do more testing but this looks great.
I just wanted to mention here something that we discussed over Slack in the past: the possibility to upload a large file to S3 using multithreading with multipart to achieve much better performances. I understand this is not in the scope of this PR and that's fine, but, for reference, a solid path would be to switch the multipart upload path over to the AWS SDK v2 `S3TransferManager`, which wraps the async S3 client and automatically fans out each part to a configurable thread pool, handling retries, checksums, and ordering... we’d build transfer requests from the existing processor context, submit them, and we would likely block on the returned `CompletedMultipartUpload` future.
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NIFI-13192 Upgraded AWS S3 Processors to use AWS 2.x libraries [nifi]
Posted by "turcsanyip (via GitHub)" <gi...@apache.org>.
turcsanyip commented on code in PR #10460:
URL: https://github.com/apache/nifi/pull/10460#discussion_r2463873044
##########
nifi-extension-bundles/nifi-aws-bundle/nifi-aws-abstract-processors/src/main/java/org/apache/nifi/processors/aws/s3/AbstractS3Processor.java:
##########
@@ -207,230 +147,120 @@ public abstract class AbstractS3Processor extends AbstractAWSCredentialsProvider
.defaultValue("false")
.build();
- /**
- * Create client using credentials provider. This is the preferred way for creating clients
- */
- @Override
- protected AmazonS3Client createClient(final ProcessContext context, final AWSCredentialsProvider credentialsProvider, final Region region,
- final ClientConfiguration config, final AwsClientBuilder.EndpointConfiguration endpointConfiguration) {
- getLogger().info("Creating client with credentials provider");
- initializeSignerOverride(context, config);
- AmazonS3EncryptionService encryptionService = context.getProperty(ENCRYPTION_SERVICE).asControllerService(AmazonS3EncryptionService.class);
-
- final Consumer<AmazonS3Builder<?, ?>> clientBuilder = builder -> {
- if (endpointConfiguration == null) {
- builder.withRegion(region.getName());
- } else {
- builder.withEndpointConfiguration(endpointConfiguration);
- }
- builder.withClientConfiguration(config);
- builder.withCredentials(credentialsProvider);
-
- final Boolean useChunkedEncoding = context.getProperty(USE_CHUNKED_ENCODING).asBoolean();
- if (useChunkedEncoding == Boolean.FALSE) {
- builder.disableChunkedEncoding();
- }
-
- final Boolean usePathStyleAccess = context.getProperty(USE_PATH_STYLE_ACCESS).asBoolean();
- final boolean endpointOverrideSet = !StringUtils.trimToEmpty(context.getProperty(ENDPOINT_OVERRIDE).evaluateAttributeExpressions().getValue()).isEmpty();
- if (usePathStyleAccess == Boolean.TRUE || endpointOverrideSet) {
- builder.withPathStyleAccessEnabled(true);
- }
- };
-
- AmazonS3 s3Client = null;
- if (encryptionService != null) {
- s3Client = encryptionService.createEncryptionClient(clientBuilder);
- }
- if (s3Client == null) {
- final AmazonS3ClientBuilder builder = AmazonS3Client.builder();
- clientBuilder.accept(builder);
- s3Client = builder.build();
- }
-
- return (AmazonS3Client) s3Client;
- }
-
+ // maps AWS SDK v1 CannedAccessControlList to v2 ObjectCannedACL
+ private static final Map<String, ObjectCannedACL> CANNED_ACL_MAPPING = Map.of(
+ "Private", ObjectCannedACL.PRIVATE,
+ "PublicRead", ObjectCannedACL.PUBLIC_READ,
+ "PublicReadWrite", ObjectCannedACL.PUBLIC_READ_WRITE,
+ "AuthenticatedRead", ObjectCannedACL.AUTHENTICATED_READ,
+ "LogDeliveryWrite", ObjectCannedACL.UNKNOWN_TO_SDK_VERSION,
Review Comment:
Thanks for catching it, @pvillard31. Running into error is definitely a wrong option to handle this case.
Now I looked into the S3 server access logging in more detail and as far as I can see, `LogDeliveryWrite` `Canned ACL` has never had a value use-case in Put/CopyS3Object processors. The permission is applicable to buckets, not objects (NiFi works with objects). Also, it is used for the target bucket where the access logs are written to by AWS (NiFi reads/writes objects in the source bucket being monitored via access logs).
So I think that removing this option would not even mean a breaking change. We can just migrate `LogDeliveryWrite` to empty property value (in order not to cause an error due to the unknown enum value).
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org