fix(telegram): parse correctly ChatID and MessageThreadID #198
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
With this commit, the ChatID and MessageThreadID configuration of the Telegram notifier now can be configured either with a string or an integer, without any breaking changes for anyone consuming this library or the end users using it in their configuration files.
Fixes grafana/grafana#69950
Reasoning
The Telegram API specification tells us that ChatID can be an Integer or a String and MessageThreadID must be an integer.
In the PR grafana/grafana#63085, Grafana started to interpret correctly numbers and booleans from environment variables when provisioning files. So before the change, the following got evaluated:
ENV="1"
->"1"
(string)And after the change it evaluated as:
ENV="1"
->1
(integer)With this new change, it is currently impossible to provision Telegram's ChatID or MessageThredID, since they're integers, and get parsed as such, like so:
TELEGRAM_CHAT_ID="-1234"
->-1234
And then grafana (using this library to parse the Telegram configuration), throws the following error:
Implementation details
In order to avoid introducing breaking changes, I've opted to duplicate the
Config
structure, making it private and transformingChatID
andMessageThreadID
to the typeany
. Afterwards, the configuration value gets parsed and transformed back to a string, ensuring bothstring
andint
variants work.Other implementations explored
I've also explored two other options:
ChatID
andMessageThreadID
fromstring
toint
. I've also discarted this option, since it would introduce breaking changes for all the users that are already configuring Telegram using astring
in the provisioning templates.That's why I've opted for duplicating the struct, which is ugly but gets the work done and, more importantly, doesn't introduce any breaking change.
Let me know what you think! This issue is forcing us to stay at Grafana 9.3, which has some security vulnerabilities and not the latest features.
Thank you,
Néfix Estrada
IsardVDI