From 1896ae336e1c4b8700cff07c9e35cc2d766c3848 Mon Sep 17 00:00:00 2001
From: Stefan Prodan <stefan.prodan@gmail.com>
Date: Mon, 7 Nov 2022 16:59:27 +0200
Subject: [PATCH] Add reconciliation interval to providers and receivers
 Periodically reconcile providers and receivers with their Secret references
 to surface config errors after initialisation.

Signed-off-by: Stefan Prodan <stefan.prodan@gmail.com>
---
 api/v1beta2/provider_types.go                 |  7 +++
 api/v1beta2/receiver_types.go                 |  7 +++
 api/v1beta2/zz_generated.deepcopy.go          |  2 +
 ...ification.toolkit.fluxcd.io_providers.yaml |  7 +++
 ...ification.toolkit.fluxcd.io_receivers.yaml |  7 +++
 controllers/alert_controller_test.go          |  4 ++
 controllers/provider_controller.go            |  2 +-
 controllers/provider_controller_test.go       |  5 ++
 controllers/receiver_controller.go            |  2 +-
 controllers/receiver_controller_test.go       |  8 +++
 docs/api/notification.md                      | 52 +++++++++++++++++++
 docs/spec/v1beta2/providers.md                |  6 +++
 docs/spec/v1beta2/receivers.md                |  6 +++
 13 files changed, 113 insertions(+), 2 deletions(-)

diff --git a/api/v1beta2/provider_types.go b/api/v1beta2/provider_types.go
index b2cc29a73..57a30ee17 100644
--- a/api/v1beta2/provider_types.go
+++ b/api/v1beta2/provider_types.go
@@ -55,6 +55,13 @@ type ProviderSpec struct {
 	// +required
 	Type string `json:"type"`
 
+	// Interval at which to reconcile the Provider with its Secret references.
+	// +kubebuilder:default="10m"
+	// +kubebuilder:validation:Type=string
+	// +kubebuilder:validation:Pattern="^([0-9]+(\\.[0-9]+)?(ms|s|m|h))+$"
+	// +required
+	Interval metav1.Duration `json:"interval"`
+
 	// Channel specifies the destination channel where events should be posted.
 	// +kubebuilder:validation:MaxLength:=2048
 	// +optional
diff --git a/api/v1beta2/receiver_types.go b/api/v1beta2/receiver_types.go
index 7465d4810..cdaaaef92 100644
--- a/api/v1beta2/receiver_types.go
+++ b/api/v1beta2/receiver_types.go
@@ -49,6 +49,13 @@ type ReceiverSpec struct {
 	// +required
 	Type string `json:"type"`
 
+	// Interval at which to reconcile the Receiver with its Secret references.
+	// +kubebuilder:default="10m"
+	// +kubebuilder:validation:Type=string
+	// +kubebuilder:validation:Pattern="^([0-9]+(\\.[0-9]+)?(ms|s|m|h))+$"
+	// +required
+	Interval metav1.Duration `json:"interval"`
+
 	// Events specifies the list of event types to handle,
 	// e.g. 'push' for GitHub or 'Push Hook' for GitLab.
 	// +optional
diff --git a/api/v1beta2/zz_generated.deepcopy.go b/api/v1beta2/zz_generated.deepcopy.go
index 4d8542c9c..6d9527e4f 100644
--- a/api/v1beta2/zz_generated.deepcopy.go
+++ b/api/v1beta2/zz_generated.deepcopy.go
@@ -221,6 +221,7 @@ func (in *ProviderList) DeepCopyObject() runtime.Object {
 // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
 func (in *ProviderSpec) DeepCopyInto(out *ProviderSpec) {
 	*out = *in
+	out.Interval = in.Interval
 	if in.Timeout != nil {
 		in, out := &in.Timeout, &out.Timeout
 		*out = new(v1.Duration)
@@ -333,6 +334,7 @@ func (in *ReceiverList) DeepCopyObject() runtime.Object {
 // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
 func (in *ReceiverSpec) DeepCopyInto(out *ReceiverSpec) {
 	*out = *in
+	out.Interval = in.Interval
 	if in.Events != nil {
 		in, out := &in.Events, &out.Events
 		*out = make([]string, len(*in))
diff --git a/config/crd/bases/notification.toolkit.fluxcd.io_providers.yaml b/config/crd/bases/notification.toolkit.fluxcd.io_providers.yaml
index 7ce1517fd..b4863f4cc 100644
--- a/config/crd/bases/notification.toolkit.fluxcd.io_providers.yaml
+++ b/config/crd/bases/notification.toolkit.fluxcd.io_providers.yaml
@@ -249,6 +249,12 @@ spec:
                   should be posted.
                 maxLength: 2048
                 type: string
+              interval:
+                default: 10m
+                description: Interval at which to reconcile the Provider with its
+                  Secret references.
+                pattern: ^([0-9]+(\.[0-9]+)?(ms|s|m|h))+$
+                type: string
               proxy:
                 description: Proxy the HTTP/S address of the proxy server.
                 maxLength: 2048
@@ -302,6 +308,7 @@ spec:
                 maxLength: 2048
                 type: string
             required:
+            - interval
             - type
             type: object
           status:
diff --git a/config/crd/bases/notification.toolkit.fluxcd.io_receivers.yaml b/config/crd/bases/notification.toolkit.fluxcd.io_receivers.yaml
index d723f6ea8..5a09643ec 100644
--- a/config/crd/bases/notification.toolkit.fluxcd.io_receivers.yaml
+++ b/config/crd/bases/notification.toolkit.fluxcd.io_receivers.yaml
@@ -253,6 +253,12 @@ spec:
                 items:
                   type: string
                 type: array
+              interval:
+                default: 10m
+                description: Interval at which to reconcile the Receiver with its
+                  Secret references.
+                pattern: ^([0-9]+(\.[0-9]+)?(ms|s|m|h))+$
+                type: string
               resources:
                 description: A list of resources to be notified about changes.
                 items:
@@ -330,6 +336,7 @@ spec:
                 - acr
                 type: string
             required:
+            - interval
             - resources
             - type
             type: object
diff --git a/controllers/alert_controller_test.go b/controllers/alert_controller_test.go
index 771e454e3..8b89ae307 100644
--- a/controllers/alert_controller_test.go
+++ b/controllers/alert_controller_test.go
@@ -88,6 +88,7 @@ func TestAlertReconciler_Reconcile(t *testing.T) {
 	g.Expect(k8sClient.Create(context.Background(), alert)).To(Succeed())
 
 	t.Run("fails with provider not found error", func(t *testing.T) {
+		g := NewWithT(t)
 		g.Eventually(func() bool {
 			_ = k8sClient.Get(context.Background(), client.ObjectKeyFromObject(alert), resultA)
 			return conditions.Has(resultA, meta.ReadyCondition)
@@ -104,6 +105,7 @@ func TestAlertReconciler_Reconcile(t *testing.T) {
 	})
 
 	t.Run("recovers when provider exists", func(t *testing.T) {
+		g := NewWithT(t)
 		g.Expect(k8sClient.Create(context.Background(), provider)).To(Succeed())
 
 		g.Eventually(func() bool {
@@ -117,6 +119,7 @@ func TestAlertReconciler_Reconcile(t *testing.T) {
 	})
 
 	t.Run("handles reconcileAt", func(t *testing.T) {
+		g := NewWithT(t)
 		reconcileRequestAt := metav1.Now().String()
 		resultA.SetAnnotations(map[string]string{
 			meta.ReconcileRequestAnnotation: reconcileRequestAt,
@@ -130,6 +133,7 @@ func TestAlertReconciler_Reconcile(t *testing.T) {
 	})
 
 	t.Run("finalizes suspended object", func(t *testing.T) {
+		g := NewWithT(t)
 		resultA.Spec.Suspend = true
 		g.Expect(k8sClient.Update(context.Background(), resultA)).To(Succeed())
 
diff --git a/controllers/provider_controller.go b/controllers/provider_controller.go
index 087c21c56..d358a2026 100644
--- a/controllers/provider_controller.go
+++ b/controllers/provider_controller.go
@@ -135,7 +135,7 @@ func (r *ProviderReconciler) reconcile(ctx context.Context, obj *apiv1.Provider)
 	conditions.MarkTrue(obj, meta.ReadyCondition, meta.SucceededReason, apiv1.InitializedReason)
 	ctrl.LoggerFrom(ctx).Info("Provider initialized")
 
-	return ctrl.Result{}, nil
+	return ctrl.Result{RequeueAfter: obj.Spec.Interval.Duration}, nil
 }
 
 func (r *ProviderReconciler) validate(ctx context.Context, provider *apiv1.Provider) error {
diff --git a/controllers/provider_controller_test.go b/controllers/provider_controller_test.go
index ded30ca8d..f6cac063b 100644
--- a/controllers/provider_controller_test.go
+++ b/controllers/provider_controller_test.go
@@ -62,6 +62,7 @@ func TestProviderReconciler_Reconcile(t *testing.T) {
 	g.Expect(k8sClient.Create(context.Background(), provider)).To(Succeed())
 
 	t.Run("reports ready status", func(t *testing.T) {
+		g := NewWithT(t)
 		g.Eventually(func() bool {
 			_ = k8sClient.Get(context.Background(), client.ObjectKeyFromObject(provider), resultP)
 			return resultP.Status.ObservedGeneration == resultP.Generation
@@ -75,6 +76,7 @@ func TestProviderReconciler_Reconcile(t *testing.T) {
 	})
 
 	t.Run("fails with secret not found error", func(t *testing.T) {
+		g := NewWithT(t)
 		resultP.Spec.SecretRef = &meta.LocalObjectReference{
 			Name: secretName,
 		}
@@ -95,6 +97,7 @@ func TestProviderReconciler_Reconcile(t *testing.T) {
 	})
 
 	t.Run("recovers when secret exists", func(t *testing.T) {
+		g := NewWithT(t)
 		secret := &corev1.Secret{
 			ObjectMeta: metav1.ObjectMeta{
 				Name:      secretName,
@@ -117,6 +120,7 @@ func TestProviderReconciler_Reconcile(t *testing.T) {
 	})
 
 	t.Run("handles reconcileAt", func(t *testing.T) {
+		g := NewWithT(t)
 		reconcileRequestAt := metav1.Now().String()
 		resultP.SetAnnotations(map[string]string{
 			meta.ReconcileRequestAnnotation: reconcileRequestAt,
@@ -130,6 +134,7 @@ func TestProviderReconciler_Reconcile(t *testing.T) {
 	})
 
 	t.Run("finalizes suspended object", func(t *testing.T) {
+		g := NewWithT(t)
 		resultP.Spec.Suspend = true
 		g.Expect(k8sClient.Update(context.Background(), resultP)).To(Succeed())
 
diff --git a/controllers/receiver_controller.go b/controllers/receiver_controller.go
index 5013b9173..c7d1dd9fd 100644
--- a/controllers/receiver_controller.go
+++ b/controllers/receiver_controller.go
@@ -150,7 +150,7 @@ func (r *ReceiverReconciler) reconcile(ctx context.Context, obj *apiv1.Receiver)
 
 	ctrl.LoggerFrom(ctx).Info(msg)
 
-	return ctrl.Result{}, nil
+	return ctrl.Result{RequeueAfter: obj.Spec.Interval.Duration}, nil
 }
 
 // patch updates the object status, conditions and finalizers.
diff --git a/controllers/receiver_controller_test.go b/controllers/receiver_controller_test.go
index 33d7fc8c3..c1e0e8de4 100644
--- a/controllers/receiver_controller_test.go
+++ b/controllers/receiver_controller_test.go
@@ -89,6 +89,7 @@ func TestReceiverReconciler_Reconcile(t *testing.T) {
 	g.Expect(k8sClient.Create(context.Background(), receiver)).To(Succeed())
 
 	t.Run("reports ready status", func(t *testing.T) {
+		g := NewWithT(t)
 		g.Eventually(func() bool {
 			_ = k8sClient.Get(context.Background(), client.ObjectKeyFromObject(receiver), resultR)
 			return resultR.Status.ObservedGeneration == resultR.Generation
@@ -102,6 +103,7 @@ func TestReceiverReconciler_Reconcile(t *testing.T) {
 	})
 
 	t.Run("fails with secret not found error", func(t *testing.T) {
+		g := NewWithT(t)
 		g.Expect(k8sClient.Delete(context.Background(), secret)).To(Succeed())
 
 		reconcileRequestAt := metav1.Now().String()
@@ -124,6 +126,7 @@ func TestReceiverReconciler_Reconcile(t *testing.T) {
 	})
 
 	t.Run("recovers when secret exists", func(t *testing.T) {
+		g := NewWithT(t)
 		newSecret := &corev1.Secret{
 			ObjectMeta: metav1.ObjectMeta{
 				Name:      secretName,
@@ -146,6 +149,7 @@ func TestReceiverReconciler_Reconcile(t *testing.T) {
 	})
 
 	t.Run("handles reconcileAt", func(t *testing.T) {
+		g := NewWithT(t)
 		reconcileRequestAt := metav1.Now().String()
 		resultR.SetAnnotations(map[string]string{
 			meta.ReconcileRequestAnnotation: reconcileRequestAt,
@@ -159,6 +163,7 @@ func TestReceiverReconciler_Reconcile(t *testing.T) {
 	})
 
 	t.Run("finalizes suspended object", func(t *testing.T) {
+		g := NewWithT(t)
 		resultR.Spec.Suspend = true
 		g.Expect(k8sClient.Update(context.Background(), resultR)).To(Succeed())
 
@@ -254,6 +259,7 @@ func TestReceiverReconciler_EventHandler(t *testing.T) {
 	address := fmt.Sprintf("/hook/%s", sha256sum(token+receiverKey.Name+receiverKey.Namespace))
 
 	t.Run("generates URL when ready", func(t *testing.T) {
+		g := NewWithT(t)
 		g.Eventually(func() bool {
 			_ = k8sClient.Get(context.Background(), client.ObjectKeyFromObject(receiver), resultR)
 			return conditions.IsReady(resultR)
@@ -263,6 +269,7 @@ func TestReceiverReconciler_EventHandler(t *testing.T) {
 	})
 
 	t.Run("doesn't update the URL on spec updates", func(t *testing.T) {
+		g := NewWithT(t)
 		resultR.Spec.Events = []string{"ping", "push"}
 		g.Expect(k8sClient.Update(context.Background(), resultR)).To(Succeed())
 
@@ -276,6 +283,7 @@ func TestReceiverReconciler_EventHandler(t *testing.T) {
 	})
 
 	t.Run("handles event", func(t *testing.T) {
+		g := NewWithT(t)
 		res, err := http.Post("http://localhost:56788/"+address, "application/json", nil)
 		g.Expect(err).ToNot(HaveOccurred())
 		g.Expect(res.StatusCode).To(Equal(http.StatusOK))
diff --git a/docs/api/notification.md b/docs/api/notification.md
index d7b81e8b2..95891a664 100644
--- a/docs/api/notification.md
+++ b/docs/api/notification.md
@@ -239,6 +239,19 @@ string
 </tr>
 <tr>
 <td>
+<code>interval</code><br>
+<em>
+<a href="https://godoc.org/k8s.io/apimachinery/pkg/apis/meta/v1#Duration">
+Kubernetes meta/v1.Duration
+</a>
+</em>
+</td>
+<td>
+<p>Interval at which to reconcile the Provider with its Secret references.</p>
+</td>
+</tr>
+<tr>
+<td>
 <code>channel</code><br>
 <em>
 string
@@ -432,6 +445,19 @@ the validation procedure and payload deserialization.</p>
 </tr>
 <tr>
 <td>
+<code>interval</code><br>
+<em>
+<a href="https://godoc.org/k8s.io/apimachinery/pkg/apis/meta/v1#Duration">
+Kubernetes meta/v1.Duration
+</a>
+</em>
+</td>
+<td>
+<p>Interval at which to reconcile the Receiver with its Secret references.</p>
+</td>
+</tr>
+<tr>
+<td>
 <code>events</code><br>
 <em>
 []string
@@ -776,6 +802,19 @@ string
 </tr>
 <tr>
 <td>
+<code>interval</code><br>
+<em>
+<a href="https://godoc.org/k8s.io/apimachinery/pkg/apis/meta/v1#Duration">
+Kubernetes meta/v1.Duration
+</a>
+</em>
+</td>
+<td>
+<p>Interval at which to reconcile the Provider with its Secret references.</p>
+</td>
+</tr>
+<tr>
+<td>
 <code>channel</code><br>
 <em>
 string
@@ -976,6 +1015,19 @@ the validation procedure and payload deserialization.</p>
 </tr>
 <tr>
 <td>
+<code>interval</code><br>
+<em>
+<a href="https://godoc.org/k8s.io/apimachinery/pkg/apis/meta/v1#Duration">
+Kubernetes meta/v1.Duration
+</a>
+</em>
+</td>
+<td>
+<p>Interval at which to reconcile the Receiver with its Secret references.</p>
+</td>
+</tr>
+<tr>
+<td>
 <code>events</code><br>
 <em>
 []string
diff --git a/docs/spec/v1beta2/providers.md b/docs/spec/v1beta2/providers.md
index f6f7a1853..34c33f434 100644
--- a/docs/spec/v1beta2/providers.md
+++ b/docs/spec/v1beta2/providers.md
@@ -227,6 +227,12 @@ stringData:
 
 `.spec.proxy` is an optional field to specify an HTTP/S proxy address.
 
+### Interval
+
+`.spec.interval` is a required field with a default of ten minutes that specifies
+the time interval at which the controller reconciles the provider with its Secret
+references.
+
 ### Suspend
 
 `.spec.suspend` is an optional field to suspend the provider.
diff --git a/docs/spec/v1beta2/receivers.md b/docs/spec/v1beta2/receivers.md
index dec2be907..fe6b12a1d 100644
--- a/docs/spec/v1beta2/receivers.md
+++ b/docs/spec/v1beta2/receivers.md
@@ -128,6 +128,12 @@ to another tenant's resources.
 `.spec.secretRef.name` is a required field to specify a name reference to a
 Secret in the same namespace as the Receiver, containing the secret token.
 
+### Interval
+
+`.spec.interval` is a required field with a default of ten minutes that specifies
+the time interval at which the controller reconciles the provider with its Secret
+references.
+
 ### Suspend
 
 `.spec.suspend` is an optional field to suspend the receiver.