Skip to content

Remove ServiceMetadata usage from DefaultS3Presigner, S3Utilities,and DefaultPollyPresigner#7110

Open
S-Saranya1 wants to merge 5 commits into
feature/master/remove-service-metadata-usagefrom
somepal/presigners-remove-servicemetadata
Open

Remove ServiceMetadata usage from DefaultS3Presigner, S3Utilities,and DefaultPollyPresigner#7110
S-Saranya1 wants to merge 5 commits into
feature/master/remove-service-metadata-usagefrom
somepal/presigners-remove-servicemetadata

Conversation

@S-Saranya1

@S-Saranya1 S-Saranya1 commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Motivation and Context

Remove ServiceMetadata dependency from DefaultS3Presigner, S3Utilities, and DefaultPollyPresigner. These three classes call AwsClientEndpointProvider with serviceEndpointPrefix/defaultProtocol/region, which triggers the expensive clientEndpointFromServiceMetadata() code path — loading GeneratedServiceMetadataProvider and incurring 350-500ms static initialization overhead. This is part of the ServiceMetadata removal work, feature branch - feature/master/remove-service-metadata-usage

Modifications

  • DefaultS3Presigner: Use resolveFromOverrides() to check for endpoint overrides/environment. If none found, use https://localhost placeholder (marked as not overridden) — S3ResolveEndpointInterceptor replaces it with the real endpoint during presigning. Added region validation at build time for fail-fast on invalid regions.

  • S3Utilities: Same approach — resolveFromOverrides() with localhost fallback. The endpoint is replaced by S3EndpointProvider during the getUrl() flow.

  • DefaultPollyPresigner: Use resolveFromOverrides() to check for overrides/environment. If none found, call PollyEndpointProvider (Endpoints 2.0) directly to get an accurate endpoint. Unlike S3, the Polly presigner has no downstream endpoint resolution — the endpoint becomes the final presigned URL host.

  • InvalidRegionTest: Updated assertions to match new error messages. S3Utilities errors now come from S3EndpointProvider ("Invalid region: region was not a valid DNS name.") and S3Presigner errors come from build-time validation ("Configured region .. resulted in an invalid URI"). the exception type is same, but the error message slightly varies( the exception type and fail-fast behavior are preserved).

Testing

  • Existing S3PresignerTest and S3UtilitiesTest suites validate full endpoint URLs (virtual-hosted, path-style, accelerate, dualstack, overrides) — any localhost leak would immediately fail these tests.
  • Updated InvalidRegionTest assertions to match new error messages.
  • Added presign_noEndpointOverride_usesDefaultEndpoint test in DefaultPollyPresignerTest to verify that the Endpoints 2.0 fallback produces the correct host (polly.us-east-1.amazonaws.com).

Screenshots (if appropriate)

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)

Checklist

  • I have read the CONTRIBUTING document
  • Local run of mvn install succeeds
  • My code follows the code style of this project
  • My change requires a change to the Javadoc documentation
  • I have updated the Javadoc documentation accordingly
  • I have added tests to cover my changes
  • All new and existing tests passed
  • I have added a changelog entry. Adding a new entry must be accomplished by running the scripts/new-change script and following the instructions. Commit the new file created by the script in .changes/next-release with your changes.
  • My change is to implement 1.11 parity feature and I have updated LaunchChangelog

License

  • I confirm that this pull request can be released under the Apache 2 license

@S-Saranya1 S-Saranya1 requested a review from a team as a code owner July 7, 2026 22:25
+ testEndpoint + ". This is usually caused by an invalid region "
+ "configuration.");
}
endpointProvider = ClientEndpointProvider.create(URI.create("https://localhost"), false);

@zoewangg zoewangg Jul 14, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hmm, why are we using localhost here?

NVM, it's used as a placeholder and will be replaced anyway. Can we follow the same pattern in EC2 and add some comments https://github.com/aws/aws-sdk-java-v2/blob/master/services/ec2/src/main/java/software/amazon/awssdk/services/ec2/transform/internal/GeneratePreSignUrlInterceptor.java#L67

.resolveFromOverrides();

ClientEndpointProvider endpointProvider;
if (overrideEndpoint.isPresent()) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Any reason we are not using S3EndpointProvider here?

.resolveFromOverrides();

return overrideEndpoint.orElseGet(() -> CompletableFutureUtils.joinLikeSync(
PollyEndpointProvider.defaultProvider()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Any reason we are not using localhost like we do in other presigners?

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.

2 participants