fix(RecordSet): make adoption id field optional in pre/post annotation hooks - #121
fix(RecordSet): make adoption id field optional in pre/post annotation hooks#121mahasiva-amazon wants to merge 2 commits into
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: mahasiva-amazon The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
ack-prow
Bot
added
the
needs-ok-to-test
|
Hi @mahasiva-amazon. 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 Regular contributors should join the org to skip this step. Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions 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-sigs/prow repository. |
The id field in adoption annotations maps to Status.ID (the Route53 ChangeInfo ID from the last change operation). Pre-existing records have no associated ChangeInfo ID, causing adoption to fail with 'required field missing: id'. Since resource.go is generated, the fix is implemented via two hooks: - pre_populate_resource_from_annotation: injects an empty id sentinel into the fields map when id is absent, allowing the generated required-check to pass - post_populate_resource_from_annotation: clears Status.ID when it holds the empty sentinel, so sdkFind skips the ChangeInfo lookup for adopted records Also guard syncStatus against empty-string Status.ID via the sdk_read_many_post_set_output template to prevent spurious ChangeInfo lookups. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Reproduces the exact CX scenario: pre-existing record adopted via adoption-fields annotation without id field. Proves bug on upstream (terminal error 'required field missing: id') and fix (Synced=True, Status.ID nil). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
ff9711f to
b76182e
Compare
|
Branch rebased onto upstream |
1 similar comment
|
Branch rebased onto upstream |
1 participant
Fixes aws-controllers-k8s/community#2981
When adopting a pre-existing Route53 record via the
adoption-fieldsannotation, customers omit theidfield because pre-existing records have no associated ChangeInfo ID. The generatedPopulateResourceFromAnnotationtreatsidas required and returns a terminal error:required field missing: id.Fix
Add
pre_populate_resource_from_annotationhook that injects an emptyidsentinel when the field is absent from the annotation map, allowing the required-field check to pass. Apost_populate_resource_from_annotationhook then clearsStatus.IDso the ChangeInfo lookup insdkFindis skipped for adopted records with no ChangeInfo.Testing
resource_test.goadoption-fieldsannotation (noid) reproduced terminal error on upstream, confirmed Synced=True with this fixFixes: R53-2