fix: filter peering zones in google provider

[itspngu]

Description

Filters peering zones from the list of zones considered for creation of records by the google provider. This is needed because peering zones do not allow the creation of records, so if a peering zone matches a domain filter, external-dns tries to create records in it, causing the entire changeset to fail and no records to be created (even in other, valid zones).

Fixes #2947

Checklist

• Unit tests updated
• End user documentation updated

Notes

Changes to end user docs were not necessary.

[k8s-ci-robot]

Welcome @itspngu!

It looks like this is your first PR to kubernetes-sigs/external-dns 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes-sigs/external-dns has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

[johngmyers]

/ok-to-test

[szuecs]

/approve

[andrewchinnadorai]

Thanks for raising this, tried to raise a PR to add a test for the peering filter but as it's a forked branch I'm unable to do so, if it's any help you can add the test from this branch

[itspngu]

Thanks for raising this, tried to raise a PR to add a test for the peering filter but as it's a forked branch I'm unable to do so, if it's any help you can add the test from this branch

Thank you! I didn't realize the zones were created during the e2e test, should have looked at the code more closely. I'll try to get your additions into this branch while retaining your authorship without making the CI throw up 🤞

[itspngu]

This doesn't look like it'd work :( https://github.com/itspngu/external-dns/compare/google-peering-zone-bugfix...andrewchinnadorai:external-dns:3690/add-peering-filtering-tests

[szuecs]

/approve cancel

[andrewchinnadorai]

This doesn't look like it'd work :( itspngu/[email protected]:external-dns:3690/add-peering-filtering-tests

Hm, have you rebased your forked branch with master?

[linux-foundation-easycla]

The committers listed above are authorized under a signed CLA.

• ✅ login: itspngu / name: Thorben (b1084be)
• ✅ login: andrewchinnadorai / name: Andrew Chinnadorai (a7884f1)

[itspngu]

This doesn't look like it'd work :( itspngu/[email protected]:external-dns:3690/add-peering-filtering-tests

Hm, have you rebased your forked branch with master?

Right, that sorted it. Now the CI is running again and you need to sign the CLA but tests are worth it. Thanks! :D

[andrewchinnadorai]

This doesn't look like it'd work :( itspngu/[email protected]:external-dns:3690/add-peering-filtering-tests

Hm, have you rebased your forked branch with master?

Right, that sorted it. Now the CI is running again and you need to sign the CLA but tests are worth it. Thanks! :D

Nice, signed

[andrewchinnadorai]

@szuecs Can we get this reviewed by a maintainer please

[itspngu]

Totally forgot about this because we're running a fork including the change. Would be great to get a fresh review, sorry for the chaos caused while adding the tests.

[johngmyers]

/lgtm
/assign @seanmalloy

[itspngu]

@seanmalloy @szuecs Sorry for the direct ping, but I was wondering if you had the chance to give this a look yet. Apologies for the noise caused by merging the tests. If there's anything I can do to help get this merged, please do let me know.

[itspngu]

Here's some additional context on what these zones are and why they need to be filtered: https://cloud.google.com/dns/docs/zones/zones-overview#dns_peering_limitations_and_key_points

TL;DR: They are not something records can be created in, they're a sort of read-only reference to a DNS zone in another project. When external-dns tries to create records in these zones, an error is returned by the Cloud DNS APIs, resulting in partial (or no) record creation in valid zones.

[andrewchinnadorai]

@seanmalloy @szuecs Sorry for another ping, but just want to bump this as we're currently using a self-built version to work around this and are hoping this fix will get in to a official release at some point

[mloiseleur]

/lgtm
/retest

[johngmyers]

/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: johngmyers

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:

• OWNERS [johngmyers]

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