Alerting: Check if TimeInterval is used in ActiveTimings when deleting (#110691)

* check for active timing in route

* Update test

* Add integration test

pkg/services/ngalert/provisioning/mute_timings.go

@@ -247,7 +247,7 @@ func (svc *MuteTimingService) DeleteMuteTiming(ctx context.Context, nameOrUID st
 		return err
 	}
 
-	if isMuteTimeInUseInRoutes(existing.Name, revision.Config.AlertmanagerConfig.Route) {
+	if isTimeIntervalInUseInRoutes(existing.Name, revision.Config.AlertmanagerConfig.Route) {
 		ns, _ := svc.ruleNotificationsStore.ListNotificationSettings(ctx, models.ListNotificationSettingsQuery{OrgID: orgID, TimeIntervalName: existing.Name})
 		// ignore error here because it's not important
 		return MakeErrTimeIntervalInUse(true, maps.Keys(ns))
@@ -275,15 +275,20 @@ func (svc *MuteTimingService) DeleteMuteTiming(ctx context.Context, nameOrUID st
 	})
 }
 
-func isMuteTimeInUseInRoutes(name string, route *definitions.Route) bool {
+func isTimeIntervalInUseInRoutes(name string, route *definitions.Route) bool {
 	if route == nil {
 		return false
 	}
 	if slices.Contains(route.MuteTimeIntervals, name) {
 		return true
 	}
+
+	if slices.Contains(route.ActiveTimeIntervals, name) {
+		return true
+	}
+
 	for _, route := range route.Routes {
-		if isMuteTimeInUseInRoutes(name, route) {
+		if isTimeIntervalInUseInRoutes(name, route) {
 			return true
 		}
 	}

pkg/services/ngalert/provisioning/mute_timings_test.go

@@ -765,8 +765,8 @@ func TestUpdateMuteTimings(t *testing.T) {
 
 		revision := store.Calls[1].Args[1].(*legacy_storage.ConfigRevision)
 		assert.EqualValues(t, append(initialConfig().AlertmanagerConfig.TimeIntervals, config.TimeInterval(interval)), revision.Config.AlertmanagerConfig.TimeIntervals)
-		assert.Falsef(t, isMuteTimeInUseInRoutes(expected.Name, revision.Config.AlertmanagerConfig.Route), "There are still references to the old time interval")
-		assert.Truef(t, isMuteTimeInUseInRoutes(interval.Name, revision.Config.AlertmanagerConfig.Route), "There are no references to the new time interval")
+		assert.Falsef(t, isTimeIntervalInUseInRoutes(expected.Name, revision.Config.AlertmanagerConfig.Route), "There are still references to the old time interval")
+		assert.Truef(t, isTimeIntervalInUseInRoutes(interval.Name, revision.Config.AlertmanagerConfig.Route), "There are no references to the new time interval")
 	})
 
 	t.Run("returns ErrTimeIntervalDependentResourcesProvenance if route has different provenance status", func(t *testing.T) {
@@ -942,22 +942,27 @@ func TestDeleteMuteTimings(t *testing.T) {
 
 	timingToDelete := config.MuteTimeInterval{Name: "unused-timing"}
 	correctVersion := calculateMuteTimeIntervalFingerprint(timingToDelete)
-	usedTiming := "used-timing"
+	usedMuteTiming := "used-timing"
+	usedActiveTiming := "used-active-timing"
 	initialConfig := func() *definitions.PostableUserConfig {
 		return &definitions.PostableUserConfig{
 			TemplateFiles: nil,
 			AlertmanagerConfig: definitions.PostableApiAlertingConfig{
 				Config: definitions.Config{
 					Route: &definitions.Route{
-						MuteTimeIntervals: []string{usedTiming},
+						MuteTimeIntervals:   []string{usedMuteTiming},
+						ActiveTimeIntervals: []string{usedActiveTiming},
 					},
 					MuteTimeIntervals: []config.MuteTimeInterval{
 						{
-							Name: usedTiming,
+							Name: usedMuteTiming,
 						},
 						timingToDelete,
 					},
 					TimeIntervals: []config.TimeInterval{
+						{
+							Name: usedActiveTiming,
+						},
 						{
 							Name: "timing-to-delete2",
 						},
@@ -990,7 +995,22 @@ func TestDeleteMuteTimings(t *testing.T) {
 		}
 		prov.EXPECT().GetProvenance(mock.Anything, mock.Anything, mock.Anything).Return(models.ProvenanceAPI, nil)
 
-		err := sut.DeleteMuteTiming(context.Background(), usedTiming, orgID, definitions.Provenance(models.ProvenanceAPI), correctVersion)
+		err := sut.DeleteMuteTiming(context.Background(), usedMuteTiming, orgID, definitions.Provenance(models.ProvenanceAPI), correctVersion)
+
+		require.Len(t, store.Calls, 1)
+		require.Equal(t, "Get", store.Calls[0].Method)
+		require.Equal(t, orgID, store.Calls[0].Args[1])
+		require.ErrorIs(t, err, ErrTimeIntervalInUse)
+	})
+
+	t.Run("returns ErrTimeIntervalInUse if active timing is used by a route", func(t *testing.T) {
+		sut, store, prov := createMuteTimingSvcSut()
+		store.GetFn = func(ctx context.Context, orgID int64) (*legacy_storage.ConfigRevision, error) {
+			return &legacy_storage.ConfigRevision{Config: initialConfig()}, nil
+		}
+		prov.EXPECT().GetProvenance(mock.Anything, mock.Anything, mock.Anything).Return(models.ProvenanceAPI, nil)
+
+		err := sut.DeleteMuteTiming(context.Background(), usedActiveTiming, orgID, definitions.Provenance(models.ProvenanceAPI), correctVersion)
 
 		require.Len(t, store.Calls, 1)
 		require.Equal(t, "Get", store.Calls[0].Method)
@@ -1110,7 +1130,7 @@ func TestDeleteMuteTimings(t *testing.T) {
 				return nil
 			})
 
-		timingToDelete := initialConfig().AlertmanagerConfig.TimeIntervals[0]
+		timingToDelete := initialConfig().AlertmanagerConfig.TimeIntervals[1]
 		correctVersion := calculateMuteTimeIntervalFingerprint(config.MuteTimeInterval(timingToDelete))
 
 		err := sut.DeleteMuteTiming(context.Background(), timingToDelete.Name, orgID, "", correctVersion)

pkg/tests/apis/alerting/notifications/timeinterval/test-data/notification-settings.json

@@ -14,10 +14,13 @@
             ]
           ],
           "mute_time_intervals": [
-            "test-interval", "persisted-interval"
+            "test-interval",
+            "persisted-interval"
           ],
           "active_time_intervals": [
-            "test-interval", "persisted-interval"
+            "test-interval",
+            "persisted-interval",
+            "test-interval-for-active-time-interval"
           ]
         }
       ]
@@ -40,6 +43,15 @@
             "end_time": "23:59"
           }
         ]
+      },
+      {
+        "name": "test-interval-for-active-time-interval",
+        "time_intervals": [
+          {
+            "start_time": "06:00",
+            "end_time": "23:59"
+          }
+        ]
       }
     ],
     "receivers": [

pkg/tests/apis/alerting/notifications/timeinterval/timeinterval_test.go

@@ -675,7 +675,7 @@ func TestIntegrationTimeIntervalReferentialIntegrity(t *testing.T) {
 
 	intervals, err := adminClient.List(ctx, v1.ListOptions{})
 	require.NoError(t, err)
-	require.Len(t, intervals.Items, 2)
+	require.Len(t, intervals.Items, 3)
 	intervalIdx := slices.IndexFunc(intervals.Items, func(interval v0alpha1.TimeInterval) bool {
 		return interval.Spec.Name == "test-interval"
 	})
@@ -764,6 +764,16 @@ func TestIntegrationTimeIntervalReferentialIntegrity(t *testing.T) {
 			err = adminClient.Delete(ctx, interval.Name, v1.DeleteOptions{})
 			require.Truef(t, errors.IsConflict(err), "Expected Conflict, got: %s", err)
 		})
+
+		t.Run("should fail to delete if time interval is used in route as an active time interval", func(t *testing.T) {
+			idx := slices.IndexFunc(intervals.Items, func(interval v0alpha1.TimeInterval) bool {
+				return interval.Spec.Name == "test-interval-for-active-time-interval"
+			})
+			intervalToDelete := intervals.Items[idx]
+
+			err = adminClient.Delete(ctx, intervalToDelete.Name, v1.DeleteOptions{})
+			require.Truef(t, errors.IsConflict(err), "Expected Conflict, got: %s", err)
+		})
 	})
 }