Skip to content

Ignore TransitEncryptionEnabled in modify when unchanged #171

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jun 25, 2025
Merged

Ignore TransitEncryptionEnabled in modify when unchanged #171

merged 1 commit into from
Jun 25, 2025

Conversation

@gfrey
Copy link
Contributor

@gfrey gfrey commented Jun 24, 2025 •
edited
Loading

If the TransitEncryptionEnabled value is unchanged, but added to a ModifyReplicationGroup call, the ElastiCache API will return an errorMessage saying:

Modification of transit encryption is not supported for access control
enabled clusters.

This is "fixed" by not setting it, or setting it to nil.

Fixes aws-controllers-k8s/community#2536

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

Ignore TransitEncryptionEnabled in modify when unchanged
d0e3ea7 
If the `TransitEncryptionEnabled` value is unchanged, but added to a
ModifyReplicationGroup call, the ElastiCache API will return an
errorMessage saying:

> Modification of transit encryption is not supported for access control
> enabled clusters.

This is "fixed" by not setting it, or setting it to nil.

Fixes #2536
@ack-prow ack-prow bot requested review from a-hilaly and michaelhtm June 24, 2025 11:50
@ack-prow 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 Jun 24, 2025
@ack-prow
Copy link

ack-prow bot commented Jun 24, 2025

Hi @gfrey. 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.

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.

michaelhtm
michaelhtm reviewed Jun 24, 2025
View reviewed changes
Copy link
Member

@michaelhtm michaelhtm left a comment

Choose a reason for hiding this comment

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

This looks great @gfrey 👍
/ok-to-test

templates/hooks/replication_group/sdk_update_post_build_request.go.tpl
}
}
}
}
Copy link
Member

@michaelhtm michaelhtm Jun 24, 2025

Choose a reason for hiding this comment

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

👍

@ack-prow 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 Jun 24, 2025
@ack-prow
Copy link

ack-prow bot commented Jun 24, 2025

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

Test name Commit Details Required Rerun command
elasticache-verify-attribution d0e3ea7 link false /test elasticache-verify-attribution

Full PR test history. Your PR dashboard.

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.

@michaelhtm
Copy link
Member

michaelhtm commented Jun 25, 2025

Thanks @gfrey
would you like to trigger a release PR?
/lgtm

@ack-prow ack-prow bot added the lgtm Indicates that a PR is ready to be merged. label Jun 25, 2025
@ack-prow
Copy link

ack-prow bot commented Jun 25, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: gfrey, michaelhtm

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

The pull request process is described here

Needs approval from an approver in each of these files:

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

@ack-prow ack-prow bot added the approved label Jun 25, 2025
@ack-prow ack-prow bot merged commit 97fb6b2 into aws-controllers-k8s:main Jun 25, 2025
6 of 7 checks passed
@gfrey gfrey deleted the issue-2536/transit-encryption-enabled branch June 26, 2025 06:19
@gfrey
Copy link
Contributor Author

gfrey commented Jun 26, 2025

@michaelhtm Thanks for review and merge! I created the release PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@michaelhtm michaelhtm michaelhtm left review comments

@a-hilaly a-hilaly Awaiting requested review from a-hilaly

Assignees

@michaelhtm michaelhtm

Labels

approved lgtm Indicates that a PR is ready to be merged. ok-to-test Indicates a non-member PR verified by an org member that is safe to test.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

ElastiCache ReplicationGroup: Modification of transit encryption is not supported for access control enabled clusters.

2 participants

@gfrey @michaelhtm