From 8b8fc293b79a7d21f263d5a8e422fb6fa85eaf3c Mon Sep 17 00:00:00 2001 From: Owen Diehl Date: Wed, 7 Apr 2021 15:36:50 -0400 Subject: [PATCH] safer, more idiomatic proxy helper (#32732) Co-authored-by: Sofia Papagiannaki --- pkg/services/ngalert/api/lotex.go | 107 ++++++------- pkg/services/ngalert/api/lotex_am.go | 150 +++++++++--------- pkg/services/ngalert/api/lotex_prom.go | 28 ++-- .../test-data/am-alertmanager-recipient.http | 30 ++-- pkg/services/ngalert/api/util.go | 14 +- 5 files changed, 171 insertions(+), 158 deletions(-) diff --git a/pkg/services/ngalert/api/lotex.go b/pkg/services/ngalert/api/lotex.go index 8aacbd447bd..1e9b4d0a8d4 100644 --- a/pkg/services/ngalert/api/lotex.go +++ b/pkg/services/ngalert/api/lotex.go @@ -3,8 +3,6 @@ package api import ( "bytes" "fmt" - "io" - "io/ioutil" "net/http" "net/url" @@ -40,14 +38,14 @@ func (r *LotexRuler) RouteDeleteNamespaceRulesConfig(ctx *models.ReqContext) res } return r.withReq( ctx, - &http.Request{ - Method: "DELETE", - URL: withPath( - *ctx.Req.URL, - fmt.Sprintf("%s/%s", legacyRulerPrefix, ctx.Params("Namespace")), - ), - }, + http.MethodDelete, + withPath( + *ctx.Req.URL, + fmt.Sprintf("%s/%s", legacyRulerPrefix, ctx.Params("Namespace")), + ), + nil, messageExtractor, + nil, ) } @@ -58,19 +56,19 @@ func (r *LotexRuler) RouteDeleteRuleGroupConfig(ctx *models.ReqContext) response } return r.withReq( ctx, - &http.Request{ - Method: "DELETE", - URL: withPath( - *ctx.Req.URL, - fmt.Sprintf( - "%s/%s/%s", - legacyRulerPrefix, - ctx.Params("Namespace"), - ctx.Params("Groupname"), - ), + http.MethodDelete, + withPath( + *ctx.Req.URL, + fmt.Sprintf( + "%s/%s/%s", + legacyRulerPrefix, + ctx.Params("Namespace"), + ctx.Params("Groupname"), ), - }, + ), + nil, messageExtractor, + nil, ) } @@ -80,17 +78,19 @@ func (r *LotexRuler) RouteGetNamespaceRulesConfig(ctx *models.ReqContext) respon return response.Error(500, err.Error(), nil) } return r.withReq( - ctx, &http.Request{ - URL: withPath( - *ctx.Req.URL, - fmt.Sprintf( - "%s/%s", - legacyRulerPrefix, - ctx.Params("Namespace"), - ), + ctx, + http.MethodGet, + withPath( + *ctx.Req.URL, + fmt.Sprintf( + "%s/%s", + legacyRulerPrefix, + ctx.Params("Namespace"), ), - }, + ), + nil, yamlExtractor(apimodels.NamespaceConfigResponse{}), + nil, ) } @@ -101,18 +101,19 @@ func (r *LotexRuler) RouteGetRulegGroupConfig(ctx *models.ReqContext) response.R } return r.withReq( ctx, - &http.Request{ - URL: withPath( - *ctx.Req.URL, - fmt.Sprintf( - "%s/%s/%s", - legacyRulerPrefix, - ctx.Params("Namespace"), - ctx.Params("Groupname"), - ), + http.MethodGet, + withPath( + *ctx.Req.URL, + fmt.Sprintf( + "%s/%s/%s", + legacyRulerPrefix, + ctx.Params("Namespace"), + ctx.Params("Groupname"), ), - }, + ), + nil, yamlExtractor(apimodels.RuleGroupConfigResponse{}), + nil, ) } @@ -123,13 +124,14 @@ func (r *LotexRuler) RouteGetRulesConfig(ctx *models.ReqContext) response.Respon } return r.withReq( ctx, - &http.Request{ - URL: withPath( - *ctx.Req.URL, - legacyRulerPrefix, - ), - }, + http.MethodGet, + withPath( + *ctx.Req.URL, + legacyRulerPrefix, + ), + nil, yamlExtractor(apimodels.NamespaceConfigResponse{}), + nil, ) } @@ -142,18 +144,9 @@ func (r *LotexRuler) RoutePostNameRulesConfig(ctx *models.ReqContext, conf apimo if err != nil { return response.Error(500, "Failed marshal rule group", err) } - body, ln := payload(yml) - ns := ctx.Params("Namespace") - u := withPath(*ctx.Req.URL, fmt.Sprintf("%s/%s", legacyRulerPrefix, ns)) - req := &http.Request{ - Method: "POST", - URL: u, - Body: body, - ContentLength: ln, - } - return r.withReq(ctx, req, jsonExtractor(nil)) + return r.withReq(ctx, http.MethodPost, u, bytes.NewBuffer(yml), jsonExtractor(nil), nil) } func (r *LotexRuler) getPrefix(ctx *models.ReqContext) (string, error) { @@ -173,7 +166,3 @@ func withPath(u url.URL, newPath string) *url.URL { u.Path = newPath return &u } - -func payload(b []byte) (io.ReadCloser, int64) { - return ioutil.NopCloser(bytes.NewBuffer(b)), int64(len(b)) -} diff --git a/pkg/services/ngalert/api/lotex_am.go b/pkg/services/ngalert/api/lotex_am.go index 1fc4a66bd03..89b946fba85 100644 --- a/pkg/services/ngalert/api/lotex_am.go +++ b/pkg/services/ngalert/api/lotex_am.go @@ -1,6 +1,7 @@ package api import ( + "bytes" "encoding/json" "fmt" "net/http" @@ -37,102 +38,111 @@ func (am *LotexAM) RouteCreateSilence(ctx *models.ReqContext, silenceBody apimod if err != nil { return response.Error(500, "Failed marshal silence", err) } - body, ln := payload(blob) return am.withReq( - ctx, &http.Request{ - Method: "POST", - URL: withPath(*ctx.Req.URL, amSilencesPath), - Body: body, - ContentLength: ln, - Header: map[string][]string{"Content-Type": {"application/json"}}, - }, - jsonExtractor(nil), + ctx, + http.MethodPost, + withPath(*ctx.Req.URL, amSilencesPath), + bytes.NewBuffer(blob), + jsonExtractor(&apimodels.GettableSilence{}), + map[string]string{"Content-Type": "application/json"}, ) } func (am *LotexAM) RouteDeleteAlertingConfig(ctx *models.ReqContext) response.Response { return am.withReq( - ctx, &http.Request{ - Method: "DELETE", - URL: withPath( - *ctx.Req.URL, - amConfigPath, - ), - }, + ctx, + http.MethodDelete, + withPath( + *ctx.Req.URL, + amConfigPath, + ), + nil, messageExtractor, + nil, ) } func (am *LotexAM) RouteDeleteSilence(ctx *models.ReqContext) response.Response { return am.withReq( - ctx, &http.Request{ - Method: "DELETE", - URL: withPath( - *ctx.Req.URL, - fmt.Sprintf(amSilencePath, ctx.Params(":SilenceId")), - ), - }, + ctx, + http.MethodDelete, + withPath( + *ctx.Req.URL, + fmt.Sprintf(amSilencePath, ctx.Params(":SilenceId")), + ), + nil, messageExtractor, + nil, ) } func (am *LotexAM) RouteGetAlertingConfig(ctx *models.ReqContext) response.Response { return am.withReq( - ctx, &http.Request{ - URL: withPath( - *ctx.Req.URL, - amConfigPath, - ), - }, + ctx, + http.MethodGet, + withPath( + *ctx.Req.URL, + amConfigPath, + ), + nil, yamlExtractor(&apimodels.GettableUserConfig{}), + nil, ) } func (am *LotexAM) RouteGetAMAlertGroups(ctx *models.ReqContext) response.Response { return am.withReq( - ctx, &http.Request{ - URL: withPath( - *ctx.Req.URL, - amAlertGroupsPath, - ), - }, + ctx, + http.MethodGet, + withPath( + *ctx.Req.URL, + amAlertGroupsPath, + ), + nil, jsonExtractor(&apimodels.AlertGroups{}), + nil, ) } func (am *LotexAM) RouteGetAMAlerts(ctx *models.ReqContext) response.Response { return am.withReq( - ctx, &http.Request{ - URL: withPath( - *ctx.Req.URL, - amAlertsPath, - ), - }, + ctx, + http.MethodGet, + withPath( + *ctx.Req.URL, + amAlertsPath, + ), + nil, jsonExtractor(&apimodels.GettableAlerts{}), + nil, ) } func (am *LotexAM) RouteGetSilence(ctx *models.ReqContext) response.Response { return am.withReq( - ctx, &http.Request{ - URL: withPath( - *ctx.Req.URL, - fmt.Sprintf(amSilencePath, ctx.Params(":SilenceId")), - ), - }, + ctx, + http.MethodGet, + withPath( + *ctx.Req.URL, + fmt.Sprintf(amSilencePath, ctx.Params(":SilenceId")), + ), + nil, jsonExtractor(&apimodels.GettableSilence{}), + nil, ) } func (am *LotexAM) RouteGetSilences(ctx *models.ReqContext) response.Response { return am.withReq( - ctx, &http.Request{ - URL: withPath( - *ctx.Req.URL, - amSilencesPath, - ), - }, + ctx, + http.MethodGet, + withPath( + *ctx.Req.URL, + amSilencesPath, + ), + nil, jsonExtractor(&apimodels.GettableSilences{}), + nil, ) } @@ -141,16 +151,15 @@ func (am *LotexAM) RoutePostAlertingConfig(ctx *models.ReqContext, config apimod if err != nil { return response.Error(500, "Failed marshal alert manager configuration ", err) } - body, ln := payload(yml) - u := withPath(*ctx.Req.URL, amConfigPath) - req := &http.Request{ - Method: "POST", - URL: u, - Body: body, - ContentLength: ln, - } - return am.withReq(ctx, req, messageExtractor) + return am.withReq( + ctx, + http.MethodPost, + withPath(*ctx.Req.URL, amConfigPath), + bytes.NewBuffer(yml), + messageExtractor, + nil, + ) } func (am *LotexAM) RoutePostAMAlerts(ctx *models.ReqContext, alerts apimodels.PostableAlerts) response.Response { @@ -158,14 +167,13 @@ func (am *LotexAM) RoutePostAMAlerts(ctx *models.ReqContext, alerts apimodels.Po if err != nil { return response.Error(500, "Failed marshal postable alerts", err) } - body, ln := payload(yml) - u := withPath(*ctx.Req.URL, amAlertsPath) - req := &http.Request{ - Method: "POST", - URL: u, - Body: body, - ContentLength: ln, - } - return am.withReq(ctx, req, messageExtractor) + return am.withReq( + ctx, + http.MethodPost, + withPath(*ctx.Req.URL, amAlertsPath), + bytes.NewBuffer(yml), + messageExtractor, + nil, + ) } diff --git a/pkg/services/ngalert/api/lotex_prom.go b/pkg/services/ngalert/api/lotex_prom.go index 01bc4e7fd0b..2886bdddc81 100644 --- a/pkg/services/ngalert/api/lotex_prom.go +++ b/pkg/services/ngalert/api/lotex_prom.go @@ -44,13 +44,15 @@ func (p *LotexProm) RouteGetAlertStatuses(ctx *models.ReqContext) response.Respo } return p.withReq( - ctx, &http.Request{ - URL: withPath( - *ctx.Req.URL, - endpoints.alerts, - ), - }, + ctx, + http.MethodGet, + withPath( + *ctx.Req.URL, + endpoints.alerts, + ), + nil, jsonExtractor(&apimodels.AlertResponse{}), + nil, ) } @@ -61,13 +63,15 @@ func (p *LotexProm) RouteGetRuleStatuses(ctx *models.ReqContext) response.Respon } return p.withReq( - ctx, &http.Request{ - URL: withPath( - *ctx.Req.URL, - endpoints.rules, - ), - }, + ctx, + http.MethodGet, + withPath( + *ctx.Req.URL, + endpoints.rules, + ), + nil, jsonExtractor(&apimodels.RuleResponse{}), + nil, ) } diff --git a/pkg/services/ngalert/api/test-data/am-alertmanager-recipient.http b/pkg/services/ngalert/api/test-data/am-alertmanager-recipient.http index 24472f79d10..6ca904b2837 100644 --- a/pkg/services/ngalert/api/test-data/am-alertmanager-recipient.http +++ b/pkg/services/ngalert/api/test-data/am-alertmanager-recipient.http @@ -2,7 +2,7 @@ ### # create AM configuration -POST http://admin:admin@localhost:3000/alertmanager/{{alertManagerDatasourceID}}/config/api/v1/alerts +POST http://admin:admin@localhost:3000/api/alertmanager/{{alertManagerDatasourceID}}/config/api/v1/alerts content-type: application/json { @@ -63,16 +63,16 @@ content-type: application/json ### # get latest AM configuration -GET http://admin:admin@localhost:3000/alertmanager/{{alertManagerDatasourceID}}/config/api/v1/alerts +GET http://admin:admin@localhost:3000/api/alertmanager/{{alertManagerDatasourceID}}/config/api/v1/alerts content-type: application/json ### # delete AM configuration -DELETE http://admin:admin@localhost:3000/alertmanager/{{alertManagerDatasourceID}}/config/api/v1/alerts +DELETE http://admin:admin@localhost:3000/api/alertmanager/{{alertManagerDatasourceID}}/config/api/v1/alerts ### # create AM alerts -POST http://admin:admin@localhost:3000/alertmanager/{{alertManagerDatasourceID}}/api/v2/alerts +POST http://admin:admin@localhost:3000/api/alertmanager/{{alertManagerDatasourceID}}/api/v2/alerts content-type: application/json [ @@ -95,15 +95,15 @@ content-type: application/json ### # get AM alerts -GET http://admin:admin@localhost:3000/alertmanager/{{alertManagerDatasourceID}}/api/v2/alerts +GET http://admin:admin@localhost:3000/api/alertmanager/{{alertManagerDatasourceID}}/api/v2/alerts ### # get silences - no silences -GET http://admin:admin@localhost:3000/alertmanager/{{alertManagerDatasourceID}}/api/v2/silences?Filter=foo="bar"&Filter=bar="foo" +GET http://admin:admin@localhost:3000/api/alertmanager/{{alertManagerDatasourceID}}/api/v2/silences?Filter=foo="bar"&Filter=bar="foo" ### # create silence -POST http://admin:admin@localhost:3000/alertmanager/{{alertManagerDatasourceID}}/api/v2/silences +POST http://admin:admin@localhost:3000/api/alertmanager/{{alertManagerDatasourceID}}/api/v2/silences content-type: application/json { @@ -117,12 +117,12 @@ content-type: application/json "createdBy": "spapagian", "comment": "a comment", "startsAt": "2021-04-05T14:45:09.885Z", - "endsAt": "2021-04-05T16:45:09.885Z" + "endsAt": "2021-04-09T16:45:09.885Z" } ### # update silence - does not exist -POST http://admin:admin@localhost:3000/alertmanager/{{alertManagerDatasourceID}}/api/v2/silences +POST http://admin:admin@localhost:3000/api/alertmanager/{{alertManagerDatasourceID}}/api/v2/silences content-type: application/json { @@ -143,25 +143,25 @@ content-type: application/json ### # get silences # @name getSilences -GET http://admin:admin@localhost:3000/alertmanager/{{alertManagerDatasourceID}}/api/v2/silences +GET http://admin:admin@localhost:3000/api/alertmanager/{{alertManagerDatasourceID}}/api/v2/silences ### -@silenceID = {{getSilences.response.body.$.[3].id}} +@silenceID = {{getSilences.response.body.$.[0].id}} ### # get silence -GET http://admin:admin@localhost:3000/alertmanager/{{alertManagerDatasourceID}}/api/v2/silence/{{silenceID}} +GET http://admin:admin@localhost:3000/api/alertmanager/{{alertManagerDatasourceID}}/api/v2/silence/{{silenceID}} ### # get silence - unknown -GET http://admin:admin@localhost:3000/alertmanager/{{alertManagerDatasourceID}}/api/v2/silence/unknown +GET http://admin:admin@localhost:3000/api/alertmanager/{{alertManagerDatasourceID}}/api/v2/silence/unknown ### # delete silence -DELETE http://admin:admin@localhost:3000/alertmanager/{{alertManagerDatasourceID}}/api/v2/silence/{{silenceID}} +DELETE http://admin:admin@localhost:3000/api/alertmanager/{{alertManagerDatasourceID}}/api/v2/silence/{{silenceID}} ### # delete silence - unknown -DELETE http://admin:admin@localhost:3000/alertmanager/{{alertManagerDatasourceID}}/api/v2/silence/unknown \ No newline at end of file +DELETE http://admin:admin@localhost:3000/api/alertmanager/{{alertManagerDatasourceID}}/api/v2/silence/unknown \ No newline at end of file diff --git a/pkg/services/ngalert/api/util.go b/pkg/services/ngalert/api/util.go index 95db69f63f1..151e9dc7278 100644 --- a/pkg/services/ngalert/api/util.go +++ b/pkg/services/ngalert/api/util.go @@ -4,7 +4,9 @@ import ( "bytes" "encoding/json" "fmt" + "io" "net/http" + "net/url" "regexp" "strconv" "strings" @@ -86,9 +88,19 @@ type AlertingProxy struct { // withReq proxies a different request func (p *AlertingProxy) withReq( ctx *models.ReqContext, - req *http.Request, + method string, + u *url.URL, + body io.Reader, extractor func([]byte) (interface{}, error), + headers map[string]string, ) response.Response { + req, err := http.NewRequest(method, u.String(), body) + if err != nil { + return response.Error(400, err.Error(), nil) + } + for h, v := range headers { + req.Header.Add(h, v) + } newCtx, resp := replacedResponseWriter(ctx) newCtx.Req.Request = req p.DataProxy.ProxyDatasourceRequestWithID(newCtx, ctx.ParamsInt64("Recipient"))