From b0916470901fc1ab9a06fb11f5f0ce34ec478ede Mon Sep 17 00:00:00 2001 From: "R.B. Boyer" Date: Thu, 17 Oct 2019 16:46:49 -0500 Subject: [PATCH] agent: allow mesh gateways to initialize even if there are no connect services registered yet (#6576) Fixes #6543 Also improved some of the proxycfg tests to cover snapshot validity better. --- agent/proxycfg/snapshot.go | 33 ++++++- agent/proxycfg/state.go | 2 + agent/proxycfg/state_test.go | 88 +++++++++++++++++++ agent/proxycfg/testing.go | 21 ++++- agent/xds/clusters_test.go | 5 ++ agent/xds/endpoints_test.go | 4 + agent/xds/listeners_test.go | 4 + .../clusters/mesh-gateway-no-services.golden | 7 ++ .../endpoints/mesh-gateway-no-services.golden | 7 ++ .../listeners/mesh-gateway-no-services.golden | 38 ++++++++ 10 files changed, 203 insertions(+), 6 deletions(-) create mode 100644 agent/xds/testdata/clusters/mesh-gateway-no-services.golden create mode 100644 agent/xds/testdata/endpoints/mesh-gateway-no-services.golden create mode 100644 agent/xds/testdata/listeners/mesh-gateway-no-services.golden diff --git a/agent/proxycfg/snapshot.go b/agent/proxycfg/snapshot.go index 47ee19785..bd8671183 100644 --- a/agent/proxycfg/snapshot.go +++ b/agent/proxycfg/snapshot.go @@ -14,19 +14,46 @@ type configSnapshotConnectProxy struct { WatchedUpstreamEndpoints map[string]map[string]structs.CheckServiceNodes WatchedGateways map[string]map[string]context.CancelFunc WatchedGatewayEndpoints map[string]map[string]structs.CheckServiceNodes - WatchedServiceChecks map[string][]structs.CheckType + WatchedServiceChecks map[string][]structs.CheckType // TODO: missing garbage collection UpstreamEndpoints map[string]structs.CheckServiceNodes // DEPRECATED:see:WatchedUpstreamEndpoints } +func (c *configSnapshotConnectProxy) IsEmpty() bool { + if c == nil { + return true + } + return c.Leaf == nil && + len(c.DiscoveryChain) == 0 && + len(c.WatchedUpstreams) == 0 && + len(c.WatchedUpstreamEndpoints) == 0 && + len(c.WatchedGateways) == 0 && + len(c.WatchedGatewayEndpoints) == 0 && + len(c.WatchedServiceChecks) == 0 && + len(c.UpstreamEndpoints) == 0 +} + type configSnapshotMeshGateway struct { WatchedServices map[string]context.CancelFunc + WatchedServicesSet bool WatchedDatacenters map[string]context.CancelFunc ServiceGroups map[string]structs.CheckServiceNodes ServiceResolvers map[string]*structs.ServiceResolverConfigEntry GatewayGroups map[string]structs.CheckServiceNodes } +func (c *configSnapshotMeshGateway) IsEmpty() bool { + if c == nil { + return true + } + return len(c.WatchedServices) == 0 && + !c.WatchedServicesSet && + len(c.WatchedDatacenters) == 0 && + len(c.ServiceGroups) == 0 && + len(c.ServiceResolvers) == 0 && + len(c.GatewayGroups) == 0 +} + // ConfigSnapshot captures all the resulting config needed for a proxy instance. // It is meant to be point-in-time coherent and is used to deliver the current // config state to observers who need it to be pushed in (e.g. XDS server). @@ -54,11 +81,9 @@ type ConfigSnapshot struct { func (s *ConfigSnapshot) Valid() bool { switch s.Kind { case structs.ServiceKindConnectProxy: - // TODO(rb): sanity check discovery chain things here? return s.Roots != nil && s.ConnectProxy.Leaf != nil case structs.ServiceKindMeshGateway: - // TODO (mesh-gateway) - what happens if all the connect services go away - return s.Roots != nil && len(s.MeshGateway.ServiceGroups) > 0 + return s.Roots != nil && (s.MeshGateway.WatchedServicesSet || len(s.MeshGateway.WatchedServices) > 0) default: return false } diff --git a/agent/proxycfg/state.go b/agent/proxycfg/state.go index a83677f94..8622d4ecc 100644 --- a/agent/proxycfg/state.go +++ b/agent/proxycfg/state.go @@ -726,6 +726,8 @@ func (s *state) handleUpdateMeshGateway(u cache.UpdateEvent, snap *ConfigSnapsho cancelFn() } } + + snap.MeshGateway.WatchedServicesSet = true case datacentersWatchID: datacentersRaw, ok := u.Result.(*[]string) if !ok { diff --git a/agent/proxycfg/state_test.go b/agent/proxycfg/state_test.go index 591e81ff9..eb1be81f9 100644 --- a/agent/proxycfg/state_test.go +++ b/agent/proxycfg/state_test.go @@ -290,6 +290,16 @@ func genVerifyServiceWatch(expectedService, expectedFilter, expectedDatacenter s func TestState_WatchesAndUpdates(t *testing.T) { t.Parallel() + indexedRoots, issuedCert := TestCerts(t) + + rootWatchEvent := func() cache.UpdateEvent { + return cache.UpdateEvent{ + CorrelationID: rootsWatchID, + Result: indexedRoots, + Err: nil, + } + } + type verificationStage struct { requiredWatches map[string]verifyWatchRequest events []cache.UpdateEvent @@ -417,6 +427,12 @@ func TestState_WatchesAndUpdates(t *testing.T) { }), }, events: []cache.UpdateEvent{ + rootWatchEvent(), + cache.UpdateEvent{ + CorrelationID: leafWatchID, + Result: issuedCert, + Err: nil, + }, cache.UpdateEvent{ CorrelationID: "discovery-chain:api", Result: &structs.DiscoveryChainResponse{ @@ -477,6 +493,21 @@ func TestState_WatchesAndUpdates(t *testing.T) { Err: nil, }, }, + verifySnapshot: func(t testing.TB, snap *ConfigSnapshot) { + require.True(t, snap.Valid()) + require.True(t, snap.MeshGateway.IsEmpty()) + require.Equal(t, indexedRoots, snap.Roots) + + require.Equal(t, issuedCert, snap.ConnectProxy.Leaf) + require.Len(t, snap.ConnectProxy.DiscoveryChain, 5, "%+v", snap.ConnectProxy.DiscoveryChain) + require.Len(t, snap.ConnectProxy.WatchedUpstreams, 5, "%+v", snap.ConnectProxy.WatchedUpstreams) + require.Len(t, snap.ConnectProxy.WatchedUpstreamEndpoints, 5, "%+v", snap.ConnectProxy.WatchedUpstreamEndpoints) + require.Len(t, snap.ConnectProxy.WatchedGateways, 5, "%+v", snap.ConnectProxy.WatchedGateways) + require.Len(t, snap.ConnectProxy.WatchedGatewayEndpoints, 5, "%+v", snap.ConnectProxy.WatchedGatewayEndpoints) + + require.Len(t, snap.ConnectProxy.WatchedServiceChecks, 0, "%+v", snap.ConnectProxy.WatchedServiceChecks) + require.Len(t, snap.ConnectProxy.UpstreamEndpoints, 0, "%+v", snap.ConnectProxy.UpstreamEndpoints) + }, } stage1 := verificationStage{ @@ -488,6 +519,21 @@ func TestState_WatchesAndUpdates(t *testing.T) { "mesh-gateway:dc2:api-failover-remote?dc=dc2": genVerifyGatewayWatch("dc2"), "mesh-gateway:dc1:api-failover-local?dc=dc2": genVerifyGatewayWatch("dc1"), }, + verifySnapshot: func(t testing.TB, snap *ConfigSnapshot) { + require.True(t, snap.Valid()) + require.True(t, snap.MeshGateway.IsEmpty()) + require.Equal(t, indexedRoots, snap.Roots) + + require.Equal(t, issuedCert, snap.ConnectProxy.Leaf) + require.Len(t, snap.ConnectProxy.DiscoveryChain, 5, "%+v", snap.ConnectProxy.DiscoveryChain) + require.Len(t, snap.ConnectProxy.WatchedUpstreams, 5, "%+v", snap.ConnectProxy.WatchedUpstreams) + require.Len(t, snap.ConnectProxy.WatchedUpstreamEndpoints, 5, "%+v", snap.ConnectProxy.WatchedUpstreamEndpoints) + require.Len(t, snap.ConnectProxy.WatchedGateways, 5, "%+v", snap.ConnectProxy.WatchedGateways) + require.Len(t, snap.ConnectProxy.WatchedGatewayEndpoints, 5, "%+v", snap.ConnectProxy.WatchedGatewayEndpoints) + + require.Len(t, snap.ConnectProxy.WatchedServiceChecks, 0, "%+v", snap.ConnectProxy.WatchedServiceChecks) + require.Len(t, snap.ConnectProxy.UpstreamEndpoints, 0, "%+v", snap.ConnectProxy.UpstreamEndpoints) + }, } if meshGatewayProxyConfigValue == structs.MeshGatewayModeLocal { @@ -518,6 +564,48 @@ func TestState_WatchesAndUpdates(t *testing.T) { serviceListWatchID: genVerifyListServicesWatch("dc1"), datacentersWatchID: verifyDatacentersWatch, }, + verifySnapshot: func(t testing.TB, snap *ConfigSnapshot) { + require.False(t, snap.Valid(), "gateway without root is not valid") + require.True(t, snap.ConnectProxy.IsEmpty()) + }, + }, + verificationStage{ + events: []cache.UpdateEvent{ + rootWatchEvent(), + }, + verifySnapshot: func(t testing.TB, snap *ConfigSnapshot) { + require.False(t, snap.Valid(), "gateway without services is valid") + require.True(t, snap.ConnectProxy.IsEmpty()) + require.Equal(t, indexedRoots, snap.Roots) + require.Empty(t, snap.MeshGateway.WatchedServices) + require.False(t, snap.MeshGateway.WatchedServicesSet) + require.Empty(t, snap.MeshGateway.WatchedDatacenters) + require.Empty(t, snap.MeshGateway.ServiceGroups) + require.Empty(t, snap.MeshGateway.ServiceResolvers) + require.Empty(t, snap.MeshGateway.GatewayGroups) + }, + }, + verificationStage{ + events: []cache.UpdateEvent{ + cache.UpdateEvent{ + CorrelationID: serviceListWatchID, + Result: &structs.IndexedServices{ + Services: make(structs.Services), + }, + Err: nil, + }, + }, + verifySnapshot: func(t testing.TB, snap *ConfigSnapshot) { + require.True(t, snap.Valid(), "gateway with empty service list is vaild") + require.True(t, snap.ConnectProxy.IsEmpty()) + require.Equal(t, indexedRoots, snap.Roots) + require.Empty(t, snap.MeshGateway.WatchedServices) + require.True(t, snap.MeshGateway.WatchedServicesSet) + require.Empty(t, snap.MeshGateway.WatchedDatacenters) + require.Empty(t, snap.MeshGateway.ServiceGroups) + require.Empty(t, snap.MeshGateway.ServiceResolvers) + require.Empty(t, snap.MeshGateway.GatewayGroups) + }, }, }, }, diff --git a/agent/proxycfg/testing.go b/agent/proxycfg/testing.go index a100e4d70..9643d12f1 100644 --- a/agent/proxycfg/testing.go +++ b/agent/proxycfg/testing.go @@ -967,8 +967,16 @@ func testConfigSnapshotDiscoveryChain(t testing.T, variation string, additionalE } func TestConfigSnapshotMeshGateway(t testing.T) *ConfigSnapshot { + return testConfigSnapshotMeshGateway(t, true) +} + +func TestConfigSnapshotMeshGatewayNoServices(t testing.T) *ConfigSnapshot { + return testConfigSnapshotMeshGateway(t, false) +} + +func testConfigSnapshotMeshGateway(t testing.T, populateServices bool) *ConfigSnapshot { roots, _ := TestCerts(t) - return &ConfigSnapshot{ + snap := &ConfigSnapshot{ Kind: structs.ServiceKindMeshGateway, Service: "mesh-gateway", ProxyID: "mesh-gateway", @@ -990,10 +998,17 @@ func TestConfigSnapshotMeshGateway(t testing.T) *ConfigSnapshot { Roots: roots, Datacenter: "dc1", MeshGateway: configSnapshotMeshGateway{ + WatchedServicesSet: true, + }, + } + + if populateServices { + snap.MeshGateway = configSnapshotMeshGateway{ WatchedServices: map[string]context.CancelFunc{ "foo": nil, "bar": nil, }, + WatchedServicesSet: true, WatchedDatacenters: map[string]context.CancelFunc{ "dc2": nil, }, @@ -1004,8 +1019,10 @@ func TestConfigSnapshotMeshGateway(t testing.T) *ConfigSnapshot { GatewayGroups: map[string]structs.CheckServiceNodes{ "dc2": TestGatewayNodesDC2(t), }, - }, + } } + + return snap } func TestConfigSnapshotExposeConfig(t testing.T) *ConfigSnapshot { diff --git a/agent/xds/clusters_test.go b/agent/xds/clusters_test.go index ad5328f4d..62f9a8037 100644 --- a/agent/xds/clusters_test.go +++ b/agent/xds/clusters_test.go @@ -197,6 +197,11 @@ func TestClustersFromSnapshot(t *testing.T) { create: proxycfg.TestConfigSnapshotMeshGateway, setup: nil, }, + { + name: "mesh-gateway-no-services", + create: proxycfg.TestConfigSnapshotMeshGatewayNoServices, + setup: nil, + }, { name: "mesh-gateway-service-subsets", create: proxycfg.TestConfigSnapshotMeshGateway, diff --git a/agent/xds/endpoints_test.go b/agent/xds/endpoints_test.go index 0dff69675..6238a9903 100644 --- a/agent/xds/endpoints_test.go +++ b/agent/xds/endpoints_test.go @@ -238,6 +238,10 @@ func Test_endpointsFromSnapshot(t *testing.T) { create: proxycfg.TestConfigSnapshotMeshGateway, setup: nil, }, + { + name: "mesh-gateway-no-services", + create: proxycfg.TestConfigSnapshotMeshGatewayNoServices, + }, { name: "connect-proxy-with-chain", create: proxycfg.TestConfigSnapshotDiscoveryChain, diff --git a/agent/xds/listeners_test.go b/agent/xds/listeners_test.go index f1a02453a..f08d34f7d 100644 --- a/agent/xds/listeners_test.go +++ b/agent/xds/listeners_test.go @@ -224,6 +224,10 @@ func TestListenersFromSnapshot(t *testing.T) { name: "mesh-gateway", create: proxycfg.TestConfigSnapshotMeshGateway, }, + { + name: "mesh-gateway-no-services", + create: proxycfg.TestConfigSnapshotMeshGatewayNoServices, + }, { name: "mesh-gateway-tagged-addresses", create: proxycfg.TestConfigSnapshotMeshGateway, diff --git a/agent/xds/testdata/clusters/mesh-gateway-no-services.golden b/agent/xds/testdata/clusters/mesh-gateway-no-services.golden new file mode 100644 index 000000000..1e4be3b4e --- /dev/null +++ b/agent/xds/testdata/clusters/mesh-gateway-no-services.golden @@ -0,0 +1,7 @@ +{ + "versionInfo": "00000001", + "resources": [ + ], + "typeUrl": "type.googleapis.com/envoy.api.v2.Cluster", + "nonce": "00000001" +} \ No newline at end of file diff --git a/agent/xds/testdata/endpoints/mesh-gateway-no-services.golden b/agent/xds/testdata/endpoints/mesh-gateway-no-services.golden new file mode 100644 index 000000000..b11569ce9 --- /dev/null +++ b/agent/xds/testdata/endpoints/mesh-gateway-no-services.golden @@ -0,0 +1,7 @@ +{ + "versionInfo": "00000001", + "resources": [ + ], + "typeUrl": "type.googleapis.com/envoy.api.v2.ClusterLoadAssignment", + "nonce": "00000001" +} \ No newline at end of file diff --git a/agent/xds/testdata/listeners/mesh-gateway-no-services.golden b/agent/xds/testdata/listeners/mesh-gateway-no-services.golden new file mode 100644 index 000000000..f020c5a2a --- /dev/null +++ b/agent/xds/testdata/listeners/mesh-gateway-no-services.golden @@ -0,0 +1,38 @@ +{ + "versionInfo": "00000001", + "resources": [ + { + "@type": "type.googleapis.com/envoy.api.v2.Listener", + "name": "default:1.2.3.4:8443", + "address": { + "socketAddress": { + "address": "1.2.3.4", + "portValue": 8443 + } + }, + "filterChains": [ + { + "filters": [ + { + "name": "envoy.filters.network.sni_cluster" + }, + { + "name": "envoy.tcp_proxy", + "config": { + "cluster": "", + "stat_prefix": "mesh_gateway_local_default_tcp" + } + } + ] + } + ], + "listenerFilters": [ + { + "name": "envoy.listener.tls_inspector" + } + ] + } + ], + "typeUrl": "type.googleapis.com/envoy.api.v2.Listener", + "nonce": "00000001" +} \ No newline at end of file