Skip to content

NIFI-15583 - S3 Processors use global endpoint instead of regional endpoint for us-east-1#10887

Merged
exceptionfactory merged 1 commit intoapache:mainfrom
pvillard31:NIFI-15583
Feb 13, 2026
Merged

NIFI-15583 - S3 Processors use global endpoint instead of regional endpoint for us-east-1#10887
exceptionfactory merged 1 commit intoapache:mainfrom
pvillard31:NIFI-15583

Conversation

@pvillard31
Copy link
Contributor

Summary

NIFI-15583 - S3 Processors use global endpoint instead of regional endpoint for us-east-1

More details in the JIRA description.

Important note: given the capabilities in the client when using encryption on the client side, this fix would not apply. If we do want a solution that would also work there, then an option is to set a system property directly in the code. I'm open to this approach but this is not amazing. I guess we would leave it to the user to properly configure NiFi and pass the right JVM arguments.

Tracking

Please complete the following tracking steps prior to pull request creation.

Issue Tracking

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 contains commits signed with a registered key indicating Verified status

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 according to the License Policy
  • New dependencies are documented in applicable LICENSE and NOTICE files

Documentation

  • Documentation formatting appears as expected in rendered files

…dpoint for us-east-1

Signed-off-by: Pierre Villard <pierre.villard.fr@gmail.com>
@pvillard31 pvillard31 added the bug label Feb 12, 2026
@turcsanyip
Copy link
Contributor

@pvillard31 Thanks for fixing this issue. Do I understand correctly that defaultsMode() and endpointProvider() cannot be applied on S3EncryptionClient? I see you moved defaultsMode() to the regular client only in NIFI-15569 with the comment "Fixed AWS SDK S3 DefaultsMode when using encryption" but it is not clear for me what the issue was. Is it the same for endpointProvider()?

@pvillard31
Copy link
Contributor Author

@turcsanyip - yeah so in the initial PR for adding DefaultsMode.STANDARD (#10870), I was not careful and it did break the client when using encryption (failing integration tests) so I fixed it as part of #10873 where integration tests are running as part of the checks.

It is the same in this PR basically. We cannot easily apply the same fix when we use encryption (we would get the same unsupported operation exception)

S3EncryptionClient.Builder.build(), when no wrappedClient is provided (which is NiFi's case), it creates its own internal S3Client. So the encryption builder's internally-created S3Client has the same issue AND doesn't even set DefaultsMode.STANDARD. However, the encryption builder also has wrappedClient(S3Client). If we provide a pre-built S3Client, it skips the internal creation entirely. We could leverage this by having S3ClientBuilderWrapper maintain a parallel S3ClientBuilder for the encryption case that mirrors all configuration calls and has our endpoint provider. But that means duplicating every delegation method to also configure the parallel builder. It feels fragile and complex. I'd rather submit an issue on the repo on the AWS side to expose options for us. But if you see a better approach, I'm definitely happy to discuss an alternative.

Copy link
Contributor

@exceptionfactory exceptionfactory left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for fixing this issue @pvillard31, the adjustment makes sense and the test looks good. +1 merging

@exceptionfactory exceptionfactory merged commit 65ed41e into apache:main Feb 13, 2026
13 of 15 checks passed
@turcsanyip
Copy link
Contributor

Thanks for the insights @pvillard31. I double checked why I did not use wrappedClient earlier and whether it could be passed to S3EncryptionClient.Builder but it is non-trivial because S3EncryptionClient depends on both the sync and async clients (that is wrappedAsyncClient should be set too). The system property looks the easy path for the encryption use-case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants