Alerting: Fix some status codes returned from provisioning API. (#90117)

The contact point deletion API was returning 500 when it should have been
returning a 4xx error, when the contact point is in use:

- When in use by a notificiation policy, we were missing
  the `.Errorf("")` to convert `errutil.Base` into `errutil.Error`.
- When in use by an alert rule, an regular error was returned.
This commit is contained in:
Steve Simpson 2024-07-05 19:06:37 +02:00 committed by GitHub
parent 650616a404
commit e9fd191065
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 158 additions and 3 deletions

View File

@ -339,7 +339,7 @@ func (ecp *ContactPointService) DeleteContactPoint(ctx context.Context, orgID in
} }
} }
if fullRemoval && isContactPointInUse(name, []*apimodels.Route{revision.cfg.AlertmanagerConfig.Route}) { if fullRemoval && isContactPointInUse(name, []*apimodels.Route{revision.cfg.AlertmanagerConfig.Route}) {
return ErrContactPointReferenced return ErrContactPointReferenced.Errorf("")
} }
return ecp.xact.InTransaction(ctx, func(ctx context.Context) error { return ecp.xact.InTransaction(ctx, func(ctx context.Context) error {
@ -354,7 +354,7 @@ func (ecp *ContactPointService) DeleteContactPoint(ctx context.Context, orgID in
uids = append(uids, key.UID) uids = append(uids, key.UID)
} }
ecp.log.Error("Cannot delete contact point because it is used in rule's notification settings", "receiverName", name, "rulesUid", strings.Join(uids, ",")) ecp.log.Error("Cannot delete contact point because it is used in rule's notification settings", "receiverName", name, "rulesUid", strings.Join(uids, ","))
return fmt.Errorf("contact point '%s' is currently used in notification settings by one or many alert rules", name) return ErrContactPointUsedInRule.Errorf("")
} }
} }

View File

@ -28,7 +28,8 @@ var (
ErrTimeIntervalInvalid = errutil.BadRequest("alerting.notifications.time-intervals.invalidFormat").MustTemplate("Invalid format of the submitted time interval", errutil.WithPublic("Time interval is in invalid format. Correct the payload and try again.")) ErrTimeIntervalInvalid = errutil.BadRequest("alerting.notifications.time-intervals.invalidFormat").MustTemplate("Invalid format of the submitted time interval", errutil.WithPublic("Time interval is in invalid format. Correct the payload and try again."))
ErrTimeIntervalInUse = errutil.Conflict("alerting.notifications.time-intervals.used", errutil.WithPublicMessage("Time interval is used by one or many notification policies")) ErrTimeIntervalInUse = errutil.Conflict("alerting.notifications.time-intervals.used", errutil.WithPublicMessage("Time interval is used by one or many notification policies"))
ErrContactPointReferenced = errutil.BadRequest("alerting.notifications.contact-points.referenced", errutil.WithPublicMessage("Contact point is currently referenced by a notification policy.")) ErrContactPointReferenced = errutil.Conflict("alerting.notifications.contact-points.referenced", errutil.WithPublicMessage("Contact point is currently referenced by a notification policy."))
ErrContactPointUsedInRule = errutil.Conflict("alerting.notifications.contact-points.used-by-rule", errutil.WithPublicMessage("Contact point is currently used in the notification settings of one or many alert rules."))
) )
func makeErrBadAlertmanagerConfiguration(err error) error { func makeErrBadAlertmanagerConfiguration(err error) error {

View File

@ -10,13 +10,16 @@ import (
"sort" "sort"
"strings" "strings"
"testing" "testing"
"time"
"github.com/prometheus/alertmanager/config" "github.com/prometheus/alertmanager/config"
"github.com/prometheus/alertmanager/timeinterval" "github.com/prometheus/alertmanager/timeinterval"
"github.com/prometheus/common/model"
"github.com/stretchr/testify/assert" "github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require" "github.com/stretchr/testify/require"
"github.com/grafana/grafana/pkg/apimachinery/errutil" "github.com/grafana/grafana/pkg/apimachinery/errutil"
"github.com/grafana/grafana/pkg/expr"
"github.com/grafana/grafana/pkg/services/ngalert/api/tooling/definitions" "github.com/grafana/grafana/pkg/services/ngalert/api/tooling/definitions"
"github.com/grafana/grafana/pkg/services/ngalert/models" "github.com/grafana/grafana/pkg/services/ngalert/models"
"github.com/grafana/grafana/pkg/services/org" "github.com/grafana/grafana/pkg/services/org"
@ -24,6 +27,50 @@ import (
"github.com/grafana/grafana/pkg/tests/testinfra" "github.com/grafana/grafana/pkg/tests/testinfra"
) )
func createRuleWithNotificationSettings(t *testing.T, client apiClient, folder string, nfSettings *definitions.AlertRuleNotificationSettings) (definitions.PostableRuleGroupConfig, string) {
t.Helper()
interval, err := model.ParseDuration("1m")
require.NoError(t, err)
doubleInterval := 2 * interval
rules := definitions.PostableRuleGroupConfig{
Name: "arulegroup",
Interval: interval,
Rules: []definitions.PostableExtendedRuleNode{
{
ApiRuleNode: &definitions.ApiRuleNode{
For: &doubleInterval,
Labels: map[string]string{"label1": "val1"},
Annotations: map[string]string{"annotation1": "val1"},
},
GrafanaManagedAlert: &definitions.PostableGrafanaRule{
Title: fmt.Sprintf("rule under folder %s", folder),
Condition: "A",
Data: []definitions.AlertQuery{
{
RefID: "A",
RelativeTimeRange: definitions.RelativeTimeRange{
From: definitions.Duration(time.Duration(5) * time.Hour),
To: definitions.Duration(time.Duration(3) * time.Hour),
},
DatasourceUID: expr.DatasourceUID,
Model: json.RawMessage(`{
"type": "math",
"expression": "2 + 3 > 1"
}`),
},
},
NotificationSettings: nfSettings,
},
},
},
}
resp, status, _ := client.PostRulesGroupWithStatus(t, folder, &rules)
assert.Equal(t, http.StatusAccepted, status)
require.Len(t, resp.Created, 1)
return rules, resp.Created[0]
}
func TestIntegrationProvisioning(t *testing.T) { func TestIntegrationProvisioning(t *testing.T) {
testinfra.SQLiteIntegrationTest(t) testinfra.SQLiteIntegrationTest(t)
@ -258,6 +305,113 @@ func TestIntegrationProvisioning(t *testing.T) {
require.Equal(t, 202, resp.StatusCode) require.Equal(t, 202, resp.StatusCode)
}) })
createContactPoint := func(t *testing.T, name string) definitions.EmbeddedContactPoint {
cpBody := fmt.Sprintf(`
{
"name": "%s",
"type": "slack",
"settings": {
"recipient": "value_recipient",
"token": "value_token"
}
}`, name)
req := createTestRequest("POST", url, "admin", cpBody)
resp, err := http.DefaultClient.Do(req)
require.NoError(t, err)
require.Equal(t, 202, resp.StatusCode)
ecp := definitions.EmbeddedContactPoint{}
require.NoError(t, json.NewDecoder(resp.Body).Decode(&ecp))
require.NoError(t, resp.Body.Close())
return ecp
}
createPolicyForContactPoint := func(t *testing.T, receiver string) {
url := fmt.Sprintf("http://%s/api/v1/provisioning/policies", grafanaListedAddr)
body := fmt.Sprintf(`
{
"receiver": "%s",
"group_by": [
"..."
],
"routes": []
}`, receiver)
req := createTestRequest("PUT", url, "admin", body)
resp, err := http.DefaultClient.Do(req)
require.NoError(t, err)
require.NoError(t, resp.Body.Close())
require.Equal(t, 202, resp.StatusCode)
}
t.Run("viewer DELETE should 403", func(t *testing.T) {
ecp := createContactPoint(t, "my-contact-point")
deleteURL := fmt.Sprintf("http://%s/api/v1/provisioning/contact-points/%s", grafanaListedAddr, ecp.UID)
req := createTestRequest("DELETE", deleteURL, "viewer", body)
resp, err := http.DefaultClient.Do(req)
require.NoError(t, err)
require.NoError(t, resp.Body.Close())
require.Equal(t, 403, resp.StatusCode)
})
t.Run("admin DELETE should succeed", func(t *testing.T) {
ecp := createContactPoint(t, "my-contact-point")
deleteURL := fmt.Sprintf("http://%s/api/v1/provisioning/contact-points/%s", grafanaListedAddr, ecp.UID)
req := createTestRequest("DELETE", deleteURL, "admin", "")
resp, err := http.DefaultClient.Do(req)
require.NoError(t, err)
require.NoError(t, resp.Body.Close())
require.Equal(t, 202, resp.StatusCode)
})
t.Run("admin DELETE should 409 when contact point used by notification policy", func(t *testing.T) {
ecp := createContactPoint(t, "my-cp-used-by-policy")
createPolicyForContactPoint(t, "my-cp-used-by-policy")
deleteURL := fmt.Sprintf("http://%s/api/v1/provisioning/contact-points/%s", grafanaListedAddr, ecp.UID)
deleteReq := createTestRequest("DELETE", deleteURL, "admin", "")
resp, err := http.DefaultClient.Do(deleteReq)
require.NoError(t, err)
require.Equal(t, 409, resp.StatusCode)
var validationError errutil.PublicError
assert.NoError(t, json.NewDecoder(resp.Body).Decode(&validationError))
require.NoError(t, resp.Body.Close())
assert.NotEmpty(t, validationError, validationError.Message)
assert.Equal(t, "alerting.notifications.contact-points.referenced", validationError.MessageID)
})
t.Run("admin DELETE should 409 when contact point used by rule", func(t *testing.T) {
ecp := createContactPoint(t, "my-cp-used-by-rule")
nfSettings := &definitions.AlertRuleNotificationSettings{
Receiver: "my-cp-used-by-rule",
}
apiClient := newAlertingApiClient(grafanaListedAddr, "admin", "admin")
createRuleWithNotificationSettings(t, apiClient, namespaceUID, nfSettings)
deleteURL := fmt.Sprintf("http://%s/api/v1/provisioning/contact-points/%s", grafanaListedAddr, ecp.UID)
deleteReq := createTestRequest("DELETE", deleteURL, "admin", "")
resp, err := http.DefaultClient.Do(deleteReq)
require.NoError(t, err)
require.Equal(t, 409, resp.StatusCode)
var validationError errutil.PublicError
assert.NoError(t, json.NewDecoder(resp.Body).Decode(&validationError))
require.NoError(t, resp.Body.Close())
assert.NotEmpty(t, validationError, validationError.Message)
assert.Equal(t, "alerting.notifications.contact-points.used-by-rule", validationError.MessageID)
})
}) })
t.Run("when provisioning templates", func(t *testing.T) { t.Run("when provisioning templates", func(t *testing.T) {