Skip to content
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

Http client conflicts #120

Open
samu-developments opened this issue Dec 28, 2021 · 17 comments · May be fixed by #303
Open

Http client conflicts #120

samu-developments opened this issue Dec 28, 2021 · 17 comments · May be fixed by #303
Assignees
Labels
research Needs research

Comments

@samu-developments
Copy link

Hi,

I'm getting the error below when integrating schema registry in our project.

Caused by: software.amazon.awssdk.core.exception.SdkClientException: Multiple HTTP implementations 
were found on the classpath. To avoid non-deterministic loading implementations, please explicitly provide 
an HTTP client via the client builders, set the software.amazon.awssdk.http.service.impl system property 
with the FQCN of the HTTP service to use as the default, or remove all but one HTTP implementation 
from the classpath

I cannot resolve this with the directions above (with our current httpClient), and believe the issue is that UrlConnectionHttpClient is used explicitly in the builder below.

GlueClientBuilder glueClientBuilder = GlueClient
                .builder()
                .credentialsProvider(credentialsProvider)
                .overrideConfiguration(overrideConfiguration)
                .httpClient(UrlConnectionHttpClient.builder().build())
                .region(Region.of(glueSchemaRegistryConfiguration.getRegion()));

source

@callado4
Copy link

callado4 commented Jan 6, 2022

It seems like it was added with this PR: #49

@mohitpali is the UrlConnectionHttpClient needed for the JSON support? Have this dependency in the library conflicts with being able to use the ProfileCredentialProvider in the default configuration when this dependency is in the classpath.

@blacktooth
Copy link
Contributor

Thanks for reporting! I will investigate this and post my findings.

@blacktooth
Copy link
Contributor

I think we were specifying it explicitly to avoid conflicts with HttpClient. Are you constructing a AWS Client in your application?
Did you try explicitly specifying the http client?

Related AWS SDK issue

@blacktooth blacktooth self-assigned this Jan 11, 2022
@blacktooth blacktooth added the research Needs research label Jan 11, 2022
@samu-developments
Copy link
Author

You're right, found where it was implicitly created, creating it explicitly seems to have fixed the problem!

@samu-developments
Copy link
Author

I reopened the issue; I feel like the fact that it is explicitly specified here in this dependency should not affect the rest of the codebase having glue-schema-registry as a dependency. Instead of having to specify it explicitly everywhere else, glue-schema-registry should also use the service loader approach and support several http clients.

@blacktooth
Copy link
Contributor

glue-schema-registry should also use the service loader approach and support several http clients.

What is the service loader approach?

@samu-developments
Copy link
Author

It would be to place a provider configuration file in resources/services folder specifying which httpClient implementation to use.

Dug through the aws-sdk-v2 source code and it seems it is simply not supported to have more than one http client in the classpath. Loading a provider with a provider configuration file won't work since it is done after loading any normal named modules ref java source.
Aws recommends just removing any other dependency in the classpath, or preferably explicitly using it. So I guess you're doing it correct.

@esfomeado
Copy link

Any plans on fixing this?

@samu-developments
Copy link
Author

samu-developments commented Jan 12, 2023

Here's how we have worked around it. We build the library with url-connection-client dependency removed (look at pr linked above), and use it instead of the common module in our project. Need to add some transitive dependencies that are not pulled after the exclusion.

// Handle software.amazon.glue:schema-registry-common excplicitly depends on a SdkHttpClient implementaion.
// https://github.com/awslabs/aws-glue-schema-registry/pull/240
implementation("software.amazon.glue:schema-registry-serde:2.18.21") {
    exclude("software.amazon.awssdk", "url-connection-client")
    exclude("software.amazon.glue", "schema-registry-common")
}
implementation("org.apache.commons:commons-lang3:3.12.0") // Missing transitive dependency
implementation("software.amazon.awssdk:glue:2.18.21")

// Add custom schema-registry-common built without url-connection-client
implementation(files("$projectDir/libs/schema-registry-common-1.1.14.jar"))

@artembilan
Copy link

Plus 1 for this. Other AWS clients resolves and HTTP client via service loader.
This one has a hard-coded .httpClient(UrlConnectionHttpClient.builder().build()) making the pain for other clients.

The workaround is to to do this:

		<dependency>
			<groupId>software.amazon.kinesis</groupId>
			<artifactId>amazon-kinesis-client</artifactId>
			<exclusions>
				<exclusion>
					<groupId>software.amazon.awssdk</groupId>
					<artifactId>apache-client</artifactId>
				</exclusion>
			</exclusions>
		</dependency>

But is it really what we are looking for?

Thanks for understanding!

artembilan added a commit to spring-cloud/spring-cloud-stream-binder-aws-kinesis that referenced this issue May 3, 2023
* Exclude `apache-client` from the `amazon-kinesis-client`
to avoid conflict with an `url-connection-client`
which is hard-coded to the `aws-glue-schema-registry`.
See more info in the: awslabs/aws-glue-schema-registry#120
@dbottini
Copy link

Yep, admittedly I'm piling on here, but this module basically poison-pills any project that has been safely using AWS sdk clients for years if it's added - like with software.amazon.kinesis:amazon-kinesis-client 2.4.0 that added the glue client and knocked out our ability to make Java SDK v1 clients due to classpath discovery stuff. Still using SDK v1 clients is its own issue, but having glue add something to the classpath when every other aws sdk client/expanded library doesn't and require additional exclusions is a bit frustrating.

https://github.com/awslabs/amazon-kinesis-client/blob/master/amazon-kinesis-client/src/main/java/software/amazon/kinesis/common/ConfigsBuilder.java#L130-L152
Kinesis Client Library lets the user provide their own KinesisClient/DynamoDBClient/etc, which encourages use of of KinesisClient.builder() etc. to pick up the the appropriate http client from the classpath.

This library is running an entire wrapper of GlueClient.builder()....build() https://github.com/awslabs/aws-glue-schema-registry/blob/master/common/src/main/java/com/amazonaws/services/schemaregistry/common/AWSSchemaRegistryClient.java#L88-L116 is running builders it doesn't need to - it should take in a GlueClient or GlueClient.Builder and finalize it with the modifications it needs. Example code: https://github.com/awslabs/amazon-kinesis-client/blob/master/amazon-kinesis-client/src/main/java/software/amazon/kinesis/common/KinesisClientUtil.java#L40-L49 (KCL making some HTTP configuration settings using builder modifications).

Admittedly, stripping the existing use of UrlConnectionHttpClient is a bit of a breaking change for users of this library, however.

@blacktooth
Copy link
Contributor

Agree that adding this library shouldn't require exclusions, we will re-evaluate the use of UrlConnectionHttpClient and come up with some solutions to solve the problem.

@dbottini
Copy link

#263
Raised the PR as a proposed solution to pulling out the runtime dependency on url connection http library. It does strip out the first-class integration with proxy url, as that was an implementation detail of the underlying http client, and should not have been baked into schema registry client as a whole. That does make this a breaking change, however.

@YangSan0622
Copy link
Contributor

YangSan0622 commented May 17, 2023

@dbottini Is there a quick example you can share with us to reproduce the original issue, I wanted to experiment and understand this issue further before reviewing your PR?

@dbottini
Copy link

https://github.com/dbottini/schema-registry-client-test
Sorry for the delay - this should be a simple POC of a basic SQS Client with schema-registry-common, and you can just change the schema-registry-common 1.0.2 -> 1.1.0 and you should see it fail to instantiate.

@jhchee
Copy link

jhchee commented Jul 20, 2023

@blacktooth Is there any chance to prioritize this given the issue has existed since 2021?

@singhbaljit
Copy link
Contributor

singhbaljit commented Oct 12, 2023

This actually breaks the WebIdentityTokenFileCredentialsProvider. While other SDKv2 client builders provide override to specify the HTTP clients, there is no way to do so with WebIdentityTokenFileCredentialsProvider. This breaks using service accounts in IAM roles in Kubernetes/Containerized environments. Moreover, the preferred usage with the rest of the AWS SDK is to use the default provider chain; so we should not be forced to override that. The Glue clients should use the Apache client, as it the default with the rest of the AWS SDK.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
research Needs research
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants