[dtest] API tests for /query and /query_range endpoints (#2873)

src/cmd/tools/dtest/docker/harness/carbon_test.go

@@ -31,12 +31,16 @@ import (
 	"github.com/stretchr/testify/assert"
 )
 
-func findVerifier(expected string) resources.GoalStateVerifier {
-	return func(s string, err error) error {
+func findVerifier(expected string) resources.ResponseVerifier {
+	return func(status int, s string, err error) error {
 		if err != nil {
 			return err
 		}
 
+		if status/100 != 2 {
+			return fmt.Errorf("expected 200 status code, got %v", status)
+		}
+
 		if s == expected {
 			return nil
 		}
@@ -45,17 +49,21 @@ func findVerifier(expected string) resources.GoalStateVerifier {
 	}
 }
 
-func renderVerifier(metric string, v float64) resources.GoalStateVerifier {
+func renderVerifier(v float64) resources.ResponseVerifier {
 	type graphiteRender struct {
 		Target     string      `json:"target"`
 		Datapoints [][]float64 `json:"datapoints"`
 	}
 
-	return func(s string, err error) error {
+	return func(status int, s string, err error) error {
 		if err != nil {
 			return err
 		}
 
+		if status/100 != 2 {
+			return fmt.Errorf("expected 200 status code, got %v", status)
+		}
+
 		var render []graphiteRender
 		if err := json.Unmarshal([]byte(s), &render); err != nil {
 			return err
@@ -103,7 +111,7 @@ func TestCarbon(t *testing.T) {
 
 	read := func(metric string, expected float64) {
 		assert.NoError(t, coord.RunQuery(
-			renderVerifier(metric, expected),
+			renderVerifier(expected),
 			graphiteQuery(metric, timestamp)))
 	}
 

src/cmd/tools/dtest/docker/harness/query_api_test.go

@@ -0,0 +1,131 @@
+// Copyright (c) 2020 Uber Technologies, Inc.
+//
+// Permission is hereby granted, free of charge, to any person obtaining a copy
+// of this software and associated documentation files (the "Software"), to deal
+// in the Software without restriction, including without limitation the rights
+// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+// copies of the Software, and to permit persons to whom the Software is
+// furnished to do so, subject to the following conditions:
+//
+// The above copyright notice and this permission notice shall be included in
+// all copies or substantial portions of the Software.
+//
+// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+// THE SOFTWARE.
+
+package harness
+
+import (
+	"fmt"
+	"strings"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+
+	"github.com/m3db/m3/src/cmd/tools/dtest/docker/harness/resources"
+)
+
+type urlTest struct {
+	name string
+	url  string
+}
+
+func TestInvalidInstantQueryReturns400(t *testing.T) {
+	coord := singleDBNodeDockerResources.Coordinator()
+
+	instantQueryBadRequestTest := []urlTest{
+		// FAILING issue #2: invalid or missing query string should result in 400
+		// {"missing query", queryURL("query", "")},
+		// {"invalid query", queryURL("query", "@!")},
+		{"invalid time", queryURL("time", "INVALID")},
+		{"invalid timeout", queryURL("timeout", "INVALID")},
+	}
+
+	for _, tt := range instantQueryBadRequestTest {
+		t.Run(tt.name, func(t *testing.T) {
+			assert.NoError(t, coord.RunQuery(verifyStatus(400), tt.url), "for query '%v'", tt.url)
+		})
+	}
+}
+
+func TestInvalidRangeQueryReturns400(t *testing.T) {
+	coord := singleDBNodeDockerResources.Coordinator()
+
+	queryBadRequestTest := []urlTest{
+		{"missing query", queryRangeURL("query", "")},
+		// FAILING issue #2: invalid query string should result in 400
+		// {"invalid query", queryRangeURL("query", "@!")},
+		{"missing start", queryRangeURL("start", "")},
+		{"invalid start", queryRangeURL("start", "INVALID")},
+		{"missing end", queryRangeURL("end", "")},
+		{"invalid end", queryRangeURL("end", "INVALID")},
+		{"missing step", queryRangeURL("step", "")},
+		{"invalid step", queryRangeURL("step", "INVALID")},
+		{"invalid timeout", queryRangeURL("timeout", "INVALID")},
+	}
+
+	for _, tt := range queryBadRequestTest {
+		t.Run(tt.name, func(t *testing.T) {
+			assert.NoError(t, coord.RunQuery(verifyStatus(400), tt.url), "for query '%v'", tt.url)
+		})
+	}
+}
+
+func queryURL(key, value string) string {
+	params := map[string]string{
+		"query": "foo",
+	}
+
+	if value == "" {
+		delete(params, key)
+	} else {
+		params[key] = value
+	}
+
+	return "api/v1/query?" + queryString(params)
+}
+
+func queryRangeURL(key, value string) string {
+	params := map[string]string{
+		"query": "foo",
+		"start": "now",
+		"end":   "now",
+		"step":  "1000",
+	}
+
+	if value == "" {
+		delete(params, key)
+	} else {
+		params[key] = value
+	}
+
+	return "api/v1/query_range?" + queryString(params)
+}
+
+func queryString(params map[string]string) string {
+	p := make([]string, 0)
+	for k, v := range params {
+		p = append(p, fmt.Sprintf("%v=%v", k, v))
+	}
+
+	return strings.Join(p, "&")
+}
+
+func verifyStatus(expectedStatus int) resources.ResponseVerifier {
+	return func(status int, resp string, err error) error {
+		if err != nil {
+			return err
+		}
+
+		if status != expectedStatus {
+			return fmt.Errorf("expeceted %v status code, got %v", expectedStatus, status)
+		}
+
+		return nil
+	}
+}

src/cmd/tools/dtest/docker/harness/resources/coordinator.go

@@ -54,6 +54,9 @@ var (
 	}
 )
 
+// ResponseVerifier is a function that checks if the query response is valid.
+type ResponseVerifier func(int, string, error) error
+
 // Coordinator is a wrapper for a coordinator. It provides a wrapper on HTTP
 // endpoints that expose cluster management APIs as well as read and write
 // endpoints for series data.
@@ -64,7 +67,7 @@ type Coordinator interface {
 	// WriteCarbon writes a carbon metric datapoint at a given time.
 	WriteCarbon(port int, metric string, v float64, t time.Time) error
 	// RunQuery runs the given query with a given verification function.
-	RunQuery(verifier GoalStateVerifier, query string) error
+	RunQuery(verifier ResponseVerifier, query string) error
 }
 
 // Admin is a wrapper for admin functions.
@@ -341,7 +344,7 @@ func makePostRequest(logger *zap.Logger, url string, body proto.Message) (*http.
 }
 
 func (c *coordinator) query(
-	verifier GoalStateVerifier, query string,
+	verifier ResponseVerifier, query string,
 ) error {
 	if c.resource.closed {
 		return errClosed
@@ -357,20 +360,14 @@ func (c *coordinator) query(
 		return err
 	}
 
-	defer resp.Body.Close()
-	if resp.StatusCode/100 != 2 {
-		logger.Error("status code not 2xx",
-			zap.Int("status code", resp.StatusCode),
-			zap.String("status", resp.Status))
-		return fmt.Errorf("status code %d", resp.StatusCode)
-	}
 	defer resp.Body.Close()
 	b, err := ioutil.ReadAll(resp.Body)
-	return verifier(string(b), err)
+
+	return verifier(resp.StatusCode, string(b), err)
 }
 
 func (c *coordinator) RunQuery(
-	verifier GoalStateVerifier, query string,
+	verifier ResponseVerifier, query string,
 ) error {
 	if c.resource.closed {
 		return errClosed