From 06a8700472abb7a7f6d68207140b8ced092c455e Mon Sep 17 00:00:00 2001 From: Fabian Reinartz Date: Fri, 10 Jun 2016 10:51:41 +0200 Subject: [PATCH] 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 | 22 ++-- provider/mesh/state.go | 32 ++++-- provider/mesh/state_test.go | 221 ++++++++++++++++++++++++++++++++++++ 3 files changed, 254 insertions(+), 21 deletions(-) diff --git a/provider/mesh/peer_test.go b/provider/mesh/peer_test.go index a7b7601b..e39f5829 100644 --- a/provider/mesh/peer_test.go +++ b/provider/mesh/peer_test.go @@ -303,12 +303,8 @@ func TestNotificationInfosGet(t *testing.T) { func TestSilencesSet(t *testing.T) { var ( - t0 = time.Now() - t1 = t0.Add(10 * time.Minute) - now = time.Now() - - 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, - EndsAt: t1, + StartsAt: now.Add(time.Minute), + 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, - EndsAt: t1, + StartsAt: now.Add(time.Minute), + 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 { - t.Errorf("Silence ID unexpectedly changed") + if beforeID != uuid.Nil && uid != beforeID { + t.Errorf("Silence ID unexpectedly changed: before %q, after %q", beforeID, uid) continue } diff --git a/provider/mesh/state.go b/provider/mesh/state.go index f9683b38..d117b284 100644 --- a/provider/mesh/state.go +++ b/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 } diff --git a/provider/mesh/state_test.go b/provider/mesh/state_test.go index 50e67720..dcc40650 100644 --- a/provider/mesh/state_test.go +++ b/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) + } + } +}