-
Notifications
You must be signed in to change notification settings - Fork 2.8k
fix(gateway-api): ensure to use only latest generation with HTTPRoutes #5349
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
fix(gateway-api): ensure to use only latest generation with HTTPRoutes #5349
Conversation
Welcome @kashalls! |
Hi @kashalls. 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-sigs/prow repository. |
/ok-to-test |
fix: add unit tests for generation check
/test pull-external-dns-unit-test |
/test pull-external-dns-unit-test |
fix: add unit test for route never being accepted
The code looks ok. Was it tested on real cluster setup? What was the result? |
I'm running it on my home cluster, I can confirm it results in only the newest generation having records generated, instead of all. I have an |
Example logs of flipping it from external to internal: Without this change:
With this change:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
cc @mloiseleur
/retitle fix(gateway-api): ensure to use only latest generation with HTTPRoutes @abursavich any comments on this PR ? |
/retitle fix(gateway-api): ensure to use only latest generation with HTTPRoutes |
Any updates on this? @mloiseleur @abursavich |
Sorry, I still haven't been able to review with as much attention as I would like. The goal of this change makes sense, but my first reaction is that it's actually very dangerous. Erroneously rejecting a route could be worse than erroneously accepting a route update. After a route is updated (e.g. an annotation), its accepted generation will no longer match and ExternalDNS could delete the DNS records. |
That is a good point, the understanding is that External-DNS should only take action on the most up to date parentRefs. The chosen gateway controller is what handles that, but it doesn't specify that the gateway controller is supposed to clean up old instances that don't reflect the spec. @tholinka and I can test this case where the annotations are modified real quick. |
ExternalDNS generates a list of all endpoints from cluster sources then syncs those with the external DNS provider. Unfortunately, it can't necessarily have access to a previous generation of an object's spec that matches the currently accepted generation. If we don't have matching generations when generating the endpoints, the sync could delete the DNS records. The good news is that I believe the issue you're trying to fix was already fixed by #5241, which also checks the spec parentRefs instead of blindly trusting the status parentRefs. |
Thanks @kashalls 👍 |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ivankatliarchuk, mloiseleur 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 |
…o v0.17.0 (#712) This PR contains the following updates: | Package | Update | Change | |---|---|---| | [registry.k8s.io/external-dns/external-dns](https://github.com/kubernetes-sigs/external-dns) | minor | `v0.16.1` -> `v0.17.0` | --- ### Release Notes <details> <summary>kubernetes-sigs/external-dns (registry.k8s.io/external-dns/external-dns)</summary> ### [`v0.17.0`](https://github.com/kubernetes-sigs/external-dns/releases/tag/v0.17.0) [Compare Source](kubernetes-sigs/external-dns@v0.16.1...v0.17.0) #### Global information :information_source: On Pi Hole, support for v6 has been added. Pi Hole v5 support is deprecated and will be removed in a future version :information_source: On OVH, the provider has been heavily rewritten. New ACLs are need. See documentation and PR [#​5143](kubernetes-sigs/external-dns#5143) for details. :information_source: On IPv6, the [proposal 002](https://github.com/kubernetes-sigs/external-dns/blob/master/docs/proposal/002-internal-ipv6-handling-rollback.md) has been implemented, thanks to [@​hjoshi123](https://github.com/hjoshi123) -⚠️ In the next release, the default of this new `expose-internal-ipv6` flag will be set to false⚠️ There is a severe known issue with Active Directory ([#​5240](kubernetes-sigs/external-dns#5240)) since v0.16.0. PR to fix it ([#​5385](kubernetes-sigs/external-dns#5385)) is in review :information_source: The legacy txt-format will be removed in the next minor version ([#​5172](kubernetes-sigs/external-dns#5172)) -⚠️ There is currently no migration script to clean old style txt records. If you need it, if you think you can do it, PRs are welcome. ℹ️ A new Nomad source may be added to external-dns ([#​5284](kubernetes-sigs/external-dns#5284)) - This PR can be merged only after a review and test from real Nomad users #### 🚀 Features - feat: additional template functions by [@​matkam](https://github.com/matkam) in [#​3949](kubernetes-sigs/external-dns#3949) - feat(banner): standardize user agent and output by [@​ivankatliarchuk](https://github.com/ivankatliarchuk) in [#​5154](kubernetes-sigs/external-dns#5154) - feat(chart): Update image to v0.16.1 by [@​stevehipwell](https://github.com/stevehipwell) in [#​5201](kubernetes-sigs/external-dns#5201) - feat(cloudflare): multiple custom hostnames support by [@​mrozentsvayg](https://github.com/mrozentsvayg) in [#​5239](kubernetes-sigs/external-dns#5239) - feat(helm): allow extraArgs to also be a map enabling overrides of individual values by [@​frittentheke](https://github.com/frittentheke) in [#​5293](kubernetes-sigs/external-dns#5293) - feat: IDNA awareness in the zone finder by [@​lexisother](https://github.com/lexisother) in [#​5147](kubernetes-sigs/external-dns#5147) - feat(ovh): major rewriting of the provider by [@​rbeuque74](https://github.com/rbeuque74) in [#​5143](kubernetes-sigs/external-dns#5143) - feat(pihole): add optional support for v6 by [@​tJouve](https://github.com/tJouve) in [#​5226](kubernetes-sigs/external-dns#5226) - feat(pihole): add support for IPv6 Dual format by [@​tJouve](https://github.com/tJouve) in [#​5253](kubernetes-sigs/external-dns#5253) - feat(source): optional exclusion of unschedulable nodes by [@​Hayajiro](https://github.com/Hayajiro) in [#​5045](kubernetes-sigs/external-dns#5045) - feat(source): optional expose of nodes internal ipv6 by [@​hjoshi123](https://github.com/hjoshi123) in [#​5192](kubernetes-sigs/external-dns#5192) #### 🐛 Bug fixes - fix(aws): typo on route53 IAM by [@​tico24](https://github.com/tico24) in [#​5197](kubernetes-sigs/external-dns#5197) - fix(chart): add missing types for empty values by [@​t3mi](https://github.com/t3mi) in [#​5207](kubernetes-sigs/external-dns#5207) - fix(cloudflare): custom hostnames edge-cases causing duplicates by [@​mrozentsvayg](https://github.com/mrozentsvayg) in [#​5183](kubernetes-sigs/external-dns#5183) - fix(cloudflare): regional hostnames by [@​vflaux](https://github.com/vflaux) in [#​5175](kubernetes-sigs/external-dns#5175) - fix(Gateway API): ensure generation match by [@​davidwin93](https://github.com/davidwin93) in [#​5241](kubernetes-sigs/external-dns#5241) - fix(gateway-api): ensure to use only latest generation with HTTPRoutes by [@​kashalls](https://github.com/kashalls) in [#​5349](kubernetes-sigs/external-dns#5349) - fix(helm): added missing schema values by [@​ivankatliarchuk](https://github.com/ivankatliarchuk) in [#​5228](kubernetes-sigs/external-dns#5228) - fix(helm): update helm schema by [@​semnell](https://github.com/semnell) in [#​5297](kubernetes-sigs/external-dns#5297) - fix(log testing): re-use logger library testing functionality by [@​ivankatliarchuk](https://github.com/ivankatliarchuk) in [#​5368](kubernetes-sigs/external-dns#5368) - fix(node): logger test fixed by [@​ivankatliarchuk](https://github.com/ivankatliarchuk) in [#​5232](kubernetes-sigs/external-dns#5232) - fix(ovh): handling capitalized DNS records + prevent panic by [@​rbeuque74](https://github.com/rbeuque74) in [#​5390](kubernetes-sigs/external-dns#5390) - fix(webhook): api json object plan.Changes case by [@​ivankatliarchuk](https://github.com/ivankatliarchuk) in [#​5355](kubernetes-sigs/external-dns#5355) - fix(zonefinder): handle underscores in dns records by [@​arthlr](https://github.com/arthlr) in [#​5281](kubernetes-sigs/external-dns#5281) #### 📝 Documentation - docs(contributing): add conventional commits by [@​mloiseleur](https://github.com/mloiseleur) in [#​5333](kubernetes-sigs/external-dns#5333) - docs(proposal): externaldns api graduation to beta by [@​ivankatliarchuk](https://github.com/ivankatliarchuk) in [#​5079](kubernetes-sigs/external-dns#5079) - docs(rfc2136): fix env variable in the guideline by [@​riupie](https://github.com/riupie) in [#​5352](kubernetes-sigs/external-dns#5352) - docs(tutorials): add IONOS Cloud setup tutorial for ExternalDNS by [@​smilutinovic-ionos](https://github.com/smilutinovic-ionos) in [#​5364](kubernetes-sigs/external-dns#5364) - docs(typo): grcp → grpc by [@​octo](https://github.com/octo) in [#​5231](kubernetes-sigs/external-dns#5231) - docs: update link to Anexia webhook provider by [@​mloiseleur](https://github.com/mloiseleur) in [#​5244](kubernetes-sigs/external-dns#5244) - docs: update rfc2136 by [@​BasJ93](https://github.com/BasJ93) in [#​5325](kubernetes-sigs/external-dns#5325) #### 📦 Others - Add Yandex Cloud Webhook by [@​ismailbaskin](https://github.com/ismailbaskin) in [#​5190](kubernetes-sigs/external-dns#5190) - chore: add se for nlb, alb in thailand region by [@​xshot9011](https://github.com/xshot9011) in [#​5200](kubernetes-sigs/external-dns#5200) - chore: fix typo on txtOwnerId comment/description by [@​lanandra](https://github.com/lanandra) in [#​5351](kubernetes-sigs/external-dns#5351) - chore(ci): fix random data race failure on source/node test by [@​mloiseleur](https://github.com/mloiseleur) in [#​5268](kubernetes-sigs/external-dns#5268) - chore(ci): improve release script by [@​mloiseleur](https://github.com/mloiseleur) in [#​5394](kubernetes-sigs/external-dns#5394) - chore(ci): update linter to v2.0.2 by [@​mloiseleur](https://github.com/mloiseleur) in [#​5246](kubernetes-sigs/external-dns#5246) - chore(code-cleanup): move logic away from main.go add tests by [@​ivankatliarchuk](https://github.com/ivankatliarchuk) in [#​5222](kubernetes-sigs/external-dns#5222) - chore(code): improve some tests + re-order sources flags CLI by [@​mloiseleur](https://github.com/mloiseleur) in [#​5288](kubernetes-sigs/external-dns#5288) - chore(code-quality): added lint checks by [@​ivankatliarchuk](https://github.com/ivankatliarchuk) in [#​5318](kubernetes-sigs/external-dns#5318) - chore(code-quality): providers linter warnings fixes by [@​ivankatliarchuk](https://github.com/ivankatliarchuk) in [#​5320](kubernetes-sigs/external-dns#5320) - chore(code-quality): refactoring and linter fixes by [@​ivankatliarchuk](https://github.com/ivankatliarchuk) in [#​5374](kubernetes-sigs/external-dns#5374) - chore(code-quality): webhook increase code coverage by [@​ivankatliarchuk](https://github.com/ivankatliarchuk) in [#​5337](kubernetes-sigs/external-dns#5337) - chore(crd): use conventional paths and update controller-gen to v0.17.2 by [@​mloiseleur](https://github.com/mloiseleur) in [#​5287](kubernetes-sigs/external-dns#5287) - chore(dependencies): update toools versions by [@​ivankatliarchuk](https://github.com/ivankatliarchuk) in [#​5252](kubernetes-sigs/external-dns#5252) - chore(deps): build with go 1.24.2 by [@​mloiseleur](https://github.com/mloiseleur) in [#​5306](kubernetes-sigs/external-dns#5306) - chore(deps): bump renovatebot/github-action from 41.0.14 to 41.0.16 in the dev-dependencies group by [@​app/dependabot](https://github.com/app/dependabot) in [#​5188](kubernetes-sigs/external-dns#5188) - chore(deps): bump renovatebot/github-action from 41.0.16 to 41.0.17 in the dev-dependencies group by [@​app/dependabot](https://github.com/app/dependabot) in [#​5210](kubernetes-sigs/external-dns#5210) - chore(deps): bump the dev-dependencies group across 1 directory with 14 updates by [@​app/dependabot](https://github.com/app/dependabot) in [#​5346](kubernetes-sigs/external-dns#5346) - chore(deps): bump the dev-dependencies group across 1 directory with 14 updates by [@​app/dependabot](https://github.com/app/dependabot) in [#​5382](kubernetes-sigs/external-dns#5382) - chore(deps): bump the dev-dependencies group across 1 directory with 17 updates by [@​app/dependabot](https://github.com/app/dependabot) in [#​5238](kubernetes-sigs/external-dns#5238) - chore(deps): bump the dev-dependencies group across 1 directory with 19 updates by [@​app/dependabot](https://github.com/app/dependabot) in [#​5262](kubernetes-sigs/external-dns#5262) - chore(deps): bump the dev-dependencies group across 1 directory with 20 updates by [@​app/dependabot](https://github.com/app/dependabot) in [#​5211](kubernetes-sigs/external-dns#5211) - chore(deps): bump the dev-dependencies group across 1 directory with 21 updates by [@​mloiseleur](https://github.com/mloiseleur) in [#​5313](kubernetes-sigs/external-dns#5313) - chore(deps): bump the dev-dependencies group across 1 directory with 2 updates by [@​app/dependabot](https://github.com/app/dependabot) in [#​5247](kubernetes-sigs/external-dns#5247) - chore(deps): bump the dev-dependencies group across 1 directory with 2 updates by [@​app/dependabot](https://github.com/app/dependabot) in [#​5301](kubernetes-sigs/external-dns#5301) - chore(deps): bump the dev-dependencies group across 1 directory with 2 updates by [@​app/dependabot](https://github.com/app/dependabot) in [#​5323](kubernetes-sigs/external-dns#5323) - chore(deps): code cleanup, pkg/errors is archived by [@​ivankatliarchuk](https://github.com/ivankatliarchuk) in [#​5335](kubernetes-sigs/external-dns#5335) - chore(deps): switch to goccy yaml by [@​Raffo](https://github.com/Raffo) in [#​5292](kubernetes-sigs/external-dns#5292) - chore(deps): update linter to v2.1.x by [@​mloiseleur](https://github.com/mloiseleur) in [#​5366](kubernetes-sigs/external-dns#5366) - chore(fqdn-template): fqdn templating move to specific folder and update documentation by [@​ivankatliarchuk](https://github.com/ivankatliarchuk) in [#​5354](kubernetes-sigs/external-dns#5354) - chore(github): add a release template by [@​ivankatliarchuk](https://github.com/ivankatliarchuk) in [#​5324](kubernetes-sigs/external-dns#5324) - chore(helm): add validation for prefix and suffix and capture regression by [@​ivankatliarchuk](https://github.com/ivankatliarchuk) in [#​5250](kubernetes-sigs/external-dns#5250) - chore(openstack designate)!: remove in-tree provider by [@​mloiseleur](https://github.com/mloiseleur) in [#​5126](kubernetes-sigs/external-dns#5126) - chore(providers): rename custom TTL constants to defaultTTL by [@​ivankatliarchuk](https://github.com/ivankatliarchuk) in [#​5312](kubernetes-sigs/external-dns#5312) - chore: Release Helm chart v1.16.1 by [@​stevehipwell](https://github.com/stevehipwell) in [#​5270](kubernetes-sigs/external-dns#5270) - chore(release): updates kustomize & docs with v0.16.1 by [@​mloiseleur](https://github.com/mloiseleur) in [#​5184](kubernetes-sigs/external-dns#5184) - chore(source): code cleanup by [@​ivankatliarchuk](https://github.com/ivankatliarchuk) in [#​5304](kubernetes-sigs/external-dns#5304) - chore(webhook): bump cenkalti/backoff version by [@​ivankatliarchuk](https://github.com/ivankatliarchuk) in [#​5342](kubernetes-sigs/external-dns#5342) - test: add tests for cloudflare provider by [@​natitomattis](https://github.com/natitomattis) in [#​5248](kubernetes-sigs/external-dns#5248) - test(log): execute on multiple platforms by [@​ivankatliarchuk](https://github.com/ivankatliarchuk) in [#​5370](kubernetes-sigs/external-dns#5370) - test(log): fix random race detection by [@​ivankatliarchuk](https://github.com/ivankatliarchuk) in [#​5273](kubernetes-sigs/external-dns#5273) - test(source): cover unhappy paths by [@​linoleparquet](https://github.com/linoleparquet) in [#​5369](kubernetes-sigs/external-dns#5369) - test(source): fix data race on node_test by [@​mloiseleur](https://github.com/mloiseleur) in [#​5334](kubernetes-sigs/external-dns#5334) - test(source/pod): improve code coverage by [@​ivankatliarchuk](https://github.com/ivankatliarchuk) in [#​5378](kubernetes-sigs/external-dns#5378) - test(tlsconfig): add unit tests by [@​linoleparquet](https://github.com/linoleparquet) in [#​5381](kubernetes-sigs/external-dns#5381) - test(zone_filter): improve coverage from 66.7% to 100% by [@​upsaurav12](https://github.com/upsaurav12) in [#​5388](kubernetes-sigs/external-dns#5388) #### 📦 Docker Image docker pull registry.k8s.io/external-dns/external-dns:v0.17.0 #### New Contributors - [@​ismailbaskin](https://github.com/ismailbaskin) made their first contribution in kubernetes-sigs/external-dns#5190 - [@​lexisother](https://github.com/lexisother) made their first contribution in kubernetes-sigs/external-dns#5147 - [@​tico24](https://github.com/tico24) made their first contribution in kubernetes-sigs/external-dns#5197 - [@​t3mi](https://github.com/t3mi) made their first contribution in kubernetes-sigs/external-dns#5207 - [@​octo](https://github.com/octo) made their first contribution in kubernetes-sigs/external-dns#5231 - [@​xshot9011](https://github.com/xshot9011) made their first contribution in kubernetes-sigs/external-dns#5200 - [@​tJouve](https://github.com/tJouve) made their first contribution in kubernetes-sigs/external-dns#5226 - [@​Hayajiro](https://github.com/Hayajiro) made their first contribution in kubernetes-sigs/external-dns#5045 - [@​davidwin93](https://github.com/davidwin93) made their first contribution in kubernetes-sigs/external-dns#5241 - [@​vflaux](https://github.com/vflaux) made their first contribution in kubernetes-sigs/external-dns#5175 - [@​arthlr](https://github.com/arthlr) made their first contribution in kubernetes-sigs/external-dns#5281 - [@​semnell](https://github.com/semnell) made their first contribution in kubernetes-sigs/external-dns#5297 - [@​BasJ93](https://github.com/BasJ93) made their first contribution in kubernetes-sigs/external-dns#5325 - [@​natitomattis](https://github.com/natitomattis) made their first contribution in kubernetes-sigs/external-dns#5248 - [@​riupie](https://github.com/riupie) made their first contribution in kubernetes-sigs/external-dns#5352 - [@​lanandra](https://github.com/lanandra) made their first contribution in kubernetes-sigs/external-dns#5351 - [@​smilutinovic-ionos](https://github.com/smilutinovic-ionos) made their first contribution in kubernetes-sigs/external-dns#5364 - [@​linoleparquet](https://github.com/linoleparquet) made their first contribution in kubernetes-sigs/external-dns#5369 - [@​kashalls](https://github.com/kashalls) made their first contribution in kubernetes-sigs/external-dns#5349 - [@​matkam](https://github.com/matkam) made their first contribution in kubernetes-sigs/external-dns#3949 - [@​upsaurav12](https://github.com/upsaurav12) made their first contribution in kubernetes-sigs/external-dns#5388 **Full Changelog**: kubernetes-sigs/external-dns@v0.16.1...v0.17.0 </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0MC4xMS4xNSIsInVwZGF0ZWRJblZlciI6IjQwLjExLjE1IiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6WyJkZXAvbWlub3IiXX0=--> Co-authored-by: JesusMtnez <[email protected]> Reviewed-on: https://codeberg.org/JesusMtnez/homelab/pulls/712 Co-authored-by: JesusMtnez-bot <[email protected]> Co-committed-by: JesusMtnez-bot <[email protected]>
Description
Adds a check to ensure that the parent of route is the current generation.
Fixes #5348
Checklist