diff --git a/Makefile.core.mk b/Makefile.core.mk index 3d6ce2e342da..e2b847643194 100644 --- a/Makefile.core.mk +++ b/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 diff --git a/cni/pkg/iptables/iptables.go b/cni/pkg/iptables/iptables.go index b53910ccf470..ebc07c8df636 100644 --- a/cni/pkg/iptables/iptables.go +++ b/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...) diff --git a/istio.deps b/istio.deps index cba5ede8277f..de5fe1895174 100644 --- a/istio.deps +++ b/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" } ] diff --git a/manifests/charts/istio-cni/templates/daemonset.yaml b/manifests/charts/istio-cni/templates/daemonset.yaml index f7d2962e2d64..2d93c59cd3ce 100644 --- a/manifests/charts/istio-cni/templates/daemonset.yaml +++ b/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 diff --git a/operator/cmd/mesh/manifest-generate_test.go b/operator/cmd/mesh/manifest-generate_test.go index cc220572683d..75a72b858487 100644 --- a/operator/cmd/mesh/manifest-generate_test.go +++ b/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 diff --git a/operator/pkg/helm/helm.go b/operator/pkg/helm/helm.go index da4b37c705f1..222855743f3e 100644 --- a/operator/pkg/helm/helm.go +++ b/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) diff --git a/operator/pkg/helm/path.go b/operator/pkg/helm/path.go new file mode 100644 index 000000000000..90253d8a25a7 --- /dev/null +++ b/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...) +} diff --git a/operator/pkg/helm/path_windows.go b/operator/pkg/helm/path_windows.go new file mode 100644 index 000000000000..27a8b8daeb21 --- /dev/null +++ b/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, "/") +} diff --git a/operator/pkg/render/manifest.go b/operator/pkg/render/manifest.go index c3513f9cafb3..8153590302b8 100644 --- a/operator/pkg/render/manifest.go +++ b/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 { diff --git a/pilot/pkg/model/service.go b/pilot/pkg/model/service.go index bcc86b0fa40d..9907e8ee760f 100644 --- a/pilot/pkg/model/service.go +++ b/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 } diff --git a/pilot/pkg/networking/core/cluster_builder.go b/pilot/pkg/networking/core/cluster_builder.go index 314f50baa5a7..2082555b6279 100644 --- a/pilot/pkg/networking/core/cluster_builder.go +++ b/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{ diff --git a/pilot/pkg/networking/core/listener_waypoint.go b/pilot/pkg/networking/core/listener_waypoint.go index 3fd2b3a0721d..bd74bb6661d6 100644 --- a/pilot/pkg/networking/core/listener_waypoint.go +++ b/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" diff --git a/pilot/pkg/networking/core/route/route.go b/pilot/pkg/networking/core/route/route.go index 4778bbf3ff90..2397e9d0f328 100644 --- a/pilot/pkg/networking/core/route/route.go +++ b/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 diff --git a/pilot/pkg/networking/core/route/route_test.go b/pilot/pkg/networking/core/route/route_test.go index a42c5d0ece81..055bc40cb89e 100644 --- a/pilot/pkg/networking/core/route/route_test.go +++ b/pilot/pkg/networking/core/route/route_test.go @@ -933,6 +933,19 @@ func TestBuildHTTPRoutes(t *testing.T) { g.Expect(redirectAction.Redirect.ResponseCode).To(Equal(envoyroute.RedirectAction_PERMANENT_REDIRECT)) }) + t.Run("for invalid redirect code", func(t *testing.T) { + g := NewWithT(t) + cg := core.NewConfigGenTest(t, core.TestOptions{}) + + routes, err := route.BuildHTTPRoutesForVirtualService(node(cg), virtualServiceWithInvalidRedirect, serviceRegistry, + nil, 8080, gatewayNames, route.RouteOptions{}) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(len(routes)).To(Equal(1)) + + _, ok := routes[0].Action.(*envoyroute.Route_Redirect) + g.Expect(ok).To(BeFalse()) + }) + t.Run("for path prefix redirect", func(t *testing.T) { g := NewWithT(t) cg := core.NewConfigGenTest(t, core.TestOptions{}) @@ -1862,6 +1875,26 @@ var virtualServiceWithRedirect = config.Config{ }, } +var virtualServiceWithInvalidRedirect = config.Config{ + Meta: config.Meta{ + GroupVersionKind: gvk.VirtualService, + Name: "acme", + }, + Spec: &networking.VirtualService{ + Hosts: []string{}, + Gateways: []string{"some-gateway"}, + Http: []*networking.HTTPRoute{ + { + Redirect: &networking.HTTPRedirect{ + Uri: "example.org", + Authority: "some-authority.default.svc.cluster.local", + RedirectCode: 317, + }, + }, + }, + }, +} + var virtualServiceWithRedirectPathPrefix = config.Config{ Meta: config.Meta{ GroupVersionKind: gvk.VirtualService, diff --git a/pilot/pkg/serviceregistry/kube/controller/ambient/ambientindex.go b/pilot/pkg/serviceregistry/kube/controller/ambient/ambientindex.go index 0673a6502a92..47e9f067eacf 100644 --- a/pilot/pkg/serviceregistry/kube/controller/ambient/ambientindex.go +++ b/pilot/pkg/serviceregistry/kube/controller/ambient/ambientindex.go @@ -75,9 +75,10 @@ func (n NamespaceHostname) String() string { type workloadsCollection struct { krt.Collection[model.WorkloadInfo] - ByAddress krt.Index[networkAddress, model.WorkloadInfo] - ByServiceKey krt.Index[string, model.WorkloadInfo] - ByOwningWaypoint krt.Index[NamespaceHostname, model.WorkloadInfo] + ByAddress krt.Index[networkAddress, model.WorkloadInfo] + ByServiceKey krt.Index[string, model.WorkloadInfo] + ByOwningWaypointHostname krt.Index[NamespaceHostname, model.WorkloadInfo] + ByOwningWaypointIP krt.Index[networkAddress, model.WorkloadInfo] } type waypointsCollection struct { @@ -86,8 +87,9 @@ type waypointsCollection struct { type servicesCollection struct { krt.Collection[model.ServiceInfo] - ByAddress krt.Index[networkAddress, model.ServiceInfo] - ByOwningWaypoint krt.Index[NamespaceHostname, model.ServiceInfo] + ByAddress krt.Index[networkAddress, model.ServiceInfo] + ByOwningWaypointHostname krt.Index[NamespaceHostname, model.ServiceInfo] + ByOwningWaypointIP krt.Index[networkAddress, model.ServiceInfo] } // index maintains an index of ambient WorkloadInfo objects by various keys. @@ -230,7 +232,7 @@ func New(options Options) Index { } ServiceAddressIndex := krt.NewIndex[networkAddress, model.ServiceInfo](WorkloadServices, networkAddressFromService) - ServiceInfosByOwningWaypoint := krt.NewIndex(WorkloadServices, func(s model.ServiceInfo) []NamespaceHostname { + ServiceInfosByOwningWaypointHostname := krt.NewIndex(WorkloadServices, func(s model.ServiceInfo) []NamespaceHostname { // Filter out waypoint services if s.Labels[label.GatewayManaged.Name] == constants.ManagedGatewayMeshControllerLabel { return nil @@ -249,6 +251,27 @@ func New(options Options) Index { Hostname: waypointAddress.Hostname, }} }) + ServiceInfosByOwningWaypointIP := krt.NewIndex(WorkloadServices, func(s model.ServiceInfo) []networkAddress { + // Filter out waypoint services + if s.Labels[label.GatewayManaged.Name] == constants.ManagedGatewayMeshControllerLabel { + return nil + } + waypoint := s.Service.Waypoint + if waypoint == nil { + return nil + } + waypointAddress := waypoint.GetAddress() + if waypointAddress == nil { + return nil + } + netip, _ := netip.AddrFromSlice(waypointAddress.Address) + netaddr := networkAddress{ + network: waypointAddress.Network, + ip: netip.String(), + } + + return []networkAddress{netaddr} + }) WorkloadServices.RegisterBatch(krt.BatchedEventFilter( func(a model.ServiceInfo) *workloadapi.Service { // Only trigger push if the XDS object changed; the rest is just for computation of others @@ -276,7 +299,7 @@ func New(options Options) Index { WorkloadServiceIndex := krt.NewIndex[string, model.WorkloadInfo](Workloads, func(o model.WorkloadInfo) []string { return maps.Keys(o.Services) }) - WorkloadWaypointIndex := krt.NewIndex(Workloads, func(w model.WorkloadInfo) []NamespaceHostname { + WorkloadWaypointIndexHostname := krt.NewIndex(Workloads, func(w model.WorkloadInfo) []NamespaceHostname { // Filter out waypoints. if w.Labels[label.GatewayManaged.Name] == constants.ManagedGatewayMeshControllerLabel { return nil @@ -295,6 +318,28 @@ func New(options Options) Index { Hostname: waypointAddress.Hostname, }} }) + WorkloadWaypointIndexIP := krt.NewIndex(Workloads, func(w model.WorkloadInfo) []networkAddress { + // Filter out waypoints. + if w.Labels[label.GatewayManaged.Name] == constants.ManagedGatewayMeshControllerLabel { + return nil + } + waypoint := w.Waypoint + if waypoint == nil { + return nil + } + + waypointAddress := waypoint.GetAddress() + if waypointAddress == nil { + return nil + } + netip, _ := netip.AddrFromSlice(waypointAddress.Address) + netaddr := networkAddress{ + network: waypointAddress.Network, + ip: netip.String(), + } + + return []networkAddress{netaddr} + }) Workloads.RegisterBatch(krt.BatchedEventFilter( func(a model.WorkloadInfo) *workloadapi.Workload { // Only trigger push if the XDS object changed; the rest is just for computation of others @@ -315,15 +360,17 @@ func New(options Options) Index { } a.workloads = workloadsCollection{ - Collection: Workloads, - ByAddress: WorkloadAddressIndex, - ByServiceKey: WorkloadServiceIndex, - ByOwningWaypoint: WorkloadWaypointIndex, + Collection: Workloads, + ByAddress: WorkloadAddressIndex, + ByServiceKey: WorkloadServiceIndex, + ByOwningWaypointHostname: WorkloadWaypointIndexHostname, + ByOwningWaypointIP: WorkloadWaypointIndexIP, } a.services = servicesCollection{ - Collection: WorkloadServices, - ByAddress: ServiceAddressIndex, - ByOwningWaypoint: ServiceInfosByOwningWaypoint, + Collection: WorkloadServices, + ByAddress: ServiceAddressIndex, + ByOwningWaypointHostname: ServiceInfosByOwningWaypointHostname, + ByOwningWaypointIP: ServiceInfosByOwningWaypointIP, } a.waypoints = waypointsCollection{ Collection: Waypoints, @@ -462,26 +509,60 @@ func (a *index) AddressInformation(addresses sets.String) ([]model.AddressInfo, } func (a *index) ServicesForWaypoint(key model.WaypointKey) []model.ServiceInfo { - var out []model.ServiceInfo + out := map[string]model.ServiceInfo{} for _, host := range key.Hostnames { - out = append(out, a.services.ByOwningWaypoint.Lookup(NamespaceHostname{ + for _, res := range a.services.ByOwningWaypointHostname.Lookup(NamespaceHostname{ Namespace: key.Namespace, Hostname: host, - })...) + }) { + name := res.ResourceName() + if _, f := out[name]; !f { + out[name] = res + } + } } - return out + + for _, addr := range key.Addresses { + for _, res := range a.services.ByOwningWaypointIP.Lookup(networkAddress{ + network: key.Network, + ip: addr, + }) { + name := res.ResourceName() + if _, f := out[name]; !f { + out[name] = res + } + } + } + // Response is unsorted; it is up to the caller to sort + return maps.Values(out) } func (a *index) WorkloadsForWaypoint(key model.WaypointKey) []model.WorkloadInfo { - var out []model.WorkloadInfo + out := map[string]model.WorkloadInfo{} for _, host := range key.Hostnames { - out = append(out, a.workloads.ByOwningWaypoint.Lookup(NamespaceHostname{ + for _, res := range a.workloads.ByOwningWaypointHostname.Lookup(NamespaceHostname{ Namespace: key.Namespace, Hostname: host, - })...) + }) { + name := res.ResourceName() + if _, f := out[name]; !f { + out[name] = res + } + } + } + + for _, addr := range key.Addresses { + for _, res := range a.workloads.ByOwningWaypointIP.Lookup(networkAddress{ + network: key.Network, + ip: addr, + }) { + name := res.ResourceName() + if _, f := out[name]; !f { + out[name] = res + } + } } - out = model.SortWorkloadsByCreationTime(out) - return out + return model.SortWorkloadsByCreationTime(maps.Values(out)) } func (a *index) AdditionalPodSubscriptions( diff --git a/pilot/pkg/serviceregistry/kube/controller/ambient/ambientindex_test.go b/pilot/pkg/serviceregistry/kube/controller/ambient/ambientindex_test.go index 85bdf93bc6ff..807819f6d728 100644 --- a/pilot/pkg/serviceregistry/kube/controller/ambient/ambientindex_test.go +++ b/pilot/pkg/serviceregistry/kube/controller/ambient/ambientindex_test.go @@ -608,6 +608,79 @@ func TestAmbientIndex_WaypointInboundBinding(t *testing.T) { }) } +func TestAmbientIndex_ServicesForWaypoint(t *testing.T) { + wpKey := model.WaypointKey{ + Namespace: testNS, + Hostnames: []string{fmt.Sprintf("%s.%s.svc.company.com", "wp", testNS)}, + Addresses: []string{"10.0.0.1"}, + Network: testNW, + } + t.Run("hostname", func(t *testing.T) { + s := newAmbientTestServer(t, testC, testNW) + s.addService(t, "svc1", + map[string]string{label.IoIstioUseWaypoint.Name: "wp"}, + map[string]string{}, + []int32{80}, map[string]string{"app": "app1"}, "11.0.0.1") + s.assertEvent(s.t, s.svcXdsName("svc1")) + + s.addWaypointSpecificAddress(t, "", s.hostnameForService("wp"), "wp", constants.AllTraffic, true) + s.addService(t, "wp", + map[string]string{}, + map[string]string{}, + []int32{80}, map[string]string{"app": "waypoint"}, "10.0.0.2") + s.assertEvent(s.t, s.svcXdsName("svc1")) + + svc1Host := ptr.ToList(s.services.GetKey(krt.Key[model.ServiceInfo](fmt.Sprintf("%s/%s", testNS, s.hostnameForService("svc1"))))) + assert.Equal(t, len(svc1Host), 1) + assert.EventuallyEqual(t, func() []model.ServiceInfo { + return s.ServicesForWaypoint(wpKey) + }, svc1Host) + }) + t.Run("ip", func(t *testing.T) { + s := newAmbientTestServer(t, testC, testNW) + + s.addService(t, "svc1", + map[string]string{label.IoIstioUseWaypoint.Name: "wp"}, + map[string]string{}, + []int32{80}, map[string]string{"app": "app1"}, "11.0.0.1") + s.assertEvent(s.t, s.svcXdsName("svc1")) + + s.addWaypointSpecificAddress(t, "10.0.0.1", "", "wp", constants.AllTraffic, true) + s.addService(t, "wp", + map[string]string{}, + map[string]string{}, + []int32{80}, map[string]string{"app": "waypoint"}, "10.0.0.1") + s.assertEvent(s.t, s.svcXdsName("svc1")) + + svc1Host := ptr.ToList(s.services.GetKey(krt.Key[model.ServiceInfo](fmt.Sprintf("%s/%s", testNS, s.hostnameForService("svc1"))))) + assert.Equal(t, len(svc1Host), 1) + assert.EventuallyEqual(t, func() []model.ServiceInfo { + return s.ServicesForWaypoint(wpKey) + }, svc1Host) + }) + t.Run("mixed", func(t *testing.T) { + s := newAmbientTestServer(t, testC, testNW) + s.addService(t, "svc1", + map[string]string{label.IoIstioUseWaypoint.Name: "wp"}, + map[string]string{}, + []int32{80}, map[string]string{"app": "app1"}, "11.0.0.1") + s.assertEvent(s.t, s.svcXdsName("svc1")) + + s.addWaypointSpecificAddress(t, "10.0.0.1", s.hostnameForService("wp"), "wp", constants.AllTraffic, true) + s.addService(t, "wp", + map[string]string{}, + map[string]string{}, + []int32{80}, map[string]string{"app": "waypoint"}, "10.0.0.1") + s.assertEvent(s.t, s.svcXdsName("svc1")) + + svc1Host := ptr.ToList(s.services.GetKey(krt.Key[model.ServiceInfo](fmt.Sprintf("%s/%s", testNS, s.hostnameForService("svc1"))))) + assert.Equal(t, len(svc1Host), 1) + assert.EventuallyEqual(t, func() []model.ServiceInfo { + return s.ServicesForWaypoint(wpKey) + }, svc1Host) + }) +} + // define constants for the different types of XDS events which occur during policy unit tests const ( xdsConvertedPeerAuthSelector = "converted_peer_authentication_selector" diff --git a/pkg/kube/inject/webhook.go b/pkg/kube/inject/webhook.go index b4b209db9034..ebaf90a09478 100644 --- a/pkg/kube/inject/webhook.go +++ b/pkg/kube/inject/webhook.go @@ -218,10 +218,10 @@ func NewWebhook(p WebhookParameters) (*Webhook, error) { wh.MultiCast = mc sidecarConfig, valuesConfig, err := p.Watcher.Get() if err != nil { - return nil, err + return nil, fmt.Errorf("failed to get initial configuration: %v", err) } if err := wh.updateConfig(sidecarConfig, valuesConfig); err != nil { - log.Errorf("failed to process webhook config: %v", err) + return nil, fmt.Errorf("failed to process webhook config: %v", err) } p.Mux.HandleFunc("/inject", wh.serveInject) @@ -247,7 +247,7 @@ func (wh *Webhook) updateConfig(sidecarConfig *Config, valuesConfig string) erro wh.Config = sidecarConfig vc, err := NewValuesConfig(valuesConfig) if err != nil { - return err + return fmt.Errorf("failed to create new values config: %v", err) } wh.valuesConfig = vc return nil diff --git a/pkg/kube/inject/webhook_test.go b/pkg/kube/inject/webhook_test.go index 945deee98fd7..9b502c04e442 100644 --- a/pkg/kube/inject/webhook_test.go +++ b/pkg/kube/inject/webhook_test.go @@ -1377,3 +1377,68 @@ func defaultInstallPackageDir() string { } return filepath.Join(wd, "../../../manifests/") } + +func TestNewWebhookConfigParsingError(t *testing.T) { + // Create a watcher that returns valid sidecarConfig but invalid valuesConfig + faultyWatcher := &FaultyWatcher{ + sidecarConfig: &Config{}, + valuesConfig: "invalid: values: config", + } + + whParams := WebhookParameters{ + Watcher: faultyWatcher, + Port: 0, + Env: &model.Environment{}, + Mux: http.NewServeMux(), + } + + _, err := NewWebhook(whParams) + if err == nil || !strings.Contains(err.Error(), "failed to process webhook config") { + t.Fatalf("Expected error when creating webhook with faulty valuesConfig, but got: %v", err) + } +} + +// FaultyWatcher is a mock Watcher that returns predefined sidecarConfig and valuesConfig +type FaultyWatcher struct { + sidecarConfig *Config + valuesConfig string +} + +func (fw *FaultyWatcher) Run(stop <-chan struct{}) {} + +func (fw *FaultyWatcher) Get() (*Config, string, error) { + return fw.sidecarConfig, fw.valuesConfig, nil +} + +func (fw *FaultyWatcher) SetHandler(handler func(*Config, string) error) {} + +func TestNewWebhookConfigParsingSuccess(t *testing.T) { + // Create a watcher that returns valid sidecarConfig and valid valuesConfig + validValuesConfig := ` +global: + proxy: + image: proxyv2 +` + faultyWatcher := &FaultyWatcher{ + sidecarConfig: &Config{}, + valuesConfig: validValuesConfig, + } + + whParams := WebhookParameters{ + Watcher: faultyWatcher, + Port: 0, + Env: &model.Environment{ + Watcher: mesh.NewFixedWatcher(&meshconfig.MeshConfig{}), + }, + Mux: http.NewServeMux(), + } + + wh, err := NewWebhook(whParams) + if err != nil { + t.Fatalf("Expected no error when creating webhook with valid valuesConfig, but got: %v", err) + } + + if wh.valuesConfig.raw != validValuesConfig { + t.Fatalf("Expected valuesConfig to be set correctly, but got: %v", wh.valuesConfig.raw) + } +} diff --git a/pkg/proto/merge/merge.go b/pkg/proto/merge/merge.go index 6e00addcbd09..319cd89b75cc 100644 --- a/pkg/proto/merge/merge.go +++ b/pkg/proto/merge/merge.go @@ -32,7 +32,7 @@ import ( ) type ( - MergeFunction func(dst, src protoreflect.Message) + MergeFunction func(dst, src protoreflect.Message) protoreflect.Message mergeOptions struct { customMergeFn map[protoreflect.FullName]MergeFunction } @@ -46,16 +46,10 @@ func MergeFunctionOptionFn(name protoreflect.FullName, function MergeFunction) O } } -// ReplaceMergeFn instead of merging all subfields one by one, takes src and set it to dest -var ReplaceMergeFn MergeFunction = func(dst, src protoreflect.Message) { - dst.Range(func(fd protoreflect.FieldDescriptor, v protoreflect.Value) bool { - dst.Clear(fd) - return true - }) - src.Range(func(fd protoreflect.FieldDescriptor, v protoreflect.Value) bool { - dst.Set(fd, v) - return true - }) +// ReplaceMergeFn instead of merging all subfields one by one, returns src +var ReplaceMergeFn MergeFunction = func(dst, src protoreflect.Message) protoreflect.Message { + // we can return src directly because this is a replace + return src } var options = []OptionFn{ @@ -99,7 +93,8 @@ func (o mergeOptions) mergeMessage(dst, src protoreflect.Message) { case fd.Message() != nil: mergeFn, exists := o.customMergeFn[fd.Message().FullName()] if exists { - mergeFn(dst.Mutable(fd).Message(), v.Message()) + dstV := mergeFn(dst.Mutable(fd).Message(), v.Message()) + dst.Set(fd, protoreflect.ValueOf(dstV)) } else { o.mergeMessage(dst.Mutable(fd).Message(), v.Message()) } diff --git a/pkg/proto/merge/merge_test.go b/pkg/proto/merge/merge_test.go new file mode 100644 index 000000000000..da5880aeb71e --- /dev/null +++ b/pkg/proto/merge/merge_test.go @@ -0,0 +1,39 @@ +// 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 merge + +import ( + "testing" + + listener "github.com/envoyproxy/go-control-plane/envoy/config/listener/v3" + "google.golang.org/protobuf/types/known/durationpb" + + "istio.io/istio/pkg/test/util/assert" +) + +func TestMerge(t *testing.T) { + src := &durationpb.Duration{Seconds: 123, Nanos: 456} + dst := &durationpb.Duration{Seconds: 789, Nanos: 999} + + srcListener := &listener.Listener{ListenerFiltersTimeout: src} + dstListener := &listener.Listener{ListenerFiltersTimeout: dst} + + Merge(dstListener, srcListener) + + assert.Equal(t, dstListener.ListenerFiltersTimeout, src) + + // dst duration not changed after merge + assert.Equal(t, dst, &durationpb.Duration{Seconds: 789, Nanos: 999}) +} diff --git a/releasenotes/notes/53801.yaml b/releasenotes/notes/53801.yaml new file mode 100644 index 000000000000..3877989b62fe --- /dev/null +++ b/releasenotes/notes/53801.yaml @@ -0,0 +1,6 @@ +apiVersion: release-notes/v2 +kind: bug-fix +area: traffic-management +releaseNotes: + - | + **Fixed** merging Duration with a envoyfilter can lead to all listeners associated attributes unexpectedly modified becasue all listeners share a same pointer typed `listener_filters_timeout`. \ No newline at end of file diff --git a/releasenotes/notes/53829.yaml b/releasenotes/notes/53829.yaml new file mode 100644 index 000000000000..fd375a636f14 --- /dev/null +++ b/releasenotes/notes/53829.yaml @@ -0,0 +1,10 @@ +apiVersion: release-notes/v2 +kind: bug-fix +area: security +releaseNotes: +- | + **Added** unconfined AppArmor annotation to the istio-cni-node DaemonSet to avoid conflicts with + AppArmor profiles which block certain privileged pod capabilities. Previously, AppArmor + (when enabled) was bypassed for the istio-cni-node DaemonSet since privileged was set to true + in the SecurityContext. This change ensures that the AppArmor profile is set to unconfined + for the istio-cni-node DaemonSet. diff --git a/releasenotes/notes/53852.yaml b/releasenotes/notes/53852.yaml new file mode 100644 index 000000000000..238b1f7a8fc2 --- /dev/null +++ b/releasenotes/notes/53852.yaml @@ -0,0 +1,6 @@ +apiVersion: release-notes/v2 +kind: bug-fix +area: installation +releaseNotes: + - | + **Added** an issue that `istioctl install` not working on windows. diff --git a/releasenotes/notes/53861.yaml b/releasenotes/notes/53861.yaml new file mode 100644 index 000000000000..78366452677e --- /dev/null +++ b/releasenotes/notes/53861.yaml @@ -0,0 +1,6 @@ +apiVersion: release-notes/v2 +kind: bug-fix +area: installation +releaseNotes: + - | + **Added** a pod `dnsPolicy` of ClusterFirstWithHostNet to `istio-cni` when it runs with `hostNetwork=true` (i.e. ambient mode). diff --git a/releasenotes/notes/53880.yaml b/releasenotes/notes/53880.yaml new file mode 100644 index 000000000000..c1f6b6e7b5f2 --- /dev/null +++ b/releasenotes/notes/53880.yaml @@ -0,0 +1,9 @@ +apiVersion: release-notes/v2 +kind: bug-fix +area: istioctl +issue: + - 53875 +releaseNotes: +- | + **Fixed** an issue where `istioctl install` deadlocks if multiple ingress gateways are specified in the IstioOperator file + diff --git a/releasenotes/notes/53951.yaml b/releasenotes/notes/53951.yaml new file mode 100644 index 000000000000..3539792c1c0b --- /dev/null +++ b/releasenotes/notes/53951.yaml @@ -0,0 +1,6 @@ +apiVersion: release-notes/v2 +kind: bug-fix +area: traffic-management +releaseNotes: +- | + **Fixed** errors being raised during cleanup of iptables rules that are conditional on the iptables configuration. \ No newline at end of file diff --git a/releasenotes/notes/waypoint-revision.yaml b/releasenotes/notes/waypoint-revision.yaml new file mode 100644 index 000000000000..e985bfd17c16 --- /dev/null +++ b/releasenotes/notes/waypoint-revision.yaml @@ -0,0 +1,7 @@ +apiVersion: release-notes/v2 +kind: bug-fix +area: traffic-management +issue: [53883] +releaseNotes: + - | + **Fixed** an issue when upgrading waypoint proxies from Istio 1.23.x to Istio 1.24.x. diff --git a/tests/integration/ambient/baseline_test.go b/tests/integration/ambient/baseline_test.go index e47dc4856c05..aa811e18b9f7 100644 --- a/tests/integration/ambient/baseline_test.go +++ b/tests/integration/ambient/baseline_test.go @@ -2546,7 +2546,7 @@ spec: - match: metric: REQUEST_COUNT tagOverrides: - custom_dimension: + custom_dimension: value: "'test'" source_principal: operation: REMOVE @@ -3161,6 +3161,45 @@ func daemonsetsetComplete(ds *appsv1.DaemonSet) bool { ds.Status.ObservedGeneration >= ds.Generation } +func TestWaypointWithInvalidBackend(t *testing.T) { + framework.NewTest(t). + Run(func(t framework.TestContext) { + // We should expect a 500 error since the backend is invalid. + t.ConfigIstio(). + Eval(apps.Namespace.Name(), apps.Namespace.Name(), `apiVersion: gateway.networking.k8s.io/v1 +kind: HTTPRoute +metadata: + name: add-header +spec: + parentRefs: + - name: sidecar + kind: Service + group: "" + port: 80 + rules: + - filters: + - type: RequestHeaderModifier + requestHeaderModifier: + add: + - name: greeting + value: "hello world!" + backendRefs: + - name: invalid + port: 80 +`). + ApplyOrFail(t) + SetWaypoint(t, Sidecar, "waypoint") + client := apps.Captured + client[0].CallOrFail(t, echo.CallOptions{ + To: apps.Sidecar, + Port: ports.HTTP, + Check: check.And( + check.Status(500), + ), + }) + }) +} + func TestWaypointWithSidecarBackend(t *testing.T) { framework.NewTest(t). Run(func(t framework.TestContext) { diff --git a/tools/istio-iptables/pkg/dependencies/implementation_linux.go b/tools/istio-iptables/pkg/dependencies/implementation_linux.go index 288f53df9fe9..fa9d91aa8f3c 100644 --- a/tools/istio-iptables/pkg/dependencies/implementation_linux.go +++ b/tools/istio-iptables/pkg/dependencies/implementation_linux.go @@ -274,6 +274,9 @@ func (r *RealDependencies) executeXTablesWithOutput(cmd constants.IptablesCmd, i } log.Errorf("Command error output: %v", stderrStr) + } else if err != nil && ignoreErrors { + // Log ignored errors for debugging purposes + log.Debugf("Ignoring iptables command error: %v", err) } return stdout, err