(feat) - Add option to not deploy notification secret

Signed-off-by: Guillaume Doussin <[email protected]>

api/v1alpha1/argorollouts_types.go

@@ -49,6 +49,9 @@ type RolloutManagerSpec struct {
 
 	// Resources requests/limits for Argo Rollout controller
 	ControllerResources *corev1.ResourceRequirements `json:"controllerResources,omitempty"`
+
+	// SkipNotificationSecretDeployment lets you specify if the argo notification secret should be deployed
+	SkipNotificationSecretDeployment bool `json:"skipNotificationSecretDeployment,omitempty"`
 }
 
 // ArgoRolloutsNodePlacementSpec is used to specify NodeSelector and Tolerations for Rollouts workloads

config/crd/bases/argoproj.io_rolloutmanagers.yaml

@@ -286,6 +286,10 @@ spec:
                       type: object
                     type: array
                 type: object
+              skipNotificationSecretDeployment:
+                description: SkipNotificationSecretDeployment lets you specify if
+                  the argo notification secret should be deployed
+                type: boolean
               version:
                 description: Version defines Argo Rollouts controller tag (optional)
                 type: string

controllers/resources.go

@@ -664,6 +664,7 @@ func (r *RolloutManagerReconciler) reconcileRolloutsMetricsService(ctx context.C
 
 // Reconciles Secrets for Rollouts controller
 func (r *RolloutManagerReconciler) reconcileRolloutsSecrets(ctx context.Context, cr rolloutsmanagerv1alpha1.RolloutManager) error {
+
 	expectedSecret := &corev1.Secret{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      DefaultRolloutsNotificationSecretName,
@@ -674,20 +675,44 @@ func (r *RolloutManagerReconciler) reconcileRolloutsSecrets(ctx context.Context,
 
 	setRolloutsLabelsAndAnnotationsToObject(&expectedSecret.ObjectMeta, cr)
 
+	// If the Secret doesn't exist (or an unrelated error occurred)....
 	liveSecret := &corev1.Secret{ObjectMeta: metav1.ObjectMeta{Name: expectedSecret.Name, Namespace: expectedSecret.Namespace}}
 	if err := fetchObject(ctx, r.Client, cr.Namespace, liveSecret.Name, liveSecret); err != nil {
-		if !apierrors.IsNotFound(err) {
+		if !apierrors.IsNotFound(err) { // unrelated error: return
 			return fmt.Errorf("failed to get the Secret %s: %w", liveSecret.Name, err)
 		}
 
+		if cr.Spec.SkipNotificationSecretDeployment {
+			// Secret does not exist, but SkipNotificationSecretDeployment is set to true, hence skipping the creation
+			return nil
+		}
+
+		// Secret does not exist (and SkipNotificationSecretDeployment is set to false) so create Secret
 		if err := controllerutil.SetControllerReference(&cr, expectedSecret, r.Scheme); err != nil {
 			return err
 		}
 
 		log.Info(fmt.Sprintf("Creating Secret %s", expectedSecret.Name))
 		return r.Client.Create(ctx, expectedSecret)
+
 	}
 
+	// If SkipNotificationSecretDeployment is true, and the secret exists, delete it
+	if cr.Spec.SkipNotificationSecretDeployment {
+
+		// If the controller created/owns the Secret, delete it
+		controller := metav1.GetControllerOf(liveSecret)
+		if controller != nil && controller.Name == cr.Name {
+			log.Info(fmt.Sprintf("SkipNotificationSecretDeployment has been set to true, deleting secret %s", liveSecret.Name))
+			return r.Client.Delete(ctx, liveSecret)
+		}
+
+		// Otherwise, the secret exists, but the controller didn't create it, so just return (don't touch it)
+		return nil
+	}
+
+	// Otherwise, the Secret exists, so update it if the labels/annotations are inconsistent
+
 	updateNeeded := false
 
 	normalizedLiveSecret := liveSecret.DeepCopy()

controllers/resources_test.go

@@ -993,6 +993,104 @@ var _ = Describe("Resource creation and cleanup tests", func() {
 		})
 	})
 
+	Context("Rollouts notification secret reconciliation tests", func() {
+		var (
+			ctx context.Context
+			a   v1alpha1.RolloutManager
+			r   *RolloutManagerReconciler
+		)
+
+		BeforeEach(func() {
+			ctx = context.Background()
+			a = *makeTestRolloutManager()
+			r = makeTestReconciler(&a)
+			err := createNamespace(r, a.Namespace)
+			Expect(err).ToNot(HaveOccurred())
+		})
+
+		It("Verify that RolloutManager with SkipNotificationSecretDeployment doesn't create a secret", func() {
+
+			By("Creating RolloutManager with SkipNotificationSecretDeployment set to true")
+			a.Spec.SkipNotificationSecretDeployment = true
+			Expect(r.reconcileRolloutsSecrets(ctx, a)).To(Succeed())
+
+			secret := &corev1.Secret{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      DefaultRolloutsNotificationSecretName,
+					Namespace: a.Namespace,
+				},
+				Type: corev1.SecretTypeOpaque,
+			}
+			Expect(fetchObject(ctx, r.Client, a.Namespace, secret.Name, secret)).ToNot(Succeed(), "secret should not exist after reconcile call")
+
+			By("Updating the RolloutManager with SkipNotificationSecretDeployment set to false")
+			a.Spec.SkipNotificationSecretDeployment = false
+			Expect(r.reconcileRolloutsSecrets(ctx, a)).To(Succeed())
+			Expect(fetchObject(ctx, r.Client, a.Namespace, secret.Name, secret)).To(Succeed(), "secret should exist after reconcile call")
+		})
+
+		It("Verify that RolloutManager can handle the case where SkipNotificationSecretDeployment is set on an existing instance", func() {
+			By("Creating RolloutManager with SkipNotificationSecretDeployment set to false")
+			a.Spec.SkipNotificationSecretDeployment = false
+			Expect(r.reconcileRolloutsSecrets(ctx, a)).To(Succeed())
+
+			secret := &corev1.Secret{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      DefaultRolloutsNotificationSecretName,
+					Namespace: a.Namespace,
+				},
+				Type: corev1.SecretTypeOpaque,
+			}
+			Expect(fetchObject(ctx, r.Client, a.Namespace, secret.Name, secret)).To(Succeed(), "secret should exist after reconcile call")
+
+			By("Updating the RolloutManager with SkipNotificationSecretDeployment set to true")
+			a.Spec.SkipNotificationSecretDeployment = true
+			Expect(r.reconcileRolloutsSecrets(ctx, a)).To(Succeed())
+			Expect(fetchObject(ctx, r.Client, a.Namespace, secret.Name, secret)).ToNot(Succeed(), "secret should not exist after reconcile call")
+		})
+
+		It("Verify that RolloutManager does not update an existing notification secret if it doesn't have the ownership", func() {
+			By("Creating RolloutManager with SkipNotificationSecretDeployment set to true")
+
+			a.Spec.SkipNotificationSecretDeployment = true
+			Expect(r.reconcileRolloutsSecrets(ctx, a)).To(Succeed())
+
+			secret := &corev1.Secret{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      DefaultRolloutsNotificationSecretName,
+					Namespace: a.Namespace,
+				},
+				Type: corev1.SecretTypeOpaque,
+			}
+			Expect(fetchObject(ctx, r.Client, a.Namespace, secret.Name, secret)).ToNot(Succeed(), "secret should exist after reconcile call")
+
+			By("Create the secret without the owner reference")
+			Expect(r.Client.Create(ctx, secret)).To(Succeed())
+
+			By("Updating the RolloutManager")
+			Expect(r.reconcileRolloutsSecrets(ctx, a)).To(Succeed())
+
+			By("Verifying that the secret was not updated")
+			Expect(fetchObject(ctx, r.Client, a.Namespace, secret.Name, secret)).To(Succeed(), "secret should exist after reconcile call")
+			Expect(secret.OwnerReferences).To(BeNil())
+
+			By("Adding another owner reference")
+			testRef := metav1.OwnerReference{
+				Name: "test",
+			}
+			secret.OwnerReferences = append(secret.OwnerReferences, testRef)
+			Expect(r.Client.Update(ctx, secret)).To(Succeed())
+
+			By("Updating the RolloutManager")
+			Expect(r.reconcileRolloutsSecrets(ctx, a)).To(Succeed())
+
+			By("Verifying that the secret was not updated")
+			Expect(fetchObject(ctx, r.Client, a.Namespace, secret.Name, secret)).To(Succeed(), "secret should exist after reconcile call")
+			Expect(secret.OwnerReferences).To(ContainElement(testRef))
+			Expect(len(secret.OwnerReferences)).To(Equal(1))
+		})
+	})
+
 })
 
 func serviceMonitor() *monitoringv1.ServiceMonitor {

docs/crd_reference.md

@@ -78,7 +78,7 @@ spec:
 ```
 
 
-### RolloutManager example with metadata for the resources generatd
+### RolloutManager example with metadata for the resources generated
 
 You can provide labels and annotation for all the resources generated (Argo Rollouts controller, ConfigMap, etc.).
 
@@ -117,4 +117,19 @@ spec:
     limits:
       memory: "128Mi"
       cpu: "500m"
-```
+```
+
+
+### RolloutManager example with an option to skip the argo rollouts notification secret deployment
+
+``` yaml
+apiVersion: argoproj.io/v1alpha1
+kind: RolloutManager
+metadata:
+  name: argo-rollout
+  labels:
+    example: with-metadata-example
+spec:
+  skipNotificationSecretDeployment: true
+```
+

tests/e2e/rollout_tests_all.go

@@ -503,6 +503,118 @@ func RunRolloutsTests(namespaceScopedParam bool) {
 					return deployment.Spec.Template.Spec.Containers[0].Resources.Limits[corev1.ResourceCPU] == resource.MustParse("555m")
 
 				}, "1m", "1s").Should(BeTrue(), "Deployment should switch to the new CPU limit on update of RolloutManager CR")
+			})
+		})
+
+		When("A RolloutManager is created with the SkipNotificationSecretDeployment option to true", func() {
+			It("should create the controller without the notification secret", func() {
+
+				rolloutsManager := rolloutsmanagerv1alpha1.RolloutManager{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      "basic-rollouts-manager-with-skip-notification-secret",
+						Namespace: fixture.TestE2ENamespace,
+					},
+					Spec: rolloutsmanagerv1alpha1.RolloutManagerSpec{
+						NamespaceScoped:                  namespaceScopedParam,
+						SkipNotificationSecretDeployment: true,
+					},
+				}
+
+				Expect(k8sClient.Create(ctx, &rolloutsManager)).To(Succeed())
+
+				Eventually(rolloutsManager, "1m", "1s").Should(rolloutManagerFixture.HavePhase(rolloutsmanagerv1alpha1.PhaseAvailable))
+
+				secret := corev1.Secret{
+					ObjectMeta: metav1.ObjectMeta{Name: controllers.DefaultRolloutsNotificationSecretName, Namespace: rolloutManager.Namespace},
+				}
+				Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(&secret), &secret)).ToNot(Succeed())
+
+				By("Setting the SkipNotificationSecretDeployment to false")
+				err := k8s.UpdateWithoutConflict(ctx, &rolloutsManager, k8sClient, func(obj client.Object) {
+					goObj, ok := obj.(*rolloutsmanagerv1alpha1.RolloutManager)
+					Expect(ok).To(BeTrue())
+					goObj.Spec.SkipNotificationSecretDeployment = false
+				})
+
+				Expect(err).ToNot(HaveOccurred())
+				Eventually(&secret, "10s", "1s").Should(k8s.ExistByName(k8sClient))
+
+			})
+		})
+
+		When("A RolloutManager is created with the SkipNotificationSecretDeployment option to false", func() {
+			It("should create the controller with the notification secret", func() {
+
+				rolloutsManager := rolloutsmanagerv1alpha1.RolloutManager{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      "basic-rollouts-manager-without-skip-notification-secret",
+						Namespace: fixture.TestE2ENamespace,
+					},
+					Spec: rolloutsmanagerv1alpha1.RolloutManagerSpec{
+						NamespaceScoped:                  namespaceScopedParam,
+						SkipNotificationSecretDeployment: false,
+					},
+				}
+
+				Expect(k8sClient.Create(ctx, &rolloutsManager)).To(Succeed())
+
+				Eventually(rolloutsManager, "1m", "1s").Should(rolloutManagerFixture.HavePhase(rolloutsmanagerv1alpha1.PhaseAvailable))
+
+				secret := corev1.Secret{
+					ObjectMeta: metav1.ObjectMeta{Name: controllers.DefaultRolloutsNotificationSecretName, Namespace: rolloutManager.Namespace},
+				}
+				Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(&secret), &secret)).To(Succeed())
+
+				By("Setting the SkipNotificationSecretDeployment to true")
+				err := k8s.UpdateWithoutConflict(ctx, &rolloutsManager, k8sClient, func(obj client.Object) {
+					goObj, ok := obj.(*rolloutsmanagerv1alpha1.RolloutManager)
+					Expect(ok).To(BeTrue())
+					goObj.Spec.SkipNotificationSecretDeployment = true
+				})
+
+				Expect(err).ToNot(HaveOccurred())
+				Eventually(&secret, "10s", "1s").ShouldNot(k8s.ExistByName(k8sClient))
+
+			})
+		})
+
+		When("A RolloutManager is deleted but the notification secret is owned by another controller", func() {
+			It("should not delete the secret", func() {
+
+				rolloutsManager := rolloutsmanagerv1alpha1.RolloutManager{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      "basic-rollouts-manager-without-secret",
+						Namespace: fixture.TestE2ENamespace,
+					},
+					Spec: rolloutsmanagerv1alpha1.RolloutManagerSpec{
+						NamespaceScoped:                  namespaceScopedParam,
+						SkipNotificationSecretDeployment: true,
+					},
+				}
+
+				Expect(k8sClient.Create(ctx, &rolloutsManager)).To(Succeed())
+
+				Eventually(rolloutsManager, "1m", "1s").Should(rolloutManagerFixture.HavePhase(rolloutsmanagerv1alpha1.PhaseAvailable))
+
+				secret := corev1.Secret{
+					ObjectMeta: metav1.ObjectMeta{Name: controllers.DefaultRolloutsNotificationSecretName, Namespace: rolloutManager.Namespace},
+				}
+				Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(&secret), &secret)).ToNot(Succeed())
+
+				By("Creating the secret with another owner")
+				secret.OwnerReferences = append(secret.OwnerReferences, metav1.OwnerReference{
+					Name:       "another-owner",
+					APIVersion: "v1",
+					Kind:       "OwnerKind",
+					UID:        "1234",
+				})
+				Expect(k8sClient.Create(ctx, &secret)).To(Succeed())
+				Eventually(&secret, "10s", "1s").Should(k8s.ExistByName(k8sClient))
+
+				By("Deleting the RolloutManager")
+				Expect(k8sClient.Delete(ctx, &rolloutsManager)).To(Succeed())
+
+				Eventually(&secret, "10s", "1s").Should(k8s.ExistByName(k8sClient))
 
 			})
 		})