KMS: Add list-key-rotations flag #12853

demaj · 2025-07-11T00:07:09Z

Motivation

Adding support for ListKeyRotations. Mentioned in #12342.

Changes

Implementation of list-key-rotations feature for keys that may have multiple key materials associated with them.

As per AWS's, list-key-rotation is only allowed for:

symmetric keys
Imported keys from single-region-only

TODO

What's left to do:

Add support for imported EXTERNAL origin single-region keys

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.

localstack-bot · 2025-07-11T00:07:19Z

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

demaj · 2025-07-11T00:08:16Z

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

demaj · 2025-07-17T03:04:54Z

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

+                    f"failed to satisfy constraint: Member must satisfy enum value set: [ALL_KEY_MATERIAL, ROTATIONS_ONLY]"
+                )
+
+            if key.metadata["KeySpec"] != KeySpec.SYMMETRIC_DEFAULT:


As per AWS docs, for asymmetric keys when IncludeKeyMaterial is not provided we simply return the Rotations as empty [].
See here: https://docs.aws.amazon.com/kms/latest/developerguide/symm-asymm-compare.html#key-type-table

question: From the test test_list_key_rotations_unsupported_key_types seems like it always raises an error for asymmetric keys, could you explain this a bit more here why it should be []?

Based on the AWS Docs, and the behavior of the aws-cli, we return [] for unsupported KMS keys. But, whenever the attribute IncludeKeyMaterial is provided, it raises an error. See here: https://docs.aws.amazon.com/kms/latest/developerguide/key-state.html#key-state-table
In the test_list_key_rotations_unsupported_key_types we provide the IncludeKeyMaterial parameter.

demaj · 2025-07-17T03:06:30Z

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

+                if include_key_material == IncludeKeyMaterial.ALL_KEY_MATERIAL:
+                    rotation_history.append(rotation_entry)
+                else:  # Default ROTATIONS_ONLY
+                    if rotation.rotation_type in ["AUTOMATIC", "ON_DEMAND"]:


A KMS key on its creation doesn't have any rotation_type.

I suggest its worth writing a test case to validate the behavior in case of automatically rotated keys, what do you think?

I agree, it makes sense to add one. I'll do it. Thanks!

demaj · 2025-07-17T03:18:32Z

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

+            initial_rotation = KeyRotationEntry(
+                key_id=self.metadata["Arn"],
+                key_material_state="CURRENT",
+                key_material_id=long_uid(),  # FIXME: a more appropriate KMS output


Currently relying on uuid, open to suggestions.

key_material_id should follow the constraints as mentioned in AWS documentation here: https://docs.aws.amazon.com/kms/latest/APIReference/API_RotationsListEntry.html#KMS-Type-RotationsListEntry-KeyMaterialId.

Hint: try playing around generating random bytes from os, you could find several references for it in the codebase 😉 and then try to convert it to a 64-character hexadecimal string.

demaj · 2025-07-17T03:25:01Z

tests/aws/services/kms/test_kms.snapshot.json

I'm getting "Truncated": false here. Not entirely sure why it's being included, I do not see it on my awscli output; there is a discrepancy between AWS Docs and AWS CLI.
Removed them manually from this file (I know we're not supposed to manually edit it). Let me know if I should re-add it and make changes on the function's response payload.

Rightly said 👍 I wouldn't manually make any changes in snapshot.

What i suggest here is to simple skip verification of Truncated in the test and add a TODO in there explaining the same discrepancy.

sannya-singal

Thanks for tackling this issue 🚀 The test coverage and your understanding of the issue looks decent 🙂 🎉
I have added a few comments and questions regarding the use of default classes, snapshots, testing, validations, rotation of automatic keys, etc.. Please feel free to reach out if anything is unclear, happy to clarify and help 🙂

sannya-singal · 2025-07-21T05:02:59Z

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

+@dataclass
+class KeyRotationEntry:
+    key_id: str
+    key_material: bytes
+    key_material_id: str
+    key_material_state: str
+    rotation_date: datetime.datetime | None = None
+    rotation_type: str | None = None
+    key_material_description: str | None = None
+    expiration_model: str | None = None
+    import_state: str | None = None
+    valid_to: str | None = None


Did you consider using the predefined class in __init__.py file which already includes this type RotationsListEntry?

I don't know how I missed it. That is a valid point, I will refactor and use the classes from __init__.py
Thank you!

sannya-singal · 2025-07-21T05:03:15Z

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

+        if self.supports_rotation():
+            initial_rotation = KeyRotationEntry(
+                key_id=self.metadata["Arn"],
+                key_material_state="CURRENT",


Instead of using hardcoded values, import the class defined in __init__.py file which already includes the types you have defined here.
The file is automatically generated, you can find more details here: https://github.com/localstack/localstack/blob/main/docs/localstack-concepts/README.md

sannya-singal · 2025-07-21T05:03:19Z

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

+            initial_rotation = KeyRotationEntry(
+                key_id=self.metadata["Arn"],
+                key_material_state="CURRENT",
+                key_material_id=long_uid(),  # FIXME: a more appropriate KMS output


key_material_id should follow the constraints as mentioned in AWS documentation here: https://docs.aws.amazon.com/kms/latest/APIReference/API_RotationsListEntry.html#KMS-Type-RotationsListEntry-KeyMaterialId.

Hint: try playing around generating random bytes from os, you could find several references for it in the codebase 😉 and then try to convert it to a 64-character hexadecimal string.

sannya-singal · 2025-07-21T05:03:27Z

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

+        on_demand_count = sum(
+            1 for rotation in self.key_rotations if rotation.rotation_type == "ON_DEMAND"
+        )
+
+        if on_demand_count >= ON_DEMAND_ROTATION_LIMIT:


What happens if the rotation type is AUTOMATIC here?

sannya-singal · 2025-07-21T05:04:30Z

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

+                    f"failed to satisfy constraint: Member must satisfy enum value set: [ALL_KEY_MATERIAL, ROTATIONS_ONLY]"
+                )
+
+            if key.metadata["KeySpec"] != KeySpec.SYMMETRIC_DEFAULT:


question: From the test test_list_key_rotations_unsupported_key_types seems like it always raises an error for asymmetric keys, could you explain this a bit more here why it should be []?

sannya-singal · 2025-07-21T05:07:35Z

tests/aws/services/kms/test_kms.py

+        [
+            ("RSA_2048", "SIGN_VERIFY", "ValidationException"),
+            ("RSA_4096", "ENCRYPT_DECRYPT", "ValidationException"),
+            ("ECC_NIST_P256", "SIGN_VERIFY", "ValidationException"),
+            ("HMAC_256", "GENERATE_VERIFY_MAC", "ValidationException"),


Can you add an id to these parameterised tests to make it more readable?

sannya-singal · 2025-07-21T05:07:39Z

tests/aws/services/kms/test_kms.py

+        key_id = kms_create_key(KeyUsage="ENCRYPT_DECRYPT", KeySpec="SYMMETRIC_DEFAULT")["KeyId"]
+
+        snapshot.add_transformer(
+            snapshot.transform.key_value("KeyMaterialId", reference_replacement=False)


Instead of adding it in every test, we should move the transformer to transformer_utility.py.

sannya-singal · 2025-07-21T05:07:41Z

tests/aws/services/kms/test_kms.py

+        with pytest.raises(ClientError) as e:
+            aws_client.kms.list_key_rotations(KeyId=key_id, IncludeKeyMaterial="INVALID_VALUE")


Since this usecase doesn't really belong to the intent of the test named test_list_key_rotations_include_key_material, we should write a separate test for this part.

sannya-singal · 2025-07-21T05:07:47Z

tests/aws/services/kms/test_kms.snapshot.json

Rightly said 👍 I wouldn't manually make any changes in snapshot.

What i suggest here is to simple skip verification of Truncated in the test and add a TODO in there explaining the same discrepancy.

sannya-singal · 2025-07-21T05:07:50Z

tests/aws/services/kms/test_kms.snapshot.json

+  "tests/aws/services/kms/test_kms.py::TestKMS::test_list_key_rotations_unsupported_key_types[RSA_2048-SIGN_VERIFY-UnsupportedOperationException]": {
+    "recorded-date": "16-07-2025, 15:59:30",
+    "recorded-content": {}
+  },
+  "tests/aws/services/kms/test_kms.py::TestKMS::test_list_key_rotations_unsupported_key_types[RSA_4096-ENCRYPT_DECRYPT-UnsupportedOperationException]": {
+    "recorded-date": "16-07-2025, 15:59:31",
+    "recorded-content": {}
+  },
+  "tests/aws/services/kms/test_kms.py::TestKMS::test_list_key_rotations_unsupported_key_types[ECC_NIST_P256-SIGN_VERIFY-UnsupportedOperationException]": {
+    "recorded-date": "16-07-2025, 15:59:31",
+    "recorded-content": {}
+  },
+  "tests/aws/services/kms/test_kms.py::TestKMS::test_list_key_rotations_unsupported_key_types[HMAC_256-GENERATE_VERIFY_MAC-UnsupportedOperationException]": {
+    "recorded-date": "16-07-2025, 15:59:32",
+    "recorded-content": {}
+  },


Are these the leftovers? Because i cant see related tests for UnsupportedOperationException specifically?

sannya-singal · 2025-08-25T07:55:22Z

Hi @demaj! Thanks again for your contribution. Would you like to continue working on this PR? If you need any help to move it forward, please let me know 🙂

demaj · 2025-08-26T08:39:31Z

Hi @sannya-singal, sincere apologies, I was not able to actively work on this. I will try to find some time during this week, is it okay with you? I would love to wrap this PR up, and have my first contribution to Localstack! Thank you! 😊

sannya-singal · 2025-08-27T10:03:41Z

No worries at all 🙂 Glad want to wrap it up, please go ahead and work on it 🎉 Excited for your first contribution! 🚀

demaj added 2 commits July 10, 2025 21:49

KMS: add list-key-rotations

55e1ead

KMS: fix tests for list-key-rotations

2c2d89f

localstack-bot reviewed Jul 11, 2025

View reviewed changes

localstack-bot added a commit that referenced this pull request Jul 11, 2025

@demaj has signed the CLA in #12853

3ccf9e5

viren-nadkarni added the semver: minor Non-breaking changes which can be included in minor releases, but not in patch releases label Jul 14, 2025

viren-nadkarni self-assigned this Jul 14, 2025

demaj added 2 commits July 16, 2025 18:04

KMS: enhance tests for list-key-rotations

5ec95ef

KMS: drop EXTERNAL-origin support for ListKeyRotations

fd7df95

demaj force-pushed the kms-add-list-key-rotations branch from 6570477 to fd7df95 Compare July 17, 2025 02:57

demaj marked this pull request as ready for review July 17, 2025 02:58

demaj requested a review from sannya-singal as a code owner July 17, 2025 02:58

demaj commented Jul 17, 2025

View reviewed changes

sannya-singal requested changes Jul 21, 2025

View reviewed changes

sannya-singal added the aws:kms AWS Key Management Service label Jul 22, 2025

alexrashed modified the milestones: 4.8, Playground Jul 23, 2025

viren-nadkarni assigned sannya-singal and unassigned viren-nadkarni Aug 26, 2025

		with pytest.raises(ClientError) as e:
		aws_client.kms.list_key_rotations(KeyId=key_id, IncludeKeyMaterial="INVALID_VALUE")

Uh oh!

KMS: Add list-key-rotations flag #12853

Are you sure you want to change the base?

KMS: Add list-key-rotations flag #12853

Uh oh!

Conversation

demaj commented Jul 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation

Changes

TODO

Uh oh!

localstack-bot left a comment

Choose a reason for hiding this comment

Uh oh!

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

Uh oh!

Uh oh!

demaj commented Jul 11, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sannya-singal left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sannya-singal commented Aug 25, 2025

Uh oh!

demaj commented Aug 26, 2025

Uh oh!

sannya-singal commented Aug 27, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

demaj commented Jul 11, 2025 •

edited

Loading

localstack-bot commented Jul 11, 2025 •

edited

Loading