Refactor to providers

controllers/finalize.go

@@ -2,82 +2,21 @@ package controllers
 
 import (
 	"context"
-	"fmt"
-
-	"github.com/AbsaOSS/k8gb/controllers/depresolver"
 
 	k8gbv1beta1 "github.com/AbsaOSS/k8gb/api/v1beta1"
-	"k8s.io/apimachinery/pkg/api/errors"
-	"sigs.k8s.io/controller-runtime/pkg/client"
-	externaldns "sigs.k8s.io/external-dns/endpoint"
 )
 
-func (r *GslbReconciler) finalizeGslb(gslb *k8gbv1beta1.Gslb) error {
+func (r *GslbReconciler) finalizeGslb(gslb *k8gbv1beta1.Gslb) (err error) {
 	// needs to do before the CR can be deleted. Examples
 	// of finalizers include performing backups and deleting
 	// resources that are not owned by this CR, like a PVC.
-
-	if r.Config.EdgeDNSType == depresolver.DNSTypeRoute53 {
-		log.Info("Removing Zone Delegation entries...")
-		dnsEndpointRoute53 := &externaldns.DNSEndpoint{}
-		err := r.Get(context.Background(), client.ObjectKey{Namespace: r.Config.K8gbNamespace, Name: "k8gb-ns-route53"}, dnsEndpointRoute53)
-		if err != nil {
-			if errors.IsNotFound(err) {
-				log.Info(fmt.Sprint(err))
-				return nil
-			}
-			return err
-		}
-		err = r.Delete(context.Background(), dnsEndpointRoute53)
-		if err != nil {
-			return err
-		}
-	}
-
-	if r.Config.EdgeDNSType == depresolver.DNSTypeInfoblox {
-		objMgr, err := infobloxConnection(r.Config)
-		if err != nil {
-			return err
-		}
-		findZone, err := objMgr.GetZoneDelegated(r.Config.DNSZone)
-		if err != nil {
-			return err
-		}
-
-		if findZone != nil {
-			err = checkZoneDelegated(findZone, r.Config.DNSZone)
-			if err != nil {
-				return err
-			}
-			if len(findZone.Ref) > 0 {
-				log.Info(fmt.Sprintf("Deleting delegated zone(%s)...", r.Config.DNSZone))
-				_, err := objMgr.DeleteZoneDelegated(findZone.Ref)
-				if err != nil {
-					return err
-				}
-			}
-		}
-
-		heartbeatTXTName := fmt.Sprintf("%s-heartbeat-%s.%s", gslb.Name, r.Config.ClusterGeoTag, r.Config.EdgeDNSZone)
-		findTXT, err := objMgr.GetTXTRecord(heartbeatTXTName)
-		if err != nil {
-			return err
-		}
-
-		if findTXT != nil {
-			if len(findTXT.Ref) > 0 {
-				log.Info(fmt.Sprintf("Deleting split brain TXT record(%s)...", heartbeatTXTName))
-				_, err := objMgr.DeleteTXTRecord(findTXT.Ref)
-				if err != nil {
-					return err
-				}
-
-			}
-		}
+	err = r.DNSProvider.Finalize(gslb)
+	if err != nil {
+		log.Error(err, "Can't finalize GSLB (%s)")
+		return
 	}
-
 	log.Info("Successfully finalized Gslb")
-	return nil
+	return
 }
 
 func (r *GslbReconciler) addFinalizer(gslb *k8gbv1beta1.Gslb) error {

controllers/gslb_controller.go

@@ -21,7 +21,9 @@ import (
 	"fmt"
 	"time"
 
-	"github.com/AbsaOSS/k8gb/controllers/metrics"
+	"github.com/AbsaOSS/k8gb/controllers/providers/dns"
+
+	"github.com/AbsaOSS/k8gb/controllers/providers/metrics"
 
 	"github.com/AbsaOSS/k8gb/controllers/depresolver"
 	"github.com/go-logr/logr"
@@ -52,6 +54,7 @@ type GslbReconciler struct {
 	Config      *depresolver.Config
 	DepResolver *depresolver.DependencyResolver
 	Metrics     *metrics.PrometheusMetrics
+	DNSProvider dns.IDnsProvider
 }
 
 const (
@@ -142,14 +145,13 @@ func (r *GslbReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) {
 		return ctrl.Result{}, err
 	}
 
-	result, err = r.ensureDNSEndpoint(gslb.Namespace, dnsEndpoint)
+	result, err = r.DNSProvider.SaveDNSEndpoint(gslb, dnsEndpoint)
 	if result != nil {
 		return *result, err
 	}
 
 	// == handle delegated zone in Edge DNS
-
-	result, err = r.configureZoneDelegation(gslb)
+	result, err = r.DNSProvider.CreateZoneDelegationForExternalDNS(gslb)
 	if result != nil {
 		return *result, err
 	}

controllers/gslb_controller_test.go

@@ -10,22 +10,20 @@ import (
 	"testing"
 	"time"
 
-	"github.com/prometheus/client_golang/prometheus"
-	"github.com/prometheus/client_golang/prometheus/testutil"
-
-	"github.com/AbsaOSS/k8gb/controllers/metrics"
-
-	"github.com/stretchr/testify/require"
-
-	ibclient "github.com/infobloxopen/infoblox-go-client"
-	"github.com/stretchr/testify/assert"
-	"k8s.io/apimachinery/pkg/api/errors"
+	"github.com/AbsaOSS/k8gb/controllers/providers/assistant"
 
 	k8gbv1beta1 "github.com/AbsaOSS/k8gb/api/v1beta1"
 	"github.com/AbsaOSS/k8gb/controllers/depresolver"
 	"github.com/AbsaOSS/k8gb/controllers/internal/utils"
+	"github.com/AbsaOSS/k8gb/controllers/providers/dns"
+	"github.com/AbsaOSS/k8gb/controllers/providers/metrics"
+	"github.com/prometheus/client_golang/prometheus"
+	"github.com/prometheus/client_golang/prometheus/testutil"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/api/extensions/v1beta1"
+	"k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
@@ -48,6 +46,7 @@ type testSettings struct {
 	client     client.Client
 	ingress    *v1beta1.Ingress
 	finalCall  bool
+	assistant  assistant.IAssistant
 }
 
 var crSampleYaml = "../deploy/crds/k8gb.absa.oss_v1beta1_gslb_cr.yaml"
@@ -72,6 +71,8 @@ var predefinedConfig = depresolver.Config{
 	},
 }
 
+const coreDNSExtServiceName = "k8gb-coredns-lb"
+
 func TestNotFoundServiceStatus(t *testing.T) {
 	// arrange
 	defer cleanup()
@@ -384,20 +385,6 @@ func TestLocalDNSRecordsHasSpecialAnnotation(t *testing.T) {
 	assert.Equal(t, got, want, "got:\n %q annotation value,\n\n want:\n %q", got, want, got, want)
 }
 
-func TestGeneratesProperExternalNSTargetFQDNsAccordingToTheGeoTags(t *testing.T) {
-	// arrange
-	defer cleanup()
-	want := []string{"gslb-ns-cloud-example-com-za.example.com"}
-	customConfig := predefinedConfig
-	customConfig.EdgeDNSZone = "example.com"
-	customConfig.ExtClustersGeoTags = []string{"za"}
-	settings := provideSettings(t, customConfig)
-	// act
-	got := settings.reconciler.nsServerNameExt()
-	// assert
-	assert.Equal(t, want, got, "got:\n %q externalGslb NS records,\n\n want:\n %q", got, want)
-}
-
 func TestCanGetExternalTargetsFromK8gbInAnotherLocation(t *testing.T) {
 	// arrange
 	defer cleanup()
@@ -454,61 +441,24 @@ func TestCanCheckExternalGslbTXTRecordForValidityAndFailIfItIsExpired(t *testing
 	customConfig := predefinedConfig
 	customConfig.Override.FakeDNSEnabled = true
 	customConfig.EdgeDNSServer = "fake"
-	// act
-	got := checkAliveFromTXT("test-gslb-heartbeat-eu.example.com", &customConfig, time.Minute*5)
-	want := errors.NewGone("Split brain TXT record expired the time threshold: (5m0s)")
-	// assert
-	assert.Equal(t, want, got, "got:\n %s from TXT split brain check,\n\n want error:\n %v", got, want)
-}
-
-func TestCanFilterOutDelegatedZoneEntryAccordingFQDNProvided(t *testing.T) {
-	// arrange
-	defer cleanup()
-	delegateTo := []ibclient.NameServer{
-		{Address: "10.0.0.1", Name: "gslb-ns-cloud-example-com-eu.example.com"},
-		{Address: "10.0.0.2", Name: "gslb-ns-cloud-example-com-eu.example.com"},
-		{Address: "10.0.0.3", Name: "gslb-ns-cloud-example-com-eu.example.com"},
-		{Address: "10.1.0.1", Name: "gslb-ns-cloud-example-com-za.example.com"},
-		{Address: "10.1.0.2", Name: "gslb-ns-cloud-example-com-za.example.com"},
-		{Address: "10.1.0.3", Name: "gslb-ns-cloud-example-com-za.example.com"},
-	}
-	want := []ibclient.NameServer{
-		{Address: "10.0.0.1", Name: "gslb-ns-cloud-example-com-eu.example.com"},
-		{Address: "10.0.0.2", Name: "gslb-ns-cloud-example-com-eu.example.com"},
-		{Address: "10.0.0.3", Name: "gslb-ns-cloud-example-com-eu.example.com"},
-	}
-	customConfig := predefinedConfig
-	customConfig.EdgeDNSZone = "example.com"
-	customConfig.ExtClustersGeoTags = []string{"za"}
 	settings := provideSettings(t, customConfig)
 	// act
-	extClusters := settings.reconciler.nsServerNameExt()
-	got := filterOutDelegateTo(delegateTo, extClusters[0])
+	got := settings.assistant.InspectTXTThreshold("test-gslb-heartbeat-eu.example.com",
+		customConfig.Override.FakeDNSEnabled, time.Minute*5)
+	want := errors.NewResourceExpired("Split brain TXT record expired the time threshold: (5m0s)")
 	// assert
-	assert.Equal(t, want, got, "got:\n %q filtered out delegation records,\n\n want:\n %q", got, want)
-}
-
-func TestCanGenerateExternalHeartbeatFQDNs(t *testing.T) {
-	// arrange
-	defer cleanup()
-	want := []string{"test-gslb-heartbeat-za.example.com"}
-	customConfig := predefinedConfig
-	customConfig.EdgeDNSZone = "example.com"
-	customConfig.ExtClustersGeoTags = []string{"za"}
-	settings := provideSettings(t, customConfig)
-	// act
-	got := getExternalClusterHeartbeatFQDNs(settings.gslb, &settings.config)
-	// assert
-	assert.Equal(t, want, got, "got:\n %s unexpected heartbeat records,\n\n want:\n %s", got, want)
+	assert.Equal(t, want, got, "got:\n %s from TXT split brain check,\n\n want error:\n %v", got, want)
 }
 
 func TestCanCheckExternalGslbTXTRecordForValidityAndPAssIfItISNotExpired(t *testing.T) {
 	// arrange
 	customConfig := predefinedConfig
 	customConfig.Override.FakeDNSEnabled = true
 	customConfig.EdgeDNSServer = "fake"
+	settings := provideSettings(t, customConfig)
 	// act
-	err2 := checkAliveFromTXT("test-gslb-heartbeat-za.example.com", &customConfig, time.Minute*5)
+	err2 := settings.assistant.InspectTXTThreshold("test-gslb-heartbeat-za.example.com",
+		customConfig.Override.FakeDNSEnabled, time.Minute*5)
 	// assert
 	assert.NoError(t, err2, "got:\n %s from TXT split brain check,\n\n want error:\n %v", err2, nil)
 }
@@ -742,6 +692,10 @@ func TestCreatesNSDNSRecordsForRoute53(t *testing.T) {
 	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.Log)
+	settings.reconciler.DNSProvider = f.Provider()
 
 	reconcileAndUpdateGslb(t, settings)
 	err = settings.client.Get(context.TODO(), client.ObjectKey{Namespace: predefinedConfig.K8gbNamespace, Name: "k8gb-ns-route53"}, dnsEndpointRoute53)
@@ -809,6 +763,10 @@ func TestCreatesNSDNSRecordsForNS1(t *testing.T) {
 	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.Log)
+	settings.reconciler.DNSProvider = f.Provider()
 
 	reconcileAndUpdateGslb(t, settings)
 	err = settings.client.Get(context.TODO(), client.ObjectKey{Namespace: predefinedConfig.K8gbNamespace, Name: "k8gb-ns-ns1"}, dnsEndpointNS1)
@@ -1118,6 +1076,14 @@ func provideSettings(t *testing.T, expected depresolver.Config) (settings testSe
 			Namespace: gslb.Namespace,
 		},
 	}
+
+	var f *dns.ProviderFactory
+	f, err = dns.NewDNSProviderFactory(r.Client, *r.Config, r.Log)
+	if err != nil {
+		t.Fatalf("reconcile: (%v)", err)
+	}
+	r.DNSProvider = f.Provider()
+	a := assistant.NewGslbAssistant(r.Client, r.Log, r.Config.K8gbNamespace, r.Config.EdgeDNSServer)
 	res, err := r.Reconcile(req)
 	if err != nil {
 		t.Fatalf("reconcile: (%v)", err)
@@ -1131,6 +1097,7 @@ func provideSettings(t *testing.T, expected depresolver.Config) (settings testSe
 	if err != nil {
 		t.Fatalf("Failed to get expected ingress: (%v)", err)
 	}
+
 	// Reconcile again so Reconcile() checks services and updates the Gslb
 	// resources' Status.
 	settings = testSettings{
@@ -1141,6 +1108,7 @@ func provideSettings(t *testing.T, expected depresolver.Config) (settings testSe
 		client:     cl,
 		ingress:    ingress,
 		finalCall:  false,
+		assistant:  a,
 	}
 	reconcileAndUpdateGslb(t, settings)
 	return settings

controllers/internal/utils/reflection.go

@@ -0,0 +1,11 @@
+package utils
+
+import "reflect"
+
+func GetType(v interface{}) string {
+	t := reflect.TypeOf(v)
+	if t.Kind() == reflect.Ptr {
+		return "*" + t.Elem().Name()
+	}
+	return t.Name()
+}

controllers/internal/utils/reflection_test.go

@@ -0,0 +1,36 @@
+package utils
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestGetTypeStruct(t *testing.T) {
+	// arrange
+	type ab struct{}
+	x := ab{}
+	// act
+	r := GetType(x)
+	// assert
+	assert.Equal(t, "ab", r)
+}
+
+func TestGetTypePrimitive(t *testing.T) {
+	// arrange
+	x := 5
+	// act
+	r := GetType(x)
+	// assert
+	assert.Equal(t, "int", r)
+}
+
+func TestGetTypePointer(t *testing.T) {
+	// arrange
+	type ab struct{}
+	x := &ab{}
+	// act
+	r := GetType(x)
+	// assert
+	assert.Equal(t, "*ab", r)
+}