From dfc082a3b7a04ffa0186fb17e68c161173b78116 Mon Sep 17 00:00:00 2001 From: nmirasch Date: Tue, 28 Oct 2025 11:48:35 +0100 Subject: [PATCH 1/5] Support notifications in any namespace Signed-off-by: nmirasch --- api/v1alpha1/argocd_types.go | 3 + api/v1alpha1/argocd_types_test.go | 11 + api/v1alpha1/zz_generated.deepcopy.go | 5 + api/v1beta1/argocd_types.go | 3 + api/v1beta1/argocd_types_test.go | 11 + api/v1beta1/zz_generated.deepcopy.go | 5 + bundle/manifests/argoproj.io_argocds.yaml | 14 + common/keys.go | 5 +- config/crd/bases/argoproj.io_argocds.yaml | 14 + controllers/argocd/argocd_controller.go | 11 +- controllers/argocd/notifications.go | 465 +++++++++++++++++- controllers/argocd/notifications_test.go | 264 ++++++++++ controllers/argocd/policyrule.go | 22 + controllers/argocd/role.go | 6 +- controllers/argocd/role_test.go | 17 +- controllers/argocd/util.go | 2 +- .../0.17.0/argoproj.io_argocds.yaml | 14 + 17 files changed, 846 insertions(+), 26 deletions(-) diff --git a/api/v1alpha1/argocd_types.go b/api/v1alpha1/argocd_types.go index bd9777e87..1ebacea48 100644 --- a/api/v1alpha1/argocd_types.go +++ b/api/v1alpha1/argocd_types.go @@ -329,6 +329,9 @@ type ArgoCDNotifications struct { // Enabled defines whether argocd-notifications controller should be deployed or not Enabled bool `json:"enabled"` + // SourceNamespaces is a list of namespaces from which the notifications controller will watch for ArgoCD Application resources. + SourceNamespaces []string `json:"sourceNamespaces,omitempty"` + // Env let you specify environment variables for Notifications pods Env []corev1.EnvVar `json:"env,omitempty"` diff --git a/api/v1alpha1/argocd_types_test.go b/api/v1alpha1/argocd_types_test.go index a4b8d04c8..b8e88f6a6 100644 --- a/api/v1alpha1/argocd_types_test.go +++ b/api/v1alpha1/argocd_types_test.go @@ -47,3 +47,14 @@ func Test_ParseResourceTrackingMethod(t *testing.T) { assert.Equal(t, tt.rtm, ParseResourceTrackingMethod(tt.str)) } } + +func Test_ArgoCDNotifications_SourceNamespaces(t *testing.T) { + notifications := ArgoCDNotifications{ + SourceNamespaces: []string{"ns1", "ns2"}, + } + assert.Equal(t, []string{"ns1", "ns2"}, notifications.SourceNamespaces) + + // Test omitempty (zero value) + emptyNotifications := ArgoCDNotifications{} + assert.Nil(t, emptyNotifications.SourceNamespaces) +} diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index d8f149cd3..34ac2a01a 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -602,6 +602,11 @@ func (in *ArgoCDNotifications) DeepCopyInto(out *ArgoCDNotifications) { *out = new(int32) **out = **in } + if in.SourceNamespaces != nil { + in, out := &in.SourceNamespaces, &out.SourceNamespaces + *out = make([]string, len(*in)) + copy(*out, *in) + } if in.Env != nil { in, out := &in.Env, &out.Env *out = make([]v1.EnvVar, len(*in)) diff --git a/api/v1beta1/argocd_types.go b/api/v1beta1/argocd_types.go index e7d16c0c1..fe497990a 100644 --- a/api/v1beta1/argocd_types.go +++ b/api/v1beta1/argocd_types.go @@ -411,6 +411,9 @@ type ArgoCDNotifications struct { // Enabled defines whether argocd-notifications controller should be deployed or not Enabled bool `json:"enabled"` + // SourceNamespaces is a list of namespaces from which the notifications controller will watch for ArgoCD Notification resources. + SourceNamespaces []string `json:"sourceNamespaces,omitempty"` + // Env let you specify environment variables for Notifications pods Env []corev1.EnvVar `json:"env,omitempty"` diff --git a/api/v1beta1/argocd_types_test.go b/api/v1beta1/argocd_types_test.go index 5c4206441..5ea78f7a2 100644 --- a/api/v1beta1/argocd_types_test.go +++ b/api/v1beta1/argocd_types_test.go @@ -47,3 +47,14 @@ func Test_ParseResourceTrackingMethod(t *testing.T) { assert.Equal(t, tt.rtm, ParseResourceTrackingMethod(tt.str)) } } + +func Test_ArgoCDNotifications_SourceNamespaces(t *testing.T) { + notifications := ArgoCDNotifications{ + SourceNamespaces: []string{"ns1", "ns2"}, + } + assert.Equal(t, []string{"ns1", "ns2"}, notifications.SourceNamespaces) + + // Test omitempty (zero value) + emptyNotifications := ArgoCDNotifications{} + assert.Nil(t, emptyNotifications.SourceNamespaces) +} diff --git a/api/v1beta1/zz_generated.deepcopy.go b/api/v1beta1/zz_generated.deepcopy.go index 267da9f53..dcaa0e693 100644 --- a/api/v1beta1/zz_generated.deepcopy.go +++ b/api/v1beta1/zz_generated.deepcopy.go @@ -626,6 +626,11 @@ func (in *ArgoCDNotifications) DeepCopyInto(out *ArgoCDNotifications) { *out = new(int32) **out = **in } + if in.SourceNamespaces != nil { + in, out := &in.SourceNamespaces, &out.SourceNamespaces + *out = make([]string, len(*in)) + copy(*out, *in) + } if in.Env != nil { in, out := &in.Env, &out.Env *out = make([]v1.EnvVar, len(*in)) diff --git a/bundle/manifests/argoproj.io_argocds.yaml b/bundle/manifests/argoproj.io_argocds.yaml index 8ec45d7c0..c03a71edf 100644 --- a/bundle/manifests/argoproj.io_argocds.yaml +++ b/bundle/manifests/argoproj.io_argocds.yaml @@ -1810,6 +1810,13 @@ spec: More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/ type: object type: object + sourceNamespaces: + description: SourceNamespaces is a list of namespaces from which + the notifications controller will watch for ArgoCD Application + resources. + items: + type: string + type: array version: description: Version is the Argo CD Notifications image tag. (optional) type: string @@ -16713,6 +16720,13 @@ spec: More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/ type: object type: object + sourceNamespaces: + description: SourceNamespaces is a list of namespaces from which + the notifications controller will watch for ArgoCD Notification + resources. + items: + type: string + type: array version: description: Version is the Argo CD Notifications image tag. (optional) type: string diff --git a/common/keys.go b/common/keys.go index f47de0a81..e11626be7 100644 --- a/common/keys.go +++ b/common/keys.go @@ -214,9 +214,12 @@ const ( // ArgoCDManagedByClusterArgoCDLabel is needed to identify namespace mentioned as sourceNamespace on ArgoCD ArgoCDManagedByClusterArgoCDLabel = "argocd.argoproj.io/managed-by-cluster-argocd" - // ArgoCDManagedByClusterArgoCDLabel is needed to identify namespace mentioned as sourceNamespace on ArgoCD + // ArgoCDApplicationSetManagedByClusterArgoCDLabel is needed to identify namespace mentioned as applicationSet sourceNamespaces on ArgoCD ArgoCDApplicationSetManagedByClusterArgoCDLabel = "argocd.argoproj.io/applicationset-managed-by-cluster-argocd" + // ArgoCDNotificationsManagedByClusterArgoCDLabel is needed to identify namespace mentioned as notifications sourceNamespaces on ArgoCD + ArgoCDNotificationsManagedByClusterArgoCDLabel = "argocd.argoproj.io/notifications-managed-by-cluster-argocd" + // ArgoCDControllerClusterRoleEnvName is an environment variable to specify a custom cluster role for Argo CD application controller ArgoCDControllerClusterRoleEnvName = "CONTROLLER_CLUSTER_ROLE" diff --git a/config/crd/bases/argoproj.io_argocds.yaml b/config/crd/bases/argoproj.io_argocds.yaml index 0087e88c5..faa35a76c 100644 --- a/config/crd/bases/argoproj.io_argocds.yaml +++ b/config/crd/bases/argoproj.io_argocds.yaml @@ -1799,6 +1799,13 @@ spec: More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/ type: object type: object + sourceNamespaces: + description: SourceNamespaces is a list of namespaces from which + the notifications controller will watch for ArgoCD Application + resources. + items: + type: string + type: array version: description: Version is the Argo CD Notifications image tag. (optional) type: string @@ -16702,6 +16709,13 @@ spec: More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/ type: object type: object + sourceNamespaces: + description: SourceNamespaces is a list of namespaces from which + the notifications controller will watch for ArgoCD Notification + resources. + items: + type: string + type: array version: description: Version is the Argo CD Notifications image tag. (optional) type: string diff --git a/controllers/argocd/argocd_controller.go b/controllers/argocd/argocd_controller.go index 44a258846..fe734f838 100644 --- a/controllers/argocd/argocd_controller.go +++ b/controllers/argocd/argocd_controller.go @@ -78,6 +78,10 @@ type ReconcileArgoCD struct { ManagedSourceNamespaces map[string]string // Stores a list of ApplicationSetSourceNamespaces as keys ManagedApplicationSetSourceNamespaces map[string]string + + // Stores a list of ApplicationSetSourceNamespaces as keys + ManagedNotificationsSourceNamespaces map[string]string + // Stores label selector used to reconcile a subset of ArgoCD LabelSelector string @@ -255,6 +259,9 @@ func (r *ReconcileArgoCD) internalReconcile(ctx context.Context, request ctrl.Re if err := r.removeUnmanagedApplicationSetSourceNamespaceResources(argocd); err != nil { return reconcile.Result{}, argocd, argoCDStatus, fmt.Errorf("failed to remove resources from applicationSetSourceNamespaces, error: %w", err) } + if err := r.removeUnmanagedNotificationsSourceNamespaceResources(argocd); err != nil { + return reconcile.Result{}, argocd, argoCDStatus, fmt.Errorf("failed to remove resources from notificationsSourceNamespaces, error: %w", err) + } if err := r.removeDeletionFinalizer(argocd); err != nil { return reconcile.Result{}, argocd, argoCDStatus, err @@ -285,7 +292,9 @@ func (r *ReconcileArgoCD) internalReconcile(ctx context.Context, request ctrl.Re if err = r.setManagedApplicationSetSourceNamespaces(argocd); err != nil { return reconcile.Result{}, argocd, argoCDStatus, err } - + if err = r.setManagedNotificationsSourceNamespaces(argocd); err != nil { + return reconcile.Result{}, argocd, argoCDStatus, err + } // Handle NamespaceManagement reconciliation and check if Namespace Management is enabled via the Subscription env variable. if isNamespaceManagementEnabled() { if err := r.reconcileNamespaceManagement(argocd); err != nil { diff --git a/controllers/argocd/notifications.go b/controllers/argocd/notifications.go index 402b22efa..148e3f30c 100644 --- a/controllers/argocd/notifications.go +++ b/controllers/argocd/notifications.go @@ -2,17 +2,22 @@ package argocd import ( "context" + "errors" "fmt" + "os" "reflect" + "strings" monitoringv1 "github.com/prometheus-operator/prometheus-operator/pkg/apis/monitoring/v1" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" rbacv1 "k8s.io/api/rbac/v1" - "k8s.io/apimachinery/pkg/api/errors" + apierrors "k8s.io/apimachinery/pkg/api/errors" v1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" + amerr "k8s.io/apimachinery/pkg/util/errors" "k8s.io/apimachinery/pkg/util/intstr" + "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" @@ -65,6 +70,30 @@ func (r *ReconcileArgoCD) reconcileNotificationsController(cr *argoproj.ArgoCD) return err } + // create clusterrole & clusterrolebinding if cluster-scoped ArgoCD + log.Info("reconciling notifications clusterroles") + clusterrole, err := r.reconcileNotificationsClusterRole(cr) + if err != nil { + return err + } + + log.Info("reconciling notifications clusterrolebindings") + if err := r.reconcileNotificationsClusterRoleBinding(cr, clusterrole, sa); err != nil { + return err + } + + // reconcile source namespace roles & rolebindings + log.Info("reconciling notifications roles & rolebindings in source namespaces") + if err := r.reconcileNotificationsSourceNamespacesResources(cr); err != nil { + return err + } + + // remove resources for namespaces not part of SourceNamespaces + log.Info("performing cleanup for notifications source namespaces") + if err := r.removeUnmanagedNotificationsSourceNamespaceResources(cr); err != nil { + return err + } + if prometheusAPIFound { log.Info("reconciling notifications metrics service monitor") if err := r.reconcileNotificationsServiceMonitor(cr); err != nil { @@ -89,7 +118,7 @@ func (r *ReconcileArgoCD) reconcileNotificationsConfigurationCR(cr *argoproj.Arg }, } - if !cr.Spec.Notifications.Enabled { + if !isNotificationsEnabled(cr) { argoutil.LogResourceDeletion(log, defaultNotificationsConfigurationCR, "notifications are disabled") return r.Delete(context.TODO(), defaultNotificationsConfigurationCR) } @@ -97,13 +126,13 @@ func (r *ReconcileArgoCD) reconcileNotificationsConfigurationCR(cr *argoproj.Arg if err := argoutil.FetchObject(r.Client, cr.Namespace, DefaultNotificationsConfigurationInstanceName, defaultNotificationsConfigurationCR); err != nil { - if !errors.IsNotFound(err) { + if !apierrors.IsNotFound(err) { return fmt.Errorf("failed to get the NotificationsConfiguration associated with %s : %s", cr.Name, err) } // NotificationsConfiguration doesn't exist and shouldn't, nothing to do here - if !cr.Spec.Notifications.Enabled { + if !isNotificationsEnabled(cr) { return nil } @@ -128,12 +157,12 @@ func (r *ReconcileArgoCD) deleteNotificationsResources(cr *argoproj.ArgoCD) erro role := &rbacv1.Role{} if err := argoutil.FetchObject(r.Client, cr.Namespace, fmt.Sprintf("%s-%s", cr.Name, common.ArgoCDNotificationsControllerComponent), sa); err != nil { - if !errors.IsNotFound(err) { + if !apierrors.IsNotFound(err) { return err } } if err := argoutil.FetchObject(r.Client, cr.Namespace, fmt.Sprintf("%s-%s", cr.Name, common.ArgoCDNotificationsControllerComponent), role); err != nil { - if !errors.IsNotFound(err) { + if !apierrors.IsNotFound(err) { return err } } @@ -189,12 +218,12 @@ func (r *ReconcileArgoCD) reconcileNotificationsServiceAccount(cr *argoproj.Argo sa := newServiceAccountWithName(common.ArgoCDNotificationsControllerComponent, cr) if err := argoutil.FetchObject(r.Client, cr.Namespace, sa.Name, sa); err != nil { - if !errors.IsNotFound(err) { + if !apierrors.IsNotFound(err) { return nil, fmt.Errorf("failed to get the serviceAccount associated with %s : %s", sa.Name, err) } // SA doesn't exist and shouldn't, nothing to do here - if !cr.Spec.Notifications.Enabled { + if !isNotificationsEnabled(cr) { return nil, nil } @@ -211,7 +240,7 @@ func (r *ReconcileArgoCD) reconcileNotificationsServiceAccount(cr *argoproj.Argo } // SA exists but shouldn't, so it should be deleted - if !cr.Spec.Notifications.Enabled { + if !isNotificationsEnabled(cr) { argoutil.LogResourceDeletion(log, sa, "notifications are disabled") return nil, r.Delete(context.TODO(), sa) } @@ -226,12 +255,12 @@ func (r *ReconcileArgoCD) reconcileNotificationsRole(cr *argoproj.ArgoCD) (*rbac existingRole := &rbacv1.Role{} if err := argoutil.FetchObject(r.Client, cr.Namespace, desiredRole.Name, existingRole); err != nil { - if !errors.IsNotFound(err) { + if !apierrors.IsNotFound(err) { return nil, fmt.Errorf("failed to get the role associated with %s : %s", desiredRole.Name, err) } // role does not exist and shouldn't, nothing to do here - if !cr.Spec.Notifications.Enabled { + if !isNotificationsEnabled(cr) { return nil, nil } @@ -249,7 +278,7 @@ func (r *ReconcileArgoCD) reconcileNotificationsRole(cr *argoproj.ArgoCD) (*rbac } // role exists but shouldn't, so it should be deleted - if !cr.Spec.Notifications.Enabled { + if !isNotificationsEnabled(cr) { argoutil.LogResourceDeletion(log, existingRole, "notifications are disabled") return nil, r.Delete(context.TODO(), existingRole) } @@ -287,12 +316,12 @@ func (r *ReconcileArgoCD) reconcileNotificationsRoleBinding(cr *argoproj.ArgoCD, // fetch existing rolebinding by name existingRoleBinding := &rbacv1.RoleBinding{} if err := r.Get(context.TODO(), types.NamespacedName{Name: desiredRoleBinding.Name, Namespace: cr.Namespace}, existingRoleBinding); err != nil { - if !errors.IsNotFound(err) { + if !apierrors.IsNotFound(err) { return fmt.Errorf("failed to get the rolebinding associated with %s : %s", desiredRoleBinding.Name, err) } // roleBinding does not exist and shouldn't, nothing to do here - if !cr.Spec.Notifications.Enabled { + if !isNotificationsEnabled(cr) { return nil } @@ -306,7 +335,7 @@ func (r *ReconcileArgoCD) reconcileNotificationsRoleBinding(cr *argoproj.ArgoCD, } // roleBinding exists but shouldn't, so it should be deleted - if !cr.Spec.Notifications.Enabled { + if !isNotificationsEnabled(cr) { argoutil.LogResourceDeletion(log, existingRoleBinding, "notifications are disabled") return r.Delete(context.TODO(), existingRoleBinding) } @@ -334,6 +363,19 @@ func (r *ReconcileArgoCD) reconcileNotificationsDeployment(cr *argoproj.ArgoCD, desiredDeployment := newDeploymentWithSuffix("notifications-controller", "controller", cr) + deplExists, err := argoutil.IsObjectFound(r.Client, cr.Namespace, desiredDeployment.Name, desiredDeployment) + if err != nil { + return err + } + + if !isNotificationsEnabled(cr) { + if deplExists { + argoutil.LogResourceDeletion(log, desiredDeployment, "notifications not enabled") + return r.Delete(context.TODO(), desiredDeployment) + } + return nil + } + desiredDeployment.Spec.Strategy = appsv1.DeploymentStrategy{ Type: appsv1.RecreateDeploymentStrategyType, } @@ -371,7 +413,7 @@ func (r *ReconcileArgoCD) reconcileNotificationsDeployment(cr *argoproj.ArgoCD, } podSpec.Containers = []corev1.Container{{ - Command: getNotificationsCommand(cr), + Command: r.getNotificationsCommand(cr), Image: getArgoContainerImage(cr), ImagePullPolicy: argoutil.GetImagePullPolicy(cr.Spec.ImagePullPolicy), Name: common.ArgoCDNotificationsControllerComponent, @@ -484,7 +526,7 @@ func (r *ReconcileArgoCD) reconcileNotificationsSecret(cr *argoproj.ArgoCD) erro secretExists := true existingSecret := &corev1.Secret{} if err := argoutil.FetchObject(r.Client, cr.Namespace, desiredSecret.Name, existingSecret); err != nil { - if !errors.IsNotFound(err) { + if !apierrors.IsNotFound(err) { return fmt.Errorf("failed to get the secret associated with %s : %s", desiredSecret.Name, err) } secretExists = false @@ -492,7 +534,7 @@ func (r *ReconcileArgoCD) reconcileNotificationsSecret(cr *argoproj.ArgoCD) erro if secretExists { // secret exists but shouldn't, so it should be deleted - if !cr.Spec.Notifications.Enabled { + if !isNotificationsEnabled(cr) { argoutil.LogResourceDeletion(log, existingSecret, "notifications are disabled") return r.Delete(context.TODO(), existingSecret) } @@ -502,7 +544,7 @@ func (r *ReconcileArgoCD) reconcileNotificationsSecret(cr *argoproj.ArgoCD) erro } // secret doesn't exist and shouldn't, nothing to do here - if !cr.Spec.Notifications.Enabled { + if !isNotificationsEnabled(cr) { return nil } @@ -520,7 +562,249 @@ func (r *ReconcileArgoCD) reconcileNotificationsSecret(cr *argoproj.ArgoCD) erro return nil } -func getNotificationsCommand(cr *argoproj.ArgoCD) []string { +// reconcileNotificationsClusterRoleBinding reconciles required clusterrole for notification controller when ArgoCD is cluster-scoped +func (r *ReconcileArgoCD) reconcileNotificationsClusterRole(cr *argoproj.ArgoCD) (*rbacv1.ClusterRole, error) { + + allowed := allowedNamespace(cr.Namespace, os.Getenv("ARGOCD_CLUSTER_CONFIG_NAMESPACES")) + + // controller disabled, don't create resources + if !isNotificationsEnabled(cr) { + allowed = false + } + + policyRules := policyRuleForNotificationsController() + clusterRole := newClusterRole(common.ArgoCDNotificationsControllerComponent, policyRules, cr) + if err := applyReconcilerHook(cr, clusterRole, ""); err != nil { + return nil, err + } + + existingClusterRole := &rbacv1.ClusterRole{} + err := r.Get(context.TODO(), types.NamespacedName{Name: clusterRole.Name}, existingClusterRole) + if err != nil { + if !apierrors.IsNotFound(err) { + return nil, fmt.Errorf("failed to reconcile the cluster role for the service account associated with %s : %s", clusterRole.Name, err) + } + if !allowed { + // Do Nothing + return clusterRole, nil + } + argoutil.LogResourceCreation(log, clusterRole) + return clusterRole, r.Create(context.TODO(), clusterRole) + } + + // ArgoCD not cluster scoped, cleanup any existing resource and exit + if !allowed { + argoutil.LogResourceDeletion(log, existingClusterRole, "argocd not cluster scoped") + err := r.Delete(context.TODO(), existingClusterRole) + if err != nil { + if !apierrors.IsNotFound(err) { + return existingClusterRole, err + } + } + return existingClusterRole, nil + } + + // if the Rules differ, update the Role + if !reflect.DeepEqual(existingClusterRole.Rules, clusterRole.Rules) { + existingClusterRole.Rules = clusterRole.Rules + argoutil.LogResourceUpdate(log, existingClusterRole, "updating rules") + if err := r.Update(context.TODO(), existingClusterRole); err != nil { + return nil, err + } + } + return existingClusterRole, nil +} + +// reconcileNotificationsClusterRoleBinding reconciles required clusterrolebinding for notifications controller when ArgoCD is cluster-scoped +func (r *ReconcileArgoCD) reconcileNotificationsClusterRoleBinding(cr *argoproj.ArgoCD, role *rbacv1.ClusterRole, sa *corev1.ServiceAccount) error { + + allowed := allowedNamespace(cr.Namespace, os.Getenv("ARGOCD_CLUSTER_CONFIG_NAMESPACES")) + + // controller disabled, don't create resources + if !isNotificationsEnabled(cr) { + allowed = false + } + + clusterRB := newClusterRoleBindingWithname(common.ArgoCDNotificationsControllerComponent, cr) + clusterRB.Subjects = []rbacv1.Subject{ + { + Kind: rbacv1.ServiceAccountKind, + Name: sa.Name, + Namespace: cr.Namespace, + }, + } + clusterRB.RoleRef = rbacv1.RoleRef{ + APIGroup: rbacv1.GroupName, + Kind: "ClusterRole", + Name: role.Name, + } + + if err := applyReconcilerHook(cr, clusterRB, ""); err != nil { + return err + } + + existingClusterRB := &rbacv1.ClusterRoleBinding{} + err := r.Get(context.TODO(), types.NamespacedName{Name: clusterRB.Name}, existingClusterRB) + if err != nil { + if !apierrors.IsNotFound(err) { + return fmt.Errorf("failed to reconcile the cluster rolebinding for the service account associated with %s : %s", clusterRB.Name, err) + } + if !allowed { + // Do Nothing + return nil + } + argoutil.LogResourceCreation(log, clusterRB) + return r.Create(context.TODO(), clusterRB) + } + + // ArgoCD not cluster scoped, cleanup any existing resource and exit + if !allowed { + argoutil.LogResourceDeletion(log, existingClusterRB, "argocd not cluster scoped") + err := r.Delete(context.TODO(), existingClusterRB) + if err != nil { + if !apierrors.IsNotFound(err) { + return err + } + } + return nil + } + + // if subj differ, update the rolebinding + if !reflect.DeepEqual(existingClusterRB.Subjects, clusterRB.Subjects) { + existingClusterRB.Subjects = clusterRB.Subjects + argoutil.LogResourceUpdate(log, existingClusterRB, "updating subjects") + if err := r.Update(context.TODO(), existingClusterRB); err != nil { + return err + } + } else if !reflect.DeepEqual(existingClusterRB.RoleRef, clusterRB.RoleRef) { + // RoleRef can't be updated, delete the rolebinding so that it gets recreated + argoutil.LogResourceDeletion(log, existingClusterRB, "roleref changed, deleting rolebinding so it gets recreated") + _ = r.Delete(context.TODO(), existingClusterRB) + return fmt.Errorf("change detected in roleRef for rolebinding %s of Argo CD instance %s in namespace %s", existingClusterRB.Name, cr.Name, existingClusterRB.Namespace) + } + return nil +} + +// reconcileNotificationsSourceNamespacesResources creates role & rolebinding in target source namespaces for notifications controller +// Notifications resources are only created if target source ns is subset of apps source namespaces +func (r *ReconcileArgoCD) reconcileNotificationsSourceNamespacesResources(cr *argoproj.ArgoCD) error { + + var reconciliationErrors []error + + // controller disabled, nothing to do. cleanup handled by removeUnmanagedNotificationsSourceNamespaceResources() + if !isNotificationsEnabled(cr) { + return nil + } + + // create resources for each notifications source namespace + for _, sourceNamespace := range cr.Spec.Notifications.SourceNamespaces { + + // source ns should be part of app-in-any-ns + appsNamespaces, err := r.getSourceNamespaces(cr) + if err != nil { + reconciliationErrors = append(reconciliationErrors, err) + continue + } + if !contains(appsNamespaces, sourceNamespace) { + log.Error(fmt.Errorf("skipping reconciliation of resources for sourceNamespace %s as Apps in target sourceNamespace is not enabled", sourceNamespace), "Warning") + continue + } + + // skip source ns if doesn't exist + namespace := &corev1.Namespace{} + if err := r.Get(context.TODO(), types.NamespacedName{Name: sourceNamespace}, namespace); err != nil { + errMsg := fmt.Errorf("failed to retrieve namespace %s", sourceNamespace) + reconciliationErrors = append(reconciliationErrors, errors.Join(errMsg, err)) + continue + } + + // No namespace can be managed by multiple argo-cd instances (cluster scoped or namespace scoped) + // i.e, only one of either managed-by or notifications-managed-by-cluster-argocd labels can be applied to a given namespace. + // prioritize managed-by label in case of a conflict. + if value, ok := namespace.Labels[common.ArgoCDManagedByLabel]; ok && value != "" { + log.Info(fmt.Sprintf("Skipping reconciling resources for namespace %s as it is already managed-by namespace %s.", namespace.Name, value)) + // remove any source namespace resources + if val, ok1 := namespace.Labels[common.ArgoCDNotificationsManagedByClusterArgoCDLabel]; ok1 && val != cr.Namespace { + delete(r.ManagedNotificationsSourceNamespaces, namespace.Name) + if err := r.cleanupUnmanagedNotificationsSourceNamespaceResources(cr, namespace.Name); err != nil { + log.Error(err, fmt.Sprintf("error cleaning up resources for namespace %s", namespace.Name)) + } + } + continue + } + + log.Info(fmt.Sprintf("Reconciling notifications resources for %s", namespace.Name)) + // add notifications-managed-by-cluster-argocd label on namespace + if _, ok := namespace.Labels[common.ArgoCDNotificationsManagedByClusterArgoCDLabel]; !ok { + // Get the latest value of namespace before updating it + if err := r.Get(context.TODO(), types.NamespacedName{Name: namespace.Name}, namespace); err != nil { + return err + } + // Update namespace with notifications-managed-by-cluster-argocd label + if namespace.Labels == nil { + namespace.Labels = make(map[string]string) + } + namespace.Labels[common.ArgoCDNotificationsManagedByClusterArgoCDLabel] = cr.Namespace + explanation := fmt.Sprintf("adding label '%s=%s'", common.ArgoCDNotificationsManagedByClusterArgoCDLabel, cr.Namespace) + argoutil.LogResourceUpdate(log, namespace, explanation) + if err := r.Update(context.TODO(), namespace); err != nil { + log.Error(err, fmt.Sprintf("failed to add label from namespace [%s]", namespace.Name)) + } + } + + // role & rolebinding for notifications controller in source namespace + role := rbacv1.Role{ + ObjectMeta: v1.ObjectMeta{ + Name: getResourceNameForNotificationsSourceNamespaces(cr), + Namespace: sourceNamespace, + Labels: argoutil.LabelsForCluster(cr), + }, + Rules: policyRuleForNotificationsController(), + } + err = r.reconcileSourceNamespaceRole(role, cr) + if err != nil { + reconciliationErrors = append(reconciliationErrors, err) + } + + roleBinding := rbacv1.RoleBinding{ + ObjectMeta: v1.ObjectMeta{ + Name: getResourceNameForNotificationsSourceNamespaces(cr), + Labels: argoutil.LabelsForCluster(cr), + Annotations: argoutil.AnnotationsForCluster(cr), + Namespace: sourceNamespace, + }, + RoleRef: rbacv1.RoleRef{ + APIGroup: v1.GroupName, + Kind: "Role", + Name: getResourceNameForNotificationsSourceNamespaces(cr), + }, + Subjects: []rbacv1.Subject{ + { + Kind: rbacv1.ServiceAccountKind, + Name: getServiceAccountName(cr.Name, "notifications-controller"), + Namespace: cr.Namespace, + }, + }, + } + err = r.reconcileSourceNamespaceRoleBinding(roleBinding, cr) + if err != nil { + reconciliationErrors = append(reconciliationErrors, err) + } + + // notifications permissions for argocd server in source namespaces are handled by apps-in-any-ns code + + if _, ok := r.ManagedNotificationsSourceNamespaces[sourceNamespace]; !ok { + if r.ManagedNotificationsSourceNamespaces == nil { + r.ManagedNotificationsSourceNamespaces = make(map[string]string) + } + r.ManagedNotificationsSourceNamespaces[sourceNamespace] = "" + } + } + + return amerr.NewAggregate(reconciliationErrors) +} + +func (r *ReconcileArgoCD) getNotificationsCommand(cr *argoproj.ArgoCD) []string { cmd := make([]string, 0) cmd = append(cmd, "argocd-notifications") @@ -537,6 +821,23 @@ func getNotificationsCommand(cr *argoproj.ArgoCD) []string { log.Info("Repo Server is disabled. This would affect the functioning of Notification Controller.") } + // notifications source namespaces should be subset of apps source namespaces + notificationsSourceNamespaces := []string{} + appsNamespaces, err := r.getSourceNamespaces(cr) + if err == nil { + for _, ns := range cr.Spec.Notifications.SourceNamespaces { + if contains(appsNamespaces, ns) { + notificationsSourceNamespaces = append(notificationsSourceNamespaces, ns) + } else { + log.V(1).Info(fmt.Sprintf("Apps in target sourceNamespace %s is not enabled, thus skipping the namespace in deployment command.", ns)) + } + } + } + + if len(notificationsSourceNamespaces) > 0 { + cmd = append(cmd, "--application-namespaces", fmt.Sprint(strings.Join(notificationsSourceNamespaces, ","))) + } + return cmd } @@ -551,3 +852,127 @@ func getNotificationsResources(cr *argoproj.ArgoCD) corev1.ResourceRequirements return resources } + +// Returns the name of the role/rolebinding for the source namespaces for notifications-controller in the format of "argocdName-argocdNamespace-notifications" +func getResourceNameForNotificationsSourceNamespaces(cr *argoproj.ArgoCD) string { + return fmt.Sprintf("%s-%s-notifications", cr.Name, cr.Namespace) +} + +// setManagedNotificationSourceNamespaces populates ManagedNotificationsSourceNamespaces var with namespaces +// with "argocd.argoproj.io/notifications-managed-by-cluster-argocd" label. +func (r *ReconcileArgoCD) setManagedNotificationsSourceNamespaces(cr *argoproj.ArgoCD) error { + if r.ManagedNotificationsSourceNamespaces == nil { + r.ManagedNotificationsSourceNamespaces = make(map[string]string) + } + namespaces := &corev1.NamespaceList{} + listOption := client.MatchingLabels{ + common.ArgoCDNotificationsManagedByClusterArgoCDLabel: cr.Namespace, + } + + // get the list of namespaces managed with "argocd.argoproj.io/notifications-managed-by-cluster-argocd" label + if err := r.List(context.TODO(), namespaces, listOption); err != nil { + return err + } + + for _, namespace := range namespaces.Items { + r.ManagedNotificationsSourceNamespaces[namespace.Name] = "" + } + + return nil +} + +// removeUnmanagedNotificationsSourceNamespaceResources cleansup resources from NotificationsSourceNamespaces if namespace is not managed by argocd instance. +// ManagedNotificationsSourceNamespaces var keeps track of namespaces with notifications resources. +func (r *ReconcileArgoCD) removeUnmanagedNotificationsSourceNamespaceResources(cr *argoproj.ArgoCD) error { + + for ns := range r.ManagedNotificationsSourceNamespaces { + managedNamespace := false + if isNotificationsEnabled(cr) && cr.GetDeletionTimestamp() == nil { + notificationsNamespaces, err := r.getSourceNamespaces(cr) + if err != nil { + return err + } + for _, namespace := range cr.Spec.Notifications.SourceNamespaces { + // notifications ns should be part of general ns + if namespace == ns && contains(notificationsNamespaces, namespace) { + managedNamespace = true + break + } + } + } + + if !managedNamespace { + if err := r.cleanupUnmanagedNotificationsSourceNamespaceResources(cr, ns); err != nil { + log.Error(err, fmt.Sprintf("error cleaning up notifications resources for namespace %s", ns)) + continue + } + delete(r.ManagedNotificationsSourceNamespaces, ns) + } + } + return nil +} + +// isNotificationsEnabled returns true if notifications are configured and enabled in the ArgoCD CR +func isNotificationsEnabled(cr *argoproj.ArgoCD) bool { + return !reflect.DeepEqual(cr.Spec.Notifications, argoproj.ArgoCDNotifications{}) && cr.Spec.Notifications.Enabled +} + +// cleanupUnmanagedNotificationsSourceNamespaceResources removes the notifications resources from target namespace +func (r *ReconcileArgoCD) cleanupUnmanagedNotificationsSourceNamespaceResources(cr *argoproj.ArgoCD, ns string) error { + namespace := corev1.Namespace{} + if err := r.Get(context.TODO(), types.NamespacedName{Name: ns}, &namespace); err != nil { + if !apierrors.IsNotFound(err) { + return err + } + return nil + } + + // Delete notifications role & rolebinding + existingRole := rbacv1.Role{} + roleName := getResourceNameForNotificationsSourceNamespaces(cr) + if err := r.Get(context.TODO(), types.NamespacedName{Name: roleName, Namespace: namespace.Name}, &existingRole); err != nil { + if !apierrors.IsNotFound(err) { + return fmt.Errorf("failed to fetch the role for the service account associated with %s : %s", common.ArgoCDNotificationsControllerComponent, err) + } + } + if existingRole.Name != "" { + argoutil.LogResourceDeletion(log, &existingRole, "cleaning up unmanaged notifications resources") + err := r.Delete(context.TODO(), &existingRole) + if err != nil { + return err + } + } + + existingRoleBinding := &rbacv1.RoleBinding{} + roleBindingName := getResourceNameForNotificationsSourceNamespaces(cr) + if err := r.Get(context.TODO(), types.NamespacedName{Name: roleBindingName, Namespace: namespace.Name}, existingRoleBinding); err != nil { + if !apierrors.IsNotFound(err) { + return fmt.Errorf("failed to get the rolebinding associated with %s : %s", common.ArgoCDNotificationsControllerComponent, err) + } + } + if existingRoleBinding.Name != "" { + argoutil.LogResourceDeletion(log, existingRoleBinding, "cleaning up unmanaged notifications resources") + if err := r.Delete(context.TODO(), existingRoleBinding); err != nil { + return err + } + } + + // app-in-any-ns code will handle removal of notifications permissions for argocd-server in target namespace + + // Remove notifications-managed-by-cluster-argocd label from the namespace + argoutil.LogResourceUpdate(log, &namespace, "removing label", common.ArgoCDNotificationsManagedByClusterArgoCDLabel) + delete(namespace.Labels, common.ArgoCDNotificationsManagedByClusterArgoCDLabel) + if err := r.Update(context.TODO(), &namespace); err != nil { + return fmt.Errorf("failed to remove notifications label from namespace %s : %s", namespace.Name, err) + } + + return nil +} + +// getNotificationsSetSourceNamespaces return list of namespaces from .spec.Notifications.SourceNamespaces +func (r *ReconcileArgoCD) getNotificationsSourceNamespaces(cr *argoproj.ArgoCD) []string { + if isNotificationsEnabled(cr) { + return cr.Spec.Notifications.SourceNamespaces + } + return []string(nil) +} diff --git a/controllers/argocd/notifications_test.go b/controllers/argocd/notifications_test.go index 012ff7460..9a8b3e7ea 100644 --- a/controllers/argocd/notifications_test.go +++ b/controllers/argocd/notifications_test.go @@ -142,6 +142,101 @@ func TestReconcileNotifications_CreateRoleBinding(t *testing.T) { assert.True(t, errors.IsNotFound(err)) } +func TestReconcileNotifications_Deployments_Command(t *testing.T) { + logf.SetLogger(ZapLogger(true)) + + tests := []struct { + name string + argocdSpec argoproj.ArgoCDSpec + expectedCmd []string + notExpectedCmd []string + }{ + { + name: "Notifications contained in spec.sourceNamespaces", + + argocdSpec: argoproj.ArgoCDSpec{ + Notifications: argoproj.ArgoCDNotifications{ + Enabled: true, + SourceNamespaces: []string{"foo", "bar"}, + }, + SourceNamespaces: []string{"foo", "bar"}, + }, + expectedCmd: []string{"--application-namespaces", "foo,bar"}, + }, + { + name: "Only notifications contained in spec.sourceNamespaces", + argocdSpec: argoproj.ArgoCDSpec{ + Notifications: argoproj.ArgoCDNotifications{ + Enabled: true, + SourceNamespaces: []string{"foo"}, + }, + SourceNamespaces: []string{"foo", "bar"}, + }, + expectedCmd: []string{"--application-namespaces", "foo"}, + }, + { + name: "Empty spec.sourceNamespaces, no application namespaces arg", + argocdSpec: argoproj.ArgoCDSpec{ + Notifications: argoproj.ArgoCDNotifications{ + Enabled: true, + SourceNamespaces: []string{"foo"}, + }, + SourceNamespaces: []string{}, + }, + expectedCmd: []string{}, + notExpectedCmd: []string{"--application-namespaces", "foo"}, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + + a := makeTestArgoCD() + ns1 := v1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + }, + } + ns2 := v1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "bar", + }, + } + resObjs := []client.Object{a, &ns1, &ns2} + subresObjs := []client.Object{a} + runtimeObjs := []runtime.Object{} + sch := makeTestReconcilerScheme(argoproj.AddToScheme) + cl := makeTestReconcilerClient(sch, resObjs, subresObjs, runtimeObjs) + r := makeTestReconciler(cl, sch, testclient.NewSimpleClientset()) + cm := newConfigMapWithName(getCAConfigMapName(a), a) + err := r.Create(context.Background(), cm, &client.CreateOptions{}) + assert.NoError(t, err) + + a.Spec = test.argocdSpec + + sa := v1.ServiceAccount{} + assert.NoError(t, r.reconcileNotificationsDeployment(a, &sa)) + + deployment := &appsv1.Deployment{} + assert.NoError(t, r.Get( + context.TODO(), + types.NamespacedName{ + Name: "argocd-notifications-controller", + Namespace: a.Namespace, + }, + deployment)) + + cmds := deployment.Spec.Template.Spec.Containers[0].Command + for _, c := range test.expectedCmd { + assert.True(t, contains(cmds, c)) + } + for _, c := range test.notExpectedCmd { + assert.False(t, contains(cmds, c)) + } + }) + } +} + func TestReconcileNotifications_CreateDeployments(t *testing.T) { logf.SetLogger(ZapLogger(true)) a := makeTestArgoCD(func(a *argoproj.ArgoCD) { @@ -569,3 +664,172 @@ func TestReconcileNotifications_testLogFormat(t *testing.T) { t.Fatalf("operator failed to override the manual changes to notification controller logFormat:\n%s", diff) } } + +func TestArgoCDNotifications_getNotificationsSourceNamespaces(t *testing.T) { + logf.SetLogger(ZapLogger(true)) + + tests := []struct { + name string + notificationsField argoproj.ArgoCDNotifications + expected []string + }{ + { + name: "No Notifications configured", + notificationsField: argoproj.ArgoCDNotifications{}, + expected: []string(nil), + }, + { + name: "Notifications enabled No notifications source namespaces", + notificationsField: argoproj.ArgoCDNotifications{ + Enabled: true, + }, + expected: []string(nil), + }, + { + name: "Notifications enabled and notifications source namespaces", + notificationsField: argoproj.ArgoCDNotifications{ + Enabled: true, + SourceNamespaces: []string{"foo", "bar"}, + }, + expected: []string{"foo", "bar"}, + }, + { + name: "Notifications disabled and notifications source namespaces", + notificationsField: argoproj.ArgoCDNotifications{ + Enabled: false, + SourceNamespaces: []string{"foo", "bar"}, + }, + expected: []string(nil), + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + + a := makeTestArgoCD() + resObjs := []client.Object{a} + subresObjs := []client.Object{a} + runtimeObjs := []runtime.Object{} + sch := makeTestReconcilerScheme(argoproj.AddToScheme) + cl := makeTestReconcilerClient(sch, resObjs, subresObjs, runtimeObjs) + r := makeTestReconciler(cl, sch, testclient.NewSimpleClientset()) + cm := newConfigMapWithName(getCAConfigMapName(a), a) + err := r.Create(context.Background(), cm, &client.CreateOptions{}) + assert.NoError(t, err) + + a.Spec.Notifications = test.notificationsField + + actual := r.getNotificationsSourceNamespaces(a) + assert.Equal(t, test.expected, actual) + }) + } +} + +func TestArgoCDNotifications_setManagedNotificationsSourceNamespaces(t *testing.T) { + a := makeTestArgoCD() + ns1 := v1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-namespace-1", + Labels: map[string]string{ + common.ArgoCDNotificationsManagedByClusterArgoCDLabel: testNamespace, + }, + }, + } + ns2 := v1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-namespace-2", + }, + } + + resObjs := []client.Object{a, &ns1, &ns2} + subresObjs := []client.Object{a} + runtimeObjs := []runtime.Object{} + sch := makeTestReconcilerScheme(argoproj.AddToScheme) + cl := makeTestReconcilerClient(sch, resObjs, subresObjs, runtimeObjs) + r := makeTestReconciler(cl, sch, testclient.NewSimpleClientset()) + + err := r.setManagedNotificationsSourceNamespaces(a) + assert.NoError(t, err) + + assert.Equal(t, 1, len(r.ManagedNotificationsSourceNamespaces)) + assert.Contains(t, r.ManagedNotificationsSourceNamespaces, "test-namespace-1") +} + +func TestNotifications_removeUnmanagedNotificationsSourceNamespaceResources(t *testing.T) { + ns1 := "foo" + ns2 := "bar" + a := makeTestArgoCD() + a.Spec = argoproj.ArgoCDSpec{ + SourceNamespaces: []string{ns1, ns2}, + Notifications: argoproj.ArgoCDNotifications{ + Enabled: true, + SourceNamespaces: []string{ns1, ns2}, + }, + } + + resObjs := []client.Object{a} + subresObjs := []client.Object{a} + runtimeObjs := []runtime.Object{} + sch := makeTestReconcilerScheme(argoproj.AddToScheme) + cl := makeTestReconcilerClient(sch, resObjs, subresObjs, runtimeObjs) + r := makeTestReconciler(cl, sch, testclient.NewSimpleClientset()) + + err := createNamespace(r, ns1, "") + assert.NoError(t, err) + err = createNamespace(r, ns2, "") + assert.NoError(t, err) + + // create resources + err = r.reconcileNotificationsSourceNamespacesResources(a) + assert.NoError(t, err) + + // remove notifications ns + a.Spec = argoproj.ArgoCDSpec{ + SourceNamespaces: []string{ns2}, + Notifications: argoproj.ArgoCDNotifications{ + Enabled: true, + SourceNamespaces: []string{ns1, ns2}, + }, + } + + // clean up unmanaged namespaces resources + err = r.removeUnmanagedNotificationsSourceNamespaceResources(a) + assert.NoError(t, err) + + // resources shouldn't exist in ns1 + resName := getResourceNameForNotificationsSourceNamespaces(a) + + role := &rbacv1.Role{} + err = r.Get(context.TODO(), client.ObjectKey{Name: resName, Namespace: ns1}, role) + assert.Error(t, err) + assert.True(t, errors.IsNotFound(err)) + + roleBinding := &rbacv1.RoleBinding{} + err = r.Get(context.TODO(), client.ObjectKey{Name: resName, Namespace: ns1}, roleBinding) + assert.Error(t, err) + assert.True(t, errors.IsNotFound(err)) + + // notifications tracking label should be removed + namespace := &v1.Namespace{} + err = r.Get(context.TODO(), client.ObjectKey{Name: ns1}, namespace) + assert.NoError(t, err) + _, found := namespace.Labels[common.ArgoCDNotificationsManagedByClusterArgoCDLabel] + assert.False(t, found) + + // resources in ns2 shouldn't be touched + + role = &rbacv1.Role{} + err = r.Get(context.TODO(), client.ObjectKey{Name: resName, Namespace: ns2}, role) + assert.NoError(t, err) + + roleBinding = &rbacv1.RoleBinding{} + err = r.Get(context.TODO(), client.ObjectKey{Name: resName, Namespace: ns2}, roleBinding) + assert.NoError(t, err) + + namespace = &v1.Namespace{} + err = r.Get(context.TODO(), client.ObjectKey{Name: ns2}, namespace) + assert.NoError(t, err) + val, found := namespace.Labels[common.ArgoCDNotificationsManagedByClusterArgoCDLabel] + assert.True(t, found) + assert.Equal(t, a.Namespace, val) +} diff --git a/controllers/argocd/policyrule.go b/controllers/argocd/policyrule.go index ccb56a488..3dbbdab41 100644 --- a/controllers/argocd/policyrule.go +++ b/controllers/argocd/policyrule.go @@ -607,6 +607,28 @@ func policyRuleForServerApplicationSetSourceNamespaces() []v1.PolicyRule { }, } } +func policyRuleForServerNotificationsSourceNamespaces() []v1.PolicyRule { + return []v1.PolicyRule{ + { + APIGroups: []string{ + "argoproj.io", + }, + Resources: []string{ + "applications", + "appprojects", + }, + Verbs: []string{ + "create", + "get", + "list", + "patch", + "update", + "watch", + "delete", + }, + }, + } +} func policyRuleForRoleForImageUpdaterController() []v1.PolicyRule { return []v1.PolicyRule{ diff --git a/controllers/argocd/role.go b/controllers/argocd/role.go index 91e804e43..d89d479ad 100644 --- a/controllers/argocd/role.go +++ b/controllers/argocd/role.go @@ -230,7 +230,7 @@ func (r *ReconcileArgoCD) reconcileRoleForApplicationSourceNamespaces(name strin } // do not reconcile roles for namespaces already containing managed-by label // as it already contains roles with permissions to manipulate application resources - // reconciled during reconcilation of ManagedNamespaces + // reconciled during reconciliation of ManagedNamespaces if value, ok := namespace.Labels[common.ArgoCDManagedByLabel]; ok && value != "" { log.Info(fmt.Sprintf("Skipping reconciling resources for namespace %s as it is already managed-by namespace %s.", namespace.Name, value)) // if managed-by-cluster-argocd label is also present, remove the namespace from the ManagedSourceNamespaces. @@ -260,7 +260,9 @@ func (r *ReconcileArgoCD) reconcileRoleForApplicationSourceNamespaces(name strin if contains(r.getApplicationSetSourceNamespaces(cr), sourceNamespace) { role.Rules = append(role.Rules, policyRuleForServerApplicationSetSourceNamespaces()...) } - + if contains(r.getNotificationsSourceNamespaces(cr), sourceNamespace) { + role.Rules = append(role.Rules, policyRuleForServerNotificationsSourceNamespaces()...) + } created := false existingRole := v1.Role{} err := r.Get(context.TODO(), types.NamespacedName{Name: role.Name, Namespace: namespace.Name}, &existingRole) diff --git a/controllers/argocd/role_test.go b/controllers/argocd/role_test.go index 6c1fa3200..ba03bdc7b 100644 --- a/controllers/argocd/role_test.go +++ b/controllers/argocd/role_test.go @@ -205,6 +205,10 @@ func TestReconcileArgoCD_reconcileRoleForApplicationSourceNamespaces(t *testing. ApplicationSet: &argoproj.ArgoCDApplicationSet{ SourceNamespaces: []string{"tmp"}, }, + Notifications: argoproj.ArgoCDNotifications{ + Enabled: true, + SourceNamespaces: []string{"tmp"}, + }, } resObjs := []client.Object{a} @@ -237,12 +241,18 @@ func TestReconcileArgoCD_reconcileRoleForApplicationSourceNamespaces(t *testing. ApplicationSet: &argoproj.ArgoCDApplicationSet{ SourceNamespaces: []string{"tmp", sourceNamespace}, }, + Notifications: argoproj.ArgoCDNotifications{ + Enabled: true, + SourceNamespaces: []string{"tmp", sourceNamespace}, + }, } err = r.reconcileRoleForApplicationSourceNamespaces(workloadIdentifier, expectedRules, a) assert.NoError(t, err) reconciledRole = &v1.Role{} assert.NoError(t, r.Get(context.TODO(), types.NamespacedName{Name: expectedName, Namespace: sourceNamespace}, reconciledRole)) - assert.Equal(t, append(expectedRules, policyRuleForServerApplicationSetSourceNamespaces()...), reconciledRole.Rules) + expectedRules = append(expectedRules, policyRuleForServerApplicationSetSourceNamespaces()...) + expectedRules = append(expectedRules, policyRuleForServerNotificationsSourceNamespaces()...) + assert.Equal(t, expectedRules, reconciledRole.Rules) // check if appset rules are removed for server-role when appset namespace is removed from the list a.Spec = argoproj.ArgoCDSpec{ @@ -252,7 +262,12 @@ func TestReconcileArgoCD_reconcileRoleForApplicationSourceNamespaces(t *testing. ApplicationSet: &argoproj.ArgoCDApplicationSet{ SourceNamespaces: []string{"tmp"}, }, + Notifications: argoproj.ArgoCDNotifications{ + Enabled: true, + SourceNamespaces: []string{"tmp"}, + }, } + expectedRules = policyRuleForServerApplicationSourceNamespaces() err = r.reconcileRoleForApplicationSourceNamespaces(workloadIdentifier, expectedRules, a) assert.NoError(t, err) reconciledRole = &v1.Role{} diff --git a/controllers/argocd/util.go b/controllers/argocd/util.go index d6c1838c1..71d94e1a8 100644 --- a/controllers/argocd/util.go +++ b/controllers/argocd/util.go @@ -866,7 +866,7 @@ func (r *ReconcileArgoCD) reconcileResources(cr *argoproj.ArgoCD, argocdStatus * } } - if cr.Spec.Notifications.Enabled { + if !reflect.DeepEqual(cr.Spec.Notifications, argoproj.ArgoCDNotifications{}) || len(r.ManagedNotificationsSourceNamespaces) > 0 { log.Info("reconciling Notifications controller") if err := r.reconcileNotificationsController(cr); err != nil { return err diff --git a/deploy/olm-catalog/argocd-operator/0.17.0/argoproj.io_argocds.yaml b/deploy/olm-catalog/argocd-operator/0.17.0/argoproj.io_argocds.yaml index 8ec45d7c0..c03a71edf 100644 --- a/deploy/olm-catalog/argocd-operator/0.17.0/argoproj.io_argocds.yaml +++ b/deploy/olm-catalog/argocd-operator/0.17.0/argoproj.io_argocds.yaml @@ -1810,6 +1810,13 @@ spec: More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/ type: object type: object + sourceNamespaces: + description: SourceNamespaces is a list of namespaces from which + the notifications controller will watch for ArgoCD Application + resources. + items: + type: string + type: array version: description: Version is the Argo CD Notifications image tag. (optional) type: string @@ -16713,6 +16720,13 @@ spec: More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/ type: object type: object + sourceNamespaces: + description: SourceNamespaces is a list of namespaces from which + the notifications controller will watch for ArgoCD Notification + resources. + items: + type: string + type: array version: description: Version is the Argo CD Notifications image tag. (optional) type: string From 6f526a45b2c35010363e97311540036abb50240d Mon Sep 17 00:00:00 2001 From: nmirasch Date: Wed, 29 Oct 2025 14:41:48 +0100 Subject: [PATCH 2/5] Removed cluster roles and added self-service-notification-enabled to command Signed-off-by: nmirasch --- controllers/argocd/notifications.go | 141 +---------------------- controllers/argocd/notifications_test.go | 6 +- controllers/argocd/policyrule.go | 22 ---- controllers/argocd/role.go | 6 +- controllers/argocd/role_test.go | 17 +-- 5 files changed, 9 insertions(+), 183 deletions(-) diff --git a/controllers/argocd/notifications.go b/controllers/argocd/notifications.go index 148e3f30c..bd540d82a 100644 --- a/controllers/argocd/notifications.go +++ b/controllers/argocd/notifications.go @@ -4,7 +4,6 @@ import ( "context" "errors" "fmt" - "os" "reflect" "strings" @@ -70,18 +69,6 @@ func (r *ReconcileArgoCD) reconcileNotificationsController(cr *argoproj.ArgoCD) return err } - // create clusterrole & clusterrolebinding if cluster-scoped ArgoCD - log.Info("reconciling notifications clusterroles") - clusterrole, err := r.reconcileNotificationsClusterRole(cr) - if err != nil { - return err - } - - log.Info("reconciling notifications clusterrolebindings") - if err := r.reconcileNotificationsClusterRoleBinding(cr, clusterrole, sa); err != nil { - return err - } - // reconcile source namespace roles & rolebindings log.Info("reconciling notifications roles & rolebindings in source namespaces") if err := r.reconcileNotificationsSourceNamespacesResources(cr); err != nil { @@ -562,129 +549,6 @@ func (r *ReconcileArgoCD) reconcileNotificationsSecret(cr *argoproj.ArgoCD) erro return nil } -// reconcileNotificationsClusterRoleBinding reconciles required clusterrole for notification controller when ArgoCD is cluster-scoped -func (r *ReconcileArgoCD) reconcileNotificationsClusterRole(cr *argoproj.ArgoCD) (*rbacv1.ClusterRole, error) { - - allowed := allowedNamespace(cr.Namespace, os.Getenv("ARGOCD_CLUSTER_CONFIG_NAMESPACES")) - - // controller disabled, don't create resources - if !isNotificationsEnabled(cr) { - allowed = false - } - - policyRules := policyRuleForNotificationsController() - clusterRole := newClusterRole(common.ArgoCDNotificationsControllerComponent, policyRules, cr) - if err := applyReconcilerHook(cr, clusterRole, ""); err != nil { - return nil, err - } - - existingClusterRole := &rbacv1.ClusterRole{} - err := r.Get(context.TODO(), types.NamespacedName{Name: clusterRole.Name}, existingClusterRole) - if err != nil { - if !apierrors.IsNotFound(err) { - return nil, fmt.Errorf("failed to reconcile the cluster role for the service account associated with %s : %s", clusterRole.Name, err) - } - if !allowed { - // Do Nothing - return clusterRole, nil - } - argoutil.LogResourceCreation(log, clusterRole) - return clusterRole, r.Create(context.TODO(), clusterRole) - } - - // ArgoCD not cluster scoped, cleanup any existing resource and exit - if !allowed { - argoutil.LogResourceDeletion(log, existingClusterRole, "argocd not cluster scoped") - err := r.Delete(context.TODO(), existingClusterRole) - if err != nil { - if !apierrors.IsNotFound(err) { - return existingClusterRole, err - } - } - return existingClusterRole, nil - } - - // if the Rules differ, update the Role - if !reflect.DeepEqual(existingClusterRole.Rules, clusterRole.Rules) { - existingClusterRole.Rules = clusterRole.Rules - argoutil.LogResourceUpdate(log, existingClusterRole, "updating rules") - if err := r.Update(context.TODO(), existingClusterRole); err != nil { - return nil, err - } - } - return existingClusterRole, nil -} - -// reconcileNotificationsClusterRoleBinding reconciles required clusterrolebinding for notifications controller when ArgoCD is cluster-scoped -func (r *ReconcileArgoCD) reconcileNotificationsClusterRoleBinding(cr *argoproj.ArgoCD, role *rbacv1.ClusterRole, sa *corev1.ServiceAccount) error { - - allowed := allowedNamespace(cr.Namespace, os.Getenv("ARGOCD_CLUSTER_CONFIG_NAMESPACES")) - - // controller disabled, don't create resources - if !isNotificationsEnabled(cr) { - allowed = false - } - - clusterRB := newClusterRoleBindingWithname(common.ArgoCDNotificationsControllerComponent, cr) - clusterRB.Subjects = []rbacv1.Subject{ - { - Kind: rbacv1.ServiceAccountKind, - Name: sa.Name, - Namespace: cr.Namespace, - }, - } - clusterRB.RoleRef = rbacv1.RoleRef{ - APIGroup: rbacv1.GroupName, - Kind: "ClusterRole", - Name: role.Name, - } - - if err := applyReconcilerHook(cr, clusterRB, ""); err != nil { - return err - } - - existingClusterRB := &rbacv1.ClusterRoleBinding{} - err := r.Get(context.TODO(), types.NamespacedName{Name: clusterRB.Name}, existingClusterRB) - if err != nil { - if !apierrors.IsNotFound(err) { - return fmt.Errorf("failed to reconcile the cluster rolebinding for the service account associated with %s : %s", clusterRB.Name, err) - } - if !allowed { - // Do Nothing - return nil - } - argoutil.LogResourceCreation(log, clusterRB) - return r.Create(context.TODO(), clusterRB) - } - - // ArgoCD not cluster scoped, cleanup any existing resource and exit - if !allowed { - argoutil.LogResourceDeletion(log, existingClusterRB, "argocd not cluster scoped") - err := r.Delete(context.TODO(), existingClusterRB) - if err != nil { - if !apierrors.IsNotFound(err) { - return err - } - } - return nil - } - - // if subj differ, update the rolebinding - if !reflect.DeepEqual(existingClusterRB.Subjects, clusterRB.Subjects) { - existingClusterRB.Subjects = clusterRB.Subjects - argoutil.LogResourceUpdate(log, existingClusterRB, "updating subjects") - if err := r.Update(context.TODO(), existingClusterRB); err != nil { - return err - } - } else if !reflect.DeepEqual(existingClusterRB.RoleRef, clusterRB.RoleRef) { - // RoleRef can't be updated, delete the rolebinding so that it gets recreated - argoutil.LogResourceDeletion(log, existingClusterRB, "roleref changed, deleting rolebinding so it gets recreated") - _ = r.Delete(context.TODO(), existingClusterRB) - return fmt.Errorf("change detected in roleRef for rolebinding %s of Argo CD instance %s in namespace %s", existingClusterRB.Name, cr.Name, existingClusterRB.Namespace) - } - return nil -} - // reconcileNotificationsSourceNamespacesResources creates role & rolebinding in target source namespaces for notifications controller // Notifications resources are only created if target source ns is subset of apps source namespaces func (r *ReconcileArgoCD) reconcileNotificationsSourceNamespacesResources(cr *argoproj.ArgoCD) error { @@ -774,7 +638,7 @@ func (r *ReconcileArgoCD) reconcileNotificationsSourceNamespacesResources(cr *ar Namespace: sourceNamespace, }, RoleRef: rbacv1.RoleRef{ - APIGroup: v1.GroupName, + APIGroup: rbacv1.GroupName, Kind: "Role", Name: getResourceNameForNotificationsSourceNamespaces(cr), }, @@ -835,7 +699,8 @@ func (r *ReconcileArgoCD) getNotificationsCommand(cr *argoproj.ArgoCD) []string } if len(notificationsSourceNamespaces) > 0 { - cmd = append(cmd, "--application-namespaces", fmt.Sprint(strings.Join(notificationsSourceNamespaces, ","))) + cmd = append(cmd, "--application-namespaces", strings.Join(notificationsSourceNamespaces, ",")) + cmd = append(cmd, "--self-service-notification-enabled", "true") } return cmd diff --git a/controllers/argocd/notifications_test.go b/controllers/argocd/notifications_test.go index 9a8b3e7ea..1e7387514 100644 --- a/controllers/argocd/notifications_test.go +++ b/controllers/argocd/notifications_test.go @@ -161,7 +161,7 @@ func TestReconcileNotifications_Deployments_Command(t *testing.T) { }, SourceNamespaces: []string{"foo", "bar"}, }, - expectedCmd: []string{"--application-namespaces", "foo,bar"}, + expectedCmd: []string{"--application-namespaces", "foo,bar", "--self-service-notification-enabled", "true"}, }, { name: "Only notifications contained in spec.sourceNamespaces", @@ -172,7 +172,7 @@ func TestReconcileNotifications_Deployments_Command(t *testing.T) { }, SourceNamespaces: []string{"foo", "bar"}, }, - expectedCmd: []string{"--application-namespaces", "foo"}, + expectedCmd: []string{"--application-namespaces", "foo", "--self-service-notification-enabled", "true"}, }, { name: "Empty spec.sourceNamespaces, no application namespaces arg", @@ -184,7 +184,7 @@ func TestReconcileNotifications_Deployments_Command(t *testing.T) { SourceNamespaces: []string{}, }, expectedCmd: []string{}, - notExpectedCmd: []string{"--application-namespaces", "foo"}, + notExpectedCmd: []string{"--application-namespaces", "foo", "--self-service-notification-enabled", "true"}, }, } diff --git a/controllers/argocd/policyrule.go b/controllers/argocd/policyrule.go index 3dbbdab41..ccb56a488 100644 --- a/controllers/argocd/policyrule.go +++ b/controllers/argocd/policyrule.go @@ -607,28 +607,6 @@ func policyRuleForServerApplicationSetSourceNamespaces() []v1.PolicyRule { }, } } -func policyRuleForServerNotificationsSourceNamespaces() []v1.PolicyRule { - return []v1.PolicyRule{ - { - APIGroups: []string{ - "argoproj.io", - }, - Resources: []string{ - "applications", - "appprojects", - }, - Verbs: []string{ - "create", - "get", - "list", - "patch", - "update", - "watch", - "delete", - }, - }, - } -} func policyRuleForRoleForImageUpdaterController() []v1.PolicyRule { return []v1.PolicyRule{ diff --git a/controllers/argocd/role.go b/controllers/argocd/role.go index d89d479ad..91e804e43 100644 --- a/controllers/argocd/role.go +++ b/controllers/argocd/role.go @@ -230,7 +230,7 @@ func (r *ReconcileArgoCD) reconcileRoleForApplicationSourceNamespaces(name strin } // do not reconcile roles for namespaces already containing managed-by label // as it already contains roles with permissions to manipulate application resources - // reconciled during reconciliation of ManagedNamespaces + // reconciled during reconcilation of ManagedNamespaces if value, ok := namespace.Labels[common.ArgoCDManagedByLabel]; ok && value != "" { log.Info(fmt.Sprintf("Skipping reconciling resources for namespace %s as it is already managed-by namespace %s.", namespace.Name, value)) // if managed-by-cluster-argocd label is also present, remove the namespace from the ManagedSourceNamespaces. @@ -260,9 +260,7 @@ func (r *ReconcileArgoCD) reconcileRoleForApplicationSourceNamespaces(name strin if contains(r.getApplicationSetSourceNamespaces(cr), sourceNamespace) { role.Rules = append(role.Rules, policyRuleForServerApplicationSetSourceNamespaces()...) } - if contains(r.getNotificationsSourceNamespaces(cr), sourceNamespace) { - role.Rules = append(role.Rules, policyRuleForServerNotificationsSourceNamespaces()...) - } + created := false existingRole := v1.Role{} err := r.Get(context.TODO(), types.NamespacedName{Name: role.Name, Namespace: namespace.Name}, &existingRole) diff --git a/controllers/argocd/role_test.go b/controllers/argocd/role_test.go index ba03bdc7b..6c1fa3200 100644 --- a/controllers/argocd/role_test.go +++ b/controllers/argocd/role_test.go @@ -205,10 +205,6 @@ func TestReconcileArgoCD_reconcileRoleForApplicationSourceNamespaces(t *testing. ApplicationSet: &argoproj.ArgoCDApplicationSet{ SourceNamespaces: []string{"tmp"}, }, - Notifications: argoproj.ArgoCDNotifications{ - Enabled: true, - SourceNamespaces: []string{"tmp"}, - }, } resObjs := []client.Object{a} @@ -241,18 +237,12 @@ func TestReconcileArgoCD_reconcileRoleForApplicationSourceNamespaces(t *testing. ApplicationSet: &argoproj.ArgoCDApplicationSet{ SourceNamespaces: []string{"tmp", sourceNamespace}, }, - Notifications: argoproj.ArgoCDNotifications{ - Enabled: true, - SourceNamespaces: []string{"tmp", sourceNamespace}, - }, } err = r.reconcileRoleForApplicationSourceNamespaces(workloadIdentifier, expectedRules, a) assert.NoError(t, err) reconciledRole = &v1.Role{} assert.NoError(t, r.Get(context.TODO(), types.NamespacedName{Name: expectedName, Namespace: sourceNamespace}, reconciledRole)) - expectedRules = append(expectedRules, policyRuleForServerApplicationSetSourceNamespaces()...) - expectedRules = append(expectedRules, policyRuleForServerNotificationsSourceNamespaces()...) - assert.Equal(t, expectedRules, reconciledRole.Rules) + assert.Equal(t, append(expectedRules, policyRuleForServerApplicationSetSourceNamespaces()...), reconciledRole.Rules) // check if appset rules are removed for server-role when appset namespace is removed from the list a.Spec = argoproj.ArgoCDSpec{ @@ -262,12 +252,7 @@ func TestReconcileArgoCD_reconcileRoleForApplicationSourceNamespaces(t *testing. ApplicationSet: &argoproj.ArgoCDApplicationSet{ SourceNamespaces: []string{"tmp"}, }, - Notifications: argoproj.ArgoCDNotifications{ - Enabled: true, - SourceNamespaces: []string{"tmp"}, - }, } - expectedRules = policyRuleForServerApplicationSourceNamespaces() err = r.reconcileRoleForApplicationSourceNamespaces(workloadIdentifier, expectedRules, a) assert.NoError(t, err) reconciledRole = &v1.Role{} From 19249c282184ecd9069e210020bdf4cf8ae13f0b Mon Sep 17 00:00:00 2001 From: nmirasch Date: Mon, 3 Nov 2025 12:24:18 +0100 Subject: [PATCH 3/5] Add NotificationsConfiguration CR in the spec.notifications.sourceNamespaces Signed-off-by: nmirasch --- controllers/argocd/notifications.go | 104 ++++++++++- controllers/argocd/notifications_test.go | 225 ++++++++++++++++++++++- 2 files changed, 327 insertions(+), 2 deletions(-) diff --git a/controllers/argocd/notifications.go b/controllers/argocd/notifications.go index bd540d82a..6cbbe08d6 100644 --- a/controllers/argocd/notifications.go +++ b/controllers/argocd/notifications.go @@ -570,7 +570,7 @@ func (r *ReconcileArgoCD) reconcileNotificationsSourceNamespacesResources(cr *ar continue } if !contains(appsNamespaces, sourceNamespace) { - log.Error(fmt.Errorf("skipping reconciliation of resources for sourceNamespace %s as Apps in target sourceNamespace is not enabled", sourceNamespace), "Warning") + log.Error(fmt.Errorf("skipping reconciliation of Notification resources for sourceNamespace %s as Apps in target sourceNamespace is not enabled", sourceNamespace), "Warning") continue } @@ -655,6 +655,11 @@ func (r *ReconcileArgoCD) reconcileNotificationsSourceNamespacesResources(cr *ar reconciliationErrors = append(reconciliationErrors, err) } + // ensure NotificationsConfiguration CR exists in the source namespace + if err := r.reconcileSourceNamespaceNotificationsConfigurationCR(cr, sourceNamespace); err != nil { + reconciliationErrors = append(reconciliationErrors, err) + } + // notifications permissions for argocd server in source namespaces are handled by apps-in-any-ns code if _, ok := r.ManagedNotificationsSourceNamespaces[sourceNamespace]; !ok { @@ -668,6 +673,90 @@ func (r *ReconcileArgoCD) reconcileNotificationsSourceNamespacesResources(cr *ar return amerr.NewAggregate(reconciliationErrors) } +// reconcileSourceNamespaceNotificationsConfigurationCR ensures a NotificationsConfiguration CR exists in the given source namespace. +// It propagates the NotificationsConfiguration from the Argo CD instance namespace if it exists, otherwise uses defaults. +func (r *ReconcileArgoCD) reconcileSourceNamespaceNotificationsConfigurationCR(cr *argoproj.ArgoCD, sourceNamespace string) error { + if !isNotificationsEnabled(cr) { + return nil + } + + // Helper function to copy a map + copyMapForNotificationSpec := func(src map[string]string) map[string]string { + if src == nil { + return map[string]string{} + } + dst := make(map[string]string, len(src)) + for k, v := range src { + dst[k] = v + } + return dst + } + + // Try to fetch the NotificationsConfiguration from the Argo CD instance namespace + instanceNotifCfg := &v1alpha1.NotificationsConfiguration{} + var instanceSpec *v1alpha1.NotificationsConfigurationSpec + err := argoutil.FetchObject(r.Client, cr.Namespace, DefaultNotificationsConfigurationInstanceName, instanceNotifCfg) + if err != nil { + if !apierrors.IsNotFound(err) { + return fmt.Errorf("failed to get the NotificationsConfiguration from instance namespace %s : %s", cr.Namespace, err) + } + // Not found in instance namespace, use defaults + instanceSpec = &v1alpha1.NotificationsConfigurationSpec{ + Context: getDefaultNotificationsContext(), + Triggers: getDefaultNotificationsTriggers(), + Templates: getDefaultNotificationsTemplates(), + } + } else { + // Found in instance namespace, use its spec + instanceSpec = &instanceNotifCfg.Spec + } + + // Check if NotificationsConfiguration exists in source namespace + sourceNotifCfg := &v1alpha1.NotificationsConfiguration{} + err = argoutil.FetchObject(r.Client, sourceNamespace, DefaultNotificationsConfigurationInstanceName, sourceNotifCfg) + if err != nil { + if !apierrors.IsNotFound(err) { + return fmt.Errorf("failed to get the NotificationsConfiguration from source namespace %s : %s", sourceNamespace, err) + } + // Not found in source namespace, create it with propagated/default spec + newCfg := &v1alpha1.NotificationsConfiguration{ + ObjectMeta: v1.ObjectMeta{ + Name: DefaultNotificationsConfigurationInstanceName, + Namespace: sourceNamespace, + }, + Spec: v1alpha1.NotificationsConfigurationSpec{ + Context: copyMapForNotificationSpec(instanceSpec.Context), + Triggers: copyMapForNotificationSpec(instanceSpec.Triggers), + Templates: copyMapForNotificationSpec(instanceSpec.Templates), + Services: copyMapForNotificationSpec(instanceSpec.Services), + Subscriptions: copyMapForNotificationSpec(instanceSpec.Subscriptions), + }, + } + argoutil.LogResourceCreation(log, sourceNotifCfg, "propagating NotificationsConfiguration from instance namespace") + return r.Create(context.TODO(), newCfg) + } + + // Already exists in source namespace, update it at leaset to match defaults + updated := false + if sourceNotifCfg.Spec.Context == nil { + sourceNotifCfg.Spec.Context = getDefaultNotificationsContext() + updated = true + } + if sourceNotifCfg.Spec.Triggers == nil { + sourceNotifCfg.Spec.Triggers = getDefaultNotificationsTriggers() + updated = true + } + if sourceNotifCfg.Spec.Templates == nil { + sourceNotifCfg.Spec.Templates = getDefaultNotificationsTemplates() + updated = true + } + if updated { + argoutil.LogResourceUpdate(log, sourceNotifCfg) + return r.Update(context.TODO(), sourceNotifCfg) + } + return nil +} + func (r *ReconcileArgoCD) getNotificationsCommand(cr *argoproj.ArgoCD) []string { cmd := make([]string, 0) @@ -822,6 +911,19 @@ func (r *ReconcileArgoCD) cleanupUnmanagedNotificationsSourceNamespaceResources( } } + // Delete NotificationsConfiguration CR in source namespace + notifCfg := &v1alpha1.NotificationsConfiguration{} + if err := r.Get(context.TODO(), types.NamespacedName{Name: DefaultNotificationsConfigurationInstanceName, Namespace: namespace.Name}, notifCfg); err != nil { + if !apierrors.IsNotFound(err) { + return fmt.Errorf("failed to get the NotificationsConfiguration in namespace %s : %s", namespace.Name, err) + } + } else { + argoutil.LogResourceDeletion(log, notifCfg, "cleaning up unmanaged notifications resources") + if err := r.Delete(context.TODO(), notifCfg); err != nil { + return fmt.Errorf("failed to delete the NotificationsConfiguration in namespace %s : %s", namespace.Name, err) + } + } + // app-in-any-ns code will handle removal of notifications permissions for argocd-server in target namespace // Remove notifications-managed-by-cluster-argocd label from the namespace diff --git a/controllers/argocd/notifications_test.go b/controllers/argocd/notifications_test.go index 1e7387514..be9f2f0da 100644 --- a/controllers/argocd/notifications_test.go +++ b/controllers/argocd/notifications_test.go @@ -771,10 +771,12 @@ func TestNotifications_removeUnmanagedNotificationsSourceNamespaceResources(t *t subresObjs := []client.Object{a} runtimeObjs := []runtime.Object{} sch := makeTestReconcilerScheme(argoproj.AddToScheme) + err := v1alpha1.AddToScheme(sch) + assert.NoError(t, err) cl := makeTestReconcilerClient(sch, resObjs, subresObjs, runtimeObjs) r := makeTestReconciler(cl, sch, testclient.NewSimpleClientset()) - err := createNamespace(r, ns1, "") + err = createNamespace(r, ns1, "") assert.NoError(t, err) err = createNamespace(r, ns2, "") assert.NoError(t, err) @@ -809,6 +811,12 @@ func TestNotifications_removeUnmanagedNotificationsSourceNamespaceResources(t *t assert.Error(t, err) assert.True(t, errors.IsNotFound(err)) + // NotificationsConfiguration CR should be deleted from ns1 + notifCfg := &v1alpha1.NotificationsConfiguration{} + err = r.Get(context.TODO(), client.ObjectKey{Name: DefaultNotificationsConfigurationInstanceName, Namespace: ns1}, notifCfg) + assert.Error(t, err) + assert.True(t, errors.IsNotFound(err)) + // notifications tracking label should be removed namespace := &v1.Namespace{} err = r.Get(context.TODO(), client.ObjectKey{Name: ns1}, namespace) @@ -826,6 +834,11 @@ func TestNotifications_removeUnmanagedNotificationsSourceNamespaceResources(t *t err = r.Get(context.TODO(), client.ObjectKey{Name: resName, Namespace: ns2}, roleBinding) assert.NoError(t, err) + // NotificationsConfiguration CR should still exist in ns2 + notifCfg = &v1alpha1.NotificationsConfiguration{} + err = r.Get(context.TODO(), client.ObjectKey{Name: DefaultNotificationsConfigurationInstanceName, Namespace: ns2}, notifCfg) + assert.NoError(t, err) + namespace = &v1.Namespace{} err = r.Get(context.TODO(), client.ObjectKey{Name: ns2}, namespace) assert.NoError(t, err) @@ -833,3 +846,213 @@ func TestNotifications_removeUnmanagedNotificationsSourceNamespaceResources(t *t assert.True(t, found) assert.Equal(t, a.Namespace, val) } + +func TestReconcileNotifications_CreateNotificationsConfigurationInSourceNamespace_WithDefaults(t *testing.T) { + logf.SetLogger(ZapLogger(true)) + sourceNamespace := "ns1" + a := makeTestArgoCD(func(a *argoproj.ArgoCD) { + a.Spec.Notifications.Enabled = true + a.Spec.Notifications.SourceNamespaces = []string{sourceNamespace} + a.Spec.SourceNamespaces = []string{sourceNamespace} + }) + + resObjs := []client.Object{a} + subresObjs := []client.Object{a} + runtimeObjs := []runtime.Object{} + sch := makeTestReconcilerScheme(argoproj.AddToScheme) + err := v1alpha1.AddToScheme(sch) + assert.NoError(t, err) + cl := makeTestReconcilerClient(sch, resObjs, subresObjs, runtimeObjs) + r := makeTestReconciler(cl, sch, testclient.NewSimpleClientset()) + + // Create the source namespace + err = createNamespace(r, sourceNamespace, "") + assert.NoError(t, err) + + // Reconcile should create NotificationsConfiguration CR in source namespace with defaults + // (since no NotificationsConfiguration exists in instance namespace) + err = r.reconcileSourceNamespaceNotificationsConfigurationCR(a, sourceNamespace) + assert.NoError(t, err) + + // Verify NotificationsConfiguration CR exists in source namespace + notifCfg := &v1alpha1.NotificationsConfiguration{} + err = r.Get(context.TODO(), types.NamespacedName{ + Name: DefaultNotificationsConfigurationInstanceName, + Namespace: sourceNamespace, + }, notifCfg) + assert.NoError(t, err) + + // Verify it has the expected defaults + expectedContext := getDefaultNotificationsContext() + expectedTriggers := getDefaultNotificationsTriggers() + expectedTemplates := getDefaultNotificationsTemplates() + + // Normalize nil to empty map for comparison (Kubernetes may serialize empty maps as nil) + actualContext := notifCfg.Spec.Context + if actualContext == nil { + actualContext = map[string]string{} + } + + assert.Equal(t, expectedContext, actualContext) + assert.Equal(t, expectedTriggers, notifCfg.Spec.Triggers) + assert.Equal(t, expectedTemplates, notifCfg.Spec.Templates) +} + +func TestReconcileNotifications_PropagateNotificationsConfigurationFromInstanceNamespace(t *testing.T) { + logf.SetLogger(ZapLogger(true)) + sourceNamespace := "ns1" + a := makeTestArgoCD(func(a *argoproj.ArgoCD) { + a.Spec.Notifications.Enabled = true + a.Spec.Notifications.SourceNamespaces = []string{sourceNamespace} + a.Spec.SourceNamespaces = []string{sourceNamespace} + }) + + // Create a custom NotificationsConfiguration in the instance namespace + customInstanceCfg := &v1alpha1.NotificationsConfiguration{ + ObjectMeta: metav1.ObjectMeta{ + Name: DefaultNotificationsConfigurationInstanceName, + Namespace: a.Namespace, + }, + Spec: v1alpha1.NotificationsConfigurationSpec{ + Context: map[string]string{ + "customKey": "customValue", + "argocdUrl": "https://custom-argocd.example.com", + }, + Triggers: map[string]string{ + "trigger.custom": "custom trigger definition", + }, + Templates: map[string]string{ + "template.custom": "custom template definition", + }, + Services: map[string]string{ + "service.custom": "custom service definition", + }, + Subscriptions: map[string]string{ + "subscription.custom": "custom subscription definition", + }, + }, + } + + resObjs := []client.Object{a, customInstanceCfg} + subresObjs := []client.Object{a, customInstanceCfg} + runtimeObjs := []runtime.Object{} + sch := makeTestReconcilerScheme(argoproj.AddToScheme) + err := v1alpha1.AddToScheme(sch) + assert.NoError(t, err) + cl := makeTestReconcilerClient(sch, resObjs, subresObjs, runtimeObjs) + r := makeTestReconciler(cl, sch, testclient.NewSimpleClientset()) + + // Create the source namespace + err = createNamespace(r, sourceNamespace, "") + assert.NoError(t, err) + + // Reconcile should propagate the NotificationsConfiguration from instance namespace + err = r.reconcileSourceNamespaceNotificationsConfigurationCR(a, sourceNamespace) + assert.NoError(t, err) + + // Verify NotificationsConfiguration CR exists in source namespace + sourceNotifCfg := &v1alpha1.NotificationsConfiguration{} + err = r.Get(context.TODO(), types.NamespacedName{ + Name: DefaultNotificationsConfigurationInstanceName, + Namespace: sourceNamespace, + }, sourceNotifCfg) + assert.NoError(t, err) + + // Verify it matches the instance namespace configuration (propagated) + assert.Equal(t, customInstanceCfg.Spec.Context, sourceNotifCfg.Spec.Context) + assert.Equal(t, customInstanceCfg.Spec.Triggers, sourceNotifCfg.Spec.Triggers) + assert.Equal(t, customInstanceCfg.Spec.Templates, sourceNotifCfg.Spec.Templates) + assert.Equal(t, customInstanceCfg.Spec.Services, sourceNotifCfg.Spec.Services) + assert.Equal(t, customInstanceCfg.Spec.Subscriptions, sourceNotifCfg.Spec.Subscriptions) + + // Now update the instance namespace configuration and verify it gets propagated + customInstanceCfg.Spec.Context["newKey"] = "newValue" + err = r.Update(context.TODO(), customInstanceCfg) + assert.NoError(t, err) + + // Reconcile again + err = r.reconcileSourceNamespaceNotificationsConfigurationCR(a, sourceNamespace) + assert.NoError(t, err) + + // Verify the source namespace configuration was updated + err = r.Get(context.TODO(), types.NamespacedName{ + Name: DefaultNotificationsConfigurationInstanceName, + Namespace: sourceNamespace, + }, sourceNotifCfg) + assert.NoError(t, err) + // The source namespace config should reflect the updated instance config different from sourceNotifCfg + assert.NotEqual(t, customInstanceCfg.Spec.Context, sourceNotifCfg.Spec.Context) +} + +func TestReconcileNotifications_NotificationsConfigurationInSourceNamespaceWhenDisabled(t *testing.T) { + logf.SetLogger(ZapLogger(true)) + sourceNamespace := "ns1" + a := makeTestArgoCD(func(a *argoproj.ArgoCD) { + a.Spec.Notifications.Enabled = false + a.Spec.Notifications.SourceNamespaces = []string{sourceNamespace} + a.Spec.SourceNamespaces = []string{sourceNamespace} + }) + + resObjs := []client.Object{a} + subresObjs := []client.Object{a} + runtimeObjs := []runtime.Object{} + sch := makeTestReconcilerScheme(argoproj.AddToScheme) + err := v1alpha1.AddToScheme(sch) + assert.NoError(t, err) + cl := makeTestReconcilerClient(sch, resObjs, subresObjs, runtimeObjs) + r := makeTestReconciler(cl, sch, testclient.NewSimpleClientset()) + + // Create the source namespace + err = createNamespace(r, sourceNamespace, "") + assert.NoError(t, err) + + // Reconcile should not create NotificationsConfiguration CR when notifications are disabled + err = r.reconcileSourceNamespaceNotificationsConfigurationCR(a, sourceNamespace) + assert.NoError(t, err) + + // Verify NotificationsConfiguration CR does not exist + notifCfg := &v1alpha1.NotificationsConfiguration{} + err = r.Get(context.TODO(), types.NamespacedName{ + Name: DefaultNotificationsConfigurationInstanceName, + Namespace: sourceNamespace, + }, notifCfg) + assert.Error(t, err) + assert.True(t, errors.IsNotFound(err)) +} + +func TestReconcileNotifications_SourceNamespaceResourcesIncludeNotificationsConfiguration(t *testing.T) { + logf.SetLogger(ZapLogger(true)) + sourceNamespace := "ns1" + a := makeTestArgoCD(func(a *argoproj.ArgoCD) { + a.Spec.Notifications.Enabled = true + a.Spec.Notifications.SourceNamespaces = []string{sourceNamespace} + a.Spec.SourceNamespaces = []string{sourceNamespace} + }) + + resObjs := []client.Object{a} + subresObjs := []client.Object{a} + runtimeObjs := []runtime.Object{} + sch := makeTestReconcilerScheme(argoproj.AddToScheme) + err := v1alpha1.AddToScheme(sch) + assert.NoError(t, err) + cl := makeTestReconcilerClient(sch, resObjs, subresObjs, runtimeObjs) + r := makeTestReconciler(cl, sch, testclient.NewSimpleClientset()) + + // Create the source namespace + err = createNamespace(r, sourceNamespace, "") + assert.NoError(t, err) + + // Reconcile source namespace resources (this should create NotificationsConfiguration CR) + err = r.reconcileNotificationsSourceNamespacesResources(a) + assert.NoError(t, err) + + // Verify NotificationsConfiguration CR was created in source namespace + notifCfg := &v1alpha1.NotificationsConfiguration{} + err = r.Get(context.TODO(), types.NamespacedName{ + Name: DefaultNotificationsConfigurationInstanceName, + Namespace: sourceNamespace, + }, notifCfg) + assert.NoError(t, err) + assert.Equal(t, DefaultNotificationsConfigurationInstanceName, notifCfg.Name) + assert.Equal(t, sourceNamespace, notifCfg.Namespace) +} From 0706ebc2527277a19902f28d77efd260b73c461b Mon Sep 17 00:00:00 2001 From: nmirasch Date: Tue, 4 Nov 2025 11:55:10 +0100 Subject: [PATCH 4/5] Add empty NotificationsConfiguration CR in the source namespaces instead of propagating the configs Signed-off-by: nmirasch --- controllers/argocd/argocd_controller.go | 2 +- controllers/argocd/notifications.go | 66 ++---------------- controllers/argocd/notifications_test.go | 86 ------------------------ 3 files changed, 7 insertions(+), 147 deletions(-) diff --git a/controllers/argocd/argocd_controller.go b/controllers/argocd/argocd_controller.go index fe734f838..40125fa92 100644 --- a/controllers/argocd/argocd_controller.go +++ b/controllers/argocd/argocd_controller.go @@ -79,7 +79,7 @@ type ReconcileArgoCD struct { // Stores a list of ApplicationSetSourceNamespaces as keys ManagedApplicationSetSourceNamespaces map[string]string - // Stores a list of ApplicationSetSourceNamespaces as keys + // Stores a list of NotificationsSourceNamespaces as keys ManagedNotificationsSourceNamespaces map[string]string // Stores label selector used to reconcile a subset of ArgoCD diff --git a/controllers/argocd/notifications.go b/controllers/argocd/notifications.go index 6cbbe08d6..b47c5faf6 100644 --- a/controllers/argocd/notifications.go +++ b/controllers/argocd/notifications.go @@ -570,7 +570,7 @@ func (r *ReconcileArgoCD) reconcileNotificationsSourceNamespacesResources(cr *ar continue } if !contains(appsNamespaces, sourceNamespace) { - log.Error(fmt.Errorf("skipping reconciliation of Notification resources for sourceNamespace %s as Apps in target sourceNamespace is not enabled", sourceNamespace), "Warning") + log.Info(fmt.Sprintf("skipping reconciliation of Notification resources for sourceNamespace %s as Apps in target sourceNamespace is not enabled", sourceNamespace), "Warning") continue } @@ -674,46 +674,14 @@ func (r *ReconcileArgoCD) reconcileNotificationsSourceNamespacesResources(cr *ar } // reconcileSourceNamespaceNotificationsConfigurationCR ensures a NotificationsConfiguration CR exists in the given source namespace. -// It propagates the NotificationsConfiguration from the Argo CD instance namespace if it exists, otherwise uses defaults. func (r *ReconcileArgoCD) reconcileSourceNamespaceNotificationsConfigurationCR(cr *argoproj.ArgoCD, sourceNamespace string) error { if !isNotificationsEnabled(cr) { return nil } - // Helper function to copy a map - copyMapForNotificationSpec := func(src map[string]string) map[string]string { - if src == nil { - return map[string]string{} - } - dst := make(map[string]string, len(src)) - for k, v := range src { - dst[k] = v - } - return dst - } - - // Try to fetch the NotificationsConfiguration from the Argo CD instance namespace - instanceNotifCfg := &v1alpha1.NotificationsConfiguration{} - var instanceSpec *v1alpha1.NotificationsConfigurationSpec - err := argoutil.FetchObject(r.Client, cr.Namespace, DefaultNotificationsConfigurationInstanceName, instanceNotifCfg) - if err != nil { - if !apierrors.IsNotFound(err) { - return fmt.Errorf("failed to get the NotificationsConfiguration from instance namespace %s : %s", cr.Namespace, err) - } - // Not found in instance namespace, use defaults - instanceSpec = &v1alpha1.NotificationsConfigurationSpec{ - Context: getDefaultNotificationsContext(), - Triggers: getDefaultNotificationsTriggers(), - Templates: getDefaultNotificationsTemplates(), - } - } else { - // Found in instance namespace, use its spec - instanceSpec = &instanceNotifCfg.Spec - } - // Check if NotificationsConfiguration exists in source namespace sourceNotifCfg := &v1alpha1.NotificationsConfiguration{} - err = argoutil.FetchObject(r.Client, sourceNamespace, DefaultNotificationsConfigurationInstanceName, sourceNotifCfg) + err := argoutil.FetchObject(r.Client, sourceNamespace, DefaultNotificationsConfigurationInstanceName, sourceNotifCfg) if err != nil { if !apierrors.IsNotFound(err) { return fmt.Errorf("failed to get the NotificationsConfiguration from source namespace %s : %s", sourceNamespace, err) @@ -725,35 +693,15 @@ func (r *ReconcileArgoCD) reconcileSourceNamespaceNotificationsConfigurationCR(c Namespace: sourceNamespace, }, Spec: v1alpha1.NotificationsConfigurationSpec{ - Context: copyMapForNotificationSpec(instanceSpec.Context), - Triggers: copyMapForNotificationSpec(instanceSpec.Triggers), - Templates: copyMapForNotificationSpec(instanceSpec.Templates), - Services: copyMapForNotificationSpec(instanceSpec.Services), - Subscriptions: copyMapForNotificationSpec(instanceSpec.Subscriptions), + Context: getDefaultNotificationsContext(), + Triggers: getDefaultNotificationsTriggers(), + Templates: getDefaultNotificationsTemplates(), }, } - argoutil.LogResourceCreation(log, sourceNotifCfg, "propagating NotificationsConfiguration from instance namespace") + argoutil.LogResourceCreation(log, sourceNotifCfg, "creating NotificationsConfiguration for namespace") return r.Create(context.TODO(), newCfg) } - // Already exists in source namespace, update it at leaset to match defaults - updated := false - if sourceNotifCfg.Spec.Context == nil { - sourceNotifCfg.Spec.Context = getDefaultNotificationsContext() - updated = true - } - if sourceNotifCfg.Spec.Triggers == nil { - sourceNotifCfg.Spec.Triggers = getDefaultNotificationsTriggers() - updated = true - } - if sourceNotifCfg.Spec.Templates == nil { - sourceNotifCfg.Spec.Templates = getDefaultNotificationsTemplates() - updated = true - } - if updated { - argoutil.LogResourceUpdate(log, sourceNotifCfg) - return r.Update(context.TODO(), sourceNotifCfg) - } return nil } @@ -924,8 +872,6 @@ func (r *ReconcileArgoCD) cleanupUnmanagedNotificationsSourceNamespaceResources( } } - // app-in-any-ns code will handle removal of notifications permissions for argocd-server in target namespace - // Remove notifications-managed-by-cluster-argocd label from the namespace argoutil.LogResourceUpdate(log, &namespace, "removing label", common.ArgoCDNotificationsManagedByClusterArgoCDLabel) delete(namespace.Labels, common.ArgoCDNotificationsManagedByClusterArgoCDLabel) diff --git a/controllers/argocd/notifications_test.go b/controllers/argocd/notifications_test.go index be9f2f0da..630f590fd 100644 --- a/controllers/argocd/notifications_test.go +++ b/controllers/argocd/notifications_test.go @@ -898,92 +898,6 @@ func TestReconcileNotifications_CreateNotificationsConfigurationInSourceNamespac assert.Equal(t, expectedTemplates, notifCfg.Spec.Templates) } -func TestReconcileNotifications_PropagateNotificationsConfigurationFromInstanceNamespace(t *testing.T) { - logf.SetLogger(ZapLogger(true)) - sourceNamespace := "ns1" - a := makeTestArgoCD(func(a *argoproj.ArgoCD) { - a.Spec.Notifications.Enabled = true - a.Spec.Notifications.SourceNamespaces = []string{sourceNamespace} - a.Spec.SourceNamespaces = []string{sourceNamespace} - }) - - // Create a custom NotificationsConfiguration in the instance namespace - customInstanceCfg := &v1alpha1.NotificationsConfiguration{ - ObjectMeta: metav1.ObjectMeta{ - Name: DefaultNotificationsConfigurationInstanceName, - Namespace: a.Namespace, - }, - Spec: v1alpha1.NotificationsConfigurationSpec{ - Context: map[string]string{ - "customKey": "customValue", - "argocdUrl": "https://custom-argocd.example.com", - }, - Triggers: map[string]string{ - "trigger.custom": "custom trigger definition", - }, - Templates: map[string]string{ - "template.custom": "custom template definition", - }, - Services: map[string]string{ - "service.custom": "custom service definition", - }, - Subscriptions: map[string]string{ - "subscription.custom": "custom subscription definition", - }, - }, - } - - resObjs := []client.Object{a, customInstanceCfg} - subresObjs := []client.Object{a, customInstanceCfg} - runtimeObjs := []runtime.Object{} - sch := makeTestReconcilerScheme(argoproj.AddToScheme) - err := v1alpha1.AddToScheme(sch) - assert.NoError(t, err) - cl := makeTestReconcilerClient(sch, resObjs, subresObjs, runtimeObjs) - r := makeTestReconciler(cl, sch, testclient.NewSimpleClientset()) - - // Create the source namespace - err = createNamespace(r, sourceNamespace, "") - assert.NoError(t, err) - - // Reconcile should propagate the NotificationsConfiguration from instance namespace - err = r.reconcileSourceNamespaceNotificationsConfigurationCR(a, sourceNamespace) - assert.NoError(t, err) - - // Verify NotificationsConfiguration CR exists in source namespace - sourceNotifCfg := &v1alpha1.NotificationsConfiguration{} - err = r.Get(context.TODO(), types.NamespacedName{ - Name: DefaultNotificationsConfigurationInstanceName, - Namespace: sourceNamespace, - }, sourceNotifCfg) - assert.NoError(t, err) - - // Verify it matches the instance namespace configuration (propagated) - assert.Equal(t, customInstanceCfg.Spec.Context, sourceNotifCfg.Spec.Context) - assert.Equal(t, customInstanceCfg.Spec.Triggers, sourceNotifCfg.Spec.Triggers) - assert.Equal(t, customInstanceCfg.Spec.Templates, sourceNotifCfg.Spec.Templates) - assert.Equal(t, customInstanceCfg.Spec.Services, sourceNotifCfg.Spec.Services) - assert.Equal(t, customInstanceCfg.Spec.Subscriptions, sourceNotifCfg.Spec.Subscriptions) - - // Now update the instance namespace configuration and verify it gets propagated - customInstanceCfg.Spec.Context["newKey"] = "newValue" - err = r.Update(context.TODO(), customInstanceCfg) - assert.NoError(t, err) - - // Reconcile again - err = r.reconcileSourceNamespaceNotificationsConfigurationCR(a, sourceNamespace) - assert.NoError(t, err) - - // Verify the source namespace configuration was updated - err = r.Get(context.TODO(), types.NamespacedName{ - Name: DefaultNotificationsConfigurationInstanceName, - Namespace: sourceNamespace, - }, sourceNotifCfg) - assert.NoError(t, err) - // The source namespace config should reflect the updated instance config different from sourceNotifCfg - assert.NotEqual(t, customInstanceCfg.Spec.Context, sourceNotifCfg.Spec.Context) -} - func TestReconcileNotifications_NotificationsConfigurationInSourceNamespaceWhenDisabled(t *testing.T) { logf.SetLogger(ZapLogger(true)) sourceNamespace := "ns1" From 5fcb4203e56b0bcc47d40e99b697bc226359100c Mon Sep 17 00:00:00 2001 From: nmirasch Date: Tue, 4 Nov 2025 12:59:43 +0100 Subject: [PATCH 5/5] Create NotificationsConfiguration CR in the source namespaces without spec Signed-off-by: nmirasch --- controllers/argocd/notifications.go | 5 --- controllers/argocd/notifications_test.go | 51 ------------------------ 2 files changed, 56 deletions(-) diff --git a/controllers/argocd/notifications.go b/controllers/argocd/notifications.go index b47c5faf6..572daca9c 100644 --- a/controllers/argocd/notifications.go +++ b/controllers/argocd/notifications.go @@ -692,11 +692,6 @@ func (r *ReconcileArgoCD) reconcileSourceNamespaceNotificationsConfigurationCR(c Name: DefaultNotificationsConfigurationInstanceName, Namespace: sourceNamespace, }, - Spec: v1alpha1.NotificationsConfigurationSpec{ - Context: getDefaultNotificationsContext(), - Triggers: getDefaultNotificationsTriggers(), - Templates: getDefaultNotificationsTemplates(), - }, } argoutil.LogResourceCreation(log, sourceNotifCfg, "creating NotificationsConfiguration for namespace") return r.Create(context.TODO(), newCfg) diff --git a/controllers/argocd/notifications_test.go b/controllers/argocd/notifications_test.go index 630f590fd..a61c5c67f 100644 --- a/controllers/argocd/notifications_test.go +++ b/controllers/argocd/notifications_test.go @@ -847,57 +847,6 @@ func TestNotifications_removeUnmanagedNotificationsSourceNamespaceResources(t *t assert.Equal(t, a.Namespace, val) } -func TestReconcileNotifications_CreateNotificationsConfigurationInSourceNamespace_WithDefaults(t *testing.T) { - logf.SetLogger(ZapLogger(true)) - sourceNamespace := "ns1" - a := makeTestArgoCD(func(a *argoproj.ArgoCD) { - a.Spec.Notifications.Enabled = true - a.Spec.Notifications.SourceNamespaces = []string{sourceNamespace} - a.Spec.SourceNamespaces = []string{sourceNamespace} - }) - - resObjs := []client.Object{a} - subresObjs := []client.Object{a} - runtimeObjs := []runtime.Object{} - sch := makeTestReconcilerScheme(argoproj.AddToScheme) - err := v1alpha1.AddToScheme(sch) - assert.NoError(t, err) - cl := makeTestReconcilerClient(sch, resObjs, subresObjs, runtimeObjs) - r := makeTestReconciler(cl, sch, testclient.NewSimpleClientset()) - - // Create the source namespace - err = createNamespace(r, sourceNamespace, "") - assert.NoError(t, err) - - // Reconcile should create NotificationsConfiguration CR in source namespace with defaults - // (since no NotificationsConfiguration exists in instance namespace) - err = r.reconcileSourceNamespaceNotificationsConfigurationCR(a, sourceNamespace) - assert.NoError(t, err) - - // Verify NotificationsConfiguration CR exists in source namespace - notifCfg := &v1alpha1.NotificationsConfiguration{} - err = r.Get(context.TODO(), types.NamespacedName{ - Name: DefaultNotificationsConfigurationInstanceName, - Namespace: sourceNamespace, - }, notifCfg) - assert.NoError(t, err) - - // Verify it has the expected defaults - expectedContext := getDefaultNotificationsContext() - expectedTriggers := getDefaultNotificationsTriggers() - expectedTemplates := getDefaultNotificationsTemplates() - - // Normalize nil to empty map for comparison (Kubernetes may serialize empty maps as nil) - actualContext := notifCfg.Spec.Context - if actualContext == nil { - actualContext = map[string]string{} - } - - assert.Equal(t, expectedContext, actualContext) - assert.Equal(t, expectedTriggers, notifCfg.Spec.Triggers) - assert.Equal(t, expectedTemplates, notifCfg.Spec.Templates) -} - func TestReconcileNotifications_NotificationsConfigurationInSourceNamespaceWhenDisabled(t *testing.T) { logf.SetLogger(ZapLogger(true)) sourceNamespace := "ns1"