diff --git a/agent/health_endpoint.go b/agent/health_endpoint.go index 9573ae691..e690ff674 100644 --- a/agent/health_endpoint.go +++ b/agent/health_endpoint.go @@ -12,6 +12,12 @@ import ( "github.com/hashicorp/consul/api" ) +const ( + serviceHealth = "service" + connectHealth = "connect" + ingressHealth = "ingress" +) + func (s *HTTPServer) HealthChecksInState(resp http.ResponseWriter, req *http.Request) (interface{}, error) { // Set default DC args := structs.ChecksInStateRequest{} @@ -154,15 +160,26 @@ RETRY_ONCE: return out.HealthChecks, nil } +// HealthIngressServiceNodes should return "all the healthy ingress gateway instances +// that I can use to access this connect-enabled service without mTLS". +func (s *HTTPServer) HealthIngressServiceNodes(resp http.ResponseWriter, req *http.Request) (interface{}, error) { + return s.healthServiceNodes(resp, req, ingressHealth) +} + +// HealthConnectServiceNodes should return "all healthy connect-enabled +// endpoints (e.g. could be side car proxies or native instances) for this +// service so I can connect with mTLS". func (s *HTTPServer) HealthConnectServiceNodes(resp http.ResponseWriter, req *http.Request) (interface{}, error) { - return s.healthServiceNodes(resp, req, true) + return s.healthServiceNodes(resp, req, connectHealth) } +// HealthServiceNodes should return "all the healthy instances of this service +// registered so I can connect directly to them". func (s *HTTPServer) HealthServiceNodes(resp http.ResponseWriter, req *http.Request) (interface{}, error) { - return s.healthServiceNodes(resp, req, false) + return s.healthServiceNodes(resp, req, serviceHealth) } -func (s *HTTPServer) healthServiceNodes(resp http.ResponseWriter, req *http.Request, connect bool) (interface{}, error) { +func (s *HTTPServer) healthServiceNodes(resp http.ResponseWriter, req *http.Request, healthType string) (interface{}, error) { // Set default DC args := structs.ServiceSpecificRequest{} if err := s.parseEntMetaNoWildcard(req, &args.EnterpriseMeta); err != nil { @@ -182,23 +199,17 @@ func (s *HTTPServer) healthServiceNodes(resp http.ResponseWriter, req *http.Requ } // Determine the prefix - prefix := "/v1/health/service/" - if connect { + var prefix string + switch healthType { + case connectHealth: prefix = "/v1/health/connect/" - - // Check for ingress request only when requesting connect services - ingress, err := getBoolQueryParam(params, "ingress") - if err != nil { - resp.WriteHeader(http.StatusBadRequest) - fmt.Fprint(resp, "Invalid value for ?ingress") - return nil, nil - } - - if ingress { - args.Ingress = true - } else { - args.Connect = true - } + args.Connect = true + case ingressHealth: + prefix = "/v1/health/ingress/" + args.Ingress = true + default: + // serviceHealth is the default type + prefix = "/v1/health/service/" } // Pull out the service name diff --git a/agent/health_endpoint_test.go b/agent/health_endpoint_test.go index 03fa9c747..61de07439 100644 --- a/agent/health_endpoint_test.go +++ b/agent/health_endpoint_test.go @@ -1139,7 +1139,7 @@ func TestHealthConnectServiceNodes(t *testing.T) { assert.Len(nodes[0].Checks, 0) } -func TestHealthConnectServiceNodes_Ingress(t *testing.T) { +func TestHealthIngressServiceNodes(t *testing.T) { t.Parallel() a := NewTestAgent(t, "") @@ -1179,12 +1179,12 @@ func TestHealthConnectServiceNodes_Ingress(t *testing.T) { require.Nil(t, a.RPC("ConfigEntry.Apply", req, &outB)) require.True(t, outB) - t.Run("no_query_value", func(t *testing.T) { + t.Run("associated service", func(t *testing.T) { assert := assert.New(t) req, _ := http.NewRequest("GET", fmt.Sprintf( - "/v1/health/connect/%s?ingress", args.Service.Service), nil) + "/v1/health/ingress/%s", args.Service.Service), nil) resp := httptest.NewRecorder() - obj, err := a.srv.HealthConnectServiceNodes(resp, req) + obj, err := a.srv.HealthIngressServiceNodes(resp, req) assert.Nil(err) assertIndex(t, resp) @@ -1195,47 +1195,18 @@ func TestHealthConnectServiceNodes_Ingress(t *testing.T) { require.Equal(t, gatewayArgs.Service.Proxy, nodes[0].Service.Proxy) }) - t.Run("true_value", func(t *testing.T) { + t.Run("non-associated service", func(t *testing.T) { assert := assert.New(t) - req, _ := http.NewRequest("GET", fmt.Sprintf( - "/v1/health/connect/%s?ingress=true", args.Service.Service), nil) + req, _ := http.NewRequest("GET", + "/v1/health/connect/notexist", nil) resp := httptest.NewRecorder() - obj, err := a.srv.HealthConnectServiceNodes(resp, req) - assert.Nil(err) - assertIndex(t, resp) - - nodes := obj.(structs.CheckServiceNodes) - require.Len(t, nodes, 1) - require.Equal(t, structs.ServiceKindIngressGateway, nodes[0].Service.Kind) - require.Equal(t, gatewayArgs.Service.Address, nodes[0].Service.Address) - require.Equal(t, gatewayArgs.Service.Proxy, nodes[0].Service.Proxy) - }) - - t.Run("false_value", func(t *testing.T) { - assert := assert.New(t) - req, _ := http.NewRequest("GET", fmt.Sprintf( - "/v1/health/connect/%s?ingress=false", args.Service.Service), nil) - resp := httptest.NewRecorder() - obj, err := a.srv.HealthConnectServiceNodes(resp, req) + obj, err := a.srv.HealthIngressServiceNodes(resp, req) assert.Nil(err) assertIndex(t, resp) nodes := obj.(structs.CheckServiceNodes) require.Len(t, nodes, 0) }) - - t.Run("invalid_value", func(t *testing.T) { - assert := assert.New(t) - req, _ := http.NewRequest("GET", fmt.Sprintf( - "/v1/health/connect/%s?ingress=notabool", args.Service.Service), nil) - resp := httptest.NewRecorder() - _, err := a.srv.HealthConnectServiceNodes(resp, req) - assert.Equal(400, resp.Code) - - body, err := ioutil.ReadAll(resp.Body) - assert.Nil(err) - assert.True(bytes.Contains(body, []byte("Invalid value for ?ingress"))) - }) } func TestHealthConnectServiceNodes_Filter(t *testing.T) { diff --git a/agent/http_register.go b/agent/http_register.go index fc2f84ee4..ba4b8a5a9 100644 --- a/agent/http_register.go +++ b/agent/http_register.go @@ -91,6 +91,7 @@ func init() { registerEndpoint("/v1/health/state/", []string{"GET"}, (*HTTPServer).HealthChecksInState) registerEndpoint("/v1/health/service/", []string{"GET"}, (*HTTPServer).HealthServiceNodes) registerEndpoint("/v1/health/connect/", []string{"GET"}, (*HTTPServer).HealthConnectServiceNodes) + registerEndpoint("/v1/health/ingress/", []string{"GET"}, (*HTTPServer).HealthIngressServiceNodes) registerEndpoint("/v1/internal/ui/nodes", []string{"GET"}, (*HTTPServer).UINodes) registerEndpoint("/v1/internal/ui/node/", []string{"GET"}, (*HTTPServer).UINodeInfo) registerEndpoint("/v1/internal/ui/services", []string{"GET"}, (*HTTPServer).UIServices) diff --git a/api/health.go b/api/health.go index 37d25d000..99b9ac257 100644 --- a/api/health.go +++ b/api/health.go @@ -17,6 +17,12 @@ const ( HealthMaint = "maintenance" ) +const ( + serviceHealth = "service" + connectHealth = "connect" + ingressHealth = "ingress" +) + const ( // NodeMaint is the special key set by a node in maintenance mode. NodeMaint = "_node_maintenance" @@ -269,11 +275,11 @@ func (h *Health) Service(service, tag string, passingOnly bool, q *QueryOptions) if tag != "" { tags = []string{tag} } - return h.service(service, tags, passingOnly, q, false, false) + return h.service(service, tags, passingOnly, q, serviceHealth) } func (h *Health) ServiceMultipleTags(service string, tags []string, passingOnly bool, q *QueryOptions) ([]*ServiceEntry, *QueryMeta, error) { - return h.service(service, tags, passingOnly, q, false, false) + return h.service(service, tags, passingOnly, q, serviceHealth) } // Connect is equivalent to Service except that it will only return services @@ -286,25 +292,31 @@ func (h *Health) Connect(service, tag string, passingOnly bool, q *QueryOptions) if tag != "" { tags = []string{tag} } - return h.service(service, tags, passingOnly, q, true, false) + return h.service(service, tags, passingOnly, q, connectHealth) } func (h *Health) ConnectMultipleTags(service string, tags []string, passingOnly bool, q *QueryOptions) ([]*ServiceEntry, *QueryMeta, error) { - return h.service(service, tags, passingOnly, q, true, false) + return h.service(service, tags, passingOnly, q, connectHealth) } // Ingress is equivalent to Connect except that it will only return associated // ingress gateways for the requested service. func (h *Health) Ingress(service string, passingOnly bool, q *QueryOptions) ([]*ServiceEntry, *QueryMeta, error) { var tags []string - return h.service(service, tags, passingOnly, q, false, true) + return h.service(service, tags, passingOnly, q, ingressHealth) } -func (h *Health) service(service string, tags []string, passingOnly bool, q *QueryOptions, connect, ingress bool) ([]*ServiceEntry, *QueryMeta, error) { - path := "/v1/health/service/" + service - if connect || ingress { +func (h *Health) service(service string, tags []string, passingOnly bool, q *QueryOptions, healthType string) ([]*ServiceEntry, *QueryMeta, error) { + var path string + switch healthType { + case connectHealth: path = "/v1/health/connect/" + service + case ingressHealth: + path = "/v1/health/ingress/" + service + default: + path = "/v1/health/service/" + service } + r := h.c.newRequest("GET", path) r.setQueryOptions(q) if len(tags) > 0 { @@ -315,9 +327,6 @@ func (h *Health) service(service string, tags []string, passingOnly bool, q *Que if passingOnly { r.params.Set(HealthPassing, "1") } - if ingress { - r.params.Set("ingress", "1") - } rtt, resp, err := requireOK(h.c.doRequest(r)) if err != nil { return nil, nil, err diff --git a/api/health_test.go b/api/health_test.go index b0670c58c..356105c65 100644 --- a/api/health_test.go +++ b/api/health_test.go @@ -539,7 +539,7 @@ func TestAPI_HealthConnect_Filter(t *testing.T) { require.Len(t, services, 1) } -func TestAPI_HealthConnect_Ingress(t *testing.T) { +func TestAPI_HealthIngress(t *testing.T) { t.Parallel() c, s := makeClient(t) defer s.Stop()