From 29638a485b9ffda23064ebfbe7344bf0764b500b Mon Sep 17 00:00:00 2001 From: George Robinson Date: Wed, 6 Oct 2021 12:17:29 +0100 Subject: [PATCH] Panel ID annotation cannot be set without Dashboard UID (#40019) (#40063) (cherry picked from commit 935bd34a305312367c9328ae3e32813a48da9b04) --- pkg/services/ngalert/store/alert_rule.go | 4 ++ .../sqlstore/migrations/ualert/ualert.go | 16 ++++-- pkg/tests/api/alerting/api_prometheus_test.go | 54 +++++++++++++++++++ 3 files changed, 69 insertions(+), 5 deletions(-) diff --git a/pkg/services/ngalert/store/alert_rule.go b/pkg/services/ngalert/store/alert_rule.go index a3c8cf263b6..6f37e912042 100644 --- a/pkg/services/ngalert/store/alert_rule.go +++ b/pkg/services/ngalert/store/alert_rule.go @@ -503,6 +503,10 @@ func (st DBstore) validateAlertRule(alertRule ngmodels.AlertRule) error { return fmt.Errorf("%w: no organisation is found", ngmodels.ErrAlertRuleFailedValidation) } + if alertRule.DashboardUID == nil && alertRule.PanelID != nil { + return fmt.Errorf("%w: cannot have Panel ID without a Dashboard UID", ngmodels.ErrAlertRuleFailedValidation) + } + return nil } diff --git a/pkg/services/sqlstore/migrations/ualert/ualert.go b/pkg/services/sqlstore/migrations/ualert/ualert.go index 71b605eb8a5..c1a248d48f1 100644 --- a/pkg/services/sqlstore/migrations/ualert/ualert.go +++ b/pkg/services/sqlstore/migrations/ualert/ualert.go @@ -158,11 +158,17 @@ func (m *updateDashboardUIDPanelIDMigration) Exec(sess *xorm.Session, mg *migrat } panelID = &i } - if _, err := sess.Exec(`UPDATE alert_rule SET dashboard_uid = ?, panel_id = ? WHERE id = ?`, - dashboardUID, - panelID, - next.ID); err != nil { - return fmt.Errorf("failed to set dashboard_uid and panel_id for alert rule: %w", err) + // We do not want to set panel_id to a non-nil value when dashboard_uid is nil + // as panel_id is not unique and so cannot be queried without its dashboard_uid. + // This can happen where users have deleted the dashboard_uid annotation but kept + // the panel_id annotation. + if dashboardUID != nil { + if _, err := sess.Exec(`UPDATE alert_rule SET dashboard_uid = ?, panel_id = ? WHERE id = ?`, + dashboardUID, + panelID, + next.ID); err != nil { + return fmt.Errorf("failed to set dashboard_uid and panel_id for alert rule: %w", err) + } } } return nil diff --git a/pkg/tests/api/alerting/api_prometheus_test.go b/pkg/tests/api/alerting/api_prometheus_test.go index 7b35fdef914..969af2b56ba 100644 --- a/pkg/tests/api/alerting/api_prometheus_test.go +++ b/pkg/tests/api/alerting/api_prometheus_test.go @@ -150,6 +150,60 @@ func TestPrometheusRules(t *testing.T) { require.JSONEq(t, `{"message":"rule group updated successfully"}`, string(b)) } + // Check that we cannot create a rule that has a panel_id and no dashboard_uid + { + rules := apimodels.PostableRuleGroupConfig{ + Name: "anotherrulegroup", + Rules: []apimodels.PostableExtendedRuleNode{ + { + ApiRuleNode: &apimodels.ApiRuleNode{ + For: interval, + Labels: map[string]string{}, + Annotations: map[string]string{"__panelId__": "1"}, + }, + // this rule does not explicitly set no data and error states + // therefore it should get the default values + GrafanaManagedAlert: &apimodels.PostableGrafanaRule{ + Title: "NeverCreated", + Condition: "A", + Data: []ngmodels.AlertQuery{ + { + RefID: "A", + RelativeTimeRange: ngmodels.RelativeTimeRange{ + From: ngmodels.Duration(time.Duration(5) * time.Hour), + To: ngmodels.Duration(time.Duration(3) * time.Hour), + }, + DatasourceUID: "-100", + Model: json.RawMessage(`{ + "type": "math", + "expression": "2 + 3 > 1" + }`), + }, + }, + }, + }, + }, + } + buf := bytes.Buffer{} + enc := json.NewEncoder(&buf) + err := enc.Encode(&rules) + require.NoError(t, err) + + u := fmt.Sprintf("http://grafana:password@%s/api/ruler/grafana/api/v1/rules/default", grafanaListedAddr) + // nolint:gosec + resp, err := http.Post(u, "application/json", &buf) + require.NoError(t, err) + t.Cleanup(func() { + err := resp.Body.Close() + require.NoError(t, err) + }) + b, err := ioutil.ReadAll(resp.Body) + require.NoError(t, err) + + assert.Equal(t, 400, resp.StatusCode) + require.JSONEq(t, `{"message":"failed to update rule group: invalid alert rule: cannot have Panel ID without a Dashboard UID"}`, string(b)) + } + // Now, let's see how this looks like. { promRulesURL := fmt.Sprintf("http://grafana:password@%s/api/prometheus/grafana/api/v1/rules", grafanaListedAddr)