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.

Sign up for GitHub

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

Jump to bottom

feat: trigger name limit #1080

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
+177 −9
Open

feat: trigger name limit #1080

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
20fd82f
feat: add limit for trigger
AleksandrMatsko Sep 3, 2024
4845fc3
test: fix broken
AleksandrMatsko Sep 3, 2024
20c9678
test: for validating trigger name
AleksandrMatsko Sep 4, 2024
e17aa33
style: use linter
AleksandrMatsko Sep 4, 2024
b413fb3
refactor: make some errors in dto private
AleksandrMatsko Sep 5, 2024
d28871c
refactor: move code from limits package to api
AleksandrMatsko Sep 6, 2024
1a64d49
refactor: move number to constant
AleksandrMatsko Sep 10, 2024
b389960
merge master into feat/trigger-name-limit
AleksandrMatsko Sep 17, 2024
f101d93
merge master into feat/trigger-name-limit
AleksandrMatsko Sep 24, 2024
d2f045c
merge master into feat/trigger-name-limit
AleksandrMatsko Sep 27, 2024
7bcb0da
test: fix panics because of no limits config in middleware
AleksandrMatsko Sep 27, 2024
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
27 changes: 27 additions & 0 deletions api/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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,
},
}
}
31 changes: 25 additions & 6 deletions api/dto/triggers.go
View file
Open in desktop
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 are required")

// errTagsRequired is returned when there is no tags in Trigger.
errTagsRequired = fmt.Errorf("tags are 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),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

а эту тогда ошибку почему не утащил наверх?)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

А тут параметр есть, поэтому не стал...

}
}

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
52 changes: 50 additions & 2 deletions api/dto/triggers_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"fmt"
"net/http"
"strings"
"testing"
"time"

Expand All @@ -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()
Expand All @@ -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"
Expand Down Expand Up @@ -203,13 +251,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
View file
Open in desktop
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
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
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", api.GetTestLimitsConfig()))

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", api.GetTestLimitsConfig()))

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", api.GetTestLimitsConfig()))

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", api.GetTestLimitsConfig()))

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", api.GetTestLimitsConfig()))

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", api.GetTestLimitsConfig()))

responseWriter := httptest.NewRecorder()
updateTrigger(responseWriter, request)
Expand Down
13 changes: 13 additions & 0 deletions api/handler/triggers_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,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", api.GetTestLimitsConfig()))

Convey("It should be parsed successfully", func() {
triggerDTO.TTL = moira.DefaultTTL
Expand Down Expand Up @@ -138,6 +139,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", api.GetTestLimitsConfig()))

Convey("Parser should return en error", func() {
_, err := getTriggerFromRequest(request)
Expand Down Expand Up @@ -190,6 +192,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", allSourceProvider))
request = request.WithContext(middleware.SetContextValueForTest(request.Context(), "limits", api.GetTestLimitsConfig()))

testLogger, _ := logging.GetLogger("Test")

Expand All @@ -214,6 +217,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", allSourceProvider))
request = request.WithContext(middleware.SetContextValueForTest(request.Context(), "limits", api.GetTestLimitsConfig()))

var returnedErr error = &prometheus.Error{
Type: prometheus.ErrBadData,
Expand All @@ -240,6 +244,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", allSourceProvider))
request = request.WithContext(middleware.SetContextValueForTest(request.Context(), "limits", api.GetTestLimitsConfig()))

var returnedErr error = &prometheus.Error{
Type: errType,
Expand Down Expand Up @@ -366,6 +371,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", api.GetTestLimitsConfig()))

triggerCheck(responseWriter, testRequest)

Expand Down Expand Up @@ -430,6 +436,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", api.GetTestLimitsConfig()))

responseWriter := httptest.NewRecorder()
createTrigger(responseWriter, testRequest)
Expand Down Expand Up @@ -467,6 +474,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", api.GetTestLimitsConfig()))

responseWriter := httptest.NewRecorder()
createTrigger(responseWriter, request)
Expand Down Expand Up @@ -505,6 +513,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", api.GetTestLimitsConfig()))

responseWriter := httptest.NewRecorder()
createTrigger(responseWriter, request)
Expand All @@ -528,6 +537,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", api.GetTestLimitsConfig()))

responseWriter := httptest.NewRecorder()
createTrigger(responseWriter, request)
Expand Down Expand Up @@ -590,6 +600,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", api.GetTestLimitsConfig()))

responseWriter := httptest.NewRecorder()
createTrigger(responseWriter, request)
Expand All @@ -607,6 +618,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", api.GetTestLimitsConfig()))

responseWriter := httptest.NewRecorder()
createTrigger(responseWriter, request)
Expand Down Expand Up @@ -826,6 +838,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", api.GetTestLimitsConfig()))

return request
}
Expand Down
10 changes: 10 additions & 0 deletions api/middleware/context.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -331,3 +331,13 @@ func StatesContext() func(next http.Handler) http.Handler {
})
}
}

// LimitsContext places api.LimitsConfig to request context.
func LimitsContext(limit api.LimitsConfig) 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))
})
}
}
6 changes: 6 additions & 0 deletions api/middleware/middleware.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ var (
authKey ContextKey = "auth"
metricContextKey ContextKey = "metric"
statesContextKey ContextKey = "states"
limitsContextKey ContextKey = "limits"
anonymousUser = "anonymous"
)

Expand Down Expand Up @@ -174,3 +175,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) api.LimitsConfig {
return request.Context().Value(limitsContextKey).(api.LimitsConfig)
}
Loading
Loading