Skip to content
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

Alerting: Fix notification channel migration #38983

Merged
merged 1 commit into from
Sep 9, 2021
Merged

Conversation

papagian
Copy link
Contributor

@papagian papagian commented Sep 8, 2021

What this PR does / why we need it:
Starting from grafana 7.2 notification channel secure settings are stored encrypted in the database. But for the existing alert notification channels, there was no automatic migration of storing sensitive settings encrypted. Therefore there can still exist such unencrypted secure settings (if the channel is not saved since upgrading to >= 7.2)
The dashboard alert migration when enabling the new alerting system takes cares of it and encrypts those settings so that all the secure settings are stored encrypted in the Alertmanager configuration stored in the database.
However, there was a bug in case such a migrated receiver was used in multiple contact points because we used to delete this setting. As a result, the secure setting was stored only the 1st time and was missing from all the rest.

Example of such problematic configuration

The slack receiver in autogen-contact-point-default has url property while not in autogen-contact-point-1

{
  "template_files": {},
  "alertmanager_config": {
    "route": {
      "receiver": "autogen-contact-point-default",
      "routes": [
        {
          "receiver": "autogen-contact-point-1",
          "matchers": [
            "rule_uid=\"-lmj0SI7zm\""
          ]
        }
      ]
    },
    "templates": null,
    "receivers": [
      {
        "name": "autogen-contact-point-default",
        "grafana_managed_receiver_configs": [
          {
            "uid": "m_mC0IInkz",
            "name": "slack receiver",
            "type": "slack",
            "disableResolveMessage": false,
            "settings": {
              "autoResolve": true,
              "httpMethod": "POST",
              "iconEmoji": "",
              "iconUrl": "",
              "mentionGroups": "",
              "mentionUsers": "",
              "recipient": "#test-alerts",
              "severity": "critical",
              "token": "",
              "uploadImage": true,
              "username": ""
            },
            "secureFields": {
              "url": true
            }
          }
        ]
      },
      {
        "name": "autogen-contact-point-1",
        "grafana_managed_receiver_configs": [
          {
            "uid": "3lmC0SSnz",
            "name": "webhook",
            "type": "webhook",
            "disableResolveMessage": false,
            "settings": {
              "autoResolve": true,
              "httpMethod": "POST",
              "password": "",
              "severity": "critical",
              "uploadImage": true,
              "url": "http://localhost:3010",
              "username": ""
            },
            "secureFields": {}
          },
          {
            "uid": "q_iCAII7zz",
            "name": "slack receiver",
            "type": "slack",
            "disableResolveMessage": false,
            "settings": {
              "autoResolve": true,
              "httpMethod": "POST",
              "iconEmoji": "",
              "iconUrl": "",
              "mentionGroups": "",
              "mentionUsers": "",
              "recipient": "#test-alerts",
              "severity": "critical",
              "token": "",
              "uploadImage": true,
              "username": ""
            },
            "secureFields": {}
          }
        ]
      }
    ]
  }
}

Special notes for your reviewer:
For reproducing the error one has to create an entry in the alert_notification table containing such an un-encrypted property (eg url property under the settings column for a slack notifier), use this channel in more than one alerts and enabling ngalert.

@papagian papagian requested a review from a team as a code owner September 8, 2021 12:28
@papagian papagian requested review from sdboyer and ying-jeanne and removed request for a team September 8, 2021 12:28
@armandgrillet armandgrillet requested a review from a team September 8, 2021 12:37
@papagian papagian added this to the 8.1.4 milestone Sep 8, 2021
@papagian papagian added the old backport v8.1.x Mark PR for automatic backport to v8.1.x label Sep 8, 2021
@papagian papagian merged commit b56bf83 into main Sep 9, 2021
@papagian papagian deleted the papagian/fix-migration branch September 9, 2021 15:53
@grafanabot
Copy link
Contributor

The backport to v8.1.x failed:

The process '/usr/bin/git' failed with exit code 1

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new branch
git switch --create backport-38983-to-v8.1.x origin/v8.1.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x b56bf83c19fc8f2dafdc745f02283f7ff27a387e
# Push it to GitHub
git push --set-upstream origin backport-38983-to-v8.1.x
git switch main
# Remove the local backport branch
git branch -D backport-38983-to-v8.1.x

Then, create a pull request where the base branch is v8.1.x and the compare/head branch is backport-38983-to-v8.1.x.

papagian added a commit that referenced this pull request Sep 9, 2021
papagian added a commit that referenced this pull request Sep 9, 2021
@sunker sunker added this to the 8.1.4 milestone Sep 16, 2021
stevepostill pushed a commit to Reveal-International/grafana that referenced this pull request Nov 3, 2021
…-github to revdev

* commit '56aebaac32bd7a82f84a571969b51c62ba54fd6d': (23 commits)
  "Release: Updated versions in package to 8.1.4" (grafana#39273)
  Docs: Update azuread docs to mention about env variables (grafana#39203) (grafana#39265)
  Docs: plugin migration guide 7.0.0 to 8.0.0 (grafana#38911) (grafana#39246)
  Docs: Added "manageAlerts" provisioning option. (grafana#38836) (grafana#39253)
  BarChart: Fix legend and tooltip colors off by 1 after data refresh (grafana#39234) (grafana#39252)
  Update silences.md (grafana#38834) (grafana#39243)
  Explore: Ensure logs volume bar colors match legend colors (grafana#39072) (grafana#39120)
  Graph: make old graph panel thresholds work even if ngalert is enabled  (grafana#38918) (grafana#39233)
  Fix regex to identify / as separator (grafana#39220)
  [v8.1.x] BarChart: fix stale bar values and x axis labels
  Add link to default template (grafana#39106) (grafana#39138)
  Doc: Created a separate topic for AWS authentication  (grafana#39012) (grafana#39160)
  LDAP: Search all DNs for users (grafana#38891) (grafana#39167)
  Docs: Fix broken link to signed GCS URLs docs (grafana#39144) (grafana#39149)
  [v8.1.x] Variables: Fix not updating inside a Panel when the preceding Row uses "Repeat For" (grafana#39141)
  Docs: Improve v8 upgrade notes for SQL data sources (grafana#38792) (grafana#39124)
  Explore: Trace view now shows trace start time in selected timezone (grafana#39054) (grafana#39093)
  Alerting: Fix notification channel migration (grafana#38983) (grafana#39053)
  Annotations: Fixes blank panels for queries with unknown data sources (grafana#39017) (grafana#39034)
  Copy cue.mod and packages/grafana-schema to container workdir (grafana#38998) (grafana#39018)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants