Alerting: MuteTiming service return errutil + GetTiming by name (#79772)

* add get mute timing by name to MuteTimingService
* update get mute timing request handler to use the service method

* replace validation, uniqueness and used errors with errutils
* update mute timing methods return errutil responses
* use the term "time interval" in errors bevause mute timings are deprecated in Alertmanager and will be replaced by time intervals in the future.

* update create and update methods to return struct instead of pointer

pkg/services/ngalert/api/api_provisioning.go

@@ -51,8 +51,9 @@ type NotificationPolicyService interface {
 
 type MuteTimingService interface {
 	GetMuteTimings(ctx context.Context, orgID int64) ([]definitions.MuteTimeInterval, error)
-	CreateMuteTiming(ctx context.Context, mt definitions.MuteTimeInterval, orgID int64) (*definitions.MuteTimeInterval, error)
-	UpdateMuteTiming(ctx context.Context, mt definitions.MuteTimeInterval, orgID int64) (*definitions.MuteTimeInterval, error)
+	GetMuteTiming(ctx context.Context, name string, orgID int64) (definitions.MuteTimeInterval, error)
+	CreateMuteTiming(ctx context.Context, mt definitions.MuteTimeInterval, orgID int64) (definitions.MuteTimeInterval, error)
+	UpdateMuteTiming(ctx context.Context, mt definitions.MuteTimeInterval, orgID int64) (definitions.MuteTimeInterval, error)
 	DeleteMuteTiming(ctx context.Context, name string, orgID int64) error
 }
 
@@ -243,22 +244,17 @@ func (srv *ProvisioningSrv) RouteDeleteTemplate(c *contextmodel.ReqContext, name
 }
 
 func (srv *ProvisioningSrv) RouteGetMuteTiming(c *contextmodel.ReqContext, name string) response.Response {
-	timings, err := srv.muteTimings.GetMuteTimings(c.Req.Context(), c.SignedInUser.GetOrgID())
+	timing, err := srv.muteTimings.GetMuteTiming(c.Req.Context(), name, c.SignedInUser.GetOrgID())
 	if err != nil {
-		return ErrResp(http.StatusInternalServerError, err, "")
+		return response.ErrOrFallback(http.StatusInternalServerError, "failed to get mute timing by name", err)
 	}
-	for _, timing := range timings {
-		if name == timing.Name {
-			return response.JSON(http.StatusOK, timing)
-		}
-	}
-	return response.Empty(http.StatusNotFound)
+	return response.JSON(http.StatusOK, timing)
 }
 
 func (srv *ProvisioningSrv) RouteGetMuteTimingExport(c *contextmodel.ReqContext, name string) response.Response {
 	timings, err := srv.muteTimings.GetMuteTimings(c.Req.Context(), c.SignedInUser.GetOrgID())
 	if err != nil {
-		return ErrResp(http.StatusInternalServerError, err, "")
+		return response.ErrOrFallback(http.StatusInternalServerError, "failed to get mute timings", err)
 	}
 	for _, timing := range timings {
 		if name == timing.Name {
@@ -272,7 +268,7 @@ func (srv *ProvisioningSrv) RouteGetMuteTimingExport(c *contextmodel.ReqContext,
 func (srv *ProvisioningSrv) RouteGetMuteTimings(c *contextmodel.ReqContext) response.Response {
 	timings, err := srv.muteTimings.GetMuteTimings(c.Req.Context(), c.SignedInUser.GetOrgID())
 	if err != nil {
-		return ErrResp(http.StatusInternalServerError, err, "")
+		return response.ErrOrFallback(http.StatusInternalServerError, "failed to get mute timings", err)
 	}
 	return response.JSON(http.StatusOK, timings)
 }
@@ -280,7 +276,7 @@ func (srv *ProvisioningSrv) RouteGetMuteTimings(c *contextmodel.ReqContext) resp
 func (srv *ProvisioningSrv) RouteGetMuteTimingsExport(c *contextmodel.ReqContext) response.Response {
 	timings, err := srv.muteTimings.GetMuteTimings(c.Req.Context(), c.SignedInUser.GetOrgID())
 	if err != nil {
-		return ErrResp(http.StatusInternalServerError, err, "")
+		return response.ErrOrFallback(http.StatusInternalServerError, "failed to get mute timings", err)
 	}
 	e := AlertingFileExportFromMuteTimings(c.SignedInUser.GetOrgID(), timings)
 	return exportResponse(c, e)
@@ -290,10 +286,7 @@ func (srv *ProvisioningSrv) RoutePostMuteTiming(c *contextmodel.ReqContext, mt d
 	mt.Provenance = determineProvenance(c)
 	created, err := srv.muteTimings.CreateMuteTiming(c.Req.Context(), mt, c.SignedInUser.GetOrgID())
 	if err != nil {
-		if errors.Is(err, provisioning.ErrValidation) {
-			return ErrResp(http.StatusBadRequest, err, "")
-		}
-		return ErrResp(http.StatusInternalServerError, err, "")
+		return response.ErrOrFallback(http.StatusInternalServerError, "failed to create mute timing", err)
 	}
 	return response.JSON(http.StatusCreated, created)
 }
@@ -303,13 +296,7 @@ func (srv *ProvisioningSrv) RoutePutMuteTiming(c *contextmodel.ReqContext, mt de
 	mt.Provenance = determineProvenance(c)
 	updated, err := srv.muteTimings.UpdateMuteTiming(c.Req.Context(), mt, c.SignedInUser.GetOrgID())
 	if err != nil {
-		if errors.Is(err, provisioning.ErrValidation) {
-			return ErrResp(http.StatusBadRequest, err, "")
-		}
-		return ErrResp(http.StatusInternalServerError, err, "")
-	}
-	if updated == nil {
-		return response.Empty(http.StatusNotFound)
+		return response.ErrOrFallback(http.StatusInternalServerError, "failed to update mute timing", err)
 	}
 	return response.JSON(http.StatusAccepted, updated)
 }
@@ -317,7 +304,7 @@ func (srv *ProvisioningSrv) RoutePutMuteTiming(c *contextmodel.ReqContext, mt de
 func (srv *ProvisioningSrv) RouteDeleteMuteTiming(c *contextmodel.ReqContext, name string) response.Response {
 	err := srv.muteTimings.DeleteMuteTiming(c.Req.Context(), name, c.SignedInUser.GetOrgID())
 	if err != nil {
-		return ErrResp(http.StatusInternalServerError, err, "")
+		return response.ErrOrFallback(http.StatusInternalServerError, "failed to delete mute timing", err)
 	}
 	return response.JSON(http.StatusNoContent, nil)
 }

pkg/services/ngalert/api/tooling/api.json

@@ -1281,6 +1281,14 @@
    "title": "Frames is a slice of Frame pointers.",
    "type": "array"
   },
+  "GenericPublicError": {
+   "properties": {
+    "body": {
+     "$ref": "#/definitions/PublicError"
+    }
+   },
+   "type": "object"
+  },
   "GettableAlertmanagers": {
    "properties": {
     "data": {
@@ -4405,6 +4413,7 @@
    "type": "object"
   },
   "alertGroup": {
+   "description": "AlertGroup alert group",
    "properties": {
     "alerts": {
      "description": "alerts",
@@ -4533,7 +4542,6 @@
    "type": "object"
   },
   "gettableAlert": {
-   "description": "GettableAlert gettable alert",
    "properties": {
     "annotations": {
      "$ref": "#/definitions/labelSet"
@@ -5641,6 +5649,12 @@
     "responses": {
      "204": {
       "description": " The mute timing was deleted successfully."
+     },
+     "409": {
+      "description": "GenericPublicError",
+      "schema": {
+       "$ref": "#/definitions/GenericPublicError"
+      }
      }
     },
     "summary": "Delete a mute timing.",

pkg/services/ngalert/api/tooling/definitions/provisioning_mute_timings.go

@@ -63,6 +63,7 @@ import (
 //
 //     Responses:
 //       204: description: The mute timing was deleted successfully.
+//       409: GenericPublicError
 
 // swagger:route
 

pkg/services/ngalert/api/tooling/definitions/shared.go

@@ -20,3 +20,10 @@ type ForbiddenError struct {
 	// in: body
 	Body errutil.PublicError `json:"body"`
 }
+
+// swagger:model
+type GenericPublicError struct {
+	// The response message
+	// in: body
+	Body errutil.PublicError `json:"body"`
+}

pkg/services/ngalert/api/tooling/post.json

@@ -1281,6 +1281,14 @@
    "title": "Frames is a slice of Frame pointers.",
    "type": "array"
   },
+  "GenericPublicError": {
+   "properties": {
+    "body": {
+     "$ref": "#/definitions/PublicError"
+    }
+   },
+   "type": "object"
+  },
   "GettableAlertmanagers": {
    "properties": {
     "data": {
@@ -4590,13 +4598,13 @@
    "type": "object"
   },
   "gettableAlerts": {
+   "description": "GettableAlerts gettable alerts",
    "items": {
     "$ref": "#/definitions/gettableAlert"
    },
    "type": "array"
   },
   "gettableSilence": {
-   "description": "GettableSilence gettable silence",
    "properties": {
     "comment": {
      "description": "comment",
@@ -7624,6 +7632,12 @@
     "responses": {
      "204": {
       "description": " The mute timing was deleted successfully."
+     },
+     "409": {
+      "description": "GenericPublicError",
+      "schema": {
+       "$ref": "#/definitions/GenericPublicError"
+      }
      }
     },
     "summary": "Delete a mute timing.",

pkg/services/ngalert/api/tooling/spec.json

@@ -2785,6 +2785,12 @@
         "responses": {
           "204": {
             "description": " The mute timing was deleted successfully."
+          },
+          "409": {
+            "description": "GenericPublicError",
+            "schema": {
+              "$ref": "#/definitions/GenericPublicError"
+            }
           }
         }
       }
@@ -4688,6 +4694,14 @@
         "$ref": "#/definitions/Frame"
       }
     },
+    "GenericPublicError": {
+      "type": "object",
+      "properties": {
+        "body": {
+          "$ref": "#/definitions/PublicError"
+        }
+      }
+    },
     "GettableAlertmanagers": {
       "type": "object",
       "properties": {
@@ -8001,6 +8015,7 @@
       "$ref": "#/definitions/gettableAlert"
     },
     "gettableAlerts": {
+      "description": "GettableAlerts gettable alerts",
       "type": "array",
       "items": {
         "$ref": "#/definitions/gettableAlert"
@@ -8008,7 +8023,6 @@
       "$ref": "#/definitions/gettableAlerts"
     },
     "gettableSilence": {
-      "description": "GettableSilence gettable silence",
       "type": "object",
       "required": [
         "comment",

pkg/services/ngalert/provisioning/errors.go

@@ -14,6 +14,11 @@ var ErrPermissionDenied = errors.New("permission denied")
 var (
 	ErrNoAlertmanagerConfiguration  = errutil.Internal("alerting.notification.configMissing", errutil.WithPublicMessage("No alertmanager configuration present in this organization"))
 	ErrBadAlertmanagerConfiguration = errutil.Internal("alerting.notification.configCorrupted").MustTemplate("Failed to unmarshal the Alertmanager configuration", errutil.WithPublic("Current Alertmanager configuration in the storage is corrupted. Reset the configuration or rollback to a recent valid one."))
+
+	ErrTimeIntervalNotFound = errutil.NotFound("alerting.notifications.time-intervals.notFound")
+	ErrTimeIntervalExists   = errutil.BadRequest("alerting.notifications.time-intervals.nameExists", errutil.WithPublicMessage("Time interval with this name already exists. Use a different name or update existing one."))
+	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"))
 )
 
 func makeErrBadAlertmanagerConfiguration(err error) error {
@@ -23,6 +28,17 @@ func makeErrBadAlertmanagerConfiguration(err error) error {
 		},
 		Error: err,
 	}
-
 	return ErrBadAlertmanagerConfiguration.Build(data)
 }
+
+// MakeErrTimeIntervalInvalid creates an error with the ErrTimeIntervalInvalid template
+func MakeErrTimeIntervalInvalid(err error) error {
+	data := errutil.TemplateData{
+		Public: map[string]interface{}{
+			"Error": err.Error(),
+		},
+		Error: err,
+	}
+
+	return ErrTimeIntervalInvalid.Build(data)
+}

pkg/services/ngalert/provisioning/mute_timings.go

@@ -2,7 +2,6 @@ package provisioning
 
 import (
 	"context"
-	"fmt"
 
 	"github.com/prometheus/alertmanager/config"
 
@@ -45,15 +44,40 @@ func (svc *MuteTimingService) GetMuteTimings(ctx context.Context, orgID int64) (
 	return result, nil
 }
 
+// GetMuteTiming returns a mute timing by name
+func (svc *MuteTimingService) GetMuteTiming(ctx context.Context, name string, orgID int64) (definitions.MuteTimeInterval, error) {
+	rev, err := svc.configStore.Get(ctx, orgID)
+	if err != nil {
+		return definitions.MuteTimeInterval{}, err
+	}
+
+	mt, _, err := getMuteTiming(rev, name)
+	if err != nil {
+		return definitions.MuteTimeInterval{}, err
+	}
+
+	result := definitions.MuteTimeInterval{
+		MuteTimeInterval: mt,
+	}
+
+	_, err = svc.provenanceStore.GetProvenance(ctx, &result, orgID)
+	if err != nil {
+		return definitions.MuteTimeInterval{}, err
+	}
+	// TODO uncomment in a follow up
+	// result.Provenance = definitions.Provenance(prov)
+	return result, nil
+}
+
 // CreateMuteTiming adds a new mute timing within the specified org. The created mute timing is returned.
-func (svc *MuteTimingService) CreateMuteTiming(ctx context.Context, mt definitions.MuteTimeInterval, orgID int64) (*definitions.MuteTimeInterval, error) {
+func (svc *MuteTimingService) CreateMuteTiming(ctx context.Context, mt definitions.MuteTimeInterval, orgID int64) (definitions.MuteTimeInterval, error) {
 	if err := mt.Validate(); err != nil {
-		return nil, fmt.Errorf("%w: %s", ErrValidation, err.Error())
+		return definitions.MuteTimeInterval{}, MakeErrTimeIntervalInvalid(err)
 	}
 
 	revision, err := svc.configStore.Get(ctx, orgID)
 	if err != nil {
-		return nil, err
+		return definitions.MuteTimeInterval{}, err
 	}
 
 	if revision.cfg.AlertmanagerConfig.MuteTimeIntervals == nil {
@@ -61,7 +85,7 @@ func (svc *MuteTimingService) CreateMuteTiming(ctx context.Context, mt definitio
 	}
 	for _, existing := range revision.cfg.AlertmanagerConfig.MuteTimeIntervals {
 		if mt.Name == existing.Name {
-			return nil, fmt.Errorf("%w: %s", ErrValidation, "a mute timing with this name already exists")
+			return definitions.MuteTimeInterval{}, ErrTimeIntervalExists.Errorf("")
 		}
 	}
 	revision.cfg.AlertmanagerConfig.MuteTimeIntervals = append(revision.cfg.AlertmanagerConfig.MuteTimeIntervals, mt.MuteTimeInterval)
@@ -73,37 +97,34 @@ func (svc *MuteTimingService) CreateMuteTiming(ctx context.Context, mt definitio
 		return svc.provenanceStore.SetProvenance(ctx, &mt, orgID, models.Provenance(mt.Provenance))
 	})
 	if err != nil {
-		return nil, err
+		return definitions.MuteTimeInterval{}, err
 	}
-	return &mt, nil
+	return mt, nil
 }
 
-// UpdateMuteTiming replaces an existing mute timing within the specified org. The replaced mute timing is returned. If the mute timing does not exist, nil is returned and no action is taken.
-func (svc *MuteTimingService) UpdateMuteTiming(ctx context.Context, mt definitions.MuteTimeInterval, orgID int64) (*definitions.MuteTimeInterval, error) {
+// UpdateMuteTiming replaces an existing mute timing within the specified org. The replaced mute timing is returned. If the mute timing does not exist, ErrMuteTimingsNotFound is returned.
+func (svc *MuteTimingService) UpdateMuteTiming(ctx context.Context, mt definitions.MuteTimeInterval, orgID int64) (definitions.MuteTimeInterval, error) {
 	if err := mt.Validate(); err != nil {
-		return nil, fmt.Errorf("%w: %s", ErrValidation, err.Error())
+		return definitions.MuteTimeInterval{}, MakeErrTimeIntervalInvalid(err)
 	}
 
 	revision, err := svc.configStore.Get(ctx, orgID)
 	if err != nil {
-		return nil, err
+		return definitions.MuteTimeInterval{}, err
 	}
 
 	if revision.cfg.AlertmanagerConfig.MuteTimeIntervals == nil {
-		return nil, nil
-	}
-	updated := false
-	for i, existing := range revision.cfg.AlertmanagerConfig.MuteTimeIntervals {
-		if mt.Name == existing.Name {
-			revision.cfg.AlertmanagerConfig.MuteTimeIntervals[i] = mt.MuteTimeInterval
-			updated = true
-			break
-		}
-	}
-	if !updated {
-		return nil, nil
+		return definitions.MuteTimeInterval{}, nil
 	}
 
+	_, idx, err := getMuteTiming(revision, mt.Name)
+	if err != nil {
+		return definitions.MuteTimeInterval{}, err
+	}
+	revision.cfg.AlertmanagerConfig.MuteTimeIntervals[idx] = mt.MuteTimeInterval
+
+	// TODO add diff and noop detection
+	// TODO add fail if different provenance
 	err = svc.xact.InTransaction(ctx, func(ctx context.Context) error {
 		if err := svc.configStore.Save(ctx, revision, orgID); err != nil {
 			return err
@@ -111,9 +132,9 @@ func (svc *MuteTimingService) UpdateMuteTiming(ctx context.Context, mt definitio
 		return svc.provenanceStore.SetProvenance(ctx, &mt, orgID, models.Provenance(mt.Provenance))
 	})
 	if err != nil {
-		return nil, err
+		return definitions.MuteTimeInterval{}, err
 	}
-	return &mt, err
+	return mt, err
 }
 
 // DeleteMuteTiming deletes the mute timing with the given name in the given org. If the mute timing does not exist, no error is returned.
@@ -127,7 +148,7 @@ func (svc *MuteTimingService) DeleteMuteTiming(ctx context.Context, name string,
 		return nil
 	}
 	if isMuteTimeInUse(name, []*definitions.Route{revision.cfg.AlertmanagerConfig.Route}) {
-		return fmt.Errorf("mute time '%s' is currently used by a notification policy", name)
+		return ErrTimeIntervalInUse.Errorf("")
 	}
 	for i, existing := range revision.cfg.AlertmanagerConfig.MuteTimeIntervals {
 		if name == existing.Name {
@@ -161,3 +182,15 @@ func isMuteTimeInUse(name string, routes []*definitions.Route) bool {
 	}
 	return false
 }
+
+func getMuteTiming(rev *cfgRevision, name string) (config.MuteTimeInterval, int, error) {
+	if rev.cfg.AlertmanagerConfig.MuteTimeIntervals == nil {
+		return config.MuteTimeInterval{}, -1, ErrTimeIntervalNotFound.Errorf("")
+	}
+	for idx, mt := range rev.cfg.AlertmanagerConfig.MuteTimeIntervals {
+		if mt.Name == name {
+			return mt, idx, nil
+		}
+	}
+	return config.MuteTimeInterval{}, -1, ErrTimeIntervalNotFound.Errorf("")
+}

pkg/services/ngalert/provisioning/mute_timings_test.go

@@ -91,6 +91,94 @@ func TestGetMuteTimings(t *testing.T) {
 	})
 }
 
+func TestGetMuteTiming(t *testing.T) {
+	orgID := int64(1)
+	revision := &cfgRevision{
+		cfg: &definitions.PostableUserConfig{
+			AlertmanagerConfig: definitions.PostableApiAlertingConfig{
+				Config: definitions.Config{
+					MuteTimeIntervals: []config.MuteTimeInterval{
+						{
+							Name:          "Test1",
+							TimeIntervals: nil,
+						},
+					},
+				},
+			},
+		},
+	}
+
+	t.Run("service returns timing by name", func(t *testing.T) {
+		sut, store, prov := createMuteTimingSvcSut()
+		store.GetFn = func(ctx context.Context, orgID int64) (*cfgRevision, error) {
+			return revision, nil
+		}
+		prov.EXPECT().GetProvenance(mock.Anything, mock.Anything, mock.Anything).Return(models.ProvenanceAPI, nil)
+
+		result, err := sut.GetMuteTiming(context.Background(), "Test1", orgID)
+
+		require.NoError(t, err)
+
+		require.Equal(t, "Test1", result.Name)
+		require.EqualValues(t, "", result.Provenance) // TODO this is bug and that's how it works now. Fix it in follow up
+
+		require.Len(t, store.Calls, 1)
+		require.Equal(t, "Get", store.Calls[0].Method)
+		require.Equal(t, orgID, store.Calls[0].Args[1])
+
+		prov.AssertCalled(t, "GetProvenance", mock.Anything, &result, orgID)
+	})
+
+	t.Run("service returns ErrTimeIntervalNotFound if no mute timings", func(t *testing.T) {
+		sut, store, _ := createMuteTimingSvcSut()
+		store.GetFn = func(ctx context.Context, orgID int64) (*cfgRevision, error) {
+			return &cfgRevision{cfg: &definitions.PostableUserConfig{}}, nil
+		}
+
+		_, err := sut.GetMuteTiming(context.Background(), "Test1", orgID)
+
+		require.Truef(t, ErrTimeIntervalNotFound.Is(err), "expected ErrTimeIntervalNotFound but got %s", err)
+	})
+
+	t.Run("service returns ErrTimeIntervalNotFound if no mute timing by name", func(t *testing.T) {
+		sut, store, _ := createMuteTimingSvcSut()
+		store.GetFn = func(ctx context.Context, orgID int64) (*cfgRevision, error) {
+			return revision, nil
+		}
+
+		_, err := sut.GetMuteTiming(context.Background(), "Test123", orgID)
+
+		require.Truef(t, ErrTimeIntervalNotFound.Is(err), "expected ErrTimeIntervalNotFound but got %s", err)
+	})
+
+	t.Run("service propagates errors", func(t *testing.T) {
+		t.Run("when unable to read config", func(t *testing.T) {
+			sut, store, _ := createMuteTimingSvcSut()
+			expected := fmt.Errorf("failed")
+			store.GetFn = func(ctx context.Context, orgID int64) (*cfgRevision, error) {
+				return nil, expected
+			}
+
+			_, err := sut.GetMuteTiming(context.Background(), "Test1", orgID)
+
+			require.ErrorIs(t, err, expected)
+		})
+
+		t.Run("when unable to read provenance", func(t *testing.T) {
+			sut, store, prov := createMuteTimingSvcSut()
+			store.GetFn = func(ctx context.Context, orgID int64) (*cfgRevision, error) {
+				return revision, nil
+			}
+			expected := fmt.Errorf("failed")
+			prov.EXPECT().GetProvenance(mock.Anything, mock.Anything, mock.Anything).Return("", expected)
+
+			_, err := sut.GetMuteTiming(context.Background(), "Test1", orgID)
+
+			require.ErrorIs(t, err, expected)
+		})
+	})
+}
+
 func TestCreateMuteTimings(t *testing.T) {
 	orgID := int64(1)
 
@@ -128,7 +216,7 @@ func TestCreateMuteTimings(t *testing.T) {
 		Provenance:       definitions.Provenance(expectedProvenance),
 	}
 
-	t.Run("returns error if mute timings fail validation", func(t *testing.T) {
+	t.Run("returns ErrTimeIntervalInvalid if mute timings fail validation", func(t *testing.T) {
 		sut, _, _ := createMuteTimingSvcSut()
 		timing := definitions.MuteTimeInterval{
 			MuteTimeInterval: config.MuteTimeInterval{
@@ -139,10 +227,10 @@ func TestCreateMuteTimings(t *testing.T) {
 
 		_, err := sut.CreateMuteTiming(context.Background(), timing, orgID)
 
-		require.ErrorIs(t, err, ErrValidation)
+		require.Truef(t, ErrTimeIntervalInvalid.Base.Is(err), "expected ErrTimeIntervalInvalid but got %s", err)
 	})
 
-	t.Run("returns error if mute timing with the name exists", func(t *testing.T) {
+	t.Run("returns ErrTimeIntervalExists if mute timing with the name exists", func(t *testing.T) {
 		sut, store, _ := createMuteTimingSvcSut()
 		store.GetFn = func(ctx context.Context, orgID int64) (*cfgRevision, error) {
 			return &cfgRevision{cfg: initialConfig()}, nil
@@ -156,7 +244,7 @@ func TestCreateMuteTimings(t *testing.T) {
 
 		_, err := sut.CreateMuteTiming(context.Background(), timing, orgID)
 
-		require.ErrorContains(t, err, "a mute timing with this name already exists")
+		require.Truef(t, ErrTimeIntervalExists.Is(err), "expected ErrTimeIntervalExists but got %s", err)
 	})
 
 	t.Run("saves mute timing and provenance in a transaction", func(t *testing.T) {
@@ -297,11 +385,11 @@ func TestUpdateMuteTimings(t *testing.T) {
 
 		_, err := sut.UpdateMuteTiming(context.Background(), timing, orgID)
 
-		require.ErrorIs(t, err, ErrValidation)
+		require.Truef(t, ErrTimeIntervalInvalid.Base.Is(err), "expected ErrTimeIntervalInvalid but got %s", err)
 	})
 
-	t.Run("returns nil if mute timing does not exist", func(t *testing.T) {
-		sut, store, prov := createMuteTimingSvcSut()
+	t.Run("returns ErrMuteTimingsNotFound if mute timing does not exist", func(t *testing.T) {
+		sut, store, _ := createMuteTimingSvcSut()
 		store.GetFn = func(ctx context.Context, orgID int64) (*cfgRevision, error) {
 			return &cfgRevision{cfg: initialConfig()}, nil
 		}
@@ -313,15 +401,9 @@ func TestUpdateMuteTimings(t *testing.T) {
 			Provenance: definitions.Provenance(models.ProvenanceFile),
 		}
 
-		mt, err := sut.UpdateMuteTiming(context.Background(), timing, orgID)
-		require.NoError(t, err)
-		require.Nil(t, mt)
+		_, err := sut.UpdateMuteTiming(context.Background(), timing, orgID)
 
-		require.Len(t, store.Calls, 1)
-		require.Equal(t, "Get", store.Calls[0].Method)
-		require.Equal(t, orgID, store.Calls[0].Args[1])
-
-		prov.AssertNotCalled(t, "SetProvenance", mock.Anything, &timing, orgID, expectedProvenance)
+		require.Truef(t, ErrTimeIntervalNotFound.Is(err), "expected ErrTimeIntervalNotFound but got %s", err)
 	})
 
 	t.Run("saves mute timing and provenance in a transaction", func(t *testing.T) {
@@ -461,7 +543,7 @@ func TestDeleteMuteTimings(t *testing.T) {
 		prov.AssertCalled(t, "DeleteProvenance", mock.Anything, &definitions.MuteTimeInterval{MuteTimeInterval: config.MuteTimeInterval{Name: "no-timing"}}, orgID)
 	})
 
-	t.Run("returns error if mute timing is used", func(t *testing.T) {
+	t.Run("returns ErrTimeIntervalInUse if mute timing is used", func(t *testing.T) {
 		sut, store, _ := createMuteTimingSvcSut()
 		store.GetFn = func(ctx context.Context, orgID int64) (*cfgRevision, error) {
 			return &cfgRevision{cfg: initialConfig()}, nil
@@ -469,11 +551,10 @@ func TestDeleteMuteTimings(t *testing.T) {
 
 		err := sut.DeleteMuteTiming(context.Background(), usedTiming, orgID)
 
-		require.ErrorContains(t, err, "is currently used by a notification policy")
-
 		require.Len(t, store.Calls, 1)
 		require.Equal(t, "Get", store.Calls[0].Method)
 		require.Equal(t, orgID, store.Calls[0].Args[1])
+		require.Truef(t, ErrTimeIntervalInUse.Is(err), "expected ErrTimeIntervalInUse but got %s", err)
 	})
 
 	t.Run("deletes mute timing and provenance in transaction", func(t *testing.T) {

pkg/tests/api/alerting/api_provisioning_test.go

@@ -21,6 +21,7 @@ import (
 	"github.com/grafana/grafana/pkg/services/org"
 	"github.com/grafana/grafana/pkg/services/user"
 	"github.com/grafana/grafana/pkg/tests/testinfra"
+	"github.com/grafana/grafana/pkg/util/errutil"
 )
 
 func TestIntegrationProvisioning(t *testing.T) {
@@ -491,9 +492,10 @@ func TestMuteTimings(t *testing.T) {
 		_, status, body := apiClient.CreateMuteTimingWithStatus(t, m)
 		t.Log(body)
 		requireStatusCode(t, http.StatusBadRequest, status, body)
-		var validationError map[string]any
+		var validationError errutil.PublicError
 		assert.NoError(t, json.Unmarshal([]byte(body), &validationError))
-		assert.Contains(t, validationError, "message")
+		assert.NotEmpty(t, validationError, validationError.Message)
+		assert.Equal(t, "alerting.notifications.time-intervals.nameExists", validationError.MessageID)
 		if t.Failed() {
 			t.Fatalf("response: %s", body)
 		}
@@ -585,7 +587,7 @@ func TestMuteTimings(t *testing.T) {
 		requireStatusCode(t, http.StatusNotFound, status, body)
 	})
 
-	t.Run("should get BadRequest if deletes used mute-timing", func(t *testing.T) {
+	t.Run("should get 409 Conflict if deletes used mute-timing", func(t *testing.T) {
 		route, status, response := apiClient.GetRouteWithStatus(t)
 		requireStatusCode(t, http.StatusOK, status, response)
 		route.Routes = append(route.Routes, &definitions.Route{
@@ -602,7 +604,14 @@ func TestMuteTimings(t *testing.T) {
 		requireStatusCode(t, http.StatusAccepted, status, response)
 
 		status, response = apiClient.DeleteMuteTimingWithStatus(t, anotherMuteTiming.Name)
-		requireStatusCode(t, http.StatusInternalServerError, status, response) // TODO should be bad request
+		requireStatusCode(t, http.StatusConflict, status, response)
+		var validationError errutil.PublicError
+		assert.NoError(t, json.Unmarshal([]byte(response), &validationError))
+		assert.NotEmpty(t, validationError, validationError.Message)
+		assert.Equal(t, "alerting.notifications.time-intervals.used", validationError.MessageID)
+		if t.Failed() {
+			t.Fatalf("response: %s", response)
+		}
 	})
 }
 

public/api-merged.json

@@ -3568,6 +3568,12 @@
         "responses": {
           "204": {
             "description": " The mute timing was deleted successfully."
+          },
+          "409": {
+            "description": "GenericPublicError",
+            "schema": {
+              "$ref": "#/definitions/GenericPublicError"
+            }
           }
         }
       }
@@ -14995,6 +15001,14 @@
         "$ref": "#/definitions/Frame"
       }
     },
+    "GenericPublicError": {
+      "type": "object",
+      "properties": {
+        "body": {
+          "$ref": "#/definitions/PublicError"
+        }
+      }
+    },
     "GetAnnotationTagsResponse": {
       "type": "object",
       "title": "GetAnnotationTagsResponse is a response struct for FindTagsResult.",
@@ -21364,6 +21378,7 @@
       }
     },
     "alertGroup": {
+      "description": "AlertGroup alert group",
       "type": "object",
       "required": [
         "alerts",
@@ -21520,7 +21535,6 @@
       }
     },
     "gettableAlert": {
-      "description": "GettableAlert gettable alert",
       "type": "object",
       "required": [
         "labels",

public/openapi3.json

@@ -5532,6 +5532,14 @@
         "title": "Frames is a slice of Frame pointers.",
         "type": "array"
       },
+      "GenericPublicError": {
+        "properties": {
+          "body": {
+            "$ref": "#/components/schemas/PublicError"
+          }
+        },
+        "type": "object"
+      },
       "GetAnnotationTagsResponse": {
         "properties": {
           "result": {
@@ -11899,6 +11907,7 @@
         "type": "object"
       },
       "alertGroup": {
+        "description": "AlertGroup alert group",
         "properties": {
           "alerts": {
             "description": "alerts",
@@ -12055,7 +12064,6 @@
         "type": "object"
       },
       "gettableAlert": {
-        "description": "GettableAlert gettable alert",
         "properties": {
           "annotations": {
             "$ref": "#/components/schemas/labelSet"
@@ -16456,6 +16464,16 @@
         "responses": {
           "204": {
             "description": " The mute timing was deleted successfully."
+          },
+          "409": {
+            "content": {
+              "application/json": {
+                "schema": {
+                  "$ref": "#/components/schemas/GenericPublicError"
+                }
+              }
+            },
+            "description": "GenericPublicError"
           }
         },
         "summary": "Delete a mute timing.",