fix(ui): return error on out of range percentiles

Also add a few missing summary-trend-stats tests. Resolves: #1143 (comment)
grafana · Oct 24, 2019 · a40db55 · a40db55
1 parent 6fd34b6
commit a40db55
Show file tree

Hide file tree

Showing 2 changed files with 12 additions and 4 deletions.
diff --git a/cmd/config_consolidation_test.go b/cmd/config_consolidation_test.go
@@ -409,9 +409,16 @@ func getConfigConsolidationTestCases() []configConsolidationTestCase {
 		{opts{}, exp{}, func(t *testing.T, c Config) {
 			assert.Equal(t, lib.DefaultSummaryTrendStats, c.Options.SummaryTrendStats)
 		}},
-		{opts{cli: []string{"--summary-trend-stats", `""`}}, exp{}, func(t *testing.T, c Config) {
+		{opts{cli: []string{"--summary-trend-stats", ""}}, exp{}, func(t *testing.T, c Config) {
 			assert.Equal(t, []string{}, c.Options.SummaryTrendStats)
 		}},
+		{opts{cli: []string{"--summary-trend-stats", "coun"}}, exp{consolidationError: true}, nil},
+		{opts{cli: []string{"--summary-trend-stats", "med,avg,p("}}, exp{consolidationError: true}, nil},
+		{opts{cli: []string{"--summary-trend-stats", "med,avg,p(-1)"}}, exp{consolidationError: true}, nil},
+		{opts{cli: []string{"--summary-trend-stats", "med,avg,p(101)"}}, exp{consolidationError: true}, nil},
+		{opts{cli: []string{"--summary-trend-stats", "med,avg,p(99.999)"}}, exp{}, func(t *testing.T, c Config) {
+			assert.Equal(t, []string{"med", "avg", "p(99.999)"}, c.Options.SummaryTrendStats)
+		}},
 		{
 			opts{runner: &lib.Options{SummaryTrendStats: []string{"avg", "p(90)", "count"}}},
 			exp{},

diff --git a/ui/summary.go b/ui/summary.go
@@ -46,8 +46,9 @@ const (
 var (
 	errStatEmptyString            = errors.New("invalid stat, empty string")
 	errStatUnknownFormat          = errors.New("invalid stat, unknown format")
-	errPercentileStatInvalidValue = errors.New("invalid percentile stat value, accepts a number")
-	staticResolvers               = map[string]func(s *stats.TrendSink) interface{}{
+	errPercentileStatInvalidValue = errors.New(
+		"invalid percentile stat value, accepts a number between 0 and 100")
+	staticResolvers = map[string]func(s *stats.TrendSink) interface{}{
 		"avg":   func(s *stats.TrendSink) interface{} { return s.Avg },
 		"min":   func(s *stats.TrendSink) interface{} { return s.Min },
 		"med":   func(s *stats.TrendSink) interface{} { return s.Med },
@@ -127,7 +128,7 @@ func validatePercentile(stat string) (float64, error) {
 
 	percentile, err := strconv.ParseFloat(stat[2:len(stat)-1], 64)
 
-	if err != nil {
+	if err != nil || ((0 > percentile) || (percentile > 100)) {
 		return 0, errPercentileStatInvalidValue
 	}