envoy/ads: program upsteam's service cert when making all secrets

There is a bug where the upstream service's service cert that
is referenced in the corresponding inbound filter chain for
the service is not programmed when there is a request to push
all secrets via XDS. Without this, it has been observed that
in certain environments the secrets often don't transition
from warming to active. As a result, mTLS is broken when
such a situation occurs.

This change programs the upsteam service's service cert
as a part of the request to provision all secrets associated
with the proxy.

This change does not address other potential XDS problems,
such as the one pertaining to XDS DiscoveryRequest with
an empty TypeUrl.

Potentially addresses #2749

Signed-off-by: Shashank Ram <shashr2204@gmail.com>

pkg/envoy/ads/response.go

@@ -112,7 +112,24 @@ func makeRequestForAllSecrets(proxy *envoy.Proxy, meshCatalog catalog.MeshCatalo
 		TypeUrl: string(envoy.TypeSDS),
 	}
 
-	// There is an SDS validation cert corresponding to each upstream service.
+	// Create an SDS service cert for each service associated with this proxy.
+	// This service cert is referenced in the per service inbound filter chain's DownstreamTlsContext
+	// that is a part the proxy's inbound listener.
+	proxyServices, err := meshCatalog.GetServicesFromEnvoyCertificate(proxy.GetCertificateCommonName())
+	if err != nil {
+		log.Error().Err(err).Msgf("Error getting services associated with Envoy with certificate SerialNumber=%s on Pod with UID=%s",
+			proxy.GetCertificateSerialNumber(), proxy.GetPodUID())
+		return nil
+	}
+	for _, proxySvc := range proxyServices {
+		proxySvcCert := envoy.SDSCert{
+			Name:     proxySvc.String(),
+			CertType: envoy.ServiceCertType,
+		}.String()
+		discoveryRequest.ResourceNames = append(discoveryRequest.ResourceNames, proxySvcCert)
+	}
+
+	// Create an SDS validation cert corresponding to each upstream service that this proxy can connect to.
 	// Each cert is used to validate the certificate presented by the corresponding upstream service.
 	upstreamServices := meshCatalog.ListAllowedOutboundServicesForIdentity(proxyIdentity)
 	for _, upstream := range upstreamServices {

pkg/envoy/ads/response_test.go

@@ -22,6 +22,7 @@ import (
 	"github.com/openservicemesh/osm/pkg/configurator"
 	"github.com/openservicemesh/osm/pkg/constants"
 	"github.com/openservicemesh/osm/pkg/envoy"
+	"github.com/openservicemesh/osm/pkg/service"
 	"github.com/openservicemesh/osm/pkg/tests"
 )
 
@@ -42,7 +43,7 @@ var _ = Describe("Test ADS response functions", func() {
 	kubeClient := testclient.NewSimpleClientset()
 	namespace := tests.Namespace
 	proxyUUID := tests.ProxyUUID
-	serviceName := tests.BookstoreV1ServiceName
+	proxyService := service.MeshService{Name: tests.BookstoreV1ServiceName, Namespace: namespace}
 	proxySvcAccount := tests.BookstoreServiceAccount
 
 	labels := map[string]string{constants.EnvoyUniqueIDLabelName: tests.ProxyUUID}
@@ -56,7 +57,7 @@ var _ = Describe("Test ADS response functions", func() {
 		Expect(err).ToNot(HaveOccurred())
 	})
 
-	svc := tests.NewServiceFixture(serviceName, namespace, labels)
+	svc := tests.NewServiceFixture(proxyService.Name, namespace, labels)
 	_, err = kubeClient.CoreV1().Services(namespace).Create(context.TODO(), svc, metav1.CreateOptions{})
 	It("should have created a service", func() {
 		Expect(err).ToNot(HaveOccurred())
@@ -81,17 +82,25 @@ var _ = Describe("Test ADS response functions", func() {
 				TypeUrl: string(envoy.TypeSDS),
 				ResourceNames: []string{
 					envoy.SDSCert{
+						// Client certificate presented when this proxy is a downstream
 						Name:     proxySvcAccount.String(),
 						CertType: envoy.ServiceCertType,
 					}.String(),
 					envoy.SDSCert{
+						// Validation certificate for mTLS when this proxy is an upstream
 						Name:     proxySvcAccount.String(),
 						CertType: envoy.RootCertTypeForMTLSInbound,
 					}.String(),
 					envoy.SDSCert{
+						// Validation ceritificate for TLS when this proxy is an upstream
 						Name:     proxySvcAccount.String(),
 						CertType: envoy.RootCertTypeForHTTPS,
 					}.String(),
+					envoy.SDSCert{
+						// Server certificate presented when this proxy is an upstream
+						Name:     proxyService.String(),
+						CertType: envoy.ServiceCertType,
+					}.String(),
 				},
 			}
 			Expect(actual).ToNot(BeNil())
@@ -141,7 +150,13 @@ var _ = Describe("Test ADS response functions", func() {
 			Expect((*actualResponses)[4].VersionInfo).To(Equal("1"))
 			Expect((*actualResponses)[4].TypeUrl).To(Equal(string(envoy.TypeSDS)))
 			log.Printf("%v", len((*actualResponses)[4].Resources))
-			Expect(len((*actualResponses)[4].Resources)).To(Equal(3))
+
+			// Expect 4 SDS certs:
+			// 1. client cert when this proxy is a downstream
+			// 2. mTLS validation cert when this proxy is an upstream
+			// 3. TLS validation cert when this proxy is an upstream
+			// 4. server cert when this proxy is an upstream
+			Expect(len((*actualResponses)[4].Resources)).To(Equal(4))
 
 			secretOne := xds_auth.Secret{}
 			firstSecret := (*actualResponses)[4].Resources[0]
@@ -166,6 +181,15 @@ var _ = Describe("Test ADS response functions", func() {
 				Name:     proxySvcAccount.String(),
 				CertType: envoy.RootCertTypeForHTTPS,
 			}.String()))
+
+			secretFour := xds_auth.Secret{}
+			fourthSecret := (*actualResponses)[4].Resources[3]
+			err = ptypes.UnmarshalAny(fourthSecret, &secretFour)
+			Expect(err).To(BeNil())
+			Expect(secretFour.Name).To(Equal(envoy.SDSCert{
+				Name:     proxyService.String(),
+				CertType: envoy.ServiceCertType,
+			}.String()))
 		})
 	})
 
@@ -200,7 +224,13 @@ var _ = Describe("Test ADS response functions", func() {
 
 			Expect(sdsResponse.VersionInfo).To(Equal("2")) // 2 because first update was by the previous test for the proxy
 			Expect(sdsResponse.TypeUrl).To(Equal(string(envoy.TypeSDS)))
-			Expect(len(sdsResponse.Resources)).To(Equal(3))
+
+			// Expect 4 SDS certs:
+			// 1. client cert when this proxy is a downstream
+			// 2. mTLS validation cert when this proxy is an upstream
+			// 3. TLS validation cert when this proxy is an upstream
+			// 4. server cert when this proxy is an upstream
+			Expect(len(sdsResponse.Resources)).To(Equal(4))
 
 			secretOne := xds_auth.Secret{}
 			firstSecret := sdsResponse.Resources[0]
@@ -225,6 +255,15 @@ var _ = Describe("Test ADS response functions", func() {
 				Name:     proxySvcAccount.String(),
 				CertType: envoy.RootCertTypeForHTTPS,
 			}.String()))
+
+			secretFour := xds_auth.Secret{}
+			fourthSecret := sdsResponse.Resources[3]
+			err = ptypes.UnmarshalAny(fourthSecret, &secretFour)
+			Expect(err).To(BeNil())
+			Expect(secretFour.Name).To(Equal(envoy.SDSCert{
+				Name:     proxyService.String(),
+				CertType: envoy.ServiceCertType,
+			}.String()))
 		})
 	})
 })