Skip to content

KMS: return NotImplementedError for rotation of imported keys #12932

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

Merged
merged 2 commits into from
Jul 30, 2025

Conversation

sannya-singal
Copy link
Contributor

@sannya-singal sannya-singal commented Jul 30, 2025

Motivation

After the launch of this feature, AWS introduced support for on demand rotation of imported keys i.e. keys with Origin set to EXTERNAL.

At the moment, LocalStack does not support this feature and we return a 400 error with UnsupportedOperationException even though the request itself is valid.

Changes

Raise appropriate error response with error code 501 when rotation of keys is done with imported keys.

Please note, this PR does not aim to fix existing tests and other logic in the code which revolves around assuming that external keys are unsupported, this should be tackled in a separate PR.

@sannya-singal sannya-singal self-assigned this Jul 30, 2025
@sannya-singal sannya-singal added aws:kms AWS Key Management Service semver: patch Non-breaking changes which can be included in patch releases labels Jul 30, 2025
@sannya-singal sannya-singal marked this pull request as ready for review July 30, 2025 11:11
@sannya-singal sannya-singal requested a review from k-a-il July 30, 2025 11:11
Copy link
Contributor

@k-a-il k-a-il left a comment

Choose a reason for hiding this comment

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

LGTM 🚀

@sannya-singal sannya-singal added this to the 4.7 milestone Jul 30, 2025
Copy link

github-actions bot commented Jul 30, 2025

LocalStack Community integration with Pro

  2 files    2 suites   4m 33s ⏱️
709 tests 701 ✅  8 💤 0 ❌
711 runs  701 ✅ 10 💤 0 ❌

Results for commit 40a5ce0.

♻️ This comment has been updated with latest results.

Copy link

github-actions bot commented Jul 30, 2025

Test Results - Preflight, Unit

21 992 tests  ±0   20 258 ✅ ±0   6m 22s ⏱️ -1s
     1 suites ±0    1 734 💤 ±0 
     1 files   ±0        0 ❌ ±0 

Results for commit 40a5ce0. ± Comparison against base commit 8e93d8a.

♻️ This comment has been updated with latest results.

@sannya-singal
Copy link
Contributor Author

The pipeline failed: https://github.com/localstack/localstack/actions/runs/16620811305/job/47024507676?pr=12932 because test_rotate_key_on_demand_raises_error_given_key_with_imported_key_material should be fixed/removed with the introduction of the new feature by AWS. I have skipped it for now.

@sannya-singal sannya-singal added the review: merge when ready Signals to the reviewer that a PR can be merged if accepted label Jul 30, 2025
Copy link

Test Results (amd64) - Acceptance

7 tests  ±0   5 ✅ ±0   3m 14s ⏱️ +8s
1 suites ±0   2 💤 ±0 
1 files   ±0   0 ❌ ±0 

Results for commit 40a5ce0. ± Comparison against base commit 8e93d8a.

Copy link

Test Results (amd64) - Integration, Bootstrap

  5 files    5 suites   17m 48s ⏱️
733 tests 725 ✅  8 💤 0 ❌
739 runs  725 ✅ 14 💤 0 ❌

Results for commit 40a5ce0.

@sannya-singal sannya-singal merged commit dfad92b into main Jul 30, 2025
41 checks passed
@sannya-singal sannya-singal deleted the kms-throw-500 branch July 30, 2025 11:46
@coveralls
Copy link

Coverage Status

coverage: 58.762% (+7.4%) from 51.39%
when pulling 40a5ce0 on kms-throw-500
into 8e93d8a on main.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
aws:kms AWS Key Management Service review: merge when ready Signals to the reviewer that a PR can be merged if accepted semver: patch Non-breaking changes which can be included in patch releases
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants