-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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-controller): Set cert resolver to use injected "argocd-tls-certs-cm"(#15392) #15394
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #15394 +/- ##
=======================================
Coverage 49.43% 49.43%
=======================================
Files 269 269
Lines 46790 46793 +3
=======================================
+ Hits 23131 23134 +3
Misses 21385 21385
Partials 2274 2274
☔ View full report in Codecov by Sentry. |
091eb73
to
ed5146e
Compare
Signed-off-by: Siddhesh Ghadi <sghadi1203@gmail.com>
c48ab45
to
59dcc3c
Compare
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!
Signed-off-by: Ishita Sequeira <46771830+ishitasequeira@users.noreply.github.com>
Thanks @ishitasequeira. Will this fix be backported to 2.9 & 2.8? |
/cherry-pick release-2.9 |
/cherry-pick release-2.8 |
Signed-off-by: Siddhesh Ghadi <sghadi1203@gmail.com>
Signed-off-by: Siddhesh Ghadi <sghadi1203@gmail.com>
@svghadi , the change has been cherry-picked in 2.8 and 2.9 |
Signed-off-by: Siddhesh Ghadi <sghadi1203@gmail.com> Signed-off-by: jmilic1 <70441727+jmilic1@users.noreply.github.com>
Signed-off-by: Siddhesh Ghadi <sghadi1203@gmail.com>
Signed-off-by: Siddhesh Ghadi <sghadi1203@gmail.com>
Signed-off-by: Siddhesh Ghadi <sghadi1203@gmail.com> Signed-off-by: Kevin Lyda <kevin@lyda.ie>
Signed-off-by: Siddhesh Ghadi <sghadi1203@gmail.com>
Fixes #15392
notifications-engine
includes functionality to use custom certs for notification services. However, in currentnotifications-controller
code we are missing the initialization of cert resolver function.Previously, when notification-controller was in a separate repo, the initialization of cert resolver was happening here.
This PR adds the missing initialization of cert resolver function in notifications-controller so that controller trusts custom certs for notification services.
I could not come up with a unit/e2e test because of dependency on external notification service with custom cert, but I validated the change local following the steps from linked issue and it worked as expected.
Checklist: