diff --git a/command/agent/health_endpoint.go b/command/agent/health_endpoint.go index 5b0bac4c49..29e6bff113 100644 --- a/command/agent/health_endpoint.go +++ b/command/agent/health_endpoint.go @@ -3,6 +3,7 @@ package agent import ( "fmt" "net/http" + "strconv" "strings" "github.com/hashicorp/consul/api" @@ -148,7 +149,22 @@ func (s *HTTPServer) HealthServiceNodes(resp http.ResponseWriter, req *http.Requ // Filter to only passing if specified if _, ok := params[api.HealthPassing]; ok { - out.Nodes = filterNonPassing(out.Nodes) + val := params.Get(api.HealthPassing) + // Backwards-compat to allow users to specify ?passing without a value. This + // should be removed in Consul 0.10. + if val == "" { + out.Nodes = filterNonPassing(out.Nodes) + } else { + filter, err := strconv.ParseBool(val) + if err != nil { + resp.WriteHeader(400) + fmt.Fprint(resp, "Invalid value for ?passing") + return nil, nil + } + if filter { + out.Nodes = filterNonPassing(out.Nodes) + } + } } // Translate addresses after filtering so we don't waste effort. diff --git a/command/agent/health_endpoint_test.go b/command/agent/health_endpoint_test.go index d017f1f772..46ab1d1730 100644 --- a/command/agent/health_endpoint_test.go +++ b/command/agent/health_endpoint_test.go @@ -1,7 +1,9 @@ package agent import ( + "bytes" "fmt" + "io/ioutil" "net/http" "net/http/httptest" "reflect" @@ -15,7 +17,7 @@ import ( func TestHealthChecksInState(t *testing.T) { t.Parallel() - t.Run("", func(t *testing.T) { + t.Run("warning", func(t *testing.T) { a := NewTestAgent(t.Name(), nil) defer a.Shutdown() @@ -38,7 +40,7 @@ func TestHealthChecksInState(t *testing.T) { }) }) - t.Run("", func(t *testing.T) { + t.Run("passing", func(t *testing.T) { a := NewTestAgent(t.Name(), nil) defer a.Shutdown() @@ -612,20 +614,74 @@ func TestHealthServiceNodes_PassingFilter(t *testing.T) { t.Fatalf("err: %v", err) } - req, _ := http.NewRequest("GET", "/v1/health/service/consul?passing", nil) - resp := httptest.NewRecorder() - obj, err := a.srv.HealthServiceNodes(resp, req) - if err != nil { - t.Fatalf("err: %v", err) - } + t.Run("bc_no_query_value", func(t *testing.T) { + req, _ := http.NewRequest("GET", "/v1/health/service/consul?passing", nil) + resp := httptest.NewRecorder() + obj, err := a.srv.HealthServiceNodes(resp, req) + if err != nil { + t.Fatalf("err: %v", err) + } - assertIndex(t, resp) + assertIndex(t, resp) - // Should be 0 health check for consul - nodes := obj.(structs.CheckServiceNodes) - if len(nodes) != 0 { - t.Fatalf("bad: %v", obj) - } + // Should be 0 health check for consul + nodes := obj.(structs.CheckServiceNodes) + if len(nodes) != 0 { + t.Fatalf("bad: %v", obj) + } + }) + + t.Run("passing_true", func(t *testing.T) { + req, _ := http.NewRequest("GET", "/v1/health/service/consul?passing=true", nil) + resp := httptest.NewRecorder() + obj, err := a.srv.HealthServiceNodes(resp, req) + if err != nil { + t.Fatalf("err: %v", err) + } + + assertIndex(t, resp) + + // Should be 0 health check for consul + nodes := obj.(structs.CheckServiceNodes) + if len(nodes) != 0 { + t.Fatalf("bad: %v", obj) + } + }) + + t.Run("passing_false", func(t *testing.T) { + req, _ := http.NewRequest("GET", "/v1/health/service/consul?passing=false", nil) + resp := httptest.NewRecorder() + obj, err := a.srv.HealthServiceNodes(resp, req) + if err != nil { + t.Fatalf("err: %v", err) + } + + assertIndex(t, resp) + + // Should be 0 health check for consul + nodes := obj.(structs.CheckServiceNodes) + if len(nodes) != 1 { + t.Fatalf("bad: %v", obj) + } + }) + + t.Run("passing_bad", func(t *testing.T) { + req, _ := http.NewRequest("GET", "/v1/health/service/consul?passing=nope-nope-nope", nil) + resp := httptest.NewRecorder() + a.srv.HealthServiceNodes(resp, req) + + if code := resp.Code; code != 400 { + t.Errorf("bad response code %d, expected %d", code, 400) + } + + body, err := ioutil.ReadAll(resp.Body) + if err != nil { + t.Fatal(err) + } + if !bytes.Contains(body, []byte("Invalid value for ?passing")) { + t.Errorf("bad %s", body) + } + }) } func TestHealthServiceNodes_WanTranslation(t *testing.T) {