fix(egress): same external service tag in multiple meshes (#11667)

Signed-off-by: Jakub Dyszkiewicz <[email protected]>

pkg/xds/generator/egress/external_services_generator.go

@@ -88,23 +88,20 @@ func (*ExternalServicesGenerator) generateCDS(
 			continue
 		}
 
-		// There is a case where multiple meshes contain services with
-		// the same names, so we cannot use just "serviceName" as a cluster
-		// name as we would overwrite some clusters with the latest one
-		clusterName := envoy_names.GetMeshClusterName(meshName, serviceName)
-
-		clusterBuilder := envoy_clusters.NewClusterBuilder(apiVersion, clusterName)
+		var clusterBuilder *envoy_clusters.ClusterBuilder
 		isMes := isMeshExternalService(endpoints)
-
 		if isMes {
-			clusterBuilder.WithName(serviceName)
-			clusterBuilder.
+			clusterBuilder = envoy_clusters.NewClusterBuilder(apiVersion, serviceName).
 				Configure(envoy_clusters.ProvidedCustomEndpointCluster(isIPV6, isMes, endpoints...)).
 				Configure(
 					envoy_clusters.MeshExternalServiceClientSideTLS(endpoints, systemCaPath, true),
 				)
 		} else {
-			clusterBuilder.
+			// There is a case where multiple meshes contain services with
+			// the same names, so we cannot use just "serviceName" as a cluster
+			// name as we would overwrite some clusters with the latest one
+			clusterName := envoy_names.GetMeshClusterName(meshName, serviceName)
+			clusterBuilder = envoy_clusters.NewClusterBuilder(apiVersion, clusterName).
 				Configure(envoy_clusters.ProvidedEndpointCluster(
 					isIPV6,
 					endpoints...,
@@ -127,7 +124,7 @@ func (*ExternalServicesGenerator) generateCDS(
 		}
 
 		resource := &core_xds.Resource{
-			Name:     serviceName,
+			Name:     cluster.GetName(),
 			Origin:   OriginEgress,
 			Resource: cluster,
 		}

pkg/xds/generator/egress/generator_test.go

@@ -19,6 +19,7 @@ import (
 	meshhttproute_api "github.com/kumahq/kuma/pkg/plugins/policies/meshhttproute/api/v1alpha1"
 	. "github.com/kumahq/kuma/pkg/test/matchers"
 	"github.com/kumahq/kuma/pkg/test/xds"
+	"github.com/kumahq/kuma/pkg/util/maps"
 	util_proto "github.com/kumahq/kuma/pkg/util/proto"
 	xds_context "github.com/kumahq/kuma/pkg/xds/context"
 	envoy_common "github.com/kumahq/kuma/pkg/xds/envoy"
@@ -164,8 +165,8 @@ var _ = Describe("EgressGenerator", func() {
 			}
 
 			var meshResourcesList []*core_xds.MeshResources
-			for _, meshResources := range meshResourcesMap {
-				meshResourcesList = append(meshResourcesList, meshResources)
+			for _, meshName := range maps.SortedKeys(meshResourcesMap) {
+				meshResourcesList = append(meshResourcesList, meshResourcesMap[meshName])
 			}
 
 			proxy := &core_xds.Proxy{
@@ -234,5 +235,9 @@ var _ = Describe("EgressGenerator", func() {
 			fileWithResourcesName: "subsets-with-external-meshhttproute.yaml",
 			expected:              "subsets-with-external-meshhttproute.golden.yaml",
 		}),
+		Entry("same kuma.io/service", testCase{
+			fileWithResourcesName: "same-kuma-io-service.yaml",
+			expected:              "same-kuma-io-service.golden.yaml",
+		}),
 	)
 })

pkg/xds/generator/egress/testdata/01.externalservice-only.golden.yaml

@@ -1,5 +1,5 @@
 resources:
-- name: externalservice-1
+- name: mesh-1:externalservice-1
   resource:
     '@type': type.googleapis.com/envoy.config.cluster.v3.Cluster
     altStatName: mesh-1_externalservice-1

pkg/xds/generator/egress/testdata/03.mixed-services.golden.yaml

@@ -1,5 +1,5 @@
 resources:
-- name: externalservice-1
+- name: mesh-1:externalservice-1
   resource:
     '@type': type.googleapis.com/envoy.config.cluster.v3.Cluster
     altStatName: mesh-1_externalservice-1
@@ -30,7 +30,7 @@ resources:
         '@type': type.googleapis.com/envoy.extensions.upstreams.http.v3.HttpProtocolOptions
         explicitHttpConfig:
           httpProtocolOptions: {}
-- name: externalservice-2
+- name: mesh-1:externalservice-2
   resource:
     '@type': type.googleapis.com/envoy.config.cluster.v3.Cluster
     altStatName: mesh-1_externalservice-2

pkg/xds/generator/egress/testdata/04.mixed-services-custom-trafficroute.golden.yaml

@@ -1,5 +1,5 @@
 resources:
-- name: externalservice-1
+- name: mesh-1:externalservice-1
   resource:
     '@type': type.googleapis.com/envoy.config.cluster.v3.Cluster
     altStatName: mesh-1_externalservice-1
@@ -30,7 +30,7 @@ resources:
         '@type': type.googleapis.com/envoy.extensions.upstreams.http.v3.HttpProtocolOptions
         explicitHttpConfig:
           httpProtocolOptions: {}
-- name: externalservice-2
+- name: mesh-1:externalservice-2
   resource:
     '@type': type.googleapis.com/envoy.config.cluster.v3.Cluster
     altStatName: mesh-1_externalservice-2

pkg/xds/generator/egress/testdata/05.mixed-services-with-custom-trafficpermissions.golden.yaml

@@ -1,5 +1,5 @@
 resources:
-- name: externalservice-1
+- name: mesh-1:externalservice-1
   resource:
     '@type': type.googleapis.com/envoy.config.cluster.v3.Cluster
     altStatName: mesh-1_externalservice-1
@@ -30,7 +30,7 @@ resources:
         '@type': type.googleapis.com/envoy.extensions.upstreams.http.v3.HttpProtocolOptions
         explicitHttpConfig:
           httpProtocolOptions: {}
-- name: externalservice-2
+- name: mesh-1:externalservice-2
   resource:
     '@type': type.googleapis.com/envoy.config.cluster.v3.Cluster
     altStatName: mesh-1_externalservice-2

pkg/xds/generator/egress/testdata/06.mixed-services-with-external-in-other-zone.golden.yaml

@@ -1,5 +1,5 @@
 resources:
-- name: externalservice-1
+- name: mesh-1:externalservice-1
   resource:
     '@type': type.googleapis.com/envoy.config.cluster.v3.Cluster
     altStatName: mesh-1_externalservice-1

pkg/xds/generator/egress/testdata/input/same-kuma-io-service.yaml

@@ -0,0 +1,90 @@
+type: ZoneEgress
+name: zoneegress-1
+zone: zone-1
+networking:
+  address: 192.168.0.1
+  port: 10002
+---
+type: Mesh
+name: mesh-1
+mtls:
+  enabledBackend: ca-1
+  backends:
+    - name: ca-1
+      type: builtin
+---
+type: TrafficPermission
+name: allow-all-traffic-1
+mesh: mesh-1
+sources:
+  - match:
+      kuma.io/service: '*'
+destinations:
+  - match:
+      kuma.io/service: '*'
+---
+type: TrafficRoute
+name: trafficroute-1
+mesh: mesh-1
+sources:
+  - match:
+      kuma.io/service: "*"
+destinations:
+  - match:
+      kuma.io/service: "*"
+conf:
+  loadBalancer:
+    roundRobin: {}
+  destination:
+    kuma.io/service: "*"
+---
+type: ExternalService
+name: externalservice-1
+mesh: mesh-1
+tags:
+  kuma.io/service: externalservice # same kuma.io/service
+  kuma.io/protocol: http
+networking:
+  address: kuma.io:80
+---
+type: Mesh
+name: mesh-2
+mtls:
+  enabledBackend: ca-1
+  backends:
+    - name: ca-1
+      type: builtin
+---
+type: TrafficPermission
+name: allow-all-traffic-2
+mesh: mesh-2
+sources:
+  - match:
+      kuma.io/service: '*'
+destinations:
+  - match:
+      kuma.io/service: '*'
+---
+type: TrafficRoute
+name: trafficroute-2
+mesh: mesh-2
+sources:
+  - match:
+      kuma.io/service: "*"
+destinations:
+  - match:
+      kuma.io/service: "*"
+conf:
+  loadBalancer:
+    roundRobin: {}
+  destination:
+    kuma.io/service: "*"
+---
+type: ExternalService
+name: externalservice-2
+mesh: mesh-2
+tags:
+  kuma.io/service: externalservice # same kuma.io/service
+  kuma.io/protocol: http
+networking:
+  address: kuma.io:80