From 64b4a9cf9dd5fff0b66a14ed685a2c686d4a4766 Mon Sep 17 00:00:00 2001 From: beorn7 Date: Thu, 10 Sep 2020 13:14:07 +0200 Subject: [PATCH] API client: Enable fallback on status code 501, too When discussing #801, I remembered #794. While dealing with the latter, I read the HTTP RFC, stumbling upon the following: When a request method is received that is unrecognized or not implemented by an origin server, the origin server SHOULD respond with the 501 (Not Implemented) status code. When a request method is received that is known by an origin server but not allowed for the target resource, the origin server SHOULD respond with the 405 (Method Not Allowed) status code. Concluding from that, it is possible that a server desiring a fallback to GET will send a status code of 501. It is even preferred if that server does not offer any resource to be used with the POST method. Therefore, I think we should fallback to GET on a 501, too. Signed-off-by: beorn7 --- api/prometheus/v1/api.go | 5 +++-- api/prometheus/v1/api_test.go | 29 ++++++++++++++++++++++++++--- 2 files changed, 29 insertions(+), 5 deletions(-) diff --git a/api/prometheus/v1/api.go b/api/prometheus/v1/api.go index 549cc8d37..138a33b7d 100644 --- a/api/prometheus/v1/api.go +++ b/api/prometheus/v1/api.go @@ -1004,7 +1004,8 @@ func (h *apiClientImpl) Do(ctx context.Context, req *http.Request) (*http.Respon } -// DoGetFallback will attempt to do the request as-is, and on a 405 it will fallback to a GET request. +// DoGetFallback will attempt to do the request as-is, and on a 405 or 501 it +// will fallback to a GET request. func (h *apiClientImpl) DoGetFallback(ctx context.Context, u *url.URL, args url.Values) (*http.Response, []byte, Warnings, error) { req, err := http.NewRequest(http.MethodPost, u.String(), strings.NewReader(args.Encode())) if err != nil { @@ -1013,7 +1014,7 @@ func (h *apiClientImpl) DoGetFallback(ctx context.Context, u *url.URL, args url. req.Header.Set("Content-Type", "application/x-www-form-urlencoded") resp, body, warnings, err := h.Do(ctx, req) - if resp != nil && resp.StatusCode == http.StatusMethodNotAllowed { + if resp != nil && (resp.StatusCode == http.StatusMethodNotAllowed || resp.StatusCode == http.StatusNotImplemented) { u.RawQuery = args.Encode() req, err = http.NewRequest(http.MethodGet, u.String(), nil) if err != nil { diff --git a/api/prometheus/v1/api_test.go b/api/prometheus/v1/api_test.go index 210757504..a85a2ab38 100644 --- a/api/prometheus/v1/api_test.go +++ b/api/prometheus/v1/api_test.go @@ -1447,12 +1447,19 @@ func TestDoGetFallback(t *testing.T) { body, _ := json.Marshal(apiResp) if req.Method == http.MethodPost { - if req.URL.Path == "/blockPost" { + if req.URL.Path == "/blockPost405" { http.Error(w, string(body), http.StatusMethodNotAllowed) return } } + if req.Method == http.MethodPost { + if req.URL.Path == "/blockPost501" { + http.Error(w, string(body), http.StatusNotImplemented) + return + } + } + w.Write(body) })) // Close the server when test finishes. @@ -1483,8 +1490,24 @@ func TestDoGetFallback(t *testing.T) { t.Fatalf("Mismatch in values") } - // Do a fallbcak to a get. - u.Path = "/blockPost" + // Do a fallback to a get on 405. + u.Path = "/blockPost405" + _, b, _, err = api.DoGetFallback(context.TODO(), u, v) + if err != nil { + t.Fatalf("Error doing local request: %v", err) + } + if err := json.Unmarshal(b, resp); err != nil { + t.Fatal(err) + } + if resp.Method != http.MethodGet { + t.Fatalf("Mismatch method") + } + if resp.Values != v.Encode() { + t.Fatalf("Mismatch in values") + } + + // Do a fallback to a get on 501. + u.Path = "/blockPost501" _, b, _, err = api.DoGetFallback(context.TODO(), u, v) if err != nil { t.Fatalf("Error doing local request: %v", err)