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

Add support for service-specific endpoint overrides without code changes. #5562

Merged
merged 15 commits into from
Sep 12, 2024

Conversation

millems
Copy link
Contributor

@millems millems commented Sep 4, 2024

This adds support for this feature: https://docs.aws.amazon.com/sdkref/latest/guide/feature-ss-endpoints.html

The commit has been broken up into 6 commits, to hopefully simplify the code review:

Commit 1 - Made protected API changes in support of service-specific endpoint overrides.

This introduces new ClientEndpointProvider abstraction, which is used by clients to resolve the "default" endpoint for the client. The main implementation is the new AwsClientEndpointProvider class, which checks the client-level endpoint override, environment variables, system properties and profile properties, in priority order. More information about what it checks is documented here.

Other Protected API changes:

  • DefaultServiceEndpointBuilder - Deprecated in favor of the new ClientEndpointProvider and AwsClientEndpointProvider
  • Deprecated SdkClientOption.ENDPOINT and SdkClientOption.ENDPOINT_OVERRIDDEN in favor of a new SdkClientOption.CLIENT_ENDPOINT_PROVIDER. The new property allows updating both values in a single field. It was a necessary improvement so that these properties can stay in sync, reliably.
  • Deprecated SdkExecutionAttribute.CLIENT_ENDPOINT and SdkClientOption.ENDPOINT_OVERRIDDEN in favor of a new SdkInternalExecutionAttribute.CLIENT_ENDPOINT_PROVIDER. The new property exists for much the same reasons as the new SdkClientOption, but also provides an opportunity to hide these should-be-internal-to-the-SDK attributes.

Commit 2 - Wires up the new AwsClientEndpointProvider for service clients

In this change, service client builders set the AwsClientEndpointProvider when they are created. This allows setting the service-specific environment variables, system settings and profile properties.

We also update the default AWS client builder to set the ClientEndpointProvider, so that old client versions get the new CLIENT_ENDPOINT_PROVIDER option set. The default SDK client builder is also updated to use the new provider abstraction for endpoint overrides.

This also includes EndpointSharedConfigTest, which tests to make sure that the new ways of overriding the endpoint work for clients.

Commit 3 - Wire up the new AwsClientEndpointProvider for non-clients

We want the new feature to work for other places that endpoints are derived. This commit includes updating S3Utilities, S3Presigner and PollyPresigner to also support the new way of specifying endpoint overrides.

We also update RdsPresignInterceptors in RDS, Neptune and DocDB to use the new AwsClientEndpointProvider. Here, we don't wire up support for the new environment-based endpoint overrides, because the endpoint in the presigned URL for these APIs needs to be the actual RDS endpoint URL, not overrides.

Commit 4 - Move users of the old SdkClientOptions and SdkExecutionAttributes over to the new settings

These changes are less interesting, and a bit tedious. In this, we change everywhere in the SDK that is using the old SdkClientOption.ENDPOINT, SdkClientOption.ENDPOINT_OVERRIDDEN to the new SdkClientOption.CLIENT_ENDPOINT_PROVIDER. We also update everywhere in the SDK we're using the old SdkExecutionAttribute.CLIENT_ENDPOINT and SdkClientOption.ENDPOINT_OVERRIDDEN to the new SdkInternalExecutionAttribute.CLIENT_ENDPOINT_PROVIDER.

Commit 5 - CodeGen test updates

These are even less interesting. These are the codegen test files we changed in support of commit 2 above.

Commit 6 - Changelog Entry

Self-explanatory.

…errides.

Class-Level Changes:
* DefaultServiceEndpointBuilder - Deprecated in favor of the new ClientEndpointProvider and AwsClientEndpointProvider
* ClientEndpointProvider - An interface for resolving the client-level endpoint. This endpoint may be overridden by the request-level EndpointProvider. This joins the existing fleet of endpoint-related providers, like region providers, dualstack providers, and FIPS providers.
* AwsClientEndpointProvider - An implementation of ClientEndpointProvider that checks the client configuration, environment and service metadata to determine the client endpoint.

Field Changes:
* Deprecated SdkClientOption.ENDPOINT and SdkClientOption.ENDPOINT_OVERRIDDEN in favor of a new SdkClientOption.CLIENT_ENDPOINT_PROVIDER. The new property allows updating both values in a single field. It was a necessary improvement so that these properties can stay in sync, reliably.
* Deprecated SdkExecutionAttribute.CLIENT_ENDPOINT and SdkClientOption.ENDPOINT_OVERRIDDEN in favor of a new SdkInternalExecutionAttribute.CLIENT_ENDPOINT_PROVIDER. The new property exists for much the same reasons as the new SdkClientOption, but also provides an opportunity to hide these should-be-internal-to-the-SDK attributes.
…ndpoint providers.

1. Make clients set the SdkClientOption.CLIENT_ENDPOINT_PROVIDER when they are created.
2. Update the default AWS and SDK client builders to default the SdkClientOption.CLIENT_ENDPOINT_PROVIDERs so that old clients continue to work with the new core. Old clients versions won't support setting the endpoint with the new environment variable/system property/profile settings.

This commit also includes EndpointSharedConfigTest which tests to make sure that the new ways of overriding the endpoint work for clients.
…ientEndpointProvider.

These are the users that remain after the last commit:
1. RdsPresignInterceptors in RDS, Neptune and DocDB. It's unfortunate that these are pretty much copy+pasted classes, but that seems like a future problem to figure out.
2. S3Utilities
3. S3Presigner
4. PollyPresigner
…lientOptions and SdkExecutionAttributes to the new SdkClientOptions and SdkInternalExecutionAttribute.
@millems millems requested a review from a team as a code owner September 4, 2024 23:28
@millems millems force-pushed the millem/endpoint-url-part-3 branch from 4ddedd9 to 2a3cce2 Compare September 4, 2024 23:53
@stantonk
Copy link

stantonk commented Sep 9, 2024

So excited for this!!

Comment on lines +93 to +100
/**
* This is the same as {@link #derivedBuilder(String, Class, ExecutionAttribute)}, but the real attribute is loaded
* lazily at runtime. This is useful when the real attribute is in the same class hierarchy, to avoid initialization
* order problems.
*/
public static <T, U> DerivedAttributeBuilder<T, U> derivedBuilder(String name,
@SuppressWarnings("unused") Class<T> attributeType,
Supplier<ExecutionAttribute<U>> realAttribute) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Took me a second to figure out what this meant! Just curious what happens if we do encounter this ordering issue with the other overload?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Spotbugs complains about the dependency. I didn't test what happens if we ignore spotbugs. I suspect it could be a weird "no class def found" because of the cyclic static initialization dependency?

Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
79.7% Coverage on New Code (required ≥ 80%)
D Reliability Rating on New Code (required ≥ A)

See analysis details on SonarCloud

Catch issues before they fail your Quality Gate with our IDE extension SonarLint

@millems millems added this pull request to the merge queue Sep 12, 2024
Merged via the queue into master with commit cfc4a3e Sep 12, 2024
16 of 17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants