Backport change to use correct map key when deleting endpoints in san…

changelog/v1.8.15/upstream-sanitizer-fix.yaml

@@ -0,0 +1,5 @@
+changelog:
+  - type: FIX
+    issueLink: https://github.com/solo-io/gloo/issues/5022
+    resolvesIssue: true
+    description: Ensure that endpoints for invalid upstreams are deleted from snapshots and invalid upstreams are not accepted.

projects/gloo/pkg/syncer/envoy_translator_syncer.go

@@ -137,14 +137,14 @@ func (s *translatorSyncer) syncEnvoy(ctx context.Context, snap *v1.ApiSnapshot,
 
 		sanitizedSnapshot, err := s.sanitizer.SanitizeSnapshot(ctx, snap, xdsSnapshot, reports)
 		if err != nil {
-			logger.Warnf("proxy %v was rejected due to invalid config: %v\n"+
+			logger.Errorf("proxy %v was rejected due to invalid config: %v\n"+
 				"Attempting to update only EDS information", proxy.GetMetadata().Ref().Key(), err)
 
 			// If the snapshot is invalid, attempt at least to update the EDS information. This is important because
 			// endpoints are relatively ephemeral entities and the previous snapshot Envoy got might be stale by now.
 			sanitizedSnapshot, err = s.updateEndpointsOnly(key, xdsSnapshot)
 			if err != nil {
-				logger.Warnf("endpoint update failed. xDS snapshot for proxy %v will not be updated. "+
+				logger.Errorf("endpoint update failed. xDS snapshot for proxy %v will not be updated. "+
 					"Error is: %s", proxy.GetMetadata().Ref().Key(), err)
 				continue
 			}
@@ -153,7 +153,6 @@ func (s *translatorSyncer) syncEnvoy(ctx context.Context, snap *v1.ApiSnapshot,
 
 		// Merge reports after sanitization to capture changes made by the sanitizers
 		allReports.Merge(reports)
-
 		if err := s.xdsCache.SetSnapshot(key, sanitizedSnapshot); err != nil {
 			err := eris.Wrapf(err, "failed while updating xDS snapshot cache")
 			logger.DPanicw("", zap.Error(err))

projects/gloo/pkg/syncer/sanitizer/upstream_removing_sanitizer.go

@@ -3,15 +3,15 @@ package sanitizer
 import (
 	"context"
 
+	envoy_config_cluster_v3 "github.com/envoyproxy/go-control-plane/envoy/config/cluster/v3"
 	"github.com/solo-io/gloo/pkg/utils"
-	"github.com/solo-io/gloo/projects/gloo/pkg/syncer/stats"
-	"github.com/solo-io/solo-kit/pkg/api/v1/control-plane/resource"
-
 	v1 "github.com/solo-io/gloo/projects/gloo/pkg/api/v1"
+	"github.com/solo-io/gloo/projects/gloo/pkg/syncer/stats"
 	"github.com/solo-io/gloo/projects/gloo/pkg/translator"
 	"github.com/solo-io/gloo/projects/gloo/pkg/xds"
 	"github.com/solo-io/go-utils/contextutils"
 	envoycache "github.com/solo-io/solo-kit/pkg/api/v1/control-plane/cache"
+	"github.com/solo-io/solo-kit/pkg/api/v1/control-plane/resource"
 	"github.com/solo-io/solo-kit/pkg/api/v2/reporter"
 )
 
@@ -46,16 +46,26 @@ func (s *UpstreamRemovingSanitizer) SanitizeSnapshot(
 
 	clusters := xdsSnapshot.GetResources(resource.ClusterTypeV3)
 	endpoints := xdsSnapshot.GetResources(resource.EndpointTypeV3)
-
 	var removed int64
 
 	// Find all the errored upstreams and remove them from the xDS snapshot
 	for _, up := range glooSnapshot.Upstreams.AsInputResources() {
+
 		if reports[up].Errors != nil {
+
 			clusterName := translator.UpstreamToClusterName(up.GetMetadata().Ref())
+			endpointName := clusterName
+			cluster, _ := clusters.Items[clusterName].ResourceProto().(*envoy_config_cluster_v3.Cluster)
+			if cluster.GetType() == envoy_config_cluster_v3.Cluster_EDS {
+				if cluster.GetEdsClusterConfig().GetServiceName() != "" {
+					endpointName = cluster.GetEdsClusterConfig().GetServiceName()
+				} else {
+					endpointName = cluster.GetName()
+				}
+			}
 			// remove cluster and endpoints
 			delete(clusters.Items, clusterName)
-			delete(endpoints.Items, clusterName)
+			delete(endpoints.Items, endpointName)
 			removed++
 		}
 	}

projects/gloo/pkg/syncer/sanitizer/upstream_removing_sanitizer_test.go

@@ -3,6 +3,7 @@ package sanitizer_test
 import (
 	"context"
 
+	envoy_config_cluster_v3 "github.com/envoyproxy/go-control-plane/envoy/config/cluster/v3"
 	envoyclusterapi "github.com/envoyproxy/go-control-plane/envoy/config/cluster/v3"
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
@@ -26,9 +27,19 @@ var _ = Describe("UpstreamRemovingSanitizer", func() {
 				Namespace: "upstream",
 			},
 		}
+		clusterType = &envoy_config_cluster_v3.Cluster_Type{
+			Type: envoy_config_cluster_v3.Cluster_EDS,
+		}
 		goodClusterName = translator.UpstreamToClusterName(us.Metadata.Ref())
 		goodCluster     = &envoyclusterapi.Cluster{
-			Name: goodClusterName,
+			Name:                 goodClusterName,
+			ClusterDiscoveryType: clusterType,
+			EdsClusterConfig: &envoy_config_cluster_v3.Cluster_EdsClusterConfig{
+				ServiceName: goodClusterName + "-123",
+			},
+		}
+		goodEndpoint = &envoyclusterapi.Cluster{
+			Name: goodClusterName + "-123",
 		}
 
 		badUs = &v1.Upstream{
@@ -39,21 +50,30 @@ var _ = Describe("UpstreamRemovingSanitizer", func() {
 		}
 		badClusterName = translator.UpstreamToClusterName(badUs.Metadata.Ref())
 		badCluster     = &envoyclusterapi.Cluster{
-			Name: badClusterName,
+			Name:                 badClusterName,
+			ClusterDiscoveryType: clusterType,
+			EdsClusterConfig: &envoy_config_cluster_v3.Cluster_EdsClusterConfig{
+				ServiceName: badClusterName + "-123",
+			},
+		}
+		badEndpoint = &envoyclusterapi.Cluster{
+			Name: badClusterName + "-123",
 		}
 	)
 	It("removes upstreams whose reports have an error, and changes the error to a warning", func() {
 
 		xdsSnapshot := xds.NewSnapshotFromResources(
-			envoycache.NewResources("", nil),
-			envoycache.NewResources("clusters", []envoycache.Resource{
+			envoycache.NewResources("unit_test", []envoycache.Resource{
+				resource.NewEnvoyResource(goodEndpoint),
+				resource.NewEnvoyResource(badEndpoint),
+			}),
+			envoycache.NewResources("unit_test", []envoycache.Resource{
 				resource.NewEnvoyResource(goodCluster),
 				resource.NewEnvoyResource(badCluster),
 			}),
 			envoycache.NewResources("", nil),
 			envoycache.NewResources("", nil),
 		)
-
 		sanitizer := NewUpstreamRemovingSanitizer()
 
 		reports := reporter.ResourceReports{