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 sigv4aRegionSet to Client Builder for Multi-Auth Services Supporting Sigv4a #5772

Merged

Conversation

joviegas
Copy link
Contributor

@joviegas joviegas commented Jan 4, 2025

Motivation and Context

  • Clients with Sigv4a Multi auth should provide API on client to set the region set
  • The Sigv4aRegion Set build takes highest Precedence

Note : Even though we have RegionScope this has been not used because SRA introduced RegionSet so using the latest representation class for
Region set

Modifications

Interface

  • Code gen changes that will generate API on Client Builder as below
    /**
     * Sets the {@link RegionSet} to be used for operations using Sigv4a signing requests. This is optional; if not
     * provided, the following precedence is used:
     * <ol>
     * <li>{@link software.amazon.awssdk.core.SdkSystemSetting#AWS_SIGV4A_SIGNING_REGION_SET}.</li>
     * <li>as <code>sigv4a_signing_region_set</code> in the configuration file.</li>
     * <li>The region configured for the client.</li>
     * </ol>
     */
    default B sigv4aRegionSet(RegionSet sigv4aRegionSet) {
        throw new UnsupportedOperationException();
    }

Default Builder

abstract class DefaultMultiauthBaseClientBuilder<B extends MultiauthBaseClientBuilder<B, C>, C> extends
        AwsDefaultClientBuilder<B, C> {
// ...code 
    public B sigv4aRegionSet(RegionSet sigv4aRegionSet) {
        clientConfiguration.option(AwsClientOption.AWS_SIGV4A_SIGNING_REGION_SET,
                sigv4aRegionSet == null ? Collections.emptySet() : sigv4aRegionSet.asSet());
        return thisBuilder();
    }
  • Also added change to set ClientOption when builder is built

Testing

  • Junit for codegen
  • Test case to verify precedence of sigv4aRegionSet on Builder

Screenshots (if appropriate)

License

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

@joviegas joviegas requested a review from a team as a code owner January 4, 2025 00:03
@@ -721,6 +726,18 @@ private MethodSpec authSchemeProviderMethod() {
.build();
}

private MethodSpec sigv4aRegionSetMethod() {
return MethodSpec.methodBuilder("sigv4aRegionSet")
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this just be regionSet? I guess we can finalize it in API surface area review

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure , I have this discussion during Surface API review

@joviegas joviegas merged commit 953e589 into feature/master/multi-auth-sigv4a Jan 4, 2025
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.

2 participants