Save exposed ports in agent's store and expose them via API (#10173)

* Save exposed HTTP or GRPC ports to the agent's store
* Add those the health checks API so we can retrieve them from the API
* Change redirect-traffic command to also exclude those ports from inbound traffic redirection when expose.checks is set to true.

.changelog/10173.txt

@@ -0,0 +1,9 @@
+```release-note:improvement
+agent: Save exposed Envoy ports to the agent's state when `Expose.Checks` is true in proxy's configuration.
+```
+```release-note:improvement
+api: Add `ExposedPort` to the health check API resource.
+```
+```release-note:improvement
+command: Exclude exposed Envoy ports from traffic redirection when providing `-proxy-id` and `Expose.Checks` is set.
+```

agent/agent.go

@@ -2555,6 +2555,7 @@ func (a *Agent) addCheck(check *structs.HealthCheck, chkType *structs.CheckType,
 					return err
 				}
 				http.ProxyHTTP = httpInjectAddr(http.HTTP, proxy.Address, port)
+				check.ExposedPort = port
 			}
 
 			http.Start()
@@ -2624,6 +2625,7 @@ func (a *Agent) addCheck(check *structs.HealthCheck, chkType *structs.CheckType,
 					return err
 				}
 				grpc.ProxyGRPC = grpcInjectAddr(grpc.GRPC, proxy.Address, port)
+				check.ExposedPort = port
 			}
 
 			grpc.Start()
@@ -3809,6 +3811,8 @@ func (a *Agent) rerouteExposedChecks(serviceID structs.ServiceID, proxyAddr stri
 			return err
 		}
 		c.ProxyHTTP = httpInjectAddr(c.HTTP, proxyAddr, port)
+		hc := a.State.Check(cid)
+		hc.ExposedPort = port
 	}
 	for cid, c := range a.checkGRPCs {
 		if c.ServiceID != serviceID {
@@ -3819,6 +3823,8 @@ func (a *Agent) rerouteExposedChecks(serviceID structs.ServiceID, proxyAddr stri
 			return err
 		}
 		c.ProxyGRPC = grpcInjectAddr(c.GRPC, proxyAddr, port)
+		hc := a.State.Check(cid)
+		hc.ExposedPort = port
 	}
 	return nil
 }
@@ -3831,12 +3837,16 @@ func (a *Agent) resetExposedChecks(serviceID structs.ServiceID) {
 	for cid, c := range a.checkHTTPs {
 		if c.ServiceID == serviceID {
 			c.ProxyHTTP = ""
+			hc := a.State.Check(cid)
+			hc.ExposedPort = 0
 			ids = append(ids, cid)
 		}
 	}
 	for cid, c := range a.checkGRPCs {
 		if c.ServiceID == serviceID {
 			c.ProxyGRPC = ""
+			hc := a.State.Check(cid)
+			hc.ExposedPort = 0
 			ids = append(ids, cid)
 		}
 	}

agent/agent_test.go

@@ -1389,6 +1389,7 @@ func TestAgent_IndexChurn(t *testing.T) {
 // verifyIndexChurn registers some things and runs anti-entropy a bunch of times
 // in a row to make sure there are no index bumps.
 func verifyIndexChurn(t *testing.T, tags []string) {
+	t.Helper()
 	a := NewTestAgent(t, "")
 	defer a.Shutdown()
 
@@ -4299,8 +4300,8 @@ func TestAgent_RerouteExistingHTTPChecks(t *testing.T) {
 		t.Fatalf("failed to add svc: %v", err)
 	}
 
-	// Register a proxy and expose HTTP checks
-	// This should trigger setting ProxyHTTP and ProxyGRPC in the checks
+	// Register a proxy and expose HTTP checks.
+	// This should trigger setting ProxyHTTP and ProxyGRPC in the checks.
 	proxy := &structs.NodeService{
 		Kind:    "connect-proxy",
 		ID:      "web-proxy",
@@ -4324,36 +4325,30 @@ func TestAgent_RerouteExistingHTTPChecks(t *testing.T) {
 
 	retry.Run(t, func(r *retry.R) {
 		chks := a.ServiceHTTPBasedChecks(structs.NewServiceID("web", nil))
+		require.Equal(r, chks[0].ProxyHTTP, "http://localhost:21500/mypath?query")
+	})
 
-		got := chks[0].ProxyHTTP
-		if got == "" {
-			r.Fatal("proxyHTTP addr not set in check")
-		}
-
-		want := "http://localhost:21500/mypath?query"
-		if got != want {
-			r.Fatalf("unexpected proxy addr in check, want: %s, got: %s", want, got)
-		}
+	retry.Run(t, func(r *retry.R) {
+		hc := a.State.Check(structs.NewCheckID("http", nil))
+		require.Equal(r, hc.ExposedPort, 21500)
 	})
 
 	retry.Run(t, func(r *retry.R) {
 		chks := a.ServiceHTTPBasedChecks(structs.NewServiceID("web", nil))
 
-		// Will be at a later index than HTTP check because of the fetching order in ServiceHTTPBasedChecks
-		got := chks[1].ProxyGRPC
-		if got == "" {
-			r.Fatal("ProxyGRPC addr not set in check")
-		}
+		// GRPC check will be at a later index than HTTP check because of the fetching order in ServiceHTTPBasedChecks.
+		// Note that this relies on listener ports auto-incrementing in a.listenerPortLocked.
+		require.Equal(r, chks[1].ProxyGRPC, "localhost:21501/myservice")
+	})
 
-		// Node that this relies on listener ports auto-incrementing in a.listenerPortLocked
-		want := "localhost:21501/myservice"
-		if got != want {
-			r.Fatalf("unexpected proxy addr in check, want: %s, got: %s", want, got)
-		}
+	retry.Run(t, func(r *retry.R) {
+		hc := a.State.Check(structs.NewCheckID("grpc", nil))
+		require.Equal(r, hc.ExposedPort, 21501)
 	})
 
-	// Re-register a proxy and disable exposing HTTP checks
+	// Re-register a proxy and disable exposing HTTP checks.
 	// This should trigger resetting ProxyHTTP and ProxyGRPC to empty strings
+	// and reset saved exposed ports in the agent's state.
 	proxy = &structs.NodeService{
 		Kind:    "connect-proxy",
 		ID:      "web-proxy",
@@ -4377,21 +4372,24 @@ func TestAgent_RerouteExistingHTTPChecks(t *testing.T) {
 
 	retry.Run(t, func(r *retry.R) {
 		chks := a.ServiceHTTPBasedChecks(structs.NewServiceID("web", nil))
+		require.Empty(r, chks[0].ProxyHTTP, "ProxyHTTP addr was not reset")
+	})
 
-		got := chks[0].ProxyHTTP
-		if got != "" {
-			r.Fatal("ProxyHTTP addr was not reset")
-		}
+	retry.Run(t, func(r *retry.R) {
+		hc := a.State.Check(structs.NewCheckID("http", nil))
+		require.Equal(r, hc.ExposedPort, 0)
 	})
 
 	retry.Run(t, func(r *retry.R) {
 		chks := a.ServiceHTTPBasedChecks(structs.NewServiceID("web", nil))
 
-		// Will be at a later index than HTTP check because of the fetching order in ServiceHTTPBasedChecks
-		got := chks[1].ProxyGRPC
-		if got != "" {
-			r.Fatal("ProxyGRPC addr was not reset")
-		}
+		// Will be at a later index than HTTP check because of the fetching order in ServiceHTTPBasedChecks.
+		require.Empty(r, chks[1].ProxyGRPC, "ProxyGRPC addr was not reset")
+	})
+
+	retry.Run(t, func(r *retry.R) {
+		hc := a.State.Check(structs.NewCheckID("grpc", nil))
+		require.Equal(r, hc.ExposedPort, 0)
 	})
 }
 
@@ -4480,31 +4478,24 @@ func TestAgent_RerouteNewHTTPChecks(t *testing.T) {
 
 	retry.Run(t, func(r *retry.R) {
 		chks := a.ServiceHTTPBasedChecks(structs.NewServiceID("web", nil))
+		require.Equal(r, chks[0].ProxyHTTP, "http://localhost:21500/mypath?query")
+	})
 
-		got := chks[0].ProxyHTTP
-		if got == "" {
-			r.Fatal("ProxyHTTP addr not set in check")
-		}
-
-		want := "http://localhost:21500/mypath?query"
-		if got != want {
-			r.Fatalf("unexpected proxy addr in http check, want: %s, got: %s", want, got)
-		}
+	retry.Run(t, func(r *retry.R) {
+		hc := a.State.Check(structs.NewCheckID("http", nil))
+		require.Equal(r, hc.ExposedPort, 21500)
 	})
 
 	retry.Run(t, func(r *retry.R) {
 		chks := a.ServiceHTTPBasedChecks(structs.NewServiceID("web", nil))
 
-		// Will be at a later index than HTTP check because of the fetching order in ServiceHTTPBasedChecks
-		got := chks[1].ProxyGRPC
-		if got == "" {
-			r.Fatal("ProxyGRPC addr not set in check")
-		}
+		// GRPC check will be at a later index than HTTP check because of the fetching order in ServiceHTTPBasedChecks.
+		require.Equal(r, chks[1].ProxyGRPC, "localhost:21501/myservice")
+	})
 
-		want := "localhost:21501/myservice"
-		if got != want {
-			r.Fatalf("unexpected proxy addr in grpc check, want: %s, got: %s", want, got)
-		}
+	retry.Run(t, func(r *retry.R) {
+		hc := a.State.Check(structs.NewCheckID("grpc", nil))
+		require.Equal(r, hc.ExposedPort, 21501)
 	})
 }
 

agent/structs/structs.go

@@ -1400,7 +1400,7 @@ type NodeServiceList struct {
 	Services []*NodeService
 }
 
-// HealthCheck represents a single check on a given node
+// HealthCheck represents a single check on a given node.
 type HealthCheck struct {
 	Node        string
 	CheckID     types.CheckID // Unique per-node ID
@@ -1413,6 +1413,10 @@ type HealthCheck struct {
 	ServiceTags []string      // optional service tags
 	Type        string        // Check type: http/ttl/tcp/etc
 
+	// ExposedPort is the port of the exposed Envoy listener representing the
+	// HTTP or GRPC health check of the service.
+	ExposedPort int
+
 	Definition HealthCheckDefinition `bexpr:"-"`
 
 	EnterpriseMeta `hcl:",squash" mapstructure:",squash" bexpr:"-"`

agent/structs/structs_filtering_test.go

@@ -9,7 +9,7 @@ import (
 	"sync"
 	"testing"
 
-	bexpr "github.com/hashicorp/go-bexpr"
+	"github.com/hashicorp/go-bexpr"
 	"github.com/mitchellh/pointerstructure"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
@@ -545,6 +545,11 @@ var expectedFieldConfigHealthCheck bexpr.FieldConfigurations = bexpr.FieldConfig
 		SupportedOperations: []bexpr.MatchOperator{bexpr.MatchEqual, bexpr.MatchNotEqual, bexpr.MatchIn, bexpr.MatchNotIn, bexpr.MatchMatches, bexpr.MatchNotMatches},
 		StructFieldName:     "Type",
 	},
+	"ExposedPort": &bexpr.FieldConfiguration{
+		CoerceFn:            bexpr.CoerceInt,
+		SupportedOperations: []bexpr.MatchOperator{bexpr.MatchEqual, bexpr.MatchNotEqual},
+		StructFieldName:     "ExposedPort",
+	},
 }
 
 var expectedFieldConfigCheckServiceNode bexpr.FieldConfigurations = bexpr.FieldConfigurations{

api/agent.go

@@ -63,6 +63,7 @@ type AgentCheck struct {
 	ServiceID   string
 	ServiceName string
 	Type        string
+	ExposedPort int
 	Definition  HealthCheckDefinition
 	Namespace   string `json:",omitempty"`
 }

command/connect/redirecttraffic/redirect_traffic.go

@@ -197,6 +197,21 @@ func (c *cmd) generateConfigFromFlags() (iptables.Config, error) {
 				cfg.ExcludeInboundPorts = append(cfg.ExcludeInboundPorts, strconv.Itoa(exposePath.ListenerPort))
 			}
 		}
+
+		// Exclude any exposed health check ports when Proxy.Expose.Checks is true.
+		if svc.Proxy.Expose.Checks {
+			// Get the health checks of the destination service.
+			checks, err := c.client.Agent().ChecksWithFilter(fmt.Sprintf("ServiceName == %q", svc.Proxy.DestinationServiceName))
+			if err != nil {
+				return iptables.Config{}, err
+			}
+
+			for _, check := range checks {
+				if check.ExposedPort != 0 {
+					cfg.ExcludeInboundPorts = append(cfg.ExcludeInboundPorts, strconv.Itoa(check.ExposedPort))
+				}
+			}
+		}
 	}
 
 	for _, port := range c.excludeInboundPorts {