From c0c1f5e4032f289aed6c8354ae82f07ebbb50eb2 Mon Sep 17 00:00:00 2001 From: dttung2905 Date: Wed, 21 Dec 2022 14:14:18 +0000 Subject: [PATCH 1/9] backport of commit af36e5211bc10952afae105d3d13c908751bd19c --- command/agent/http.go | 15 +++++++++++---- command/agent/http_test.go | 28 ++++++++++++++++++++++------ 2 files changed, 33 insertions(+), 10 deletions(-) diff --git a/command/agent/http.go b/command/agent/http.go index 2f9ac7474d4..60f0e9de704 100644 --- a/command/agent/http.go +++ b/command/agent/http.go @@ -760,10 +760,17 @@ 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 { + if staleVal[0] == "true" || staleVal[0] == "" { + b.AllowStale = true + } else if staleVal[0] == "false" { + // fall through + } else { + resp.WriteHeader(400) + resp.Write([]byte(fmt.Sprintf("Expect `true` or `false` for `stale` query string parameter, got %s", staleVal[0]))) + } } } @@ -861,7 +868,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 a23024f572a..411b826691b 100644 --- a/command/agent/http_test.go +++ b/command/agent/http_test.go @@ -476,16 +476,31 @@ func TestParseWait_InvalidIndex(t *testing.T) { func TestParseConsistency(t *testing.T) { ci.Parallel(t) var b structs.QueryOptions + var resp *httptest.ResponseRecorder + + testCases := [3]string{"/v1/catalog/nodes?stale", "/v1/catalog/nodes?stale=true", "/v1/catalog/nodes?stale=false"} + for _, url := range testCases { + req, err := http.NewRequest("GET", + url, nil) + if err != nil { + t.Fatalf("err: %v", err) + } + resp = httptest.NewRecorder() + parseConsistency(resp, req, &b) + if !b.AllowStale { + t.Fatalf("Bad: %v", b) + } + } req, err := http.NewRequest("GET", - "/v1/catalog/nodes?stale", nil) + "/v1/catalog/nodes?stale=random", nil) if err != nil { t.Fatalf("err: %v", err) } - - parseConsistency(req, &b) - if !b.AllowStale { - t.Fatalf("Bad: %v", b) + resp = httptest.NewRecorder() + parseConsistency(resp, req, &b) + if resp.Code != 400 { + t.Fatalf("Bad: %v. Expect response code 400, got %v", b, resp.Code) } b = structs.QueryOptions{} @@ -495,7 +510,8 @@ func TestParseConsistency(t *testing.T) { t.Fatalf("err: %v", err) } - parseConsistency(req, &b) + resp = httptest.NewRecorder() + parseConsistency(resp, req, &b) if b.AllowStale { t.Fatalf("Bad: %v", b) } From ad823ea19534b1682b956ff33b499ea0eea49bbb Mon Sep 17 00:00:00 2001 From: dttung2905 Date: Wed, 21 Dec 2022 14:22:18 +0000 Subject: [PATCH 2/9] backport of commit 62018ee913ceeec20898f3eeb311f934689b0615 --- command/agent/http_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/command/agent/http_test.go b/command/agent/http_test.go index 411b826691b..c7c22cc4e1f 100644 --- a/command/agent/http_test.go +++ b/command/agent/http_test.go @@ -500,7 +500,7 @@ func TestParseConsistency(t *testing.T) { resp = httptest.NewRecorder() parseConsistency(resp, req, &b) if resp.Code != 400 { - t.Fatalf("Bad: %v. Expect response code 400, got %v", b, resp.Code) + t.Fatalf("bad code: %v", resp.Code) } b = structs.QueryOptions{} From 3def99ba4c7c3e0ea5674add8b8211b34f99641d Mon Sep 17 00:00:00 2001 From: dttung2905 Date: Thu, 22 Dec 2022 15:46:57 +0000 Subject: [PATCH 3/9] backport of commit 4ed12c970750da796c5fbf00189fbd2cab9c5748 --- .changelog/15605.txt | 3 +++ command/agent/http.go | 11 ++++++----- command/agent/http_test.go | 13 +++++-------- 3 files changed, 14 insertions(+), 13 deletions(-) create mode 100644 .changelog/15605.txt diff --git a/.changelog/15605.txt b/.changelog/15605.txt new file mode 100644 index 00000000000..b8906939923 --- /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 60f0e9de704..36a58963375 100644 --- a/command/agent/http.go +++ b/command/agent/http.go @@ -763,14 +763,15 @@ func parseWait(resp http.ResponseWriter, req *http.Request, b *structs.QueryOpti func parseConsistency(resp http.ResponseWriter, req *http.Request, b *structs.QueryOptions) { query := req.URL.Query() if staleVal, ok := query["stale"]; ok { - if staleVal[0] == "true" || staleVal[0] == "" { - b.AllowStale = true - } else if staleVal[0] == "false" { - // fall through - } else { + 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]))) } + + if staleQuery || staleVal[0] == "" { + b.AllowStale = true + } } } diff --git a/command/agent/http_test.go b/command/agent/http_test.go index c7c22cc4e1f..fea26dcb1f2 100644 --- a/command/agent/http_test.go +++ b/command/agent/http_test.go @@ -7,6 +7,7 @@ import ( "encoding/json" "errors" "fmt" + "github.com/shoenig/test/must" "io" "io/ioutil" "net" @@ -482,9 +483,7 @@ func TestParseConsistency(t *testing.T) { for _, url := range testCases { req, err := http.NewRequest("GET", url, nil) - if err != nil { - t.Fatalf("err: %v", err) - } + must.NoError(t, err) resp = httptest.NewRecorder() parseConsistency(resp, req, &b) if !b.AllowStale { @@ -492,8 +491,7 @@ func TestParseConsistency(t *testing.T) { } } - req, err := http.NewRequest("GET", - "/v1/catalog/nodes?stale=random", nil) + req, err := http.NewRequest("GET", "/v1/catalog/nodes?stale=random", nil) if err != nil { t.Fatalf("err: %v", err) } @@ -502,6 +500,7 @@ func TestParseConsistency(t *testing.T) { if resp.Code != 400 { t.Fatalf("bad code: %v", resp.Code) } + must.EqOp(t, resp.Code, 400) b = structs.QueryOptions{} req, err = http.NewRequest("GET", @@ -512,9 +511,7 @@ func TestParseConsistency(t *testing.T) { resp = httptest.NewRecorder() parseConsistency(resp, req, &b) - if b.AllowStale { - t.Fatalf("Bad: %v", b) - } + must.True(t, !b.AllowStale) } func TestParseRegion(t *testing.T) { From ee3ed7ec948700f16052888636be5d6ad489b40d Mon Sep 17 00:00:00 2001 From: dttung2905 Date: Thu, 22 Dec 2022 16:13:40 +0000 Subject: [PATCH 4/9] backport of commit b4141e78e5caf23417b092625ae7258a9c453278 --- command/agent/http.go | 2 ++ command/agent/http_test.go | 28 ++++++++++++---------------- 2 files changed, 14 insertions(+), 16 deletions(-) diff --git a/command/agent/http.go b/command/agent/http.go index 36a58963375..567f3c608fa 100644 --- a/command/agent/http.go +++ b/command/agent/http.go @@ -771,6 +771,8 @@ func parseConsistency(resp http.ResponseWriter, req *http.Request, b *structs.Qu if staleQuery || staleVal[0] == "" { b.AllowStale = true + } else { + b.AllowStale = false } } } diff --git a/command/agent/http_test.go b/command/agent/http_test.go index fea26dcb1f2..44c94e46337 100644 --- a/command/agent/http_test.go +++ b/command/agent/http_test.go @@ -479,35 +479,31 @@ func TestParseConsistency(t *testing.T) { var b structs.QueryOptions var resp *httptest.ResponseRecorder - testCases := [3]string{"/v1/catalog/nodes?stale", "/v1/catalog/nodes?stale=true", "/v1/catalog/nodes?stale=false"} + testCases := [2]string{"/v1/catalog/nodes?stale", "/v1/catalog/nodes?stale=true"} for _, url := range testCases { - req, err := http.NewRequest("GET", - url, nil) + req, err := http.NewRequest("GET", url, nil) must.NoError(t, err) resp = httptest.NewRecorder() parseConsistency(resp, req, &b) - if !b.AllowStale { - t.Fatalf("Bad: %v", b) - } + must.True(t, b.AllowStale) } - req, err := http.NewRequest("GET", "/v1/catalog/nodes?stale=random", nil) - if err != nil { - t.Fatalf("err: %v", err) - } + req, err := http.NewRequest("GET", "/v1/catalog/nodes?stale=false", nil) + must.Nil(t, err, must.Sprintf("err: %v", err)) + resp = httptest.NewRecorder() + parseConsistency(resp, req, &b) + must.True(t, !b.AllowStale) + + req, err = http.NewRequest("GET", "/v1/catalog/nodes?stale=random", nil) + must.Nil(t, err, must.Sprintf("err: %v", err)) resp = httptest.NewRecorder() parseConsistency(resp, req, &b) - if resp.Code != 400 { - t.Fatalf("bad code: %v", resp.Code) - } must.EqOp(t, resp.Code, 400) b = structs.QueryOptions{} req, err = http.NewRequest("GET", "/v1/catalog/nodes?consistent", nil) - if err != nil { - t.Fatalf("err: %v", err) - } + must.Nil(t, err, must.Sprintf("err: %v", err)) resp = httptest.NewRecorder() parseConsistency(resp, req, &b) From c5d6e0d7096a919c03dd4c3704b03424a7010015 Mon Sep 17 00:00:00 2001 From: dttung2905 Date: Thu, 22 Dec 2022 16:17:03 +0000 Subject: [PATCH 5/9] backport of commit 4daf080609584e1534523dad8334b191f8ed2923 --- command/agent/http_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/command/agent/http_test.go b/command/agent/http_test.go index 44c94e46337..a99c68e3245 100644 --- a/command/agent/http_test.go +++ b/command/agent/http_test.go @@ -489,13 +489,13 @@ func TestParseConsistency(t *testing.T) { } req, err := http.NewRequest("GET", "/v1/catalog/nodes?stale=false", nil) - must.Nil(t, err, must.Sprintf("err: %v", err)) + must.NoError(t, err) resp = httptest.NewRecorder() parseConsistency(resp, req, &b) must.True(t, !b.AllowStale) req, err = http.NewRequest("GET", "/v1/catalog/nodes?stale=random", nil) - must.Nil(t, err, must.Sprintf("err: %v", err)) + must.NoError(t, err) resp = httptest.NewRecorder() parseConsistency(resp, req, &b) must.EqOp(t, resp.Code, 400) @@ -503,7 +503,7 @@ func TestParseConsistency(t *testing.T) { b = structs.QueryOptions{} req, err = http.NewRequest("GET", "/v1/catalog/nodes?consistent", nil) - must.Nil(t, err, must.Sprintf("err: %v", err)) + must.NoError(t, err) resp = httptest.NewRecorder() parseConsistency(resp, req, &b) From 61fd1e0858414e12477bbf51e2f63f86bb47d630 Mon Sep 17 00:00:00 2001 From: dttung2905 Date: Thu, 22 Dec 2022 16:20:22 +0000 Subject: [PATCH 6/9] backport of commit 9776b92920e194eee688f95b9eb519b32366ea50 --- command/agent/http_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/command/agent/http_test.go b/command/agent/http_test.go index a99c68e3245..4894e5646b0 100644 --- a/command/agent/http_test.go +++ b/command/agent/http_test.go @@ -7,7 +7,6 @@ import ( "encoding/json" "errors" "fmt" - "github.com/shoenig/test/must" "io" "io/ioutil" "net" @@ -30,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" ) From 4baa46e85b983801b2445226de41ee5d83e7fe2b Mon Sep 17 00:00:00 2001 From: dttung2905 Date: Thu, 22 Dec 2022 16:23:43 +0000 Subject: [PATCH 7/9] backport of commit f00ed4133a095168434b9200fc4d7e3ada5541c6 --- command/agent/http_test.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/command/agent/http_test.go b/command/agent/http_test.go index 4894e5646b0..026391d0448 100644 --- a/command/agent/http_test.go +++ b/command/agent/http_test.go @@ -501,8 +501,7 @@ func TestParseConsistency(t *testing.T) { must.EqOp(t, resp.Code, 400) b = structs.QueryOptions{} - req, err = http.NewRequest("GET", - "/v1/catalog/nodes?consistent", nil) + req, err = http.NewRequest("GET", "/v1/catalog/nodes?consistent", nil) must.NoError(t, err) resp = httptest.NewRecorder() From 93f682b1a1c0422149e49d4e4ac3c1151f52fca5 Mon Sep 17 00:00:00 2001 From: dttung2905 Date: Fri, 23 Dec 2022 02:47:09 +0000 Subject: [PATCH 8/9] backport of commit 5a50a71ab69af9f3f1f9d6f5bae91267b4cd11a3 --- command/agent/http.go | 6 +----- command/agent/http_test.go | 9 +++++---- 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/command/agent/http.go b/command/agent/http.go index 567f3c608fa..b8b523ea3d8 100644 --- a/command/agent/http.go +++ b/command/agent/http.go @@ -769,11 +769,7 @@ func parseConsistency(resp http.ResponseWriter, req *http.Request, b *structs.Qu resp.Write([]byte(fmt.Sprintf("Expect `true` or `false` for `stale` query string parameter, got %s", staleVal[0]))) } - if staleQuery || staleVal[0] == "" { - b.AllowStale = true - } else { - b.AllowStale = false - } + b.AllowStale = staleQuery || staleVal[0] == "" } } diff --git a/command/agent/http_test.go b/command/agent/http_test.go index 026391d0448..9071ce28c7b 100644 --- a/command/agent/http_test.go +++ b/command/agent/http_test.go @@ -480,8 +480,8 @@ func TestParseConsistency(t *testing.T) { var resp *httptest.ResponseRecorder testCases := [2]string{"/v1/catalog/nodes?stale", "/v1/catalog/nodes?stale=true"} - for _, url := range testCases { - req, err := http.NewRequest("GET", url, nil) + for _, urlPath := range testCases { + req, err := http.NewRequest("GET", urlPath, nil) must.NoError(t, err) resp = httptest.NewRecorder() parseConsistency(resp, req, &b) @@ -492,12 +492,13 @@ func TestParseConsistency(t *testing.T) { must.NoError(t, err) resp = httptest.NewRecorder() parseConsistency(resp, req, &b) - must.True(t, !b.AllowStale) + 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, resp.Code, 400) b = structs.QueryOptions{} @@ -506,7 +507,7 @@ func TestParseConsistency(t *testing.T) { resp = httptest.NewRecorder() parseConsistency(resp, req, &b) - must.True(t, !b.AllowStale) + must.False(t, b.AllowStale) } func TestParseRegion(t *testing.T) { From 111fda81076d4dc1a4972d57e04553847a43703c Mon Sep 17 00:00:00 2001 From: dttung2905 Date: Sat, 24 Dec 2022 00:49:42 +0000 Subject: [PATCH 9/9] backport of commit 70c9b3676bbda25388d64eccdf2fb433ccef404a --- command/agent/http_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/command/agent/http_test.go b/command/agent/http_test.go index 9071ce28c7b..41fce7a0f3d 100644 --- a/command/agent/http_test.go +++ b/command/agent/http_test.go @@ -499,7 +499,7 @@ func TestParseConsistency(t *testing.T) { resp = httptest.NewRecorder() parseConsistency(resp, req, &b) must.False(t, b.AllowStale) - must.EqOp(t, resp.Code, 400) + must.EqOp(t, 400, resp.Code) b = structs.QueryOptions{} req, err = http.NewRequest("GET", "/v1/catalog/nodes?consistent", nil)