Abhishek/fix api gw peering

.changelog/23454.txt

@@ -0,0 +1,3 @@
+```release-note:bug
+xds: Fixed XDS package to generate correct endpoints and cluster configurations for API Gateways when peered, and updated the API Gateway update handler to propogate mesh gateway config to its upstreams.
+```

agent/proxycfg/api_gateway.go

@@ -367,6 +367,19 @@ func (h *handlerAPIGateway) handleRouteConfigUpdate(ctx context.Context, u Updat
 		for _, rule := range route.Rules {
 			for _, service := range rule.Services {
 				effectiveLimits := apiGatewayEffectiveUpstreamLimits(defaultLimits, service.Limits)
+
+				// Retrieving the meshGatewayConfig from handlerAPIGateway instance.
+				// `handlerAPIGateway` embeds `handlerState`, which exposes `serviceInstance.proxyCfg`.
+				// serviceInstance.proxyCfg.MeshGateway is replicated from NodeService during state setup/update.
+				// and NodeService populated for all gateway's during service resistration `AgentRegisterService`.
+				//
+				// So, Whenever any change happens in NodeService, proxyCfg manager will recreate
+				// the state where it copies NodeService to serviceInstance and
+				// then calls this api_gateway handleUpdates method.
+				// which will update the Mesh-Gateway config to api_gateway upstreams (below).
+				// h.service = <name of api-gateway>
+				meshGatewayConfig := h.proxyCfg.MeshGateway
+
 				for _, listener := range snap.APIGateway.Listeners {
 					shouldBind := false
 					for _, parent := range route.Parents {
@@ -393,6 +406,11 @@ func (h *handlerAPIGateway) handleRouteConfigUpdate(ctx context.Context, u Updat
 						// Pass the protocol that was configured on the listener in order
 						// to force that protocol on the Envoy listener.
 						Config: upstreamCfg,
+
+						// Propogate the meshGatewayConfig in api gateway upstreams
+						// so that meshGatewayMode can be used in XDS for
+						// endpoints and cluster config generation.
+						MeshGateway: meshGatewayConfig,
 					}
 
 					listenerKey := APIGatewayListenerKeyFromListener(listener)
@@ -425,6 +443,7 @@ func (h *handlerAPIGateway) handleRouteConfigUpdate(ctx context.Context, u Updat
 
 		for _, service := range route.Services {
 			effectiveLimits := apiGatewayEffectiveUpstreamLimits(defaultLimits, service.Limits)
+			meshGatewayConfig := h.proxyCfg.MeshGateway
 			upstreamID := NewUpstreamIDFromServiceName(service.ServiceName())
 			seenUpstreamIDs.add(upstreamID)
 
@@ -454,7 +473,8 @@ func (h *handlerAPIGateway) handleRouteConfigUpdate(ctx context.Context, u Updat
 					LocalBindPort:        listener.Port,
 					// Pass the protocol that was configured on the ingress listener in order
 					// to force that protocol on the Envoy listener.
-					Config: upstreamCfg,
+					Config:      upstreamCfg,
+					MeshGateway: meshGatewayConfig,
 				}
 
 				listenerKey := APIGatewayListenerKeyFromListener(listener)

agent/proxycfg/state.go

@@ -226,6 +226,7 @@ func newKindHandler(config stateConfig, s serviceInstance, ch chan UpdateEvent)
 	case structs.ServiceKindIngressGateway:
 		handler = &handlerIngressGateway{handlerState: h}
 	case structs.ServiceKindAPIGateway:
+		h.logger = config.logger.Named(logging.APIGateway)
 		handler = &handlerAPIGateway{handlerState: h}
 	default:
 		return nil, errors.New("not a connect-proxy, terminating-gateway, mesh-gateway, or ingress-gateway")

agent/proxycfg/state_test.go

@@ -26,7 +26,7 @@ import (
 	"github.com/hashicorp/consul/sdk/testutil"
 )
 
-func TestStateChanged(t *testing.T) {
+func TestStateChangedConnectProxy(t *testing.T) {
 	tests := []struct {
 		name   string
 		ns     *structs.NodeService
@@ -108,6 +108,131 @@ func TestStateChanged(t *testing.T) {
 			},
 			want: true,
 		},
+		{
+			name:  "different proxy mesh gateway mode",
+			ns:    structs.TestNodeServiceAPIGateway(t),
+			token: "foo",
+			mutate: func(ns structs.NodeService, token string) (*structs.NodeService, string) {
+				ns.Proxy.MeshGateway.Mode = structs.MeshGatewayModeLocal
+				return &ns, token
+			},
+			want: true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			proxyID := ProxyID{ServiceID: tt.ns.CompoundServiceID()}
+			state, err := newState(proxyID, tt.ns, testSource, tt.token, stateConfig{logger: hclog.New(nil)}, rate.NewLimiter(rate.Inf, 1))
+			require.NoError(t, err)
+			otherNS, otherToken := tt.mutate(*tt.ns, tt.token)
+			require.Equal(t, tt.want, state.Changed(otherNS, otherToken))
+		})
+	}
+}
+
+func TestStateChangedAPIGateway(t *testing.T) {
+	tests := []struct {
+		name   string
+		ns     *structs.NodeService
+		token  string
+		mutate func(ns structs.NodeService, token string) (*structs.NodeService, string)
+		want   bool
+	}{
+		{
+			name: "nil node service",
+			ns:   structs.TestNodeServiceAPIGateway(t),
+			mutate: func(ns structs.NodeService, token string) (*structs.NodeService, string) {
+				return nil, token
+			},
+			want: true,
+		},
+		{
+			name: "same service",
+			ns:   structs.TestNodeServiceAPIGateway(t),
+			mutate: func(ns structs.NodeService, token string) (*structs.NodeService, string) {
+				return &ns, token
+			}, want: false,
+		},
+		{
+			name:  "same service, different token",
+			ns:    structs.TestNodeServiceAPIGateway(t),
+			token: "foo",
+			mutate: func(ns structs.NodeService, token string) (*structs.NodeService, string) {
+				return &ns, "bar"
+			},
+			want: true,
+		},
+		{
+			name:  "different address",
+			ns:    structs.TestNodeServiceAPIGateway(t),
+			token: "foo",
+			mutate: func(ns structs.NodeService, token string) (*structs.NodeService, string) {
+				ns.Address = "10.10.10.10"
+				return &ns, token
+			},
+			want: true,
+		},
+		{
+			name:  "different port",
+			ns:    structs.TestNodeServiceAPIGateway(t),
+			token: "foo",
+			mutate: func(ns structs.NodeService, token string) (*structs.NodeService, string) {
+				ns.Port = 12345
+				return &ns, token
+			},
+			want: true,
+		},
+		{
+			name:  "different service kind",
+			ns:    structs.TestNodeServiceAPIGateway(t),
+			token: "foo",
+			mutate: func(ns structs.NodeService, token string) (*structs.NodeService, string) {
+				ns.Kind = ""
+				return &ns, token
+			},
+			want: true,
+		},
+		{
+			name:  "different proxy target",
+			ns:    structs.TestNodeServiceAPIGateway(t),
+			token: "foo",
+			mutate: func(ns structs.NodeService, token string) (*structs.NodeService, string) {
+				ns.Proxy.DestinationServiceName = "badger"
+				return &ns, token
+			},
+			want: true,
+		},
+		{
+			name:  "different proxy upstreams",
+			ns:    structs.TestNodeServiceAPIGateway(t),
+			token: "foo",
+			mutate: func(ns structs.NodeService, token string) (*structs.NodeService, string) {
+				ns.Proxy.Upstreams = nil
+				return &ns, token
+			},
+			want: true,
+		},
+		{
+			name:  "different mesh gateway mode (local)",
+			ns:    structs.TestNodeServiceAPIGateway(t),
+			token: "foo",
+			mutate: func(ns structs.NodeService, token string) (*structs.NodeService, string) {
+				ns.Proxy.MeshGateway.Mode = structs.MeshGatewayModeLocal
+				return &ns, token
+			},
+			want: true,
+		},
+		{
+			name:  "different mesh gateway mode (remote)",
+			ns:    structs.TestNodeServiceAPIGateway(t),
+			token: "foo",
+			mutate: func(ns structs.NodeService, token string) (*structs.NodeService, string) {
+				ns.Proxy.MeshGateway.Mode = structs.MeshGatewayModeRemote
+				return &ns, token
+			},
+			want: true,
+		},
 	}
 
 	for _, tt := range tests {

agent/structs/testing_catalog.go

@@ -176,10 +176,35 @@ func TestNodeServiceMeshGateway(t testing.T) *NodeService {
 }
 
 func TestNodeServiceAPIGateway(t testing.T) *NodeService {
+	entMeta := DefaultEnterpriseMetaInPartition("")
 	return &NodeService{
 		Kind:    ServiceKindAPIGateway,
 		Service: "api-gateway",
 		Address: "1.1.1.1",
+
+		// ---------------------------------------
+		// Adding TestConnectProxyConfig to the proxy field here within TestNodeServiceAPIGateway
+		// to test whether APIGateway is able to handle state changes within ConnectProxyConfig (TestStateChangedAPIGateway).
+		// Please note:
+		// The naming may suggest that ConnectProxyConfig should only be used for ConnectProxy,
+		// but "APIGateway state" uses serviceInstance, which embeds ConnectProxyConfig as part of its state,
+		// so any changes to ConnectProxyConfig will also impact APIGateway, such as change in "mesh gateway mode".
+		//
+		// For example, let's say a user updates the mesh gateway mode of an API gateway,
+		// First NodeService.Proxy will be updated and then proxyCfg manager detects change in config
+		// and it recreates the state for api_gateway which would copy the
+		// NodeService.Proxy.MeshGateway to serviceInstance.proxyCfg.MeshGateway in `newServiceInstanceFromNodeService` (serviceInstance is part of state)
+		// and then proxyCfg manager calls the api_gateway handleUpdates method which would
+		// update the api_gateway upstreams with new meshGateway config.
+		//
+		// Now, this serviceInstance.proxyCfg and NodeService.Proxy
+		// refers to same proxy configuration, which is of type ConnectProxyConfig.
+
+		// So, we need to test it as well.
+		// ---------------------------------------
+
+		Proxy:          TestConnectProxyConfig(t),
+		EnterpriseMeta: *entMeta,
 	}
 }