From b154f7fedc5748a0303c6983643c768d8fed44a3 Mon Sep 17 00:00:00 2001 From: Kalya Subramanian Date: Mon, 17 Aug 2020 17:02:04 -0700 Subject: [PATCH] feat(envoy): Return list of services in GetServiceFromEnvoyCertificate --- DESIGN.md | 5 +++-- pkg/catalog/types.go | 4 ++-- pkg/catalog/xds_certificates.go | 18 ++++++++++++------ pkg/catalog/xds_certificates_test.go | 16 ++++++++++------ pkg/envoy/ads/response.go | 3 ++- pkg/envoy/ads/stream.go | 4 +++- pkg/envoy/cds/response.go | 3 ++- pkg/envoy/eds/response.go | 3 ++- pkg/envoy/lds/response.go | 3 ++- pkg/envoy/rds/response.go | 3 ++- pkg/envoy/sds/response.go | 6 ++++-- 11 files changed, 44 insertions(+), 24 deletions(-) diff --git a/DESIGN.md b/DESIGN.md index 65dbdfdfc9..7eefd9f7ae 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -298,8 +298,9 @@ type MeshCataloger interface { // ExpectProxy catalogs the fact that a certificate was issued for an Envoy proxy and this is expected to connect to XDS. ExpectProxy(certificate.CommonName) - // GetServiceFromEnvoyCertificate returns the single service given Envoy is a member of based on the certificate provided, which is a cert issued to an Envoy for XDS communication (not Envoy-to-Envoy). - GetServiceFromEnvoyCertificate(certificate.CommonName) (*service.MeshService, error) + // GetServicesFromEnvoyCertificate returns a list of services the given Envoy is a member of based on the certificate provided, + // which is a cert issued to an Envoy for XDS communication (not Envoy-to-Envoy). + GetServicesFromEnvoyCertificate(certificate.CommonName) ([]*service.MeshService, error) // RegisterProxy registers a newly connected proxy with the service mesh catalog. RegisterProxy(*envoy.Proxy) diff --git a/pkg/catalog/types.go b/pkg/catalog/types.go index e7997c3987..c3d0a25a65 100644 --- a/pkg/catalog/types.go +++ b/pkg/catalog/types.go @@ -80,8 +80,8 @@ type MeshCataloger interface { // ExpectProxy catalogs the fact that a certificate was issued for an Envoy proxy and this is expected to connect to XDS. ExpectProxy(certificate.CommonName) - // GetServiceFromEnvoyCertificate returns the single service given Envoy is a member of based on the certificate provided, which is a cert issued to an Envoy for XDS communication (not Envoy-to-Envoy). - GetServiceFromEnvoyCertificate(certificate.CommonName) (*service.MeshService, error) + // GetServicesFromEnvoyCertificate returns a list of services the given Envoy is a member of based on the certificate provided, which is a cert issued to an Envoy for XDS communication (not Envoy-to-Envoy). + GetServicesFromEnvoyCertificate(certificate.CommonName) ([]*service.MeshService, error) // RegisterProxy registers a newly connected proxy with the service mesh catalog. RegisterProxy(*envoy.Proxy) diff --git a/pkg/catalog/xds_certificates.go b/pkg/catalog/xds_certificates.go index 0d3009247d..6c899b9960 100644 --- a/pkg/catalog/xds_certificates.go +++ b/pkg/catalog/xds_certificates.go @@ -15,8 +15,9 @@ import ( "github.com/openservicemesh/osm/pkg/service" ) -// GetServiceFromEnvoyCertificate returns the single service given Envoy is a member of based on the certificate provided, which is a cert issued to an Envoy for XDS communication (not Envoy-to-Envoy). -func (mc *MeshCatalog) GetServiceFromEnvoyCertificate(cn certificate.CommonName) (*service.MeshService, error) { +// GetServicesFromEnvoyCertificate returns a list of services the given Envoy is a member of based on the certificate provided, which is a cert issued to an Envoy for XDS communication (not Envoy-to-Envoy). +func (mc *MeshCatalog) GetServicesFromEnvoyCertificate(cn certificate.CommonName) ([]*service.MeshService, error) { + serviceList := []*service.MeshService{} pod, err := GetPodFromCertificate(cn, mc.kubeClient) if err != nil { return nil, err @@ -41,10 +42,15 @@ func (mc *MeshCatalog) GetServiceFromEnvoyCertificate(cn certificate.CommonName) return nil, err } - return &service.MeshService{ - Namespace: cnMeta.Namespace, - Name: services[0].Name, - }, nil + for _, svc := range services { + meshService := &service.MeshService{ + Namespace: cnMeta.Namespace, + Name: svc.Name, + } + serviceList = append(serviceList, meshService) + + } + return serviceList, nil } // filterTrafficSplitServices takes a list of services and removes from it the ones diff --git a/pkg/catalog/xds_certificates_test.go b/pkg/catalog/xds_certificates_test.go index c5794dc9ff..406f3c4515 100644 --- a/pkg/catalog/xds_certificates_test.go +++ b/pkg/catalog/xds_certificates_test.go @@ -25,7 +25,7 @@ var _ = Describe("Test XDS certificate tooling", func() { mc := NewFakeMeshCatalog(kubeClient) cn := certificate.CommonName(fmt.Sprintf("%s.%s.%s", tests.EnvoyUID, tests.BookstoreServiceAccountName, tests.Namespace)) - Context("Test GetServiceFromEnvoyCertificate()", func() { + Context("Test GetServicesFromEnvoyCertificate()", func() { It("works as expected", func() { pod := tests.NewPodTestFixtureWithOptions(tests.Namespace, "pod-name", tests.BookstoreServiceAccountName) _, err := kubeClient.CoreV1().Pods(tests.Namespace).Create(context.TODO(), &pod, metav1.CreateOptions{}) @@ -39,17 +39,19 @@ var _ = Describe("Test XDS certificate tooling", func() { _, err = kubeClient.CoreV1().Services(tests.Namespace).Create(context.TODO(), svc, metav1.CreateOptions{}) Expect(err).ToNot(HaveOccurred()) - meshService, err := mc.GetServiceFromEnvoyCertificate(cn) + meshServices, err := mc.GetServicesFromEnvoyCertificate(cn) Expect(err).ToNot(HaveOccurred()) expected := service.MeshService{ Namespace: tests.Namespace, Name: svcName, } - Expect(meshService).To(Equal(&expected)) + expectedList := []*service.MeshService{&expected} + + Expect(meshServices).To(Equal(expectedList)) }) It("returns an error with an invalid CN", func() { - service, err := mc.GetServiceFromEnvoyCertificate("getAllowedDirectionalServices") + service, err := mc.GetServicesFromEnvoyCertificate("getAllowedDirectionalServices") Expect(err).To(HaveOccurred()) Expect(service).To(BeNil()) }) @@ -77,14 +79,16 @@ var _ = Describe("Test XDS certificate tooling", func() { Expect(err).ToNot(HaveOccurred()) podCN := certificate.CommonName(fmt.Sprintf("%s.%s.%s", envoyUID, tests.BookstoreServiceAccountName, namespace)) - meshService, err := mc.GetServiceFromEnvoyCertificate(podCN) + meshServices, err := mc.GetServicesFromEnvoyCertificate(podCN) Expect(err).ToNot(HaveOccurred()) expected := service.MeshService{ Namespace: namespace, Name: svcName, } - Expect(meshService).To(Equal(&expected)) + expectedList := []*service.MeshService{&expected} + + Expect(meshServices).To(Equal(expectedList)) }) }) diff --git a/pkg/envoy/ads/response.go b/pkg/envoy/ads/response.go index e4124a4b3c..3b9e105f7d 100644 --- a/pkg/envoy/ads/response.go +++ b/pkg/envoy/ads/response.go @@ -46,11 +46,12 @@ 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, catalog catalog.MeshCataloger) *xds_discovery.DiscoveryRequest { - serviceForProxy, err := catalog.GetServiceFromEnvoyCertificate(proxy.GetCommonName()) + svcList, err := catalog.GetServicesFromEnvoyCertificate(proxy.GetCommonName()) if err != nil { log.Error().Err(err).Msgf("Error looking up MeshService for Envoy with CN=%q", proxy.GetCommonName()) return nil } + serviceForProxy := svcList[0] return &xds_discovery.DiscoveryRequest{ ResourceNames: []string{ diff --git a/pkg/envoy/ads/stream.go b/pkg/envoy/ads/stream.go index 35d4153b4e..8cd70c82d7 100644 --- a/pkg/envoy/ads/stream.go +++ b/pkg/envoy/ads/stream.go @@ -24,11 +24,13 @@ func (s *Server) StreamAggregatedResources(server xds_discovery.AggregatedDiscov ip := utils.GetIPFromContext(server.Context()) - namespacedService, err := s.catalog.GetServiceFromEnvoyCertificate(cn) + svcList, err := s.catalog.GetServicesFromEnvoyCertificate(cn) if err != nil { log.Error().Err(err).Msgf("Error fetching service for Envoy %s with CN %s", ip, cn) return err } + namespacedService := svcList[0] + log.Info().Msgf("Client %s connected: Subject CN=%s; Service=%s", ip, cn, namespacedService) proxy := envoy.NewProxy(cn, ip) diff --git a/pkg/envoy/cds/response.go b/pkg/envoy/cds/response.go index fb9880552e..6e0c403ea0 100644 --- a/pkg/envoy/cds/response.go +++ b/pkg/envoy/cds/response.go @@ -17,11 +17,12 @@ import ( // NewResponse creates a new Cluster Discovery Response. func NewResponse(_ context.Context, catalog catalog.MeshCataloger, proxy *envoy.Proxy, _ *xds_discovery.DiscoveryRequest, cfg configurator.Configurator) (*xds_discovery.DiscoveryResponse, error) { - svc, err := catalog.GetServiceFromEnvoyCertificate(proxy.GetCommonName()) + svcList, err := catalog.GetServicesFromEnvoyCertificate(proxy.GetCommonName()) if err != nil { log.Error().Err(err).Msgf("Error looking up MeshService for Envoy with CN=%q", proxy.GetCommonName()) return nil, err } + svc := svcList[0] proxyServiceName := *svc resp := &xds_discovery.DiscoveryResponse{ diff --git a/pkg/envoy/eds/response.go b/pkg/envoy/eds/response.go index 4e61dcb84d..0281c49704 100644 --- a/pkg/envoy/eds/response.go +++ b/pkg/envoy/eds/response.go @@ -18,11 +18,12 @@ import ( // NewResponse creates a new Endpoint Discovery Response. func NewResponse(_ context.Context, catalog catalog.MeshCataloger, proxy *envoy.Proxy, request *xds_discovery.DiscoveryRequest, cfg configurator.Configurator) (*xds_discovery.DiscoveryResponse, error) { - svc, err := catalog.GetServiceFromEnvoyCertificate(proxy.GetCommonName()) + svcList, err := catalog.GetServicesFromEnvoyCertificate(proxy.GetCommonName()) if err != nil { log.Error().Err(err).Msgf("Error looking up MeshService for Envoy with CN=%q", proxy.GetCommonName()) return nil, err } + svc := svcList[0] proxyServiceName := *svc allTrafficPolicies, err := catalog.ListTrafficPolicies(proxyServiceName) diff --git a/pkg/envoy/lds/response.go b/pkg/envoy/lds/response.go index 46940cd1ac..dd741b6592 100644 --- a/pkg/envoy/lds/response.go +++ b/pkg/envoy/lds/response.go @@ -25,11 +25,12 @@ const ( // 2. Outbound listener to handle outgoing traffic // 3. Prometheus listener for metrics func NewResponse(ctx context.Context, catalog catalog.MeshCataloger, proxy *envoy.Proxy, request *xds_discovery.DiscoveryRequest, cfg configurator.Configurator) (*xds_discovery.DiscoveryResponse, error) { - svc, err := catalog.GetServiceFromEnvoyCertificate(proxy.GetCommonName()) + svcList, err := catalog.GetServicesFromEnvoyCertificate(proxy.GetCommonName()) if err != nil { log.Error().Err(err).Msgf("Error looking up MeshService for Envoy with CN=%q", proxy.GetCommonName()) return nil, err } + svc := svcList[0] proxyServiceName := *svc resp := &xds_discovery.DiscoveryResponse{ diff --git a/pkg/envoy/rds/response.go b/pkg/envoy/rds/response.go index 8713a2215f..2ee5cbddf9 100644 --- a/pkg/envoy/rds/response.go +++ b/pkg/envoy/rds/response.go @@ -19,11 +19,12 @@ import ( // NewResponse creates a new Route Discovery Response. func NewResponse(ctx context.Context, catalog catalog.MeshCataloger, proxy *envoy.Proxy, request *xds_discovery.DiscoveryRequest, cfg configurator.Configurator) (*xds_discovery.DiscoveryResponse, error) { - svc, err := catalog.GetServiceFromEnvoyCertificate(proxy.GetCommonName()) + svcList, err := catalog.GetServicesFromEnvoyCertificate(proxy.GetCommonName()) if err != nil { log.Error().Err(err).Msgf("Error looking up MeshService for Envoy with CN=%q", proxy.GetCommonName()) return nil, err } + svc := svcList[0] proxyServiceName := *svc allTrafficPolicies, err := catalog.ListTrafficPolicies(proxyServiceName) diff --git a/pkg/envoy/sds/response.go b/pkg/envoy/sds/response.go index 426435de44..cd695525b6 100644 --- a/pkg/envoy/sds/response.go +++ b/pkg/envoy/sds/response.go @@ -28,7 +28,8 @@ var directionMap = map[envoy.SDSCertType]string{ func NewResponse(_ context.Context, catalog catalog.MeshCataloger, proxy *envoy.Proxy, request *xds_discovery.DiscoveryRequest, cfg configurator.Configurator) (*xds_discovery.DiscoveryResponse, error) { log.Info().Msgf("Composing SDS Discovery Response for proxy: %s", proxy.GetCommonName()) - serviceForProxy, err := catalog.GetServiceFromEnvoyCertificate(proxy.GetCommonName()) + svcList, err := catalog.GetServicesFromEnvoyCertificate(proxy.GetCommonName()) + serviceForProxy := svcList[0] if err != nil { log.Error().Err(err).Msgf("Error looking up service for Envoy with CN=%q", proxy.GetCommonName()) return nil, err @@ -68,11 +69,12 @@ func getEnvoySDSSecrets(cert certificate.Certificater, proxy *envoy.Proxy, reque var envoySecrets []*xds_auth.Secret - svc, err := catalog.GetServiceFromEnvoyCertificate(proxy.GetCommonName()) + svcList, err := catalog.GetServicesFromEnvoyCertificate(proxy.GetCommonName()) if err != nil { log.Error().Err(err).Msgf("Error looking up service for Envoy with CN=%q", proxy.GetCommonName()) return nil } + svc := svcList[0] serviceForProxy := *svc // The Envoy makes a request for a list of resources (aka certificates), which we will send as a response to the SDS request.