Merge tag '1.24.1' into tetratefips-release-1.24

Istio release 1.24.1

Makefile.core.mk

@@ -49,7 +49,7 @@ endif
 export VERSION
 
 # Base version of Istio image to use
-BASE_VERSION ?= master-2024-09-19T19-01-03
+BASE_VERSION ?= 1.24-2024-11-19T19-01-56
 ISTIO_BASE_REGISTRY ?= gcr.io/istio-release
 
 export GO111MODULE ?= on

cni/pkg/iptables/iptables.go

@@ -144,14 +144,14 @@ func (cfg *IptablesConfigurator) executeDeleteCommands() error {
 	deleteCmds := [][]string{
 		{"-t", iptablesconstants.MANGLE, "-D", iptablesconstants.PREROUTING, "-j", ChainInpodPrerouting},
 		{"-t", iptablesconstants.MANGLE, "-D", iptablesconstants.OUTPUT, "-j", ChainInpodOutput},
-		{"-t", iptablesconstants.NAT, "-D", iptablesconstants.PREROUTING, "-j", ChainInpodPrerouting},
 		{"-t", iptablesconstants.NAT, "-D", iptablesconstants.OUTPUT, "-j", ChainInpodOutput},
-		{"-t", iptablesconstants.RAW, "-D", iptablesconstants.PREROUTING, "-j", ChainInpodPrerouting},
-		{"-t", iptablesconstants.RAW, "-D", iptablesconstants.OUTPUT, "-j", ChainInpodOutput},
 	}
 
-	// these sometimes fail due to "Device or resource busy"
+	// these sometimes fail due to "Device or resource busy" or because they are optional given the iptables cfg
 	optionalDeleteCmds := [][]string{
+		{"-t", iptablesconstants.RAW, "-D", iptablesconstants.PREROUTING, "-j", ChainInpodPrerouting},
+		{"-t", iptablesconstants.RAW, "-D", iptablesconstants.OUTPUT, "-j", ChainInpodOutput},
+		{"-t", iptablesconstants.NAT, "-D", iptablesconstants.PREROUTING, "-j", ChainInpodPrerouting},
 		// flush-then-delete our created chains
 		{"-t", iptablesconstants.MANGLE, "-F", ChainInpodPrerouting},
 		{"-t", iptablesconstants.MANGLE, "-F", ChainInpodOutput},
@@ -182,10 +182,7 @@ func (cfg *IptablesConfigurator) executeDeleteCommands() error {
 		}
 
 		for _, cmd := range optionalDeleteCmds {
-			err := cfg.ext.Run(iptablesconstants.IPTables, &iptVer, nil, cmd...)
-			if err != nil {
-				log.Debugf("ignoring error deleting optional iptables rule: %v", err)
-			}
+			cfg.ext.RunQuietlyAndIgnore(iptablesconstants.IPTables, &iptVer, nil, cmd...)
 		}
 	}
 	return errors.Join(delErrs...)

istio.deps

@@ -4,13 +4,13 @@
     "name": "PROXY_REPO_SHA",
     "repoName": "proxy",
     "file": "",
-    "lastStableSHA": "739644f84930a8c0d416319aea97f58c2222f7ef"
+    "lastStableSHA": "147cca4e7da4e8b3f8006e9fe3d8b3d6abd89462"
   },
   {
     "_comment": "",
     "name": "ZTUNNEL_REPO_SHA",
     "repoName": "ztunnel",
     "file": "",
-    "lastStableSHA": "1226c1b35f50938f428c71f7dcad3602ea991675"
+    "lastStableSHA": "4c7cdf1b62ddcc786402499c03eff0d5172c95ef"
   }
 ]

manifests/charts/istio-cni/templates/daemonset.yaml

@@ -41,12 +41,21 @@ spec:
         prometheus.io/scrape: 'true'
         prometheus.io/port: "15014"
         prometheus.io/path: '/metrics'
+        # Add AppArmor annotation
+        # This is required to avoid conflicts with AppArmor profiles which block certain
+        # privileged pod capabilities.
+        # Required for Kubernetes 1.29 which does not support setting appArmorProfile in the
+        # securityContext which is otherwise preferred.
+        container.apparmor.security.beta.kubernetes.io/install-cni: unconfined
         # Custom annotations
         {{- if .Values.podAnnotations }}
 {{ toYaml .Values.podAnnotations | indent 8 }}
         {{- end }}
     spec:
-      {{if .Values.ambient.enabled }}hostNetwork: true{{ end }}
+{{if .Values.ambient.enabled }}
+      hostNetwork: true
+      dnsPolicy: ClusterFirstWithHostNet
+{{ end }}
       nodeSelector:
         kubernetes.io/os: linux
       # Can be configured to allow for excluding istio-cni from being scheduled on specified nodes

operator/cmd/mesh/manifest-generate_test.go

@@ -817,6 +817,18 @@ func TestLDFlags(t *testing.T) {
 	assert.Equal(t, vals.GetPathString("spec.tag"), version.DockerInfo.Tag)
 }
 
+// TestManifestGenerateStructure makes some basic assertions about the structure of GeneratedManifests output.
+// This is to ensure that we only generate a single ManifestSet per component-type (in this case ingress gateways).
+// prevent an `istioctl install` regression of https://github.com/istio/istio/issues/53875
+func TestManifestGenerateStructure(t *testing.T) {
+	multiGatewayFile := filepath.Join(testDataDir, "input/gateways.yaml")
+	sets, _, err := render.GenerateManifest([]string{multiGatewayFile}, []string{}, false, nil, nil)
+	assert.NoError(t, err)
+	assert.Equal(t, len(sets), 1) // if this produces more than 1 ManifestSet it will cause a deadlock during install
+	gateways := sets[0].Manifests
+	assert.Equal(t, len(gateways), 21) // 7 kube resources * 3 gateways
+}
+
 func runTestGroup(t *testing.T, tests testGroup) {
 	for _, tt := range tests {
 		tt := tt

operator/pkg/helm/helm.go

@@ -46,7 +46,7 @@ func Render(namespace string, directory string, iop values.Map, kubernetesVersio
 	vals, _ := iop.GetPathMap("spec.values")
 	installPackagePath := iop.GetPathString("spec.installPackagePath")
 	f := manifests.BuiltinOrDir(installPackagePath)
-	path := filepath.Join("charts", directory)
+	path := pathJoin("charts", directory)
 	chrt, err := loadChart(f, path)
 	if err != nil {
 		return nil, nil, fmt.Errorf("load chart: %v", err)

operator/pkg/helm/path.go

@@ -0,0 +1,25 @@
+//go:build !windows
+
+// Copyright Istio Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package helm
+
+import (
+	"path/filepath"
+)
+
+func pathJoin(elem ...string) string {
+	return filepath.Join(elem...)
+}

operator/pkg/helm/path_windows.go

@@ -0,0 +1,25 @@
+//go:build windows
+
+// Copyright Istio Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package helm
+
+import "strings"
+
+func pathJoin(elem ...string) string {
+	elems := make([]string, len(elem))
+	elems = append(elems, elem...)
+	return strings.Join(elems, "/")
+}

operator/pkg/render/manifest.go

@@ -65,7 +65,7 @@ func GenerateManifest(files []string, setFlags []string, force bool, client kube
 	}
 
 	// Render each component
-	var allManifests []manifest.ManifestSet
+	allManifests := map[component.Name]manifest.ManifestSet{}
 	var chartWarnings util.Errors
 	for _, comp := range component.AllComponents {
 		specs, err := comp.Get(merged)
@@ -86,10 +86,16 @@ func GenerateManifest(files []string, setFlags []string, force bool, client kube
 			if err != nil {
 				return nil, nil, fmt.Errorf("post processing: %v", err)
 			}
-			allManifests = append(allManifests, manifest.ManifestSet{
-				Component: comp.UserFacingName,
-				Manifests: finalized,
-			})
+			manifests, found := allManifests[comp.UserFacingName]
+			if found {
+				manifests.Manifests = append(manifests.Manifests, finalized...)
+				allManifests[comp.UserFacingName] = manifests
+			} else {
+				allManifests[comp.UserFacingName] = manifest.ManifestSet{
+					Component: comp.UserFacingName,
+					Manifests: finalized,
+				}
+			}
 		}
 	}
 
@@ -99,7 +105,14 @@ func GenerateManifest(files []string, setFlags []string, force bool, client kube
 			logger.LogAndErrorf("%s %v", "❗", w)
 		}
 	}
-	return allManifests, merged, nil
+
+	values := make([]manifest.ManifestSet, 0, len(allManifests))
+
+	for _, v := range allManifests {
+		values = append(values, v)
+	}
+
+	return values, merged, nil
 }
 
 type MigrationResult struct {

pilot/pkg/model/service.go

@@ -887,15 +887,34 @@ type AmbientIndexes interface {
 type WaypointKey struct {
 	Namespace string
 	Hostnames []string
+
+	Network   string
+	Addresses []string
 }
 
 // WaypointKeyForProxy builds a key from a proxy to lookup
 func WaypointKeyForProxy(node *Proxy) WaypointKey {
 	key := WaypointKey{
 		Namespace: node.ConfigNamespace,
+		Network:   node.Metadata.Network.String(),
 	}
 	for _, svct := range node.ServiceTargets {
 		key.Hostnames = append(key.Hostnames, svct.Service.Hostname.String())
+
+		ips := svct.Service.ClusterVIPs.GetAddressesFor(node.GetClusterID())
+		// if we find autoAllocated addresses then ips should contain constants.UnspecifiedIP which should not be used
+		foundAutoAllocated := false
+		if svct.Service.AutoAllocatedIPv4Address != "" {
+			key.Addresses = append(key.Addresses, svct.Service.AutoAllocatedIPv4Address)
+			foundAutoAllocated = true
+		}
+		if svct.Service.AutoAllocatedIPv6Address != "" {
+			key.Addresses = append(key.Addresses, svct.Service.AutoAllocatedIPv6Address)
+			foundAutoAllocated = true
+		}
+		if !foundAutoAllocated {
+			key.Addresses = append(key.Addresses, ips...)
+		}
 	}
 	return key
 }

pilot/pkg/networking/core/cluster_builder.go

@@ -43,7 +43,6 @@ import (
 	"istio.io/istio/pkg/config/host"
 	"istio.io/istio/pkg/log"
 	"istio.io/istio/pkg/security"
-	"istio.io/istio/pkg/util/protomarshal"
 	"istio.io/istio/pkg/util/sets"
 )
 
@@ -323,7 +322,7 @@ func (cb *ClusterBuilder) buildCluster(name string, discoveryType cluster.Cluste
 				c.DnsLookupFamily = cluster.Cluster_V4_ONLY
 			}
 		}
-		c.DnsRefreshRate = protomarshal.ShallowClone(cb.req.Push.Mesh.DnsRefreshRate)
+		c.DnsRefreshRate = cb.req.Push.Mesh.DnsRefreshRate
 		c.RespectDnsTtl = true
 		// we want to run all the STATIC parts as well to build the load assignment
 		fallthrough
@@ -514,7 +513,7 @@ func (cb *ClusterBuilder) buildBlackHoleCluster() *cluster.Cluster {
 	c := &cluster.Cluster{
 		Name:                 util.BlackHoleCluster,
 		ClusterDiscoveryType: &cluster.Cluster_Type{Type: cluster.Cluster_STATIC},
-		ConnectTimeout:       protomarshal.ShallowClone(cb.req.Push.Mesh.ConnectTimeout),
+		ConnectTimeout:       cb.req.Push.Mesh.ConnectTimeout,
 		LbPolicy:             cluster.Cluster_ROUND_ROBIN,
 	}
 	c.AltStatName = util.DelimitedStatsPrefix(util.BlackHoleCluster)
@@ -527,7 +526,7 @@ func (cb *ClusterBuilder) buildDefaultPassthroughCluster() *cluster.Cluster {
 	cluster := &cluster.Cluster{
 		Name:                 util.PassthroughCluster,
 		ClusterDiscoveryType: &cluster.Cluster_Type{Type: cluster.Cluster_ORIGINAL_DST},
-		ConnectTimeout:       protomarshal.ShallowClone(cb.req.Push.Mesh.ConnectTimeout),
+		ConnectTimeout:       cb.req.Push.Mesh.ConnectTimeout,
 		LbPolicy:             cluster.Cluster_CLUSTER_PROVIDED,
 		TypedExtensionProtocolOptions: map[string]*anypb.Any{
 			v3.HttpProtocolOptionsType: passthroughHttpProtocolOptions,
@@ -734,7 +733,7 @@ func (cb *ClusterBuilder) buildExternalSDSCluster(addr string) *cluster.Cluster
 	c := &cluster.Cluster{
 		Name:                 security.SDSExternalClusterName,
 		ClusterDiscoveryType: &cluster.Cluster_Type{Type: cluster.Cluster_STATIC},
-		ConnectTimeout:       protomarshal.ShallowClone(cb.req.Push.Mesh.ConnectTimeout),
+		ConnectTimeout:       cb.req.Push.Mesh.ConnectTimeout,
 		LoadAssignment: &endpoint.ClusterLoadAssignment{
 			ClusterName: security.SDSExternalClusterName,
 			Endpoints: []*endpoint.LocalityLbEndpoints{

pilot/pkg/networking/core/listener_waypoint.go

@@ -628,7 +628,7 @@ func (lb *ListenerBuilder) translateWaypointRoute(
 ) *route.Route {
 	gatewaySemantics := model.UseGatewaySemantics(virtualService)
 	// When building routes, it's okay if the target cluster cannot be
-	// resolved Traffic to such clusters will blackhole.
+	// resolved. Traffic to such clusters will blackhole.
 
 	// Match by the destination port specified in the match condition
 	if match != nil && match.Port != 0 && match.Port != uint32(listenPort) {
@@ -701,9 +701,15 @@ func (lb *ListenerBuilder) waypointRouteDestination(
 		action.Timeout = in.Timeout
 	}
 	// Use deprecated value for now as the replacement MaxStreamDuration has some regressions.
+	// TODO: check and see if the replacement has been fixed.
 	// nolint: staticcheck
 	action.MaxGrpcTimeout = action.Timeout
 
+	if gatewaySemantics {
+		// return 500 for invalid backends
+		// https://github.com/kubernetes-sigs/gateway-api/blob/cea484e38e078a2c1997d8c7a62f410a1540f519/apis/v1beta1/httproute_types.go#L204
+		action.ClusterNotFoundResponseCode = route.RouteAction_INTERNAL_SERVER_ERROR
+	}
 	out.Action = &route.Route_Route{Route: action}
 
 	if in.Rewrite != nil {
@@ -814,9 +820,13 @@ func (lb *ListenerBuilder) waypointRouteDestination(
 	}
 }
 
-// getWaypointDestinationCluster generates a cluster name for the route, or error if no cluster
-// can be found. Called by translateRule to determine if
+// getWaypointDestinationCluster generates a cluster name for the route. If the destination is invalid
+// or cannot be found, "UnknownService" is returned.
 func (lb *ListenerBuilder) getWaypointDestinationCluster(destination *networking.Destination, service *model.Service, listenerPort int) string {
+	if len(destination.GetHost()) == 0 {
+		// only happens when the gateway-api BackendRef is invalid
+		return "UnknownService"
+	}
 	dir, port := model.TrafficDirectionInboundVIP, listenerPort
 
 	if destination.GetPort() != nil {
@@ -851,10 +861,14 @@ func (lb *ListenerBuilder) getWaypointDestinationCluster(destination *networking
 
 // portToSubset helps translate a port to the waypoint subset to use
 func portToSubset(service *model.Service, port int, destination *networking.Destination) string {
-	p, ok := service.Ports.GetByPort(port)
+	var p *model.Port
+	var ok bool
+	if service != nil {
+		p, ok = service.Ports.GetByPort(port)
+	}
 	if !ok {
 		// Port is unknown.
-		if destination.Subset != "" {
+		if destination != nil && destination.Subset != "" {
 			return "http/" + destination.Subset
 		}
 		return "http"

pilot/pkg/networking/core/route/route.go

@@ -329,8 +329,8 @@ func buildSidecarVirtualHostForService(svc *model.Service,
 	}
 }
 
-// GetDestinationCluster generates a cluster name for the route, or error if no cluster
-// can be found. Called by translateRule to determine if
+// GetDestinationCluster generates a cluster name for the route. If the destination is invalid
+// or cannot be found, "UnknownService" is returned.
 func GetDestinationCluster(destination *networking.Destination, service *model.Service, listenerPort int) string {
 	if len(destination.GetHost()) == 0 {
 		// only happens when the gateway-api BackendRef is invalid
@@ -790,7 +790,12 @@ func ApplyRedirect(out *route.Route, redirect *networking.HTTPRedirect, port int
 		action.Redirect.ResponseCode = route.RedirectAction_PERMANENT_REDIRECT
 	default:
 		log.Warnf("Redirect Code %d is not yet supported", redirect.RedirectCode)
-		action = nil
+		// Can't just set action to nil here because the proto marshaller will still see
+		// the Route_Redirect type of the variable and assume that the value is set
+		// (and panic because it's not). What we need to do is set out.Action directly to
+		// (a typeless) nil so that type assertions to Route_Redirect will fail.
+		out.Action = nil
+		return
 	}
 
 	out.Action = action