Add subdomain filter to AWS provider #1375

spohner · 2020-01-16T09:32:20Z

This PR adds a subdomain filter to the AWS provider.

For example a filter defined with foo.example.org only allows changes to foo.example.org and subdomains of this domain. So bar.foo.example.org is allowed, but foo.bar.example.org is filtered by this filter.

Any feedback is much appreciated.

k8s-ci-robot · 2020-01-16T09:33:16Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: spohner
To complete the pull request process, please assign hjacobs
You can assign the PR to them by writing /assign @hjacobs in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

OWNERS

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

spohner · 2020-01-16T09:52:11Z

/assign @hjacobs

njuettner

Thanks for the PR 👍. I added some comments, PTAL 🙂.

njuettner · 2020-02-03T14:10:11Z

docs/tutorials/aws.md

-        - --aws-zone-type=public # only look at public hosted zones (valid values are public, private or no value for both)
-        - --registry=txt
-        - --txt-owner-id=my-hostedzone-identifier
+        - name: external-dns


I think the indent are wrong here

also the other changes, please revert them

You are right. My bad. I'll revert and add the subdomain filter section with correct indentation.

njuettner · 2020-02-03T14:12:30Z

pkg/apis/externaldns/types.go

@@ -300,6 +302,7 @@ func (cfg *Config) ParseFlags(args []string) error {
 	app.Flag("provider", "The DNS provider where the DNS records will be created (required, options: aws, aws-sd, google, azure, azure-dns, azure-private-dns, cloudflare, rcodezero, digitalocean, dnsimple, akamai, infoblox, dyn, designate, coredns, skydns, inmemory, pdns, oci, exoscale, linode, rfc2136, ns1, transip, vinyldns, rdns)").Required().PlaceHolder("provider").EnumVar(&cfg.Provider, "aws", "aws-sd", "google", "azure", "azure-dns", "azure-private-dns", "alibabacloud", "cloudflare", "rcodezero", "digitalocean", "dnsimple", "akamai", "infoblox", "dyn", "designate", "coredns", "skydns", "inmemory", "pdns", "oci", "exoscale", "linode", "rfc2136", "ns1", "transip", "vinyldns", "rdns")
 	app.Flag("domain-filter", "Limit possible target zones by a domain suffix; specify multiple times for multiple domains (optional)").Default("").StringsVar(&cfg.DomainFilter)
 	app.Flag("exclude-domains", "Exclude subdomains (optional)").Default("").StringsVar(&cfg.ExcludeDomains)
+	app.Flag("subdomain-filter", "Allow only changes to specific subdomain").Default("").StringsVar(&cfg.AWSSubdomainFilter)


it would be nice to only name it SubdomainFilter, this might be also useable for other providers

Agreed. Will change.

njuettner · 2020-02-03T14:13:49Z

provider/aws.go

@@ -124,7 +124,9 @@ type AWSProvider struct {
 	zoneTypeFilter ZoneTypeFilter
 	// filter hosted zones by tags
 	zoneTagFilter ZoneTagFilter
-	preferCNAME   bool
+	// only allow changes to specified subdomain and its subdomains
+	subdomainFilter DomainFilter


should this not be changed to SubdomainFilter instead of DomainFilter?

The subdomain filter is still a domain filter and the domain filtering provided by DomainFilter will be sufficient. However, I can add a SubdomainFilter type for readability and future changes. What do you think?

Yes please, just thought it would be better to divide it because of readability and future changes as well.

njuettner · 2020-02-03T14:14:30Z

provider/aws.go

@@ -133,6 +135,7 @@ type AWSConfig struct {
 	ZoneIDFilter         ZoneIDFilter
 	ZoneTypeFilter       ZoneTypeFilter
 	ZoneTagFilter        ZoneTagFilter
+	SubdomainFilter      DomainFilter


Also here DomainFilter -> SubDomainFilter?

njuettner · 2020-02-03T14:17:19Z

provider/aws_test.go

@@ -301,7 +301,7 @@ func TestAWSZones(t *testing.T) {
 		{"zone id filter", NewZoneIDFilter([]string{"/hostedzone/zone-3.ext-dns-test-2.teapot.zalan.do."}), NewZoneTypeFilter(""), NewZoneTagFilter([]string{}), privateZones},
 		{"tag filter", NewZoneIDFilter([]string{}), NewZoneTypeFilter(""), NewZoneTagFilter([]string{"zone=3"}), privateZones},
 	} {
-		provider, _ := newAWSProviderWithTagFilter(t, NewDomainFilter([]string{"ext-dns-test-2.teapot.zalan.do."}), ti.zoneIDFilter, ti.zoneTypeFilter, ti.zoneTagFilter, defaultEvaluateTargetHealth, false, []*endpoint.Endpoint{})
+		provider, _ := newAWSProviderWithTagFilter(t, NewDomainFilter([]string{"ext-dns-test-2.teapot.zalan.do."}), NewDomainFilter([]string{}), ti.zoneIDFilter, ti.zoneTypeFilter, ti.zoneTagFilter, defaultEvaluateTargetHealth, false, []*endpoint.Endpoint{})


NewSubDomainFilter? And please also add subdomains inside of []string to see if it works.

Same for the ones below

This reverts commit 2c9210b.

njuettner · 2020-02-04T09:59:14Z

provider/aws.go

@@ -124,7 +124,9 @@ type AWSProvider struct {
 	zoneTypeFilter ZoneTypeFilter
 	// filter hosted zones by tags
 	zoneTagFilter ZoneTagFilter
-	preferCNAME   bool
+	// only allow changes to specified subdomain and its subdomains
+	subdomainFilter DomainFilter


Yes please, just thought it would be better to divide it because of readability and future changes as well.

spohner · 2020-03-06T22:26:03Z

Did the subdomain wrapper do the trick? Anything I can do to help with this PR?

This reverts commit 2c9210b.

…-dns into subdomainfilter

seanmalloy · 2020-08-19T04:58:20Z

/kind feature

k8s-ci-robot · 2020-08-19T04:58:27Z

@spohner: 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.

fejta-bot · 2020-11-17T05:33:31Z

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

seanmalloy · 2020-11-18T04:26:53Z

/remove-lifecycle stale

fejta-bot · 2021-02-16T05:09:05Z

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-contributor-experience at kubernetes/community.
/lifecycle stale

fejta-bot · 2021-03-18T05:55:39Z

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Send feedback to sig-contributor-experience at kubernetes/community.
/lifecycle rotten

fejta-bot · 2021-04-17T06:36:11Z

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

Send feedback to sig-contributor-experience at kubernetes/community.
/close

k8s-ci-robot · 2021-04-17T06:36:16Z

@fejta-bot: Closed this PR.

In response to this:

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

Send feedback to sig-contributor-experience at kubernetes/community.
/close

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 cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jan 16, 2020

k8s-ci-robot requested review from hjacobs and njuettner January 16, 2020 09:32

spohner mentioned this pull request Jan 16, 2020

Question: No way to only allow subdomains under a domain using --domain-filter? #474

Closed

k8s-ci-robot assigned hjacobs Jan 16, 2020

spohner added 3 commits January 27, 2020 13:15

added subdomain filter for aws provider

3c81959

Added tests for subdomainfilter

656730d

Added subdomain section

2c9210b

spohner force-pushed the subdomainfilter branch from cc3ea34 to 2c9210b Compare January 27, 2020 12:16

njuettner requested changes Feb 3, 2020

View reviewed changes

spohner added 3 commits February 3, 2020 15:34

Revert "Added subdomain section"

773817d

This reverts commit 2c9210b.

Fix wrong indentations

d26c7ef

Change name to Subdomain filter

e0b67d0

spohner requested a review from njuettner February 3, 2020 15:33

njuettner requested changes Feb 4, 2020

View reviewed changes

Added SubdomainFilter wrapper

ee134b2

spohner requested a review from njuettner February 5, 2020 07:59

spohner added 8 commits May 11, 2020 15:16

added subdomain filter for aws provider

d6fe14f

Added tests for subdomainfilter

e100a79

Added subdomain section

7d13591

Revert "Added subdomain section"

41a86c0

This reverts commit 2c9210b.

Fix wrong indentations

6dbd36b

Change name to Subdomain filter

ea9f612

Added SubdomainFilter wrapper

618f585

Merge branch 'subdomainfilter' of https://github.com/spohner/external…

b1ce5f5

…-dns into subdomainfilter

k8s-ci-robot removed the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label May 11, 2020

k8s-ci-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label May 11, 2020

spohner changed the base branch from master to njuettner/go_modules/github.com/pkg/errors-0.9.1 May 11, 2020 13:20

spohner changed the base branch from njuettner/go_modules/github.com/pkg/errors-0.9.1 to master May 11, 2020 13:20

k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels May 11, 2020

Raffo unassigned hjacobs Jul 1, 2020

k8s-ci-robot added the kind/feature Categorizes issue or PR as related to a new feature. label Aug 19, 2020

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 19, 2020

k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Nov 17, 2020

k8s-ci-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Nov 18, 2020

k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Feb 16, 2021

k8s-ci-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Mar 18, 2021

k8s-ci-robot closed this Apr 17, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add subdomain filter to AWS provider #1375

Add subdomain filter to AWS provider #1375

spohner commented Jan 16, 2020

k8s-ci-robot commented Jan 16, 2020

spohner commented Jan 16, 2020

njuettner left a comment

njuettner Feb 3, 2020

njuettner Feb 3, 2020

spohner Feb 3, 2020

njuettner Feb 3, 2020

spohner Feb 3, 2020

njuettner Feb 3, 2020

spohner Feb 3, 2020

njuettner Feb 4, 2020

njuettner Feb 3, 2020

njuettner Feb 3, 2020

njuettner Feb 3, 2020

njuettner Feb 4, 2020

spohner commented Mar 6, 2020

seanmalloy commented Aug 19, 2020

k8s-ci-robot commented Aug 19, 2020

fejta-bot commented Nov 17, 2020

seanmalloy commented Nov 18, 2020

fejta-bot commented Feb 16, 2021

fejta-bot commented Mar 18, 2021

fejta-bot commented Apr 17, 2021

k8s-ci-robot commented Apr 17, 2021

Add subdomain filter to AWS provider #1375

Add subdomain filter to AWS provider #1375

Conversation

spohner commented Jan 16, 2020

k8s-ci-robot commented Jan 16, 2020

spohner commented Jan 16, 2020

njuettner left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

spohner commented Mar 6, 2020

seanmalloy commented Aug 19, 2020

k8s-ci-robot commented Aug 19, 2020

fejta-bot commented Nov 17, 2020

seanmalloy commented Nov 18, 2020

fejta-bot commented Feb 16, 2021

fejta-bot commented Mar 18, 2021

fejta-bot commented Apr 17, 2021

k8s-ci-robot commented Apr 17, 2021