Allow SDS NewResponse with no request object (#5049)

* allow sds request object to be nil, in support of envoy's snapshot cache * reduce outbound certs to 1 per service, instead of potentially 1 for each headless service This commit also renames the certificate names, and reduces the set of inbound validation secrets to a single, static secret Signed-off-by: Sean Teeling <[email protected]>
openservicemesh · Aug 31, 2022 · ab69461 · ab69461
1 parent 64703c8
commit ab69461
Show file tree

Hide file tree

Showing 16 changed files with 354 additions and 931 deletions.
diff --git a/pkg/cli/verifier/envoy_config.go b/pkg/cli/verifier/envoy_config.go
@@ -718,14 +718,11 @@ func (v *EnvoyConfigVerifier) findTLSSecretsOnSource(secrets []*xds_secret.Secre
 		return fmt.Errorf("pod %s not found", srcPod)
 	}
 	downstreamIdentity := identity.K8sServiceAccount{Namespace: pod.Namespace, Name: pod.Spec.ServiceAccountName}.ToServiceIdentity()
-	downstreamSecretName := envoySecrets.SDSCert{
-		Name:     envoySecrets.GetSecretNameForIdentity(downstreamIdentity),
-		CertType: envoySecrets.ServiceCertType,
-	}.String()
-	upstreamPeerValidationSecretName := envoySecrets.SDSCert{
-		Name:     v.configAttr.trafficAttr.DstService.String(),
-		CertType: envoySecrets.RootCertTypeForMTLSOutbound,
-	}.String()
+	downstreamSecretName := envoySecrets.NameForIdentity(downstreamIdentity)
+
+	upstreamPeerValidationSecretName := envoySecrets.NameForUpstreamService(
+		v.configAttr.trafficAttr.DstService.Name,
+		v.configAttr.trafficAttr.DstService.Namespace)
 
 	expectedSecrets := mapset.NewSetWith(downstreamSecretName, upstreamPeerValidationSecretName)
 	actualSecrets := mapset.NewSet()
@@ -750,14 +747,8 @@ func (v *EnvoyConfigVerifier) findTLSSecretsOnDestination(secrets []*xds_secret.
 		return fmt.Errorf("pod %s not found", dstPod)
 	}
 	upstreamIdentity := identity.K8sServiceAccount{Namespace: pod.Namespace, Name: pod.Spec.ServiceAccountName}.ToServiceIdentity()
-	upstreamSecretName := envoySecrets.SDSCert{
-		Name:     envoySecrets.GetSecretNameForIdentity(upstreamIdentity),
-		CertType: envoySecrets.ServiceCertType,
-	}.String()
-	downstreamPeerValidationSecretName := envoySecrets.SDSCert{
-		Name:     envoySecrets.GetSecretNameForIdentity(upstreamIdentity),
-		CertType: envoySecrets.RootCertTypeForMTLSInbound,
-	}.String()
+	upstreamSecretName := envoySecrets.NameForIdentity(upstreamIdentity)
+	downstreamPeerValidationSecretName := envoySecrets.NameForMTLSInbound
 
 	expectedSecrets := mapset.NewSetWith(upstreamSecretName, downstreamPeerValidationSecretName)
 	actualSecrets := mapset.NewSet()

diff --git a/pkg/cli/verifier/testdata/httpbin1_permissive.json b/pkg/cli/verifier/testdata/httpbin1_permissive.json
@@ -1374,7 +1374,7 @@
              }
             ],
             "validation_context_sds_secret_config": {
-             "name": "root-cert-for-mtls-inbound:httpbin/httpbin",
+             "name": "root-cert-for-mtls-inbound",
              "sds_config": {
               "ads": {},
               "resource_api_version": "V3"
@@ -1837,12 +1837,12 @@
      }
     },
     {
-     "name": "root-cert-for-mtls-inbound:httpbin/httpbin",
+     "name": "root-cert-for-mtls-inbound",
      "version_info": "4",
      "last_updated": "2022-04-18T17:19:51.751Z",
      "secret": {
       "@type": "type.googleapis.com/envoy.extensions.transport_sockets.tls.v3.Secret",
-      "name": "root-cert-for-mtls-inbound:httpbin/httpbin",
+      "name": "root-cert-for-mtls-inbound",
       "validation_context": {
        "trusted_ca": {
         "inline_bytes": "LS0tLS1CRUdJTiBDRVJUSUZJQ0FURS0tLS0tCk1JSURnVENDQW1tZ0F3SUJBZ0lSQU1MVEorTXRMNVczd3VTLy8wRVpqMGN3RFFZSktvWklodmNOQVFFTEJRQXcKV2pFTE1Ba0dBMVVFQmhNQ1ZWTXhDekFKQmdOVkJBY1RBa05CTVJvd0dBWURWUVFLRXhGUGNHVnVJRk5sY25acApZMlVnVFdWemFERWlNQ0FHQTFVRUF4TVpiM050TFdOaExtOXdaVzV6WlhKMmFXTmxiV1Z6YUM1cGJ6QWVGdzB5Ck1qQTBNVFF5TVRReU1UZGFGdzB6TWpBME1URXlNVFF5TVRkYU1Gb3hDekFKQmdOVkJBWVRBbFZUTVFzd0NRWUQKVlFRSEV3SkRRVEVhTUJnR0ExVUVDaE1SVDNCbGJpQlRaWEoyYVdObElFMWxjMmd4SWpBZ0JnTlZCQU1UR1c5egpiUzFqWVM1dmNHVnVjMlZ5ZG1salpXMWxjMmd1YVc4d2dnRWlNQTBHQ1NxR1NJYjNEUUVCQVFVQUE0SUJEd0F3CmdnRUtBb0lCQVFEUGIvMlYxcDh0NXFmK0dIUFlIWTVFRm54SzJSUVJkWUJDR2lzMUlwcm9xTXFQb0MxUEhGNmcKcHlQUzFxTHUrZitmL1lUOC9HNks2M3RvSUwyN0V1a1Z0RlB1eEhxSC9uS2IxZTJMbXFmRkNsd2YwMFZ3TllWbQpNbmVhS2d1U1pXMkZ2Nkxsd3I1eHlqc2ppemYyTU92cTFtSUNMZXZXeVpQZ3ROanhONk5HTnBaR1JOVVFnUUo3CkM4RDRVd2lVOVdZWmgwNjJUMmhkb1dNekxkQ2JkMjVNcGFzdEQxQ2xXVjlTZnloSnJIZFNDNWU1UzJKLytianMKTVZhR2ZPMTQzUGZmaTJkcEY1cGJRa1lNdjUvUzlValhONmEvT2hEUG10MFBHWDdXT2xkWjJreThyaFQvTnk4MQp6RHdHWXdFckQvYTJkLzkrS1JPMlpUVEM5SVdpOCtKNUFnTUJBQUdqUWpCQU1BNEdBMVVkRHdFQi93UUVBd0lCCkJqQVBCZ05WSFJNQkFmOEVCVEFEQVFIL01CMEdBMVVkRGdRV0JCUnJQVytYNVpwUWZFSDl1UytUTDR5MXAzamkKclRBTkJna3Foa2lHOXcwQkFRc0ZBQU9DQVFFQU96NnZxTHJKNTFkMHl4VVlFSGpnc0lDTktaQmpHTU9Tb25rSQo4L0h0RnVvQmdtQWtoSVRObmxrempMeTZIQVhqZDFINU4vdGlwTEZqMUpWYkpEWUxmYXB5ZDBTUjVVRWtxVktwClhTUm4raUx2NzYxZlBsU2NnNEJ5S0hZM3RyclFmZzdBdSsyekNpU242cjNIYWYrNUo3aTBNYXcyTFpSckxIR0gKVDJld0ZwMm9sNlIvOVU0L2UxS21OU0ZaNEtnZlhyRnJYWGlIWkhhODVpckY5dDBYcTJ2VkM3WjFUd0lRdFlncgo4dEtsVEVFUHdZQ1dzT1ZoMGlyTjV4M3kwQ1lIT25KNm1jZFhMVnJCUi9mb0tPeUNYSUt6TFBOSjJQUFJQVUJ6CngvdHZ2Y1BsUjFOcVFaZ25sRjVoM2l6L0VaTVdPeHNEMWVVUUFsTHNjbmVhT1V5M0RRPT0KLS0tLS1FTkQgQ0VSVElGSUNBVEUtLS0tLQo="

diff --git a/pkg/cli/verifier/testdata/httpbin2_permissive.json b/pkg/cli/verifier/testdata/httpbin2_permissive.json
@@ -1374,7 +1374,7 @@
              }
             ],
             "validation_context_sds_secret_config": {
-             "name": "root-cert-for-mtls-inbound:httpbin/httpbin",
+             "name": "root-cert-for-mtls-inbound",
              "sds_config": {
               "ads": {},
               "resource_api_version": "V3"
@@ -1837,12 +1837,12 @@
      }
     },
     {
-     "name": "root-cert-for-mtls-inbound:httpbin/httpbin",
+     "name": "root-cert-for-mtls-inbound",
      "version_info": "4",
      "last_updated": "2022-04-18T17:19:45.842Z",
      "secret": {
       "@type": "type.googleapis.com/envoy.extensions.transport_sockets.tls.v3.Secret",
-      "name": "root-cert-for-mtls-inbound:httpbin/httpbin",
+      "name": "root-cert-for-mtls-inbound",
       "validation_context": {
        "trusted_ca": {
         "inline_bytes": "LS0tLS1CRUdJTiBDRVJUSUZJQ0FURS0tLS0tCk1JSURnVENDQW1tZ0F3SUJBZ0lSQU1MVEorTXRMNVczd3VTLy8wRVpqMGN3RFFZSktvWklodmNOQVFFTEJRQXcKV2pFTE1Ba0dBMVVFQmhNQ1ZWTXhDekFKQmdOVkJBY1RBa05CTVJvd0dBWURWUVFLRXhGUGNHVnVJRk5sY25acApZMlVnVFdWemFERWlNQ0FHQTFVRUF4TVpiM050TFdOaExtOXdaVzV6WlhKMmFXTmxiV1Z6YUM1cGJ6QWVGdzB5Ck1qQTBNVFF5TVRReU1UZGFGdzB6TWpBME1URXlNVFF5TVRkYU1Gb3hDekFKQmdOVkJBWVRBbFZUTVFzd0NRWUQKVlFRSEV3SkRRVEVhTUJnR0ExVUVDaE1SVDNCbGJpQlRaWEoyYVdObElFMWxjMmd4SWpBZ0JnTlZCQU1UR1c5egpiUzFqWVM1dmNHVnVjMlZ5ZG1salpXMWxjMmd1YVc4d2dnRWlNQTBHQ1NxR1NJYjNEUUVCQVFVQUE0SUJEd0F3CmdnRUtBb0lCQVFEUGIvMlYxcDh0NXFmK0dIUFlIWTVFRm54SzJSUVJkWUJDR2lzMUlwcm9xTXFQb0MxUEhGNmcKcHlQUzFxTHUrZitmL1lUOC9HNks2M3RvSUwyN0V1a1Z0RlB1eEhxSC9uS2IxZTJMbXFmRkNsd2YwMFZ3TllWbQpNbmVhS2d1U1pXMkZ2Nkxsd3I1eHlqc2ppemYyTU92cTFtSUNMZXZXeVpQZ3ROanhONk5HTnBaR1JOVVFnUUo3CkM4RDRVd2lVOVdZWmgwNjJUMmhkb1dNekxkQ2JkMjVNcGFzdEQxQ2xXVjlTZnloSnJIZFNDNWU1UzJKLytianMKTVZhR2ZPMTQzUGZmaTJkcEY1cGJRa1lNdjUvUzlValhONmEvT2hEUG10MFBHWDdXT2xkWjJreThyaFQvTnk4MQp6RHdHWXdFckQvYTJkLzkrS1JPMlpUVEM5SVdpOCtKNUFnTUJBQUdqUWpCQU1BNEdBMVVkRHdFQi93UUVBd0lCCkJqQVBCZ05WSFJNQkFmOEVCVEFEQVFIL01CMEdBMVVkRGdRV0JCUnJQVytYNVpwUWZFSDl1UytUTDR5MXAzamkKclRBTkJna3Foa2lHOXcwQkFRc0ZBQU9DQVFFQU96NnZxTHJKNTFkMHl4VVlFSGpnc0lDTktaQmpHTU9Tb25rSQo4L0h0RnVvQmdtQWtoSVRObmxrempMeTZIQVhqZDFINU4vdGlwTEZqMUpWYkpEWUxmYXB5ZDBTUjVVRWtxVktwClhTUm4raUx2NzYxZlBsU2NnNEJ5S0hZM3RyclFmZzdBdSsyekNpU242cjNIYWYrNUo3aTBNYXcyTFpSckxIR0gKVDJld0ZwMm9sNlIvOVU0L2UxS21OU0ZaNEtnZlhyRnJYWGlIWkhhODVpckY5dDBYcTJ2VkM3WjFUd0lRdFlncgo4dEtsVEVFUHdZQ1dzT1ZoMGlyTjV4M3kwQ1lIT25KNm1jZFhMVnJCUi9mb0tPeUNYSUt6TFBOSjJQUFJQVUJ6CngvdHZ2Y1BsUjFOcVFaZ25sRjVoM2l6L0VaTVdPeHNEMWVVUUFsTHNjbmVhT1V5M0RRPT0KLS0tLS1FTkQgQ0VSVElGSUNBVEUtLS0tLQo="

diff --git a/pkg/cli/verifier/testdata/sample-envoy-config-dump-bookstore.json b/pkg/cli/verifier/testdata/sample-envoy-config-dump-bookstore.json
@@ -1595,7 +1595,7 @@
              }
             ],
             "validation_context_sds_secret_config": {
-             "name": "root-cert-for-mtls-inbound:bookstore/bookstore-v1",
+             "name": "root-cert-for-mtls-inbound",
              "sds_config": {
               "ads": {},
               "resource_api_version": "V3"
@@ -2310,12 +2310,12 @@
      }
     },
     {
-     "name": "root-cert-for-mtls-inbound:bookstore/bookstore-v1",
+     "name": "root-cert-for-mtls-inbound",
      "version_info": "4",
      "last_updated": "2021-08-02T18:54:19.802Z",
      "secret": {
       "@type": "type.googleapis.com/envoy.extensions.transport_sockets.tls.v3.Secret",
-      "name": "root-cert-for-mtls-inbound:bookstore/bookstore-v1",
+      "name": "root-cert-for-mtls-inbound",
       "validation_context": {
        "trusted_ca": {
         "inline_bytes": ""

diff --git a/pkg/envoy/ads/response_benchmark_test.go b/pkg/envoy/ads/response_benchmark_test.go
@@ -168,7 +168,7 @@ func BenchmarkSendXDSResponse(b *testing.B) {
 			setupTestServer(b)
 
 			// Set subscribed resources
-			proxy.SetSubscribedResources(xdsType, mapset.NewSetWith("service-cert:default/bookstore", "root-cert-for-mtls-inbound:default/bookstore|80"))
+			proxy.SetSubscribedResources(xdsType, mapset.NewSetWith("service-cert:default/bookstore", "root-cert-for-mtls-inbound"))
 
 			b.ResetTimer()
 			b.StartTimer()

diff --git a/pkg/envoy/ads/response_test.go b/pkg/envoy/ads/response_test.go
@@ -33,11 +33,11 @@ var _ = Describe("Test ADS response functions", func() {
 	fakeCertManager, err := certificate.FakeCertManager()
 	Expect(err).ToNot(HaveOccurred())
 	proxyUUID := uuid.New()
-	proxySvcAccount := tests.BookstoreServiceAccount
+	proxySvcID := tests.BookstoreServiceIdentity
 
 	proxyRegistry := registry.NewProxyRegistry()
 
-	proxy := envoy.NewProxy(envoy.KindSidecar, proxyUUID, proxySvcAccount.ToServiceIdentity(), nil, 1)
+	proxy := envoy.NewProxy(envoy.KindSidecar, proxyUUID, proxySvcID, nil, 1)
 
 	Context("Proxy is valid", func() {
 		Expect(proxy).ToNot((BeNil()))
@@ -51,7 +51,7 @@ var _ = Describe("Test ADS response functions", func() {
 	Context("Test sendAllResponses()", func() {
 
 		certManager := tresorFake.NewFake(1 * time.Hour)
-		certPEM, _ := certManager.IssueCertificate(proxySvcAccount.ToServiceIdentity().String(), certificate.Service)
+		certPEM, _ := certManager.IssueCertificate(proxySvcID.String(), certificate.Service)
 		cert, _ := certificate.DecodePEMCertificate(certPEM.GetCertificateChain())
 		server, actualResponses := tests.NewFakeXDSServer(cert, nil, nil)
 		kubectrlMock := k8s.NewMockController(mockCtrl)
@@ -74,7 +74,7 @@ var _ = Describe("Test ADS response functions", func() {
 			Expect(s).ToNot(BeNil())
 
 			// Set subscribed resources for SDS
-			proxy.SetSubscribedResources(envoy.TypeSDS, mapset.NewSetWith("service-cert:default/bookstore", "root-cert-for-mtls-inbound:default/bookstore"))
+			proxy.SetSubscribedResources(envoy.TypeSDS, mapset.NewSetWith("service-cert:default/bookstore", "root-cert-for-mtls-inbound"))
 
 			err := s.sendResponse(proxy, &server, nil, envoy.XDSResponseOrder...)
 			Expect(err).To(BeNil())
@@ -108,19 +108,13 @@ var _ = Describe("Test ADS response functions", func() {
 			tmpResource = (*actualResponses)[4].Resources[0]
 			err = tmpResource.UnmarshalTo(&proxyServiceCert)
 			Expect(err).To(BeNil())
-			Expect(proxyServiceCert.Name).To(Equal(secrets.SDSCert{
-				Name:     proxySvcAccount.String(),
-				CertType: secrets.RootCertTypeForMTLSInbound,
-			}.String()))
+			Expect(proxyServiceCert.Name).To(Equal(secrets.NameForIdentity(proxySvcID)))
 
 			serverRootCertTypeForMTLSInbound := xds_auth.Secret{}
 			tmpResource = (*actualResponses)[4].Resources[1]
 			err = tmpResource.UnmarshalTo(&serverRootCertTypeForMTLSInbound)
 			Expect(err).To(BeNil())
-			Expect(serverRootCertTypeForMTLSInbound.Name).To(Equal(secrets.SDSCert{
-				Name:     proxySvcAccount.String(),
-				CertType: secrets.ServiceCertType,
-			}.String()))
+			Expect(serverRootCertTypeForMTLSInbound.Name).To(Equal(secrets.NameForMTLSInbound))
 
 			Expect(metricsstore.DefaultMetricsStore.Contains(fmt.Sprintf("osm_proxy_response_send_success_count{identity=%q,proxy_uuid=%q,type=%q} 1\n", proxy.Identity, proxy.UUID, envoy.TypeCDS))).To(BeTrue())
 		})
@@ -129,7 +123,7 @@ var _ = Describe("Test ADS response functions", func() {
 	Context("Test sendSDSResponse()", func() {
 
 		certManager := tresorFake.NewFake(1 * time.Hour)
-		certCNPrefix := fmt.Sprintf("%s.%s.%s.%s", uuid.New(), envoy.KindSidecar, proxySvcAccount.Name, proxySvcAccount.Namespace)
+		certCNPrefix := fmt.Sprintf("%s.%s.%s", uuid.New(), envoy.KindSidecar, proxySvcID)
 		certPEM, _ := certManager.IssueCertificate(certCNPrefix, certificate.Service)
 		cert, _ := certificate.DecodePEMCertificate(certPEM.GetCertificateChain())
 		server, actualResponses := tests.NewFakeXDSServer(cert, nil, nil)
@@ -150,7 +144,7 @@ var _ = Describe("Test ADS response functions", func() {
 			Expect(s).ToNot(BeNil())
 
 			// Set subscribed resources
-			proxy.SetSubscribedResources(envoy.TypeSDS, mapset.NewSetWith("service-cert:default/bookstore", "root-cert-for-mtls-inbound:default/bookstore"))
+			proxy.SetSubscribedResources(envoy.TypeSDS, mapset.NewSetWith("service-cert:default/bookstore", "root-cert-for-mtls-inbound"))
 
 			err := s.sendResponse(proxy, &server, nil, envoy.TypeSDS)
 			Expect(err).To(BeNil())
@@ -167,25 +161,15 @@ var _ = Describe("Test ADS response functions", func() {
 			// 2. mTLS validation cert when this proxy is an upstream
 			Expect(len(sdsResponse.Resources)).To(Equal(2))
 
-			var tmpResource *any.Any
-
 			proxyServiceCert := xds_auth.Secret{}
-			tmpResource = sdsResponse.Resources[0]
-			err = tmpResource.UnmarshalTo(&proxyServiceCert)
+			err = sdsResponse.Resources[0].UnmarshalTo(&proxyServiceCert)
 			Expect(err).To(BeNil())
-			Expect(proxyServiceCert.Name).To(Equal(secrets.SDSCert{
-				Name:     proxySvcAccount.String(),
-				CertType: secrets.RootCertTypeForMTLSInbound,
-			}.String()))
+			Expect(proxyServiceCert.Name).To(Equal(secrets.NameForIdentity(proxySvcID)))
 
 			serverRootCertTypeForMTLSInbound := xds_auth.Secret{}
-			tmpResource = sdsResponse.Resources[1]
-			err = tmpResource.UnmarshalTo(&serverRootCertTypeForMTLSInbound)
+			err = sdsResponse.Resources[1].UnmarshalTo(&serverRootCertTypeForMTLSInbound)
 			Expect(err).To(BeNil())
-			Expect(serverRootCertTypeForMTLSInbound.Name).To(Equal(secrets.SDSCert{
-				Name:     proxySvcAccount.String(),
-				CertType: secrets.ServiceCertType,
-			}.String()))
+			Expect(serverRootCertTypeForMTLSInbound.Name).To(Equal(secrets.NameForMTLSInbound))
 		})
 	})
 })
diff --git a/pkg/envoy/cds/response_test.go b/pkg/envoy/cds/response_test.go
@@ -256,7 +256,7 @@ func TestNewResponse(t *testing.T) {
 			}},
 			ValidationContextType: &xds_auth.CommonTlsContext_ValidationContextSdsSecretConfig{
 				ValidationContextSdsSecretConfig: &xds_auth.SdsSecretConfig{
-					Name: fmt.Sprintf("%s%s%s", secrets.RootCertTypeForMTLSOutbound, secrets.Separator, "default/bookstore-v1"),
+					Name: secrets.NameForUpstreamService(tests.BookstoreV1Service.Name, "default"),
 					SdsConfig: &xds_core.ConfigSource{
 						ConfigSourceSpecifier: &xds_core.ConfigSource_Ads{
 							Ads: &xds_core.AggregatedConfigSource{},
@@ -287,7 +287,7 @@ func TestNewResponse(t *testing.T) {
 			}},
 			ValidationContextType: &xds_auth.CommonTlsContext_ValidationContextSdsSecretConfig{
 				ValidationContextSdsSecretConfig: &xds_auth.SdsSecretConfig{
-					Name: fmt.Sprintf("%s%s%s", secrets.RootCertTypeForMTLSOutbound, secrets.Separator, "default/bookstore-v2"),
+					Name: secrets.NameForUpstreamService(tests.BookstoreV2Service.Name, "default"),
 					SdsConfig: &xds_core.ConfigSource{
 						ConfigSourceSpecifier: &xds_core.ConfigSource_Ads{
 							Ads: &xds_core.AggregatedConfigSource{},