provider/mesh: fix and test modification validity

This change fixes, extends, and tests silenceModAllowed. It now can also
be applied to new silences.

provider/mesh/peer_test.go

@@ -303,12 +303,8 @@ func TestNotificationInfosGet(t *testing.T) {
 
 func TestSilencesSet(t *testing.T) {
 	var (
-		t0  = time.Now()
+		now      = time.Now()
-		t1  = t0.Add(10 * time.Minute)
+		id1      = uuid.NewV4()
-		now = time.Now()
-
-		id1 = uuid.NewV4()
-
 		matchers = types.NewMatchers(types.NewMatcher("a", "b"))
 	)
 	cases := []struct {
@@ -326,8 +322,8 @@ func TestSilencesSet(t *testing.T) {
 			input: &types.Silence{
 				ID:        id1,
 				Matchers:  matchers,
-				StartsAt:  t0,
+				StartsAt:  now.Add(time.Minute),
-				EndsAt:    t1,
+				EndsAt:    now.Add(time.Hour),
 				CreatedBy: "x",
 				Comment:   "x",
 			},
@@ -335,8 +331,8 @@ func TestSilencesSet(t *testing.T) {
 				id1: &types.Silence{
 					ID:        id1,
 					Matchers:  matchers,
-					StartsAt:  t0,
+					StartsAt:  now.Add(time.Minute),
-					EndsAt:    t1,
+					EndsAt:    now.Add(time.Hour),
 					UpdatedAt: now,
 					CreatedBy: "x",
 					Comment:   "x",
@@ -355,7 +351,7 @@ func TestSilencesSet(t *testing.T) {
 		beforeID := c.input.ID
 
 		uid, err := s.Set(c.input)
-		if err != nil && c.fail {
+		if err != nil {
 			if c.fail {
 				continue
 			}
@@ -367,8 +363,8 @@ func TestSilencesSet(t *testing.T) {
 			continue
 		}
 
-		if beforeID != uuid.Nil && uid != c.input.ID {
+		if beforeID != uuid.Nil && uid != beforeID {
-			t.Errorf("Silence ID unexpectedly changed")
+			t.Errorf("Silence ID unexpectedly changed: before %q, after %q", beforeID, uid)
 			continue
 		}
 

provider/mesh/state.go

@@ -134,9 +134,26 @@ func (st *silenceState) Encode() [][]byte {
 
 // silenceModAllowed checks whether silence a may be changed to silence b.
 // Returns an error stating the reason if not.
+// The silences are guaranteed to be valid. Silence a may be nil if b is a new.
 func silenceModAllowed(a, b *types.Silence, now time.Time) error {
+	if a == nil {
+		if b.StartsAt.Before(now) {
+			// From b being valid it follows that EndsAt will also be
+			// in the future.
+			return fmt.Errorf("new silence may not start in the past")
+		}
+		return nil
+	}
+	if a.ID != b.ID {
+		return fmt.Errorf("IDs do not match")
+	}
 	if !b.StartsAt.Equal(a.StartsAt) {
-		return fmt.Errorf("silence start time must not be modified")
+		if a.StartsAt.Before(now) {
+			return fmt.Errorf("start time of active silence must not be modified")
+		}
+		if b.StartsAt.Before(now) {
+			return fmt.Errorf("start time cannot be moved into the past")
+		}
 	}
 	if a.EndsAt.Before(now) {
 		return fmt.Errorf("end time must not be modified for elapsed silence")
@@ -157,15 +174,15 @@ func (st *silenceState) set(s *types.Silence) error {
 	now := st.now()
 	s.UpdatedAt = now
 
+	prev, ok := st.m[s.ID]
+	// Silence start for new silences must not be before now.
+	// Simplest solution is to reset it here if necessary.
+	if !ok && s.StartsAt.Before(now) {
+		s.StartsAt = now
+	}
 	if err := s.Validate(); err != nil {
 		return err
 	}
-
-	prev, ok := st.m[s.ID]
-	if !ok {
-		st.m[s.ID] = s
-		return nil
-	}
 	if err := silenceModAllowed(prev, s, now); err != nil {
 		return err
 	}
@@ -191,7 +208,6 @@ func (st *silenceState) del(id uuid.UUID) (*types.Silence, error) {
 	if err := sil.Validate(); err != nil {
 		return nil, err
 	}
-
 	if err := silenceModAllowed(prev, &sil, now); err != nil {
 		return nil, err
 	}

provider/mesh/state_test.go

@@ -57,6 +57,29 @@ func TestSilenceStateSet(t *testing.T) {
 				CreatedBy: "x",
 				Comment:   "x",
 			},
+		}, {
+			initial: map[uuid.UUID]*types.Silence{},
+			final: map[uuid.UUID]*types.Silence{
+				id1: &types.Silence{
+					ID:       id1,
+					Matchers: matchers,
+					// StartsAt should be reset to now if it's before
+					// now for a new silence.
+					StartsAt:  now,
+					EndsAt:    now.Add(time.Hour),
+					UpdatedAt: now,
+					CreatedBy: "x",
+					Comment:   "x",
+				},
+			},
+			input: &types.Silence{
+				ID:        id1,
+				Matchers:  matchers,
+				StartsAt:  now.Add(-time.Second),
+				EndsAt:    now.Add(time.Hour),
+				CreatedBy: "x",
+				Comment:   "x",
+			},
 		}, {
 			initial: map[uuid.UUID]*types.Silence{
 				id1: &types.Silence{
@@ -250,3 +273,201 @@ func TestSilenceStateDel(t *testing.T) {
 		}
 	}
 }
+
+func TestSilenceModAllowed(t *testing.T) {
+	var (
+		now      = time.Now()
+		id1      = uuid.NewV4()
+		matchers = types.NewMatchers(types.NewMatcher("a", "b"))
+	)
+	cases := []struct {
+		a, b *types.Silence
+		err  string
+	}{
+		{
+			a: nil,
+			b: &types.Silence{
+				ID:        id1,
+				Matchers:  matchers,
+				StartsAt:  now.Add(1 * time.Minute),
+				EndsAt:    now.Add(5 * time.Minute),
+				UpdatedAt: now,
+				CreatedBy: "x",
+				Comment:   "x",
+			},
+		},
+		{
+			a: &types.Silence{
+				ID:        id1,
+				Matchers:  matchers,
+				StartsAt:  now.Add(-10 * time.Minute),
+				EndsAt:    now.Add(5 * time.Minute),
+				UpdatedAt: now.Add(-10 * time.Minute),
+				CreatedBy: "x",
+				Comment:   "x",
+			},
+			b: &types.Silence{
+				ID:        id1,
+				Matchers:  matchers,
+				StartsAt:  now.Add(-10 * time.Minute),
+				EndsAt:    now.Add(100 * time.Minute),
+				UpdatedAt: now,
+				CreatedBy: "y",
+				Comment:   "y",
+			},
+		},
+		{
+			a: nil,
+			b: &types.Silence{
+				ID:        id1,
+				Matchers:  matchers,
+				StartsAt:  now.Add(-10 * time.Minute),
+				EndsAt:    now.Add(5 * time.Minute),
+				UpdatedAt: now,
+				CreatedBy: "x",
+				Comment:   "x",
+			},
+			err: "start in the past",
+		},
+		{
+			a: &types.Silence{
+				ID:        uuid.NewV4(),
+				Matchers:  matchers,
+				StartsAt:  now.Add(-10 * time.Minute),
+				EndsAt:    now.Add(-5 * time.Minute),
+				UpdatedAt: now.Add(-10 * time.Minute),
+				CreatedBy: "x",
+				Comment:   "x",
+			},
+			b: &types.Silence{
+				ID:        uuid.NewV4(),
+				Matchers:  matchers,
+				StartsAt:  now.Add(-10 * time.Minute),
+				EndsAt:    now.Add(-5 * time.Minute),
+				UpdatedAt: now.Add(-10 * time.Minute),
+				CreatedBy: "x",
+				Comment:   "x",
+			},
+			err: "IDs do not match",
+		},
+		{
+			a: &types.Silence{
+				ID:        id1,
+				Matchers:  matchers,
+				StartsAt:  now.Add(-10 * time.Minute),
+				EndsAt:    now.Add(5 * time.Minute),
+				UpdatedAt: now.Add(-10 * time.Minute),
+				CreatedBy: "x",
+				Comment:   "x",
+			},
+			b: &types.Silence{
+				ID:        id1,
+				Matchers:  matchers,
+				StartsAt:  now.Add(1 * time.Minute),
+				EndsAt:    now.Add(5 * time.Minute),
+				UpdatedAt: now,
+				CreatedBy: "x",
+				Comment:   "x",
+			},
+			err: "start time of active silence",
+		},
+		{
+			a: &types.Silence{
+				ID:        id1,
+				Matchers:  matchers,
+				StartsAt:  now.Add(1 * time.Minute),
+				EndsAt:    now.Add(5 * time.Minute),
+				UpdatedAt: now.Add(-10 * time.Minute),
+				CreatedBy: "x",
+				Comment:   "x",
+			},
+			b: &types.Silence{
+				ID:        id1,
+				Matchers:  matchers,
+				StartsAt:  now.Add(-1 * time.Minute),
+				EndsAt:    now.Add(5 * time.Minute),
+				UpdatedAt: now,
+				CreatedBy: "x",
+				Comment:   "x",
+			},
+			err: "start time cannot be moved into the past",
+		},
+		{
+			a: &types.Silence{
+				ID:        id1,
+				Matchers:  matchers,
+				StartsAt:  now.Add(-5 * time.Minute),
+				EndsAt:    now.Add(-1 * time.Minute),
+				UpdatedAt: now.Add(-10 * time.Minute),
+				CreatedBy: "x",
+				Comment:   "x",
+			},
+			b: &types.Silence{
+				ID:        id1,
+				Matchers:  matchers,
+				StartsAt:  now.Add(-5 * time.Minute),
+				EndsAt:    now.Add(5 * time.Minute),
+				UpdatedAt: now,
+				CreatedBy: "x",
+				Comment:   "x",
+			},
+			err: "end time must not be modified for elapsed silence",
+		},
+		{
+			a: &types.Silence{
+				ID:        id1,
+				Matchers:  matchers,
+				StartsAt:  now.Add(-5 * time.Minute),
+				EndsAt:    now.Add(5 * time.Minute),
+				UpdatedAt: now.Add(-10 * time.Minute),
+				CreatedBy: "x",
+				Comment:   "x",
+			},
+			b: &types.Silence{
+				ID:        id1,
+				Matchers:  matchers,
+				StartsAt:  now.Add(-5 * time.Minute),
+				EndsAt:    now.Add(-1 * time.Minute),
+				UpdatedAt: now,
+				CreatedBy: "x",
+				Comment:   "x",
+			},
+			err: "end time must not be in the past",
+		},
+		{
+			a: &types.Silence{
+				ID:        id1,
+				Matchers:  types.NewMatchers(types.NewMatcher("a", "b")),
+				StartsAt:  now.Add(-5 * time.Minute),
+				EndsAt:    now.Add(5 * time.Minute),
+				UpdatedAt: now.Add(-10 * time.Minute),
+				CreatedBy: "x",
+				Comment:   "x",
+			},
+			b: &types.Silence{
+				ID:        id1,
+				Matchers:  types.NewMatchers(types.NewMatcher("a", "c")),
+				StartsAt:  now.Add(-5 * time.Minute),
+				EndsAt:    now.Add(5 * time.Minute),
+				UpdatedAt: now,
+				CreatedBy: "x",
+				Comment:   "x",
+			},
+			err: "matchers must not be modified",
+		},
+	}
+	for _, c := range cases {
+		got := silenceModAllowed(c.a, c.b, now)
+		if got == nil {
+			if c.err != "" {
+				t.Errorf("Expected error containing %q but got none", c.err)
+			}
+			continue
+		}
+		if c.err == "" {
+			t.Errorf("Expected no error but got %q", got)
+		} else if !strings.Contains(got.Error(), c.err) {
+			t.Errorf("Expected error containing %q but got %q", c.err, got)
+		}
+	}
+}