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

fix: validating prometheus target #1077

Merged
merged 15 commits into from
Sep 26, 2024
Merged

fix: validating prometheus target #1077

Changes from 2 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
797daa1
feat: prometheus error handling in getTriggerFromRequest and in trigg…
AleksandrMatsko Aug 30, 2024
f02ae6c
refactor: error message
AleksandrMatsko Aug 30, 2024
80eb5fd
test: for handling errors returned from prometheus
AleksandrMatsko Sep 3, 2024
cec15c1
test: for graphite remote
AleksandrMatsko Sep 3, 2024
389994d
style: use linter
AleksandrMatsko Sep 3, 2024
681a1eb
merge origin/master into fix/validating-prometheus-target
AleksandrMatsko Sep 3, 2024
e5a223c
refactor: change status code returned by `/api/trigger/check` for pro…
AleksandrMatsko Sep 10, 2024
dfba55e
merge master into fix/validating-prometheus-target
AleksandrMatsko Sep 17, 2024
d6cd538
refactor: add debug prints for error handling
AleksandrMatsko Sep 18, 2024
339fd16
refactor: remove debug prints
AleksandrMatsko Sep 18, 2024
f219c88
fix: handling error type of prometheus errors
AleksandrMatsko Sep 18, 2024
edfaa20
fix: add return statement, use linter
AleksandrMatsko Sep 18, 2024
6808ccf
merge master into fix/validating-prometheus-target
AleksandrMatsko Sep 24, 2024
2854ea1
refactor: 4xx code recognition
AleksandrMatsko Sep 24, 2024
0fa8b25
fix: use int64 in 4xx checking code func
AleksandrMatsko Sep 24, 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
28 changes: 24 additions & 4 deletions api/handler/triggers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ import (
"strings"
"time"

prometheus "github.com/prometheus/client_golang/api/prometheus/v1"
kissken marked this conversation as resolved.
Show resolved Hide resolved

"github.com/go-chi/chi"
"github.com/go-chi/render"
"github.com/moira-alert/moira"
Expand Down Expand Up @@ -153,7 +155,7 @@ func createTrigger(writer http.ResponseWriter, request *http.Request) {
func getTriggerFromRequest(request *http.Request) (*dto.Trigger, *api.ErrorResponse) {
trigger := &dto.Trigger{}
if err := render.Bind(request, trigger); err != nil {
switch err.(type) { // nolint:errorlint
switch typedErr := err.(type) { // nolint:errorlint
case local.ErrParseExpr, local.ErrEvalExpr, local.ErrUnknownFunction:
return nil, api.ErrorInvalidRequest(fmt.Errorf("invalid graphite targets: %s", err.Error()))
case expression.ErrInvalidExpression:
Expand All @@ -169,6 +171,13 @@ func getTriggerFromRequest(request *http.Request) (*dto.Trigger, *api.ErrorRespo
return nil, response
case *json.UnmarshalTypeError:
return nil, api.ErrorInvalidRequest(fmt.Errorf("invalid payload: %s", err.Error()))
case *prometheus.Error:
almostinf marked this conversation as resolved.
Show resolved Hide resolved
switch typedErr.Type {
case prometheus.ErrBadData:
return nil, api.ErrorInvalidRequest(fmt.Errorf("invalid prometheus targets: %w", err))
default:
return nil, api.ErrorInternalServer(err)
}
default:
return nil, api.ErrorInternalServer(err)
}
Expand Down Expand Up @@ -208,10 +217,21 @@ func triggerCheck(writer http.ResponseWriter, request *http.Request) {
response := dto.TriggerCheckResponse{}

if err := render.Bind(request, trigger); err != nil {
switch err.(type) { // nolint:errorlint
switch typedErr := err.(type) { // nolint:errorlint
case expression.ErrInvalidExpression, local.ErrParseExpr, local.ErrEvalExpr, local.ErrUnknownFunction:
// TODO write comment, why errors are ignored, it is not obvious.
// In getTriggerFromRequest these types of errors lead to 400.
// TODO: move ErrInvalidExpression to separate case

// These errors are skipped because if there are error from local source then it will be caught in
// dto.TargetVerification and will be explained in detail.
case *prometheus.Error:
switch typedErr.Type {
case prometheus.ErrBadData:
render.Render(writer, request, api.ErrorInvalidRequest(fmt.Errorf("invalid prometheus targets: %w", err))) //nolint
return
default:
render.Render(writer, request, api.ErrorInvalidRequest(err)) //nolint
almostinf marked this conversation as resolved.
Show resolved Hide resolved
return
}
default:
render.Render(writer, request, api.ErrorInvalidRequest(err)) //nolint
return
Expand Down
Loading