Skip to content

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

Merged
S-Saranya1 merged 6 commits into
feature/master/remove-service-metadata-usagefrom
somepal/presigners-remove-servicemetadata
Jul 15, 2026
Merged

Remove ServiceMetadata usage from DefaultS3Presigner, S3Utilities,and DefaultPollyPresigner#7110
S-Saranya1 merged 6 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
.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?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Unlike other presigner the Polly presigner has no downstream endpoint resolution. The endpoint resolved here is the final presigned URL, so it must be a real endpoint, not a placeholder.

@S-Saranya1
S-Saranya1 requested a review from zoewangg July 15, 2026 17:10
@S-Saranya1
S-Saranya1 merged commit e49686a into feature/master/remove-service-metadata-usage Jul 15, 2026
3 of 4 checks passed
@github-actions

Copy link
Copy Markdown

This pull request has been closed and the conversation has been locked. Comments on closed PRs are hard for our team to see. If you need more assistance, please open a new issue that references this one.

@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Jul 15, 2026
@S-Saranya1
S-Saranya1 deleted the somepal/presigners-remove-servicemetadata branch July 17, 2026 19:09
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants