fix(kuma-cp): deep copy tags when gen. outbounds (backport #5070) (#5071

) * fix(kuma-cp): deep copy tags when gen. outbounds (#5070) It fixes potential data race (iterating and writing to tags map) Signed-off-by: Bart Smykla <bartek@smykla.com> Signed-off-by: Bart Smykla <bartek@smykla.com> (cherry picked from commit 81ccca0) # Conflicts: # pkg/xds/topology/outbound.go * chore: fix conflicts Signed-off-by: Bart Smykla <bartek@smykla.com> Signed-off-by: Bart Smykla <bartek@smykla.com> Co-authored-by: Bart Smykla <bartek@smykla.com>
kumahq · Sep 28, 2022 · 709620c · 709620c
1 parent 4cd3bcc
commit 709620c
Show file tree

Hide file tree

Showing 2 changed files with 16 additions and 15 deletions.
diff --git a/pkg/xds/sync/proxy_builder_test.go b/pkg/xds/sync/proxy_builder_test.go
@@ -138,7 +138,6 @@ var _ = Describe("Proxy Builder", func() {
 						{
 							Tags: map[string]string{
 								mesh_proto.ServiceTag: "service-in-zone-2",
-								"mesh":                "default",
 							},
 							Instances: 1,
 							Mesh:      "default",
@@ -147,7 +146,6 @@ var _ = Describe("Proxy Builder", func() {
 							Tags: map[string]string{
 								mesh_proto.ServiceTag: "external-service-in-zone-2",
 								mesh_proto.ZoneTag:    "zone-2",
-								"mesh":                "default",
 							},
 							Instances:       1,
 							Mesh:            "default",
@@ -168,7 +166,6 @@ var _ = Describe("Proxy Builder", func() {
 						{
 							Tags: map[string]string{
 								mesh_proto.ServiceTag: "service-in-zone-2",
-								"mesh":                "default",
 							},
 							Instances: 1,
 							Mesh:      "default",
@@ -177,7 +174,6 @@ var _ = Describe("Proxy Builder", func() {
 							Tags: map[string]string{
 								mesh_proto.ServiceTag: "external-service-in-zone-2",
 								mesh_proto.ZoneTag:    "zone-2",
-								"mesh":                "default",
 							},
 							Instances:       1,
 							Mesh:            "default",

diff --git a/pkg/xds/topology/outbound.go b/pkg/xds/topology/outbound.go
@@ -138,7 +138,7 @@ func fillDataplaneOutbounds(
 		dpNetworking := dpSpec.GetNetworking()
 
 		for _, inbound := range dpNetworking.GetHealthyInbounds() {
-			inboundTags := inbound.GetTags()
+			inboundTags := cloneTags(inbound.GetTags())
 			serviceName := inboundTags[mesh_proto.ServiceTag]
 			inboundInterface := dpNetworking.ToInboundInterface(inbound)
 			inboundAddress := inboundInterface.DataplaneAdvertisedIP
@@ -325,7 +325,8 @@ func fillIngressOutbounds(
 				continue
 			}
 
-			serviceTags := service.GetTags()
+			// deep copy map to not modify tags in BuildRemoteEndpointMap
+			serviceTags := cloneTags(service.GetTags())
 			serviceName := serviceTags[mesh_proto.ServiceTag]
 			serviceInstances := service.GetInstances()
 			locality := localityFromTags(mesh, priorityRemote, serviceTags)
@@ -352,7 +353,7 @@ func fillIngressOutbounds(
 					}
 					// this is necessary for correct spiffe generation for dp when
 					// traffic is routed: egress -> ingress -> egress
-					if mesh.ZoneEgressEnabled() && service.ExternalService {
+					if service.ExternalService {
 						endpoint.ExternalService = &core_xds.ExternalService{}
 					}
 
@@ -427,10 +428,8 @@ func fillExternalServicesOutboundsThroughEgress(
 	mesh *core_mesh.MeshResource,
 ) {
 	for _, externalService := range externalServices {
-		serviceTags := map[string]string{}
-		for tag, value := range externalService.Spec.GetTags() { // deep copy map to not modify tags in ExternalService.
-			serviceTags[tag] = value
-		}
+		// deep copy map to not modify tags in ExternalService.
+		serviceTags := cloneTags(externalService.Spec.GetTags())
 		serviceName := serviceTags[mesh_proto.ServiceTag]
 		locality := localityFromTags(mesh, priorityRemote, serviceTags)
 
@@ -465,10 +464,8 @@ func NewExternalServiceEndpoint(
 	spec := externalService.Spec
 	tls := spec.GetNetworking().GetTls()
 	meshName := mesh.GetMeta().GetName()
-	tags := map[string]string{}
-	for tag, value := range spec.GetTags() { // deep copy map to not modify tags in ExternalService.
-		tags[tag] = value
-	}
+	// deep copy map to not modify tags in ExternalService.
+	tags := cloneTags(spec.GetTags())
 
 	caCert, err := loadBytes(tls.GetCaCert(), meshName, loader)
 	if err != nil {
@@ -514,6 +511,14 @@ func NewExternalServiceEndpoint(
 	}, nil
 }
 
+func cloneTags(tags map[string]string) map[string]string {
+	result := map[string]string{}
+	for tag, value := range tags {
+		result[tag] = value
+	}
+	return result
+}
+
 func loadBytes(ds *v1alpha1.DataSource, mesh string, loader datasource.Loader) ([]byte, error) {
 	if ds == nil {
 		return nil, nil