Skip to content

Commit

Permalink
API client: Enable fallback on status code 501, too
Browse files Browse the repository at this point in the history
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 <beorn@grafana.com>
  • Loading branch information
beorn7 committed Sep 10, 2020
1 parent 65c5578 commit 64b4a9c
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 5 deletions.
5 changes: 3 additions & 2 deletions api/prometheus/v1/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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 {
Expand Down
29 changes: 26 additions & 3 deletions api/prometheus/v1/api_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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)
Expand Down

0 comments on commit 64b4a9c

Please sign in to comment.