pkg/*: add support for client pods without services

This change adds support to allow downstream clients
without services to connect to upstream services while
removing the need for synethetic services completely.

At a high level, this is accomplished by assigning
client proxies certificates based on their identities
(service account) instead of services associated with
the proxies. This is possible because the certificates
for a proxy does not have any service metadata in them.

Some XDS tls_context related utility structs and helpers
have been made generic enough to not depend on the SDS
secret being associated with a service.

Resolves openservicemesh#2064

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

demo/deploy-bookbuyer.sh

@@ -21,25 +21,6 @@ metadata:
   namespace: $BOOKBUYER_NAMESPACE
 EOF
 
-echo -e "Deploy BookBuyer Service"
-kubectl apply -f - <<EOF
-apiVersion: v1
-kind: Service
-metadata:
-  name: bookbuyer
-  namespace: "$BOOKBUYER_NAMESPACE"
-  labels:
-    app: bookbuyer
-spec:
-  ports:
-
-  - port: 9999
-    name: dummy-unused-port
-
-  selector:
-    app: bookbuyer
-EOF
-
 echo -e "Deploy BookBuyer Deployment"
 kubectl apply -f - <<EOF
 apiVersion: apps/v1

go.mod

@@ -22,7 +22,7 @@ require (
 	github.com/hashicorp/go-version v1.1.0
 	github.com/hashicorp/vault/api v1.0.4
 	github.com/jetstack/cert-manager v0.16.1
-	github.com/jinzhu/copier v0.0.0-20190924061706-b57f9002281a
+	github.com/jinzhu/copier v0.2.4
 	github.com/jstemmer/go-junit-report v0.0.0-20190106144839-af01ea7f8024
 	github.com/matm/gocov-html v0.0.0-20200509184451-71874e2e203b
 	github.com/mitchellh/gox v1.0.1

pkg/catalog/xds_certificates.go

@@ -29,7 +29,7 @@ func (mc *MeshCatalog) GetServicesFromEnvoyCertificate(cn certificate.CommonName
 	}
 
 	if len(services) == 0 {
-		return makeSyntheticServiceForPod(pod, cn), nil
+		return nil, nil
 	}
 
 	// Remove services that have been split into other services.
@@ -62,17 +62,6 @@ func listServiceNames(meshServices []service.MeshService) (serviceNames []string
 	return serviceNames
 }
 
-func makeSyntheticServiceForPod(pod *v1.Pod, proxyCommonName certificate.CommonName) []service.MeshService {
-	svcAccount := service.K8sServiceAccount{
-		Namespace: pod.Namespace,
-		Name:      pod.Spec.ServiceAccountName,
-	}
-	syntheticService := svcAccount.GetSyntheticService()
-	log.Debug().Msgf("Creating synthetic service %s since no actual services found for Envoy with xDS CN %s",
-		syntheticService, proxyCommonName)
-	return []service.MeshService{syntheticService}
-}
-
 // filterTrafficSplitServices takes a list of services and removes from it the ones
 // that have been split via an SMI TrafficSplit.
 func (mc *MeshCatalog) filterTrafficSplitServices(services []v1.Service) []v1.Service {

pkg/catalog/xds_certificates_test.go

@@ -32,31 +32,6 @@ var _ = Describe("Test XDS certificate tooling", func() {
 	mc := NewFakeMeshCatalog(kubeClient)
 	cn := certificate.CommonName(fmt.Sprintf("%s.%s.%s", tests.ProxyUUID, tests.BookstoreServiceAccountName, tests.Namespace))
 
-	Context("Test makeSyntheticServiceForPod()", func() {
-		It("creates a MeshService struct with properly formatted Name and Namespace of the synthetic service", func() {
-			namespace := uuid.New().String()
-			serviceAccountName := uuid.New().String()
-			cn := certificate.CommonName(uuid.New().String())
-			pod := &v1.Pod{
-				ObjectMeta: metav1.ObjectMeta{
-					Namespace: namespace,
-				},
-				Spec: v1.PodSpec{
-					ServiceAccountName: serviceAccountName,
-				},
-			}
-
-			actual := makeSyntheticServiceForPod(pod, cn)
-
-			expected := service.MeshService{
-				Name:      fmt.Sprintf("%s.%s.osm.synthetic-%s", serviceAccountName, namespace, service.SyntheticServiceSuffix),
-				Namespace: namespace,
-			}
-			Expect(len(actual)).To(Equal(1))
-			Expect(actual[0]).To(Equal(expected))
-		})
-	})
-
 	Context("Test GetServicesFromEnvoyCertificate()", func() {
 		It("works as expected", func() {
 			pod := tests.NewPodFixture(tests.Namespace, "pod-name", tests.BookstoreServiceAccountName, tests.PodLabels)

pkg/endpoint/providers/kube/client.go

@@ -39,7 +39,7 @@ func (c Client) ListEndpointsForService(svc service.MeshService) []endpoint.Endp
 
 	kubernetesEndpoints, err := c.kubeController.GetEndpoints(svc)
 	if err != nil || kubernetesEndpoints == nil {
-		log.Error().Err(err).Msgf("[%s] Error fetching Kubernetes Endpoints from cache", c.providerIdent)
+		log.Error().Err(err).Msgf("[%s] Error fetching Kubernetes Endpoints from cache for service %s", c.providerIdent, svc)
 		return endpoints
 	}
 
@@ -124,15 +124,11 @@ func (c Client) GetServicesForServiceAccount(svcAccount service.K8sServiceAccoun
 	}
 
 	if services.Cardinality() == 0 {
-		// Add a service, which is a representation of the ServiceAccount, but not a real K8s service.
-		// This will ensure that all pods in the service account are represented as one service.
-		synthService := svcAccount.GetSyntheticService()
-		services.Add(synthService)
-		log.Trace().Msgf("[%s] No services for service account %s/%s; Adding synthetic service %s", c.providerIdent, svcAccount.Name, svcAccount.Namespace, synthService)
-	} else {
-		log.Trace().Msgf("[%s] Services for service account %s: %+v", c.providerIdent, svcAccount, services)
+		log.Error().Err(errServiceNotFound).Msgf("[%s] No services for service account %s", c.providerIdent, svcAccount)
+		return nil, errServiceNotFound
 	}
 
+	log.Trace().Msgf("[%s] Services for service account %s: %+v", c.providerIdent, svcAccount, services)
 	servicesSlice := make([]service.MeshService, 0, services.Cardinality())
 	for svc := range services.Iterator().C {
 		servicesSlice = append(servicesSlice, svc.(service.MeshService))

pkg/endpoint/providers/kube/client_test.go

@@ -2,7 +2,6 @@ package kube
 
 import (
 	"context"
-	"fmt"
 	"net"
 	"testing"
 	"time"
@@ -278,7 +277,7 @@ var _ = Describe("Test Kube Client Provider (/w kubecontroller)", func() {
 		stop <- struct{}{}
 	})
 
-	It("should return a synthetic service when a pod matching the selector doesn't exist", func() {
+	It("should return an error when a pod matching the selector doesn't exist", func() {
 		svc := &corev1.Service{
 			ObjectMeta: metav1.ObjectMeta{
 				Name:      "test-1",
@@ -300,10 +299,8 @@ var _ = Describe("Test Kube Client Provider (/w kubecontroller)", func() {
 		Expect(err).ToNot(HaveOccurred())
 
 		services, err := provider.GetServicesForServiceAccount(tests.BookbuyerServiceAccount)
-		Expect(err).ToNot(HaveOccurred())
-		Expect(len(services)).To(Equal(1))
-		expectedServiceName := fmt.Sprintf("bookbuyer.default.osm.synthetic-%s", service.SyntheticServiceSuffix)
-		Expect(services[0].Name).To(Equal(expectedServiceName))
+		Expect(err).To(HaveOccurred())
+		Expect(services).To(BeNil())
 
 		err = fakeClientSet.CoreV1().Services(testNamespace).Delete(context.TODO(), svc.Name, metav1.DeleteOptions{})
 		Expect(err).ToNot(HaveOccurred())
@@ -390,7 +387,7 @@ var _ = Describe("Test Kube Client Provider (/w kubecontroller)", func() {
 		<-podsAndServiceChannel
 	})
 
-	It("should return a synthetic service when the Service selector doesn't match the pod", func() {
+	It("should return an error when the Service selector doesn't match the pod", func() {
 		podsChannel := events.GetPubSubInstance().Subscribe(announcements.PodAdded,
 			announcements.PodDeleted,
 			announcements.PodUpdated)
@@ -454,17 +451,15 @@ var _ = Describe("Test Kube Client Provider (/w kubecontroller)", func() {
 
 		// Expect a MeshService that corresponds to a Service that matches the Deployment spec labels
 		svcs, err := provider.GetServicesForServiceAccount(givenSvcAccount)
-		Expect(err).ToNot(HaveOccurred())
-		Expect(len(svcs)).To(Equal(1))
-		expectedServiceName := fmt.Sprintf("test-service-account.testNamespace.osm.synthetic-%s", service.SyntheticServiceSuffix)
-		Expect(svcs[0].Name).To(Equal(expectedServiceName))
+		Expect(err).To(HaveOccurred())
+		Expect(svcs).To(BeNil())
 
 		err = fakeClientSet.CoreV1().Pods(testNamespace).Delete(context.Background(), pod.Name, metav1.DeleteOptions{})
 		Expect(err).ToNot(HaveOccurred())
 		<-podsChannel
 	})
 
-	It("should return a synthetic service when the service doesn't have a selector", func() {
+	It("should return an error when the service doesn't have a selector", func() {
 		podsChannel := events.GetPubSubInstance().Subscribe(announcements.PodAdded,
 			announcements.PodDeleted,
 			announcements.PodUpdated)
@@ -524,10 +519,8 @@ var _ = Describe("Test Kube Client Provider (/w kubecontroller)", func() {
 
 		// Expect a MeshService that corresponds to a Service that matches the Deployment spec labels
 		svcs, err := provider.GetServicesForServiceAccount(givenSvcAccount)
-		Expect(err).ToNot(HaveOccurred())
-		Expect(len(svcs)).To(Equal(1))
-		expectedServiceName := fmt.Sprintf("test-service-account.testNamespace.osm.synthetic-%s", service.SyntheticServiceSuffix)
-		Expect(svcs[0].Name).To(Equal(expectedServiceName))
+		Expect(err).To(HaveOccurred())
+		Expect(svcs).To(BeNil())
 
 		err = fakeClientSet.CoreV1().Pods(testNamespace).Delete(context.Background(), pod.Name, metav1.DeleteOptions{})
 		Expect(err).ToNot(HaveOccurred())

pkg/envoy/ads/response.go

@@ -77,14 +77,6 @@ func (s *Server) sendAllResponses(proxy *envoy.Proxy, server *xds_discovery.Aggr
 // This request will result in the rest of the system creating an SDS response with the certificates
 // required by this proxy. The proxy itself did not ask for these. We know it needs them - so we send them.
 func makeRequestForAllSecrets(proxy *envoy.Proxy, meshCatalog catalog.MeshCataloger) *xds_discovery.DiscoveryRequest {
-	svcList, err := meshCatalog.GetServicesFromEnvoyCertificate(proxy.GetCertificateCommonName())
-	if err != nil {
-		log.Error().Err(err).Msgf("Error looking up MeshService for Envoy with SerialNumber=%s on Pod with UID=%s", proxy.GetCertificateSerialNumber(), proxy.GetPodUID())
-		return nil
-	}
-	// Github Issue #1575
-	serviceForProxy := svcList[0]
-
 	proxyIdentity, err := catalog.GetServiceAccountFromProxyCertificate(proxy.GetCertificateCommonName())
 	if err != nil {
 		log.Error().Err(err).Msgf("Error looking up proxy identity for proxy with SerialNumber=%s on Pod with UID=%s",
@@ -95,27 +87,28 @@ func makeRequestForAllSecrets(proxy *envoy.Proxy, meshCatalog catalog.MeshCatalo
 	discoveryRequest := &xds_discovery.DiscoveryRequest{
 		ResourceNames: []string{
 			envoy.SDSCert{
-				MeshService: serviceForProxy,
-				CertType:    envoy.ServiceCertType,
+				Name:     proxyIdentity.String(),
+				CertType: envoy.ServiceCertType,
 			}.String(),
 			envoy.SDSCert{
-				MeshService: serviceForProxy,
-				CertType:    envoy.RootCertTypeForMTLSInbound,
+				Name:     proxyIdentity.String(),
+				CertType: envoy.RootCertTypeForMTLSInbound,
 			}.String(),
 			envoy.SDSCert{
-				MeshService: serviceForProxy,
-				CertType:    envoy.RootCertTypeForHTTPS,
+				Name:     proxyIdentity.String(),
+				CertType: envoy.RootCertTypeForHTTPS,
 			}.String(),
 		},
 		TypeUrl: string(envoy.TypeSDS),
 	}
 
-	// There is an SDS validation cert corresponding to each upstream service
+	// There is an SDS validation cert corresponding to each upstream service.
+	// Each cert is used to validate the certificate presented by the corresponding upstream service.
 	upstreamServices := meshCatalog.ListAllowedOutboundServicesForIdentity(proxyIdentity)
 	for _, upstream := range upstreamServices {
 		upstreamRootCertResource := envoy.SDSCert{
-			MeshService: upstream,
-			CertType:    envoy.RootCertTypeForMTLSOutbound,
+			Name:     upstream.String(),
+			CertType: envoy.RootCertTypeForMTLSOutbound,
 		}.String()
 		discoveryRequest.ResourceNames = append(discoveryRequest.ResourceNames, upstreamRootCertResource)
 	}

pkg/envoy/ads/response_test.go

@@ -22,7 +22,6 @@ 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"
 )
 
@@ -44,7 +43,7 @@ var _ = Describe("Test ADS response functions", func() {
 	namespace := tests.Namespace
 	proxyUUID := tests.ProxyUUID
 	serviceName := tests.BookstoreV1ServiceName
-	serviceAccountName := tests.BookstoreServiceAccountName
+	proxySvcAccount := tests.BookstoreServiceAccount
 
 	labels := map[string]string{constants.EnvoyUniqueIDLabelName: tests.ProxyUUID}
 	mc := catalog.NewFakeMeshCatalog(kubeClient)
@@ -70,15 +69,10 @@ var _ = Describe("Test ADS response functions", func() {
 		GinkgoT().Fatalf("Error creating new Bookstire Apex service: %s", err.Error())
 	}
 
-	certCommonName := certificate.CommonName(fmt.Sprintf("%s.%s.%s", proxyUUID, serviceAccountName, namespace))
+	certCommonName := certificate.CommonName(fmt.Sprintf("%s.%s.%s", proxyUUID, proxySvcAccount.Name, proxySvcAccount.Namespace))
 	certSerialNumber := certificate.SerialNumber("123456")
 	proxy := envoy.NewProxy(certCommonName, certSerialNumber, nil)
 
-	meshService := service.MeshService{
-		Namespace: "default",
-		Name:      serviceName,
-	}
-
 	Context("Test makeRequestForAllSecrets()", func() {
 		It("returns service cert", func() {
 
@@ -87,16 +81,16 @@ var _ = Describe("Test ADS response functions", func() {
 				TypeUrl: string(envoy.TypeSDS),
 				ResourceNames: []string{
 					envoy.SDSCert{
-						MeshService: meshService,
-						CertType:    envoy.ServiceCertType,
+						Name:     proxySvcAccount.String(),
+						CertType: envoy.ServiceCertType,
 					}.String(),
 					envoy.SDSCert{
-						MeshService: meshService,
-						CertType:    envoy.RootCertTypeForMTLSInbound,
+						Name:     proxySvcAccount.String(),
+						CertType: envoy.RootCertTypeForMTLSInbound,
 					}.String(),
 					envoy.SDSCert{
-						MeshService: meshService,
-						CertType:    envoy.RootCertTypeForHTTPS,
+						Name:     proxySvcAccount.String(),
+						CertType: envoy.RootCertTypeForHTTPS,
 					}.String(),
 				},
 			}
@@ -108,7 +102,7 @@ var _ = Describe("Test ADS response functions", func() {
 	Context("Test sendAllResponses()", func() {
 
 		certManager := tresor.NewFakeCertManager(mockConfigurator)
-		certCommonName := certificate.CommonName(fmt.Sprintf("%s.%s.%s", uuid.New(), serviceAccountName, tests.Namespace))
+		certCommonName := certificate.CommonName(fmt.Sprintf("%s.%s.%s", uuid.New(), proxySvcAccount.Name, proxySvcAccount.Namespace))
 		certDuration := 1 * time.Hour
 		certPEM, _ := certManager.IssueCertificate(certCommonName, certDuration)
 		cert, _ := certificate.DecodePEMCertificate(certPEM.GetCertificateChain())
@@ -153,24 +147,24 @@ var _ = Describe("Test ADS response functions", func() {
 			firstSecret := (*actualResponses)[4].Resources[0]
 			err = ptypes.UnmarshalAny(firstSecret, &secretOne)
 			Expect(secretOne.Name).To(Equal(envoy.SDSCert{
-				MeshService: meshService,
-				CertType:    envoy.ServiceCertType,
+				Name:     proxySvcAccount.String(),
+				CertType: envoy.ServiceCertType,
 			}.String()))
 
 			secretTwo := xds_auth.Secret{}
 			secondSecret := (*actualResponses)[4].Resources[1]
 			err = ptypes.UnmarshalAny(secondSecret, &secretTwo)
 			Expect(secretTwo.Name).To(Equal(envoy.SDSCert{
-				MeshService: meshService,
-				CertType:    envoy.RootCertTypeForMTLSInbound,
+				Name:     proxySvcAccount.String(),
+				CertType: envoy.RootCertTypeForMTLSInbound,
 			}.String()))
 
 			secretThree := xds_auth.Secret{}
 			thirdSecret := (*actualResponses)[4].Resources[2]
 			err = ptypes.UnmarshalAny(thirdSecret, &secretThree)
 			Expect(secretThree.Name).To(Equal(envoy.SDSCert{
-				MeshService: meshService,
-				CertType:    envoy.RootCertTypeForHTTPS,
+				Name:     proxySvcAccount.String(),
+				CertType: envoy.RootCertTypeForHTTPS,
 			}.String()))
 		})
 	})

pkg/envoy/cds/cluster.go

@@ -23,10 +23,10 @@ const (
 )
 
 // getUpstreamServiceCluster returns an Envoy Cluster corresponding to the given upstream service
-func getUpstreamServiceCluster(upstreamSvc, downstreamSvc service.MeshService, cfg configurator.Configurator) (*xds_cluster.Cluster, error) {
+func getUpstreamServiceCluster(downstreamIdentity service.K8sServiceAccount, upstreamSvc service.MeshService, cfg configurator.Configurator) (*xds_cluster.Cluster, error) {
 	clusterName := upstreamSvc.String()
 	marshalledUpstreamTLSContext, err := ptypes.MarshalAny(
-		envoy.GetUpstreamTLSContext(downstreamSvc, upstreamSvc))
+		envoy.GetUpstreamTLSContext(downstreamIdentity, upstreamSvc))
 	if err != nil {
 		return nil, err
 	}

pkg/envoy/cds/cluster_test.go

@@ -26,7 +26,8 @@ func TestGetUpstreamServiceCluster(t *testing.T) {
 
 	mockCtrl := gomock.NewController(t)
 	mockConfigurator := configurator.NewMockConfigurator(mockCtrl)
-	downstreamSvc := tests.BookbuyerService
+
+	downstreamSvcAccount := tests.BookbuyerServiceAccount
 	upstreamSvc := tests.BookstoreV1Service
 
 	testCases := []struct {
@@ -55,7 +56,7 @@ func TestGetUpstreamServiceCluster(t *testing.T) {
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
 			mockConfigurator.EXPECT().IsPermissiveTrafficPolicyMode().Return(tc.permissiveMode).Times(1)
-			remoteCluster, err := getUpstreamServiceCluster(upstreamSvc, downstreamSvc, mockConfigurator)
+			remoteCluster, err := getUpstreamServiceCluster(downstreamSvcAccount, upstreamSvc, mockConfigurator)
 			assert.Nil(err)
 			assert.Equal(tc.expectedClusterType, remoteCluster.GetType())
 			assert.Equal(tc.expectedLbPolicy, remoteCluster.LbPolicy)