Http client conflicts about aws-glue-schema-registry HOT 17 OPEN

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.

from aws-glue-schema-registry.

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

from aws-glue-schema-registry.

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.

from aws-glue-schema-registry.

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.

from aws-glue-schema-registry.

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

from aws-glue-schema-registry.

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

from aws-glue-schema-registry.

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

from aws-glue-schema-registry.

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

What is the service loader approach?

from aws-glue-schema-registry.

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.

from aws-glue-schema-registry.

Any plans on fixing this?

from aws-glue-schema-registry.

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"))

from aws-glue-schema-registry.

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!

from aws-glue-schema-registry.

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.

from aws-glue-schema-registry.

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.

from aws-glue-schema-registry.

#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.

from aws-glue-schema-registry.

@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?

from aws-glue-schema-registry.

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.

from aws-glue-schema-registry.