feat: add ReplicaKey support for KMS key replication by sandeepbathina · Pull Request #112 · aws-controllers-k8s/kms-controller

sandeepbathina · 2025-10-28T16:49:03Z

Issue #, if available:
aws-controllers-k8s/community#2664

Description of changes:
Add support for ReplicaKey resource to enable multi-region key management

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

ack-prow · 2025-10-28T16:49:07Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: sandeepbathina
Once this PR has been reviewed and has the lgtm label, please assign michaelhtm for approval by writing /assign @michaelhtm in a comment. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

ack-prow · 2025-10-28T16:49:14Z

Hi @sandeepbathina. Thanks for your PR.

I'm waiting for a aws-controllers-k8s member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

knottnt · 2025-10-28T17:16:28Z

/ok-to-test

…tion

sandeepbathina · 2025-10-28T21:28:01Z

/retest

sandeepbathina · 2025-10-29T20:00:27Z

/retest

sandeepbathina · 2025-10-29T20:03:34Z

/retest

sandeepbathina · 2025-10-29T20:21:51Z

/retest

sandeepbathina · 2025-10-29T23:52:26Z

/retest

sandeepbathina · 2025-10-30T18:59:38Z

/retest

knottnt

Thanks for the contribution @sandeepbathina!

knottnt · 2025-10-30T20:37:40Z

+    find_operation:
+      custom_method_name: customFind
+    update_operation:
+      custom_method_name: updateNotSupported


Q: Do ReplicaKeys support updating tags?

Similarly, from the ReplicateKey documentation it sounds like the Policy field is not replicated from the primary key. Does the PutKeyPolicy operation apply to replica keys as well?

Yes, PutKeyPolicy works on replica keys. Added support for policy updates via customUpdate

knottnt · 2025-10-30T21:14:37Z

+
+                  Regex Pattern: `^[\u0009\u000A\u000D\u0020-\u00FF]+$`
+                type: string
+              replicaTags:


This seems to be duplicating the spec.tags field. If this is due to a mismatch between the input and output field name we may want to configure a rename of the output field in the generator.yaml.

Docs on using field renames

Fixed! added output field rename in generator.yaml to avoid duplication.

knottnt · 2025-10-30T21:15:28Z

+                    format: date-time
+                    type: string
+                type: object
+              replicaPolicy:


This seems to be duplicating the spec.policy field. May need to rename the output field here as well.

Fixed by adding output field rename.

knottnt · 2025-10-30T23:39:14Z

+class TestReplicaKey:
+  """Test suite for ReplicaKey resource"""
+
+  def test_create_delete_replica_key(self, simple_replica_key):


This test should also check the state of the AWS resource out of band from ACK to validate that desired state is being correctly propagated. There should be a kms_client pytest fixture defined in tests/e2e/conftest.py.

Thanks! added out of band validation using replica_kms_client which validates the AWS resource state directly. Used replica_kms_client instead of the kms_client fixture because the replica key exists in a different region. Let me know if I need to follow any other approach.

knottnt · 2025-11-10T18:39:43Z

/hold

knottnt

@sandeepbathina Thanks for the contribution. Testing these changes locally I'm seeing some unexpected behavior with the Read, Update, and Delete calls interacting with the primary key instead of the replica.

knottnt · 2025-11-06T22:18:40Z

+  build_date: "2025-10-31T03:59:17Z"
+  build_hash: eaabefb6bd7b2be8a1baf4478f22b3310e6921c8
+  go_version: go1.25.1
+  version: v0.52.0-6-geaabefb-dirty


It looks like there were some changes made to the code generator in the most recent build. If they aren't needed could you rebuild with a clean version of the code-generator?

knottnt · 2025-11-10T18:45:44Z

+
+	var resp *svcsdk.ReplicateKeyOutput
+	_ = resp
+	resp, err = rm.sdkapi.ReplicateKey(ctx, input)


Looking at the documentation for ReplicateKey I think the way that API interacts with AWS regions require some extra work and possibly some runtime/code-generation improvements to properly model the replica key as its own resource. ReplicateKey needs to be called against the same region as the primary key. However, the resulting region is created in another AWS region. This means that the Read, Update, and Delete calls need to performed in the replica region. So, the ACK controller needs configure it's SDK client region specified by Spec.replicaRegion for these operations.

Testing this PR locally I'm seeing the Create operation successfully replicate the specified primary key. However, other operations continue to interact with the primary key. This appears to happen because the replica and primary keys share the same KeyID.

ack-prow · 2026-02-20T20:59:53Z

@sandeepbathina: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
kms-verify-attribution	`eb4057e`	link	false	`/test kms-verify-attribution`
kms-verify-code-gen	`eb4057e`	link	false	`/test kms-verify-code-gen`
kms-crd-compat-check	`eb4057e`	link	true	`/test kms-crd-compat-check`

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

fix formatting and test cases

d71b97a

ack-prow Bot requested review from a-hilaly and jlbutler October 28, 2025 16:49

ack-prow Bot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Oct 28, 2025

fix replica test case

d259f49

ack-prow Bot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Oct 28, 2025

Sandeep Bathina added 5 commits October 28, 2025 14:24

test: simplify ReplicaKey e2e tests to use boto3 for primary key crea…

7a4dd0c

…tion

test: simplify ReplicaKey e2e tests to use boto3 for primary key crea…

21bfc4a

…tion

Implement sdkFind for ReplicaKey using DescribeKey API

14d657f

Fix ReplicaKey sdkFind to use replica key ID, not primary key ID

e53e637

Fix ReplicaKey sdkFind to use replica key ID, not primary key ID

16eae63

Sandeep Bathina added 2 commits October 29, 2025 14:27

Use custom_method_name pattern for ReplicaKey read operation

f8af093

regenrate code with field name find_operation

3d3d086

knottnt reviewed Oct 30, 2025

View reviewed changes

Sandeep Bathina added 4 commits October 30, 2025 22:26

Fix ReplicaKey to use ACK patterns and support tag/policy updates

31b5e1b

Fix ReplicaKey to use ACK patterns and support tag/policy updates

e70a2b2

Fix ReplicaKey to use ACK patterns and support tag/policy updates

9d7be0b

add back slices

eb4057e

sandeepbathina requested a review from knottnt October 31, 2025 04:14

ack-prow Bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 10, 2025

knottnt reviewed Nov 10, 2025

View reviewed changes

Conversation

sandeepbathina commented Oct 28, 2025

Uh oh!

ack-prow Bot commented Oct 28, 2025

Uh oh!

ack-prow Bot commented Oct 28, 2025

Uh oh!

knottnt commented Oct 28, 2025

Uh oh!

sandeepbathina commented Oct 28, 2025

Uh oh!

sandeepbathina commented Oct 29, 2025

Uh oh!

sandeepbathina commented Oct 29, 2025

Uh oh!

sandeepbathina commented Oct 29, 2025

Uh oh!

sandeepbathina commented Oct 29, 2025

Uh oh!

sandeepbathina commented Oct 30, 2025

Uh oh!

knottnt 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!

knottnt commented Nov 10, 2025

Uh oh!

knottnt 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!

ack-prow Bot commented Feb 20, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants