(feat) - Add option to not deploy notification secret

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

api/v1alpha1/argorollouts_types.go

@@ -46,6 +46,9 @@ type RolloutManagerSpec struct {
 
 	// Metadata to apply to the generated resources
 	AdditionalMetadata *ResourceMetadata `json:"additionalMetadata,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

@@ -225,6 +225,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

@@ -456,6 +456,7 @@ func (r *RolloutManagerReconciler) reconcileRolloutsMetricsService(ctx context.C
 
 // Reconciles Secrets for Rollouts controller
 func (r *RolloutManagerReconciler) reconcileRolloutsSecrets(ctx context.Context, cr rolloutsmanagerv1alpha1.RolloutManager) error {
+
 	secret := &corev1.Secret{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      DefaultRolloutsNotificationSecretName,
@@ -471,16 +472,30 @@ func (r *RolloutManagerReconciler) reconcileRolloutsSecrets(ctx context.Context,
 			return fmt.Errorf("failed to get the Secret %s: %w", secret.Name, err)
 		}
 
-		if err := controllerutil.SetControllerReference(&cr, secret, r.Scheme); err != nil {
-			return err
+		if !cr.Spec.SkipNotificationSecretDeployment {
+			if err := controllerutil.SetControllerReference(&cr, secret, r.Scheme); err != nil {
+				return err
+			}
+
+			log.Info(fmt.Sprintf("Creating Secret %s", secret.Name))
+			return r.Client.Create(ctx, secret)
+		} else {
+			// Secret not found, but SkipNotificationSecretDeployment is set to true, hence skipping the creation
+			return nil
 		}
+	} else {
 
-		log.Info(fmt.Sprintf("Creating Secret %s", secret.Name))
-		return r.Client.Create(ctx, secret)
+		// Secret found, delete it if required by the spec
+		if cr.Spec.SkipNotificationSecretDeployment {
+			controller := metav1.GetControllerOf(secret)
+			if controller != nil && controller.Name == cr.Name {
+				log.Info(fmt.Sprintf("SkipNotificationSecretDeployment has been set to true, deleting secret %s", secret.Name))
+				return r.Client.Delete(ctx, secret)
+			}
+			// If the controller is not the current owner, then do nothing
+		}
+		return nil
 	}
-
-	// secret found, do nothing
-	return nil
 }
 
 // Deletes rollout resources when the corresponding rollout CR is deleted.
@@ -534,9 +549,23 @@ func (r *RolloutManagerReconciler) deleteRolloutResources(ctx context.Context, c
 				Namespace: cr.Namespace,
 			},
 		}
-		if err := r.Client.Delete(ctx, secret); err != nil {
-			log.Error(err, fmt.Sprintf("Error deleting the secret %s in %s",
+
+		err := fetchObject(ctx, r.Client, cr.Namespace, secret.Name, secret)
+		if err != nil && !apierrors.IsNotFound(err) {
+			log.Error(err, fmt.Sprintf("Error getting the secret %s in %s",
 				DefaultRolloutsNotificationSecretName, cr.Namespace))
+		} else {
+			controller := metav1.GetControllerOf(secret)
+			if controller == nil || controller.Name != cr.Name {
+				log.Info(fmt.Sprintf("The secret %s in %s is not owned by %s, skipping deletion",
+					DefaultRolloutsNotificationSecretName, cr.Namespace, cr.Name))
+			} else {
+				err = r.Client.Delete(ctx, secret)
+				if err != nil {
+					log.Error(err, fmt.Sprintf("Error deleting the secret %s in %s",
+						DefaultRolloutsNotificationSecretName, cr.Namespace))
+				}
+			}
 		}
 
 		deploy := &appsv1.Deployment{

controllers/resources_test.go

@@ -400,6 +400,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.).
 
@@ -97,3 +97,16 @@ spec:
       myannotation: "myvalue"
 ```
 
+### 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

@@ -363,5 +363,118 @@ func RunRolloutsTests(namespaceScopedParam bool) {
 				expectMetadataOnObjectMeta(&service.ObjectMeta, rolloutsManager.Spec.AdditionalMetadata)
 			})
 		})
+
+		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))
+
+			})
+		})
 	})
 }