Merge pull request #116 from mikenairn/update_inmemory_provider

Update inmemory provider

internal/controller/dnsrecord_controller_test.go

@@ -26,6 +26,7 @@ import (
 	. "github.com/onsi/gomega"
 	. "github.com/onsi/gomega/gstruct"
 
+	v1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	externaldnsendpoint "sigs.k8s.io/external-dns/endpoint"
@@ -36,13 +37,17 @@ import (
 var _ = Describe("DNSRecordReconciler", func() {
 	var dnsRecord *v1alpha1.DNSRecord
 	var dnsRecord2 *v1alpha1.DNSRecord
+	var dnsProviderSecret *v1.Secret
 	var managedZone *v1alpha1.ManagedZone
 	var testNamespace string
 
 	BeforeEach(func() {
 		CreateNamespace(&testNamespace)
 
-		managedZone = testBuildManagedZone("mz-example-com", testNamespace, "example.com")
+		dnsProviderSecret = testBuildInMemoryCredentialsSecret("inmemory-credentials", testNamespace)
+		managedZone = testBuildManagedZone("mz-example-com", testNamespace, "example.com", dnsProviderSecret.Name)
+
+		Expect(k8sClient.Create(ctx, dnsProviderSecret)).To(Succeed())
 		Expect(k8sClient.Create(ctx, managedZone)).To(Succeed())
 		Eventually(func(g Gomega) {
 			err := k8sClient.Get(ctx, client.ObjectKeyFromObject(managedZone), managedZone)

internal/controller/helper_test.go

@@ -5,6 +5,7 @@ package controller
 import (
 	"time"
 
+	v1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	externaldnsendpoint "sigs.k8s.io/external-dns/endpoint"
 
@@ -17,27 +18,35 @@ const (
 	TestRetryIntervalMedium = time.Millisecond * 250
 	RequeueDuration         = time.Second * 6
 	ValidityDuration        = time.Second * 3
-	defaultNS               = "default"
-	providerCredential      = "secretname"
 )
 
-func testBuildManagedZone(name, ns, domainName string) *kuadrantdnsv1alpha1.ManagedZone {
+func testBuildManagedZone(name, ns, domainName, secretName string) *kuadrantdnsv1alpha1.ManagedZone {
 	return &kuadrantdnsv1alpha1.ManagedZone{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      name,
 			Namespace: ns,
 		},
 		Spec: kuadrantdnsv1alpha1.ManagedZoneSpec{
-			ID:          "1234",
 			DomainName:  domainName,
 			Description: domainName,
 			SecretRef: kuadrantdnsv1alpha1.ProviderRef{
-				Name: "secretname",
+				Name: secretName,
 			},
 		},
 	}
 }
 
+func testBuildInMemoryCredentialsSecret(name, ns string) *v1.Secret {
+	return &v1.Secret{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      name,
+			Namespace: ns,
+		},
+		Data: map[string][]byte{},
+		Type: "kuadrant.io/inmemory",
+	}
+}
+
 func getTestEndpoints() []*externaldnsendpoint.Endpoint {
 	return []*externaldnsendpoint.Endpoint{
 		{

internal/controller/managedzone_controller_test.go

@@ -19,11 +19,13 @@ limitations under the License.
 package controller
 
 import (
-	"context"
+	"time"
 
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
+	. "github.com/onsi/gomega/gstruct"
 
+	v1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -34,62 +36,65 @@ import (
 
 var _ = Describe("ManagedZoneReconciler", func() {
 	Context("testing ManagedZone controller", func() {
+		var dnsProviderSecret *v1.Secret
 		var managedZone *v1alpha1.ManagedZone
-		var ctx context.Context
+		var testNamespace string
 
 		BeforeEach(func() {
-			ctx = context.Background()
-			managedZone = &v1alpha1.ManagedZone{
-				ObjectMeta: metav1.ObjectMeta{
-					Name:      "example.com",
-					Namespace: defaultNS,
-				},
-				Spec: v1alpha1.ManagedZoneSpec{
-					ID:         "example.com",
-					DomainName: "example.com",
-					SecretRef: v1alpha1.ProviderRef{
-						Name: providerCredential,
-					},
-				},
-			}
+			CreateNamespace(&testNamespace)
+
+			dnsProviderSecret = testBuildInMemoryCredentialsSecret("inmemory-credentials", testNamespace)
+			managedZone = testBuildManagedZone("mz-example-com", testNamespace, "example.com", dnsProviderSecret.Name)
+
+			Expect(k8sClient.Create(ctx, dnsProviderSecret)).To(Succeed())
 		})
 
 		AfterEach(func() {
 			// Clean up managedZones
 			mzList := &v1alpha1.ManagedZoneList{}
-			err := k8sClient.List(ctx, mzList, client.InNamespace(defaultNS))
+			err := k8sClient.List(ctx, mzList, client.InNamespace(testNamespace))
 			Expect(err).NotTo(HaveOccurred())
 			for _, mz := range mzList.Items {
 				err = k8sClient.Delete(ctx, &mz)
 				Expect(client.IgnoreNotFound(err)).NotTo(HaveOccurred())
 			}
+
+			if dnsProviderSecret != nil {
+				err := k8sClient.Delete(ctx, dnsProviderSecret)
+				Expect(client.IgnoreNotFound(err)).ToNot(HaveOccurred())
+			}
 		})
 
 		It("should accept a managed zone for this controller and allow deletion", func() {
-			Expect(k8sClient.Create(ctx, managedZone)).To(BeNil())
-
-			createdMZ := &v1alpha1.ManagedZone{}
-
-			Eventually(func() error {
-				return k8sClient.Get(ctx, client.ObjectKey{Namespace: managedZone.Namespace, Name: managedZone.Name}, createdMZ)
-			}, TestTimeoutMedium, TestRetryIntervalMedium).ShouldNot(HaveOccurred())
-
-			Expect(k8sClient.Delete(ctx, managedZone)).To(BeNil())
-
+			Expect(k8sClient.Create(ctx, managedZone)).To(Succeed())
+
+			Eventually(func(g Gomega) {
+				err := k8sClient.Get(ctx, client.ObjectKeyFromObject(managedZone), managedZone)
+				g.Expect(err).NotTo(HaveOccurred())
+				g.Expect(managedZone.Status.Conditions).To(
+					ContainElement(MatchFields(IgnoreExtras, Fields{
+						"Type":               Equal(string(v1alpha1.ConditionTypeReady)),
+						"Status":             Equal(metav1.ConditionTrue),
+						"ObservedGeneration": Equal(managedZone.Generation),
+					})),
+				)
+			}, TestTimeoutMedium, time.Second).Should(Succeed())
+
+			Expect(k8sClient.Delete(ctx, managedZone)).To(Succeed())
 			Eventually(func() error {
-				err := k8sClient.Get(ctx, client.ObjectKey{Namespace: managedZone.Namespace, Name: managedZone.Name}, createdMZ)
+				err := k8sClient.Get(ctx, client.ObjectKeyFromObject(managedZone), managedZone)
 				if err != nil && !errors.IsNotFound(err) {
 					return err
 				}
 				return nil
-			}, TestTimeoutMedium, TestRetryIntervalMedium).Should(BeNil())
+			}, TestTimeoutMedium, TestRetryIntervalMedium).Should(Succeed())
 		})
 
 		It("should reject a managed zone with an invalid domain name", func() {
 			invalidDomainNameManagedZone := &v1alpha1.ManagedZone{
 				ObjectMeta: metav1.ObjectMeta{
 					Name:      "invalid_domain",
-					Namespace: defaultNS,
+					Namespace: testNamespace,
 				},
 				Spec: v1alpha1.ManagedZoneSpec{
 					ID:         "invalid_domain",
@@ -100,5 +105,68 @@ var _ = Describe("ManagedZoneReconciler", func() {
 			Expect(err).To(HaveOccurred())
 			Expect(err.Error()).To(ContainSubstring("spec.domainName in body should match"))
 		})
+
+		It("managed zone should not become ready with a spec.ID that does not exist", func() {
+			//Create a managed zone with no spec.ID
+			Expect(k8sClient.Create(ctx, managedZone)).To(Succeed())
+			Eventually(func(g Gomega) {
+				err := k8sClient.Get(ctx, client.ObjectKeyFromObject(managedZone), managedZone)
+				g.Expect(err).NotTo(HaveOccurred())
+				g.Expect(managedZone.Status.ID).To(Equal("example.com"))
+				g.Expect(managedZone.Status.Conditions).To(
+					ContainElement(MatchFields(IgnoreExtras, Fields{
+						"Type":               Equal(string(v1alpha1.ConditionTypeReady)),
+						"Status":             Equal(metav1.ConditionTrue),
+						"ObservedGeneration": Equal(managedZone.Generation),
+					})),
+				)
+			}, TestTimeoutMedium, time.Second).Should(Succeed())
+
+			//Create a second managed zone with spec.ID referencing a zone id that does not exist
+			mz := testBuildManagedZone("mz-example-com-2", testNamespace, "example.com", dnsProviderSecret.Name)
+			mz.Spec.ID = "dosnotexist"
+			Expect(k8sClient.Create(ctx, mz)).To(Succeed())
+			Eventually(func(g Gomega) error {
+				err := k8sClient.Get(ctx, client.ObjectKey{Namespace: mz.Namespace, Name: mz.Name}, mz)
+				g.Expect(err).NotTo(HaveOccurred())
+				g.Expect(mz.Status.Conditions).To(
+					ContainElement(MatchFields(IgnoreExtras, Fields{
+						"Type":   Equal(string(v1alpha1.ConditionTypeReady)),
+						"Status": Equal(metav1.ConditionFalse),
+						"Reason": Equal("ProviderError"),
+						"Message": And(
+							ContainSubstring("The DNS provider failed to ensure the managed zone"),
+							ContainSubstring("not found")),
+					})),
+				)
+				g.Expect(mz.Finalizers).To(ContainElement(ManagedZoneFinalizer))
+				return nil
+			}, TestTimeoutMedium, time.Second).Should(Succeed())
+
+			//Update second managed zone to use the known existing managed zone id (managedZone.Status.ID)
+			Eventually(func(g Gomega) {
+				err := k8sClient.Get(ctx, client.ObjectKeyFromObject(mz), mz)
+				g.Expect(err).NotTo(HaveOccurred())
+
+				mz.Spec.ID = managedZone.Status.ID
+				err = k8sClient.Update(ctx, mz)
+				g.Expect(err).NotTo(HaveOccurred())
+			}, TestTimeoutMedium, time.Second).Should(Succeed())
+
+			Eventually(func(g Gomega) error {
+				err := k8sClient.Get(ctx, client.ObjectKey{Namespace: mz.Namespace, Name: mz.Name}, mz)
+				Expect(err).NotTo(HaveOccurred())
+				g.Expect(mz.Status.Conditions).To(
+					ContainElement(MatchFields(IgnoreExtras, Fields{
+						"Type":               Equal(string(v1alpha1.ConditionTypeReady)),
+						"Status":             Equal(metav1.ConditionTrue),
+						"Reason":             Equal("ProviderSuccess"),
+						"ObservedGeneration": Equal(mz.Generation),
+					})),
+				)
+				g.Expect(mz.Finalizers).To(ContainElement(ManagedZoneFinalizer))
+				return nil
+			}, TestTimeoutMedium, time.Second).Should(Succeed())
+		})
 	})
 })

internal/controller/suite_test.go

@@ -44,9 +44,8 @@ import (
 	"github.com/kuadrant/dns-operator/api/v1alpha1"
 	"github.com/kuadrant/dns-operator/internal/provider"
 	_ "github.com/kuadrant/dns-operator/internal/provider/aws"
-	providerFake "github.com/kuadrant/dns-operator/internal/provider/fake"
 	_ "github.com/kuadrant/dns-operator/internal/provider/google"
-	"github.com/kuadrant/dns-operator/internal/provider/inmemory"
+	_ "github.com/kuadrant/dns-operator/internal/provider/inmemory"
 	//+kubebuilder:scaffold:imports
 )
 
@@ -59,11 +58,6 @@ var k8sClient client.Client
 var testEnv *envtest.Environment
 var ctx context.Context
 var cancel context.CancelFunc
-var dnsProviderFactory = &providerFake.Factory{
-	ProviderForFunc: func(ctx context.Context, pa v1alpha1.ProviderAccessor, c provider.Config) (provider.Provider, error) {
-		return dnsProvider, nil
-	},
-}
 
 func TestControllers(t *testing.T) {
 	RegisterFailHandler(Fail)
@@ -105,23 +99,22 @@ var _ = BeforeSuite(func() {
 	})
 	Expect(err).ToNot(HaveOccurred())
 
+	providerFactory := provider.NewFactory(mgr.GetClient())
+
 	err = (&ManagedZoneReconciler{
 		Client:          mgr.GetClient(),
 		Scheme:          mgr.GetScheme(),
-		ProviderFactory: dnsProviderFactory,
+		ProviderFactory: providerFactory,
 	}).SetupWithManager(mgr)
 	Expect(err).ToNot(HaveOccurred())
 
 	err = (&DNSRecordReconciler{
 		Client:          mgr.GetClient(),
 		Scheme:          mgr.GetScheme(),
-		ProviderFactory: dnsProviderFactory,
+		ProviderFactory: providerFactory,
 	}).SetupWithManager(mgr, RequeueDuration, ValidityDuration)
 	Expect(err).ToNot(HaveOccurred())
 
-	dnsProvider, err = inmemory.NewProviderFromSecret(ctx, &v1.Secret{}, provider.Config{})
-	Expect(err).ToNot(HaveOccurred())
-
 	go func() {
 		defer GinkgoRecover()
 		err = mgr.Start(ctx)

internal/external-dns/provider/inmemory/inmemory.go

@@ -128,6 +128,11 @@ func (im *InMemoryProvider) DeleteZone(zone string) error {
 	return im.client.DeleteZone(zone)
 }
 
+// GetZone gets a Zone if present
+func (im *InMemoryProvider) GetZone(zone string) (Zone, error) {
+	return im.client.GetZone(zone)
+}
+
 // Zones returns filtered zones as specified by domain
 func (im *InMemoryProvider) Zones() map[string]string {
 	return im.filter.Zones(im.client.Zones())
@@ -300,6 +305,13 @@ func (c *InMemoryClient) DeleteZone(zone string) error {
 	return ErrZoneNotFound
 }
 
+func (c *InMemoryClient) GetZone(zone string) (Zone, error) {
+	if _, ok := c.zones[zone]; ok {
+		return c.zones[zone], nil
+	}
+	return nil, ErrZoneNotFound
+}
+
 func (c *InMemoryClient) ApplyChanges(ctx context.Context, zoneID string, changes *plan.Changes) error {
 	if err := c.validateChangeBatch(zoneID, changes); err != nil {
 		return err

internal/provider/inmemory/inmemory.go

@@ -44,13 +44,28 @@ func NewProviderFromSecret(ctx context.Context, _ *v1.Secret, c provider.Config)
 	return p, nil
 }
 
-func (i InMemoryDNSProvider) EnsureManagedZone(managedZone *v1alpha1.ManagedZone) (provider.ManagedZoneOutput, error) {
-	_ = i.CreateZone(managedZone.Spec.DomainName)
+func (i InMemoryDNSProvider) EnsureManagedZone(mz *v1alpha1.ManagedZone) (provider.ManagedZoneOutput, error) {
+	var zoneID string
+	if mz.Spec.ID != "" {
+		zoneID = mz.Spec.ID
+	} else {
+		zoneID = mz.Status.ID
+	}
+
+	if zoneID != "" {
+		_, err := i.GetZone(zoneID)
+		return provider.ManagedZoneOutput{
+			ID:          zoneID,
+			NameServers: nil,
+			RecordCount: 0,
+		}, err
+	}
+	err := i.CreateZone(mz.Spec.DomainName)
 	return provider.ManagedZoneOutput{
-		ID:          managedZone.Spec.DomainName,
+		ID:          mz.Spec.DomainName,
 		NameServers: nil,
 		RecordCount: 0,
-	}, nil
+	}, err
 }
 
 func (i InMemoryDNSProvider) DeleteManagedZone(managedZone *v1alpha1.ManagedZone) error {