Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[OSS] Improve xDS Code Coverage - Clusters #18165

Merged
merged 1 commit into from
Jul 20, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
67 changes: 60 additions & 7 deletions agent/proxycfg/testing_peering.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,18 @@ import (
)

func TestConfigSnapshotPeering(t testing.T) *ConfigSnapshot {
return testConfigSnapshot(t, false)
return testConfigSnapshot(t, false, false)
}

func TestConfigSnapshotPeeringWithListenerOverride(t testing.T) *ConfigSnapshot {
return testConfigSnapshot(t, true)
func TestConfigSnapshotPeeringWithEscapeOverrides(t testing.T) *ConfigSnapshot {
return testConfigSnapshot(t, true, false)
}

func testConfigSnapshot(t testing.T, listenerOverride bool) *ConfigSnapshot {
func TestConfigSnapshotPeeringWithHTTP2(t testing.T) *ConfigSnapshot {
return testConfigSnapshot(t, false, true)
}

func testConfigSnapshot(t testing.T, escapeOverride bool, useHTTP2 bool) *ConfigSnapshot {
var (
paymentsUpstream = structs.Upstream{
DestinationName: "payments",
Expand All @@ -39,6 +43,11 @@ func testConfigSnapshot(t testing.T, listenerOverride bool) *ConfigSnapshot {
refundsUID = NewUpstreamID(&refundsUpstream)
)

protocol := "tcp"
if useHTTP2 {
protocol = "http2"
}

const peerTrustDomain = "1c053652-8512-4373-90cf-5a7f6263a994.consul"

return TestConfigSnapshot(t, func(ns *structs.NodeService) {
Expand All @@ -47,7 +56,7 @@ func testConfigSnapshot(t testing.T, listenerOverride bool) *ConfigSnapshot {
refundsUpstream,
}

if listenerOverride {
if escapeOverride {
if ns.Proxy.Upstreams[0].Config == nil {
ns.Proxy.Upstreams[0].Config = map[string]interface{}{}
}
Expand All @@ -58,6 +67,10 @@ func testConfigSnapshot(t testing.T, listenerOverride bool) *ConfigSnapshot {
customListenerJSON(t, customListenerJSONOptions{
Name: uid.EnvoyID() + ":custom-upstream",
})
ns.Proxy.Upstreams[0].Config["envoy_cluster_json"] =
customClusterJSON(t, customClusterJSONOptions{
Name: uid.EnvoyID() + ":custom-upstream",
})
}

}, []UpdateEvent{
Expand Down Expand Up @@ -98,7 +111,7 @@ func testConfigSnapshot(t testing.T, listenerOverride bool) *ConfigSnapshot {
SpiffeID: []string{
"spiffe://" + peerTrustDomain + "/ns/default/dc/cloud-dc/svc/payments",
},
Protocol: "tcp",
Protocol: protocol,
},
},
},
Expand Down Expand Up @@ -127,7 +140,7 @@ func testConfigSnapshot(t testing.T, listenerOverride bool) *ConfigSnapshot {
SpiffeID: []string{
"spiffe://" + peerTrustDomain + "/ns/default/dc/cloud-dc/svc/refunds",
},
Protocol: "tcp",
Protocol: protocol,
},
},
},
Expand Down Expand Up @@ -456,3 +469,43 @@ const customListenerJSONTpl = `{
}
]
}`

type customClusterJSONOptions struct {
Name string
TLSContext string
}

var customClusterJSONTpl = `{
"@type": "type.googleapis.com/envoy.config.cluster.v3.Cluster",
"name": "{{ .Name }}",
"connectTimeout": "15s",
"loadAssignment": {
"clusterName": "{{ .Name }}",
"endpoints": [
{
"lbEndpoints": [
{
"endpoint": {
"address": {
"socketAddress": {
"address": "1.2.3.4",
"portValue": 8443
}
}
}
}
]
}
]
}
}`

var customClusterJSONTemplate = template.Must(template.New("").Parse(customClusterJSONTpl))

func customClusterJSON(t testing.T, opts customClusterJSONOptions) string {
t.Helper()
var buf bytes.Buffer
err := customClusterJSONTemplate.Execute(&buf, opts)
require.NoError(t, err)
return buf.String()
}
43 changes: 2 additions & 41 deletions agent/xds/clusters.go
Original file line number Diff line number Diff line change
Expand Up @@ -808,27 +808,6 @@ func (s *ResourceGenerator) makeGatewayOutgoingClusterPeeringServiceClusters(cfg
}
cluster := s.makeGatewayCluster(cfgSnap, opts)

if serviceGroup.UseCDS {
configureClusterWithHostnames(
s.Logger,
cluster,
"", /*TODO:make configurable?*/
serviceGroup.Nodes,
true, /*isRemote*/
false, /*onlyPassing*/
)
} else {
cluster.ClusterDiscoveryType = &envoy_cluster_v3.Cluster_Type{Type: envoy_cluster_v3.Cluster_EDS}
cluster.EdsClusterConfig = &envoy_cluster_v3.Cluster_EdsClusterConfig{
EdsConfig: &envoy_core_v3.ConfigSource{
ResourceApiVersion: envoy_core_v3.ApiVersion_V3,
ConfigSourceSpecifier: &envoy_core_v3.ConfigSource_Ads{
Ads: &envoy_core_v3.AggregatedConfigSource{},
},
},
}
}

clusters = append(clusters, cluster)
}
}
Expand Down Expand Up @@ -1062,11 +1041,6 @@ func (s *ResourceGenerator) configIngressUpstreamCluster(c *envoy_cluster_v3.Clu
}
outlierDetection := ToOutlierDetection(cfgSnap.IngressGateway.Defaults.PassiveHealthCheck, override, false)

// Specail handling for failover peering service, which has set MaxEjectionPercent
if c.OutlierDetection != nil && c.OutlierDetection.MaxEjectionPercent != nil {
outlierDetection.MaxEjectionPercent = &wrapperspb.UInt32Value{Value: c.OutlierDetection.MaxEjectionPercent.Value}
}

c.OutlierDetection = outlierDetection
}

Expand Down Expand Up @@ -1445,7 +1419,7 @@ func (s *ResourceGenerator) makeUpstreamClustersForDiscoveryChain(
// These variables are prefixed with primary to avoid shaddowing bugs.
primaryTargetID := node.Resolver.Target
primaryTarget := chain.Targets[primaryTargetID]
primaryTargetClusterName := s.getTargetClusterName(upstreamsSnapshot, chain, primaryTargetID, forMeshGateway, false)
primaryTargetClusterName := s.getTargetClusterName(upstreamsSnapshot, chain, primaryTargetID, forMeshGateway)
if primaryTargetClusterName == "" {
continue
}
Expand Down Expand Up @@ -1677,11 +1651,6 @@ func makeClusterFromUserConfig(configJSON string) (*envoy_cluster_v3.Cluster, er
return &c, err
}

type addressPair struct {
host string
port int
}

type clusterOpts struct {
// name for the cluster
name string
Expand Down Expand Up @@ -2054,12 +2023,7 @@ func generatePeeredClusterName(uid proxycfg.UpstreamID, tb *pbpeering.PeeringTru
}, ".")
}

type targetClusterData struct {
targetID string
clusterName string
}

func (s *ResourceGenerator) getTargetClusterName(upstreamsSnapshot *proxycfg.ConfigSnapshotUpstreams, chain *structs.CompiledDiscoveryChain, tid string, forMeshGateway bool, failover bool) string {
func (s *ResourceGenerator) getTargetClusterName(upstreamsSnapshot *proxycfg.ConfigSnapshotUpstreams, chain *structs.CompiledDiscoveryChain, tid string, forMeshGateway bool) string {
target := chain.Targets[tid]
clusterName := target.Name
targetUID := proxycfg.NewUpstreamIDFromTargetID(tid)
Expand All @@ -2078,9 +2042,6 @@ func (s *ResourceGenerator) getTargetClusterName(upstreamsSnapshot *proxycfg.Con
clusterName = generatePeeredClusterName(targetUID, tbs)
}
clusterName = CustomizeClusterName(clusterName, chain)
if failover {
clusterName = xdscommon.FailoverClusterNamePrefix + clusterName
}
if forMeshGateway {
clusterName = meshGatewayExportedClusterNamePrefix + clusterName
}
Expand Down
101 changes: 99 additions & 2 deletions agent/xds/clusters_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,10 @@ type clusterTestCase struct {
overrideGoldenName string
}

func uint32ptr(i uint32) *uint32 {
return &i
}

func makeClusterDiscoChainTests(enterprise bool) []clusterTestCase {
return []clusterTestCase{
{
Expand All @@ -51,6 +55,14 @@ func makeClusterDiscoChainTests(enterprise bool) []clusterTestCase {
return proxycfg.TestConfigSnapshotDiscoveryChain(t, "simple", enterprise, nil, nil)
},
},
{
name: "connect-proxy-with-chain-http2",
create: func(t testinf.T) *proxycfg.ConfigSnapshot {
return proxycfg.TestConfigSnapshotDiscoveryChain(t, "simple", enterprise, func(ns *structs.NodeService) {
ns.Proxy.Upstreams[0].Config["protocol"] = "http2"
}, nil)
},
},
{
name: "connect-proxy-with-chain-external-sni",
create: func(t testinf.T) *proxycfg.ConfigSnapshot {
Expand Down Expand Up @@ -313,6 +325,42 @@ func TestClustersFromSnapshot(t *testing.T) {
}, nil)
},
},
{
name: "custom-upstream-with-prepared-query",
create: func(t testinf.T) *proxycfg.ConfigSnapshot {
return proxycfg.TestConfigSnapshot(t, func(ns *structs.NodeService) {
for i := range ns.Proxy.Upstreams {

switch ns.Proxy.Upstreams[i].DestinationName {
case "db":
if ns.Proxy.Upstreams[i].Config == nil {
ns.Proxy.Upstreams[i].Config = map[string]interface{}{}
}

uid := proxycfg.NewUpstreamID(&ns.Proxy.Upstreams[i])

// Triggers an override with the presence of the escape hatch listener
ns.Proxy.Upstreams[i].DestinationType = structs.UpstreamDestTypePreparedQuery

ns.Proxy.Upstreams[i].Config["envoy_cluster_json"] =
customClusterJSON(t, customClusterJSONOptions{
Name: uid.EnvoyID() + ":custom-upstream",
})

// Also test that http2 options are triggered.
// A separate upstream without an override is required to test
case "geo-cache":
if ns.Proxy.Upstreams[i].Config == nil {
ns.Proxy.Upstreams[i].Config = map[string]interface{}{}
}
ns.Proxy.Upstreams[i].Config["protocol"] = "http2"
default:
continue
}
}
}, nil)
},
},
{
name: "custom-timeouts",
create: func(t testinf.T) *proxycfg.ConfigSnapshot {
Expand Down Expand Up @@ -431,6 +479,10 @@ func TestClustersFromSnapshot(t *testing.T) {
})
},
},
{
name: "expose-checks",
create: proxycfg.TestConfigSnapshotExposeChecks,
},
{
name: "expose-paths-grpc-new-cluster-http1",
create: proxycfg.TestConfigSnapshotGRPCExposeHTTP1,
Expand All @@ -447,6 +499,12 @@ func TestClustersFromSnapshot(t *testing.T) {
return proxycfg.TestConfigSnapshotMeshGateway(t, "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-no-services",
create: func(t testinf.T) *proxycfg.ConfigSnapshot {
Expand Down Expand Up @@ -628,8 +686,9 @@ func TestClustersFromSnapshot(t *testing.T) {
func(entry *structs.IngressGatewayConfigEntry) {
entry.Listeners[0].Services[0].MaxConnections = 4096
entry.Listeners[0].Services[0].PassiveHealthCheck = &structs.PassiveHealthCheck{
Interval: 5000000000,
MaxFailures: 10,
Interval: 5000000000,
MaxFailures: 10,
MaxEjectionPercent: uint32ptr(90),
}
}, nil)
},
Expand All @@ -649,6 +708,7 @@ func TestClustersFromSnapshot(t *testing.T) {
Interval: 5000000000,
MaxFailures: 10,
EnforcingConsecutive5xx: &enforcingConsecutive5xx,
MaxEjectionPercent: uint32ptr(90),
},
}
}, nil)
Expand All @@ -667,6 +727,7 @@ func TestClustersFromSnapshot(t *testing.T) {
PassiveHealthCheck: &structs.PassiveHealthCheck{
Interval: 5000000000,
EnforcingConsecutive5xx: &defaultEnforcingConsecutive5xx,
MaxEjectionPercent: uint32ptr(80),
},
}
enforcingConsecutive5xx := uint32(50)
Expand All @@ -675,6 +736,7 @@ func TestClustersFromSnapshot(t *testing.T) {
entry.Listeners[0].Services[0].PassiveHealthCheck = &structs.PassiveHealthCheck{
Interval: 8000000000,
EnforcingConsecutive5xx: &enforcingConsecutive5xx,
MaxEjectionPercent: uint32ptr(90),
}
}, nil)
},
Expand Down Expand Up @@ -934,6 +996,41 @@ func customAppClusterJSON(t testinf.T, opts customClusterJSONOptions) string {
return buf.String()
}

var customClusterJSONTpl = `{
"@type": "type.googleapis.com/envoy.config.cluster.v3.Cluster",
"name": "{{ .Name }}",
"connectTimeout": "15s",
"loadAssignment": {
"clusterName": "{{ .Name }}",
"endpoints": [
{
"lbEndpoints": [
{
"endpoint": {
"address": {
"socketAddress": {
"address": "1.2.3.4",
"portValue": 8443
}
}
}
}
]
}
]
}
}`

var customClusterJSONTemplate = template.Must(template.New("").Parse(customClusterJSONTpl))

func customClusterJSON(t testinf.T, opts customClusterJSONOptions) string {
t.Helper()
var buf bytes.Buffer
err := customClusterJSONTemplate.Execute(&buf, opts)
require.NoError(t, err)
return buf.String()
}

func TestEnvoyLBConfig_InjectToCluster(t *testing.T) {
var tests = []struct {
name string
Expand Down
2 changes: 1 addition & 1 deletion agent/xds/failover_policy.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ func (s *ResourceGenerator) mapDiscoChainTargets(cfgSnap *proxycfg.ConfigSnapsho
return discoChainTargets{}, err
}

failoverTargets.baseClusterName = s.getTargetClusterName(upstreamsSnapshot, chain, primaryTargetID, forMeshGateway, false)
failoverTargets.baseClusterName = s.getTargetClusterName(upstreamsSnapshot, chain, primaryTargetID, forMeshGateway)

tids := []string{primaryTargetID}
failover := node.Resolver.Failover
Expand Down
2 changes: 1 addition & 1 deletion agent/xds/listeners.go
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ func (s *ResourceGenerator) listenersFromSnapshotConnectProxy(cfgSnap *proxycfg.
return nil, err
}

clusterName = s.getTargetClusterName(upstreamsSnapshot, chain, target.ID, false, false)
clusterName = s.getTargetClusterName(upstreamsSnapshot, chain, target.ID, false)
if clusterName == "" {
continue
}
Expand Down
Loading