Remove derived attribute logic for PRESIGNER_EXPIRATION #4602
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Because of the 2 instant() calls in the write/read mappings, a basic write this attribute and read it back, would give different values. Since this attribute should only be set/read explicitly by our presigner code, we don't need this mapping logic at all. Also, update S3PresignerTest to assert isEqualTo instead of isCloseTo and remove unnecessary override
12 tasks
millems
approved these changes
Oct 16, 2023
|
Kudos, SonarCloud Quality Gate passed! |
sugmanue
pushed a commit
that referenced
this pull request
Oct 31, 2023
* Remove derived attribute logic for PRESIGNER_EXPIRATION Because of the 2 instant() calls in the write/read mappings, a basic write this attribute and read it back, would give different values. Since this attribute should only be set/read explicitly by our presigner code, we don't need this mapping logic at all. Also, update S3PresignerTest to assert isEqualTo instead of isCloseTo and remove unnecessary override * Remove PRESIGNER_EXPIRATION mirroring test
sugmanue
added a commit
that referenced
this pull request
Nov 6, 2023
* Set TIME_OFFSET attribtue in SRA signin methods (#4483) * Verify that execution attributes set by old interceptors can be read by new signers. (#4494) * Move S3Presigner fully to SRA with HttpSigner (#4569) * Remove derived attribute logic for PRESIGNER_EXPIRATION (#4602) * Remove derived attribute logic for PRESIGNER_EXPIRATION Because of the 2 instant() calls in the write/read mappings, a basic write this attribute and read it back, would give different values. Since this attribute should only be set/read explicitly by our presigner code, we don't need this mapping logic at all. Also, update S3PresignerTest to assert isEqualTo instead of isCloseTo and remove unnecessary override * Remove PRESIGNER_EXPIRATION mirroring test * Enable SRA Auth for S3 * Make follow up changes from PR 4396 (#4454) * Make SigingStage updates similar to AsyncSigningStage. * Simplify (Async)SigningStageTests based on updated logic. * Minor typos * If authType=none don't use the old signer, even if overridden (#4523) * Add authType=none tests to AwsExecutionContentBuilderTest Some of these tests are failing. Will fix them in next commit * If authType=none don't use the old signer, even if overridden * Update signing stage logic for authType=none * Add asserts on identity resolution for authType=none * Add a missing http-auth-aws dependecy to aws-core * Add a changelog entry --------- Co-authored-by: Zoe Wang <[email protected]> Co-authored-by: Matthew Miller <[email protected]> Co-authored-by: Jaykumar Gosar <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation and Context
Because of the 2 instant() calls in the write/read mappings, a basic write this attribute and read it back, would give different values.
Since this attribute should only be set/read explicitly by our presigner code, we don't need this mapping logic at all.
Modifications
*Remove derived attribute logic for PRESIGNER_EXPIRATION
*Also, update S3PresignerTest to assert isEqualTo instead of isCloseTo and remove unnecessary override
Testing
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