Add Telegram Receiver type to AlertmanagerConfig CRD #4726

angelbarrera92 · 2022-04-18T12:07:13Z

Description

Add Telegram Receiver type to AlertmanagerConfig CRD
Fixes #4703

There are a few things to consider while using this new feature:

API URL is mandatory in the alertmanager go project, but docs say the opposite: telegram_config.api_url should be optional prometheus/alertmanager#2876
The default parse mode, markdownv2, is not fully compatible yet, meanwhile consider using HTML: Issue with Telegram bot ID prometheus/alertmanager#2866

Type of change

CHANGE (fix or feature that would cause existing functionality to not work as expected)
FEATURE (non-breaking change which adds functionality)
BUGFIX (non-breaking change which fixes an issue)
ENHANCEMENT (non-breaking change which improves existing functionality)
NONE (if none of the other choices apply. Example, tooling, build system, CI, docs, etc.)

Changelog entry

Add Telegram Receiver type to AlertmanagerConfig CRD

- Add Telegram Receiver type to AlertmanagerConfig CRD

pkg/apis/monitoring/v1alpha1/alertmanager_config_types.go

pkg/alertmanager/operator.go

pkg/alertmanager/amcfg.go

angelbarrera92 · 2022-04-23T07:42:00Z

@philipgough i did the changes mentioned above :)
let me know if everything is fine or if there are something else missing

pkg/alertmanager/validation.go

philipgough · 2022-04-28T13:51:33Z

pkg/apis/monitoring/v1alpha1/alertmanager_config_types.go

+	// +optional
+	DisableNotifications *bool `json:"disableNotifications,omitempty"`
+	// Parse mode for telegram message
+	//+kubebuilder:validation:Enum="";MarkdownV2;Markdown;HTML


nit: since this is marked as optional, I don't believe we need to include the empty string in the enum

Ye! I was having doubts with the enum, I looked for examples in the project and I found:
https://github.com/prometheus-operator/prometheus-operator/blob/main/pkg/apis/monitoring/v1/thanos_types.go#L145
So I decided to leave it with the empty value as it is a valid value documented: https://prometheus.io/docs/alerting/latest/configuration/#telegram_config

Still, if you think it could be better to remove it... let me know and I'll do it

I did a quick test and indeed the Kube API makes a distinction between

"the field is not present in the submitted request" ✔️

"the field is present and the value is empty" 🚫

"the field is present and the value is in the enums list" ✔️

From an API perspective, it's probably better to remove "" from the allowed values but we can tackle in a follow-up PR and globally.

pkg/alertmanager/validation.go

philipgough

/lgtm

thanks @angelbarrera92 for the contribution.

@simonpasquier - do you want to take a look before we merge?

pkg/apis/monitoring/v1alpha1/alertmanager_config_types.go

simonpasquier · 2022-05-03T15:05:56Z

pkg/apis/monitoring/v1alpha1/alertmanager_config_types.go

+	// +optional
+	DisableNotifications *bool `json:"disableNotifications,omitempty"`
+	// Parse mode for telegram message
+	//+kubebuilder:validation:Enum="";MarkdownV2;Markdown;HTML


I did a quick test and indeed the Kube API makes a distinction between

"the field is not present in the submitted request" ✔️

"the field is present and the value is empty" 🚫

"the field is present and the value is in the enums list" ✔️

From an API perspective, it's probably better to remove "" from the allowed values but we can tackle in a follow-up PR and globally.

pkg/alertmanager/validation.go

pkg/alertmanager/operator.go

pkg/apis/monitoring/v1alpha1/alertmanager_config_types.go

pkg/alertmanager/amcfg.go

angelbarrera92 · 2022-05-04T07:07:56Z

I think I resolved all the points @simonpasquier raised, so asking for a new review round!
Thanks!

angelbarrera92 · 2022-05-04T08:03:36Z

Maybe I can do a rebase to squash all commits before merging. What do you think?

simonpasquier · 2022-05-04T08:16:43Z

Maybe I can do a rebase to squash all commits before merging. What do you think?

We can "Squash and merge" from the GitHub UI :)

philipgough · 2022-05-08T09:20:59Z

@angelbarrera92 - thanks for taking care of this. Looks in good shape now but needs a rebase :)

angelbarrera92 · 2022-05-10T04:19:54Z

@angelbarrera92 - thanks for taking care of this. Looks in good shape now but needs a rebase :)

Ready!

mtb-xt · 2022-05-16T06:22:21Z

Guys please merge this! ⏳

This commit adds the logic to enable the usage of telegram as receiver for alertmanger Fixes: prometheus-operator#4703

Changes chatID from secret to int and adds a check for the alertmanager version Fixes: prometheus-operator#4703

Moves parseMode to be an enum in the telegram configuration. Fixes: prometheus-operator#4703

Removes not needed validation as the input is validated with the openAPI schema Fixes: prometheus-operator#4703

change errors -> fmt Co-authored-by: Simon Pasquier <spasquie@redhat.com>

change error message as suggested Co-authored-by: Simon Pasquier <spasquie@redhat.com>

check if exists config before checking the version Co-authored-by: Simon Pasquier <spasquie@redhat.com>

Move botToken and chatID validation to sanitize function

fix unit testing

change unused kubebuilder attributes

angelbarrera92 force-pushed the issues/4703 branch 6 times, most recently from 20d8a02 to 432aaaa Compare April 19, 2022 15:53

angelbarrera92 changed the title ~~WIP: Add Telegram Receiver type to AlertmanagerConfig CRD~~ Add Telegram Receiver type to AlertmanagerConfig CRD Apr 19, 2022

angelbarrera92 marked this pull request as ready for review April 19, 2022 16:49

angelbarrera92 requested a review from a team as a code owner April 19, 2022 16:49

angelbarrera92 mentioned this pull request Apr 19, 2022

Add Telegram Receiver type to AlertmanagerConfig CRD #4703

Closed

philipgough reviewed Apr 20, 2022

View reviewed changes

pkg/apis/monitoring/v1alpha1/alertmanager_config_types.go Outdated Show resolved Hide resolved

philipgough reviewed Apr 20, 2022

View reviewed changes

pkg/alertmanager/operator.go Show resolved Hide resolved

philipgough reviewed Apr 20, 2022

View reviewed changes

pkg/alertmanager/amcfg.go Show resolved Hide resolved

angelbarrera92 force-pushed the issues/4703 branch 6 times, most recently from 6783b9f to 7ef6c27 Compare April 23, 2022 06:49

angelbarrera92 requested a review from philipgough April 25, 2022 09:35

philipgough reviewed Apr 27, 2022

View reviewed changes

pkg/alertmanager/validation.go Outdated Show resolved Hide resolved

angelbarrera92 requested a review from philipgough April 28, 2022 13:02

philipgough reviewed Apr 28, 2022

View reviewed changes

pkg/alertmanager/validation.go Outdated Show resolved Hide resolved

angelbarrera92 requested a review from philipgough April 29, 2022 08:54

philipgough approved these changes Apr 29, 2022

View reviewed changes

simonpasquier reviewed May 3, 2022

View reviewed changes

angelbarrera92 requested a review from simonpasquier May 4, 2022 07:07

angelbarrera92 force-pushed the issues/4703 branch 2 times, most recently from a9d8d4d to f2c3d14 Compare May 9, 2022 14:01

angelbarrera92 and others added 11 commits May 16, 2022 09:32

alertmanager: Add Telegram Receiver type to AlertmanagerConfig CRD

5e56ca9

This commit adds the logic to enable the usage of telegram as receiver for alertmanger Fixes: prometheus-operator#4703

alertmanager: Add Telegram Receiver type to AlertmanagerConfig CRD

8c2907b

Changes chatID from secret to int and adds a check for the alertmanager version Fixes: prometheus-operator#4703

alertmanager: Add Telegram Receiver type to AlertmanagerConfig CRD

4b0abd0

Moves parseMode to be an enum in the telegram configuration. Fixes: prometheus-operator#4703

alertmanager: Add Telegram Receiver type to AlertmanagerConfig CRD

8765dc5

Removes not needed validation as the input is validated with the openAPI schema Fixes: prometheus-operator#4703

alertmanager: Add Telegram Receiver type to AlertmanagerConfig CRD

fdb1513

change errors -> fmt Co-authored-by: Simon Pasquier <spasquie@redhat.com>

alertmanager: Add Telegram Receiver type to AlertmanagerConfig CRD

435f8af

change errors -> fmt Co-authored-by: Simon Pasquier <spasquie@redhat.com>

alertmanager: Add Telegram Receiver type to AlertmanagerConfig CRD

f389a1d

change error message as suggested Co-authored-by: Simon Pasquier <spasquie@redhat.com>

alertmanager: Add Telegram Receiver type to AlertmanagerConfig CRD

40e9fca

check if exists config before checking the version Co-authored-by: Simon Pasquier <spasquie@redhat.com>

alertmanager: Add Telegram Receiver type to AlertmanagerConfig CRD

376dcbc

Move botToken and chatID validation to sanitize function

alertmanager: Add Telegram Receiver type to AlertmanagerConfig CRD

ec8a638

fix unit testing

alertmanager: Add Telegram Receiver type to AlertmanagerConfig CRD

f6cbd01

change unused kubebuilder attributes

angelbarrera92 force-pushed the issues/4703 branch from f2c3d14 to f6cbd01 Compare May 16, 2022 07:33

philipgough merged commit 2c2ad10 into prometheus-operator:main May 16, 2022

angelbarrera92 deleted the issues/4703 branch May 16, 2022 14:00

bryanasdev000 mentioned this pull request May 31, 2022

[kube-prometheus-stack] alertmanager v0.24.0, telegram not working prometheus-community/helm-charts#2101

Closed

RomanenkoDenys mentioned this pull request May 31, 2022

[prometheus-operator] bump version to support Telegram receiver deckhouse/deckhouse#1672

Closed

2 tasks

simonpasquier mentioned this pull request Feb 16, 2023

Webex Feature/API #5305

Merged

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Telegram Receiver type to AlertmanagerConfig CRD #4726

Add Telegram Receiver type to AlertmanagerConfig CRD #4726

angelbarrera92 commented Apr 18, 2022 •

edited

angelbarrera92 commented Apr 23, 2022

philipgough Apr 28, 2022

angelbarrera92 Apr 29, 2022

angelbarrera92 Apr 29, 2022

simonpasquier May 3, 2022

philipgough left a comment

simonpasquier May 3, 2022

angelbarrera92 commented May 4, 2022

angelbarrera92 commented May 4, 2022

simonpasquier commented May 4, 2022

philipgough commented May 8, 2022

angelbarrera92 commented May 10, 2022

mtb-xt commented May 16, 2022

Add Telegram Receiver type to AlertmanagerConfig CRD #4726

Add Telegram Receiver type to AlertmanagerConfig CRD #4726

Conversation

angelbarrera92 commented Apr 18, 2022 • edited

Description

Type of change

Changelog entry

angelbarrera92 commented Apr 23, 2022

philipgough Apr 28, 2022

Choose a reason for hiding this comment

angelbarrera92 Apr 29, 2022

Choose a reason for hiding this comment

angelbarrera92 Apr 29, 2022

Choose a reason for hiding this comment

simonpasquier May 3, 2022

Choose a reason for hiding this comment

philipgough left a comment

Choose a reason for hiding this comment

simonpasquier May 3, 2022

Choose a reason for hiding this comment

angelbarrera92 commented May 4, 2022

angelbarrera92 commented May 4, 2022

simonpasquier commented May 4, 2022

philipgough commented May 8, 2022

angelbarrera92 commented May 10, 2022

mtb-xt commented May 16, 2022

angelbarrera92 commented Apr 18, 2022 •

edited