Skip to content

CRT client configuration feature parity #3675

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
2 tasks
pwinckles opened this issue Jan 7, 2023 · 8 comments
Open
2 tasks

CRT client configuration feature parity #3675

pwinckles opened this issue Jan 7, 2023 · 8 comments
Labels
feature-request A feature should be added or improved. p2 This is a standard priority issue

Comments

@pwinckles
Copy link

Describe the feature

The CRT client should have feature parity with the old client builder. Specifically, the CRT builder does not currently allow you to specify S3Configuration or modify the client to trust all certificates.

For example, the following is not possible with the CRT client:

S3AsyncClient.builder()
        .endpointOverride(URI.create(S3_MOCK.getServiceEndpoint()))
        .region(Region.US_EAST_2)
        .credentialsProvider(
                StaticCredentialsProvider.create(AwsBasicCredentials.create("foo", "bar")))
        .serviceConfiguration(S3Configuration.builder().pathStyleAccessEnabled(true).build())
        .httpClient(NettyNioAsyncHttpClient.builder().buildWithDefaults(AttributeMap.builder()
                .put(TRUST_ALL_CERTIFICATES, Boolean.TRUE)
                .build()))
        .build();

Use Case

These configuration options are needed for supporting S3 mock implementations.

Proposed Solution

No response

Other Information

No response

Acknowledgements

  • I may be able to implement this feature request
  • This feature might incur a breaking change

AWS Java SDK version used

2.19.8

JDK version used

17.0.4

Operating System and version

Fedora 36

@pwinckles pwinckles added feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged. labels Jan 7, 2023
@debora-ito
Copy link
Member

Hi @pwinckles, I have some questions about your request.

When you say:

The CRT client should have feature parity with the old client builder.

Which old client builder are you referring to?

Which features specifically are you asking for in the CRT client, is it pathStyleAccessEnabled and the ability to support TRUST_ALL_CERTIFICATES?
Also, are you using the CRT client in the S3TransferManager?

@debora-ito debora-ito added response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 10 days. and removed needs-triage This issue or PR still needs to be triaged. labels Jan 9, 2023
@pwinckles
Copy link
Author

Which old client builder are you referring to?

The builders for the non-CRT sync/async clients, like the one in my example.

Which features specifically are you asking for in the CRT client, is it pathStyleAccessEnabled and the ability to support TRUST_ALL_CERTIFICATES?

For me personally, I would like path-style access and the ability to trust all certs, with the former being more important than the later. However, it would seem to me that it would be good for the builders to offer as similar of configuration options as possible.

Also, are you using the CRT client in the S3TransferManager?

Yes, I started updating a library to use the transfer manager, and noticed that it's only worth while to do so if you use it with the CRT client. However, the CRT client does not support the same configuration options.

@github-actions github-actions bot removed the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 10 days. label Jan 10, 2023
@mvillafuertem
Copy link

the environment variable disableCertChecking could be added as in previous versions, as requested in this ticket #1230

@fatih-celonis
Copy link

Path style access is an important flag for us because our integration tests and local development setup rely on it.

@chrisrhut
Copy link

I found this issue by searching for "CRT path style access". +1 for needing that in a test environment.

@StephenFlavin
Copy link
Contributor

additionally it would be nice if the crt client could support metrics publishers in the ClientOverrideConfiguration, it seems to be specifically blocked at the moment https://github.com/aws/aws-sdk-java-v2/blob/master/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/crt/DefaultS3CrtAsyncClient.java#L325-L327

@campidelli
Copy link

Isn't it fixed by this #3817 ?

@debora-ito debora-ito added the p2 This is a standard priority issue label Jul 31, 2024
@aaawuanjun
Copy link

  • I see in the doc as belows on v2:
The following system properties no longer supported: 
`com.amazonaws.sdk.disableCertChecking`,
 `com.amazonaws.sdk.enableDefaultMetrics`, 
`com.amazonaws.sdk.enableThrottledRetry`,
 `com.amazonaws.regions.RegionUtils.fileOverride`,
 `com.amazonaws.regions.RegionUtils.disableRemote`, 
`com.amazonaws.services.s3.disableImplicitGlobalClients`, 
`com.amazonaws.sdk.enableInRegionOptimizedMode`

image

so, if the aws-sdk-java v1 version will not support until December 31, 2025,
why over 5 years not finish this?
image

I see the first issue is on year 2019.
image

What is the plan, or it will never support on sdk, if already have methods(I did not see.), Please provide a way to use.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request A feature should be added or improved. p2 This is a standard priority issue
Projects
None yet
Development

No branches or pull requests

8 participants