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

Codegen changes for AuthSchemeInterceptorSpecs to update the RegionSet from Execution Attributes #5768

Merged

Conversation

joviegas
Copy link
Contributor

@joviegas joviegas commented Jan 1, 2025

Motivation and Context

  • Modify interceptor to update the RegionSet from Execution Attributes
  • Fall back region is always configured Region if no RegionSet configured on Client (Requirement doc)
    (Note Will raise a separate PR later for EndpointBased Auth case that is legacy service like SES , S3 etc)

Modifications

Generated Code looks like below

    private MultiauthAuthSchemeParams authSchemeParams(SdkRequest request, ExecutionAttributes executionAttributes) {
        String operation = executionAttributes.getAttribute(SdkExecutionAttribute.OPERATION_NAME);
        MultiauthAuthSchemeParams.Builder builder = MultiauthAuthSchemeParams.builder().operation(operation);
        Region region = executionAttributes.getAttribute(AwsExecutionAttribute.AWS_REGION);
        builder.region(region);
        // Comment on PR : This code is generate only if Service has Sigv4a Auth
        RegionSet regionSet = executionAttributes.getOptionalAttribute(AwsExecutionAttribute.AWS_SIGV4A_SIGNING_REGION_SET)
                .filter(regions -> !regions.isEmpty()).map(regions -> RegionSet.create(String.join(", ", regions)))
                .orElseGet(() -> {
                    Region fallbackRegion = executionAttributes.getAttribute(AwsExecutionAttribute.AWS_REGION);
                    return fallbackRegion != null ? RegionSet.create(fallbackRegion.toString()) : null;
                });
        ;
        builder.regionSet(regionSet);
        return builder.build();

Testing

  • Junits added for codegen
  • Junits for interceptors to check AuthScheme Params are updated

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 1, 2025 00:26
@joviegas joviegas changed the title Joviegas/autheme params spec Codegen changes for AuthSchemeInterceptorSpecs to update the RegionSet from Execution Attributes Jan 1, 2025
@@ -148,20 +149,17 @@ private MethodSpec generateAuthSchemeParams() {
if (!authSchemeSpecUtils.useEndpointBasedAuthProvider()) {
builder.addStatement("$T operation = executionAttributes.getAttribute($T.OPERATION_NAME)", String.class,
SdkExecutionAttribute.class);
builder.addStatement("$T.Builder builder = $T.builder().operation(operation)",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This part is needed because earlier Builder was built in a single line
Since we have conditional bases builder for sigv4a so splited this generation code

QueryAuthSchemeParams.builder().operation(operation).region(region).build();

@@ -0,0 +1,4 @@
{
"skipEndpointTestGeneration": true,
"useMultiAuth": true
Copy link
Contributor

Choose a reason for hiding this comment

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

Discussed offline, we should consider removing useMultiAuth

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added JAVA-8023 and also added in planning

@joviegas joviegas merged commit 1494c9b into feature/master/multi-auth-sigv4a Jan 3, 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