Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: trigger name limit #1080

Merged
merged 17 commits into from
Oct 15, 2024
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions api/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ import (
"net/http"
"time"

"github.com/moira-alert/moira/limits"

"github.com/moira-alert/moira"
)

Expand Down Expand Up @@ -38,6 +40,7 @@ type Config struct {
MetricsTTL map[moira.ClusterKey]time.Duration
Flags FeatureFlags
Authorization Authorization
Limits limits.Config
}

// WebConfig is container for web ui configuration parameters.
Expand Down
31 changes: 25 additions & 6 deletions api/dto/triggers.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"regexp"
"strconv"
"time"
"unicode/utf8"

"github.com/moira-alert/moira/templating"

Expand All @@ -19,8 +20,19 @@ 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 = fmt.Errorf("alone metrics' target name must match the pattern: ^t\\d+$, for example: 't1'")

// errTargetsRequired is returned when there is no targets in Trigger.
errTargetsRequired = fmt.Errorf("targets is required")
Tetrergeru marked this conversation as resolved.
Show resolved Hide resolved

// errTagsRequired is returned when there is no tags in Trigger.
errTagsRequired = fmt.Errorf("tags is required")

// errTriggerNameRequired is returned when there is empty Name in Trigger.
errTriggerNameRequired = fmt.Errorf("trigger name is required")
)

// TODO(litleleprikon): Remove after https://github.com/moira-alert/moira/issues/550 will be resolved.
var asteriskPattern = "*"
Expand Down Expand Up @@ -152,15 +164,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),
kissken marked this conversation as resolved.
Show resolved Hide resolved
}
}

if err := checkWarnErrorExpression(trigger); err != nil {
Expand All @@ -173,7 +192,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:]
Expand Down
54 changes: 52 additions & 2 deletions api/dto/triggers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,12 @@ import (
"context"
"fmt"
"net/http"
"strings"
"testing"
"time"

"github.com/moira-alert/moira/limits"

"github.com/moira-alert/moira"
"github.com/moira-alert/moira/api"
"github.com/moira-alert/moira/api/middleware"
Expand All @@ -18,6 +21,52 @@ import (
)

func TestTriggerValidation(t *testing.T) {
Convey("Test trigger name and tags", t, func() {
trigger := Trigger{
TriggerModel: TriggerModel{},
}

limit := limits.GetTestConfig()

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()
Expand All @@ -31,6 +80,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"), limits.GetTestConfig())
request = request.WithContext(ctx)

desc := "Graphite ClickHouse"
Expand Down Expand Up @@ -203,13 +253,13 @@ 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}
Expand Down
1 change: 1 addition & 0 deletions api/handler/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,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)
Expand Down
9 changes: 8 additions & 1 deletion api/handler/trigger_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ import (
"testing"
"time"

"github.com/moira-alert/moira/limits"

"github.com/moira-alert/moira"
"github.com/moira-alert/moira/api/dto"
"github.com/moira-alert/moira/api/middleware"
Expand Down Expand Up @@ -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", limits.GetTestConfig()))

responseWriter := httptest.NewRecorder()
updateTrigger(responseWriter, testRequest)
Expand Down Expand Up @@ -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", limits.GetTestConfig()))

responseWriter := httptest.NewRecorder()
updateTrigger(responseWriter, request)
Expand Down Expand Up @@ -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", limits.GetTestConfig()))

responseWriter := httptest.NewRecorder()
updateTrigger(responseWriter, request)
Expand All @@ -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", limits.GetTestConfig()))

responseWriter := httptest.NewRecorder()
updateTrigger(responseWriter, request)
Expand Down Expand Up @@ -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", limits.GetTestConfig()))

responseWriter := httptest.NewRecorder()
updateTrigger(responseWriter, request)
Expand All @@ -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", limits.GetTestConfig()))

responseWriter := httptest.NewRecorder()
updateTrigger(responseWriter, request)
Expand Down
12 changes: 12 additions & 0 deletions api/handler/triggers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ import (
"testing"
"time"

"github.com/moira-alert/moira/limits"

"github.com/moira-alert/moira"
"github.com/moira-alert/moira/api"
dataBase "github.com/moira-alert/moira/database"
Expand Down Expand Up @@ -95,6 +97,7 @@ func TestGetTriggerFromRequest(t *testing.T) {
request := httptest.NewRequest(http.MethodPut, "/trigger", bytes.NewReader(body))
request.Header.Add("content-type", "application/json")
request = request.WithContext(middleware.SetContextValueForTest(request.Context(), "metricSourceProvider", sourceProvider))
request = request.WithContext(middleware.SetContextValueForTest(request.Context(), "limits", limits.GetTestConfig()))

Convey("It should be parsed successfully", func() {
triggerDTO.TTL = moira.DefaultTTL
Expand Down Expand Up @@ -133,6 +136,7 @@ func TestGetTriggerFromRequest(t *testing.T) {
request := httptest.NewRequest(http.MethodPut, "/trigger", strings.NewReader(body))
request.Header.Add("content-type", "application/json")
request = request.WithContext(middleware.SetContextValueForTest(request.Context(), "metricSourceProvider", sourceProvider))
request = request.WithContext(middleware.SetContextValueForTest(request.Context(), "limits", limits.GetTestConfig()))

Convey("Parser should return en error", func() {
_, err := getTriggerFromRequest(request)
Expand Down Expand Up @@ -251,6 +255,7 @@ func TestTriggerCheckHandler(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(), "limits", limits.GetTestConfig()))

triggerCheck(responseWriter, testRequest)

Expand Down Expand Up @@ -315,6 +320,7 @@ func TestCreateTriggerHandler(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(), "limits", limits.GetTestConfig()))

responseWriter := httptest.NewRecorder()
createTrigger(responseWriter, testRequest)
Expand Down Expand Up @@ -352,6 +358,7 @@ func TestCreateTriggerHandler(t *testing.T) {
request.Header.Add("content-type", "application/json")
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(), "limits", limits.GetTestConfig()))

responseWriter := httptest.NewRecorder()
createTrigger(responseWriter, request)
Expand Down Expand Up @@ -390,6 +397,7 @@ func TestCreateTriggerHandler(t *testing.T) {
request.Header.Add("content-type", "application/json")
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(), "limits", limits.GetTestConfig()))

responseWriter := httptest.NewRecorder()
createTrigger(responseWriter, request)
Expand All @@ -413,6 +421,7 @@ func TestCreateTriggerHandler(t *testing.T) {
request.Header.Add("content-type", "application/json")
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(), "limits", limits.GetTestConfig()))

responseWriter := httptest.NewRecorder()
createTrigger(responseWriter, request)
Expand Down Expand Up @@ -475,6 +484,7 @@ func TestCreateTriggerHandler(t *testing.T) {
request.Header.Add("content-type", "application/json")
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(), "limits", limits.GetTestConfig()))

responseWriter := httptest.NewRecorder()
createTrigger(responseWriter, request)
Expand All @@ -492,6 +502,7 @@ func TestCreateTriggerHandler(t *testing.T) {
request.Header.Add("content-type", "application/json")
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(), "limits", limits.GetTestConfig()))

responseWriter := httptest.NewRecorder()
createTrigger(responseWriter, request)
Expand Down Expand Up @@ -711,6 +722,7 @@ func newTriggerCreateRequest(
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", limits.GetTestConfig()))

return request
}
Expand Down
12 changes: 12 additions & 0 deletions api/middleware/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ import (
"strings"
"time"

"github.com/moira-alert/moira/limits"

"github.com/go-chi/chi"
"github.com/go-chi/render"
"github.com/moira-alert/moira"
Expand Down Expand Up @@ -331,3 +333,13 @@ func StatesContext() func(next http.Handler) http.Handler {
})
}
}

// LimitsContext places limits.Config to request context.
func LimitsContext(limit limits.Config) func(next http.Handler) http.Handler {
return func(next http.Handler) http.Handler {
return http.HandlerFunc(func(writer http.ResponseWriter, request *http.Request) {
ctx := context.WithValue(request.Context(), limitsContextKey, limit)
next.ServeHTTP(writer, request.WithContext(ctx))
})
}
}
8 changes: 8 additions & 0 deletions api/middleware/middleware.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import (
"net/http"
"time"

"github.com/moira-alert/moira/limits"

"github.com/moira-alert/moira"
"github.com/moira-alert/moira/api"
metricSource "github.com/moira-alert/moira/metric_source"
Expand Down Expand Up @@ -41,6 +43,7 @@ var (
authKey ContextKey = "auth"
metricContextKey ContextKey = "metric"
statesContextKey ContextKey = "states"
limitsContextKey ContextKey = "limits"
anonymousUser = "anonymous"
)

Expand Down Expand Up @@ -174,3 +177,8 @@ func GetMetric(request *http.Request) string {
func GetStates(request *http.Request) map[string]struct{} {
return request.Context().Value(statesContextKey).(map[string]struct{})
}

// GetLimits returns configured limits.
func GetLimits(request *http.Request) limits.Config {
return request.Context().Value(limitsContextKey).(limits.Config)
}
Loading
Loading