Prioritize reroute-virtual-interfaces over kubevirtInterfaces (#57690)

If a pod has both the istio.io/reroute-virtual-interfaces annotation
and the older, deprecated traffic.sidecar.istio.io/kubevirtInterfaces
annotation, the newer one should take precedence. But the CNI plugin
and the injection template code were each handling them separately,
which caused unexpected behavior. This fix makes sure the newer
annotation always takes priority, while still supporting pods that
only use the deprecated annotation.

Fixes: istio/istio#57662

Signed-off-by: Sridhar Gaddam <sgaddam@redhat.com>
Co-authored-by: Sridhar Gaddam <sgaddam@redhat.com>

Author: istio-testing

cni/pkg/plugin/sidecar_redirect.go

@@ -275,18 +275,20 @@ func NewRedirect(pi *PodInfo) (*Redirect, error) {
 		return nil, fmt.Errorf("annotation value error for value %s; annotationFound = %t: %v",
 			"excludeInterfaces", isFound, valErr)
 	}
-	// kubeVirtInterfaces is deprecated, so check it first, but prefer`reroute-virtual-interfaces`
-	// if both are defined.
-	isFound, redir.rerouteVirtualInterfaces, valErr = getAnnotationOrDefault("kubevirtInterfaces", pi.Annotations)
-	if valErr != nil {
-		return nil, fmt.Errorf("annotation value error for value %s; annotationFound = %t: %v",
-			"kubevirtInterfaces", isFound, valErr)
-	}
+	// kubeVirtInterfaces is deprecated, so prefer`reroute-virtual-interfaces` if both are defined.
 	isFound, redir.rerouteVirtualInterfaces, valErr = getAnnotationOrDefault("reroute-virtual-interfaces", pi.Annotations)
 	if valErr != nil {
 		return nil, fmt.Errorf("annotation value error for value %s; annotationFound = %t: %v",
 			"reroute-virtual-interfaces", isFound, valErr)
 	}
+	// Only check deprecated kubevirtInterfaces if reroute-virtual-interfaces was not found
+	if !isFound {
+		isFound, redir.rerouteVirtualInterfaces, valErr = getAnnotationOrDefault("kubevirtInterfaces", pi.Annotations)
+		if valErr != nil {
+			return nil, fmt.Errorf("annotation value error for value %s; annotationFound = %t: %v",
+				"kubevirtInterfaces", isFound, valErr)
+		}
+	}
 	if v, found := pi.ProxyEnvironments["ISTIO_META_DNS_CAPTURE"]; found {
 		// parse and set the bool value of dnsRedirect
 		redir.dnsRedirect, valErr = strconv.ParseBool(v)

manifests/charts/istio-control/istio-discovery/files/injection-template.yaml

@@ -119,13 +119,12 @@ spec:
     - "-o"
     - "{{ annotation .ObjectMeta `traffic.sidecar.istio.io/excludeOutboundPorts` .Values.global.proxy.excludeOutboundPorts }}"
     {{ end -}}
-    {{ if (isset .ObjectMeta.Annotations `traffic.sidecar.istio.io/kubevirtInterfaces`) -}}
-    - "-k"
-    - "{{ index .ObjectMeta.Annotations `traffic.sidecar.istio.io/kubevirtInterfaces` }}"
-    {{ end -}}
     {{ if (isset .ObjectMeta.Annotations `istio.io/reroute-virtual-interfaces`) -}}
     - "-k"
     - "{{ index .ObjectMeta.Annotations `istio.io/reroute-virtual-interfaces` }}"
+    {{ else if (isset .ObjectMeta.Annotations `traffic.sidecar.istio.io/kubevirtInterfaces`) -}}
+    - "-k"
+    - "{{ index .ObjectMeta.Annotations `traffic.sidecar.istio.io/kubevirtInterfaces` }}"
     {{ end -}}
      {{ if (isset .ObjectMeta.Annotations `traffic.sidecar.istio.io/excludeInterfaces`) -}}
     - "-c"

pkg/kube/inject/testdata/inject/reroute-virtual-interfaces.yaml

@@ -13,6 +13,7 @@ spec:
     metadata:
       annotations:
         istio.io/reroute-virtual-interfaces: "net0ps2"
+        traffic.sidecar.istio.io/kubevirtInterfaces: "net1"
       labels:
         app: hello
         tier: backend

pkg/kube/inject/testdata/inject/reroute-virtual-interfaces.yaml.injected

@@ -22,6 +22,7 @@ spec:
         prometheus.io/port: "15020"
         prometheus.io/scrape: "true"
         sidecar.istio.io/status: '{"initContainers":["istio-init","istio-proxy"],"containers":null,"volumes":["workload-socket","credential-socket","workload-certs","istio-envoy","istio-data","istio-podinfo","istio-token","istiod-ca-cert","istio-ca-crl"],"imagePullSecrets":null,"revision":"default"}'
+        traffic.sidecar.istio.io/kubevirtInterfaces: net1
       creationTimestamp: null
       labels:
         app: hello

releasenotes/notes/57662.yaml

@@ -0,0 +1,8 @@
+apiVersion: release-notes/v2
+kind: bug-fix
+area: traffic-management
+issue:
+  - 57662
+releaseNotes:
+- |
+  **Fixed** an annotation issue where both istio.io/reroute-virtual-interfaces and the deprecated traffic.sidecar.istio.io/kubevirtInterfaces were processed. The newer reroute-virtual-interfaces annotation now correctly takes precedence.