From 76ee5388e76274180db5400640e7f952515004e5 Mon Sep 17 00:00:00 2001
From: pasquier-s <spasquie@redhat.com>
Date: Fri, 9 Feb 2018 10:53:46 +0100
Subject: [PATCH] Forbid 0 value for group_interval and repeat_interval (#1230)

Setting one of these parameters to a zero value doesn't make sense
semantically and can cause high CPU usage.
---
 config/config.go                |  7 +++++
 config/config_test.go           | 56 ++++++++++++++++++++++++++++-----
 test/acceptance/send_test.go    |  4 +--
 test/acceptance/silence_test.go |  4 +--
 4 files changed, 60 insertions(+), 11 deletions(-)

diff --git a/config/config.go b/config/config.go
index c25416f5..3b63cba5 100644
--- a/config/config.go
+++ b/config/config.go
@@ -421,6 +421,13 @@ func (r *Route) UnmarshalYAML(unmarshal func(interface{}) error) error {
 		groupBy[ln] = struct{}{}
 	}
 
+	if r.GroupInterval != nil && time.Duration(*r.GroupInterval) == time.Duration(0) {
+		return fmt.Errorf("group_interval cannot be zero")
+	}
+	if r.RepeatInterval != nil && time.Duration(*r.RepeatInterval) == time.Duration(0) {
+		return fmt.Errorf("repeat_interval cannot be zero")
+	}
+
 	return checkOverflow(r.XXX, "route")
 }
 
diff --git a/config/config_test.go b/config/config_test.go
index 47abba05..99c6d5e3 100644
--- a/config/config_test.go
+++ b/config/config_test.go
@@ -73,7 +73,7 @@ receivers:
 	expected := "notification config name \"team-X\" is not unique"
 
 	if err == nil {
-		t.Fatalf("no error returned, expeceted:\n%q", expected)
+		t.Fatalf("no error returned, expected:\n%q", expected)
 	}
 	if err.Error() != expected {
 		t.Errorf("\nexpected:\n%q\ngot:\n%q", expected, err.Error())
@@ -94,7 +94,7 @@ receivers:
 	expected := "undefined receiver \"team-X\" used in route"
 
 	if err == nil {
-		t.Fatalf("no error returned, expeceted:\n%q", expected)
+		t.Fatalf("no error returned, expected:\n%q", expected)
 	}
 	if err.Error() != expected {
 		t.Errorf("\nexpected:\n%q\ngot:\n%q", expected, err.Error())
@@ -114,7 +114,7 @@ receivers:
 	expected := "missing name in receiver"
 
 	if err == nil {
-		t.Fatalf("no error returned, expeceted:\n%q", expected)
+		t.Fatalf("no error returned, expected:\n%q", expected)
 	}
 	if err.Error() != expected {
 		t.Errorf("\nexpected:\n%q\ngot:\n%q", expected, err.Error())
@@ -135,7 +135,7 @@ receivers:
 	expected := "duplicated label \"cluster\" in group_by"
 
 	if err == nil {
-		t.Fatalf("no error returned, expeceted:\n%q", expected)
+		t.Fatalf("no error returned, expected:\n%q", expected)
 	}
 	if err.Error() != expected {
 		t.Errorf("\nexpected:\n%q\ngot:\n%q", expected, err.Error())
@@ -153,7 +153,7 @@ receivers:
 	expected := "no routes provided"
 
 	if err == nil {
-		t.Fatalf("no error returned, expeceted:\n%q", expected)
+		t.Fatalf("no error returned, expected:\n%q", expected)
 	}
 	if err.Error() != expected {
 		t.Errorf("\nexpected:\n%q\ngot:\n%q", expected, err.Error())
@@ -176,7 +176,7 @@ receivers:
 	expected := "root route must not have any matchers"
 
 	if err == nil {
-		t.Fatalf("no error returned, expeceted:\n%q", expected)
+		t.Fatalf("no error returned, expected:\n%q", expected)
 	}
 	if err.Error() != expected {
 		t.Errorf("\nexpected:\n%q\ngot:\n%q", expected, err.Error())
@@ -198,7 +198,7 @@ receivers:
 	expected := "cannot have continue in root route"
 
 	if err == nil {
-		t.Fatalf("no error returned, expeceted:\n%q", expected)
+		t.Fatalf("no error returned, expected:\n%q", expected)
 	}
 	if err.Error() != expected {
 		t.Errorf("\nexpected:\n%q\ngot:\n%q", expected, err.Error())
@@ -206,6 +206,48 @@ receivers:
 
 }
 
+func TestGroupIntervalIsGreaterThanZero(t *testing.T) {
+	in := `
+route:
+    receiver: team-X-mails
+    group_interval: 0s
+
+receivers:
+- name: 'team-X-mails'
+`
+	_, err := Load(in)
+
+	expected := "group_interval cannot be zero"
+
+	if err == nil {
+		t.Fatalf("no error returned, expected:\n%q", expected)
+	}
+	if err.Error() != expected {
+		t.Errorf("\nexpected:\n%q\ngot:\n%q", expected, err.Error())
+	}
+}
+
+func TestRepeatIntervalIsGreaterThanZero(t *testing.T) {
+	in := `
+route:
+    receiver: team-X-mails
+    repeat_interval: 0s
+
+receivers:
+- name: 'team-X-mails'
+`
+	_, err := Load(in)
+
+	expected := "repeat_interval cannot be zero"
+
+	if err == nil {
+		t.Fatalf("no error returned, expected:\n%q", expected)
+	}
+	if err.Error() != expected {
+		t.Errorf("\nexpected:\n%q\ngot:\n%q", expected, err.Error())
+	}
+}
+
 func TestHideConfigSecrets(t *testing.T) {
 	c, _, err := LoadFile("testdata/conf.good.yml")
 	if err != nil {
diff --git a/test/acceptance/send_test.go b/test/acceptance/send_test.go
index 64d4410d..90f54d73 100644
--- a/test/acceptance/send_test.go
+++ b/test/acceptance/send_test.go
@@ -36,7 +36,7 @@ route:
   group_by: []
   group_wait:      1s
   group_interval:  1s
-  repeat_interval: 0s
+  repeat_interval: 1ms
 
 receivers:
 - name: "default"
@@ -112,7 +112,7 @@ route:
   group_by: []
   group_wait:      1s
   group_interval:  1s
-  repeat_interval: 0s
+  repeat_interval: 1ms
 
 receivers:
 - name: "default"
diff --git a/test/acceptance/silence_test.go b/test/acceptance/silence_test.go
index 95197cfc..cc831767 100644
--- a/test/acceptance/silence_test.go
+++ b/test/acceptance/silence_test.go
@@ -30,7 +30,7 @@ route:
   group_by: []
   group_wait:      1s
   group_interval:  1s
-  repeat_interval: 0s
+  repeat_interval: 1ms
 
 receivers:
 - name: "default"
@@ -82,7 +82,7 @@ route:
   group_by: []
   group_wait:      1s
   group_interval:  1s
-  repeat_interval: 0s
+  repeat_interval: 1ms
 
 receivers:
 - name: "default"