fix kms api call response on method get-key-rotation-status #12103

pureiboi · 2025-01-05T11:48:17Z

Motivation

This PR will resolve issue #12080

Changes

The response payload as per AWS cli, and request payload used as per issue


{
    "KeyRotationEnabled": true,
    "KeyId": "46d57f31-b654-4cba-bfb5-fa5847724beb",
    "RotationPeriodInDays": 120,
    "NextRotationDate": "2025-05-06T03:16:36.190688+08:00"
}

localstack-bot · 2025-01-05T11:48:30Z

All contributors have signed the CLA ✍️ ✅
_{Posted by the CLA Assistant Lite bot.}

localstack-bot

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.

pureiboi · 2025-01-05T11:53:56Z

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

pureiboi · 2025-01-05T11:55:04Z

i don't think i can add label

sannya-singal

Thanks for tackling this issue 🎉 I have suggested some changes in the implementation and improvements in the test. Let me know if you would like more assistance on this 🙂

localstack-core/localstack/services/kms/provider.py

tests/aws/services/kms/test_kms.py

dnlopes · 2025-01-22T14:57:23Z

Will this PR move forward?

sannya-singal · 2025-01-27T06:50:15Z

Will this PR move forward?

@pureiboi would you like to continue working on this PR and help resolve the reviews? Let me know if there is any help required. Thanks 🙌

pureiboi · 2025-01-27T10:46:06Z

Will this PR move forward?

@pureiboi would you like to continue working on this PR and help resolve the reviews? Let me know if there is any help required. Thanks 🙌

@sannya-singal yes. I can still continue with this. Was occupied, will review again some tome later .

sannya-singal · 2025-01-28T05:28:21Z

Will this PR move forward?

@pureiboi would you like to continue working on this PR and help resolve the reviews? Let me know if there is any help required. Thanks 🙌

@sannya-singal yes. I can still continue with this. Was occupied, will review again some tome later .

Thats great 🚀🎉

pureiboi · 2025-03-03T03:17:04Z

hey @sannya-singal I have updated the PR based on the suggestions, would like to get review again

alexrashed

@pureiboi Commit 3b19c5e is pretty problematic for us, it effectively removes all github workflows. Please make sure to remove this commit from your PR. ;)

This reverts commit 3b19c5e.

pureiboi · 2025-03-03T08:03:52Z

@pureiboi Commit 3b19c5e is pretty problematic for us, it effectively removes all github workflows. Please make sure to remove this commit from your PR. ;)

opps sorry, it was to disable forked repo.

I have removed the commit, waiting for pipeline to run now

sannya-singal

Thanks for addressing the reviews 🚀 The code looks more clean and the snapshot test added looks really good 🙂 I just have some comments, once this is addressed we could move ahead to merging the PR 🥳

tests/aws/services/kms/test_kms.py

sannya-singal · 2025-03-04T10:02:44Z

@pureiboi it would be great if you could address the above nits as soon as possible as we would like to get this PR merged by this week 🙂 Happy to push the changes as well. Please let me know if you need assistance with this, thanks 🙌

tests/aws/services/kms/test_kms.py

…is a potential bug

sannya-singal

Thanks for addressing all the review comments, the changes LGTM 🚀

pureiboi added 3 commits January 5, 2025 19:23

fix: request payload class

ea899a1

fix: kms get-rotation-status api call as per aws doc

017e8f1

fix: formatting

6584ec3

pureiboi requested a review from sannya-singal as a code owner January 5, 2025 11:48

localstack-bot reviewed Jan 5, 2025

View reviewed changes

pureiboi marked this pull request as draft January 5, 2025 11:49

pureiboi closed this Jan 5, 2025

pureiboi reopened this Jan 5, 2025

localstack-bot added a commit that referenced this pull request Jan 5, 2025

@pureiboi has signed the CLA in #12103

f51d122

pureiboi marked this pull request as ready for review January 5, 2025 11:55

pureiboi added 2 commits January 5, 2025 20:14

fix: class attribute

e67049a

fix kms unit test

5699419

sannya-singal added the semver: patch Non-breaking changes which can be included in patch releases label Jan 6, 2025

sannya-singal added this to the 4.1 milestone Jan 6, 2025

sannya-singal requested changes Jan 6, 2025

View reviewed changes

localstack-core/localstack/services/kms/provider.py Outdated Show resolved Hide resolved

localstack-core/localstack/services/kms/provider.py Outdated Show resolved Hide resolved

tests/aws/services/kms/test_kms.py Outdated Show resolved Hide resolved

silv-io modified the milestones: 4.1, Playground Jan 29, 2025

pureiboi added 6 commits February 10, 2025 15:40

remove github workflow

3b19c5e

fix: use key_id from request context

d563230

add snapshot

5fd727d

fix snapshot

b70a0f5

update method name

c020b87

fix lint

e2cfdbb

pureiboi added 2 commits March 3, 2025 09:37

Merge remote-tracking branch 'upstream/master'

64ccfc3

Merge branch 'master' into fix/12080-kms-rotation-status

d56d77a

pureiboi requested review from alexrashed, dfangl and dominikschubert as code owners March 3, 2025 01:37

alexrashed requested changes Mar 3, 2025

View reviewed changes

Revert "remove github workflow"

0dc1920

This reverts commit 3b19c5e.

sannya-singal self-requested a review March 3, 2025 09:17

sannya-singal reviewed Mar 3, 2025

View reviewed changes

tests/aws/services/kms/test_kms.py Outdated Show resolved Hide resolved

tests/aws/services/kms/test_kms.py Outdated Show resolved Hide resolved

sannya-singal requested a review from localstack-bot March 3, 2025 18:06

pureiboi added 2 commits March 4, 2025 18:16

fix: remove unused features

336d1ac

fix: remove change

726e668

sannya-singal reviewed Mar 4, 2025

View reviewed changes

tests/aws/services/kms/test_kms.py Outdated Show resolved Hide resolved

fix revert change not relevant to MR, and comment with findings that …

d1de064

…is a potential bug

sannya-singal self-requested a review March 5, 2025 06:07

sannya-singal approved these changes Mar 5, 2025

View reviewed changes

sannya-singal requested a review from alexrashed March 5, 2025 06:10

alexrashed approved these changes Mar 5, 2025

View reviewed changes

sannya-singal merged commit 7523ada into localstack:master Mar 5, 2025
29 checks passed

agseco mentioned this pull request Mar 5, 2025

KMS: on-demand key rotation #12342

Merged

2 tasks

Uh oh!

fix kms api call response on method get-key-rotation-status #12103

fix kms api call response on method get-key-rotation-status #12103

Uh oh!

Conversation

pureiboi commented Jan 5, 2025

Motivation

Changes

Uh oh!

localstack-bot commented Jan 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

localstack-bot left a comment

Choose a reason for hiding this comment

Uh oh!

pureiboi commented Jan 5, 2025

Uh oh!

pureiboi commented Jan 5, 2025

Uh oh!

sannya-singal left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dnlopes commented Jan 22, 2025

Uh oh!

sannya-singal commented Jan 27, 2025

Uh oh!

pureiboi commented Jan 27, 2025

Uh oh!

sannya-singal commented Jan 28, 2025

Uh oh!

pureiboi commented Mar 3, 2025

Uh oh!

alexrashed left a comment

Choose a reason for hiding this comment

Uh oh!

pureiboi commented Mar 3, 2025

Uh oh!

sannya-singal left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

sannya-singal commented Mar 4, 2025

Uh oh!

Uh oh!

sannya-singal left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

localstack-bot commented Jan 5, 2025 •

edited

Loading