From 2bd4f787251866e44babd156e5b0d7ba21412b91 Mon Sep 17 00:00:00 2001 From: May Zhang Date: Fri, 6 Oct 2023 13:25:38 -0700 Subject: [PATCH] fix(notifications): Allow notifications controller to notify on all namespaces (#15702) * Allow notifications controller to notify on all namespaces This adds functionality to the notifications controller to be notified of and send notifications for applications in any namespace. The namespaces to watch are controlled by the same --application-namespaces and ARGOCD_APPLICATION_NAMESPACES variables as in the application controller. Signed-off-by: Nikolas Skoufis * Add SEEK to users.md Signed-off-by: Nikolas Skoufis * Remove unused fields Signed-off-by: Nikolas Skoufis * Revert changes to Procfile Signed-off-by: Nik Skoufis * Fix unit tests Signed-off-by: Nikolas Skoufis * - add argocd namespaces environment variable to notifications controller Signed-off-by: Stewart Thomson * - add example cluster role rbac Signed-off-by: Stewart Thomson * - only look for projects in the controller's namespace (argocd by default) Signed-off-by: Stewart Thomson * - update base manifest Signed-off-by: Stewart Thomson * - skip app processing in notification controller Signed-off-by: Stewart Thomson * added unit test and updated doc Signed-off-by: May Zhang * added unit test and updated doc Signed-off-by: May Zhang * updated examples/k8s-rbac/argocd-server-applications/kustomization.yaml's resources Signed-off-by: May Zhang --------- Signed-off-by: Nikolas Skoufis Signed-off-by: Nik Skoufis Signed-off-by: Stewart Thomson Signed-off-by: May Zhang Co-authored-by: Nikolas Skoufis Co-authored-by: Nik Skoufis Co-authored-by: Stewart Thomson --- .../commands/controller.go | 4 +- docs/operator-manual/app-any-namespace.md | 2 + ...fications-controller-rbac-clusterrole.yaml | 19 +++++++++ ...ns-controller-rbac-clusterrolebinding.yaml | 16 +++++++ .../kustomization.yaml | 2 + ...d-notifications-controller-deployment.yaml | 6 +++ manifests/ha/install.yaml | 6 +++ manifests/ha/namespace-install.yaml | 6 +++ manifests/install.yaml | 6 +++ manifests/namespace-install.yaml | 6 +++ .../controller/controller.go | 42 +++++++++++++++---- .../controller/controller_test.go | 26 ++++++++++++ 12 files changed, 133 insertions(+), 8 deletions(-) create mode 100644 examples/k8s-rbac/argocd-server-applications/argocd-notifications-controller-rbac-clusterrole.yaml create mode 100644 examples/k8s-rbac/argocd-server-applications/argocd-notifications-controller-rbac-clusterrolebinding.yaml diff --git a/cmd/argocd-notification/commands/controller.go b/cmd/argocd-notification/commands/controller.go index 3bcd721ef4b89..abd9a2e8475f0 100644 --- a/cmd/argocd-notification/commands/controller.go +++ b/cmd/argocd-notification/commands/controller.go @@ -55,6 +55,7 @@ func NewCommand() *cobra.Command { argocdRepoServerStrictTLS bool configMapName string secretName string + applicationNamespaces []string ) var command = cobra.Command{ Use: "controller", @@ -138,7 +139,7 @@ func NewCommand() *cobra.Command { log.Infof("serving metrics on port %d", metricsPort) log.Infof("loading configuration %d", metricsPort) - ctrl := notificationscontroller.NewController(k8sClient, dynamicClient, argocdService, namespace, appLabelSelector, registry, secretName, configMapName) + ctrl := notificationscontroller.NewController(k8sClient, dynamicClient, argocdService, namespace, applicationNamespaces, appLabelSelector, registry, secretName, configMapName) err = ctrl.Init(ctx) if err != nil { return fmt.Errorf("failed to initialize controller: %w", err) @@ -161,5 +162,6 @@ func NewCommand() *cobra.Command { command.Flags().BoolVar(&argocdRepoServerStrictTLS, "argocd-repo-server-strict-tls", false, "Perform strict validation of TLS certificates when connecting to repo server") command.Flags().StringVar(&configMapName, "config-map-name", "argocd-notifications-cm", "Set notifications ConfigMap name") command.Flags().StringVar(&secretName, "secret-name", "argocd-notifications-secret", "Set notifications Secret name") + command.Flags().StringSliceVar(&applicationNamespaces, "application-namespaces", env.StringsFromEnv("ARGOCD_APPLICATION_NAMESPACES", []string{}, ","), "List of additional namespaces that this controller should send notifications for") return &command } diff --git a/docs/operator-manual/app-any-namespace.md b/docs/operator-manual/app-any-namespace.md index e4efb1669193e..21743b7bc003d 100644 --- a/docs/operator-manual/app-any-namespace.md +++ b/docs/operator-manual/app-any-namespace.md @@ -71,6 +71,8 @@ We supply a `ClusterRole` and `ClusterRoleBinding` suitable for this purpose in kubectl apply -k examples/k8s-rbac/argocd-server-applications/ ``` +`argocd-notifications-controller-rbac-clusterrole.yaml` and `argocd-notifications-controller-rbac-clusterrolebinding.yaml` are used to support notifications controller to notify apps in all namespaces. + !!! note At some later point in time, we may make this cluster role part of the default installation manifests. diff --git a/examples/k8s-rbac/argocd-server-applications/argocd-notifications-controller-rbac-clusterrole.yaml b/examples/k8s-rbac/argocd-server-applications/argocd-notifications-controller-rbac-clusterrole.yaml new file mode 100644 index 0000000000000..05f92abb11717 --- /dev/null +++ b/examples/k8s-rbac/argocd-server-applications/argocd-notifications-controller-rbac-clusterrole.yaml @@ -0,0 +1,19 @@ +apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRole +metadata: + labels: + app.kubernetes.io/name: argocd-notifications-controller-cluster-apps + app.kubernetes.io/part-of: argocd + app.kubernetes.io/component: notifications-controller + name: argocd-notifications-controller-cluster-apps +rules: +- apiGroups: + - "argoproj.io" + resources: + - "applications" + verbs: + - get + - list + - watch + - update + - patch \ No newline at end of file diff --git a/examples/k8s-rbac/argocd-server-applications/argocd-notifications-controller-rbac-clusterrolebinding.yaml b/examples/k8s-rbac/argocd-server-applications/argocd-notifications-controller-rbac-clusterrolebinding.yaml new file mode 100644 index 0000000000000..c28ab688c0716 --- /dev/null +++ b/examples/k8s-rbac/argocd-server-applications/argocd-notifications-controller-rbac-clusterrolebinding.yaml @@ -0,0 +1,16 @@ +apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRoleBinding +metadata: + labels: + app.kubernetes.io/name: argocd-notifications-controller-cluster-apps + app.kubernetes.io/part-of: argocd + app.kubernetes.io/component: notifications-controller + name: argocd-notifications-controller-cluster-apps +roleRef: + apiGroup: rbac.authorization.k8s.io + kind: ClusterRole + name: argocd-notifications-controller-cluster-apps +subjects: +- kind: ServiceAccount + name: argocd-notifications-controller + namespace: argocd diff --git a/examples/k8s-rbac/argocd-server-applications/kustomization.yaml b/examples/k8s-rbac/argocd-server-applications/kustomization.yaml index 67d8d57d3f72f..330dde4a914d2 100644 --- a/examples/k8s-rbac/argocd-server-applications/kustomization.yaml +++ b/examples/k8s-rbac/argocd-server-applications/kustomization.yaml @@ -3,3 +3,5 @@ kind: Kustomization resources: - argocd-server-rbac-clusterrole.yaml - argocd-server-rbac-clusterrolebinding.yaml +- argocd-notifications-controller-rbac-clusterrole.yaml +- argocd-notifications-controller-rbac-clusterrolebinding.yaml diff --git a/manifests/base/notification/argocd-notifications-controller-deployment.yaml b/manifests/base/notification/argocd-notifications-controller-deployment.yaml index 8eab1f95570c5..9cd1a068808b1 100644 --- a/manifests/base/notification/argocd-notifications-controller-deployment.yaml +++ b/manifests/base/notification/argocd-notifications-controller-deployment.yaml @@ -48,6 +48,12 @@ spec: key: notificationscontroller.log.level name: argocd-cmd-params-cm optional: true + - name: ARGOCD_APPLICATION_NAMESPACES + valueFrom: + configMapKeyRef: + key: application.namespaces + name: argocd-cmd-params-cm + optional: true workingDir: /app livenessProbe: tcpSocket: diff --git a/manifests/ha/install.yaml b/manifests/ha/install.yaml index 0051211e1b4bb..a2e1e5b91b668 100644 --- a/manifests/ha/install.yaml +++ b/manifests/ha/install.yaml @@ -22197,6 +22197,12 @@ spec: key: notificationscontroller.log.level name: argocd-cmd-params-cm optional: true + - name: ARGOCD_APPLICATION_NAMESPACES + valueFrom: + configMapKeyRef: + key: application.namespaces + name: argocd-cmd-params-cm + optional: true image: quay.io/argoproj/argocd:latest imagePullPolicy: Always livenessProbe: diff --git a/manifests/ha/namespace-install.yaml b/manifests/ha/namespace-install.yaml index b1f5f0d68a815..ea123ef2e50ef 100644 --- a/manifests/ha/namespace-install.yaml +++ b/manifests/ha/namespace-install.yaml @@ -1853,6 +1853,12 @@ spec: key: notificationscontroller.log.level name: argocd-cmd-params-cm optional: true + - name: ARGOCD_APPLICATION_NAMESPACES + valueFrom: + configMapKeyRef: + key: application.namespaces + name: argocd-cmd-params-cm + optional: true image: quay.io/argoproj/argocd:latest imagePullPolicy: Always livenessProbe: diff --git a/manifests/install.yaml b/manifests/install.yaml index 69ea78fa5c058..6783484ed0150 100644 --- a/manifests/install.yaml +++ b/manifests/install.yaml @@ -21292,6 +21292,12 @@ spec: key: notificationscontroller.log.level name: argocd-cmd-params-cm optional: true + - name: ARGOCD_APPLICATION_NAMESPACES + valueFrom: + configMapKeyRef: + key: application.namespaces + name: argocd-cmd-params-cm + optional: true image: quay.io/argoproj/argocd:latest imagePullPolicy: Always livenessProbe: diff --git a/manifests/namespace-install.yaml b/manifests/namespace-install.yaml index e19878573725d..0490434f4046c 100644 --- a/manifests/namespace-install.yaml +++ b/manifests/namespace-install.yaml @@ -948,6 +948,12 @@ spec: key: notificationscontroller.log.level name: argocd-cmd-params-cm optional: true + - name: ARGOCD_APPLICATION_NAMESPACES + valueFrom: + configMapKeyRef: + key: application.namespaces + name: argocd-cmd-params-cm + optional: true image: quay.io/argoproj/argocd:latest imagePullPolicy: Always livenessProbe: diff --git a/notification_controller/controller/controller.go b/notification_controller/controller/controller.go index e975b5c2ded03..1ad2ab361ab93 100644 --- a/notification_controller/controller/controller.go +++ b/notification_controller/controller/controller.go @@ -6,6 +6,8 @@ import ( "fmt" "time" + "github.com/argoproj/argo-cd/v2/util/glob" + "github.com/argoproj/argo-cd/v2/util/notification/k8s" service "github.com/argoproj/argo-cd/v2/util/notification/argocd" @@ -53,14 +55,15 @@ func NewController( client dynamic.Interface, argocdService service.Service, namespace string, + applicationNamespaces []string, appLabelSelector string, registry *controller.MetricsRegistry, secretName string, configMapName string, ) *notificationController { appClient := client.Resource(applications) - appInformer := newInformer(appClient.Namespace(namespace), appLabelSelector) - appProjInformer := newInformer(newAppProjClient(client, namespace), "") + appInformer := newInformer(appClient, namespace, applicationNamespaces, appLabelSelector) + appProjInformer := newInformer(newAppProjClient(client, namespace), namespace, []string{namespace}, "") secretInformer := k8s.NewSecretInformer(k8sClient, namespace, secretName) configMapInformer := k8s.NewConfigMapInformer(k8sClient, namespace, configMapName) apiFactory := api.NewFactory(settings.GetFactorySettings(argocdService, secretName, configMapName), namespace, secretInformer, configMapInformer) @@ -77,6 +80,9 @@ func NewController( if !ok { return false, "" } + if checkAppNotInAdditionalNamespaces(app, namespace, applicationNamespaces) { + return true, "app is not in one of the application-namespaces, nor the notification controller namespace" + } return !isAppSyncStatusRefreshed(app, log.WithField("app", obj.GetName())), "sync status out of date" }), controller.WithMetricsRegistry(registry), @@ -84,6 +90,11 @@ func NewController( return res } +// Check if app is not in the namespace where the controller is in, and also app is not in one of the applicationNamespaces +func checkAppNotInAdditionalNamespaces(app *unstructured.Unstructured, namespace string, applicationNamespaces []string) bool { + return namespace != app.GetNamespace() && !glob.MatchStringInList(applicationNamespaces, app.GetNamespace(), false) +} + func (c *notificationController) alterDestinations(obj v1.Object, destinations services.Destinations, cfg api.Config) services.Destinations { app, ok := (obj).(*unstructured.Unstructured) if !ok { @@ -97,21 +108,38 @@ func (c *notificationController) alterDestinations(obj v1.Object, destinations s return destinations } -func newInformer(resClient dynamic.ResourceInterface, selector string) cache.SharedIndexInformer { +func newInformer(resClient dynamic.ResourceInterface, controllerNamespace string, applicationNamespaces []string, selector string) cache.SharedIndexInformer { informer := cache.NewSharedIndexInformer( &cache.ListWatch{ - ListFunc: func(options v1.ListOptions) (object runtime.Object, err error) { + ListFunc: func(options v1.ListOptions) (runtime.Object, error) { + // We are only interested in apps that exist in namespaces the + // user wants to be enabled. options.LabelSelector = selector - return resClient.List(context.Background(), options) + appList, err := resClient.List(context.TODO(), options) + if err != nil { + return nil, fmt.Errorf("failed to list applications: %w", err) + } + newItems := []unstructured.Unstructured{} + for _, res := range appList.Items { + if controllerNamespace == res.GetNamespace() || glob.MatchStringInList(applicationNamespaces, res.GetNamespace(), false) { + newItems = append(newItems, res) + } + } + appList.Items = newItems + return appList, nil }, WatchFunc: func(options v1.ListOptions) (watch.Interface, error) { options.LabelSelector = selector - return resClient.Watch(context.Background(), options) + return resClient.Watch(context.TODO(), options) }, }, &unstructured.Unstructured{}, resyncPeriod, - cache.Indexers{}, + cache.Indexers{ + cache.NamespaceIndex: func(obj interface{}) ([]string, error) { + return cache.MetaNamespaceIndexFunc(obj) + }, + }, ) return informer } diff --git a/notification_controller/controller/controller_test.go b/notification_controller/controller/controller_test.go index fdbd683722279..5ad1e520502a3 100644 --- a/notification_controller/controller/controller_test.go +++ b/notification_controller/controller/controller_test.go @@ -115,6 +115,7 @@ func TestInit(t *testing.T) { dynamicClient, nil, "default", + []string{}, appLabelSelector, nil, "my-secret", @@ -146,6 +147,7 @@ func TestInitTimeout(t *testing.T) { dynamicClient, nil, "default", + []string{}, appLabelSelector, nil, "my-secret", @@ -164,3 +166,27 @@ func TestInitTimeout(t *testing.T) { assert.Error(t, err) assert.Equal(t, "Timed out waiting for caches to sync", err.Error()) } + +func TestCheckAppNotInAdditionalNamespaces(t *testing.T) { + app := &unstructured.Unstructured{ + Object: map[string]interface{}{ + "spec": map[string]interface{}{}, + }, + } + namespace := "argocd" + var applicationNamespaces []string + applicationNamespaces = append(applicationNamespaces, "namespace1") + applicationNamespaces = append(applicationNamespaces, "namespace2") + + // app is in same namespace as controller's namespace + app.SetNamespace(namespace) + assert.False(t, checkAppNotInAdditionalNamespaces(app, namespace, applicationNamespaces)) + + // app is not in the namespace as controller's namespace, but it is in one of the applicationNamespaces + app.SetNamespace("namespace2") + assert.False(t, checkAppNotInAdditionalNamespaces(app, "", applicationNamespaces)) + + // app is not in the namespace as controller's namespace, and it is not in any of the applicationNamespaces + app.SetNamespace("namespace3") + assert.True(t, checkAppNotInAdditionalNamespaces(app, "", applicationNamespaces)) +}