Alerting: Various fixes for the alerts endpoint (#33182)

A set of fixes for the GET alert and groups endpoints.

- First, is the fact that the default values where not being for the query params. I've introduced a new method in the Grafana context that allow us to do this.
- Second, is the fact that alerts were never being transitioned to active. To my surprise this is actually done by the inhibitor in the pipeline - if an alert is not muted, or inhibited then it's active.
- Third, I have added an integration test to cover for regressions.

Signed-off-by: Josue Abreu <josue@grafana.com>

pkg/models/context.go

@@ -78,3 +78,13 @@ func (ctx *ReqContext) HasHelpFlag(flag HelpFlags1) bool {
 func (ctx *ReqContext) TimeRequest(timer prometheus.Summary) {
 	ctx.Data["perfmon.timer"] = timer
 }
+
+// QueryBoolWithDefault extracts a value from the request query params and applies a bool default if not present.
+func (ctx *ReqContext) QueryBoolWithDefault(field string, d bool) bool {
+	f := ctx.Query(field)
+	if f == "" {
+		return d
+	}
+
+	return ctx.QueryBool(field)
+}

pkg/models/context_test.go

@@ -0,0 +1,41 @@
+package models
+
+import (
+	"net/http"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+	"gopkg.in/macaron.v1"
+)
+
+func TestQueryBoolWithDefault(t *testing.T) {
+	tc := map[string]struct {
+		url          string
+		defaultValue bool
+		expected     bool
+	}{
+		"with no value specified, the default value is returned": {
+			url:          "http://localhost/api/v2/alerts",
+			defaultValue: true,
+			expected:     true,
+		},
+		"with a value specified, the default value is overridden": {
+			url:          "http://localhost/api/v2/alerts?silenced=false",
+			defaultValue: true,
+			expected:     false,
+		},
+	}
+
+	for name, tt := range tc {
+		t.Run(name, func(t *testing.T) {
+			req, err := http.NewRequest("GET", tt.url, nil)
+			require.NoError(t, err)
+			r := ReqContext{
+				Context: &macaron.Context{
+					Req: macaron.Request{Request: req},
+				},
+			}
+			require.Equal(t, tt.expected, r.QueryBoolWithDefault("silenced", tt.defaultValue))
+		})
+	}
+}

pkg/services/ngalert/api/api_alertmanager.go

@@ -113,9 +113,9 @@ func (srv AlertmanagerSrv) RouteGetAlertingConfig(c *models.ReqContext) response
 
 func (srv AlertmanagerSrv) RouteGetAMAlertGroups(c *models.ReqContext) response.Response {
 	groups, err := srv.am.GetAlertGroups(
-		c.QueryBool("active"),
-		c.QueryBool("silenced"),
-		c.QueryBool("inhibited"),
+		c.QueryBoolWithDefault("active", true),
+		c.QueryBoolWithDefault("silenced", true),
+		c.QueryBoolWithDefault("inhibited", true),
 		c.QueryStrings("filter"),
 		c.Query("receiver"),
 	)
@@ -132,9 +132,9 @@ func (srv AlertmanagerSrv) RouteGetAMAlertGroups(c *models.ReqContext) response.
 
 func (srv AlertmanagerSrv) RouteGetAMAlerts(c *models.ReqContext) response.Response {
 	alerts, err := srv.am.GetAlerts(
-		c.QueryBool("active"),
-		c.QueryBool("silenced"),
-		c.QueryBool("inhibited"),
+		c.QueryBoolWithDefault("active", true),
+		c.QueryBoolWithDefault("silenced", true),
+		c.QueryBoolWithDefault("inhibited", true),
 		c.QueryStrings("filter"),
 		c.Query("receiver"),
 	)

pkg/services/ngalert/notifier/alertmanager.go

@@ -14,6 +14,7 @@ import (
 	apimodels "github.com/grafana/grafana/pkg/services/ngalert/api/tooling/definitions"
 	"github.com/pkg/errors"
 	"github.com/prometheus/alertmanager/dispatch"
+	"github.com/prometheus/alertmanager/inhibit"
 	"github.com/prometheus/alertmanager/nflog"
 	"github.com/prometheus/alertmanager/nflog/nflogpb"
 	"github.com/prometheus/alertmanager/notify"
@@ -74,13 +75,14 @@ type Alertmanager struct {
 	// notificationLog keeps tracks of which notifications we've fired already.
 	notificationLog *nflog.Log
 	// silences keeps the track of which notifications we should not fire due to user configuration.
-	silencer     *silence.Silencer
-	silences     *silence.Silences
-	marker       types.Marker
-	alerts       *AlertProvider
-	route        *dispatch.Route
-	dispatcher   *dispatch.Dispatcher
-	dispatcherWG sync.WaitGroup
+	silencer   *silence.Silencer
+	silences   *silence.Silences
+	marker     types.Marker
+	alerts     *AlertProvider
+	route      *dispatch.Route
+	dispatcher *dispatch.Dispatcher
+	inhibitor  *inhibit.Inhibitor
+	wg         sync.WaitGroup
 
 	stageMetrics      *notify.Metrics
 	dispatcherMetrics *dispatch.DispatcherMetrics
@@ -159,7 +161,11 @@ func (am *Alertmanager) StopAndWait() {
 	if am.dispatcher != nil {
 		am.dispatcher.Stop()
 	}
-	am.dispatcherWG.Wait()
+
+	if am.inhibitor != nil {
+		am.inhibitor.Stop()
+	}
+	am.wg.Wait()
 }
 
 func (am *Alertmanager) SaveAndApplyConfig(cfg *apimodels.PostableUserConfig) error {
@@ -260,23 +266,32 @@ func (am *Alertmanager) applyConfig(cfg *apimodels.PostableUserConfig) error {
 	// Now, let's put together our notification pipeline
 	routingStage := make(notify.RoutingStage, len(integrationsMap))
 
+	am.inhibitor = inhibit.NewInhibitor(am.alerts, cfg.AlertmanagerConfig.InhibitRules, am.marker, gokit_log.NewNopLogger())
 	am.silencer = silence.NewSilencer(am.silences, am.marker, gokit_log.NewNopLogger())
+
+	inhibitionStage := notify.NewMuteStage(am.inhibitor)
 	silencingStage := notify.NewMuteStage(am.silencer)
 	for name := range integrationsMap {
 		stage := am.createReceiverStage(name, integrationsMap[name], waitFunc, am.notificationLog)
-		routingStage[name] = notify.MultiStage{silencingStage, stage}
+		routingStage[name] = notify.MultiStage{silencingStage, inhibitionStage, stage}
 	}
 
 	am.StopAndWait()
 	am.route = dispatch.NewRoute(cfg.AlertmanagerConfig.Route, nil)
 	am.dispatcher = dispatch.NewDispatcher(am.alerts, am.route, routingStage, am.marker, timeoutFunc, gokit_log.NewNopLogger(), am.dispatcherMetrics)
 
-	am.dispatcherWG.Add(1)
+	am.wg.Add(1)
 	go func() {
-		defer am.dispatcherWG.Done()
+		defer am.wg.Done()
 		am.dispatcher.Run()
 	}()
 
+	am.wg.Add(1)
+	go func() {
+		defer am.wg.Done()
+		am.inhibitor.Run()
+	}()
+
 	am.config = rawConfig
 	return nil
 }

pkg/services/ngalert/notifier/alerts.go

@@ -138,6 +138,7 @@ func (am *Alertmanager) alertFilter(matchers []*labels.Matcher, silenced, inhibi
 
 		// Set alert's current status based on its label set.
 		am.silencer.Mutes(a.Labels)
+		am.inhibitor.Mutes(a.Labels)
 
 		// Get alert's current status after seeing if it is suppressed.
 		status := am.marker.Status(a.Fingerprint())

pkg/tests/api/alerting/api_alertmanager_test.go

@@ -9,6 +9,7 @@ import (
 	"testing"
 	"time"
 
+	"github.com/prometheus/common/model"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
@@ -23,7 +24,9 @@ import (
 func TestAlertAndGroupsQuery(t *testing.T) {
 	dir, path := testinfra.CreateGrafDir(t, testinfra.GrafanaOpts{
 		EnableFeatureToggles: []string{"ngalert"},
+		AnonymousUserRole:    models.ROLE_EDITOR,
 	})
+
 	store := testinfra.SetUpDatabase(t, dir)
 	grafanaListedAddr := testinfra.StartGrafana(t, dir, path, store)
 
@@ -59,6 +62,87 @@ func TestAlertAndGroupsQuery(t *testing.T) {
 		require.Equal(t, 200, resp.StatusCode)
 		require.JSONEq(t, "[]", string(b))
 	}
+
+	// Now, let's test the endpoint with some alerts.
+	{
+		// Create the namespace we'll save our alerts to.
+		require.NoError(t, createFolder(t, store, 0, "default"))
+	}
+
+	// Create an alert that will fire as quickly as possible
+	{
+		interval, err := model.ParseDuration("10s")
+		require.NoError(t, err)
+		rules := apimodels.PostableRuleGroupConfig{
+			Name:     "arulegroup",
+			Interval: interval,
+			Rules: []apimodels.PostableExtendedRuleNode{
+				{
+					GrafanaManagedAlert: &apimodels.PostableGrafanaRule{
+						OrgID:     2,
+						Title:     "AlwaysFiring",
+						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),
+								},
+								Model: json.RawMessage(`{
+									"datasourceUid": "-100",
+									"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://%s/api/ruler/grafana/api/v1/rules/default", grafanaListedAddr)
+		// nolint:gosec
+		resp, err := http.Post(u, "application/json", &buf)
+		t.Cleanup(func() {
+			err := resp.Body.Close()
+			require.NoError(t, err)
+		})
+		require.NoError(t, err)
+		assert.Equal(t, resp.StatusCode, 202)
+	}
+
+	// Eventually, we'll get an alert with its state being active.
+	{
+		alertsURL := fmt.Sprintf("http://%s/api/alertmanager/grafana/api/v2/alerts", grafanaListedAddr)
+		// nolint:gosec
+		require.Eventually(t, func() bool {
+			resp, err := http.Get(alertsURL)
+			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)
+			require.Equal(t, 200, resp.StatusCode)
+
+			var alerts apimodels.GettableAlerts
+			err = json.Unmarshal(b, &alerts)
+			require.NoError(t, err)
+
+			if len(alerts) > 0 {
+				status := alerts[0].Status
+				return status != nil && status.State != nil && *status.State == "active"
+			}
+
+			return false
+		}, 18*time.Second, 2*time.Second)
+	}
 }
 
 func TestAlertRuleCRUD(t *testing.T) {