From 20fd82fc920f71c949462b1a2da4b82ac667d3b5 Mon Sep 17 00:00:00 2001 From: AleksandrMatsko Date: Tue, 3 Sep 2024 18:32:37 +0700 Subject: [PATCH 1/9] feat: add limit for trigger --- api/config.go | 2 ++ api/dto/triggers.go | 7 +++++++ api/handler/handler.go | 1 + api/middleware/context.go | 11 +++++++++++ api/middleware/middleware.go | 7 +++++++ cmd/api/config.go | 8 ++++++++ cmd/config.go | 22 ++++++++++++++++++++++ limits/limits.go | 13 +++++++++++++ local/api.yml | 3 +++ 9 files changed, 74 insertions(+) create mode 100644 limits/limits.go diff --git a/api/config.go b/api/config.go index 7750b68d5..40d34b44b 100644 --- a/api/config.go +++ b/api/config.go @@ -1,6 +1,7 @@ package api import ( + "github.com/moira-alert/moira/limits" "net/http" "time" @@ -38,6 +39,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. diff --git a/api/dto/triggers.go b/api/dto/triggers.go index b35ce20ef..f7086184a 100644 --- a/api/dto/triggers.go +++ b/api/dto/triggers.go @@ -7,6 +7,7 @@ import ( "regexp" "strconv" "time" + "unicode/utf8" "github.com/moira-alert/moira/templating" @@ -163,6 +164,12 @@ func (trigger *Trigger) Bind(request *http.Request) error { return api.ErrInvalidRequestContent{ValidationError: fmt.Errorf("trigger name is required")} } + limits := middleware.GetLimits(request) + if utf8.RuneCountInString(trigger.Name) > limits.Trigger.MaxNameSize { + return api.ErrInvalidRequestContent{ + ValidationError: fmt.Errorf("trigger name too long, should be less than %d symbols", limits.Trigger.MaxNameSize)} + } + if err := checkWarnErrorExpression(trigger); err != nil { return api.ErrInvalidRequestContent{ValidationError: err} } diff --git a/api/handler/handler.go b/api/handler/handler.go index 27647a8c3..f225e1893 100644 --- a/api/handler/handler.go +++ b/api/handler/handler.go @@ -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) diff --git a/api/middleware/context.go b/api/middleware/context.go index 0d9e26f31..28af74c07 100644 --- a/api/middleware/context.go +++ b/api/middleware/context.go @@ -3,6 +3,7 @@ package middleware import ( "context" "fmt" + "github.com/moira-alert/moira/limits" "net/http" "net/url" "strconv" @@ -331,3 +332,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)) + }) + } +} diff --git a/api/middleware/middleware.go b/api/middleware/middleware.go index 80d65066d..fe5af5cc8 100644 --- a/api/middleware/middleware.go +++ b/api/middleware/middleware.go @@ -2,6 +2,7 @@ package middleware import ( "context" + "github.com/moira-alert/moira/limits" "net/http" "time" @@ -41,6 +42,7 @@ var ( authKey ContextKey = "auth" metricContextKey ContextKey = "metric" statesContextKey ContextKey = "states" + limitsContextKey ContextKey = "limits" anonymousUser = "anonymous" ) @@ -174,3 +176,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) +} diff --git a/cmd/api/config.go b/cmd/api/config.go index 4647bb3f1..e60c9b0f1 100644 --- a/cmd/api/config.go +++ b/cmd/api/config.go @@ -46,6 +46,8 @@ type apiConfig struct { EnableCORS bool `yaml:"enable_cors"` // Authorization contains authorization configuration. Authorization authorization `yaml:"authorization"` + // Limits contains limits applied to entities and so on. + Limits cmd.LimitsConfig `yaml:"limits"` } type authorization struct { @@ -114,6 +116,7 @@ func (config *apiConfig) getSettings( MetricsTTL: metricsTTL, Flags: flags, Authorization: config.Authorization.toApiConfig(webConfig), + Limits: config.Limits.ToLimits(), } } @@ -214,6 +217,11 @@ func getDefault() config { API: apiConfig{ Listen: ":8081", EnableCORS: false, + Limits: cmd.LimitsConfig{ + Trigger: cmd.TriggerLimitsConfig{ + MaxNameSize: 200, + }, + }, }, Web: webConfig{ RemoteAllowed: false, diff --git a/cmd/config.go b/cmd/config.go index bb83ea439..71fb5fb20 100644 --- a/cmd/config.go +++ b/cmd/config.go @@ -3,6 +3,7 @@ package cmd import ( "errors" "fmt" + "github.com/moira-alert/moira/limits" "os" "strings" @@ -311,3 +312,24 @@ func PrintConfig(config interface{}) { d, _ := yaml.Marshal(&config) fmt.Println(string(d)) } + +// LimitsConfig contains configurable moira limits. +type LimitsConfig struct { + // Trigger contains the limits applied to triggers. + Trigger TriggerLimitsConfig `yaml:"trigger"` +} + +// TriggerLimitsConfig represents the limits which will be applied to all triggers. +type TriggerLimitsConfig struct { + // MaxNameSize is the max amount of characters allowed in trigger name. + MaxNameSize int `yaml:"max_name_size"` +} + +// ToLimits converts LimitsConfig to limits.Config. +func (conf LimitsConfig) ToLimits() limits.Config { + return limits.Config{ + Trigger: limits.Trigger{ + MaxNameSize: conf.Trigger.MaxNameSize, + }, + } +} diff --git a/limits/limits.go b/limits/limits.go new file mode 100644 index 000000000..736a5c02f --- /dev/null +++ b/limits/limits.go @@ -0,0 +1,13 @@ +package limits + +// Config contains limits for some entities. +type Config struct { + // Trigger contains limits for triggers. + Trigger Trigger +} + +// Trigger contains all limits applied for triggers. +type Trigger struct { + // MaxNameSize is the amount of characters allowed in trigger name. + MaxNameSize int +} diff --git a/local/api.yml b/local/api.yml index a48bddd83..631240d4e 100644 --- a/local/api.yml +++ b/local/api.yml @@ -37,6 +37,9 @@ prometheus_remote: api: listen: ":8081" enable_cors: false + limits: + trigger: + max_name_size: 200 web: contacts_template: - type: mail From 4845fc356c28bda24b66f6c255d4807fa2c01025 Mon Sep 17 00:00:00 2001 From: AleksandrMatsko Date: Tue, 3 Sep 2024 19:57:22 +0700 Subject: [PATCH 2/9] test: fix broken --- api/dto/triggers_test.go | 2 ++ api/handler/trigger_test.go | 8 +++++++- api/handler/triggers_test.go | 11 +++++++++++ cmd/api/config_test.go | 5 +++++ limits/limits.go | 9 +++++++++ 5 files changed, 34 insertions(+), 1 deletion(-) diff --git a/api/dto/triggers_test.go b/api/dto/triggers_test.go index 90b8a87d0..0edcb964a 100644 --- a/api/dto/triggers_test.go +++ b/api/dto/triggers_test.go @@ -3,6 +3,7 @@ package dto import ( "context" "fmt" + "github.com/moira-alert/moira/limits" "net/http" "testing" "time" @@ -31,6 +32,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" diff --git a/api/handler/trigger_test.go b/api/handler/trigger_test.go index f9d96ec3c..c5f9bc5ee 100644 --- a/api/handler/trigger_test.go +++ b/api/handler/trigger_test.go @@ -4,6 +4,7 @@ import ( "bytes" "encoding/json" "fmt" + "github.com/moira-alert/moira/limits" "io" "net/http" "net/http/httptest" @@ -165,8 +166,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) @@ -208,6 +209,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) @@ -247,6 +249,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) @@ -272,6 +275,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) @@ -335,6 +339,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) @@ -353,6 +358,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) diff --git a/api/handler/triggers_test.go b/api/handler/triggers_test.go index 83d0a1bea..5273f9f66 100644 --- a/api/handler/triggers_test.go +++ b/api/handler/triggers_test.go @@ -5,6 +5,7 @@ import ( "context" "encoding/json" "fmt" + "github.com/moira-alert/moira/limits" "io" "net/http" "net/http/httptest" @@ -95,6 +96,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 @@ -133,6 +135,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) @@ -251,6 +254,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) @@ -315,6 +319,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) @@ -352,6 +357,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) @@ -390,6 +396,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) @@ -413,6 +420,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) @@ -475,6 +483,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) @@ -492,6 +501,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) @@ -711,6 +721,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 } diff --git a/cmd/api/config_test.go b/cmd/api/config_test.go index 4777e70ea..1e694849f 100644 --- a/cmd/api/config_test.go +++ b/cmd/api/config_test.go @@ -88,6 +88,11 @@ func Test_webConfig_getDefault(t *testing.T) { API: apiConfig{ Listen: ":8081", EnableCORS: false, + Limits: cmd.LimitsConfig{ + cmd.TriggerLimitsConfig{ + MaxNameSize: 200, + }, + }, }, Web: webConfig{ RemoteAllowed: false, diff --git a/limits/limits.go b/limits/limits.go index 736a5c02f..b3b93816b 100644 --- a/limits/limits.go +++ b/limits/limits.go @@ -11,3 +11,12 @@ type Trigger struct { // MaxNameSize is the amount of characters allowed in trigger name. MaxNameSize int } + +// GetTestConfig is used for testing. +func GetTestConfig() Config { + return Config{ + Trigger: Trigger{ + MaxNameSize: 200, + }, + } +} From 20c9678f1e90d92bb2f2366cff4883bac0fc9eb2 Mon Sep 17 00:00:00 2001 From: AleksandrMatsko Date: Wed, 4 Sep 2024 10:58:28 +0700 Subject: [PATCH 3/9] test: for validating trigger name --- api/config.go | 3 ++- api/dto/triggers.go | 24 ++++++++++++----- api/dto/triggers_test.go | 50 +++++++++++++++++++++++++++++++++++- api/handler/trigger_test.go | 3 ++- api/handler/triggers_test.go | 3 ++- api/middleware/context.go | 3 ++- api/middleware/middleware.go | 3 ++- cmd/config.go | 3 ++- 8 files changed, 79 insertions(+), 13 deletions(-) diff --git a/api/config.go b/api/config.go index 40d34b44b..edf6682cb 100644 --- a/api/config.go +++ b/api/config.go @@ -1,10 +1,11 @@ package api import ( - "github.com/moira-alert/moira/limits" "net/http" "time" + "github.com/moira-alert/moira/limits" + "github.com/moira-alert/moira" ) diff --git a/api/dto/triggers.go b/api/dto/triggers.go index f7086184a..d3cc53ae9 100644 --- a/api/dto/triggers.go +++ b/api/dto/triggers.go @@ -20,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") + + // 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 = "*" @@ -153,21 +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 be less than %d symbols", limits.Trigger.MaxNameSize)} + ValidationError: fmt.Errorf("trigger name too long, should not be greater than %d symbols", limits.Trigger.MaxNameSize), + } } if err := checkWarnErrorExpression(trigger); err != nil { diff --git a/api/dto/triggers_test.go b/api/dto/triggers_test.go index 0edcb964a..9e9643df0 100644 --- a/api/dto/triggers_test.go +++ b/api/dto/triggers_test.go @@ -3,11 +3,13 @@ package dto import ( "context" "fmt" - "github.com/moira-alert/moira/limits" "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" @@ -19,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() diff --git a/api/handler/trigger_test.go b/api/handler/trigger_test.go index c5f9bc5ee..beb29a35e 100644 --- a/api/handler/trigger_test.go +++ b/api/handler/trigger_test.go @@ -4,13 +4,14 @@ import ( "bytes" "encoding/json" "fmt" - "github.com/moira-alert/moira/limits" "io" "net/http" "net/http/httptest" "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" diff --git a/api/handler/triggers_test.go b/api/handler/triggers_test.go index 5273f9f66..7ae11e287 100644 --- a/api/handler/triggers_test.go +++ b/api/handler/triggers_test.go @@ -5,7 +5,6 @@ import ( "context" "encoding/json" "fmt" - "github.com/moira-alert/moira/limits" "io" "net/http" "net/http/httptest" @@ -13,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" diff --git a/api/middleware/context.go b/api/middleware/context.go index 28af74c07..edcc5f7a9 100644 --- a/api/middleware/context.go +++ b/api/middleware/context.go @@ -3,13 +3,14 @@ package middleware import ( "context" "fmt" - "github.com/moira-alert/moira/limits" "net/http" "net/url" "strconv" "strings" "time" + "github.com/moira-alert/moira/limits" + "github.com/go-chi/chi" "github.com/go-chi/render" "github.com/moira-alert/moira" diff --git a/api/middleware/middleware.go b/api/middleware/middleware.go index fe5af5cc8..15319cac5 100644 --- a/api/middleware/middleware.go +++ b/api/middleware/middleware.go @@ -2,10 +2,11 @@ package middleware import ( "context" - "github.com/moira-alert/moira/limits" "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" diff --git a/cmd/config.go b/cmd/config.go index 71fb5fb20..68f6638ab 100644 --- a/cmd/config.go +++ b/cmd/config.go @@ -3,10 +3,11 @@ package cmd import ( "errors" "fmt" - "github.com/moira-alert/moira/limits" "os" "strings" + "github.com/moira-alert/moira/limits" + "github.com/moira-alert/moira" "github.com/moira-alert/moira/metrics" From e17aa33853813f9dfcc593be83965bdebb60ff9d Mon Sep 17 00:00:00 2001 From: AleksandrMatsko Date: Wed, 4 Sep 2024 12:00:11 +0700 Subject: [PATCH 4/9] style: use linter --- cmd/api/config_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/api/config_test.go b/cmd/api/config_test.go index 1e694849f..f550cfbfc 100644 --- a/cmd/api/config_test.go +++ b/cmd/api/config_test.go @@ -89,7 +89,7 @@ func Test_webConfig_getDefault(t *testing.T) { Listen: ":8081", EnableCORS: false, Limits: cmd.LimitsConfig{ - cmd.TriggerLimitsConfig{ + Trigger: cmd.TriggerLimitsConfig{ MaxNameSize: 200, }, }, From b413fb35625d282e8a66f639778ee0b15c6d83f2 Mon Sep 17 00:00:00 2001 From: AleksandrMatsko Date: Thu, 5 Sep 2024 15:15:18 +0700 Subject: [PATCH 5/9] refactor: make some errors in dto private --- api/dto/triggers.go | 24 ++++++++++++------------ api/dto/triggers_test.go | 10 +++++----- 2 files changed, 17 insertions(+), 17 deletions(-) diff --git a/api/dto/triggers.go b/api/dto/triggers.go index d3cc53ae9..734da69be 100644 --- a/api/dto/triggers.go +++ b/api/dto/triggers.go @@ -21,17 +21,17 @@ import ( var targetNameRegex = regexp.MustCompile("^t\\d+$") 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'") + // 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") + // errTargetsRequired is returned when there is no targets in Trigger. + errTargetsRequired = fmt.Errorf("targets is required") - // ErrTagsRequired is returned when there is no tags in Trigger. - ErrTagsRequired = fmt.Errorf("tags is required") + // 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") + // 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. @@ -164,15 +164,15 @@ 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: ErrTargetsRequired} + return api.ErrInvalidRequestContent{ValidationError: errTargetsRequired} } if len(trigger.Tags) == 0 { - return api.ErrInvalidRequestContent{ValidationError: ErrTagsRequired} + return api.ErrInvalidRequestContent{ValidationError: errTagsRequired} } if trigger.Name == "" { - return api.ErrInvalidRequestContent{ValidationError: ErrTriggerNameRequired} + return api.ErrInvalidRequestContent{ValidationError: errTriggerNameRequired} } limits := middleware.GetLimits(request) @@ -192,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:] diff --git a/api/dto/triggers_test.go b/api/dto/triggers_test.go index 9e9643df0..fc1e545b7 100644 --- a/api/dto/triggers_test.go +++ b/api/dto/triggers_test.go @@ -35,7 +35,7 @@ func TestTriggerValidation(t *testing.T) { Convey("with empty targets", func() { err := trigger.Bind(request) - So(err, ShouldResemble, api.ErrInvalidRequestContent{ValidationError: ErrTargetsRequired}) + So(err, ShouldResemble, api.ErrInvalidRequestContent{ValidationError: errTargetsRequired}) }) trigger.Targets = []string{"foo.bar"} @@ -45,7 +45,7 @@ func TestTriggerValidation(t *testing.T) { err := trigger.Bind(request) - So(err, ShouldResemble, api.ErrInvalidRequestContent{ValidationError: ErrTagsRequired}) + So(err, ShouldResemble, api.ErrInvalidRequestContent{ValidationError: errTagsRequired}) }) trigger.Tags = append(trigger.Tags, "tag1") @@ -53,7 +53,7 @@ func TestTriggerValidation(t *testing.T) { Convey("with empty Name", func() { err := trigger.Bind(request) - So(err, ShouldResemble, api.ErrInvalidRequestContent{ValidationError: ErrTriggerNameRequired}) + So(err, ShouldResemble, api.ErrInvalidRequestContent{ValidationError: errTriggerNameRequired}) }) Convey("with too long Name", func() { @@ -253,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} From d28871ce48f6ce7d4e2b0203685e6e476f096fda Mon Sep 17 00:00:00 2001 From: AleksandrMatsko Date: Fri, 6 Sep 2024 10:24:12 +0700 Subject: [PATCH 6/9] refactor: move code from limits package to api --- api/config.go | 25 ++++++++++++++++++++++--- api/dto/triggers.go | 4 ++-- api/dto/triggers_test.go | 6 ++---- api/handler/trigger_test.go | 14 +++++++------- api/handler/triggers_test.go | 22 ++++++++++------------ api/middleware/context.go | 6 ++---- api/middleware/middleware.go | 6 ++---- cmd/api/config.go | 27 ++++++++++++++++++++++++--- cmd/api/config_test.go | 4 ++-- cmd/config.go | 23 ----------------------- limits/limits.go | 22 ---------------------- 11 files changed, 73 insertions(+), 86 deletions(-) delete mode 100644 limits/limits.go diff --git a/api/config.go b/api/config.go index edf6682cb..c587ee962 100644 --- a/api/config.go +++ b/api/config.go @@ -4,8 +4,6 @@ import ( "net/http" "time" - "github.com/moira-alert/moira/limits" - "github.com/moira-alert/moira" ) @@ -40,7 +38,7 @@ type Config struct { MetricsTTL map[moira.ClusterKey]time.Duration Flags FeatureFlags Authorization Authorization - Limits limits.Config + Limits LimitsConfig } // WebConfig is container for web ui configuration parameters. @@ -63,3 +61,24 @@ type MetricSourceCluster struct { func (WebConfig) Render(w http.ResponseWriter, r *http.Request) error { return nil } + +// 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: 200, + }, + } +} diff --git a/api/dto/triggers.go b/api/dto/triggers.go index 734da69be..fa9ef48f5 100644 --- a/api/dto/triggers.go +++ b/api/dto/triggers.go @@ -25,10 +25,10 @@ var ( 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") + errTargetsRequired = fmt.Errorf("targets are required") // errTagsRequired is returned when there is no tags in Trigger. - errTagsRequired = fmt.Errorf("tags is required") + errTagsRequired = fmt.Errorf("tags are required") // errTriggerNameRequired is returned when there is empty Name in Trigger. errTriggerNameRequired = fmt.Errorf("trigger name is required") diff --git a/api/dto/triggers_test.go b/api/dto/triggers_test.go index fc1e545b7..cddb9b79a 100644 --- a/api/dto/triggers_test.go +++ b/api/dto/triggers_test.go @@ -8,8 +8,6 @@ import ( "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" @@ -26,7 +24,7 @@ func TestTriggerValidation(t *testing.T) { TriggerModel: TriggerModel{}, } - limit := limits.GetTestConfig() + limit := api.GetTestLimitsConfig() request, _ := http.NewRequest("PUT", "/api/trigger", nil) request.Header.Set("Content-Type", "application/json") @@ -80,7 +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"), limits.GetTestConfig()) + ctx = context.WithValue(ctx, middleware.ContextKey("limits"), api.GetTestLimitsConfig()) request = request.WithContext(ctx) desc := "Graphite ClickHouse" diff --git a/api/handler/trigger_test.go b/api/handler/trigger_test.go index beb29a35e..ad51a1840 100644 --- a/api/handler/trigger_test.go +++ b/api/handler/trigger_test.go @@ -10,7 +10,7 @@ import ( "testing" "time" - "github.com/moira-alert/moira/limits" + "github.com/moira-alert/moira/api" "github.com/moira-alert/moira" "github.com/moira-alert/moira/api/dto" @@ -168,7 +168,7 @@ func TestUpdateTrigger(t *testing.T) { 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())) + testRequest = testRequest.WithContext(middleware.SetContextValueForTest(testRequest.Context(), "limits", api.GetTestLimitsConfig())) responseWriter := httptest.NewRecorder() updateTrigger(responseWriter, testRequest) @@ -210,7 +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())) + request = request.WithContext(middleware.SetContextValueForTest(request.Context(), "limits", api.GetTestLimitsConfig())) responseWriter := httptest.NewRecorder() updateTrigger(responseWriter, request) @@ -250,7 +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())) + request = request.WithContext(middleware.SetContextValueForTest(request.Context(), "limits", api.GetTestLimitsConfig())) responseWriter := httptest.NewRecorder() updateTrigger(responseWriter, request) @@ -276,7 +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())) + request = request.WithContext(middleware.SetContextValueForTest(request.Context(), "limits", api.GetTestLimitsConfig())) responseWriter := httptest.NewRecorder() updateTrigger(responseWriter, request) @@ -340,7 +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())) + request = request.WithContext(middleware.SetContextValueForTest(request.Context(), "limits", api.GetTestLimitsConfig())) responseWriter := httptest.NewRecorder() updateTrigger(responseWriter, request) @@ -359,7 +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())) + request = request.WithContext(middleware.SetContextValueForTest(request.Context(), "limits", api.GetTestLimitsConfig())) responseWriter := httptest.NewRecorder() updateTrigger(responseWriter, request) diff --git a/api/handler/triggers_test.go b/api/handler/triggers_test.go index 7ae11e287..c0b1dc52f 100644 --- a/api/handler/triggers_test.go +++ b/api/handler/triggers_test.go @@ -12,8 +12,6 @@ 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" @@ -97,7 +95,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())) + request = request.WithContext(middleware.SetContextValueForTest(request.Context(), "limits", api.GetTestLimitsConfig())) Convey("It should be parsed successfully", func() { triggerDTO.TTL = moira.DefaultTTL @@ -136,7 +134,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())) + request = request.WithContext(middleware.SetContextValueForTest(request.Context(), "limits", api.GetTestLimitsConfig())) Convey("Parser should return en error", func() { _, err := getTriggerFromRequest(request) @@ -255,7 +253,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())) + testRequest = testRequest.WithContext(middleware.SetContextValueForTest(testRequest.Context(), "limits", api.GetTestLimitsConfig())) triggerCheck(responseWriter, testRequest) @@ -320,7 +318,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())) + testRequest = testRequest.WithContext(middleware.SetContextValueForTest(testRequest.Context(), "limits", api.GetTestLimitsConfig())) responseWriter := httptest.NewRecorder() createTrigger(responseWriter, testRequest) @@ -358,7 +356,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())) + request = request.WithContext(middleware.SetContextValueForTest(request.Context(), "limits", api.GetTestLimitsConfig())) responseWriter := httptest.NewRecorder() createTrigger(responseWriter, request) @@ -397,7 +395,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())) + request = request.WithContext(middleware.SetContextValueForTest(request.Context(), "limits", api.GetTestLimitsConfig())) responseWriter := httptest.NewRecorder() createTrigger(responseWriter, request) @@ -421,7 +419,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())) + request = request.WithContext(middleware.SetContextValueForTest(request.Context(), "limits", api.GetTestLimitsConfig())) responseWriter := httptest.NewRecorder() createTrigger(responseWriter, request) @@ -484,7 +482,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())) + request = request.WithContext(middleware.SetContextValueForTest(request.Context(), "limits", api.GetTestLimitsConfig())) responseWriter := httptest.NewRecorder() createTrigger(responseWriter, request) @@ -502,7 +500,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())) + request = request.WithContext(middleware.SetContextValueForTest(request.Context(), "limits", api.GetTestLimitsConfig())) responseWriter := httptest.NewRecorder() createTrigger(responseWriter, request) @@ -722,7 +720,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())) + request = request.WithContext(middleware.SetContextValueForTest(request.Context(), "limits", api.GetTestLimitsConfig())) return request } diff --git a/api/middleware/context.go b/api/middleware/context.go index edcc5f7a9..5373376ab 100644 --- a/api/middleware/context.go +++ b/api/middleware/context.go @@ -9,8 +9,6 @@ import ( "strings" "time" - "github.com/moira-alert/moira/limits" - "github.com/go-chi/chi" "github.com/go-chi/render" "github.com/moira-alert/moira" @@ -334,8 +332,8 @@ 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 { +// 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) diff --git a/api/middleware/middleware.go b/api/middleware/middleware.go index 15319cac5..94459dd60 100644 --- a/api/middleware/middleware.go +++ b/api/middleware/middleware.go @@ -5,8 +5,6 @@ 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" @@ -179,6 +177,6 @@ func GetStates(request *http.Request) map[string]struct{} { } // GetLimits returns configured limits. -func GetLimits(request *http.Request) limits.Config { - return request.Context().Value(limitsContextKey).(limits.Config) +func GetLimits(request *http.Request) api.LimitsConfig { + return request.Context().Value(limitsContextKey).(api.LimitsConfig) } diff --git a/cmd/api/config.go b/cmd/api/config.go index e60c9b0f1..b43e9a407 100644 --- a/cmd/api/config.go +++ b/cmd/api/config.go @@ -47,7 +47,28 @@ type apiConfig struct { // Authorization contains authorization configuration. Authorization authorization `yaml:"authorization"` // Limits contains limits applied to entities and so on. - Limits cmd.LimitsConfig `yaml:"limits"` + Limits LimitsConfig `yaml:"limits"` +} + +// LimitsConfig contains configurable moira limits. +type LimitsConfig struct { + // Trigger contains the limits applied to triggers. + Trigger TriggerLimitsConfig `yaml:"trigger"` +} + +// TriggerLimitsConfig represents the limits which will be applied to all triggers. +type TriggerLimitsConfig struct { + // MaxNameSize is the max amount of characters allowed in trigger name. + MaxNameSize int `yaml:"max_name_size"` +} + +// ToLimits converts LimitsConfig to api.LimitsConfig. +func (conf LimitsConfig) ToLimits() api.LimitsConfig { + return api.LimitsConfig{ + Trigger: api.TriggerLimits{ + MaxNameSize: conf.Trigger.MaxNameSize, + }, + } } type authorization struct { @@ -217,8 +238,8 @@ func getDefault() config { API: apiConfig{ Listen: ":8081", EnableCORS: false, - Limits: cmd.LimitsConfig{ - Trigger: cmd.TriggerLimitsConfig{ + Limits: LimitsConfig{ + Trigger: TriggerLimitsConfig{ MaxNameSize: 200, }, }, diff --git a/cmd/api/config_test.go b/cmd/api/config_test.go index f550cfbfc..693e55783 100644 --- a/cmd/api/config_test.go +++ b/cmd/api/config_test.go @@ -88,8 +88,8 @@ func Test_webConfig_getDefault(t *testing.T) { API: apiConfig{ Listen: ":8081", EnableCORS: false, - Limits: cmd.LimitsConfig{ - Trigger: cmd.TriggerLimitsConfig{ + Limits: LimitsConfig{ + Trigger: TriggerLimitsConfig{ MaxNameSize: 200, }, }, diff --git a/cmd/config.go b/cmd/config.go index 68f6638ab..bb83ea439 100644 --- a/cmd/config.go +++ b/cmd/config.go @@ -6,8 +6,6 @@ import ( "os" "strings" - "github.com/moira-alert/moira/limits" - "github.com/moira-alert/moira" "github.com/moira-alert/moira/metrics" @@ -313,24 +311,3 @@ func PrintConfig(config interface{}) { d, _ := yaml.Marshal(&config) fmt.Println(string(d)) } - -// LimitsConfig contains configurable moira limits. -type LimitsConfig struct { - // Trigger contains the limits applied to triggers. - Trigger TriggerLimitsConfig `yaml:"trigger"` -} - -// TriggerLimitsConfig represents the limits which will be applied to all triggers. -type TriggerLimitsConfig struct { - // MaxNameSize is the max amount of characters allowed in trigger name. - MaxNameSize int `yaml:"max_name_size"` -} - -// ToLimits converts LimitsConfig to limits.Config. -func (conf LimitsConfig) ToLimits() limits.Config { - return limits.Config{ - Trigger: limits.Trigger{ - MaxNameSize: conf.Trigger.MaxNameSize, - }, - } -} diff --git a/limits/limits.go b/limits/limits.go deleted file mode 100644 index b3b93816b..000000000 --- a/limits/limits.go +++ /dev/null @@ -1,22 +0,0 @@ -package limits - -// Config contains limits for some entities. -type Config struct { - // Trigger contains limits for triggers. - Trigger Trigger -} - -// Trigger contains all limits applied for triggers. -type Trigger struct { - // MaxNameSize is the amount of characters allowed in trigger name. - MaxNameSize int -} - -// GetTestConfig is used for testing. -func GetTestConfig() Config { - return Config{ - Trigger: Trigger{ - MaxNameSize: 200, - }, - } -} From 1a64d49447dff0f53c7d8165e6264555b1de59c9 Mon Sep 17 00:00:00 2001 From: AleksandrMatsko Date: Tue, 10 Sep 2024 20:03:31 +0700 Subject: [PATCH 7/9] refactor: move number to constant --- api/config.go | 7 ++++++- cmd/api/config.go | 2 +- cmd/api/config_test.go | 2 +- 3 files changed, 8 insertions(+), 3 deletions(-) diff --git a/api/config.go b/api/config.go index c587ee962..29055ae71 100644 --- a/api/config.go +++ b/api/config.go @@ -62,6 +62,11 @@ 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. @@ -78,7 +83,7 @@ type TriggerLimits struct { func GetTestLimitsConfig() LimitsConfig { return LimitsConfig{ Trigger: TriggerLimits{ - MaxNameSize: 200, + MaxNameSize: DefaultTriggerNameMaxSize, }, } } diff --git a/cmd/api/config.go b/cmd/api/config.go index b43e9a407..a5c8c8718 100644 --- a/cmd/api/config.go +++ b/cmd/api/config.go @@ -240,7 +240,7 @@ func getDefault() config { EnableCORS: false, Limits: LimitsConfig{ Trigger: TriggerLimitsConfig{ - MaxNameSize: 200, + MaxNameSize: api.DefaultTriggerNameMaxSize, }, }, }, diff --git a/cmd/api/config_test.go b/cmd/api/config_test.go index 693e55783..68abc1266 100644 --- a/cmd/api/config_test.go +++ b/cmd/api/config_test.go @@ -90,7 +90,7 @@ func Test_webConfig_getDefault(t *testing.T) { EnableCORS: false, Limits: LimitsConfig{ Trigger: TriggerLimitsConfig{ - MaxNameSize: 200, + MaxNameSize: api.DefaultTriggerNameMaxSize, }, }, }, From 7bcb0da2613ac94e1e41256cb5008d69f907f259 Mon Sep 17 00:00:00 2001 From: AleksandrMatsko Date: Fri, 27 Sep 2024 11:55:33 +0700 Subject: [PATCH 8/9] test: fix panics because of no limits config in middleware --- api/handler/triggers_test.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/api/handler/triggers_test.go b/api/handler/triggers_test.go index 167eb2457..01c3420d3 100644 --- a/api/handler/triggers_test.go +++ b/api/handler/triggers_test.go @@ -192,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") @@ -216,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, @@ -242,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, From dab6158fc8bd22013055fec7d4284131735e8ad0 Mon Sep 17 00:00:00 2001 From: AleksandrMatsko Date: Tue, 1 Oct 2024 10:28:31 +0700 Subject: [PATCH 9/9] refactor: errors declaration --- api/dto/triggers.go | 20 ++++++++++++++------ api/dto/triggers_test.go | 4 ++-- 2 files changed, 16 insertions(+), 8 deletions(-) diff --git a/api/dto/triggers.go b/api/dto/triggers.go index fa9ef48f5..e22e28990 100644 --- a/api/dto/triggers.go +++ b/api/dto/triggers.go @@ -2,6 +2,7 @@ package dto import ( + "errors" "fmt" "net/http" "regexp" @@ -22,16 +23,23 @@ var targetNameRegex = regexp.MustCompile("^t\\d+$") 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'") + 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 = fmt.Errorf("targets are required") + errTargetsRequired = errors.New("targets are required") // errTagsRequired is returned when there is no tags in Trigger. - errTagsRequired = fmt.Errorf("tags are required") + errTagsRequired = errors.New("tags are required") // errTriggerNameRequired is returned when there is empty Name in Trigger. - errTriggerNameRequired = fmt.Errorf("trigger name is required") + 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. @@ -202,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} } } @@ -243,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} } } diff --git a/api/dto/triggers_test.go b/api/dto/triggers_test.go index cddb9b79a..be944ab20 100644 --- a/api/dto/triggers_test.go +++ b/api/dto/triggers_test.go @@ -263,7 +263,7 @@ func TestTriggerValidation(t *testing.T) { 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}) }) }) @@ -285,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}) }) }) })