Refactor external DNS to be provider neutral

k8gb controller doesn't really pass any provider specifics to
external-dns, except annotation. Annotation can be unified to make any
external-dns provider "supported" as is.

This commit removes hardcoded NS1 and R53 providers from the k8gb code

partially fixes #219

Signed-off-by: Dinar Valeev <dinar.valeev@absa.africa>

chart/k8gb/templates/_helpers.tpl

@@ -62,15 +62,6 @@ Create the name of the service account to use
 {{- end -}}
 {{- end -}}
 
-{{- define "k8gb.extdnsAnnotation" -}}
-{{- if .Values.ns1.enabled -}}
-{{- print "ns1" -}}
-{{- end -}}
-{{- if .Values.route53.enabled }}
-{{- print "route53" -}}
-{{- end -}}
-{{- end -}}
-
 {{- define "k8gb.extdnsProvider" -}}
 {{- if .Values.ns1.enabled -}}
 {{- print "ns1" -}}

chart/k8gb/templates/external-dns/external-dns.yaml

@@ -28,7 +28,7 @@ spec:
         - --managed-record-types=A
         - --managed-record-types=CNAME
         - --managed-record-types=NS
-        - --annotation-filter=k8gb.absa.oss/dnstype={{ include "k8gb.extdnsAnnotation" . }} # filter out only relevant DNSEntrypoints
+        - --annotation-filter=k8gb.absa.oss/dnstype=extdns # filter out only relevant DNSEntrypoints
         - --txt-owner-id={{ include "k8gb.extdnsOwnerID" . }}
         - --provider={{ include "k8gb.extdnsProvider" . }}
 {{- if .Values.ns1.enabled -}}

chart/k8gb/templates/operator.yaml

@@ -105,12 +105,8 @@ spec:
                   name: infoblox
                   key: INFOBLOX_WAPI_PASSWORD
             {{- end }}
-            {{- if .Values.route53.enabled }}
-            - name: ROUTE53_ENABLED
-              value: "true"
-            {{- end }}
-            {{- if .Values.ns1.enabled }}
-            - name: NS1_ENABLED
+            {{- if or .Values.route53.enabled .Values.ns1.enabled }}
+            - name: EXTDNS_ENABLED
               value: "true"
             {{- end }}
             {{- if eq "LoadBalancer" ( quote .Values.coredns.serviceType ) }}

controllers/depresolver/depresolver.go

@@ -69,9 +69,7 @@ const (
 	// DNSTypeInfoblox type
 	DNSTypeInfoblox EdgeDNSType = "Infoblox"
 	// DNSTypeRoute53 type
-	DNSTypeRoute53 EdgeDNSType = "Route53"
-	// DNSTypeNS1 type
-	DNSTypeNS1 EdgeDNSType = "NS1"
+	DNSTypeExternal EdgeDNSType = "ExtDNS"
 	// DNSTypeMultipleProviders type
 	DNSTypeMultipleProviders EdgeDNSType = "MultipleProviders"
 )
@@ -138,10 +136,8 @@ type Config struct {
 	Log Log
 	// MetricsAddress in format address:port where address can be empty, IP address, or hostname, default: 0.0.0.0:8080
 	MetricsAddress string `env:"METRICS_ADDRESS, default=0.0.0.0:8080"`
-	// route53Enabled hidden. EdgeDNSType defines all enabled Enabled types
-	route53Enabled bool `env:"ROUTE53_ENABLED, default=false"`
-	// ns1Enabled flag
-	ns1Enabled bool `env:"NS1_ENABLED, default=false"`
+	// extDNSEnabled hidden. EdgeDNSType defines all enabled Enabled types
+	extDNSEnabled bool `env:"EXTDNS_ENABLED, default=false"`
 	// SplitBrainCheck flag decides whether split brain TXT records will be stored in edge DNS
 	SplitBrainCheck bool `env:"SPLIT_BRAIN_CHECK, default=false"`
 }

controllers/depresolver/depresolver_config.go

@@ -34,8 +34,7 @@ const (
 	ReconcileRequeueSecondsKey = "RECONCILE_REQUEUE_SECONDS"
 	ClusterGeoTagKey           = "CLUSTER_GEO_TAG"
 	ExtClustersGeoTagsKey      = "EXT_GSLB_CLUSTERS_GEO_TAGS"
-	Route53EnabledKey          = "ROUTE53_ENABLED"
-	NS1EnabledKey              = "NS1_ENABLED"
+	ExtDNSEnabledKey           = "EXTDNS_ENABLED"
 	EdgeDNSServersKey          = "EDGE_DNS_SERVERS"
 	EdgeDNSZoneKey             = "EDGE_DNS_ZONE"
 	DNSZoneKey                 = "DNS_ZONE"
@@ -325,11 +324,8 @@ func parseEdgeDNSServers(serverList []string) (r []utils.DNSServer) {
 // getEdgeDNSType contains logic retrieving EdgeDNSType.
 func getEdgeDNSType(config *Config) (EdgeDNSType, []EdgeDNSType) {
 	recognized := make([]EdgeDNSType, 0)
-	if config.ns1Enabled {
-		recognized = append(recognized, DNSTypeNS1)
-	}
-	if config.route53Enabled {
-		recognized = append(recognized, DNSTypeRoute53)
+	if config.extDNSEnabled {
+		recognized = append(recognized, DNSTypeExternal)
 	}
 	if isNotEmpty(config.Infoblox.Host) {
 		recognized = append(recognized, DNSTypeInfoblox)

controllers/depresolver/depresolver_test.go

@@ -633,7 +633,7 @@ func TestBothRoute53AndInfobloxAreEnabled(t *testing.T) {
 	customConfig.Infoblox.Username = defaultInfobloxUsername
 	customConfig.Infoblox.Password = defaultInfobloxPassword
 	configureEnvVar(customConfig)
-	_ = os.Setenv(Route53EnabledKey, "true")
+	_ = os.Setenv(ExtDNSEnabledKey, "true")
 	resolver := NewDependencyResolver()
 	// act
 	config, err := resolver.ResolveOperatorConfig()
@@ -647,8 +647,7 @@ func TestRoute53NS1AndInfobloxAreConfigured(t *testing.T) {
 	defer cleanup()
 	// predefinedConfig has Infoblox preconfigured
 	configureEnvVar(predefinedConfig)
-	_ = os.Setenv(Route53EnabledKey, "true")
-	_ = os.Setenv(NS1EnabledKey, "true")
+	_ = os.Setenv(ExtDNSEnabledKey, "true")
 	resolver := NewDependencyResolver()
 	// act
 	config, err := resolver.ResolveOperatorConfig()
@@ -657,7 +656,7 @@ func TestRoute53NS1AndInfobloxAreConfigured(t *testing.T) {
 	assert.Error(t, err)
 	assert.Equal(t, DNSTypeMultipleProviders, config.EdgeDNSType)
 	assert.Equal(t, recognizedEdgeDNSType, config.EdgeDNSType)
-	assert.Equal(t, recognizedEdgeDNSTypes, []EdgeDNSType{DNSTypeNS1, DNSTypeRoute53, DNSTypeInfoblox})
+	assert.Equal(t, recognizedEdgeDNSTypes, []EdgeDNSType{DNSTypeExternal, DNSTypeInfoblox})
 }
 
 func TestNoDNSIsConfigured(t *testing.T) {
@@ -682,7 +681,7 @@ func TestRoute53IsDisabledAndInfobloxIsNotConfigured(t *testing.T) {
 	defer cleanup()
 	expected := predefinedConfig
 	expected.EdgeDNSType = DNSTypeNoEdgeDNS
-	expected.route53Enabled = false
+	expected.extDNSEnabled = false
 	expected.Infoblox.Host = ""
 	// act,assert
 	// that's how our integration tests are running
@@ -693,7 +692,7 @@ func TestRoute53IsDisabledButInfobloxIsConfigured(t *testing.T) {
 	// arrange
 	defer cleanup()
 	expected := predefinedConfig
-	expected.route53Enabled = false
+	expected.extDNSEnabled = false
 	expected.EdgeDNSType = DNSTypeInfoblox
 	expected.Infoblox.Host = "Infoblox.domain"
 	expected.Infoblox.Version = defaultVersion
@@ -708,8 +707,8 @@ func TestRoute53IsEnabledButInfobloxIsNotConfigured(t *testing.T) {
 	// arrange
 	defer cleanup()
 	expected := predefinedConfig
-	expected.route53Enabled = true
-	expected.EdgeDNSType = DNSTypeRoute53
+	expected.extDNSEnabled = true
+	expected.EdgeDNSType = DNSTypeExternal
 	expected.Infoblox.Host = ""
 	expected.Infoblox.Version = defaultVersion
 	expected.Infoblox.Port = 443
@@ -723,8 +722,8 @@ func TestInfobloxGridHostIsEmpty(t *testing.T) {
 	// arrange
 	defer cleanup()
 	expected := predefinedConfig
-	expected.EdgeDNSType = DNSTypeRoute53
-	expected.route53Enabled = true
+	expected.EdgeDNSType = DNSTypeExternal
+	expected.extDNSEnabled = true
 	expected.Infoblox.Host = ""
 	expected.Infoblox.Version = ""
 	expected.Infoblox.Port = 0
@@ -766,8 +765,8 @@ func TestInfobloxGridHostIsEmptyButInfobloxPropsAreFilled(t *testing.T) {
 	// arrange
 	defer cleanup()
 	expected := predefinedConfig
-	expected.EdgeDNSType = DNSTypeRoute53
-	expected.route53Enabled = true
+	expected.EdgeDNSType = DNSTypeExternal
+	expected.extDNSEnabled = true
 	expected.Infoblox.Host = ""
 	expected.Infoblox.Version = defaultVersion
 	expected.Infoblox.Port = 443
@@ -782,7 +781,7 @@ func TestInfobloxGridHostIsUnset(t *testing.T) {
 	defer cleanup()
 	expected := predefinedConfig
 	expected.EdgeDNSType = DNSTypeNoEdgeDNS
-	expected.route53Enabled = false
+	expected.extDNSEnabled = false
 	expected.Infoblox.Host = ""
 	expected.Infoblox.Version = defaultVersion
 	expected.Infoblox.Port = 443
@@ -798,7 +797,7 @@ func TestInfobloxGridHostIsInvalid(t *testing.T) {
 	defer cleanup()
 	expected := predefinedConfig
 	expected.EdgeDNSType = DNSTypeInfoblox
-	expected.route53Enabled = false
+	expected.extDNSEnabled = false
 	expected.Infoblox.Host = "dnfkjdnf kj"
 	expected.Infoblox.Version = defaultVersion
 	expected.Infoblox.Port = 443
@@ -1387,7 +1386,7 @@ func arrangeVariablesAndAssert(t *testing.T, expected Config,
 
 func cleanup() {
 	for _, s := range []string{ReconcileRequeueSecondsKey, ClusterGeoTagKey, ExtClustersGeoTagsKey, EdgeDNSZoneKey, DNSZoneKey, EdgeDNSServersKey,
-		Route53EnabledKey, NS1EnabledKey, InfobloxGridHostKey, InfobloxVersionKey, InfobloxPortKey, InfobloxUsernameKey,
+		ExtDNSEnabledKey, InfobloxGridHostKey, InfobloxVersionKey, InfobloxPortKey, InfobloxUsernameKey,
 		InfobloxPasswordKey, K8gbNamespaceKey, CoreDNSExposedKey, InfobloxHTTPRequestTimeoutKey,
 		InfobloxHTTPPoolConnectionsKey, LogLevelKey, LogFormatKey, LogNoColorKey, MetricsAddressKey, SplitBrainCheckKey} {
 		if os.Unsetenv(s) != nil {
@@ -1404,8 +1403,7 @@ func configureEnvVar(config Config) {
 	_ = os.Setenv(EdgeDNSZoneKey, config.EdgeDNSZone)
 	_ = os.Setenv(DNSZoneKey, config.DNSZone)
 	_ = os.Setenv(K8gbNamespaceKey, config.K8gbNamespace)
-	_ = os.Setenv(Route53EnabledKey, strconv.FormatBool(config.route53Enabled))
-	_ = os.Setenv(NS1EnabledKey, strconv.FormatBool(config.ns1Enabled))
+	_ = os.Setenv(ExtDNSEnabledKey, strconv.FormatBool(config.extDNSEnabled))
 	_ = os.Setenv(CoreDNSExposedKey, strconv.FormatBool(config.CoreDNSExposed))
 	_ = os.Setenv(InfobloxGridHostKey, config.Infoblox.Host)
 	_ = os.Setenv(InfobloxVersionKey, config.Infoblox.Version)

controllers/gslb_controller_test.go

@@ -775,10 +775,10 @@ func TestDetectsIngressHostnameMismatch(t *testing.T) {
 	assert.True(t, strings.HasSuffix(err.Error(), "cloud.example.com does not match delegated zone otherdnszone.com"))
 }
 
-func TestCreatesNSDNSRecordsForRoute53(t *testing.T) {
+func TestCreatesDNSNSRecordsForExtDNS(t *testing.T) {
 	// arrange
 	const dnsZone = "cloud.example.com"
-	const want = "route53"
+	const want = "extdns"
 	wantEp := []*externaldns.Endpoint{
 		{
 			DNSName:    dnsZone,
@@ -800,80 +800,7 @@ func TestCreatesNSDNSRecordsForRoute53(t *testing.T) {
 			},
 		},
 	}
-	dnsEndpointRoute53 := &externaldns.DNSEndpoint{}
-	customConfig := predefinedConfig
-	customConfig.EdgeDNSServers = defaultEdgeDNSServers
-	customConfig.CoreDNSExposed = true
-	coreDNSService := &corev1.Service{
-		ObjectMeta: metav1.ObjectMeta{
-			Name:      defaultCoreDNSExtServiceName,
-			Namespace: predefinedConfig.K8gbNamespace,
-			Labels: map[string]string{
-				"app.kubernetes.io/name": "coredns",
-			},
-		},
-	}
-	serviceIPs := []corev1.LoadBalancerIngress{
-		{Hostname: "one.one.one.one"}, // rely on 1.1.1.1 response from Cloudflare
-	}
-	settings := provideSettings(t, customConfig)
-	err := settings.client.Create(context.TODO(), coreDNSService)
-	require.NoError(t, err, "Failed to create testing %s service", defaultCoreDNSExtServiceName)
-	coreDNSService.Status.LoadBalancer.Ingress = append(coreDNSService.Status.LoadBalancer.Ingress, serviceIPs...)
-	err = settings.client.Status().Update(context.TODO(), coreDNSService)
-	require.NoError(t, err, "Failed to update coredns service lb hostname")
-
-	// act
-	customConfig.EdgeDNSType = depresolver.DNSTypeRoute53
-	customConfig.ClusterGeoTag = "eu"
-	customConfig.ExtClustersGeoTags = []string{"za", "us"}
-	customConfig.DNSZone = dnsZone
-	// apply new environment variables and update config only
-	settings.reconciler.Config = &customConfig
-	// If config is changed, new Route53 provider needs to be re-created. There is no way and reason to change provider
-	// configuration at another time than startup
-	f, _ := dns.NewDNSProviderFactory(settings.reconciler.Client, customConfig)
-	settings.reconciler.DNSProvider = f.Provider()
-
-	reconcileAndUpdateGslb(t, settings)
-	err = settings.client.Get(context.TODO(), client.ObjectKey{Namespace: predefinedConfig.K8gbNamespace, Name: "k8gb-ns-route53"}, dnsEndpointRoute53)
-	require.NoError(t, err, "Failed to get expected DNSEndpoint")
-	got := dnsEndpointRoute53.Annotations["k8gb.absa.oss/dnstype"]
-	gotEp := dnsEndpointRoute53.Spec.Endpoints
-	prettyGot := str.ToString(gotEp)
-	prettyWant := str.ToString(wantEp)
-
-	// assert
-	assert.Equal(t, want, got, "got:\n %q annotation value,\n\n want:\n %q", got, want)
-	assert.Equal(t, wantEp, gotEp, "got:\n %s DNSEndpoint,\n\n want:\n %s", prettyGot, prettyWant)
-}
-
-func TestCreatesNSDNSRecordsForNS1(t *testing.T) {
-	// arrange
-	const dnsZone = "cloud.example.com"
-	const want = "ns1"
-	wantEp := []*externaldns.Endpoint{
-		{
-			DNSName:    dnsZone,
-			RecordTTL:  30,
-			RecordType: "NS",
-			Targets: externaldns.Targets{
-				"gslb-ns-eu-cloud.example.com",
-				"gslb-ns-us-cloud.example.com",
-				"gslb-ns-za-cloud.example.com",
-			},
-		},
-		{
-			DNSName:    "gslb-ns-eu-cloud.example.com",
-			RecordTTL:  30,
-			RecordType: "A",
-			Targets: externaldns.Targets{
-				defaultEdgeDNS0,
-				defaultEdgeDNS1,
-			},
-		},
-	}
-	dnsEndpointNS1 := &externaldns.DNSEndpoint{}
+	dnsEndpoint := &externaldns.DNSEndpoint{}
 	customConfig := predefinedConfig
 	customConfig.EdgeDNSServers = defaultEdgeDNSServers
 	customConfig.CoreDNSExposed = true
@@ -897,7 +824,7 @@ func TestCreatesNSDNSRecordsForNS1(t *testing.T) {
 	require.NoError(t, err, "Failed to update coredns service lb hostname")
 
 	// act
-	customConfig.EdgeDNSType = depresolver.DNSTypeNS1
+	customConfig.EdgeDNSType = depresolver.DNSTypeExternal
 	customConfig.ClusterGeoTag = "eu"
 	customConfig.ExtClustersGeoTags = []string{"za", "us"}
 	customConfig.DNSZone = dnsZone
@@ -909,10 +836,10 @@ func TestCreatesNSDNSRecordsForNS1(t *testing.T) {
 	settings.reconciler.DNSProvider = f.Provider()
 
 	reconcileAndUpdateGslb(t, settings)
-	err = settings.client.Get(context.TODO(), client.ObjectKey{Namespace: predefinedConfig.K8gbNamespace, Name: "k8gb-ns-ns1"}, dnsEndpointNS1)
+	err = settings.client.Get(context.TODO(), client.ObjectKey{Namespace: predefinedConfig.K8gbNamespace, Name: "k8gb-ns-extdns"}, dnsEndpoint)
 	require.NoError(t, err, "Failed to get expected DNSEndpoint")
-	got := dnsEndpointNS1.Annotations["k8gb.absa.oss/dnstype"]
-	gotEp := dnsEndpointNS1.Spec.Endpoints
+	got := dnsEndpoint.Annotations["k8gb.absa.oss/dnstype"]
+	gotEp := dnsEndpoint.Spec.Endpoints
 	prettyGot := str.ToString(gotEp)
 	prettyWant := str.ToString(wantEp)
 
@@ -984,7 +911,7 @@ func TestRoute53ZoneDelegationGarbageCollection(t *testing.T) {
 	require.NoError(t, err, "Failed to update coredns service lb hostname")
 
 	// act
-	customConfig.EdgeDNSType = depresolver.DNSTypeRoute53
+	customConfig.EdgeDNSType = depresolver.DNSTypeExternal
 	// apply new environment variables and update config only
 	settings.reconciler.Config = &customConfig
 	reconcileAndUpdateGslb(t, settings)

controllers/providers/dns/external.go

@@ -32,28 +32,21 @@ import (
 	externaldns "sigs.k8s.io/external-dns/endpoint"
 )
 
-type ExternalDNSType string
-
-const (
-	externalDNSTypeNS1     ExternalDNSType = "ns1"
-	externalDNSTypeRoute53 ExternalDNSType = "route53"
-)
+const externalDNSTypeCommon = "extdns"
 
 type ExternalDNSProvider struct {
 	assistant    assistant2.Assistant
-	dnsType      ExternalDNSType
 	config       depresolver.Config
 	endpointName string
 }
 
 var log = logging.Logger()
 
-func NewExternalDNS(dnsType ExternalDNSType, config depresolver.Config, assistant assistant2.Assistant) *ExternalDNSProvider {
+func NewExternalDNS(config depresolver.Config, assistant assistant2.Assistant) *ExternalDNSProvider {
 	return &ExternalDNSProvider{
 		assistant:    assistant,
-		dnsType:      dnsType,
 		config:       config,
-		endpointName: fmt.Sprintf("k8gb-ns-%s", dnsType),
+		endpointName: fmt.Sprintf("k8gb-ns-%s", externalDNSTypeCommon),
 	}
 }
 
@@ -81,7 +74,7 @@ func (p *ExternalDNSProvider) CreateZoneDelegationForExternalDNS(gslb *k8gbv1bet
 		ObjectMeta: metav1.ObjectMeta{
 			Name:        p.endpointName,
 			Namespace:   p.config.K8gbNamespace,
-			Annotations: map[string]string{"k8gb.absa.oss/dnstype": string(p.dnsType)},
+			Annotations: map[string]string{"k8gb.absa.oss/dnstype": externalDNSTypeCommon},
 		},
 		Spec: externaldns.DNSEndpointSpec{
 			Endpoints: []*externaldns.Endpoint{
@@ -124,5 +117,5 @@ func (p *ExternalDNSProvider) SaveDNSEndpoint(gslb *k8gbv1beta1.Gslb, i *externa
 }
 
 func (p *ExternalDNSProvider) String() string {
-	return strings.ToUpper(string(p.dnsType))
+	return strings.ToUpper(externalDNSTypeCommon)
 }