From eddd5bd73b6e0eb801fb44c1dad75a67ceffaaa2 Mon Sep 17 00:00:00 2001 From: freddygv Date: Mon, 20 Apr 2020 13:42:33 -0600 Subject: [PATCH] PR comments --- agent/proxycfg/state.go | 22 +++++++++++-------- agent/proxycfg/state_test.go | 4 ++++ .../case-terminating-gateway-simple/vars.sh | 1 + .../verify.bats | 4 ++++ .../case-terminating-gateway-subsets/vars.sh | 1 + .../verify.bats | 4 ++++ 6 files changed, 27 insertions(+), 9 deletions(-) diff --git a/agent/proxycfg/state.go b/agent/proxycfg/state.go index bce9a865b6..8d4c17767d 100644 --- a/agent/proxycfg/state.go +++ b/agent/proxycfg/state.go @@ -33,6 +33,10 @@ const ( datacentersWatchID = "datacenters" serviceResolversWatchID = "service-resolvers" gatewayServicesWatchID = "gateway-services" + externalServiceIDPrefix = "external-service:" + serviceLeafIDPrefix = "service-leaf:" + serviceResolverIDPrefix = "service-resolver:" + serviceIntentionsIDPrefix = "service-intentions:" svcChecksWatchIDPrefix = cachetype.ServiceHTTPChecksName + ":" serviceIDPrefix = string(structs.UpstreamDestTypeService) + ":" preparedQueryIDPrefix = string(structs.UpstreamDestTypePreparedQuery) + ":" @@ -921,7 +925,7 @@ func (s *state) handleUpdateTerminatingGateway(u cache.UpdateEvent, snap *Config // The gateway acts as the service's proxy, so we do NOT want to discover other proxies Connect: false, - }, fmt.Sprintf("external-service:%s", svc.Service.String()), s.ch) + }, externalServiceIDPrefix+svc.Service.String(), s.ch) if err != nil { logger.Error("failed to register watch for external-service", @@ -950,7 +954,7 @@ func (s *state) handleUpdateTerminatingGateway(u cache.UpdateEvent, snap *Config }, }, }, - }, fmt.Sprintf("service-intentions:%s", svc.Service.String()), s.ch) + }, serviceIntentionsIDPrefix+svc.Service.String(), s.ch) if err != nil { logger.Error("failed to register watch for service-intentions", @@ -972,7 +976,7 @@ func (s *state) handleUpdateTerminatingGateway(u cache.UpdateEvent, snap *Config Token: s.token, Service: svc.Service.ID, EnterpriseMeta: svc.Service.EnterpriseMeta, - }, fmt.Sprintf("service-leaf:%s", svc.Service.String()), s.ch) + }, serviceLeafIDPrefix+svc.Service.String(), s.ch) if err != nil { logger.Error("failed to register watch for a service-leaf", @@ -995,7 +999,7 @@ func (s *state) handleUpdateTerminatingGateway(u cache.UpdateEvent, snap *Config Kind: structs.ServiceResolver, Name: svc.Service.ID, EnterpriseMeta: svc.Service.EnterpriseMeta, - }, fmt.Sprintf("service-resolver:%s", svc.Service.String()), s.ch) + }, serviceResolverIDPrefix+svc.Service.String(), s.ch) if err != nil { logger.Error("failed to register watch for a service-resolver", @@ -1051,13 +1055,13 @@ func (s *state) handleUpdateTerminatingGateway(u cache.UpdateEvent, snap *Config } } - case strings.HasPrefix(u.CorrelationID, "external-service:"): + case strings.HasPrefix(u.CorrelationID, externalServiceIDPrefix): resp, ok := u.Result.(*structs.IndexedCheckServiceNodes) if !ok { return fmt.Errorf("invalid type for response: %T", u.Result) } - sid := structs.ServiceIDFromString(strings.TrimPrefix(u.CorrelationID, "external-service:")) + sid := structs.ServiceIDFromString(strings.TrimPrefix(u.CorrelationID, externalServiceIDPrefix)) if len(resp.Nodes) > 0 { snap.TerminatingGateway.ServiceGroups[sid] = resp.Nodes @@ -1066,13 +1070,13 @@ func (s *state) handleUpdateTerminatingGateway(u cache.UpdateEvent, snap *Config } // Store leaf cert for watched service - case strings.HasPrefix(u.CorrelationID, "service-leaf:"): + case strings.HasPrefix(u.CorrelationID, serviceLeafIDPrefix): leaf, ok := u.Result.(*structs.IssuedCert) if !ok { return fmt.Errorf("invalid type for response: %T", u.Result) } - sid := structs.ServiceIDFromString(strings.TrimPrefix(u.CorrelationID, "service-leaf:")) + sid := structs.ServiceIDFromString(strings.TrimPrefix(u.CorrelationID, serviceLeafIDPrefix)) snap.TerminatingGateway.ServiceLeaves[sid] = leaf case strings.HasPrefix(u.CorrelationID, "service-resolver:"): @@ -1087,7 +1091,7 @@ func (s *state) handleUpdateTerminatingGateway(u cache.UpdateEvent, snap *Config } } - case strings.HasPrefix(u.CorrelationID, "service-intentions:"): + case strings.HasPrefix(u.CorrelationID, serviceIntentionsIDPrefix): // no-op: Intentions don't get stored in the snapshot, calls to ConnectAuthorize will fetch them from the cache default: diff --git a/agent/proxycfg/state_test.go b/agent/proxycfg/state_test.go index acaf83be7b..a014f72e37 100644 --- a/agent/proxycfg/state_test.go +++ b/agent/proxycfg/state_test.go @@ -828,6 +828,8 @@ func TestState_WatchesAndUpdates(t *testing.T) { verifySnapshot: func(t testing.TB, snap *ConfigSnapshot) { require.False(t, snap.Valid(), "gateway without root is not valid") require.True(t, snap.ConnectProxy.IsEmpty()) + require.True(t, snap.MeshGateway.IsEmpty()) + require.True(t, snap.IngressGateway.IsEmpty()) }, }, verificationStage{ @@ -837,6 +839,8 @@ func TestState_WatchesAndUpdates(t *testing.T) { verifySnapshot: func(t testing.TB, snap *ConfigSnapshot) { require.True(t, snap.Valid(), "gateway without services is valid") require.True(t, snap.ConnectProxy.IsEmpty()) + require.True(t, snap.MeshGateway.IsEmpty()) + require.True(t, snap.IngressGateway.IsEmpty()) require.Equal(t, indexedRoots, snap.Roots) require.Empty(t, snap.TerminatingGateway.WatchedServices) require.Empty(t, snap.TerminatingGateway.ServiceGroups) diff --git a/test/integration/connect/envoy/case-terminating-gateway-simple/vars.sh b/test/integration/connect/envoy/case-terminating-gateway-simple/vars.sh index 88adda8ac4..da0a6321a0 100644 --- a/test/integration/connect/envoy/case-terminating-gateway-simple/vars.sh +++ b/test/integration/connect/envoy/case-terminating-gateway-simple/vars.sh @@ -1,3 +1,4 @@ #!/bin/bash +# There is no sidecar proxy for s2, since the terminating gateway acts as the proxy export REQUIRED_SERVICES="s1 s1-sidecar-proxy s2 terminating-gateway-primary" diff --git a/test/integration/connect/envoy/case-terminating-gateway-simple/verify.bats b/test/integration/connect/envoy/case-terminating-gateway-simple/verify.bats index 5396d8b54e..48983be21e 100644 --- a/test/integration/connect/envoy/case-terminating-gateway-simple/verify.bats +++ b/test/integration/connect/envoy/case-terminating-gateway-simple/verify.bats @@ -27,3 +27,7 @@ load helpers [ "$status" -eq 0 ] [ "$output" = "hello" ] } + +@test "terminating-gateway is used for the upstream connection" { + assert_envoy_metric_at_least 127.0.0.1:20000 "s2.default.primary.*cx_total" 1 +} diff --git a/test/integration/connect/envoy/case-terminating-gateway-subsets/vars.sh b/test/integration/connect/envoy/case-terminating-gateway-subsets/vars.sh index 514ef784e6..9e52629b8b 100644 --- a/test/integration/connect/envoy/case-terminating-gateway-subsets/vars.sh +++ b/test/integration/connect/envoy/case-terminating-gateway-subsets/vars.sh @@ -1,5 +1,6 @@ #!/bin/bash +# There is no sidecar proxy for s2-v1, since the terminating gateway acts as the proxy export REQUIRED_SERVICES=" s1 s1-sidecar-proxy s2-v1 diff --git a/test/integration/connect/envoy/case-terminating-gateway-subsets/verify.bats b/test/integration/connect/envoy/case-terminating-gateway-subsets/verify.bats index c69fce797c..64a2499e35 100644 --- a/test/integration/connect/envoy/case-terminating-gateway-subsets/verify.bats +++ b/test/integration/connect/envoy/case-terminating-gateway-subsets/verify.bats @@ -34,3 +34,7 @@ load helpers assert_expected_fortio_name s2-v1 } +@test "terminating-gateway is used for the upstream connection" { + assert_envoy_metric_at_least 127.0.0.1:20000 "v1.s2.default.primary.*cx_total" 1 +} +