From debb27555c4cfa1112b4fe9d2ff24eadf0862c57 Mon Sep 17 00:00:00 2001 From: Guillaume Doussin Date: Wed, 26 Jun 2024 17:20:52 +0200 Subject: [PATCH] (feat) - Add option to not deploy notification secret Signed-off-by: Guillaume Doussin --- api/v1alpha1/argorollouts_types.go | 3 + .../bases/argoproj.io_rolloutmanagers.yaml | 4 + controllers/resources.go | 29 +++-- controllers/resources_test.go | 98 +++++++++++++++ docs/crd_reference.md | 15 ++- tests/e2e/rollout_tests_all.go | 113 ++++++++++++++++++ 6 files changed, 254 insertions(+), 8 deletions(-) diff --git a/api/v1alpha1/argorollouts_types.go b/api/v1alpha1/argorollouts_types.go index 985e6ee..0c392bd 100644 --- a/api/v1alpha1/argorollouts_types.go +++ b/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 diff --git a/config/crd/bases/argoproj.io_rolloutmanagers.yaml b/config/crd/bases/argoproj.io_rolloutmanagers.yaml index ff9daf6..1bb732f 100644 --- a/config/crd/bases/argoproj.io_rolloutmanagers.yaml +++ b/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 diff --git a/controllers/resources.go b/controllers/resources.go index 666063f..4584679 100644 --- a/controllers/resources.go +++ b/controllers/resources.go @@ -504,6 +504,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, @@ -519,16 +520,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 } func setRolloutsAggregatedClusterRoleLabels(obj *metav1.ObjectMeta, name string, aggregationType string) { diff --git a/controllers/resources_test.go b/controllers/resources_test.go index 46a3174..fb9a048 100644 --- a/controllers/resources_test.go +++ b/controllers/resources_test.go @@ -448,6 +448,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 { diff --git a/docs/crd_reference.md b/docs/crd_reference.md index 52dfb05..fb5238d 100644 --- a/docs/crd_reference.md +++ b/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 +``` + diff --git a/tests/e2e/rollout_tests_all.go b/tests/e2e/rollout_tests_all.go index 1578a93..f0515ef 100644 --- a/tests/e2e/rollout_tests_all.go +++ b/tests/e2e/rollout_tests_all.go @@ -443,5 +443,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)) + + }) + }) }) }