Fix usage of infrastructure labels/annotations on gateway

Fixes istio#50983, I think

Basically, we have logic to use `infra.Labels || meta.Labels`. We modify
meta.Labels.

This is wrong, we should modify AFTER we pick which set of labels to
use.

Also, we need to deepcopy anything we mutate since its accessing a
shared object cache

pilot/pkg/config/kube/gateway/deploymentcontroller.go

@@ -18,6 +18,7 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
+	"istio.io/istio/pkg/maps"
 	"strconv"
 	"strings"
 
@@ -28,6 +29,7 @@ import (
 	klabels "k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/types"
+	gatewayv1 "sigs.k8s.io/gateway-api/apis/v1"
 	gateway "sigs.k8s.io/gateway-api/apis/v1beta1"
 	"sigs.k8s.io/yaml"
 
@@ -346,36 +348,7 @@ func (d *DeploymentController) configureIstioGateway(log *istiolog.Scope, gw gat
 		serviceType = corev1.ServiceType(o)
 	}
 
-	// TODO: Codify this API (i.e how to know if a specific gateway is an Istio waypoint gateway)
-	isWaypointGateway := strings.Contains(string(gw.Spec.GatewayClassName), "waypoint")
 
-	// Default the network label for waypoints if not explicitly set in gateway's labels
-	network := d.injectConfig().Values.Struct().GetGlobal().GetNetwork()
-	if _, ok := gw.GetLabels()[label.TopologyNetwork.Name]; !ok && network != "" && isWaypointGateway {
-		if gw.Labels == nil {
-			gw.Labels = make(map[string]string)
-		}
-		gw.Labels[label.TopologyNetwork.Name] = d.injectConfig().Values.Struct().GetGlobal().GetNetwork()
-	}
-
-	// TODO this sprays ambient annotations/labels all over EVER gateway resource (serviceaccts, services, etc)
-	// where they have no meaning/are not used/are ignored. We really only need them on the deployment podspec.
-	var hasAmbientLabel bool
-	if _, ok := gw.Labels[constants.DataplaneModeLabel]; ok {
-		hasAmbientLabel = true
-	}
-	if gw.Spec.Infrastructure != nil {
-		if _, ok := gw.Spec.Infrastructure.Labels[constants.DataplaneModeLabel]; ok {
-			hasAmbientLabel = true
-		}
-	}
-	// If no ambient redirection label is set explicitly, explicitly disable.
-	if features.EnableAmbientWaypoints && !isWaypointGateway && !hasAmbientLabel {
-		if gw.Labels == nil {
-			gw.Labels = make(map[string]string)
-		}
-		gw.Labels[constants.DataplaneModeLabel] = constants.DataplaneModeNone
-	}
 
 	input := TemplateInput{
 		Gateway:        &gw,
@@ -395,37 +368,15 @@ func (d *DeploymentController) configureIstioGateway(log *istiolog.Scope, gw gat
 	}
 
 	d.setGatewayNameLabel(&input)
+
 	// Default to the gateway labels/annotations and overwrite if infrastructure labels/annotations are set
 	gwInfra := gw.Spec.Infrastructure
-	if gwInfra != nil && gwInfra.Labels != nil {
-		infraLabels := make(map[string]string, len(gwInfra.Labels))
-		for k, v := range gw.Spec.Infrastructure.Labels {
-			if strings.HasPrefix(string(k), "gateway.networking.k8s.io/") {
-				continue // ignore this prefix to avoid conflicts
-			}
-			infraLabels[string(k)] = string(v)
-		}
+	input.InfrastructureLabels = extractInfrastructureMetadata(gwInfra, true, gw)
+	input.InfrastructureAnnotations = extractInfrastructureMetadata(gwInfra, false, gw)
 
-		// Default the network label for waypoints if not explicitly set in infra labels
-		// We do this a second time here for correctness since if infra labels are set (according to the gwapi spec),
-		// the gateway's labels are ignored.
-		if _, ok := infraLabels[label.TopologyNetwork.Name]; !ok && network != "" && isWaypointGateway {
-			infraLabels[label.TopologyNetwork.Name] = network
-		}
-
-		input.InfrastructureLabels = infraLabels
-	}
+	// Set label overrides
 
-	if gwInfra != nil && gwInfra.Annotations != nil {
-		infraAnnotations := make(map[string]string, len(gwInfra.Annotations))
-		for k, v := range gw.Spec.Infrastructure.Annotations {
-			if strings.HasPrefix(string(k), "gateway.networking.k8s.io/") {
-				continue // ignore this prefix to avoid conflicts
-			}
-			infraAnnotations[string(k)] = string(v)
-		}
-		input.InfrastructureAnnotations = infraAnnotations
-	}
+	d.setLabelOverrides(gw, input)
 
 	if overwriteControllerVersion {
 		log.Debugf("write controller version, existing=%v", existingControllerVersion)
@@ -450,6 +401,58 @@ func (d *DeploymentController) configureIstioGateway(log *istiolog.Scope, gw gat
 	return nil
 }
 
+func (d *DeploymentController) setLabelOverrides(gw gateway.Gateway, input TemplateInput) {
+	// TODO: Codify this API (i.e how to know if a specific gateway is an Istio waypoint gateway)
+	isWaypointGateway := strings.Contains(string(gw.Spec.GatewayClassName), "waypoint")
+
+	var hasAmbientLabel bool
+	if _, ok := gw.Labels[constants.DataplaneModeLabel]; ok {
+		hasAmbientLabel = true
+	}
+	if _, ok := input.InfrastructureLabels[constants.DataplaneModeLabel]; ok {
+		hasAmbientLabel = true
+	}
+	// If no ambient redirection label is set explicitly, explicitly disable.
+	// TODO this sprays ambient annotations/labels all over EVER gateway resource (serviceaccts, services, etc)
+	if features.EnableAmbientWaypoints && !isWaypointGateway && !hasAmbientLabel {
+		input.InfrastructureLabels[constants.DataplaneModeLabel] = constants.DataplaneModeNone
+	}
+
+	// Default the network label for waypoints if not explicitly set in gateway's labels
+	network := d.injectConfig().Values.Struct().GetGlobal().GetNetwork()
+	if _, ok := gw.GetLabels()[label.TopologyNetwork.Name]; !ok && network != "" && isWaypointGateway {
+		input.InfrastructureLabels[label.TopologyNetwork.Name] = d.injectConfig().Values.Struct().GetGlobal().GetNetwork()
+	}
+}
+
+func extractInfrastructureMetadata(gwInfra *gatewayv1.GatewayInfrastructure, isLabel bool, gw gateway.Gateway) map[string]string {
+	var field map[gatewayv1.AnnotationKey]gatewayv1.AnnotationValue
+	if gwInfra != nil && isLabel && gwInfra.Labels != nil {
+		field = gwInfra.Labels
+	} else if  gwInfra != nil && !isLabel && gwInfra.Annotations != nil {
+		field = gwInfra.Annotations
+	}
+	if field != nil {
+		infra := make(map[string]string, len(field))
+		for k, v := range field {
+			if strings.HasPrefix(string(k), "gateway.networking.k8s.io/") {
+				continue // ignore this prefix to avoid conflicts
+			}
+			infra[string(k)] = string(v)
+		}
+		return infra
+	} else if isLabel {
+		if gw.GetLabels() == nil {
+			return make(map[string]string)
+		}
+		return maps.Clone(gw.GetLabels())
+	}
+	if gw.GetAnnotations() == nil {
+		return make(map[string]string)
+	}
+	return maps.Clone(gw.GetAnnotations())
+}
+
 const (
 	// ControllerVersionAnnotation is an annotation added to the Gateway by the controller specifying
 	// the "controller version". The original intent of this was to work around

pilot/pkg/config/kube/gateway/deploymentcontroller_test.go

@@ -346,7 +346,7 @@ func TestConfigureIstioGateway(t *testing.T) {
 			kube.SetObjectFilter(client, tt.discoveryNamespaceFilter)
 			client.Kube().Discovery().(*fakediscovery.FakeDiscovery).FakedServerVersion = &kubeVersion.Info{Major: "1", Minor: "28"}
 			kclient.NewWriteClient[*k8sbeta.GatewayClass](client).Create(customClass)
-			kclient.NewWriteClient[*k8sbeta.Gateway](client).Create(&tt.gw)
+			kclient.NewWriteClient[*k8sbeta.Gateway](client).Create(tt.gw.DeepCopy())
 			stop := test.NewStop(t)
 			env := model.NewEnvironment()
 			env.PushContext().ProxyConfigs = tt.pcs
@@ -374,6 +374,10 @@ func TestConfigureIstioGateway(t *testing.T) {
 				resp := timestampRegex.ReplaceAll(buf.Bytes(), []byte("lastTransitionTime: fake"))
 				util.CompareContent(t, resp, filepath.Join("testdata", "deployment", tt.name+".yaml"))
 			}
+			// ensure we didn't mutate the object
+			if !tt.ignore {
+				assert.Equal(t, d.gateways.Get(tt.gw.Name, tt.gw.Namespace), &tt.gw)
+			}
 		})
 	}
 }

pilot/pkg/config/kube/gateway/testdata/deployment/infrastructure-labels-annotations.yaml

@@ -13,6 +13,7 @@ metadata:
     foo: bar
     gateway.istio.io/managed: istio.io-gateway-controller
     gateway.networking.k8s.io/gateway-name: default
+    istio.io/dataplane-mode: none
     istio.io/gateway-name: default
   name: default-istio
   namespace: default
@@ -31,6 +32,7 @@ metadata:
     foo: bar
     gateway.istio.io/managed: istio.io-gateway-controller
     gateway.networking.k8s.io/gateway-name: default
+    istio.io/dataplane-mode: none
     istio.io/gateway-name: default
   name: default-istio
   namespace: default
@@ -54,6 +56,7 @@ spec:
       labels:
         foo: bar
         gateway.networking.k8s.io/gateway-name: default
+        istio.io/dataplane-mode: none
         istio.io/gateway-name: default
         service.istio.io/canonical-name: default-istio
         service.istio.io/canonical-revision: latest
@@ -228,6 +231,7 @@ metadata:
     foo: bar
     gateway.istio.io/managed: istio.io-gateway-controller
     gateway.networking.k8s.io/gateway-name: default
+    istio.io/dataplane-mode: none
     istio.io/gateway-name: default
   name: default-istio
   namespace: default