Merge branch 'release/1.14.x' into release/1.14.5

.changelog/16495.txt

@@ -0,0 +1,3 @@
+```release-note:improvement
+mesh: Add ServiceResolver RequestTimeout for route timeouts to make request timeouts configurable
+```

.changelog/16498.txt

@@ -0,0 +1,3 @@
+```release-note:bug
+proxycfg: fix a bug where terminating gateways were not cleaning up deleted service resolvers for their referenced services
+```

.changelog/16499.txt

@@ -0,0 +1,3 @@
+```release-note:bug
+mesh: Fix resolution of service resolvers with subsets for external upstreams
+```

agent/consul/discoverychain/compile.go

@@ -963,6 +963,7 @@ RESOLVE_AGAIN:
 			Default:        resolver.IsDefault(),
 			Target:         target.ID,
 			ConnectTimeout: connectTimeout,
+			RequestTimeout: resolver.RequestTimeout,
 		},
 		LoadBalancer: resolver.LoadBalancer,
 	}

agent/proxycfg/state_test.go

@@ -2093,6 +2093,28 @@ func TestState_WatchesAndUpdates(t *testing.T) {
 						require.Equal(t, dbResolver.Entry, snap.TerminatingGateway.ServiceResolvers[db])
 					},
 				},
+				{
+					requiredWatches: map[string]verifyWatchRequest{
+						"service-resolver:" + db.String(): genVerifyResolverWatch("db", "dc1", structs.ServiceResolver),
+					},
+					events: []UpdateEvent{
+						{
+							CorrelationID: "service-resolver:" + db.String(),
+							Result: &structs.ConfigEntryResponse{
+								Entry: nil,
+							},
+							Err: nil,
+						},
+					},
+					verifySnapshot: func(t testing.TB, snap *ConfigSnapshot) {
+						require.True(t, snap.Valid(), "gateway with service list is valid")
+						// Finally ensure we cleaned up the resolver
+						require.Equal(t, []structs.ServiceName{db}, snap.TerminatingGateway.ValidServices())
+
+						require.False(t, snap.TerminatingGateway.ServiceResolversSet[db])
+						require.Nil(t, snap.TerminatingGateway.ServiceResolvers[db])
+					},
+				},
 				{
 					events: []UpdateEvent{
 						{

agent/proxycfg/terminating_gateway.go

@@ -354,8 +354,13 @@ func (s *handlerTerminatingGateway) handleUpdate(ctx context.Context, u UpdateEv
 		// There should only ever be one entry for a service resolver within a namespace
 		if resolver, ok := resp.Entry.(*structs.ServiceResolverConfigEntry); ok {
 			snap.TerminatingGateway.ServiceResolvers[sn] = resolver
+			snap.TerminatingGateway.ServiceResolversSet[sn] = true
+		} else {
+			// we likely have a deleted service resolver, and our cast is a nil
+			// cast, so clear this out
+			delete(snap.TerminatingGateway.ServiceResolvers, sn)
+			snap.TerminatingGateway.ServiceResolversSet[sn] = false
 		}
-		snap.TerminatingGateway.ServiceResolversSet[sn] = true
 
 	case strings.HasPrefix(u.CorrelationID, serviceIntentionsIDPrefix):
 		resp, ok := u.Result.(structs.Intentions)

agent/proxycfg/testing_ingress_gateway.go

@@ -659,11 +659,13 @@ func TestConfigSnapshotIngress_HTTPMultipleServices(t testing.T) *ConfigSnapshot
 			Kind:           structs.ServiceResolver,
 			Name:           "foo",
 			ConnectTimeout: 22 * time.Second,
+			RequestTimeout: 22 * time.Second,
 		},
 		&structs.ServiceResolverConfigEntry{
 			Kind:           structs.ServiceResolver,
 			Name:           "bar",
 			ConnectTimeout: 22 * time.Second,
+			RequestTimeout: 22 * time.Second,
 		},
 	}
 
@@ -814,11 +816,13 @@ func TestConfigSnapshotIngress_GRPCMultipleServices(t testing.T) *ConfigSnapshot
 			Kind:           structs.ServiceResolver,
 			Name:           "foo",
 			ConnectTimeout: 22 * time.Second,
+			RequestTimeout: 22 * time.Second,
 		},
 		&structs.ServiceResolverConfigEntry{
 			Kind:           structs.ServiceResolver,
 			Name:           "bar",
 			ConnectTimeout: 22 * time.Second,
+			RequestTimeout: 22 * time.Second,
 		},
 	}
 
@@ -1172,12 +1176,14 @@ func TestConfigSnapshotIngressGatewayWithChain(
 				Name:           "web",
 				EnterpriseMeta: *webEntMeta,
 				ConnectTimeout: 22 * time.Second,
+				RequestTimeout: 22 * time.Second,
 			},
 			&structs.ServiceResolverConfigEntry{
 				Kind:           structs.ServiceResolver,
 				Name:           "foo",
 				EnterpriseMeta: *fooEntMeta,
 				ConnectTimeout: 22 * time.Second,
+				RequestTimeout: 22 * time.Second,
 			},
 		}
 

agent/proxycfg/testing_mesh_gateway.go

@@ -182,6 +182,7 @@ func TestConfigSnapshotMeshGateway(t testing.T, variant string, nsFn func(ns *st
 						Kind:           structs.ServiceResolver,
 						Name:           "bar",
 						ConnectTimeout: 10 * time.Second,
+						RequestTimeout: 10 * time.Second,
 						Subsets: map[string]structs.ServiceResolverSubset{
 							"v1": {
 								Filter: "Service.Meta.Version == 1",
@@ -687,6 +688,7 @@ func TestConfigSnapshotPeeredMeshGateway(t testing.T, variant string, nsFn func(
 				Kind:           structs.ServiceResolver,
 				Name:           "db",
 				ConnectTimeout: 33 * time.Second,
+				RequestTimeout: 33 * time.Second,
 			},
 			&structs.ServiceResolverConfigEntry{
 				Kind: structs.ServiceResolver,

agent/proxycfg/testing_upstreams.go

@@ -242,6 +242,7 @@ func setupTestVariationDiscoveryChain(
 				Kind:           structs.ServiceResolver,
 				Name:           "db",
 				ConnectTimeout: 33 * time.Second,
+				RequestTimeout: 33 * time.Second,
 			},
 		)
 	case "external-sni":
@@ -255,6 +256,7 @@ func setupTestVariationDiscoveryChain(
 				Kind:           structs.ServiceResolver,
 				Name:           "db",
 				ConnectTimeout: 33 * time.Second,
+				RequestTimeout: 33 * time.Second,
 			},
 		)
 	case "failover":
@@ -263,6 +265,7 @@ func setupTestVariationDiscoveryChain(
 				Kind:           structs.ServiceResolver,
 				Name:           "db",
 				ConnectTimeout: 33 * time.Second,
+				RequestTimeout: 33 * time.Second,
 				Failover: map[string]structs.ServiceResolverFailover{
 					"*": {
 						Service: "fail",
@@ -285,6 +288,7 @@ func setupTestVariationDiscoveryChain(
 				Kind:           structs.ServiceResolver,
 				Name:           "db",
 				ConnectTimeout: 33 * time.Second,
+				RequestTimeout: 33 * time.Second,
 				Failover: map[string]structs.ServiceResolverFailover{
 					"*": {
 						Datacenters: []string{"dc2"},
@@ -298,6 +302,7 @@ func setupTestVariationDiscoveryChain(
 				Kind:           structs.ServiceResolver,
 				Name:           "db",
 				ConnectTimeout: 33 * time.Second,
+				RequestTimeout: 33 * time.Second,
 				Failover: map[string]structs.ServiceResolverFailover{
 					"*": {
 						Targets: []structs.ServiceResolverFailoverTarget{
@@ -313,6 +318,7 @@ func setupTestVariationDiscoveryChain(
 				Kind:           structs.ServiceResolver,
 				Name:           "db",
 				ConnectTimeout: 33 * time.Second,
+				RequestTimeout: 33 * time.Second,
 				Redirect: &structs.ServiceResolverRedirect{
 					Peer: "cluster-01",
 				},
@@ -333,6 +339,7 @@ func setupTestVariationDiscoveryChain(
 				Kind:           structs.ServiceResolver,
 				Name:           "db",
 				ConnectTimeout: 33 * time.Second,
+				RequestTimeout: 33 * time.Second,
 				Failover: map[string]structs.ServiceResolverFailover{
 					"*": {
 						Datacenters: []string{"dc2", "dc3"},
@@ -355,6 +362,7 @@ func setupTestVariationDiscoveryChain(
 				Kind:           structs.ServiceResolver,
 				Name:           "db",
 				ConnectTimeout: 33 * time.Second,
+				RequestTimeout: 33 * time.Second,
 				Failover: map[string]structs.ServiceResolverFailover{
 					"*": {
 						Datacenters: []string{"dc2"},
@@ -377,6 +385,7 @@ func setupTestVariationDiscoveryChain(
 				Kind:           structs.ServiceResolver,
 				Name:           "db",
 				ConnectTimeout: 33 * time.Second,
+				RequestTimeout: 33 * time.Second,
 				Failover: map[string]structs.ServiceResolverFailover{
 					"*": {
 						Datacenters: []string{"dc2", "dc3"},
@@ -438,6 +447,7 @@ func setupTestVariationDiscoveryChain(
 				Kind:           structs.ServiceResolver,
 				Name:           "db",
 				ConnectTimeout: 33 * time.Second,
+				RequestTimeout: 33 * time.Second,
 			},
 			&structs.ProxyConfigEntry{
 				Kind: structs.ProxyDefaults,
@@ -489,6 +499,7 @@ func setupTestVariationDiscoveryChain(
 				Kind:           structs.ServiceResolver,
 				Name:           "db",
 				ConnectTimeout: 33 * time.Second,
+				RequestTimeout: 33 * time.Second,
 			},
 			&structs.ProxyConfigEntry{
 				Kind: structs.ProxyDefaults,
@@ -520,6 +531,7 @@ func setupTestVariationDiscoveryChain(
 				Kind:           structs.ServiceResolver,
 				Name:           "db",
 				ConnectTimeout: 33 * time.Second,
+				RequestTimeout: 33 * time.Second,
 			},
 			&structs.ProxyConfigEntry{
 				Kind: structs.ProxyDefaults,

agent/rpcclient/health/view.go

@@ -50,8 +50,10 @@ func NewHealthView(req structs.ServiceSpecificRequest) (*HealthView, error) {
 		return nil, err
 	}
 	return &HealthView{
-		state:  make(map[string]structs.CheckServiceNode),
-		filter: fe,
+		state:   make(map[string]structs.CheckServiceNode),
+		filter:  fe,
+		connect: req.Connect,
+		kind:    req.ServiceKind,
 	}, nil
 }
 
@@ -61,8 +63,10 @@ func NewHealthView(req structs.ServiceSpecificRequest) (*HealthView, error) {
 // (IndexedCheckServiceNodes) and update it in place for each event - that
 // involves re-sorting each time etc. though.
 type HealthView struct {
-	state  map[string]structs.CheckServiceNode
-	filter filterEvaluator
+	connect bool
+	kind    structs.ServiceKind
+	state   map[string]structs.CheckServiceNode
+	filter  filterEvaluator
 }
 
 // Update implements View
@@ -84,6 +88,13 @@ func (s *HealthView) Update(events []*pbsubscribe.Event) error {
 			if csn == nil {
 				return errors.New("check service node was unexpectedly nil")
 			}
+
+			// check if we intentionally need to skip the filter
+			if s.skipFilter(csn) {
+				s.state[id] = *csn
+				continue
+			}
+
 			passed, err := s.filter.Evaluate(*csn)
 			if err != nil {
 				return err
@@ -100,6 +111,11 @@ func (s *HealthView) Update(events []*pbsubscribe.Event) error {
 	return nil
 }
 
+func (s *HealthView) skipFilter(csn *structs.CheckServiceNode) bool {
+	// we only do this for connect-enabled services that need to be routed through a terminating gateway
+	return s.kind == "" && s.connect && csn.Service.Kind == structs.ServiceKindTerminatingGateway
+}
+
 type filterEvaluator interface {
 	Evaluate(datum interface{}) (bool, error)
 }

agent/rpcclient/health/view_test.go

@@ -941,3 +941,39 @@ func TestNewFilterEvaluator(t *testing.T) {
 		})
 	}
 }
+
+func TestHealthView_SkipFilteringTerminatingGateways(t *testing.T) {
+	view, err := NewHealthView(structs.ServiceSpecificRequest{
+		ServiceName: "name",
+		Connect:     true,
+		QueryOptions: structs.QueryOptions{
+			Filter: "Service.Meta.version == \"v1\"",
+		},
+	})
+	require.NoError(t, err)
+
+	err = view.Update([]*pbsubscribe.Event{{
+		Index: 1,
+		Payload: &pbsubscribe.Event_ServiceHealth{
+			ServiceHealth: &pbsubscribe.ServiceHealthUpdate{
+				Op: pbsubscribe.CatalogOp_Register,
+				CheckServiceNode: &pbservice.CheckServiceNode{
+					Service: &pbservice.NodeService{
+						Kind:    structs.TerminatingGateway,
+						Service: "name",
+						Address: "127.0.0.1",
+						Port:    8443,
+					},
+				},
+			},
+		},
+	}})
+	require.NoError(t, err)
+
+	node, ok := (view.Result(1)).(*structs.IndexedCheckServiceNodes)
+	require.True(t, ok)
+
+	require.Len(t, node.Nodes, 1)
+	require.Equal(t, "127.0.0.1", node.Nodes[0].Service.Address)
+	require.Equal(t, 8443, node.Nodes[0].Service.Port)
+}

agent/structs/config_entry_discoverychain.go

@@ -857,6 +857,11 @@ type ServiceResolverConfigEntry struct {
 	// to this service.
 	ConnectTimeout time.Duration `json:",omitempty" alias:"connect_timeout"`
 
+	// RequestTimeout is the timeout for an HTTP request to complete before
+	// the connection is automatically terminated. If unspecified, defaults
+	// to 15 seconds.
+	RequestTimeout time.Duration `json:",omitempty" alias:"request_timeout"`
+
 	// LoadBalancer determines the load balancing policy and configuration for services
 	// issuing requests to this upstream service.
 	LoadBalancer *LoadBalancer `json:",omitempty" alias:"load_balancer"`
@@ -870,14 +875,19 @@ func (e *ServiceResolverConfigEntry) MarshalJSON() ([]byte, error) {
 	type Alias ServiceResolverConfigEntry
 	exported := &struct {
 		ConnectTimeout string `json:",omitempty"`
+		RequestTimeout string `json:",omitempty"`
 		*Alias
 	}{
 		ConnectTimeout: e.ConnectTimeout.String(),
+		RequestTimeout: e.RequestTimeout.String(),
 		Alias:          (*Alias)(e),
 	}
 	if e.ConnectTimeout == 0 {
 		exported.ConnectTimeout = ""
 	}
+	if e.RequestTimeout == 0 {
+		exported.RequestTimeout = ""
+	}
 
 	return json.Marshal(exported)
 }
@@ -886,6 +896,7 @@ func (e *ServiceResolverConfigEntry) UnmarshalJSON(data []byte) error {
 	type Alias ServiceResolverConfigEntry
 	aux := &struct {
 		ConnectTimeout string
+		RequestTimeout string
 		*Alias
 	}{
 		Alias: (*Alias)(e),
@@ -899,6 +910,11 @@ func (e *ServiceResolverConfigEntry) UnmarshalJSON(data []byte) error {
 			return err
 		}
 	}
+	if aux.RequestTimeout != "" {
+		if e.RequestTimeout, err = time.ParseDuration(aux.RequestTimeout); err != nil {
+			return err
+		}
+	}
 	return nil
 }
 
@@ -919,6 +935,7 @@ func (e *ServiceResolverConfigEntry) IsDefault() bool {
 		e.Redirect == nil &&
 		len(e.Failover) == 0 &&
 		e.ConnectTimeout == 0 &&
+		e.RequestTimeout == 0 &&
 		e.LoadBalancer == nil
 }
 
@@ -1117,6 +1134,10 @@ func (e *ServiceResolverConfigEntry) Validate() error {
 		return fmt.Errorf("Bad ConnectTimeout '%s', must be >= 0", e.ConnectTimeout)
 	}
 
+	if e.RequestTimeout < 0 {
+		return fmt.Errorf("Bad RequestTimeout '%s', must be >= 0", e.RequestTimeout)
+	}
+
 	if e.LoadBalancer != nil {
 		lb := e.LoadBalancer
 

agent/structs/discovery_chain.go

@@ -113,6 +113,7 @@ func (s *DiscoveryGraphNode) MapKey() string {
 type DiscoveryResolver struct {
 	Default        bool               `json:",omitempty"`
 	ConnectTimeout time.Duration      `json:",omitempty"`
+	RequestTimeout time.Duration      `json:",omitempty"`
 	Target         string             `json:",omitempty"`
 	Failover       *DiscoveryFailover `json:",omitempty"`
 }