Skip to content

7910 support having a grant with the same name on different keys#7911

Merged
viren-nadkarni merged 1 commit intolocalstack:masterfrom
atlassian-forks:7910-same-name-grants-different-keys
Mar 21, 2023
Merged

7910 support having a grant with the same name on different keys#7911
viren-nadkarni merged 1 commit intolocalstack:masterfrom
atlassian-forks:7910-same-name-grants-different-keys

Conversation

@BlueDragon23
Copy link
Contributor

This change addresses this issue #7910
Previously, due to the grant_names only mapping from grant name to grant ID, it was impossible to have more than one grant with the same name across keys. By including the key ID in the mapping, it becomes possible.

I've added a unit test to demonstrate the fix works, and allows this grant to be created on both keys.

If any extra test coverage would help, just point me in the right direction!

@localstack-bot
Copy link
Contributor

localstack-bot commented Mar 20, 2023

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

Copy link
Contributor

@localstack-bot localstack-bot left a comment

Choose a reason for hiding this comment

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

Welcome to LocalStack! Thanks for raising your first Pull Request and landing in your contributions. Our team will reach out with any reviews or feedbacks that we have shortly. We recommend joining our Slack Community and share your PR on the #community channel to share your contributions with us. Please make sure you are following our contributing guidelines and our Code of Conduct.

@BlueDragon23
Copy link
Contributor Author

I have read the CLA Document and I hereby sign the CLA

@BlueDragon23
Copy link
Contributor Author

recheck

@BlueDragon23
Copy link
Contributor Author

It looks like the failing test is related to DynamoDB rest tests, which shouldn't be impacted by this change at all. Is it possible to have that test re-run?

@viren-nadkarni viren-nadkarni self-requested a review March 21, 2023 09:20
Copy link
Member

@viren-nadkarni viren-nadkarni left a comment

Choose a reason for hiding this comment

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

LGTM!

Thanks for contributing @BlueDragon23!

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.

3 participants