-
Notifications
You must be signed in to change notification settings - Fork 137
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
Add Telegram alerting provider #232
Conversation
@somtochiama please rebase with main branch and solve the conflicts for CI to run. |
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.
What is the rationale behind using shoutrrr
for Telegram only, instead of e.g. using a Telegram API library, or using it for other providers as well?
I think we should look into switching to |
0992874
to
ce3bce5
Compare
internal/notifier/telegram.go
Outdated
// The telegram API requires that some special characters are escaped | ||
// in the message string. Docs: https://core.telegram.org/bots/api#formatting-options. | ||
// The function only handles a subset of the characters because the only special | ||
// characters in kubernetes names and namespaces are '-' and '.'. |
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.
But event.Message
can contain any character, for example a kubectl apply error.
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.
I will update the function
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.
Please test this with a Kustomization that fails to apply. I see that we don't post the revision but we should, check how alerts look for Slack: https://fluxcd.io/img/slack-error-alert.png
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.
We have to add the event metadata to the message body as separate field in the markdown, maybe a list?
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.
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
Thanks @somtochiama please squash your commits
Signed-off-by: Somtochi Onyekwere <[email protected]>
8bbacc2
to
b0563cb
Compare
Seems that's not working for {"level":"info","ts":"2021-09-22T12:19:25.795Z","logger":"event-server","msg":"Dispatching event: Helm upgrade succeeded","reconciler kind":"HelmRelease","name":"ingress-nginx","namespace":"flux-system"}
{"level":"error","ts":"2021-09-22T12:19:25.816Z","logger":"event-server","msg":"failed to send notification","reconciler kind":"HelmRelease","name":"ingress-nginx","namespace":"flux-system","error":"failed to send notification to \"-****\", response status code 400 Bad Request"} |
Closes #222