[OSS] Improve xDS Code Coverage - Endpoints and Misc

agent/proxycfg/testing.go

@@ -167,7 +167,7 @@ func TestUpstreamNodes(t testing.T, service string) structs.CheckServiceNodes {
 				Datacenter: "dc1",
 				Partition:  structs.NodeEnterpriseMetaInDefaultPartition().PartitionOrEmpty(),
 			},
-			Service: structs.TestNodeServiceWithName(t, service),
+			Service: structs.TestNodeServiceWithName(service),
 		},
 		structs.CheckServiceNode{
 			Node: &structs.Node{
@@ -177,7 +177,47 @@ func TestUpstreamNodes(t testing.T, service string) structs.CheckServiceNodes {
 				Datacenter: "dc1",
 				Partition:  structs.NodeEnterpriseMetaInDefaultPartition().PartitionOrEmpty(),
 			},
-			Service: structs.TestNodeServiceWithName(t, service),
+			Service: structs.TestNodeServiceWithName(service),
+		},
+	}
+}
+
+// TestUpstreamNodesWithServiceSubset returns a sample service discovery result with one instance tagged v1
+// and the other tagged v2
+func TestUpstreamNodesWithServiceSubset(t testing.T, service string) structs.CheckServiceNodes {
+	return structs.CheckServiceNodes{
+		structs.CheckServiceNode{
+			Node: &structs.Node{
+				ID:         "test1",
+				Node:       "test1",
+				Address:    "10.10.1.3",
+				Datacenter: "dc1",
+				Partition:  structs.NodeEnterpriseMetaInDefaultPartition().PartitionOrEmpty(),
+			},
+			Service: &structs.NodeService{
+				Kind:    structs.ServiceKindTypical,
+				Service: service,
+				Port:    8080,
+				Meta:    map[string]string{"Version": "1"},
+				Weights: &structs.Weights{
+					Passing: 300, // Check that this gets normalized to 128
+				},
+			},
+		},
+		structs.CheckServiceNode{
+			Node: &structs.Node{
+				ID:         "test2",
+				Node:       "test2",
+				Address:    "10.10.1.4",
+				Datacenter: "dc1",
+				Partition:  structs.NodeEnterpriseMetaInDefaultPartition().PartitionOrEmpty(),
+			},
+			Service: &structs.NodeService{
+				Kind:    structs.ServiceKindTypical,
+				Service: service,
+				Port:    8080,
+				Meta:    map[string]string{"Version": "2"},
+			},
 		},
 	}
 }
@@ -231,7 +271,7 @@ func TestUpstreamNodesInStatus(t testing.T, status string) structs.CheckServiceN
 				Address:    "10.10.1.1",
 				Datacenter: "dc1",
 			},
-			Service: structs.TestNodeService(t),
+			Service: structs.TestNodeService(),
 			Checks: structs.HealthChecks{
 				&structs.HealthCheck{
 					Node:        "test1",
@@ -248,7 +288,7 @@ func TestUpstreamNodesInStatus(t testing.T, status string) structs.CheckServiceN
 				Address:    "10.10.1.2",
 				Datacenter: "dc1",
 			},
-			Service: structs.TestNodeService(t),
+			Service: structs.TestNodeService(),
 			Checks: structs.HealthChecks{
 				&structs.HealthCheck{
 					Node:        "test2",
@@ -270,7 +310,7 @@ func TestUpstreamNodesDC2(t testing.T) structs.CheckServiceNodes {
 				Address:    "10.20.1.1",
 				Datacenter: "dc2",
 			},
-			Service: structs.TestNodeService(t),
+			Service: structs.TestNodeService(),
 		},
 		structs.CheckServiceNode{
 			Node: &structs.Node{
@@ -279,7 +319,7 @@ func TestUpstreamNodesDC2(t testing.T) structs.CheckServiceNodes {
 				Address:    "10.20.1.2",
 				Datacenter: "dc2",
 			},
-			Service: structs.TestNodeService(t),
+			Service: structs.TestNodeService(),
 		},
 	}
 }
@@ -293,7 +333,7 @@ func TestUpstreamNodesInStatusDC2(t testing.T, status string) structs.CheckServi
 				Address:    "10.20.1.1",
 				Datacenter: "dc2",
 			},
-			Service: structs.TestNodeService(t),
+			Service: structs.TestNodeService(),
 			Checks: structs.HealthChecks{
 				&structs.HealthCheck{
 					Node:        "test1",
@@ -310,7 +350,7 @@ func TestUpstreamNodesInStatusDC2(t testing.T, status string) structs.CheckServi
 				Address:    "10.20.1.2",
 				Datacenter: "dc2",
 			},
-			Service: structs.TestNodeService(t),
+			Service: structs.TestNodeService(),
 			Checks: structs.HealthChecks{
 				&structs.HealthCheck{
 					Node:        "test2",
@@ -332,7 +372,7 @@ func TestUpstreamNodesAlternate(t testing.T) structs.CheckServiceNodes {
 				Address:    "10.20.1.1",
 				Datacenter: "dc1",
 			},
-			Service: structs.TestNodeService(t),
+			Service: structs.TestNodeService(),
 		},
 		structs.CheckServiceNode{
 			Node: &structs.Node{
@@ -341,7 +381,7 @@ func TestUpstreamNodesAlternate(t testing.T) structs.CheckServiceNodes {
 				Address:    "10.20.1.2",
 				Datacenter: "dc1",
 			},
-			Service: structs.TestNodeService(t),
+			Service: structs.TestNodeService(),
 		},
 	}
 }

agent/proxycfg/testing_mesh_gateway.go

@@ -766,8 +766,12 @@ func TestConfigSnapshotPeeredMeshGateway(t testing.T, variant string, nsFn func(
 				Kind: structs.ServiceResolver,
 				Name: "api",
 				Subsets: map[string]structs.ServiceResolverSubset{
+					"v1": {
+						Filter: "Service.Meta.Version == 1",
+					},
 					"v2": {
-						Filter: "Service.Meta.version == v2",
+						Filter:      "Service.Meta.Version == 2",
+						OnlyPassing: true,
 					},
 				},
 			},
@@ -817,6 +821,7 @@ func TestConfigSnapshotPeeredMeshGateway(t testing.T, variant string, nsFn func(
 		var (
 			dbSN  = structs.NewServiceName("db", nil)
 			altSN = structs.NewServiceName("alt", nil)
+			apiSN = structs.NewServiceName("api", nil)
 
 			dbChain = discoverychain.TestCompileConfigEntries(t, "db", "default", "default", "dc1", connect.TestClusterID+".consul", nil, set)
 		)
@@ -826,6 +831,7 @@ func TestConfigSnapshotPeeredMeshGateway(t testing.T, variant string, nsFn func(
 		discoChains[dbSN] = dbChain
 		endpoints[dbSN] = TestUpstreamNodes(t, "db")
 		endpoints[altSN] = TestUpstreamNodes(t, "alt")
+		endpoints[apiSN] = TestUpstreamNodesWithServiceSubset(t, "api")
 
 		extraUpdates = append(extraUpdates,
 			UpdateEvent{
@@ -849,7 +855,29 @@ func TestConfigSnapshotPeeredMeshGateway(t testing.T, variant string, nsFn func(
 					},
 				},
 			},
+			UpdateEvent{
+				CorrelationID: serviceResolversWatchID,
+				Result: &structs.IndexedConfigEntries{
+					Kind: structs.ServiceResolver,
+					Entries: []structs.ConfigEntry{
+						&structs.ServiceResolverConfigEntry{
+							Kind: structs.ServiceResolver,
+							Name: "api",
+							Subsets: map[string]structs.ServiceResolverSubset{
+								"v1": {
+									Filter: "Service.Meta.Version == 1",
+								},
+								"v2": {
+									Filter:      "Service.Meta.Version == 2",
+									OnlyPassing: true,
+								},
+							},
+						},
+					},
+				},
+			},
 		)
+
 	case "peer-through-mesh-gateway":
 
 		extraUpdates = append(extraUpdates,

agent/structs/structs_test.go

@@ -1358,7 +1358,7 @@ func TestStructs_NodeService_ValidateSidecarService(t *testing.T) {
 }
 
 func TestStructs_NodeService_ConnectNativeEmptyPortError(t *testing.T) {
-	ns := TestNodeService(t)
+	ns := TestNodeService()
 	ns.Connect.Native = true
 	ns.Port = 0
 	err := ns.Validate()

agent/structs/testing_catalog.go

@@ -6,8 +6,9 @@ package structs
 import (
 	"fmt"
 
-	"github.com/hashicorp/consul/acl"
 	"github.com/mitchellh/go-testing-interface"
+
+	"github.com/hashicorp/consul/acl"
 )
 
 // TestRegisterRequest returns a RegisterRequest for registering a typical service.
@@ -47,11 +48,11 @@ func TestRegisterIngressGateway(t testing.T) *RegisterRequest {
 }
 
 // TestNodeService returns a *NodeService representing a valid regular service: "web".
-func TestNodeService(t testing.T) *NodeService {
-	return TestNodeServiceWithName(t, "web")
+func TestNodeService() *NodeService {
+	return TestNodeServiceWithName("web")
 }
 
-func TestNodeServiceWithName(t testing.T, name string) *NodeService {
+func TestNodeServiceWithName(name string) *NodeService {
 	return &NodeService{
 		Kind:    ServiceKindTypical,
 		Service: name,

agent/xds/clusters_test.go

@@ -36,6 +36,10 @@ func uint32ptr(i uint32) *uint32 {
 	return &i
 }
 
+func durationPtr(d time.Duration) *time.Duration {
+	return &d
+}
+
 func makeClusterDiscoChainTests(enterprise bool) []clusterTestCase {
 	return []clusterTestCase{
 		{
@@ -384,6 +388,20 @@ func TestClustersFromSnapshot(t *testing.T) {
 				}, nil)
 			},
 		},
+		{
+			name: "custom-passive-healthcheck-zero-consecutive_5xx",
+			create: func(t testinf.T) *proxycfg.ConfigSnapshot {
+				return proxycfg.TestConfigSnapshot(t, func(ns *structs.NodeService) {
+					ns.Proxy.Upstreams[0].Config["passive_health_check"] = map[string]interface{}{
+						"enforcing_consecutive_5xx": float64(0),
+						"max_failures":              float64(5),
+						"interval":                  float64(10 * time.Second),
+						"max_ejection_percent":      float64(100),
+						"base_ejection_time":        float64(10 * time.Second),
+					}
+				}, nil)
+			},
+		},
 		{
 			name: "custom-max-inbound-connections",
 			create: func(t testinf.T) *proxycfg.ConfigSnapshot {
@@ -737,6 +755,7 @@ func TestClustersFromSnapshot(t *testing.T) {
 							Interval:                8000000000,
 							EnforcingConsecutive5xx: &enforcingConsecutive5xx,
 							MaxEjectionPercent:      uint32ptr(90),
+							BaseEjectionTime:        durationPtr(12 * time.Second),
 						}
 					}, nil)
 			},

agent/xds/config.go

@@ -229,9 +229,9 @@ func ToOutlierDetection(p *structs.PassiveHealthCheck, override *structs.Passive
 		// NOTE: EnforcingConsecutive5xx must be great than 0 for ingress-gateway
 		if *override.EnforcingConsecutive5xx != 0 {
 			od.EnforcingConsecutive_5Xx = &wrapperspb.UInt32Value{Value: *override.EnforcingConsecutive5xx}
-		} else if allowZero {
-			od.EnforcingConsecutive_5Xx = &wrapperspb.UInt32Value{Value: *override.EnforcingConsecutive5xx}
 		}
+		// Because only ingress gateways have overrides and they cannot have a value of 0, there is no allowZero
+		// override case to handle
 	}
 
 	if override.MaxEjectionPercent != nil {

agent/xds/delta_test.go

@@ -1693,6 +1693,12 @@ func Test_applyEnvoyExtension_Validations(t *testing.T) {
 			runtimeConfig: makeRuntimeConfig(false, ">= 1.15.0", ">= 1.25.0", map[string]interface{}{"bad": "args"}),
 			err:           false,
 		},
+		{
+			name:          "valid everything - no resources and required",
+			runtimeConfig: makeRuntimeConfig(true, ">= 1.15.0", ">= 1.25.0", nil),
+			err:           true,
+			errString:     "failed to patch xDS resources in",
+		},
 		{
 			name:          "valid everything",
 			runtimeConfig: makeRuntimeConfig(false, ">= 1.15.0", ">= 1.25.0", nil),

agent/xds/endpoints.go

@@ -11,10 +11,11 @@ import (
 	envoy_cluster_v3 "github.com/envoyproxy/go-control-plane/envoy/config/cluster/v3"
 	envoy_core_v3 "github.com/envoyproxy/go-control-plane/envoy/config/core/v3"
 	envoy_endpoint_v3 "github.com/envoyproxy/go-control-plane/envoy/config/endpoint/v3"
-	"github.com/hashicorp/consul/envoyextensions/xdscommon"
 	"github.com/hashicorp/go-bexpr"
 	"google.golang.org/protobuf/proto"
 
+	"github.com/hashicorp/consul/envoyextensions/xdscommon"
+
 	"github.com/hashicorp/consul/agent/connect"
 	"github.com/hashicorp/consul/agent/proxycfg"
 	"github.com/hashicorp/consul/agent/structs"

agent/xds/endpoints_test.go

@@ -10,6 +10,7 @@ import (
 
 	envoy_core_v3 "github.com/envoyproxy/go-control-plane/envoy/config/core/v3"
 	envoy_endpoint_v3 "github.com/envoyproxy/go-control-plane/envoy/config/endpoint/v3"
+
 	"github.com/hashicorp/consul/agent/xds/testcommon"
 
 	"github.com/mitchellh/copystructure"
@@ -361,6 +362,12 @@ func TestEndpointsFromSnapshot(t *testing.T) {
 				return proxycfg.TestConfigSnapshotMeshGateway(t, "newer-info-in-federation-states", nil, nil)
 			},
 		},
+		{
+			name: "mesh-gateway-using-federation-control-plane",
+			create: func(t testinf.T) *proxycfg.ConfigSnapshot {
+				return proxycfg.TestConfigSnapshotMeshGateway(t, "mesh-gateway-federation", nil, nil)
+			},
+		},
 		{
 			name: "mesh-gateway-older-information-in-federation-states",
 			create: func(t testinf.T) *proxycfg.ConfigSnapshot {

agent/xds/rbac.go

@@ -833,6 +833,9 @@ func optimizePrincipals(orig []*envoy_rbac_v3.Principal) []*envoy_rbac_v3.Princi
 		if !ok {
 			return orig
 		}
+		// In practice, you can't hit this
+		// Only JWTs (HTTP-only) generate orPrinciples, but optimizePrinciples is only called
+		// against the combined list of principles for L4 intentions.
 		orIds = append(orIds, or.OrIds.Ids...)
 	}