-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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 (cherry-pick 2.8) #15855
fix(notifications): Allow notifications controller to notify on all namespaces (cherry-pick 2.8) #15855
Conversation
…amespaces (argoproj#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 <nskoufis@seek.com.au> * Add SEEK to users.md Signed-off-by: Nikolas Skoufis <nskoufis@seek.com.au> * Remove unused fields Signed-off-by: Nikolas Skoufis <nskoufis@seek.com.au> * Revert changes to Procfile Signed-off-by: Nik Skoufis <n.skoufis@gmail.com> * Fix unit tests Signed-off-by: Nikolas Skoufis <nskoufis@seek.com.au> * - add argocd namespaces environment variable to notifications controller Signed-off-by: Stewart Thomson <sthomson@wynshop.com> * - add example cluster role rbac Signed-off-by: Stewart Thomson <sthomson@wynshop.com> * - only look for projects in the controller's namespace (argocd by default) Signed-off-by: Stewart Thomson <sthomson@wynshop.com> * - update base manifest Signed-off-by: Stewart Thomson <sthomson@wynshop.com> * - skip app processing in notification controller Signed-off-by: Stewart Thomson <sthomson@wynshop.com> * added unit test and updated doc Signed-off-by: May Zhang <may_zhang@intuit.com> * added unit test and updated doc Signed-off-by: May Zhang <may_zhang@intuit.com> * updated examples/k8s-rbac/argocd-server-applications/kustomization.yaml's resources Signed-off-by: May Zhang <may_zhang@intuit.com> --------- Signed-off-by: Nikolas Skoufis <nskoufis@seek.com.au> Signed-off-by: Nik Skoufis <n.skoufis@gmail.com> Signed-off-by: Stewart Thomson <sthomson@wynshop.com> Signed-off-by: May Zhang <may_zhang@intuit.com> Co-authored-by: Nikolas Skoufis <nskoufis@seek.com.au> Co-authored-by: Nik Skoufis <n.skoufis@gmail.com> Co-authored-by: Stewart Thomson <sthomson@wynshop.com> undo unnecessary manifest changes Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com> undo unnecessary manifest changes Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
8f09cca
to
5d5a8b8
Compare
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## release-2.8 #15855 +/- ##
===============================================
- Coverage 50.03% 50.02% -0.01%
===============================================
Files 263 263
Lines 45108 45127 +19
===============================================
+ Hits 22569 22575 +6
- Misses 20331 20342 +11
- Partials 2208 2210 +2
☔ View full report in Codecov by Sentry. |
subjects: | ||
- kind: ServiceAccount | ||
name: argocd-notifications-controller | ||
namespace: argocd |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@@ -1080,7 +1080,13 @@ spec: | |||
args: | |||
- /readonly/haproxy_init.sh | |||
securityContext: | |||
null | |||
allowPrivilegeEscalation: false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not sure where these are from
…fix-2.8 Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
options.LabelSelector = selector | ||
return resClient.List(context.Background(), options) | ||
appList, err := resClient.List(context.TODO(), options) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is context.TODO() expected?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's equivalent to Background. iirc no functional difference
Checklist: