Merge pull request #8064 from hashicorp/ingress/health-query-param

Add API query parameter ?ingress to allow users to find ingress gateways associated to a service

agent/health_endpoint.go

@@ -3,6 +3,7 @@ package agent
 import (
 	"fmt"
 	"net/http"
+	"net/url"
 	"strconv"
 	"strings"
 
@@ -163,7 +164,7 @@ func (s *HTTPServer) HealthServiceNodes(resp http.ResponseWriter, req *http.Requ
 
 func (s *HTTPServer) healthServiceNodes(resp http.ResponseWriter, req *http.Request, connect bool) (interface{}, error) {
 	// Set default DC
-	args := structs.ServiceSpecificRequest{Connect: connect}
+	args := structs.ServiceSpecificRequest{}
 	if err := s.parseEntMetaNoWildcard(req, &args.EnterpriseMeta); err != nil {
 		return nil, err
 	}
@@ -184,6 +185,20 @@ func (s *HTTPServer) healthServiceNodes(resp http.ResponseWriter, req *http.Requ
 	prefix := "/v1/health/service/"
 	if connect {
 		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
+		}
 	}
 
 	// Pull out the service name
@@ -224,26 +239,15 @@ func (s *HTTPServer) healthServiceNodes(resp http.ResponseWriter, req *http.Requ
 	out.ConsistencyLevel = args.QueryOptions.ConsistencyLevel()
 
 	// Filter to only passing if specified
-	if _, ok := params[api.HealthPassing]; ok {
+	filter, err := getBoolQueryParam(params, api.HealthPassing)
-		val := params.Get(api.HealthPassing)
+	if err != nil {
-		// Backwards-compat to allow users to specify ?passing without a value. This
+		resp.WriteHeader(http.StatusBadRequest)
-		// should be removed in Consul 0.10.
+		fmt.Fprint(resp, "Invalid value for ?passing")
-		var filter bool
+		return nil, nil
-		if val == "" {
+	}
-			filter = true
-		} else {
-			var err error
-			filter, err = strconv.ParseBool(val)
-			if err != nil {
-				resp.WriteHeader(http.StatusBadRequest)
-				fmt.Fprint(resp, "Invalid value for ?passing")
-				return nil, nil
-			}
-		}
 
-		if filter {
+	if filter {
-			out.Nodes = filterNonPassing(out.Nodes)
+		out.Nodes = filterNonPassing(out.Nodes)
-		}
 	}
 
 	// Translate addresses after filtering so we don't waste effort.
@@ -273,6 +277,27 @@ func (s *HTTPServer) healthServiceNodes(resp http.ResponseWriter, req *http.Requ
 	return out.Nodes, nil
 }
 
+func getBoolQueryParam(params url.Values, key string) (bool, error) {
+	var param bool
+	if _, ok := params[key]; ok {
+		val := params.Get(key)
+		// Orginally a comment declared this check should be removed after Consul
+		// 0.10, to no longer support using ?passing without a value. However, I
+		// think this is a reasonable experience for a user and so am keeping it
+		// here.
+		if val == "" {
+			param = true
+		} else {
+			var err error
+			param, err = strconv.ParseBool(val)
+			if err != nil {
+				return false, err
+			}
+		}
+	}
+	return param, nil
+}
+
 // filterNonPassing is used to filter out any nodes that have check that are not passing
 func filterNonPassing(nodes structs.CheckServiceNodes) structs.CheckServiceNodes {
 	n := len(nodes)

agent/health_endpoint_test.go

@@ -1139,6 +1139,105 @@ func TestHealthConnectServiceNodes(t *testing.T) {
 	assert.Len(nodes[0].Checks, 0)
 }
 
+func TestHealthConnectServiceNodes_Ingress(t *testing.T) {
+	t.Parallel()
+
+	a := NewTestAgent(t, "")
+	defer a.Shutdown()
+	testrpc.WaitForLeader(t, a.RPC, "dc1")
+
+	// Register gateway
+	gatewayArgs := structs.TestRegisterIngressGateway(t)
+	gatewayArgs.Service.Address = "127.0.0.27"
+	var out struct{}
+	require.NoError(t, a.RPC("Catalog.Register", gatewayArgs, &out))
+
+	args := structs.TestRegisterRequest(t)
+	require.NoError(t, a.RPC("Catalog.Register", args, &out))
+
+	// Associate service to gateway
+	cfgArgs := &structs.IngressGatewayConfigEntry{
+		Name: "ingress-gateway",
+		Kind: structs.IngressGateway,
+		Listeners: []structs.IngressListener{
+			{
+				Port:     8888,
+				Protocol: "tcp",
+				Services: []structs.IngressService{
+					{Name: args.Service.Service},
+				},
+			},
+		},
+	}
+
+	req := structs.ConfigEntryRequest{
+		Op:         structs.ConfigEntryUpsert,
+		Datacenter: "dc1",
+		Entry:      cfgArgs,
+	}
+	var outB bool
+	require.Nil(t, a.RPC("ConfigEntry.Apply", req, &outB))
+	require.True(t, outB)
+
+	t.Run("no_query_value", func(t *testing.T) {
+		assert := assert.New(t)
+		req, _ := http.NewRequest("GET", fmt.Sprintf(
+			"/v1/health/connect/%s?ingress", args.Service.Service), 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("true_value", func(t *testing.T) {
+		assert := assert.New(t)
+		req, _ := http.NewRequest("GET", fmt.Sprintf(
+			"/v1/health/connect/%s?ingress=true", args.Service.Service), 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)
+		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) {
 	t.Parallel()
 

agent/structs/structs.go

@@ -513,9 +513,6 @@ type ServiceSpecificRequest struct {
 	// Connect if true will only search for Connect-compatible services.
 	Connect bool
 
-	// TODO(ingress): Add corresponding API changes after figuring out what the
-	// HTTP endpoint looks like
-
 	// Ingress if true will only search for Ingress gateways for the given service.
 	Ingress bool
 

api/health.go

@@ -269,11 +269,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)
+	return h.service(service, tags, passingOnly, q, false, false)
 }
 
 func (h *Health) ServiceMultipleTags(service string, tags []string, passingOnly bool, q *QueryOptions) ([]*ServiceEntry, *QueryMeta, error) {
-	return h.service(service, tags, passingOnly, q, false)
+	return h.service(service, tags, passingOnly, q, false, false)
 }
 
 // Connect is equivalent to Service except that it will only return services
@@ -286,16 +286,23 @@ func (h *Health) Connect(service, tag string, passingOnly bool, q *QueryOptions)
 	if tag != "" {
 		tags = []string{tag}
 	}
-	return h.service(service, tags, passingOnly, q, true)
+	return h.service(service, tags, passingOnly, q, true, false)
 }
 
 func (h *Health) ConnectMultipleTags(service string, tags []string, passingOnly bool, q *QueryOptions) ([]*ServiceEntry, *QueryMeta, error) {
-	return h.service(service, tags, passingOnly, q, true)
+	return h.service(service, tags, passingOnly, q, true, false)
 }
 
-func (h *Health) service(service string, tags []string, passingOnly bool, q *QueryOptions, connect bool) ([]*ServiceEntry, *QueryMeta, error) {
+// 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)
+}
+
+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 {
+	if connect || ingress {
 		path = "/v1/health/connect/" + service
 	}
 	r := h.c.newRequest("GET", path)
@@ -308,6 +315,9 @@ 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

api/health_test.go

@@ -211,7 +211,6 @@ func TestAPI_HealthChecks(t *testing.T) {
 	if err := agent.ServiceRegister(reg); err != nil {
 		t.Fatalf("err: %v", err)
 	}
-	defer agent.ServiceDeregister("foo")
 
 	retry.Run(t, func(r *retry.R) {
 		checks := HealthChecks{
@@ -264,7 +263,6 @@ func TestAPI_HealthChecks_NodeMetaFilter(t *testing.T) {
 	if err := agent.ServiceRegister(reg); err != nil {
 		t.Fatalf("err: %v", err)
 	}
-	defer agent.ServiceDeregister("foo")
 
 	retry.Run(t, func(r *retry.R) {
 		checks, meta, err := health.Checks("foo", &QueryOptions{NodeMeta: meta})
@@ -354,7 +352,6 @@ func TestAPI_HealthService_SingleTag(t *testing.T) {
 		},
 	}
 	require.NoError(t, agent.ServiceRegister(reg))
-	defer agent.ServiceDeregister("foo1")
 	retry.Run(t, func(r *retry.R) {
 		services, meta, err := health.Service("foo", "bar", true, nil)
 		require.NoError(r, err)
@@ -390,7 +387,6 @@ func TestAPI_HealthService_MultipleTags(t *testing.T) {
 		},
 	}
 	require.NoError(t, agent.ServiceRegister(reg))
-	defer agent.ServiceDeregister("foo1")
 
 	reg2 := &AgentServiceRegistration{
 		Name: "foo",
@@ -402,7 +398,6 @@ func TestAPI_HealthService_MultipleTags(t *testing.T) {
 		},
 	}
 	require.NoError(t, agent.ServiceRegister(reg2))
-	defer agent.ServiceDeregister("foo2")
 
 	// Test searching with one tag (two results)
 	retry.Run(t, func(r *retry.R) {
@@ -488,7 +483,6 @@ func TestAPI_HealthConnect(t *testing.T) {
 	}
 	err := agent.ServiceRegister(reg)
 	require.NoError(t, err)
-	defer agent.ServiceDeregister("foo")
 
 	// Register the proxy
 	proxyReg := &AgentServiceRegistration{
@@ -501,7 +495,6 @@ func TestAPI_HealthConnect(t *testing.T) {
 	}
 	err = agent.ServiceRegister(proxyReg)
 	require.NoError(t, err)
-	defer agent.ServiceDeregister("foo-proxy")
 
 	retry.Run(t, func(r *retry.R) {
 		services, meta, err := health.Connect("foo", "", true, nil)
@@ -546,6 +539,67 @@ func TestAPI_HealthConnect_Filter(t *testing.T) {
 	require.Len(t, services, 1)
 }
 
+func TestAPI_HealthConnect_Ingress(t *testing.T) {
+	t.Parallel()
+	c, s := makeClient(t)
+	defer s.Stop()
+
+	agent := c.Agent()
+	health := c.Health()
+
+	s.WaitForSerfCheck(t)
+
+	// Make a service with a proxy
+	reg := &AgentServiceRegistration{
+		Name: "foo",
+		Port: 8000,
+	}
+	err := agent.ServiceRegister(reg)
+	require.NoError(t, err)
+
+	// Register the gateway
+	gatewayReg := &AgentServiceRegistration{
+		Name: "foo-gateway",
+		Port: 8001,
+		Kind: ServiceKindIngressGateway,
+	}
+	err = agent.ServiceRegister(gatewayReg)
+	require.NoError(t, err)
+
+	// Associate service and gateway
+	gatewayConfig := &IngressGatewayConfigEntry{
+		Kind: IngressGateway,
+		Name: "foo-gateway",
+		Listeners: []IngressListener{
+			{
+				Port:     2222,
+				Protocol: "tcp",
+				Services: []IngressService{
+					{
+						Name: "foo",
+					},
+				},
+			},
+		},
+	}
+	_, wm, err := c.ConfigEntries().Set(gatewayConfig, nil)
+	require.NoError(t, err)
+	require.NotNil(t, wm)
+
+	retry.Run(t, func(r *retry.R) {
+		services, meta, err := health.Ingress("foo", true, nil)
+		require.NoError(r, err)
+
+		require.NotZero(r, meta.LastIndex)
+
+		// Should be exactly 1 service - the original shouldn't show up as a connect
+		// endpoint, only it's proxy.
+		require.Len(r, services, 1)
+		require.Equal(r, services[0].Node.Datacenter, "dc1")
+		require.Equal(r, services[0].Service.Service, gatewayReg.Name)
+	})
+}
+
 func TestAPI_HealthState(t *testing.T) {
 	t.Parallel()
 	c, s := makeClient(t)