Merge pull request openservicemesh#2619 from shashankram/sds-cert

pkg/*: add support for client pods without services

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)