From cdb5d4230aea344bea228bbc47c088c3bf0e91c1 Mon Sep 17 00:00:00 2001 From: Santiago Date: Wed, 26 Oct 2022 23:35:52 -0300 Subject: [PATCH] Alerting: Fix "Not Implemented" responses (#57710) * fix swagger spec, return 404 instead of 501 when an endpoint does not exist * update number of paths in authorization_test.go --- pkg/services/ngalert/api/api_alertmanager.go | 4 - pkg/services/ngalert/api/authorization.go | 4 - .../ngalert/api/authorization_test.go | 2 +- .../ngalert/api/forking_alertmanager.go | 13 --- .../api/generated_base_api_alertmanager.go | 40 ------- pkg/services/ngalert/api/lotex_am.go | 4 - pkg/services/ngalert/api/tooling/api.json | 5 +- .../api/tooling/definitions/alertmanager.go | 28 +---- pkg/services/ngalert/api/tooling/post.json | 105 +----------------- pkg/services/ngalert/api/tooling/spec.json | 105 +----------------- pkg/services/ngalert/api/util.go | 2 - 11 files changed, 10 insertions(+), 302 deletions(-) diff --git a/pkg/services/ngalert/api/api_alertmanager.go b/pkg/services/ngalert/api/api_alertmanager.go index a79d003c3cc..f1c4f0ceb2a 100644 --- a/pkg/services/ngalert/api/api_alertmanager.go +++ b/pkg/services/ngalert/api/api_alertmanager.go @@ -247,10 +247,6 @@ func (srv AlertmanagerSrv) RoutePostAlertingConfig(c *models.ReqContext, body ap return ErrResp(http.StatusInternalServerError, err, "") } -func (srv AlertmanagerSrv) RoutePostAMAlerts(_ *models.ReqContext, _ apimodels.PostableAlerts) response.Response { - return NotImplementedResp -} - func (srv AlertmanagerSrv) RouteGetReceivers(c *models.ReqContext) response.Response { am, errResp := srv.AlertmanagerFor(c.OrgID) if errResp != nil { diff --git a/pkg/services/ngalert/api/authorization.go b/pkg/services/ngalert/api/authorization.go index 980aca94cb3..aa638568b7f 100644 --- a/pkg/services/ngalert/api/authorization.go +++ b/pkg/services/ngalert/api/authorization.go @@ -114,8 +114,6 @@ func (api *API) authorize(method, path string) web.Handler { eval = ac.EvalPermission(ac.ActionAlertingInstanceRead) case http.MethodGet + "/api/alertmanager/grafana/api/v2/alerts": eval = ac.EvalPermission(ac.ActionAlertingInstanceRead) - case http.MethodPost + "/api/alertmanager/grafana/api/v2/alerts": - eval = ac.EvalAny(ac.EvalPermission(ac.ActionAlertingInstanceCreate), ac.EvalPermission(ac.ActionAlertingInstanceUpdate)) // Grafana Prometheus-compatible Paths case http.MethodGet + "/api/prometheus/grafana/api/v1/alerts": @@ -171,8 +169,6 @@ func (api *API) authorize(method, path string) web.Handler { eval = ac.EvalPermission(ac.ActionAlertingNotificationsExternalRead, datasources.ScopeProvider.GetResourceScopeUID(ac.Parameter(":DatasourceUID"))) case http.MethodPost + "/api/alertmanager/{DatasourceUID}/config/api/v1/alerts": eval = ac.EvalPermission(ac.ActionAlertingNotificationsExternalWrite, datasources.ScopeProvider.GetResourceScopeUID(ac.Parameter(":DatasourceUID"))) - case http.MethodPost + "/api/alertmanager/{DatasourceUID}/config/api/v1/receivers/test": - eval = ac.EvalPermission(ac.ActionAlertingNotificationsExternalRead, datasources.ScopeProvider.GetResourceScopeUID(ac.Parameter(":DatasourceUID"))) case http.MethodGet + "/api/v1/ngalert": // let user with any alerting permission access this API diff --git a/pkg/services/ngalert/api/authorization_test.go b/pkg/services/ngalert/api/authorization_test.go index b4da61566fd..d8b67290309 100644 --- a/pkg/services/ngalert/api/authorization_test.go +++ b/pkg/services/ngalert/api/authorization_test.go @@ -49,7 +49,7 @@ func TestAuthorize(t *testing.T) { } paths[p] = methods } - require.Len(t, paths, 41) + require.Len(t, paths, 40) ac := acmock.New() api := &API{AccessControl: ac} diff --git a/pkg/services/ngalert/api/forking_alertmanager.go b/pkg/services/ngalert/api/forking_alertmanager.go index 46314c784e2..c6d2a43ef8b 100644 --- a/pkg/services/ngalert/api/forking_alertmanager.go +++ b/pkg/services/ngalert/api/forking_alertmanager.go @@ -131,15 +131,6 @@ func (f *AlertmanagerApiHandler) handleRoutePostAMAlerts(ctx *models.ReqContext, return s.RoutePostAMAlerts(ctx, body) } -func (f *AlertmanagerApiHandler) handleRoutePostTestReceivers(ctx *models.ReqContext, body apimodels.TestReceiversConfigBodyParams, dsUID string) response.Response { - s, err := f.getService(ctx) - if err != nil { - return errorToResponse(err) - } - - return s.RoutePostTestReceivers(ctx, body) -} - func (f *AlertmanagerApiHandler) handleRouteDeleteGrafanaSilence(ctx *models.ReqContext, id string) response.Response { return f.GrafanaSvc.RouteDeleteSilence(ctx, id) } @@ -176,10 +167,6 @@ func (f *AlertmanagerApiHandler) handleRouteGetGrafanaSilences(ctx *models.ReqCo return f.GrafanaSvc.RouteGetSilences(ctx) } -func (f *AlertmanagerApiHandler) handleRoutePostGrafanaAMAlerts(ctx *models.ReqContext, conf apimodels.PostableAlerts) response.Response { - return f.GrafanaSvc.RoutePostAMAlerts(ctx, conf) -} - func (f *AlertmanagerApiHandler) handleRoutePostGrafanaAlertingConfig(ctx *models.ReqContext, conf apimodels.PostableUserConfig) response.Response { if !conf.AlertmanagerConfig.ReceiverType().Can(apimodels.GrafanaReceiverType) { return errorToResponse(backendTypeDoesNotMatchPayloadTypeError(apimodels.GrafanaBackend, conf.AlertmanagerConfig.ReceiverType().String())) diff --git a/pkg/services/ngalert/api/generated_base_api_alertmanager.go b/pkg/services/ngalert/api/generated_base_api_alertmanager.go index 81ec3137c31..c6b35d35fe0 100644 --- a/pkg/services/ngalert/api/generated_base_api_alertmanager.go +++ b/pkg/services/ngalert/api/generated_base_api_alertmanager.go @@ -40,10 +40,8 @@ type AlertmanagerApi interface { RouteGetSilences(*models.ReqContext) response.Response RoutePostAMAlerts(*models.ReqContext) response.Response RoutePostAlertingConfig(*models.ReqContext) response.Response - RoutePostGrafanaAMAlerts(*models.ReqContext) response.Response RoutePostGrafanaAlertingConfig(*models.ReqContext) response.Response RoutePostTestGrafanaReceivers(*models.ReqContext) response.Response - RoutePostTestReceivers(*models.ReqContext) response.Response } func (f *AlertmanagerApiHandler) RouteCreateGrafanaSilence(ctx *models.ReqContext) response.Response { @@ -157,14 +155,6 @@ func (f *AlertmanagerApiHandler) RoutePostAlertingConfig(ctx *models.ReqContext) } return f.handleRoutePostAlertingConfig(ctx, conf, datasourceUIDParam) } -func (f *AlertmanagerApiHandler) RoutePostGrafanaAMAlerts(ctx *models.ReqContext) response.Response { - // Parse Request Body - conf := apimodels.PostableAlerts{} - if err := web.Bind(ctx.Req, &conf); err != nil { - return response.Error(http.StatusBadRequest, "bad request data", err) - } - return f.handleRoutePostGrafanaAMAlerts(ctx, conf) -} func (f *AlertmanagerApiHandler) RoutePostGrafanaAlertingConfig(ctx *models.ReqContext) response.Response { // Parse Request Body conf := apimodels.PostableUserConfig{} @@ -181,16 +171,6 @@ func (f *AlertmanagerApiHandler) RoutePostTestGrafanaReceivers(ctx *models.ReqCo } return f.handleRoutePostTestGrafanaReceivers(ctx, conf) } -func (f *AlertmanagerApiHandler) RoutePostTestReceivers(ctx *models.ReqContext) response.Response { - // Parse Path Parameters - datasourceUIDParam := web.Params(ctx.Req)[":DatasourceUID"] - // Parse Request Body - conf := apimodels.TestReceiversConfigBodyParams{} - if err := web.Bind(ctx.Req, &conf); err != nil { - return response.Error(http.StatusBadRequest, "bad request data", err) - } - return f.handleRoutePostTestReceivers(ctx, conf, datasourceUIDParam) -} func (api *API) RegisterAlertmanagerApiEndpoints(srv AlertmanagerApi, m *metrics.API) { api.RouteRegister.Group("", func(group routing.RouteRegister) { @@ -404,16 +384,6 @@ func (api *API) RegisterAlertmanagerApiEndpoints(srv AlertmanagerApi, m *metrics m, ), ) - group.Post( - toMacaronPath("/api/alertmanager/grafana/api/v2/alerts"), - api.authorize(http.MethodPost, "/api/alertmanager/grafana/api/v2/alerts"), - metrics.Instrument( - http.MethodPost, - "/api/alertmanager/grafana/api/v2/alerts", - srv.RoutePostGrafanaAMAlerts, - m, - ), - ) group.Post( toMacaronPath("/api/alertmanager/grafana/config/api/v1/alerts"), api.authorize(http.MethodPost, "/api/alertmanager/grafana/config/api/v1/alerts"), @@ -434,15 +404,5 @@ func (api *API) RegisterAlertmanagerApiEndpoints(srv AlertmanagerApi, m *metrics m, ), ) - group.Post( - toMacaronPath("/api/alertmanager/{DatasourceUID}/config/api/v1/receivers/test"), - api.authorize(http.MethodPost, "/api/alertmanager/{DatasourceUID}/config/api/v1/receivers/test"), - metrics.Instrument( - http.MethodPost, - "/api/alertmanager/{DatasourceUID}/config/api/v1/receivers/test", - srv.RoutePostTestReceivers, - m, - ), - ) }, middleware.ReqSignedIn) } diff --git a/pkg/services/ngalert/api/lotex_am.go b/pkg/services/ngalert/api/lotex_am.go index 34bbf3a87b9..9db6b947767 100644 --- a/pkg/services/ngalert/api/lotex_am.go +++ b/pkg/services/ngalert/api/lotex_am.go @@ -255,7 +255,3 @@ func (am *LotexAM) RoutePostAMAlerts(ctx *models.ReqContext, alerts apimodels.Po nil, ) } - -func (am *LotexAM) RoutePostTestReceivers(ctx *models.ReqContext, config apimodels.TestReceiversConfigBodyParams) response.Response { - return NotImplementedResp -} diff --git a/pkg/services/ngalert/api/tooling/api.json b/pkg/services/ngalert/api/tooling/api.json index 8118db6f34c..38c5f290c4d 100644 --- a/pkg/services/ngalert/api/tooling/api.json +++ b/pkg/services/ngalert/api/tooling/api.json @@ -3301,6 +3301,7 @@ "type": "object" }, "alertGroups": { + "description": "AlertGroups alert groups", "items": { "$ref": "#/definitions/alertGroup" }, @@ -3405,6 +3406,7 @@ "type": "object" }, "gettableAlert": { + "description": "GettableAlert gettable alert", "properties": { "annotations": { "$ref": "#/definitions/labelSet" @@ -3460,6 +3462,7 @@ "type": "object" }, "gettableAlerts": { + "description": "GettableAlerts gettable alerts", "items": { "$ref": "#/definitions/gettableAlert" }, @@ -3664,7 +3667,6 @@ "type": "array" }, "postableSilence": { - "description": "PostableSilence postable silence", "properties": { "comment": { "description": "comment", @@ -3702,6 +3704,7 @@ "type": "object" }, "receiver": { + "description": "Receiver receiver", "properties": { "active": { "description": "active", diff --git a/pkg/services/ngalert/api/tooling/definitions/alertmanager.go b/pkg/services/ngalert/api/tooling/definitions/alertmanager.go index d7463591c98..d0ffd5c4cae 100644 --- a/pkg/services/ngalert/api/tooling/definitions/alertmanager.go +++ b/pkg/services/ngalert/api/tooling/definitions/alertmanager.go @@ -108,14 +108,6 @@ import ( // 400: ValidationError // 404: NotFound -// swagger:route POST /api/alertmanager/grafana/api/v2/alerts alertmanager RoutePostGrafanaAMAlerts -// -// create alertmanager alerts -// -// Responses: -// 200: Ack -// 400: ValidationError - // swagger:route POST /api/alertmanager/{DatasourceUID}/api/v2/alerts alertmanager RoutePostAMAlerts // // create alertmanager alerts @@ -163,20 +155,6 @@ import ( // 408: Failure // 409: AlertManagerNotReady -// swagger:route POST /api/alertmanager/{DatasourceUID}/config/api/v1/receivers/test alertmanager RoutePostTestReceivers -// -// Test Grafana managed receivers without saving them. -// -// Responses: -// -// 200: Ack -// 207: MultiStatus -// 400: ValidationError -// 403: PermissionDenied -// 404: NotFound -// 408: Failure -// 409: AlertManagerNotReady - // swagger:route GET /api/alertmanager/grafana/api/v2/silences alertmanager RouteGetGrafanaSilences // // get silences @@ -254,7 +232,7 @@ type AlertManagerNotReady struct{} // swagger:model type MultiStatus struct{} -// swagger:parameters RoutePostTestReceivers RoutePostTestGrafanaReceivers +// swagger:parameters RoutePostTestGrafanaReceivers type TestReceiversConfigParams struct { // in:body Body TestReceiversConfigBodyParams @@ -457,7 +435,7 @@ type AlertsParams struct { Receivers string `json:"receiver"` } -// swagger:parameters RoutePostAMAlerts RoutePostGrafanaAMAlerts +// swagger:parameters RoutePostAMAlerts type PostableAlerts struct { // in:body PostableAlerts []amv2.PostableAlert `yaml:"" json:""` @@ -470,7 +448,7 @@ type BodyAlertingConfig struct { } // alertmanager routes -// swagger:parameters RoutePostAlertingConfig RouteGetAlertingConfig RouteDeleteAlertingConfig RouteGetAMStatus RouteGetAMAlerts RoutePostAMAlerts RouteGetAMAlertGroups RouteGetSilences RouteCreateSilence RouteGetSilence RouteDeleteSilence RoutePostAlertingConfig RoutePostTestReceivers +// swagger:parameters RoutePostAlertingConfig RouteGetAlertingConfig RouteDeleteAlertingConfig RouteGetAMStatus RouteGetAMAlerts RoutePostAMAlerts RouteGetAMAlertGroups RouteGetSilences RouteCreateSilence RouteGetSilence RouteDeleteSilence RoutePostAlertingConfig // testing routes // swagger:parameters RouteTestRuleConfig // prom routes diff --git a/pkg/services/ngalert/api/tooling/post.json b/pkg/services/ngalert/api/tooling/post.json index 0afdc9e3f52..d1820889b50 100644 --- a/pkg/services/ngalert/api/tooling/post.json +++ b/pkg/services/ngalert/api/tooling/post.json @@ -3302,7 +3302,6 @@ "type": "object" }, "alertGroups": { - "description": "AlertGroups alert groups", "items": { "$ref": "#/definitions/alertGroup" }, @@ -3463,6 +3462,7 @@ "type": "object" }, "gettableAlerts": { + "description": "GettableAlerts gettable alerts", "items": { "$ref": "#/definitions/gettableAlert" }, @@ -3668,7 +3668,6 @@ "type": "array" }, "postableSilence": { - "description": "PostableSilence postable silence", "properties": { "comment": { "description": "comment", @@ -3884,39 +3883,6 @@ "tags": [ "alertmanager" ] - }, - "post": { - "description": "create alertmanager alerts", - "operationId": "RoutePostGrafanaAMAlerts", - "parameters": [ - { - "in": "body", - "name": "PostableAlerts", - "schema": { - "items": { - "$ref": "#/definitions/postableAlert" - }, - "type": "array" - } - } - ], - "responses": { - "200": { - "description": "Ack", - "schema": { - "$ref": "#/definitions/Ack" - } - }, - "400": { - "description": "ValidationError", - "schema": { - "$ref": "#/definitions/ValidationError" - } - } - }, - "tags": [ - "alertmanager" - ] } }, "/api/alertmanager/grafana/api/v2/alerts/groups": { @@ -4800,75 +4766,6 @@ ] } }, - "/api/alertmanager/{DatasourceUID}/config/api/v1/receivers/test": { - "post": { - "operationId": "RoutePostTestReceivers", - "parameters": [ - { - "in": "body", - "name": "Body", - "schema": { - "$ref": "#/definitions/TestReceiversConfigBodyParams" - } - }, - { - "description": "DatasoureUID should be the datasource UID identifier", - "in": "path", - "name": "DatasourceUID", - "required": true, - "type": "string" - } - ], - "responses": { - "200": { - "description": "Ack", - "schema": { - "$ref": "#/definitions/Ack" - } - }, - "207": { - "description": "MultiStatus", - "schema": { - "$ref": "#/definitions/MultiStatus" - } - }, - "400": { - "description": "ValidationError", - "schema": { - "$ref": "#/definitions/ValidationError" - } - }, - "403": { - "description": "PermissionDenied", - "schema": { - "$ref": "#/definitions/PermissionDenied" - } - }, - "404": { - "description": "NotFound", - "schema": { - "$ref": "#/definitions/NotFound" - } - }, - "408": { - "description": "Failure", - "schema": { - "$ref": "#/definitions/Failure" - } - }, - "409": { - "description": "AlertManagerNotReady", - "schema": { - "$ref": "#/definitions/AlertManagerNotReady" - } - } - }, - "summary": "Test Grafana managed receivers without saving them.", - "tags": [ - "alertmanager" - ] - } - }, "/api/prometheus/grafana/api/v1/alerts": { "get": { "description": "gets the current alerts", diff --git a/pkg/services/ngalert/api/tooling/spec.json b/pkg/services/ngalert/api/tooling/spec.json index aff5e10bca4..684ebd16710 100644 --- a/pkg/services/ngalert/api/tooling/spec.json +++ b/pkg/services/ngalert/api/tooling/spec.json @@ -76,39 +76,6 @@ } } } - }, - "post": { - "description": "create alertmanager alerts", - "tags": [ - "alertmanager" - ], - "operationId": "RoutePostGrafanaAMAlerts", - "parameters": [ - { - "name": "PostableAlerts", - "in": "body", - "schema": { - "type": "array", - "items": { - "$ref": "#/definitions/postableAlert" - } - } - } - ], - "responses": { - "200": { - "description": "Ack", - "schema": { - "$ref": "#/definitions/Ack" - } - }, - "400": { - "description": "ValidationError", - "schema": { - "$ref": "#/definitions/ValidationError" - } - } - } } }, "/api/alertmanager/grafana/api/v2/alerts/groups": { @@ -992,75 +959,6 @@ } } }, - "/api/alertmanager/{DatasourceUID}/config/api/v1/receivers/test": { - "post": { - "tags": [ - "alertmanager" - ], - "summary": "Test Grafana managed receivers without saving them.", - "operationId": "RoutePostTestReceivers", - "parameters": [ - { - "name": "Body", - "in": "body", - "schema": { - "$ref": "#/definitions/TestReceiversConfigBodyParams" - } - }, - { - "type": "string", - "description": "DatasoureUID should be the datasource UID identifier", - "name": "DatasourceUID", - "in": "path", - "required": true - } - ], - "responses": { - "200": { - "description": "Ack", - "schema": { - "$ref": "#/definitions/Ack" - } - }, - "207": { - "description": "MultiStatus", - "schema": { - "$ref": "#/definitions/MultiStatus" - } - }, - "400": { - "description": "ValidationError", - "schema": { - "$ref": "#/definitions/ValidationError" - } - }, - "403": { - "description": "PermissionDenied", - "schema": { - "$ref": "#/definitions/PermissionDenied" - } - }, - "404": { - "description": "NotFound", - "schema": { - "$ref": "#/definitions/NotFound" - } - }, - "408": { - "description": "Failure", - "schema": { - "$ref": "#/definitions/Failure" - } - }, - "409": { - "description": "AlertManagerNotReady", - "schema": { - "$ref": "#/definitions/AlertManagerNotReady" - } - } - } - } - }, "/api/prometheus/grafana/api/v1/alerts": { "get": { "description": "gets the current alerts", @@ -5842,7 +5740,6 @@ "$ref": "#/definitions/alertGroup" }, "alertGroups": { - "description": "AlertGroups alert groups", "type": "array", "items": { "$ref": "#/definitions/alertGroup" @@ -6005,6 +5902,7 @@ "$ref": "#/definitions/gettableAlert" }, "gettableAlerts": { + "description": "GettableAlerts gettable alerts", "type": "array", "items": { "$ref": "#/definitions/gettableAlert" @@ -6214,7 +6112,6 @@ } }, "postableSilence": { - "description": "PostableSilence postable silence", "type": "object", "required": [ "comment", diff --git a/pkg/services/ngalert/api/util.go b/pkg/services/ngalert/api/util.go index 9519eaae9ae..3a101965855 100644 --- a/pkg/services/ngalert/api/util.go +++ b/pkg/services/ngalert/api/util.go @@ -27,8 +27,6 @@ import ( var searchRegex = regexp.MustCompile(`\{(\w+)\}`) -var NotImplementedResp = ErrResp(http.StatusNotImplemented, errors.New("endpoint not implemented"), "") - func toMacaronPath(path string) string { return string(searchRegex.ReplaceAllFunc([]byte(path), func(s []byte) []byte { m := string(s[1 : len(s)-1])