-
Notifications
You must be signed in to change notification settings - Fork 963
Make s3control work with preSRA codegen #4495
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
Make s3control work with preSRA codegen #4495
Conversation
db74c34 to
6228139
Compare
|
SonarCloud Quality Gate failed.
|
| private <T extends Identity> SelectedAuthScheme<T> authSchemeWithEndpointSignerProperties( | ||
| List<EndpointAuthScheme> endpointAuthSchemes, SelectedAuthScheme<T> selectedAuthScheme) { | ||
| for (EndpointAuthScheme endpointAuthScheme : endpointAuthSchemes) { | ||
| if (!endpointAuthScheme.schemeId().equals(selectedAuthScheme.authSchemeOption().schemeId())) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @millems for highlighting that not checking this could be an issue for useSraAuth=true client, in a case for example where SelectedAuthScheme=sigv4a, but endpointAuthSchemes=["sigv4","sigv4a"] and the shared signer properties like disableDoubleEncoding have different values from endpoint rules. Without this check, the value from sigv4 EndpointAuthScheme will get used, even though it should use from sigv4a EndpointAuthScheme.
Will address this is separate PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
addressing in #4505
…1304dfad7 Pull request: release <- staging/d73ba50f-8835-45de-888c-6b41304dfad7












Motivation and Context
This is follow up from #4491, where I forced s3control to useSraAuth=true path since tests were failing with useSraAuth=false. Fixing it here.
Modifications
With #4396 since we could have a dummy auth scheme (which would definitely be true for useSraAuth=false), we add the SignerProperties to the SelectedAuthScheme, irrespective of the schemeId. For useSraAuth path, where old Signer is used, this effectively adds the ExecutionAttributes that old Signer needs because of #4396.
Testing
./mvnw clean install -pl :codegen,:s3controlpasses locally.Screenshots (if appropriate)
Types of changes
Checklist
mvn installsucceedsscripts/new-changescript and following the instructions. Commit the new file created by the script in.changes/next-releasewith your changes.License