Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## master #106 +/- ##
==========================================
- Coverage 53.53% 52.13% -1.40%
==========================================
Files 34 35 +1
Lines 3026 3113 +87
==========================================
+ Hits 1620 1623 +3
- Misses 1166 1249 +83
- Partials 240 241 +1
☔ View full report in Codecov by Sentry. |
|
@edwargix could you please solve merge conflicts? |
Sure thing; just rebased: matrix.pr-0..matrix.pr-1 |
|
looks like #128 was just merged; will rebase again... |
|
rebased: matrix.pr-1..matrix.pr-2 |
|
moved indirect dependencies to correct location: matrix.pr-2..matrix.pr-3 |
|
Thanks @edwargix , going to review this one |
| if len(parts) > 1 { | ||
| dest.Recipient = parts[1] | ||
| var before, after string | ||
| if i := strings.Index(recipient, ":"); i >= 0 { |
There was a problem hiding this comment.
Could you please comment why you need change this ?
There was a problem hiding this comment.
This PR supports sending Matrix messages to Matrix rooms and room aliases.
A Matrix room is identified with the syntax !opaque_id:domain.tld [0] and a room alias with #room_alias:domain.tld [1].
So the code change here in pkg/api/config.go is to make it so that the domain.tld part isn't lost. Here's an example config; notice that the recipients have two :s in them:
notifications:
notifiers:
service.matrix: |
accessToken: $matrix-token
deviceID: $matrix-deviceid
userID: $matrix-userid
subscriptions:
- recipients:
- 'matrix:#my-room-alias:example.org'
- 'matrix:!my-room-id:example.org'
triggers:
- on-deployed
- on-health-degraded
- on-sync-failed
- on-sync-running
- on-sync-status-unknown
- on-sync-succeededWithout this PR's change to pkg/api/config.go, we'd lose the example.org from the recipients and our Destinations for the Send function would be #my-room-alias and !my-room-id (which we can't send notifications to).
There was a problem hiding this comment.
What I'm hoping is that once this project upgrades to golang 1.18, we can revert this PR's 2nd commit to simplify the code a bit.
dacdf61 to
8e7fced
Compare
|
rebased: matrix.pr-4..matrix.pr-5 |
|
rebased: matrix.pr-5..matrix.pr-6 |
|
rebased: matrix.pr-6..matrix.pr-7 |
|
Will take a look today a bit later |
|
rebased: matrix.pr-7..matrix.pr-8 |
|
rebased: matrix.pr-8..matrix.pr-9 |
|
@pasha-codefresh let me know if there's anything I can do to help |
|
rebased: matrix.pr-9..matrix.pr-10 |
|
rebased: matrix.pr-10..matrix.pr-11 |
|
rebased: matrix.pr-11..matrix.pr-12 |
|
rebased: matrix.pr-12..matrix.pr-13 |
Matrix is an open, federated messaging platform: https://matrix.org ArgoCD Notifications would benefit greatly from Matrix support because a growing number of communities use it daily. Additionally, Matrix has interoperability with many other messaging platforms via bridges, which would give ArgoCD indirect support for notifications to those platforms: - Rocket.Chat https://matrix.org/blog/2022/05/30/welcoming-rocket-chat-to-matrix - Gitter https://matrix.org/blog/2020/12/07/gitter-now-speaks-matrix - IRC https://matrix.org/bridges/#irc A full list of bridges can be found at https://matrix.org/bridges Signed-off-by: David Florness <david@florness.com>
Looks like that function was added in go 1.18 but this project is using go 1.17 https://pkg.go.dev/strings@go1.18#Cut Signed-off-by: David Florness <david@florness.com>
|
rebased: matrix.pr-13..matrix.pr-14 |
Matrix is an open, federated messaging platform: https://matrix.org
ArgoCD Notifications would benefit greatly from Matrix support because a growing number of communities use it daily. Additionally, Matrix has interoperability with many other messaging platforms via bridges, which would give ArgoCD indirect support for notifications to those platforms:
A full list of bridges can be found at https://matrix.org/bridges
Signed-off-by: David Florness david@florness.com