-
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
feat: Allow connecting from the Notification Controller to the Repo Server without TLS (#19629) #19630
feat: Allow connecting from the Notification Controller to the Repo Server without TLS (#19629) #19630
Conversation
❌ Preview Environment deleted from BunnyshellAvailable commands (reply to this comment):
|
❌ Preview Environment deleted from BunnyshellAvailable commands (reply to this comment):
|
5fd8841
to
c631a68
Compare
Tested this on my test cluster. After deploying this version (replacing the cluster-install line of my kustomize resources with
And then after adding |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #19630 +/- ##
=========================================
Coverage ? 55.90%
=========================================
Files ? 316
Lines ? 43790
Branches ? 0
=========================================
Hits ? 24482
Misses ? 16759
Partials ? 2549 ☔ View full report in Codecov by Sentry. |
Signed-off-by: Matthew Wynn <[email protected]>
c631a68
to
3462928
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, thank you for your contribution
#19630) Signed-off-by: Matthew Wynn <[email protected]>
Unsure if this should be a fix or a feature, so I created an enhancement proposal #19629.
All of the Argo CD components I have worked with have a way to disable TLS between each other via the argocd-cmd-params-cm ConfigMap. One issue I've run into is, when calling .repo.GetCommitMetadata in notifications templates, it tries to reach out to the Repo Server over TLS. There does not seem to be an easy way to disable it, short of overlaying the notifications template deployment itself and adding a command line flag.
This is necessary for environments where Istio is managing mutual TLS between each component (#2784).
Closes #19629
Checklist: