Merge tag '1.23.4' into tetratefips-release-1.23

Istio release 1.23.4

Makefile.core.mk

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

istio.deps

@@ -4,7 +4,7 @@
     "name": "PROXY_REPO_SHA",
     "repoName": "proxy",
     "file": "",
-    "lastStableSHA": "cbd889517ed13455bf2d88facc5685d958eb54a6"
+    "lastStableSHA": "5cc45f0759f8cf837ac330b62407dee13ecbba89"
   },
   {
     "_comment": "",

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

@@ -27,3 +27,8 @@ data:
   REPAIR_INIT_CONTAINER_NAME: {{ .Values.cni.repair.initContainerName | quote }}
   REPAIR_BROKEN_POD_LABEL_KEY: {{ .Values.cni.repair.brokenPodLabelKey | quote }}
   REPAIR_BROKEN_POD_LABEL_VALUE: {{ .Values.cni.repair.brokenPodLabelValue | quote }}
+  {{- with .Values.env }}
+  {{- range $key, $val := . }}
+  {{ $key }}: "{{ $val }}"
+  {{- end }}
+  {{- end }}

manifests/charts/istio-cni/values.yaml

@@ -100,6 +100,9 @@ defaults:
     # of pods at the start of the update.
     rollingMaxUnavailable: 1
 
+    # A `key: value` mapping of environment variables to add to the pod
+    env: {}
+
   # Revision is set as 'version' label and part of the resource names when installing multiple control planes.
   revision: ""
 

manifests/charts/istio-control/istio-discovery/templates/serviceaccount.yaml

@@ -14,6 +14,6 @@ metadata:
     release: {{ .Release.Name }}
   {{- if .Values.pilot.serviceAccountAnnotations }}
   annotations:
-{{- toYaml .Values.pilot.serviceAccountAnnotations | indent 4 }}
+{{- toYaml .Values.pilot.serviceAccountAnnotations | nindent 4 }}
   {{- end }}
 ---

manifests/charts/istiod-remote/templates/serviceaccount.yaml

@@ -15,7 +15,7 @@ metadata:
     release: {{ .Release.Name }}
   {{- if .Values.pilot.serviceAccountAnnotations }}
   annotations:
-{{- toYaml .Values.pilot.serviceAccountAnnotations | indent 4 }}
+{{- toYaml .Values.pilot.serviceAccountAnnotations | nindent 4 }}
   {{- end }}
 ---
 {{- end }}

operator/pkg/apis/istio/v1alpha1/values_types.proto

@@ -133,6 +133,9 @@ message CNIConfig {
   // May be specified as a number of pods or as a percent of the total number
   // of pods at the start of the update.
   IntOrString rollingMaxUnavailable = 23;
+
+  // Environment variables passed to the istio-cni container.
+  google.protobuf.Struct env = 24;
 }
 
 message CNIUsageConfig {

pilot/pkg/bootstrap/monitoring.go

@@ -53,24 +53,14 @@ var (
 	)
 )
 
-func init() {
-	pilotVersion.With(versionTag.Value(version.Info.String())).Record(1)
-}
-
-func addMonitor(mux *http.ServeMux) error {
-	exporter, err := monitoring.RegisterPrometheusExporter(nil, nil)
-	if err != nil {
-		return fmt.Errorf("could not set up prometheus exporter: %v", err)
-	}
+func addMonitor(exporter http.Handler, mux *http.ServeMux) {
 	mux.Handle(metricsPath, metricsMiddleware(exporter))
 
 	mux.HandleFunc(versionPath, func(out http.ResponseWriter, req *http.Request) {
 		if _, err := out.Write([]byte(version.Info.String())); err != nil {
 			log.Errorf("Unable to write version string: %v", err)
 		}
 	})
-
-	return nil
 }
 
 func metricsMiddleware(handler http.Handler) http.Handler {
@@ -86,7 +76,7 @@ func metricsMiddleware(handler http.Handler) http.Handler {
 
 // Deprecated: we shouldn't have 2 http ports. Will be removed after code using
 // this port is removed.
-func startMonitor(addr string, mux *http.ServeMux) (*monitor, error) {
+func startMonitor(exporter http.Handler, addr string, mux *http.ServeMux) (*monitor, error) {
 	m := &monitor{}
 
 	// get the network stuff setup
@@ -102,9 +92,7 @@ func startMonitor(addr string, mux *http.ServeMux) (*monitor, error) {
 	// for pilot. a full design / implementation of self-monitoring and reporting
 	// is coming. that design will include proper coverage of statusz/healthz type
 	// functionality, in addition to how pilot reports its own metrics.
-	if err := addMonitor(mux); err != nil {
-		return nil, fmt.Errorf("could not establish self-monitoring: %v", err)
-	}
+	addMonitor(exporter, mux)
 	if addr != "" {
 		m.monitoringServer = &http.Server{
 			Addr:        listener.Addr().String(),
@@ -115,6 +103,7 @@ func startMonitor(addr string, mux *http.ServeMux) (*monitor, error) {
 	}
 
 	version.Info.RecordComponentBuildTag("pilot")
+	pilotVersion.With(versionTag.Value(version.Info.String())).Record(1)
 
 	if addr != "" {
 		go func() {
@@ -135,7 +124,7 @@ func (m *monitor) Close() error {
 // initMonitor initializes the configuration for the pilot monitoring server.
 func (s *Server) initMonitor(addr string) error { // nolint: unparam
 	s.addStartFunc("monitoring", func(stop <-chan struct{}) error {
-		monitor, err := startMonitor(addr, s.monitoringMux)
+		monitor, err := startMonitor(s.metricsExporter, addr, s.monitoringMux)
 		if err != nil {
 			return err
 		}

pilot/pkg/bootstrap/server.go

@@ -73,6 +73,7 @@ import (
 	"istio.io/istio/pkg/kube/multicluster"
 	"istio.io/istio/pkg/kube/namespace"
 	"istio.io/istio/pkg/log"
+	"istio.io/istio/pkg/monitoring"
 	"istio.io/istio/pkg/network"
 	"istio.io/istio/pkg/security"
 	"istio.io/istio/pkg/spiffe"
@@ -128,6 +129,8 @@ type Server struct {
 	// internalDebugMux is a mux for *internal* calls to the debug interface. That is, authentication is disabled.
 	internalDebugMux *http.ServeMux
 
+	metricsExporter http.Handler
+
 	// httpMux listens on the httpAddr (8080).
 	// If a Gateway is used in front and https is off it is also multiplexing
 	// the rest of the features if their port is empty.
@@ -228,6 +231,10 @@ func NewServer(args *PilotArgs, initFuncs ...func(*Server)) (*Server, error) {
 	})
 	e.ServiceDiscovery = ac
 
+	exporter, err := monitoring.RegisterPrometheusExporter(nil, nil)
+	if err != nil {
+		return nil, fmt.Errorf("could not set up prometheus exporter: %v", err)
+	}
 	s := &Server{
 		clusterID:               getClusterID(args),
 		environment:             e,
@@ -241,6 +248,7 @@ func NewServer(args *PilotArgs, initFuncs ...func(*Server)) (*Server, error) {
 		internalStop:            make(chan struct{}),
 		istiodCertBundleWatcher: keycertbundle.NewWatcher(),
 		webhookInfo:             &webhookInfo{},
+		metricsExporter:         exporter,
 	}
 
 	// Apply custom initialization functions.

pilot/pkg/model/telemetry_logging.go

@@ -108,7 +108,7 @@ var (
 			"request_id":                        {Kind: &structpb.Value_StringValue{StringValue: "%REQ(X-REQUEST-ID)%"}},
 			"authority":                         {Kind: &structpb.Value_StringValue{StringValue: "%REQ(:AUTHORITY)%"}},
 			"upstream_host":                     {Kind: &structpb.Value_StringValue{StringValue: "%UPSTREAM_HOST%"}},
-			"upstream_cluster":                  {Kind: &structpb.Value_StringValue{StringValue: "%UPSTREAM_CLUSTER%"}},
+			"upstream_cluster":                  {Kind: &structpb.Value_StringValue{StringValue: "%UPSTREAM_CLUSTER_RAW%"}},
 			"upstream_local_address":            {Kind: &structpb.Value_StringValue{StringValue: "%UPSTREAM_LOCAL_ADDRESS%"}},
 			"downstream_local_address":          {Kind: &structpb.Value_StringValue{StringValue: "%DOWNSTREAM_LOCAL_ADDRESS%"}},
 			"downstream_remote_address":         {Kind: &structpb.Value_StringValue{StringValue: "%DOWNSTREAM_REMOTE_ADDRESS%"}},

pilot/pkg/networking/core/cluster_builder.go

@@ -24,7 +24,6 @@ import (
 	endpoint "github.com/envoyproxy/go-control-plane/envoy/config/endpoint/v3"
 	http "github.com/envoyproxy/go-control-plane/envoy/extensions/upstreams/http/v3"
 	discovery "github.com/envoyproxy/go-control-plane/envoy/service/discovery/v3"
-	"google.golang.org/protobuf/proto"
 	anypb "google.golang.org/protobuf/types/known/anypb"
 	"google.golang.org/protobuf/types/known/durationpb"
 	"google.golang.org/protobuf/types/known/structpb"
@@ -509,7 +508,7 @@ func (cb *ClusterBuilder) buildBlackHoleCluster() *cluster.Cluster {
 	c := &cluster.Cluster{
 		Name:                 util.BlackHoleCluster,
 		ClusterDiscoveryType: &cluster.Cluster_Type{Type: cluster.Cluster_STATIC},
-		ConnectTimeout:       proto.Clone(cb.req.Push.Mesh.ConnectTimeout).(*durationpb.Duration),
+		ConnectTimeout:       cb.req.Push.Mesh.ConnectTimeout,
 		LbPolicy:             cluster.Cluster_ROUND_ROBIN,
 	}
 	c.AltStatName = util.DelimitedStatsPrefix(util.BlackHoleCluster, cb.proxyVersion)
@@ -522,7 +521,7 @@ func (cb *ClusterBuilder) buildDefaultPassthroughCluster() *cluster.Cluster {
 	cluster := &cluster.Cluster{
 		Name:                 util.PassthroughCluster,
 		ClusterDiscoveryType: &cluster.Cluster_Type{Type: cluster.Cluster_ORIGINAL_DST},
-		ConnectTimeout:       proto.Clone(cb.req.Push.Mesh.ConnectTimeout).(*durationpb.Duration),
+		ConnectTimeout:       cb.req.Push.Mesh.ConnectTimeout,
 		LbPolicy:             cluster.Cluster_CLUSTER_PROVIDED,
 		TypedExtensionProtocolOptions: map[string]*anypb.Any{
 			v3.HttpProtocolOptionsType: passthroughHttpProtocolOptions,
@@ -729,7 +728,7 @@ func (cb *ClusterBuilder) buildExternalSDSCluster(addr string) *cluster.Cluster
 	c := &cluster.Cluster{
 		Name:                 security.SDSExternalClusterName,
 		ClusterDiscoveryType: &cluster.Cluster_Type{Type: cluster.Cluster_STATIC},
-		ConnectTimeout:       proto.Clone(cb.req.Push.Mesh.ConnectTimeout).(*durationpb.Duration),
+		ConnectTimeout:       cb.req.Push.Mesh.ConnectTimeout,
 		LoadAssignment: &endpoint.ClusterLoadAssignment{
 			ClusterName: security.SDSExternalClusterName,
 			Endpoints: []*endpoint.LocalityLbEndpoints{

pilot/pkg/networking/core/cluster_builder_test.go

@@ -2217,6 +2217,18 @@ func TestShouldH2Upgrade(t *testing.T) {
 			},
 			upgrade: false,
 		},
+		{
+			name:        "mesh upgrade - dr useClientProtocol",
+			clusterName: "bar",
+			port:        &model.Port{Protocol: protocol.HTTP},
+			mesh:        &meshconfig.MeshConfig{H2UpgradePolicy: meshconfig.MeshConfig_UPGRADE},
+			connectionPool: &networking.ConnectionPoolSettings{
+				Http: &networking.ConnectionPoolSettings_HTTPSettings{
+					UseClientProtocol: true,
+				},
+			},
+			upgrade: false,
+		},
 		{
 			name:        "non-http",
 			clusterName: "bar",

pilot/pkg/networking/core/cluster_traffic_policy.go

@@ -197,6 +197,12 @@ func shouldH2Upgrade(clusterName string, port *model.Port, mesh *meshconfig.Mesh
 	// Upgrade if tls.GetMode() == networking.TLSSettings_ISTIO_MUTUAL
 	if connectionPool != nil && connectionPool.Http != nil {
 		override := connectionPool.Http.H2UpgradePolicy
+		// If useClientProtocol is set, do not upgrade
+		if connectionPool.Http.UseClientProtocol {
+			log.Debugf("Not upgrading cluster because useClientProtocol is set: %v (%v %v)",
+				clusterName, mesh.H2UpgradePolicy, override)
+			return false
+		}
 		// If user wants an upgrade at destination rule/port level that means he is sure that
 		// it is a Http port - upgrade in such case. This is useful incase protocol sniffing is
 		// enabled and user wants to upgrade/preserve http protocol from client.

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

@@ -783,7 +783,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

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

@@ -949,6 +949,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{})
@@ -1878,6 +1891,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,

pkg/config/analysis/analyzers/testdata/authorizationpolicies.yaml

@@ -39,7 +39,7 @@ spec:
         version: v1
     spec:
       containers:
-      - image: docker.io/kennethreitz/httpbin
+      - image: docker.io/mccutchen/go-httpbin:v2.15.0
         imagePullPolicy: IfNotPresent
         name: httpbin
         ports:

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

pkg/kube/inject/webhook_test.go

@@ -1391,3 +1391,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)
+	}
+}