diff --git a/.changelog/15605.txt b/.changelog/15605.txt new file mode 100644 index 000000000000..b89069399233 --- /dev/null +++ b/.changelog/15605.txt @@ -0,0 +1,3 @@ +```release-note:bug +api: Fix stale querystring parameter value as boolean +``` diff --git a/command/agent/http.go b/command/agent/http.go index 76a754e49997..4ef6e5e13d09 100644 --- a/command/agent/http.go +++ b/command/agent/http.go @@ -783,10 +783,16 @@ func parseWait(resp http.ResponseWriter, req *http.Request, b *structs.QueryOpti } // parseConsistency is used to parse the ?stale query params. -func parseConsistency(req *http.Request, b *structs.QueryOptions) { +func parseConsistency(resp http.ResponseWriter, req *http.Request, b *structs.QueryOptions) { query := req.URL.Query() - if _, ok := query["stale"]; ok { - b.AllowStale = true + if staleVal, ok := query["stale"]; ok { + staleQuery, err := strconv.ParseBool(staleVal[0]) + if err != nil { + resp.WriteHeader(400) + resp.Write([]byte(fmt.Sprintf("Expect `true` or `false` for `stale` query string parameter, got %s", staleVal[0]))) + } + + b.AllowStale = staleQuery || staleVal[0] == "" } } @@ -884,7 +890,7 @@ func (s *HTTPServer) parseToken(req *http.Request, token *string) { func (s *HTTPServer) parse(resp http.ResponseWriter, req *http.Request, r *string, b *structs.QueryOptions) bool { s.parseRegion(req, r) s.parseToken(req, &b.AuthToken) - parseConsistency(req, b) + parseConsistency(resp, req, b) parsePrefix(req, b) parseNamespace(req, &b.Namespace) parsePagination(req, b) diff --git a/command/agent/http_test.go b/command/agent/http_test.go index 5baa2a2480a5..64a5240dc0cb 100644 --- a/command/agent/http_test.go +++ b/command/agent/http_test.go @@ -29,6 +29,7 @@ import ( "github.com/hashicorp/nomad/nomad/structs" "github.com/hashicorp/nomad/nomad/structs/config" "github.com/hashicorp/nomad/testutil" + "github.com/shoenig/test/must" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -476,29 +477,37 @@ func TestParseWait_InvalidIndex(t *testing.T) { func TestParseConsistency(t *testing.T) { ci.Parallel(t) var b structs.QueryOptions + var resp *httptest.ResponseRecorder - req, err := http.NewRequest("GET", - "/v1/catalog/nodes?stale", nil) - if err != nil { - t.Fatalf("err: %v", err) + testCases := [2]string{"/v1/catalog/nodes?stale", "/v1/catalog/nodes?stale=true"} + for _, urlPath := range testCases { + req, err := http.NewRequest("GET", urlPath, nil) + must.NoError(t, err) + resp = httptest.NewRecorder() + parseConsistency(resp, req, &b) + must.True(t, b.AllowStale) } - parseConsistency(req, &b) - if !b.AllowStale { - t.Fatalf("Bad: %v", b) - } + req, err := http.NewRequest("GET", "/v1/catalog/nodes?stale=false", nil) + must.NoError(t, err) + resp = httptest.NewRecorder() + parseConsistency(resp, req, &b) + must.False(t, b.AllowStale) + + req, err = http.NewRequest("GET", "/v1/catalog/nodes?stale=random", nil) + must.NoError(t, err) + resp = httptest.NewRecorder() + parseConsistency(resp, req, &b) + must.False(t, b.AllowStale) + must.EqOp(t, 400, resp.Code) b = structs.QueryOptions{} - req, err = http.NewRequest("GET", - "/v1/catalog/nodes?consistent", nil) - if err != nil { - t.Fatalf("err: %v", err) - } + req, err = http.NewRequest("GET", "/v1/catalog/nodes?consistent", nil) + must.NoError(t, err) - parseConsistency(req, &b) - if b.AllowStale { - t.Fatalf("Bad: %v", b) - } + resp = httptest.NewRecorder() + parseConsistency(resp, req, &b) + must.False(t, b.AllowStale) } func TestParseRegion(t *testing.T) {