Allow ZoneIDFilter for Cloudflare #1307
Conversation
k8s-ci-robot
added
the
do-not-merge/invalid-commit-message
|
Thanks for your pull request. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please follow instructions at https://git.k8s.io/community/CLA.md#the-contributor-license-agreement to sign the CLA. It may take a couple minutes for the CLA signature to be fully registered; after that, please reply here with a new comment and we'll verify. Thanks.
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. |
k8s-ci-robot
added
the
cncf-cla: no
|
Welcome @dmizelle! |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: dmizelle The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
the
size/L
Additionally:
* add a .dockerignore so we don't copy .git and docs to the
container (gotta go fast)
* Change Dockerfile{,.mini} to not run `go mod` every time a file is
changed.
This commit _should_ help kubernetes-sigs#1127. While users in the past were able to
define ZoneIDFilter for this provider, it did not actually do anything
under the hood.
In this case, we're changing Zones() to iterate over the provided
zoneIDs and return only those zones.
I would have also done this for domainFilter, but unfortunately the
CloudFlare API requires that in order to list zones (and find them by
name) that you have "all" permissions, which seems silly. After talking
to their support, this is probably the best way to do this.
dmizelle
force-pushed
the
cloudflare-zone-ids
branch
from
1031c5d
to
14610e7
Compare
k8s-ci-robot
removed
the
do-not-merge/invalid-commit-message
|
signed cla! |
|
/check-cla |
| @@ -16,10 +16,13 @@ | |||
| FROM golang:1.13 as builder | |||
|
|
|||
| WORKDIR /github.com/kubernetes-sigs/external-dns | |||
| COPY go.mod . | |||
There was a problem hiding this comment.
Please leave the the Dockerfile changes out of this PR, the PR should only focus on the Cloudflare changes like in the title.
But feel free to create a separate PR.
| @@ -486,6 +528,23 @@ func TestCloudFlareZones(t *testing.T) { | |||
| }) | |||
| } | |||
|
|
|||
| func TestCloudFlareZonesWithIDFilter(t *testing.T) { | |||
There was a problem hiding this comment.
Can you add an additional test wich has both domainFilter and zoneIDFilter?
|
@dmizelle I will close this PR since it has no progress, if you have time please reopen a PR and check you CLA. |
|
@dmizelle: PR needs rebase. 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. |
k8s-ci-robot
added
the
needs-rebase
Signed-off-by: Danny Grove <danny@drgrovellc.com>
This is based off the work found in kubernetes-sigs#1307 that was never merged. It moves around the install and copy of certain conponents to take better advantage of the Docker cache ad well as drops running tests during the build of the image. The reason for dropping tests is to improve build time and as running tests within the build while they're already being run in CI seems like an unnecessary added tax. Signed-off-by: Danny Grove <danny@drgrovellc.com>
This is based off the work found in kubernetes-sigs#1307 that was never merged. It moves around the install and copy of certain conponents to take better advantage of the Docker cache ad well as drops running tests during the build of the image. The reason for dropping tests is to improve build time and as running tests within the build while they're already being run in CI seems like an unnecessary added tax. Signed-off-by: Danny Grove <danny@drgrovellc.com>
This is based off the work found in kubernetes-sigs#1307 that was never merged. It moves around the install and copy of certain conponents to take better advantage of the Docker cache ad well as drops running tests during the build of the image. The reason for dropping tests is to improve build time and as running tests within the build while they're already being run in CI seems like an unnecessary added tax. Signed-off-by: Danny Grove <danny@drgrovellc.com>
This is based off the work found in kubernetes-sigs#1307 that was never merged. It moves around the install and copy of certain conponents to take better advantage of the Docker cache ad well as drops running tests during the build of the image. The reason for dropping tests is to improve build time and as running tests within the build while they're already being run in CI seems like an unnecessary added tax. Signed-off-by: Danny Grove <danny@drgrovellc.com>
This is based off the work found in kubernetes-sigs#1307 that was never merged. It moves around the install and copy of certain conponents to take better advantage of the Docker cache ad well as drops running tests during the build of the image. The reason for dropping tests is to improve build time and as running tests within the build while they're already being run in CI seems like an unnecessary added tax. Signed-off-by: Danny Grove <danny@drgrovellc.com>
This is based off the work found in kubernetes-sigs#1307 that was never merged. It moves around the install and copy of certain conponents to take better advantage of the Docker cache ad well as drops running tests during the build of the image. The reason for dropping tests is to improve build time and as running tests within the build while they're already being run in CI seems like an unnecessary added tax. Signed-off-by: Danny Grove <danny@drgrovellc.com>
Additionally:
container (gotta go fast)
go modevery time a file ischanged.
This commit should help with #1127. While users in the past were able to
define ZoneIDFilter for this provider, it did not actually do anything
under the hood.
In this case, we're changing Zones() to iterate over the provided
zoneIDs and return only those zones.
I would have also done this for domainFilter, but unfortunately the
CloudFlare API requires that in order to list zones (and find them by
name) that you have "all" permissions, which seems silly. After talking
to their support, this is probably the best way to do this.