Repository: gardener/external-dns-management
Pull request #160 (2021-03-22 10:03:05)

consider provider zone on adding entries to zone
**What this PR does / why we need it**:
If there are multiple providers with the same domain (e.g. if private zones are used), the zone of the provider must be considered on adding a DNS entry to the zone.

**Which issue(s) this PR fixes**:
Fixes #

**Special notes for your reviewer**:

**Release note**:
<!--  Write your release note:
1. Enter your release note in the below block.
2. If no release note is required, just write "NONE" within the block.

Format of block header: <category> <target_group>
Possible values:
- category:       breaking|feature|bugfix|doc|other
- target_group:   user|operator|developer|dependency
-->
```other operator
consider provider zone on adding entries to zone
```



Patch:
diff --git a/charts/external-dns-management/templates/vpa.yaml b/charts/external-dns-management/templates/vpa.yaml
index 8010a278..7c45116f 100644
--- a/charts/external-dns-management/templates/vpa.yaml
+++ b/charts/external-dns-management/templates/vpa.yaml
@@ -1,5 +1,5 @@
 {{- if .Values.vpa.enabled }}
-apiVersion: "autoscaling.k8s.io/v1"
+apiVersion: "autoscaling.k8s.io/v1beta2"
 kind: VerticalPodAutoscaler
 metadata:
   name: {{ include "external-dns-management.fullname" . }}-vpa
diff --git a/charts/external-dns-management/values.yaml b/charts/external-dns-management/values.yaml
index 598f4d3d..e468e8bf 100644
--- a/charts/external-dns-management/values.yaml
+++ b/charts/external-dns-management/values.yaml
@@ -30,7 +30,7 @@ nodeSelector: {}
 tolerations: []
 affinity: {}
 
-createCRDs: false
+createCRDs: true
 
 #serviceAccountAnnotations:
 #  annotkey1: annotvalue1
diff --git a/pkg/dns/provider/default.go b/pkg/dns/provider/default.go
index 7efb6d19..a676295c 100644
--- a/pkg/dns/provider/default.go
+++ b/pkg/dns/provider/default.go
@@ -16,7 +16,10 @@
 
 package provider
 
-import "github.com/gardener/external-dns-management/pkg/dns"
+import (
+	"github.com/gardener/external-dns-management/pkg/dns"
+	dnsutils "github.com/gardener/external-dns-management/pkg/dns/utils"
+)
 
 ////////////////////////////////////////////////////////////////////////////////
 //  Default Implementation for DNSZoneState
@@ -78,6 +81,22 @@ func (this *DefaultDNSHostedZone) IsPrivate() bool {
 	return this.isPrivate
 }
 
+func (this *DefaultDNSHostedZone) Match(dnsname string) int {
+	return Match(this, dnsname)
+}
+
+func Match(zone DNSHostedZone, dnsname string) int {
+	for _, forwardedDomain := range zone.ForwardedDomains() {
+		if dnsutils.Match(dnsname, forwardedDomain) {
+			return 0
+		}
+	}
+	if dnsutils.Match(dnsname, zone.Domain()) {
+		return len(zone.Domain())
+	}
+	return 0
+}
+
 func NewDNSHostedZone(ptype string, id, domain, key string, forwarded []string, isPrivate bool) DNSHostedZone {
 	return &DefaultDNSHostedZone{providerType: ptype, id: id, key: key, domain: domain, forwarded: forwarded, isPrivate: isPrivate}
 }
diff --git a/pkg/dns/provider/entry.go b/pkg/dns/provider/entry.go
index 941523d1..0f32b64f 100644
--- a/pkg/dns/provider/entry.go
+++ b/pkg/dns/provider/entry.go
@@ -49,6 +49,7 @@ type EntryPremise struct {
 	ptypes   utils.StringSet
 	ptype    string
 	provider DNSProvider
+	fallback DNSProvider // provider with correct zone, but outside selection (only set if provider == nil)
 	zoneid   string
 
 	// non-identifying fields
@@ -56,7 +57,7 @@ type EntryPremise struct {
 }
 
 func (this *EntryPremise) Match(p *EntryPremise) bool {
-	return this.ptype == p.ptype && this.provider == p.provider && this.zoneid == p.zoneid
+	return this.ptype == p.ptype && this.provider == p.provider && this.zoneid == p.zoneid && this.fallback == p.fallback
 }
 
 func (this *EntryPremise) NotifyChange(p *EntryPremise) string {
@@ -70,6 +71,9 @@ func (this *EntryPremise) NotifyChange(p *EntryPremise) string {
 	if this.zoneid != p.zoneid {
 		r = append(r, fmt.Sprintf("zone (%s -> %s)", this.zoneid, p.zoneid))
 	}
+	if this.fallback != p.fallback {
+		r = append(r, fmt.Sprintf("fallback (%s -> %s)", Provider(this.fallback), Provider(p.fallback)))
+	}
 	if len(r) == 0 {
 		return ""
 	}
@@ -90,6 +94,7 @@ type EntryVersion struct {
 	responsible bool
 	valid       bool
 	duplicate   bool
+	obsolete    bool
 }
 
 func NewEntryVersion(object *dnsutils.DNSEntryObject, old *Entry) *EntryVersion {
@@ -131,6 +136,9 @@ func (this *EntryVersion) RequiresUpdateFor(e *EntryVersion) (reasons []string)
 			reasons = append(reasons, "state changed")
 		}
 	}
+	if this.obsolete != e.obsolete {
+		reasons = append(reasons, "provider responsibility changed")
+	}
 	return
 }
 
@@ -387,12 +395,12 @@ func (this *EntryVersion) Setup(logger logger.LogContext, state *state, p *Entry
 
 	if p.zoneid == "" && !utils.IsEmptyString(this.status.ProviderType) && p.ptypes.Contains(*this.status.ProviderType) {
 		// revoke assignment to actual type
-		old := utils.StringValue(this.status.ProviderType)
-		hello.Infof(logger, "revoke assignment to %s", old)
+		oldType := utils.StringValue(this.status.ProviderType)
+		hello.Infof(logger, "revoke assignment to %s", oldType)
 		this.status.Provider = nil
 		this.status.ProviderType = nil
 		this.status.Zone = nil
-		err := this.updateStatus(logger, "", "not valid for known provider anymore -> releasing provider type %s", old)
+		err := this.updateStatus(logger, "", "not valid for known provider anymore -> releasing provider type %s", oldType)
 		if err != nil {
 			return reconcile.Delay(logger, err)
 		}
diff --git a/pkg/dns/provider/interface.go b/pkg/dns/provider/interface.go
index 647d752e..66b17d68 100644
--- a/pkg/dns/provider/interface.go
+++ b/pkg/dns/provider/interface.go
@@ -118,6 +118,7 @@ type DNSHostedZone interface {
 	Id() string
 	Domain() string
 	ForwardedDomains() []string
+	Match(dnsname string) int
 	IsPrivate() bool
 }
 
@@ -230,11 +231,13 @@ type DNSProvider interface {
 	DefaultTTL() int64
 
 	GetZones() DNSHostedZones
+	IncludesZone(zoneID string) bool
 
 	GetZoneState(zone DNSHostedZone) (DNSZoneState, error)
 	ExecuteRequests(logger logger.LogContext, zone DNSHostedZone, state DNSZoneState, requests []*ChangeRequest) error
 
 	Match(dns string) int
+	MatchZone(dns string) int
 	IsValid() bool
 
 	AccountHash() string
diff --git a/pkg/dns/provider/provider.go b/pkg/dns/provider/provider.go
index ff7ca513..c551deb5 100644
--- a/pkg/dns/provider/provider.go
+++ b/pkg/dns/provider/provider.go
@@ -483,7 +483,17 @@ func (this *dnsProviderVersion) Match(dns string) int {
 	if ilen > elen {
 		return ilen
 	}
-	return -1
+	return 0
+}
+
+func (this *dnsProviderVersion) MatchZone(dns string) int {
+	for _, zone := range this.zones {
+		ilen := zone.Match(dns)
+		if ilen > 0 {
+			return ilen
+		}
+	}
+	return 0
 }
 
 func (this *dnsProviderVersion) MapTarget(t Target) Target {
@@ -566,3 +576,7 @@ func (this *dnsProviderVersion) ReportZoneStateConflict(zone DNSHostedZone, err
 func (this *dnsProviderVersion) ExecuteRequests(logger logger.LogContext, zone DNSHostedZone, state DNSZoneState, reqs []*ChangeRequest) error {
 	return this.account.ExecuteRequests(logger, zone, state, reqs)
 }
+
+func (this *dnsProviderVersion) IncludesZone(zoneID string) bool {
+	return this.included_zones != nil && this.included_zones.Contains(zoneID)
+}
diff --git a/pkg/dns/provider/state.go b/pkg/dns/provider/state.go
index 52a9630e..6362130d 100644
--- a/pkg/dns/provider/state.go
+++ b/pkg/dns/provider/state.go
@@ -305,7 +305,8 @@ func (this *state) hasProviders() bool {
 func (this *state) LookupProvider(e *EntryVersion) (DNSProvider, error) {
 	this.lock.RLock()
 	defer this.lock.RUnlock()
-	return this.lookupProvider(e.Object())
+	provider, _, err := this.lookupProvider(e.Object())
+	return provider, err
 }
 
 type providerMatch struct {
@@ -317,37 +318,49 @@ func newMatch() *providerMatch {
 	return &providerMatch{nil, -1}
 }
 
-func (this *state) lookupProvider(e *dnsutils.DNSEntryObject) (DNSProvider, error) {
+func (this *state) lookupProvider(e *dnsutils.DNSEntryObject) (DNSProvider, DNSProvider, error) {
+	handleMatch := func(match *providerMatch, p *dnsProviderVersion, n int, err error) error {
+		if match.match <= n {
+			err2 := access.CheckAccessWithRealms(e, "use", p.Object(), this.realms)
+			if err2 == nil {
+				if match.match < n || (e.Status().Provider != nil && *e.Status().Provider == p.object.ObjectName().String()) {
+					match.found = p
+					match.match = n
+				}
+				return nil
+			}
+			if match.match == 0 {
+				return err2
+			}
+		}
+		return err
+	}
 	var err error
 	validMatch := newMatch()
 	errorMatch := newMatch()
+	validMatchFallback := newMatch()
 	for _, p := range this.providers {
 		n := p.Match(e.GetDNSName())
 		if n > 0 {
-			var match *providerMatch
 			if p.IsValid() {
-				match = validMatch
+				err = handleMatch(validMatch, p, n, err)
 			} else {
-				match = errorMatch
+				err = handleMatch(errorMatch, p, n, err)
 			}
-			if match.match <= n {
-				err = access.CheckAccessWithRealms(e, "use", p.Object(), this.realms)
-				if err == nil {
-					if match.match < n || (e.Status().Provider != nil && *e.Status().Provider == p.object.ObjectName().String()) {
-						match.found = p
-						match.match = n
-					}
-				}
+		} else {
+			n = p.MatchZone(e.GetDNSName())
+			if n > 0 && p.IsValid() {
+				handleMatch(validMatchFallback, p, n, nil)
 			}
 		}
 	}
 	if validMatch.found != nil {
-		return validMatch.found, nil
+		return validMatch.found, nil, nil
 	}
 	if errorMatch.found != nil {
-		return errorMatch.found, nil
+		return errorMatch.found, nil, nil
 	}
-	return nil, err
+	return nil, validMatchFallback.found, err
 }
 
 func (this *state) GetProvider(name resources.ObjectName) DNSProvider {
@@ -381,9 +394,10 @@ func (this *state) addEntriesForZone(logger logger.LogContext, entries Entries,
 	if stale == nil {
 		stale = DNSNames{}
 	}
-	deleting := true
+	deleting := true // TODO check
 	domain := zone.Domain()
-	nested := utils.NewStringSet(zone.ForwardedDomains()...)
+	// fallback if no forwarded domains are reported
+	nested := utils.NewStringSet()
 	for _, z := range this.zones {
 		if z.Domain() != domain && dnsutils.Match(z.Domain(), domain) {
 			nested.Add(z.Domain())
@@ -392,7 +406,7 @@ func (this *state) addEntriesForZone(logger logger.LogContext, entries Entries,
 loop:
 	for dns, e := range this.dnsnames {
 		if e.IsValid() {
-			provider, err := this.lookupProvider(e.Object())
+			provider, fallback, err := this.lookupProvider(e.Object())
 			if (provider == nil || !provider.IsValid()) && !e.IsDeleting() {
 				if provider != nil {
 					logger.Infof("no valid provider found for %q(%s found, but is not valid)", e.ObjectName(), provider.ObjectName())
@@ -403,15 +417,14 @@ loop:
 						logger.Infof("no valid provider found for %q(%s)", e.ObjectName(), dns)
 					}
 				}
-				stale[e.DNSName()] = e
+				if fallback == nil || !fallback.IncludesZone(zone.Id()) {
+					stale[e.DNSName()] = e
+					continue
+				}
+			} else if !provider.IncludesZone(zone.Id()) {
 				continue
 			}
-			if dnsutils.Match(dns, domain) {
-				for _, excl := range zone.ForwardedDomains() {
-					if dnsutils.Match(dns, excl) {
-						continue loop
-					}
-				}
+			if zone.Match(dns) > 0 {
 				for excl := range nested { // fallback if no forwarded domains are reported
 					if dnsutils.Match(dns, excl) {
 						continue loop
@@ -440,23 +453,29 @@ func (this *state) GetZoneForEntry(e *Entry) string {
 	if !e.IsValid() {
 		return ""
 	}
-	zoneid, _, _ := this.GetZoneForName(e.DNSName())
-	return zoneid
+	provider, _, _ := this.lookupProvider(e.object)
+	return this.GetProviderZoneForName(e.DNSName(), provider)
 }
 
-func (this *state) GetZoneForName(name string) (string, string, int) {
+func (this *state) GetProviderZoneForName(name string, provider DNSProvider) string {
 	this.lock.RLock()
 	defer this.lock.RUnlock()
 
-	found := this.getZoneForName(name)
+	found := this.getProviderZoneForName(name, provider)
 	if found != nil {
-		return found.Id(), found.ProviderType(), len(found.Domain())
+		return found.Id()
 	}
-	return "", "", 0
+	return ""
+}
+
+func (this *state) getProviderZoneForName(hostname string, provider DNSProvider) *dnsHostedZone {
+	zones := this.getZonesForName(hostname)
+	return filterZoneByProvider(zones, provider)
 }
 
-func (this *state) getZoneForName(hostname string) *dnsHostedZone {
-	var found *dnsHostedZone
+// getZonesForName can return multiple zones in the case of private zones
+func (this *state) getZonesForName(hostname string) []*dnsHostedZone {
+	var found []*dnsHostedZone
 	length := 0
 loop:
 	for _, zone := range this.zones {
@@ -469,7 +488,9 @@ loop:
 			}
 			if length < len(name) {
 				length = len(name)
-				found = zone
+				found = []*dnsHostedZone{zone}
+			} else if length == len(name) {
+				found = append(found, zone)
 			}
 		}
 	}
diff --git a/pkg/dns/provider/state_entry.go b/pkg/dns/provider/state_entry.go
index b49f56ce..c0a2964f 100644
--- a/pkg/dns/provider/state_entry.go
+++ b/pkg/dns/provider/state_entry.go
@@ -230,13 +230,14 @@ func (this *state) EntryPremise(e *dnsutils.DNSEntryObject) (*EntryPremise, erro
 	this.lock.RLock()
 	defer this.lock.RUnlock()
 
-	provider, err := this.lookupProvider(e)
-	zone := this.getZoneForName(e.GetDNSName())
-
+	provider, fallback, err := this.lookupProvider(e)
 	p := &EntryPremise{
 		ptypes:   this.config.Enabled,
 		provider: provider,
+		fallback: fallback,
 	}
+	zone := this.getProviderZoneForName(e.GetDNSName(), provider)
+
 	if zone != nil {
 		p.ptype = zone.ProviderType()
 		p.zoneid = zone.Id()
@@ -244,6 +245,13 @@ func (this *state) EntryPremise(e *dnsutils.DNSEntryObject) (*EntryPremise, erro
 	} else if provider != nil && !provider.IsValid() && e.Status().Zone != nil {
 		p.ptype = provider.TypeCode()
 		p.zoneid = *e.Status().Zone
+	} else if p.fallback != nil {
+		zone = this.getProviderZoneForName(e.GetDNSName(), p.fallback)
+		if zone != nil {
+			p.ptype = zone.ProviderType()
+			p.zoneid = zone.Id()
+			p.zonedomain = zone.Domain()
+		}
 	}
 	return p, err
 }
@@ -267,6 +275,9 @@ func (this *state) HandleUpdateEntry(logger logger.LogContext, op string, object
 
 	logger = this.RefineLogger(logger, p.ptype)
 	v := NewEntryVersion(object, old)
+	if p.fallback != nil {
+		v.obsolete = true
+	}
 	status := v.Setup(logger, this, p, op, err, this.config, old)
 	new, status := this.AddEntryVersion(logger, v, status)
 
@@ -306,7 +317,8 @@ func (this *state) EntryDeleted(logger logger.LogContext, key resources.ClusterO
 
 	old := this.entries[key.ObjectName()]
 	if old != nil {
-		zone := this.getZoneForName(old.DNSName())
+		provider, _, _ := this.lookupProvider(old.object)
+		zone := this.getProviderZoneForName(old.DNSName(), provider)
 		if zone != nil {
 			logger.Infof("removing entry %q (%s[%s])", key.ObjectName(), old.DNSName(), zone.Id())
 			this.triggerHostedZone(zone.Id())
diff --git a/pkg/dns/provider/state_zone.go b/pkg/dns/provider/state_zone.go
index 177fb7ca..f63f156d 100644
--- a/pkg/dns/provider/state_zone.go
+++ b/pkg/dns/provider/state_zone.go
@@ -49,17 +49,6 @@ func (this *state) TriggerHostedZones() {
 	}
 }
 
-func (this *state) GetZoneInfo(logger logger.LogContext, zoneid string) (*dnsHostedZone, DNSProviders, Entries, DNSNames, bool) {
-	this.lock.RLock()
-	defer this.lock.RUnlock()
-	zone := this.zones[zoneid]
-	if zone == nil {
-		return nil, nil, nil, nil, false
-	}
-	entries, stale, deleting := this.addEntriesForZone(logger, nil, nil, zone)
-	return zone, this.getProvidersForZone(zoneid), entries, stale, deleting
-}
-
 func (this *state) GetZoneReconcilation(logger logger.LogContext, zoneid string) (time.Duration, bool, *zoneReconciliation) {
 	req := &zoneReconciliation{}
 
diff --git a/pkg/dns/provider/utils.go b/pkg/dns/provider/utils.go
index ffb793d8..3d973de5 100644
--- a/pkg/dns/provider/utils.go
+++ b/pkg/dns/provider/utils.go
@@ -41,3 +41,14 @@ func errorValue(format string, err error) string {
 	}
 	return fmt.Sprintf(format, err.Error())
 }
+
+func filterZoneByProvider(zones []*dnsHostedZone, provider DNSProvider) *dnsHostedZone {
+	if provider != nil {
+		for _, zone := range zones {
+			if provider.IncludesZone(zone.Id()) {
+				return zone
+			}
+		}
+	}
+	return nil
+}
diff --git a/pkg/dns/provider/zone.go b/pkg/dns/provider/zone.go
index 750f3fbf..b2f6f900 100644
--- a/pkg/dns/provider/zone.go
+++ b/pkg/dns/provider/zone.go
@@ -93,6 +93,10 @@ func (this *dnsHostedZone) IsPrivate() bool {
 	return this.getZone().IsPrivate()
 }
 
+func (this *dnsHostedZone) Match(dnsname string) int {
+	return Match(this, dnsname)
+}
+
 ////////////////////////////////////////////////////////////////////////////////
 
 func (this *dnsHostedZone) update(zone DNSHostedZone) {


Test patch:
diff --git a/pkg/controller/provider/openstack/handler_test.go b/pkg/controller/provider/openstack/handler_test.go
index 3de63a18..c9d5ef0c 100644
--- a/pkg/controller/provider/openstack/handler_test.go
+++ b/pkg/controller/provider/openstack/handler_test.go
@@ -72,6 +72,10 @@ func (tz *testzone) IsPrivate() bool {
 	return false
 }
 
+func (tz *testzone) Match(dnsname string) int {
+	return provider.Match(tz, dnsname)
+}
+
 type designateMockClient struct {
 	tzmap map[string]*testzone
 }