Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(notifications): Allow notifications controller to notify on all namespaces #15702

Merged
Merged
Show file tree
Hide file tree
Changes from 19 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
c7d3e33
Allow notifications controller to notify on all namespaces
Niksko Apr 24, 2023
91446f9
Merge branch 'master' into notifications-in-all-namespaces
Niksko Apr 25, 2023
0398dfc
Add SEEK to users.md
Niksko Apr 25, 2023
ca54f25
Merge branch 'notifications-in-all-namespaces' of github.com:Niksko/a…
Niksko Apr 25, 2023
6f5979e
Remove unused fields
Niksko Apr 26, 2023
36467a6
Merge branch 'master' into notifications-in-all-namespaces
Niksko Apr 26, 2023
da6be0e
Merge branch 'master' into notifications-in-all-namespaces
Niksko Apr 27, 2023
c3c1603
Revert changes to Procfile
Niksko Apr 28, 2023
ee4ab23
Fix unit tests
Niksko Apr 28, 2023
6f7ecf4
Merge branch 'master' into notifications-in-all-namespaces
Niksko May 16, 2023
863d6d4
- add argocd namespaces environment variable to notifications controller
sthomson-wyn Aug 25, 2023
b36ed87
- add example cluster role rbac
sthomson-wyn Aug 25, 2023
e4d963e
- only look for projects in the controller's namespace (argocd by def…
sthomson-wyn Aug 25, 2023
46b0404
Merge remote-tracking branch 'origin/master' into notifications-in-al…
sthomson-wyn Aug 25, 2023
362a89c
Merge remote-tracking branch 'origin/master' into notifications-in-al…
sthomson-wyn Aug 25, 2023
4996eea
- update base manifest
sthomson-wyn Aug 25, 2023
d0c25c8
Merge branch 'master' into notifications-in-all-namespaces
sthomson-wyn Sep 11, 2023
baa28c8
- skip app processing in notification controller
sthomson-wyn Sep 11, 2023
6ede79b
added unit test and updated doc
mayzhang2000 Sep 27, 2023
de662b3
added unit test and updated doc
mayzhang2000 Sep 27, 2023
4de11d5
updated examples/k8s-rbac/argocd-server-applications/kustomization.ya…
mayzhang2000 Oct 4, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion cmd/argocd-notification/commands/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ func NewCommand() *cobra.Command {
argocdRepoServerStrictTLS bool
configMapName string
secretName string
applicationNamespaces []string
)
var command = cobra.Command{
Use: "controller",
Expand Down Expand Up @@ -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)
Expand All @@ -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
}
1 change: 1 addition & 0 deletions docs/operator-manual/app-any-namespace.md
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ We supply a `ClusterRole` and `ClusterRoleBinding` suitable for this purpose in
kubectl apply -f 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.
mayzhang2000 marked this conversation as resolved.
Show resolved Hide resolved
mayzhang2000 marked this conversation as resolved.
Show resolved Hide resolved
!!! note
At some later point in time, we may make this cluster role part of the default installation manifests.

Expand Down
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
6 changes: 6 additions & 0 deletions manifests/ha/install.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -20329,6 +20329,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:
Expand Down
6 changes: 6 additions & 0 deletions manifests/ha/namespace-install.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -1829,6 +1829,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:
Expand Down
6 changes: 6 additions & 0 deletions manifests/install.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -19430,6 +19430,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:
Expand Down
6 changes: 6 additions & 0 deletions manifests/namespace-install.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -930,6 +930,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:
Expand Down
42 changes: 35 additions & 7 deletions notification_controller/controller/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
Expand All @@ -77,13 +80,21 @@ 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),
controller.WithAlterDestinations(res.alterDestinations))
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 {
Expand All @@ -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
}
Expand Down
26 changes: 26 additions & 0 deletions notification_controller/controller/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,7 @@ func TestInit(t *testing.T) {
dynamicClient,
nil,
"default",
[]string{},
appLabelSelector,
nil,
"my-secret",
Expand Down Expand Up @@ -146,6 +147,7 @@ func TestInitTimeout(t *testing.T) {
dynamicClient,
nil,
"default",
[]string{},
appLabelSelector,
nil,
"my-secret",
Expand All @@ -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))
}
Loading