api: ensure v1/health/ingress/:service endpoint works properly when streaming is enabled

.changelog/9967.txt

@@ -0,0 +1,3 @@
+```release-note:bug
+api: ensure v1/health/ingress/:service endpoint works properly when streaming is enabled
+```

agent/agent.go

@@ -322,7 +322,8 @@ type Agent struct {
 
 	// TODO: pass directly to HTTPHandlers and DNSServer once those are passed
 	// into Agent, which will allow us to remove this field.
-	rpcClientHealth *health.Client
+	rpcClientHealth            *health.Client
+	rpcClientHealthNoStreaming *health.Client
 
 	// enterpriseAgent embeds fields that we only access in consul-enterprise builds
 	enterpriseAgent
@@ -378,6 +379,11 @@ func New(bd BaseDeps) (*Agent, error) {
 		NetRPC:    &a,
 		CacheName: cacheName,
 	}
+	a.rpcClientHealthNoStreaming = &health.Client{
+		Cache:     bd.Cache,
+		NetRPC:    &a,
+		CacheName: cachetype.HealthServicesName,
+	}
 
 	a.serviceManager = NewServiceManager(&a)
 

agent/health_endpoint.go

@@ -219,14 +219,19 @@ func (s *HTTPHandlers) healthServiceNodes(resp http.ResponseWriter, req *http.Re
 		return nil, nil
 	}
 
-	useStreaming := s.agent.config.UseStreamingBackend && args.MinQueryIndex > 0
+	useStreaming := s.agent.config.UseStreamingBackend && args.MinQueryIndex > 0 && !args.Ingress
 	args.QueryOptions.UseCache = s.agent.config.HTTPUseCache && (args.QueryOptions.UseCache || useStreaming)
 
 	if args.QueryOptions.UseCache && useStreaming && args.Source.Node != "" {
 		return nil, BadRequestError{Reason: "'near' query param can not be used with streaming"}
 	}
 
-	out, md, err := s.agent.rpcClientHealth.ServiceNodes(req.Context(), args)
+	healthClient := s.agent.rpcClientHealth
+	if args.Ingress {
+		healthClient = s.agent.rpcClientHealthNoStreaming
+	}
+
+	out, md, err := healthClient.ServiceNodes(req.Context(), args)
 	if err != nil {
 		return nil, err
 	}

agent/health_endpoint_test.go

@@ -1418,13 +1418,22 @@ func TestHealthConnectServiceNodes(t *testing.T) {
 }
 
 func TestHealthIngressServiceNodes(t *testing.T) {
+	t.Run("no streaming", func(t *testing.T) {
+		testHealthIngressServiceNodes(t, ` rpc { enable_streaming = false } use_streaming_backend = false `)
+	})
+	t.Run("cache with streaming", func(t *testing.T) {
+		testHealthIngressServiceNodes(t, ` rpc { enable_streaming = true } use_streaming_backend = true `)
+	})
+}
+
+func testHealthIngressServiceNodes(t *testing.T, agentHCL string) {
+	t.Helper()
+
 	if testing.Short() {
 		t.Skip("too slow for testing.Short")
 	}
 
-	t.Parallel()
-
-	a := NewTestAgent(t, "")
+	a := NewTestAgent(t, agentHCL)
 	defer a.Shutdown()
 	testrpc.WaitForLeader(t, a.RPC, "dc1")
 
@@ -1461,34 +1470,64 @@ func TestHealthIngressServiceNodes(t *testing.T) {
 	require.Nil(t, a.RPC("ConfigEntry.Apply", req, &outB))
 	require.True(t, outB)
 
-	t.Run("associated service", func(t *testing.T) {
-		assert := assert.New(t)
+	checkResults := func(t *testing.T, obj interface{}) {
+		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)
+	}
+
+	require.True(t, t.Run("associated service", func(t *testing.T) {
 		req, _ := http.NewRequest("GET", fmt.Sprintf(
 			"/v1/health/ingress/%s", args.Service.Service), nil)
 		resp := httptest.NewRecorder()
 		obj, err := a.srv.HealthIngressServiceNodes(resp, req)
-		assert.Nil(err)
+		require.NoError(t, 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)
-	})
+		checkResults(t, obj)
+	}))
 
-	t.Run("non-associated service", func(t *testing.T) {
-		assert := assert.New(t)
+	require.True(t, t.Run("non-associated service", func(t *testing.T) {
 		req, _ := http.NewRequest("GET",
 			"/v1/health/connect/notexist", nil)
 		resp := httptest.NewRecorder()
 		obj, err := a.srv.HealthIngressServiceNodes(resp, req)
-		assert.Nil(err)
+		require.NoError(t, err)
 		assertIndex(t, resp)
 
 		nodes := obj.(structs.CheckServiceNodes)
 		require.Len(t, nodes, 0)
-	})
+	}))
+
+	require.True(t, t.Run("test caching miss", func(t *testing.T) {
+		// List instances with cache enabled
+		req, _ := http.NewRequest("GET", fmt.Sprintf(
+			"/v1/health/ingress/%s?cached", args.Service.Service), nil)
+		resp := httptest.NewRecorder()
+		obj, err := a.srv.HealthIngressServiceNodes(resp, req)
+		require.NoError(t, err)
+
+		checkResults(t, obj)
+
+		// Should be a cache miss
+		require.Equal(t, "MISS", resp.Header().Get("X-Cache"))
+	}))
+
+	require.True(t, t.Run("test caching hit", func(t *testing.T) {
+		// List instances with cache enabled
+		req, _ := http.NewRequest("GET", fmt.Sprintf(
+			"/v1/health/ingress/%s?cached", args.Service.Service), nil)
+		resp := httptest.NewRecorder()
+		obj, err := a.srv.HealthIngressServiceNodes(resp, req)
+		require.NoError(t, err)
+
+		checkResults(t, obj)
+
+		// Should be a cache HIT now!
+		require.Equal(t, "HIT", resp.Header().Get("X-Cache"))
+	}))
 }
 
 func TestHealthConnectServiceNodes_Filter(t *testing.T) {