-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
[TXT Registry] Make the nonce stable when generating delete records #3901
[TXT Registry] Make the nonce stable when generating delete records #3901
Conversation
Hi @Sewci0. Thanks for your PR. I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the 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. |
I'm unsure if and how the change in labels.go should be tested 🤔 /ok-to-test |
f1be914
to
cee0ea2
Compare
/test pull-external-dns-lint |
/retest |
1 similar comment
/retest |
/assign @njuettner |
/lgtm |
/retest |
@Sewci0 If you commit --amend --no-edit and push, it should refresh the failed job. |
4976722
to
9a2b5f2
Compare
@mloiseleur Thanks for looking into it, I had to do a few force pushes but it should be all good now. |
9a2b5f2
to
350c546
Compare
350c546
to
66b5b25
Compare
/lgtm |
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: szuecs 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 |
This PR addresses the issue of the current behavior being incompatible with Route53 when using TXT registry with encryption enabled. AWS requires the
Delete
change request to be called with the exact same record set that is currently present in Route53. However, this is not happening due to the following reasons:labelsMap
when iterating through the TXT labels.Records()
results in the deletion of the nonce from the labels.generateTXTRecord
within ApplyChanges regenerates the nonce again, leading to an entirely different target.To resolve this issue, this PR disables the generation of old-style TXT records and old to new migrations when encryption is enabled. It also ensures that the
txtEncryptionNonce
does not get removed from labels, instead it is ignored when serializing.Nonce generation has been moved outside of the
EncryptText
and the nonce get's added to the record's labels. This is required in order to ensure that the nonce exists on the record when it gets added to the cache.Fixes: #3668
Supersedes: #3808
Checklist