Repository: istio/istio (go)

Issue:
Ztunnel missing writeable /tmp dir for gathering pprof
### Is this the right place to submit this?

- [X] This is not a security vulnerability or a crashing bug
- [X] This is not a question about how to use Istio

### Bug Description

When attempting to gather a `debug/pprof/profile` for `ztunnel` on a default install the user is met with an empty response
![Screenshot 2024-03-21 at 5 14 09 PM](https://github.com/istio/istio/assets/31321277/46f28ac8-ddf7-4c81-9d98-ca54eeaa3cb9)

To resolve this, the following `volume` and `volumeMount` is required to add a `/tmp` directory in the ztunnel daemonset manually, or using helm:
```
helm upgrade --install ztunnel istio/ztunnel \
-n kube-system \
--version=1.21.0 \
-f -<<EOF
hub: docker.io/istio
tag: 1.21.0
resources:
  requests:
      cpu: 500m
      memory: 2048Mi
istioNamespace: istio-system
# Additional volumeMounts to the ztunnel container
volumeMounts:
  - name: tmp
    mountPath: /tmp
# Additional volumes to the ztunnel pod
volumes:
  - name: tmp
    emptyDir:
      medium: Memory
EOF
```

Should we be including this missing `volume` and `volumeMount` in the default ztunnel deployment templates? or alternatively, if we decide not to fix the template so it "just works" should we upgrade the handler to have a meaningful reply if `ztunnel` doesn't have the writable location it requires?

cc: @ilrudie @bleggett 


### Version

```Text
% istioctl version
client version: 1.21.0
control plane version: 1.21.0
data plane version: 1.21.0 (25 proxies)

% kubectl version
Client Version: v1.29.0
Kustomize Version: v5.0.4-0.20230601165947-6ce0bf390ce3
Server Version: v1.28.3-gke.1118000
```


### Additional Information

_No response_

Pull request:
ambient: give ztunnel writable /tmp
**Please provide a description of this PR:**

Fixes https://github.com/istio/istio/issues/50060 - same thing we do in other spots.

Patch:
diff --git a/go.mod b/go.mod
index 5bff5ff8fe..c7d0727263 100644
--- a/go.mod
+++ b/go.mod
@@ -106,8 +106,8 @@ require (
 	gopkg.in/yaml.v2 v2.4.0
 	gopkg.in/yaml.v3 v3.0.1
 	helm.sh/helm/v3 v3.14.2
-	istio.io/api v1.19.0-alpha.1.0.20240321165207-94d8c5322f50
-	istio.io/client-go v1.19.0-alpha.1.0.20240321165507-d35e94d3a4ea
+	istio.io/api v1.19.0-alpha.1.0.20240325173917-35fefa2d2903
+	istio.io/client-go v1.19.0-alpha.1.0.20240325174217-6067be7adc24
 	k8s.io/api v0.29.2
 	k8s.io/apiextensions-apiserver v0.29.2
 	k8s.io/apimachinery v0.29.2
diff --git a/go.sum b/go.sum
index 73b80dcae2..3abeed5c53 100644
--- a/go.sum
+++ b/go.sum
@@ -1097,10 +1097,10 @@ helm.sh/helm/v3 v3.14.2/go.mod h1:2itvvDv2WSZXTllknfQo6j7u3VVgMAvm8POCDgYH424=
 honnef.co/go/tools v0.0.0-20190102054323-c2f93a96b099/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4=
 honnef.co/go/tools v0.0.0-20190106161140-3f1c8253044a/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4=
 honnef.co/go/tools v0.0.0-20190523083050-ea95bdfd59fc/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4=
-istio.io/api v1.19.0-alpha.1.0.20240321165207-94d8c5322f50 h1:2Sw+Ctjc56CazNdSTKxEVpazlW0AVRxnA+gHghC/rbw=
-istio.io/api v1.19.0-alpha.1.0.20240321165207-94d8c5322f50/go.mod h1:w/4svOduCtCR+gGn/PuEBIrXfnX4MgA9TlIQgLPEFJk=
-istio.io/client-go v1.19.0-alpha.1.0.20240321165507-d35e94d3a4ea h1:NqrWq+InxPB7YR7Jan71h/Ed3/pJONk6nMspocuir4w=
-istio.io/client-go v1.19.0-alpha.1.0.20240321165507-d35e94d3a4ea/go.mod h1:ouPsLoGCOMM4Av0Hi3x94IlcB45gYUAKZ+DyPwDjkMw=
+istio.io/api v1.19.0-alpha.1.0.20240325173917-35fefa2d2903 h1:o+2ARAwO0ypJyYvQiV2XLuuatTibf+ivA2GKyumq+xQ=
+istio.io/api v1.19.0-alpha.1.0.20240325173917-35fefa2d2903/go.mod h1:qjrJ+ahABt9bvLmhhYehVGLtPRARsVfYR3HoZzdWll0=
+istio.io/client-go v1.19.0-alpha.1.0.20240325174217-6067be7adc24 h1:0E0UEwFBYCgC41BSu1ki9JGjtFPH3RLlWAUuwGVXdVc=
+istio.io/client-go v1.19.0-alpha.1.0.20240325174217-6067be7adc24/go.mod h1:pa6S038vnoWYWA2RZnH1viST8qtsf/qN8SnAecHEtLM=
 k8s.io/api v0.18.2/go.mod h1:SJCWI7OLzhZSvbY7U8zwNl9UA4o1fizoug34OV/2r78=
 k8s.io/api v0.18.4/go.mod h1:lOIQAKYgai1+vz9J7YcDZwC26Z0zQewYOGWdyIPUUQ4=
 k8s.io/api v0.29.2 h1:hBC7B9+MU+ptchxEqTNW2DkUosJpp1P+Wn6YncZ474A=
diff --git a/manifests/charts/ztunnel/templates/daemonset.yaml b/manifests/charts/ztunnel/templates/daemonset.yaml
index 11daa5abe8..9a42110052 100644
--- a/manifests/charts/ztunnel/templates/daemonset.yaml
+++ b/manifests/charts/ztunnel/templates/daemonset.yaml
@@ -135,6 +135,8 @@ spec:
           name: istio-token
         - mountPath: /var/run/ztunnel
           name: cni-ztunnel-sock-dir
+        - mountPath: /tmp
+          name: tmp
         {{- with .Values.volumeMounts }}
           {{- toYaml . | nindent 8 }}
         {{- end }}
@@ -155,6 +157,9 @@ spec:
         hostPath:
           path: /var/run/ztunnel
           type: DirectoryOrCreate # ideally this would be a socket, but ztunnel may not have started yet.
+      # pprof needs a writable /tmp, and we don't have that thanks to `readOnlyRootFilesystem: true`, so mount one
+      - name: tmp
+        emptyDir: {}
       {{- with .Values.volumes }}
         {{- toYaml . | nindent 6}}
       {{- end }}
diff --git a/pilot/pkg/serviceregistry/kube/controller/controller.go b/pilot/pkg/serviceregistry/kube/controller/controller.go
index 3ad7f8e0d6..cf35b3b202 100644
--- a/pilot/pkg/serviceregistry/kube/controller/controller.go
+++ b/pilot/pkg/serviceregistry/kube/controller/controller.go
@@ -1168,10 +1168,10 @@ func serviceUpdateNeedsPush(prev, curr *v1.Service, preConv, currConv *model.Ser
 	// Also check if target ports are changed since they are not included in `model.Service`
 	// `preConv.Equals(currConv)` already makes sure the length of ports is not changed
 	if prev != nil && curr != nil {
-		for i := 0; i < len(prev.Spec.Ports); i++ {
-			if prev.Spec.Ports[i].TargetPort != curr.Spec.Ports[i].TargetPort {
-				return true
-			}
+		if !slices.EqualFunc(prev.Spec.Ports, curr.Spec.Ports, func(a, b v1.ServicePort) bool {
+			return a.TargetPort == b.TargetPort
+		}) {
+			return true
 		}
 	}
 	return false
diff --git a/pilot/pkg/serviceregistry/serviceentry/controller.go b/pilot/pkg/serviceregistry/serviceentry/controller.go
index 5172b2d74d..f903a7fad8 100644
--- a/pilot/pkg/serviceregistry/serviceentry/controller.go
+++ b/pilot/pkg/serviceregistry/serviceentry/controller.go
@@ -41,6 +41,7 @@ import (
 	"istio.io/istio/pkg/maps"
 	"istio.io/istio/pkg/network"
 	"istio.io/istio/pkg/queue"
+	"istio.io/istio/pkg/slices"
 	"istio.io/istio/pkg/util/protomarshal"
 	"istio.io/istio/pkg/util/sets"
 )
@@ -358,7 +359,7 @@ func getUpdatedConfigs(services []*model.Service) sets.Set[model.ConfigKey] {
 }
 
 // serviceEntryHandler defines the handler for service entries
-func (s *Controller) serviceEntryHandler(_, curr config.Config, event model.Event) {
+func (s *Controller) serviceEntryHandler(old, curr config.Config, event model.Event) {
 	log.Debugf("Handle event %s for service entry %s/%s", event, curr.Namespace, curr.Name)
 	currentServiceEntry := curr.Spec.(*networking.ServiceEntry)
 	cs := convertServices(curr)
@@ -372,6 +373,15 @@ func (s *Controller) serviceEntryHandler(_, curr config.Config, event model.Even
 	switch event {
 	case model.EventUpdate:
 		addedSvcs, deletedSvcs, updatedSvcs, unchangedSvcs = servicesDiff(s.services.getServices(key), cs)
+		oldServiceEntry := old.Spec.(*networking.ServiceEntry)
+		// Also check if target ports are changed since they are not included in `model.Service`
+		if !slices.EqualFunc(oldServiceEntry.Ports, currentServiceEntry.Ports, func(a, b *networking.ServicePort) bool {
+			return a.TargetPort == b.TargetPort
+		}) {
+			// Note: If the length of ports is changed, unchangedSvcs will be nil, this is an no-op
+			updatedSvcs = append(updatedSvcs, unchangedSvcs...)
+			unchangedSvcs = nil
+		}
 		s.services.updateServices(key, cs)
 	case model.EventDelete:
 		deletedSvcs = cs
diff --git a/pkg/config/analysis/analyzers/util/proxyconfig.go b/pkg/config/analysis/analyzers/util/proxyconfig.go
index 7bb2446a61..3480a1ba56 100644
--- a/pkg/config/analysis/analyzers/util/proxyconfig.go
+++ b/pkg/config/analysis/analyzers/util/proxyconfig.go
@@ -54,7 +54,7 @@ func (e *EffectiveProxyConfigResolver) ImageType(pod *resource.Instance) string
 		if !strings.HasPrefix(k, pod.Metadata.FullName.Namespace.String()) {
 			continue
 		}
-		if maps.Contains(v.GetSelector().GetMatchLabels(), pod.Metadata.Labels) {
+		if maps.Contains(pod.Metadata.Labels, v.GetSelector().GetMatchLabels()) {
 			if v.GetImage().GetImageType() != "" {
 				variant = v.GetImage().GetImageType()
 			}
diff --git a/releasenotes/notes/50060.yaml b/releasenotes/notes/50060.yaml
new file mode 100644
index 0000000000..86ca12c8f3
--- /dev/null
+++ b/releasenotes/notes/50060.yaml
@@ -0,0 +1,8 @@
+apiVersion: release-notes/v2
+kind: bug-fix
+area: installation
+issue:
+  - 50060
+releaseNotes:
+- |
+  **Fixed** Gathering `pprof` data from the local ztunnel admin endpoint would fail due to lack of writable in-container `/tmp`
diff --git a/releasenotes/notes/se-target-port.yaml b/releasenotes/notes/se-target-port.yaml
new file mode 100644
index 0000000000..5fd4198a9b
--- /dev/null
+++ b/releasenotes/notes/se-target-port.yaml
@@ -0,0 +1,8 @@
+apiVersion: release-notes/v2
+kind: bug-fix
+area: traffic-management
+issue:
+- https://github.com/istio/istio/issues/49878
+releaseNotes:
+- |
+  **Fixed** an issue where updating a ServiceEntry's TargetPort does not trigger an xDS push.


Test patch:
diff --git a/pilot/pkg/serviceregistry/serviceentry/controller_test.go b/pilot/pkg/serviceregistry/serviceentry/controller_test.go
index a16f5877d2..30a3857aae 100644
--- a/pilot/pkg/serviceregistry/serviceentry/controller_test.go
+++ b/pilot/pkg/serviceregistry/serviceentry/controller_test.go
@@ -371,6 +371,42 @@ func TestServiceDiscoveryServiceUpdate(t *testing.T) {
 			Event{Type: "xds full", ID: "*.google.com"})
 	})
 
+	t.Run("change target port", func(t *testing.T) {
+		// Change the target port
+		targetPortChanged := func() *config.Config {
+			c := httpStaticOverlayUpdated.DeepCopy()
+			c.Spec.(*networking.ServiceEntry).Ports[0].TargetPort = 33333
+			return &c
+		}()
+		createConfigs([]*config.Config{targetPortChanged}, store, t)
+
+		// Endpoint ports should be changed
+		instances := append(baseInstances,
+			makeInstance(httpStaticOverlay, "5.5.5.5", 33333, httpStaticOverlay.Spec.(*networking.ServiceEntry).Ports[0], map[string]string{"overlay": "bar"}, PlainText),
+			makeInstance(httpStaticOverlay, "6.6.6.6", 33333, httpStaticOverlay.Spec.(*networking.ServiceEntry).Ports[0], map[string]string{"other": "bar"}, PlainText))
+		expectServiceInstances(t, sd, targetPortChanged, 0, instances)
+
+		// Expect a full push, as the target port has changed
+		expectEvents(t, events,
+			Event{Type: "service", ID: "*.google.com", Namespace: httpStaticOverlayUpdated.Namespace},
+			Event{Type: "eds cache", ID: "*.google.com", Namespace: httpStaticOverlayUpdated.Namespace},
+			Event{Type: "xds full", ID: "*.google.com"})
+
+		// Restore the target port
+		createConfigs([]*config.Config{httpStaticOverlayUpdated}, store, t)
+
+		// Endpoint ports should be changed
+		instances = append(baseInstances,
+			makeInstance(httpStaticOverlay, "5.5.5.5", 4567, httpStaticOverlay.Spec.(*networking.ServiceEntry).Ports[0], map[string]string{"overlay": "bar"}, PlainText),
+			makeInstance(httpStaticOverlay, "6.6.6.6", 4567, httpStaticOverlay.Spec.(*networking.ServiceEntry).Ports[0], map[string]string{"other": "bar"}, PlainText))
+		expectServiceInstances(t, sd, targetPortChanged, 0, instances)
+		// Expect a full push, as the target port has changed
+		expectEvents(t, events,
+			Event{Type: "service", ID: "*.google.com", Namespace: httpStaticOverlayUpdated.Namespace},
+			Event{Type: "eds cache", ID: "*.google.com", Namespace: httpStaticOverlayUpdated.Namespace},
+			Event{Type: "xds full", ID: "*.google.com"})
+	})
+
 	t.Run("change host", func(t *testing.T) {
 		// same as httpStaticOverlayUpdated but with an additional host
 		httpStaticHost := func() *config.Config {