feat: trigger name limit (#1080)

api/config.go

@@ -38,6 +38,7 @@ type Config struct {
 	MetricsTTL    map[moira.ClusterKey]time.Duration
 	Flags         FeatureFlags
 	Authorization Authorization
+	Limits        LimitsConfig
 }
 
 // WebConfig is container for web ui configuration parameters.
@@ -60,3 +61,29 @@ type MetricSourceCluster struct {
 func (WebConfig) Render(w http.ResponseWriter, r *http.Request) error {
 	return nil
 }
+
+const (
+	// DefaultTriggerNameMaxSize which will be used while validating dto.Trigger.
+	DefaultTriggerNameMaxSize = 200
+)
+
+// LimitsConfig contains limits for some entities.
+type LimitsConfig struct {
+	// Trigger contains limits for triggers.
+	Trigger TriggerLimits
+}
+
+// TriggerLimits contains all limits applied for triggers.
+type TriggerLimits struct {
+	// MaxNameSize is the amount of characters allowed in trigger name.
+	MaxNameSize int
+}
+
+// GetTestLimitsConfig is used for testing.
+func GetTestLimitsConfig() LimitsConfig {
+	return LimitsConfig{
+		Trigger: TriggerLimits{
+			MaxNameSize: DefaultTriggerNameMaxSize,
+		},
+	}
+}

api/dto/triggers.go

@@ -2,11 +2,13 @@
 package dto
 
 import (
+	"errors"
 	"fmt"
 	"net/http"
 	"regexp"
 	"strconv"
 	"time"
+	"unicode/utf8"
 
 	"github.com/moira-alert/moira/templating"
 
@@ -19,8 +21,26 @@ import (
 
 var targetNameRegex = regexp.MustCompile("^t\\d+$")
 
-// ErrBadAloneMetricName is used when any key in map TriggerModel.AloneMetric doesn't match targetNameRegex.
-var ErrBadAloneMetricName = fmt.Errorf("alone metrics' target name must match the pattern: ^t\\d+$, for example: 't1'")
+var (
+	// errBadAloneMetricName is used when any key in map TriggerModel.AloneMetric doesn't match targetNameRegex.
+	errBadAloneMetricName = errors.New("alone metrics' target name must match the pattern: ^t\\d+$, for example: 't1'")
+
+	// errTargetsRequired is returned when there is no targets in Trigger.
+	errTargetsRequired = errors.New("targets are required")
+
+	// errTagsRequired is returned when there is no tags in Trigger.
+	errTagsRequired = errors.New("tags are required")
+
+	// errTriggerNameRequired is returned when there is empty Name in Trigger.
+	errTriggerNameRequired = errors.New("trigger name is required")
+
+	// errAloneMetricTargetIndexOutOfRange is returned when target index is out of range. Example: if we have target "t1",
+	// then "1" is a target index.
+	errAloneMetricTargetIndexOutOfRange = errors.New("alone metrics target index should be in range from 1 to length of targets")
+
+	// errAsteriskPatternNotAllowed is returned then one of Trigger.Patterns contain only "*".
+	errAsteriskPatternNotAllowed = errors.New("pattern \"*\" is not allowed to use")
+)
 
 // TODO(litleleprikon): Remove after https://github.com/moira-alert/moira/issues/550 will be resolved.
 var asteriskPattern = "*"
@@ -152,15 +172,22 @@ func CreateTriggerModel(trigger *moira.Trigger) TriggerModel {
 func (trigger *Trigger) Bind(request *http.Request) error {
 	trigger.Tags = normalizeTags(trigger.Tags)
 	if len(trigger.Targets) == 0 {
-		return api.ErrInvalidRequestContent{ValidationError: fmt.Errorf("targets is required")}
+		return api.ErrInvalidRequestContent{ValidationError: errTargetsRequired}
 	}
 
 	if len(trigger.Tags) == 0 {
-		return api.ErrInvalidRequestContent{ValidationError: fmt.Errorf("tags is required")}
+		return api.ErrInvalidRequestContent{ValidationError: errTagsRequired}
 	}
 
 	if trigger.Name == "" {
-		return api.ErrInvalidRequestContent{ValidationError: fmt.Errorf("trigger name is required")}
+		return api.ErrInvalidRequestContent{ValidationError: errTriggerNameRequired}
+	}
+
+	limits := middleware.GetLimits(request)
+	if utf8.RuneCountInString(trigger.Name) > limits.Trigger.MaxNameSize {
+		return api.ErrInvalidRequestContent{
+			ValidationError: fmt.Errorf("trigger name too long, should not be greater than %d symbols", limits.Trigger.MaxNameSize),
+		}
 	}
 
 	if err := checkWarnErrorExpression(trigger); err != nil {
@@ -173,7 +200,7 @@ func (trigger *Trigger) Bind(request *http.Request) error {
 
 	for targetName := range trigger.AloneMetrics {
 		if !targetNameRegex.MatchString(targetName) {
-			return api.ErrInvalidRequestContent{ValidationError: ErrBadAloneMetricName}
+			return api.ErrInvalidRequestContent{ValidationError: errBadAloneMetricName}
 		}
 
 		targetIndexStr := targetName[1:]
@@ -183,7 +210,7 @@ func (trigger *Trigger) Bind(request *http.Request) error {
 		}
 
 		if targetIndex < 0 || targetIndex > len(trigger.Targets) {
-			return api.ErrInvalidRequestContent{ValidationError: fmt.Errorf("alone metrics target index should be in range from 1 to length of targets")}
+			return api.ErrInvalidRequestContent{ValidationError: errAloneMetricTargetIndexOutOfRange}
 		}
 	}
 
@@ -224,7 +251,7 @@ func (trigger *Trigger) Bind(request *http.Request) error {
 	// TODO(litleleprikon): Remove after https://github.com/moira-alert/moira/issues/550 will be resolved
 	for _, pattern := range trigger.Patterns {
 		if pattern == asteriskPattern {
-			return api.ErrInvalidRequestContent{ValidationError: fmt.Errorf("pattern \"*\" is not allowed to use")}
+			return api.ErrInvalidRequestContent{ValidationError: errAsteriskPatternNotAllowed}
 		}
 	}
 

api/dto/triggers_test.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"fmt"
 	"net/http"
+	"strings"
 	"testing"
 	"time"
 
@@ -18,6 +19,52 @@ import (
 )
 
 func TestTriggerValidation(t *testing.T) {
+	Convey("Test trigger name and tags", t, func() {
+		trigger := Trigger{
+			TriggerModel: TriggerModel{},
+		}
+
+		limit := api.GetTestLimitsConfig()
+
+		request, _ := http.NewRequest("PUT", "/api/trigger", nil)
+		request.Header.Set("Content-Type", "application/json")
+		request = request.WithContext(middleware.SetContextValueForTest(request.Context(), "limits", limit))
+
+		Convey("with empty targets", func() {
+			err := trigger.Bind(request)
+
+			So(err, ShouldResemble, api.ErrInvalidRequestContent{ValidationError: errTargetsRequired})
+		})
+
+		trigger.Targets = []string{"foo.bar"}
+
+		Convey("with empty tag in tag list", func() {
+			trigger.Tags = []string{""}
+
+			err := trigger.Bind(request)
+
+			So(err, ShouldResemble, api.ErrInvalidRequestContent{ValidationError: errTagsRequired})
+		})
+
+		trigger.Tags = append(trigger.Tags, "tag1")
+
+		Convey("with empty Name", func() {
+			err := trigger.Bind(request)
+
+			So(err, ShouldResemble, api.ErrInvalidRequestContent{ValidationError: errTriggerNameRequired})
+		})
+
+		Convey("with too long Name", func() {
+			trigger.Name = strings.Repeat("ё", limit.Trigger.MaxNameSize+1)
+
+			err := trigger.Bind(request)
+
+			So(err, ShouldResemble, api.ErrInvalidRequestContent{
+				ValidationError: fmt.Errorf("trigger name too long, should not be greater than %d symbols", limit.Trigger.MaxNameSize),
+			})
+		})
+	})
+
 	Convey("Tests targets, values and expression validation", t, func() {
 		mockCtrl := gomock.NewController(t)
 		defer mockCtrl.Finish()
@@ -31,6 +78,7 @@ func TestTriggerValidation(t *testing.T) {
 		request.Header.Set("Content-Type", "application/json")
 		ctx := request.Context()
 		ctx = context.WithValue(ctx, middleware.ContextKey("metricSourceProvider"), sourceProvider)
+		ctx = context.WithValue(ctx, middleware.ContextKey("limits"), api.GetTestLimitsConfig())
 		request = request.WithContext(ctx)
 
 		desc := "Graphite ClickHouse"
@@ -203,19 +251,19 @@ func TestTriggerValidation(t *testing.T) {
 				trigger.AloneMetrics = map[string]bool{"ttt": true}
 				tr := Trigger{trigger, throttling}
 				err := tr.Bind(request)
-				So(err, ShouldResemble, api.ErrInvalidRequestContent{ValidationError: ErrBadAloneMetricName})
+				So(err, ShouldResemble, api.ErrInvalidRequestContent{ValidationError: errBadAloneMetricName})
 			})
 			Convey("have more than 1 metric name but only 1 need", func() {
 				trigger.AloneMetrics = map[string]bool{"t1 t2": true}
 				tr := Trigger{trigger, throttling}
 				err := tr.Bind(request)
-				So(err, ShouldResemble, api.ErrInvalidRequestContent{ValidationError: ErrBadAloneMetricName})
+				So(err, ShouldResemble, api.ErrInvalidRequestContent{ValidationError: errBadAloneMetricName})
 			})
 			Convey("have target higher than total amount of targets", func() {
 				trigger.AloneMetrics = map[string]bool{"t3": true}
 				tr := Trigger{trigger, throttling}
 				err := tr.Bind(request)
-				So(err, ShouldResemble, api.ErrInvalidRequestContent{ValidationError: fmt.Errorf("alone metrics target index should be in range from 1 to length of targets")})
+				So(err, ShouldResemble, api.ErrInvalidRequestContent{ValidationError: errAloneMetricTargetIndexOutOfRange})
 			})
 		})
 
@@ -237,7 +285,7 @@ func TestTriggerValidation(t *testing.T) {
 				tr := Trigger{trigger, throttling}
 				fetchResult.EXPECT().GetPatterns().Return([]string{"*"}, nil).AnyTimes()
 				err := tr.Bind(request)
-				So(err, ShouldResemble, api.ErrInvalidRequestContent{ValidationError: fmt.Errorf("pattern \"*\" is not allowed to use")})
+				So(err, ShouldResemble, api.ErrInvalidRequestContent{ValidationError: errAsteriskPatternNotAllowed})
 			})
 		})
 	})

api/handler/handler.go

@@ -50,6 +50,7 @@ func NewHandler(
 	router.Use(moiramiddle.UserContext)
 	router.Use(moiramiddle.RequestLogger(log))
 	router.Use(middleware.NoCache)
+	router.Use(moiramiddle.LimitsContext(apiConfig.Limits))
 
 	router.NotFound(notFoundHandler)
 	router.MethodNotAllowed(methodNotAllowedHandler)

api/handler/trigger_test.go

@@ -10,6 +10,8 @@ import (
 	"testing"
 	"time"
 
+	"github.com/moira-alert/moira/api"
+
 	"github.com/moira-alert/moira"
 	"github.com/moira-alert/moira/api/dto"
 	"github.com/moira-alert/moira/api/middleware"
@@ -165,8 +167,8 @@ func TestUpdateTrigger(t *testing.T) {
 			testRequest.Header.Add("content-type", "application/json")
 			testRequest = testRequest.WithContext(middleware.SetContextValueForTest(testRequest.Context(), "metricSourceProvider", sourceProvider))
 			testRequest = testRequest.WithContext(middleware.SetContextValueForTest(testRequest.Context(), "clustersMetricTTL", MakeTestTTLs()))
-
 			testRequest = testRequest.WithContext(middleware.SetContextValueForTest(testRequest.Context(), triggerIDKey, triggerID))
+			testRequest = testRequest.WithContext(middleware.SetContextValueForTest(testRequest.Context(), "limits", api.GetTestLimitsConfig()))
 
 			responseWriter := httptest.NewRecorder()
 			updateTrigger(responseWriter, testRequest)
@@ -208,6 +210,7 @@ func TestUpdateTrigger(t *testing.T) {
 			request = request.WithContext(middleware.SetContextValueForTest(request.Context(), "metricSourceProvider", sourceProvider))
 			request = request.WithContext(middleware.SetContextValueForTest(request.Context(), "clustersMetricTTL", MakeTestTTLs()))
 			request = request.WithContext(middleware.SetContextValueForTest(request.Context(), triggerIDKey, triggerID))
+			request = request.WithContext(middleware.SetContextValueForTest(request.Context(), "limits", api.GetTestLimitsConfig()))
 
 			responseWriter := httptest.NewRecorder()
 			updateTrigger(responseWriter, request)
@@ -247,6 +250,7 @@ func TestUpdateTrigger(t *testing.T) {
 			request = request.WithContext(middleware.SetContextValueForTest(request.Context(), "metricSourceProvider", sourceProvider))
 			request = request.WithContext(middleware.SetContextValueForTest(request.Context(), "clustersMetricTTL", MakeTestTTLs()))
 			request = request.WithContext(middleware.SetContextValueForTest(request.Context(), triggerIDKey, triggerID))
+			request = request.WithContext(middleware.SetContextValueForTest(request.Context(), "limits", api.GetTestLimitsConfig()))
 
 			responseWriter := httptest.NewRecorder()
 			updateTrigger(responseWriter, request)
@@ -272,6 +276,7 @@ func TestUpdateTrigger(t *testing.T) {
 			request = request.WithContext(middleware.SetContextValueForTest(request.Context(), "metricSourceProvider", sourceProvider))
 			request = request.WithContext(middleware.SetContextValueForTest(request.Context(), "clustersMetricTTL", MakeTestTTLs()))
 			request = request.WithContext(middleware.SetContextValueForTest(request.Context(), triggerIDKey, triggerID))
+			request = request.WithContext(middleware.SetContextValueForTest(request.Context(), "limits", api.GetTestLimitsConfig()))
 
 			responseWriter := httptest.NewRecorder()
 			updateTrigger(responseWriter, request)
@@ -335,6 +340,7 @@ func TestUpdateTrigger(t *testing.T) {
 			request = request.WithContext(middleware.SetContextValueForTest(request.Context(), "metricSourceProvider", sourceProvider))
 			request = request.WithContext(middleware.SetContextValueForTest(request.Context(), "clustersMetricTTL", MakeTestTTLs()))
 			request = request.WithContext(middleware.SetContextValueForTest(request.Context(), triggerIDKey, triggerID))
+			request = request.WithContext(middleware.SetContextValueForTest(request.Context(), "limits", api.GetTestLimitsConfig()))
 
 			responseWriter := httptest.NewRecorder()
 			updateTrigger(responseWriter, request)
@@ -353,6 +359,7 @@ func TestUpdateTrigger(t *testing.T) {
 			request = request.WithContext(middleware.SetContextValueForTest(request.Context(), "metricSourceProvider", sourceProvider))
 			request = request.WithContext(middleware.SetContextValueForTest(request.Context(), "clustersMetricTTL", MakeTestTTLs()))
 			request = request.WithContext(middleware.SetContextValueForTest(request.Context(), triggerIDKey, triggerID))
+			request = request.WithContext(middleware.SetContextValueForTest(request.Context(), "limits", api.GetTestLimitsConfig()))
 
 			responseWriter := httptest.NewRecorder()
 			updateTrigger(responseWriter, request)