Skip to content

Conversation

alexander-akhmetov
Copy link
Contributor

@alexander-akhmetov alexander-akhmetov commented Jun 20, 2025

What this PR does

This PR changes the handler that saves Grafana Alertmanager configurtion to preserve the original JSON string during unmarshaling, instead of re-marshaling the parsed Go struct before saving it.

If we do a JSON marshalling round-trip and the Grafana config contains an upstream Alertmanager configuration, secrets, example, in this configuration are lost during marshalling and saved to the storage as redacted strings.

Instead this PR makes the handler use JSON unmarshalling to validate the configuration, and save the original JSON to the storage.

Checklist

  • Tests updated.
  • Documentation added.
  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]. If changelog entry is not needed, please add the changelog-not-needed label to the PR.
  • about-versioning.md updated with experimental features.

@alexander-akhmetov alexander-akhmetov force-pushed the alexander-akhmetov/grafana-merged-alertmanager-config branch 6 times, most recently from 283d03c to 21ee9a5 Compare June 20, 2025 11:27
@alexander-akhmetov alexander-akhmetov added changelog-not-needed PRs that don't need a CHANGELOG.md entry component/alertmanager labels Jun 20, 2025
@alexander-akhmetov alexander-akhmetov changed the title Alertmanager: Save the original string when updating Grafana configuration Alertmanager: Save the original config string when updating Grafana configuration Jun 20, 2025
@alexander-akhmetov alexander-akhmetov marked this pull request as ready for review June 20, 2025 13:43
@alexander-akhmetov alexander-akhmetov requested review from a team as code owners June 20, 2025 13:43
@alexander-akhmetov alexander-akhmetov force-pushed the alexander-akhmetov/grafana-merged-alertmanager-config branch 2 times, most recently from 0e4f123 to ca9064b Compare July 1, 2025 16:10
Copy link
Contributor

@yuri-tceretian yuri-tceretian left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@alexander-akhmetov alexander-akhmetov force-pushed the alexander-akhmetov/grafana-merged-alertmanager-config branch from ca9064b to 802644f Compare July 2, 2025 17:42
@stevesg stevesg merged commit 5b570b6 into main Jul 3, 2025
31 checks passed
@stevesg stevesg deleted the alexander-akhmetov/grafana-merged-alertmanager-config branch July 3, 2025 09:02
unkls pushed a commit to unkls/mimir that referenced this pull request Jul 15, 2025
…onfiguration (grafana#11796)

#### What this PR does

This PR changes the handler that saves Grafana Alertmanager configurtion
to preserve the original JSON string during unmarshaling, instead of
re-marshaling the parsed Go struct before saving it.

If we do a JSON marshalling round-trip and the Grafana config contains
an upstream Alertmanager configuration, secrets,
[example](https://github.com/prometheus/alertmanager/blob/0ce3cfb962db3cbb1649d3e816a49a13c4036cd1/config/config.go#L56),
in this configuration are lost during marshalling and saved to the
storage as redacted strings.

Instead this PR makes the handler use JSON unmarshalling to validate the
configuration, and save the original JSON to the storage.

#### Checklist

- [x] Tests updated.
- [ ] Documentation added.
- [ ] `CHANGELOG.md` updated - the order of entries should be
`[CHANGE]`, `[FEATURE]`, `[ENHANCEMENT]`, `[BUGFIX]`. If changelog entry
is not needed, please add the `changelog-not-needed` label to the PR.
- [ ]
[`about-versioning.md`](https://github.com/grafana/mimir/blob/main/docs/sources/mimir/configure/about-versioning.md)
updated with experimental features.
Signed-off-by: unkls <benjamin.leveque97@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog-not-needed PRs that don't need a CHANGELOG.md entry component/alertmanager
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants