diff --git a/.devcontainer/devcontainer.json b/.devcontainer/devcontainer.json index 9b599922ed..93695be277 100644 --- a/.devcontainer/devcontainer.json +++ b/.devcontainer/devcontainer.json @@ -1,6 +1,6 @@ { "name": "istio build-tools", - "image": "gcr.io/istio-testing/build-tools:release-1.24-7d1c19cf1d83ca3cfdb7cc7b90dd807fe41653da", + "image": "gcr.io/istio-testing/build-tools:release-1.24-bccd228953b7abf90170da1419699d38e95329fb", "privileged": true, "remoteEnv": { "USE_GKE_GCLOUD_AUTH_PLUGIN": "True", diff --git a/.gitignore b/.gitignore index 07341efb3e..a2a68afda9 100644 --- a/.gitignore +++ b/.gitignore @@ -63,3 +63,4 @@ var/ .DS_Store /manifests/charts/**/charts/ /manifests/charts/**/Chart.lock +/cni/pkg/plugin/istio-cni.log diff --git a/Makefile.core.mk b/Makefile.core.mk index 9804a9d0a2..9e17c15fd7 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 ?= 1.24-2025-03-04T19-01-29 +BASE_VERSION ?= 1.24-2025-04-04T19-02-09 ISTIO_BASE_REGISTRY ?= gcr.io/istio-release export GO111MODULE ?= on diff --git a/cni/pkg/cmd/root.go b/cni/pkg/cmd/root.go index 71fbb4aa44..d4dcd5ad35 100644 --- a/cni/pkg/cmd/root.go +++ b/cni/pkg/cmd/root.go @@ -262,6 +262,7 @@ func constructConfig() (*config.Config, error) { MonitoringPort: viper.GetInt(constants.MonitoringPort), ExcludeNamespaces: viper.GetString(constants.ExcludeNamespaces), + PodNamespace: viper.GetString(constants.PodNamespace), ZtunnelUDSAddress: viper.GetString(constants.ZtunnelUDSAddress), AmbientEnabled: viper.GetBool(constants.AmbientEnabled), diff --git a/cni/pkg/config/config.go b/cni/pkg/config/config.go index fa79e084e7..0a0a004cc5 100644 --- a/cni/pkg/config/config.go +++ b/cni/pkg/config/config.go @@ -48,6 +48,9 @@ type InstallConfig struct { // Comma-separated list of K8S namespaces that CNI should ignore ExcludeNamespaces string + // Singular namespace that the istio CNI node agent resides in + PodNamespace string + // KUBERNETES_SERVICE_PROTOCOL K8sServiceProtocol string // KUBERNETES_SERVICE_HOST @@ -130,6 +133,7 @@ func (c InstallConfig) String() string { b.WriteString("SkipTLSVerify: " + fmt.Sprint(c.SkipTLSVerify) + "\n") b.WriteString("ExcludeNamespaces: " + fmt.Sprint(c.ExcludeNamespaces) + "\n") + b.WriteString("PodNamespace: " + fmt.Sprint(c.PodNamespace) + "\n") b.WriteString("K8sServiceProtocol: " + c.K8sServiceProtocol + "\n") b.WriteString("K8sServiceHost: " + c.K8sServiceHost + "\n") b.WriteString("K8sServicePort: " + fmt.Sprint(c.K8sServicePort) + "\n") diff --git a/cni/pkg/constants/constants.go b/cni/pkg/constants/constants.go index 82d9ad3be9..267245b3c6 100644 --- a/cni/pkg/constants/constants.go +++ b/cni/pkg/constants/constants.go @@ -32,6 +32,7 @@ const ( CNIEventSocket = "cni-event-address" CNIAgentRunDir = "cni-agent-run-dir" ExcludeNamespaces = "exclude-namespaces" + PodNamespace = "pod-namespace" AmbientEnabled = "ambient-enabled" AmbientDNSCapture = "ambient-dns-capture" AmbientIPv6 = "ambient-ipv6" @@ -62,6 +63,8 @@ const ( UDSLogPath = "/log" CNIEventSocketName = "pluginevent.sock" LogUDSSocketName = "log.sock" + LocalRollingLogName = "istio-cni.log" + RollingLogMaxSizeMB = 10 CNIPluginKubeconfName = "istio-cni-kubeconfig" // K8s liveness and readiness endpoints LivenessEndpoint = "/healthz" diff --git a/cni/pkg/install/cniconfig.go b/cni/pkg/install/cniconfig.go index 1f4ce1bcb4..40b8cc6d72 100644 --- a/cni/pkg/install/cniconfig.go +++ b/cni/pkg/install/cniconfig.go @@ -37,6 +37,7 @@ func createCNIConfigFile(ctx context.Context, cfg *config.InstallConfig) (string CNIAgentRunDir: cfg.CNIAgentRunDir, AmbientEnabled: cfg.AmbientEnabled, ExcludeNamespaces: strings.Split(cfg.ExcludeNamespaces, ","), + PodNamespace: cfg.PodNamespace, } pluginConfig.Name = "istio-cni" diff --git a/cni/pkg/install/cniconfig_test.go b/cni/pkg/install/cniconfig_test.go index 88d34bf488..4c4c6a8cc3 100644 --- a/cni/pkg/install/cniconfig_test.go +++ b/cni/pkg/install/cniconfig_test.go @@ -366,6 +366,7 @@ const ( "name": "istio-cni", "type": "istio-cni", "plugin_log_level": "__LOG_LEVEL__", + "pod_namespace": "__POD_NAMESPACE__", "kubernetes": { "kubeconfig": "__KUBECONFIG_FILENAME__", "cni_bin_dir": "/path/cni/bin" @@ -451,6 +452,7 @@ func TestCreateCNIConfigFile(t *testing.T) { ChainedCNIPlugin: c.chainedCNIPlugin, PluginLogLevel: "debug", CNIAgentRunDir: kubeconfigFilename, + PodNamespace: "my-namespace", } cfg := config.InstallConfig{ @@ -458,6 +460,7 @@ func TestCreateCNIConfigFile(t *testing.T) { ChainedCNIPlugin: c.chainedCNIPlugin, PluginLogLevel: "debug", CNIAgentRunDir: kubeconfigFilename, + PodNamespace: "my-namespace", } test := func(cfg config.InstallConfig) func(t *testing.T) { return func(t *testing.T) { diff --git a/cni/pkg/install/testdata/bridge.conf.golden b/cni/pkg/install/testdata/bridge.conf.golden index b75459f104..dfed9e6fc0 100644 --- a/cni/pkg/install/testdata/bridge.conf.golden +++ b/cni/pkg/install/testdata/bridge.conf.golden @@ -27,6 +27,7 @@ "ipam": {}, "name": "istio-cni", "plugin_log_level": "debug", + "pod_namespace": "my-namespace", "type": "istio-cni" } ] diff --git a/cni/pkg/install/testdata/istio-cni.conf b/cni/pkg/install/testdata/istio-cni.conf index 0de95680bc..f5fd47a39c 100644 --- a/cni/pkg/install/testdata/istio-cni.conf +++ b/cni/pkg/install/testdata/istio-cni.conf @@ -9,5 +9,6 @@ "ambient_enabled": false, "exclude_namespaces": [ "" - ] + ], + "pod_namespace": "my-namespace" } diff --git a/cni/pkg/install/testdata/list-with-istio.conflist.golden b/cni/pkg/install/testdata/list-with-istio.conflist.golden index 7daad54833..6276b6ab17 100644 --- a/cni/pkg/install/testdata/list-with-istio.conflist.golden +++ b/cni/pkg/install/testdata/list-with-istio.conflist.golden @@ -37,6 +37,7 @@ "ipam": {}, "name": "istio-cni", "plugin_log_level": "debug", + "pod_namespace": "my-namespace", "type": "istio-cni" } ] diff --git a/cni/pkg/install/testdata/list.conflist.golden b/cni/pkg/install/testdata/list.conflist.golden index 7daad54833..6276b6ab17 100644 --- a/cni/pkg/install/testdata/list.conflist.golden +++ b/cni/pkg/install/testdata/list.conflist.golden @@ -37,6 +37,7 @@ "ipam": {}, "name": "istio-cni", "plugin_log_level": "debug", + "pod_namespace": "my-namespace", "type": "istio-cni" } ] diff --git a/cni/pkg/plugin/plugin.go b/cni/pkg/plugin/plugin.go index a1338f23fe..65b87a51a4 100644 --- a/cni/pkg/plugin/plugin.go +++ b/cni/pkg/plugin/plugin.go @@ -24,6 +24,7 @@ import ( "path/filepath" "runtime/debug" "strconv" + "strings" "time" "github.com/containernetworking/cni/pkg/skel" @@ -66,6 +67,7 @@ type Config struct { CNIAgentRunDir string `json:"cni_agent_run_dir"` AmbientEnabled bool `json:"ambient_enabled"` ExcludeNamespaces []string `json:"exclude_namespaces"` + PodNamespace string `json:"pod_namespace"` } // K8sArgs is the valid CNI_ARGS used for Kubernetes @@ -109,6 +111,8 @@ func parseConfig(stdin []byte) (*Config, error) { return &conf, nil } +// Logging with CNI plugins is special - we *cannot* log to stdout, as the CNI spec uses stdin/stdout to pass context between invoked plugins. +// So, we log to a rolling logfile, and also forward logs via UDS to the node agent (if available) func GetLoggingOptions(cfg *Config) *log.Options { loggingOptions := log.DefaultOptions() loggingOptions.OutputPaths = []string{"stderr"} @@ -120,6 +124,10 @@ func GetLoggingOptions(cfg *Config) *log.Options { if file.Exists(udsAddr) { loggingOptions.WithTeeToUDS(udsAddr, constants.UDSLogPath) } + + // Also tee to a rolling log on the node's local filesystem, in case the UDS server is down. + loggingOptions.WithTeeToRollingLocal(filepath.Join(cfg.CNIAgentRunDir, constants.LocalRollingLogName), constants.RollingLogMaxSizeMB) + // Override plugin log level based on their config. Not we use "all" (OverrideScopeName) since there is no scoping in the plugin. if cfg.PluginLogLevel != "" { loggingOptions.SetDefaultOutputLevel(log.OverrideScopeName, log.StringToLevel(cfg.PluginLogLevel)) @@ -222,8 +230,9 @@ func doAddRun(args *skel.CmdArgs, conf *Config, kClient kubernetes.Interface, ru cniEventAddr := filepath.Join(conf.CNIAgentRunDir, constants.CNIEventSocketName) cniClient := newCNIClient(cniEventAddr, constants.CNIAddEventPath) if err = PushCNIEvent(cniClient, args, prevResIps, podName, podNamespace); err != nil { - log.Errorf("istio-cni cmdAdd failed to signal node Istio CNI agent: %s", err) - return err + // return a more informative error in the pod event log if CNI plugin fails + wrapErr := fmt.Errorf("istio-cni cmdAdd failed to contact node Istio CNI agent: %s", err) + return wrapErr } return nil } @@ -231,6 +240,8 @@ func doAddRun(args *skel.CmdArgs, conf *Config, kClient kubernetes.Interface, ru } // End ambient plugin logic + maybeCNIPod := string(k8sArgs.K8S_POD_NAME) + maybeCNINS := string(k8sArgs.K8S_POD_NAMESPACE) pi := &PodInfo{} var k8sErr error for attempt := 1; attempt <= podRetrievalMaxRetries; attempt++ { @@ -239,6 +250,32 @@ func doAddRun(args *skel.CmdArgs, conf *Config, kClient kubernetes.Interface, ru break } log.Debugf("Failed to get %s/%s pod info: %v", podNamespace, podName, k8sErr) + + // Failsafe - if we get here, we could be in a state where + // 1. We are being upgraded - `istio-cni` node agent pod is gone + // 2. This plugin was left in place to stall pod spawns until the + // replacement arrives. + // 3. This plugin can't contact the K8S API server (creds expired/invalid) + // 4. The pod this plugin would be blocking by returning this error + // *is* our replacement `istio-cni` pod (which would refresh our creds) + // + // So, if we can't contact the K8S API server at all, fall back to checking the + // K8S_POD/K8S_NAMESPACE values from the CNI layer, and let this pod through + // if it looks like it might be our `istio-cni` node agent. + // + // We could do this check unconditionally above, but it seems smarter to only + // fall back to this (lightly) relaxed check when we know we are in a degraded state. + // + // Is this fail open? Not really, the K8S args come from the cluster's CNI and are as-authoritative + // as the hard query we would otherwise make against the API. + // + // TODO NRI could probably give us more identifying information here OOB from k8s. + if strings.HasPrefix(maybeCNIPod, "istio-cni-node-") && + maybeCNINS == conf.PodNamespace { + log.Infof("in a degraded state and %v looks like our own agent pod, skipping", maybeCNIPod) + return nil + } + time.Sleep(podRetrievalInterval) } if k8sErr != nil { diff --git a/cni/test/testdata/expected/10-calico.conflist-istioconfig b/cni/test/testdata/expected/10-calico.conflist-istioconfig index 33f83af1a4..db25557d7d 100644 --- a/cni/test/testdata/expected/10-calico.conflist-istioconfig +++ b/cni/test/testdata/expected/10-calico.conflist-istioconfig @@ -34,6 +34,7 @@ "ipam": {}, "name": "istio-cni", "plugin_log_level": "debug", + "pod_namespace": "", "type": "istio-cni" } ] diff --git a/cni/test/testdata/expected/YYY-istio-cni.conf b/cni/test/testdata/expected/YYY-istio-cni.conf index afca7db46c..330c367425 100644 --- a/cni/test/testdata/expected/YYY-istio-cni.conf +++ b/cni/test/testdata/expected/YYY-istio-cni.conf @@ -9,5 +9,6 @@ "ambient_enabled": false, "exclude_namespaces": [ "istio-system" - ] + ], + "pod_namespace": "" } diff --git a/cni/test/testdata/expected/minikube_cni.conflist.expected b/cni/test/testdata/expected/minikube_cni.conflist.expected index ae83f6f6fa..cb093b64a4 100644 --- a/cni/test/testdata/expected/minikube_cni.conflist.expected +++ b/cni/test/testdata/expected/minikube_cni.conflist.expected @@ -31,6 +31,7 @@ "ipam": {}, "name": "istio-cni", "plugin_log_level": "debug", + "pod_namespace": "", "type": "istio-cni" } ] diff --git a/common/.commonfiles.sha b/common/.commonfiles.sha index 591e455288..c8a2f68433 100644 --- a/common/.commonfiles.sha +++ b/common/.commonfiles.sha @@ -1 +1 @@ -6d1ea5c54f7aad9c31e8ff058772f9f44cbe08e0 +2a57949e8949678850564daef685829ceb137ed5 diff --git a/common/scripts/setup_env.sh b/common/scripts/setup_env.sh index f174ec78c5..d9ebed4b80 100755 --- a/common/scripts/setup_env.sh +++ b/common/scripts/setup_env.sh @@ -75,7 +75,7 @@ fi TOOLS_REGISTRY_PROVIDER=${TOOLS_REGISTRY_PROVIDER:-gcr.io} PROJECT_ID=${PROJECT_ID:-istio-testing} if [[ "${IMAGE_VERSION:-}" == "" ]]; then - IMAGE_VERSION=release-1.24-7d1c19cf1d83ca3cfdb7cc7b90dd807fe41653da + IMAGE_VERSION=release-1.24-bccd228953b7abf90170da1419699d38e95329fb fi if [[ "${IMAGE_NAME:-}" == "" ]]; then IMAGE_NAME=build-tools diff --git a/go.mod b/go.mod index eed5181c01..3ce9c37518 100644 --- a/go.mod +++ b/go.mod @@ -92,11 +92,12 @@ require ( google.golang.org/genproto/googleapis/rpc v0.0.0-20240930140551-af27646dc61f google.golang.org/grpc v1.67.1 google.golang.org/protobuf v1.34.2 + gopkg.in/natefinch/lumberjack.v2 v2.2.1 gopkg.in/yaml.v2 v2.4.0 gopkg.in/yaml.v3 v3.0.1 helm.sh/helm/v3 v3.16.1 - istio.io/api v1.24.4-0.20250310185707-46b0c1b3e4e4 - istio.io/client-go v1.24.4-0.20250310190306-37b46c6daa9d + istio.io/api v1.24.5-0.20250409200717-4933c1da972e + istio.io/client-go v1.24.5-0.20250409201417-1715c4db04bc k8s.io/api v0.31.1 k8s.io/apiextensions-apiserver v0.31.1 k8s.io/apimachinery v0.31.1 diff --git a/go.sum b/go.sum index 7f699d3cfa..c31a230362 100644 --- a/go.sum +++ b/go.sum @@ -598,6 +598,8 @@ gopkg.in/inf.v0 v0.9.1 h1:73M5CoZyi3ZLMOyDlQh031Cx6N9NDJ2Vvfl76EDAgDc= gopkg.in/inf.v0 v0.9.1/go.mod h1:cWUDdTG/fYaXco+Dcufb5Vnc6Gp2YChqWtbxRZE0mXw= gopkg.in/ini.v1 v1.67.0 h1:Dgnx+6+nfE+IfzjUEISNeydPJh9AXNNsWbGP9KzCsOA= gopkg.in/ini.v1 v1.67.0/go.mod h1:pNLf8WUiyNEtQjuu5G5vTm06TEv9tsIgeAvK8hOrP4k= +gopkg.in/natefinch/lumberjack.v2 v2.2.1 h1:bBRl1b0OH9s/DuPhuXpNl+VtCaJXFZ5/uEFST95x9zc= +gopkg.in/natefinch/lumberjack.v2 v2.2.1/go.mod h1:YD8tP3GAjkrDg1eZH7EGmyESg/lsYskCTPBJVb9jqSc= gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= gopkg.in/yaml.v2 v2.2.8/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= gopkg.in/yaml.v2 v2.4.0 h1:D8xgwECY7CYvx+Y2n4sBz93Jn9JRvxdiyyo8CTfuKaY= @@ -612,10 +614,10 @@ helm.sh/helm/v3 v3.16.1 h1:cER6tI/8PgUAsaJaQCVBUg3VI9KN4oVaZJgY60RIc0c= helm.sh/helm/v3 v3.16.1/go.mod h1:r+xBHHP20qJeEqtvBXMf7W35QDJnzY/eiEBzt+TfHps= honnef.co/go/tools v0.0.0-20190102054323-c2f93a96b099/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.24.4-0.20250310185707-46b0c1b3e4e4 h1:0ZgJiQUNTGbgGQtJ9sfWctqDbMaLutruYXtkAl9gLQI= -istio.io/api v1.24.4-0.20250310185707-46b0c1b3e4e4/go.mod h1:MQnRok7RZ20/PE56v0LxmoWH0xVxnCQPNuf9O7PAN1I= -istio.io/client-go v1.24.4-0.20250310190306-37b46c6daa9d h1:ZQ4yFn1BwC4id6WXl6oTtKgFO47wcOFe8OBaLJBMII4= -istio.io/client-go v1.24.4-0.20250310190306-37b46c6daa9d/go.mod h1:LTWLnhwfH/AZGJtdlzpWy4YdTAv7E0qGKtECN0c4aeM= +istio.io/api v1.24.5-0.20250409200717-4933c1da972e h1:5LzLnhNQtSAfK/rsW5h+hlJtUM0LCFJwQwqwtR3UiD4= +istio.io/api v1.24.5-0.20250409200717-4933c1da972e/go.mod h1:MQnRok7RZ20/PE56v0LxmoWH0xVxnCQPNuf9O7PAN1I= +istio.io/client-go v1.24.5-0.20250409201417-1715c4db04bc h1:9GZ8yyhY6Q5ozjMO51yt3eAgor+uoEDsVUVfGmAfP1M= +istio.io/client-go v1.24.5-0.20250409201417-1715c4db04bc/go.mod h1:2Gx1QpUyGeB58ndq28yHcWV4fIHeO8F5T+ox2sPT2+E= k8s.io/api v0.31.1 h1:Xe1hX/fPW3PXYYv8BlozYqw63ytA92snr96zMW9gWTU= k8s.io/api v0.31.1/go.mod h1:sbN1g6eY6XVLeqNsZGLnI5FwVseTrZX7Fv3O26rhAaI= k8s.io/apiextensions-apiserver v0.31.1 h1:L+hwULvXx+nvTYX/MKM3kKMZyei+UiSXQWciX/N6E40= diff --git a/istio.deps b/istio.deps index 903fc45e1f..c4f32857a7 100644 --- a/istio.deps +++ b/istio.deps @@ -4,13 +4,13 @@ "name": "PROXY_REPO_SHA", "repoName": "proxy", "file": "", - "lastStableSHA": "5b5a94eac1658c67702f8116c496695cc723136f" + "lastStableSHA": "e06e0d0c05e0cc4339c7f3a93faff6eeaad12a2e" }, { "_comment": "", "name": "ZTUNNEL_REPO_SHA", "repoName": "ztunnel", "file": "", - "lastStableSHA": "9d2fe78f022f4616e43feb006f077310ef21b932" + "lastStableSHA": "70c638ecb14a7b4e161af6a3d275f774e87630ef" } ] diff --git a/istioctl/pkg/writer/ztunnel/configdump/api.go b/istioctl/pkg/writer/ztunnel/configdump/api.go index 898794dddb..a4256750e0 100644 --- a/istioctl/pkg/writer/ztunnel/configdump/api.go +++ b/istioctl/pkg/writer/ztunnel/configdump/api.go @@ -119,6 +119,7 @@ type CertsDump struct { Identity string `json:"identity"` State string `json:"state"` CertChain []*Cert `json:"certChain"` + RootCert []*Cert `json:"rootCerts"` } type Cert struct { diff --git a/istioctl/pkg/writer/ztunnel/configdump/certificates.go b/istioctl/pkg/writer/ztunnel/configdump/certificates.go index 3023aaa0dc..45eab0039e 100644 --- a/istioctl/pkg/writer/ztunnel/configdump/certificates.go +++ b/istioctl/pkg/writer/ztunnel/configdump/certificates.go @@ -63,11 +63,13 @@ func (c *ConfigWriter) PrintSecretSummary() error { fmt.Fprintf(w, "%v\t%v\t%v\t%v\t%v\t%v\t%v\n", secret.Identity, valueOrNA(""), secret.State, false, valueOrNA(""), valueOrNA(""), valueOrNA("")) } else { + // Before, the root was part of the certChain. + legacyFormat := len(secret.RootCert) == 0 for i, ca := range secret.CertChain { t := "Intermediate" if i == 0 { t = "Leaf" - } else if i == len(secret.CertChain)-1 { + } else if i == len(secret.CertChain)-1 && legacyFormat { t = "Root" } n := new(big.Int) @@ -75,6 +77,12 @@ func (c *ConfigWriter) PrintSecretSummary() error { fmt.Fprintf(w, "%v\t%v\t%v\t%v\t%x\t%v\t%v\n", secret.Identity, t, secret.State, certNotExpired(ca), n, valueOrNA(ca.ExpirationTime), valueOrNA(ca.ValidFrom)) } + for _, ca := range secret.RootCert { + n := new(big.Int) + n, _ = n.SetString(ca.SerialNumber, 10) + fmt.Fprintf(w, "%v\t%v\t%v\t%v\t%x\t%v\t%v\n", + secret.Identity, "Root", secret.State, certNotExpired(ca), n, valueOrNA(ca.ExpirationTime), valueOrNA(ca.ValidFrom)) + } } } return w.Flush() diff --git a/licenses/gopkg.in/natefinch/lumberjack.v2/LICENSE b/licenses/gopkg.in/natefinch/lumberjack.v2/LICENSE new file mode 100644 index 0000000000..c3d4cc307d --- /dev/null +++ b/licenses/gopkg.in/natefinch/lumberjack.v2/LICENSE @@ -0,0 +1,21 @@ +The MIT License (MIT) + +Copyright (c) 2014 Nate Finch + +Permission is hereby granted, free of charge, to any person obtaining a copy +of this software and associated documentation files (the "Software"), to deal +in the Software without restriction, including without limitation the rights +to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +copies of the Software, and to permit persons to whom the Software is +furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in all +copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +SOFTWARE. \ No newline at end of file diff --git a/manifests/charts/istio-cni/templates/configmap-cni.yaml b/manifests/charts/istio-cni/templates/configmap-cni.yaml index 39a09fb699..75a0cb0b97 100644 --- a/manifests/charts/istio-cni/templates/configmap-cni.yaml +++ b/manifests/charts/istio-cni/templates/configmap-cni.yaml @@ -20,8 +20,8 @@ data: CNI_CONF_NAME: {{ .Values.cniConfFileName }} # Name of the CNI config file to create. Only override if you know the exact path your CNI requires.. {{- end }} CHAINED_CNI_PLUGIN: {{ .Values.chained | quote }} - EXCLUDED_NAMESPACES: "{{ range $idx, $ns := .Values.excludeNamespaces }}{{ if $idx }},{{ end }}{{ $ns }}{{ end }}" - REPAIR_ENABLED: {{ .Values.chained | quote }} + EXCLUDE_NAMESPACES: "{{ range $idx, $ns := .Values.excludeNamespaces }}{{ if $idx }},{{ end }}{{ $ns }}{{ end }}" + REPAIR_ENABLED: {{ .Values.repair.enabled | quote }} REPAIR_LABEL_PODS: {{ .Values.repair.labelPods | quote }} REPAIR_DELETE_PODS: {{ .Values.repair.deletePods | quote }} REPAIR_REPAIR_PODS: {{ .Values.repair.repairPods | quote }} diff --git a/manifests/charts/istio-cni/templates/daemonset.yaml b/manifests/charts/istio-cni/templates/daemonset.yaml index e30ab0acbb..fb236b04ff 100644 --- a/manifests/charts/istio-cni/templates/daemonset.yaml +++ b/manifests/charts/istio-cni/templates/daemonset.yaml @@ -9,6 +9,10 @@ kind: DaemonSet apiVersion: apps/v1 metadata: + # Note that this is templated but evaluates to a fixed name + # which the CNI plugin may fall back onto in some failsafe scenarios. + # if this name is changed, CNI plugin logic that checks for this name + # format should also be updated. name: {{ template "name" . }}-node namespace: {{ .Release.Namespace }} labels: diff --git a/pilot/pkg/networking/core/httproute.go b/pilot/pkg/networking/core/httproute.go index b5f16df0a1..0c6752ff58 100644 --- a/pilot/pkg/networking/core/httproute.go +++ b/pilot/pkg/networking/core/httproute.go @@ -116,7 +116,7 @@ func (configgen *ConfigGeneratorImpl) BuildHTTPRoutes( // TODO: trace decorators, inbound timeouts func buildSidecarInboundHTTPRouteConfig(lb *ListenerBuilder, cc inboundChainConfig) *route.RouteConfiguration { traceOperation := telemetry.TraceOperation(string(cc.telemetryMetadata.InstanceHostname), cc.port.Port) - defaultRoute := istio_route.BuildDefaultHTTPInboundRoute(lb.node, cc.clusterName, traceOperation) + defaultRoute := istio_route.BuildDefaultHTTPInboundRoute(lb.node, cc.clusterName, traceOperation, cc.port.Protocol) inboundVHost := &route.VirtualHost{ Name: inboundVirtualHostPrefix + strconv.Itoa(cc.port.Port), // Format: "inbound|http|%d" diff --git a/pilot/pkg/networking/core/route/route.go b/pilot/pkg/networking/core/route/route.go index 2397e9d0f3..954488b932 100644 --- a/pilot/pkg/networking/core/route/route.go +++ b/pilot/pkg/networking/core/route/route.go @@ -47,6 +47,7 @@ import ( "istio.io/istio/pkg/config/constants" "istio.io/istio/pkg/config/host" "istio.io/istio/pkg/config/labels" + "istio.io/istio/pkg/config/protocol" "istio.io/istio/pkg/jwt" "istio.io/istio/pkg/log" "istio.io/istio/pkg/util/grpc" @@ -1244,7 +1245,7 @@ func GetRouteOperation(in *route.Route, vsName string, port int) string { } // BuildDefaultHTTPInboundRoute builds a default inbound route. -func BuildDefaultHTTPInboundRoute(proxy *model.Proxy, clusterName string, operation string) *route.Route { +func BuildDefaultHTTPInboundRoute(proxy *model.Proxy, clusterName string, operation string, protocol protocol.Instance) *route.Route { out := buildDefaultHTTPRoute(clusterName, operation) // For inbound, configure with notimeout. out.GetRoute().Timeout = Notimeout @@ -1254,7 +1255,8 @@ func BuildDefaultHTTPInboundRoute(proxy *model.Proxy, clusterName string, operat // gRPC requests time out like any other requests using timeout or its default. GrpcTimeoutHeaderMax: Notimeout, } - if util.VersionGreaterOrEqual124(proxy) && features.EnableInboundRetryPolicy { + // "reset-before-request" does not work well for gRPC streaming services. + if util.VersionGreaterOrEqual124(proxy) && features.EnableInboundRetryPolicy && !protocol.IsGRPC() { out.GetRoute().RetryPolicy = &route.RetryPolicy{ RetryOn: "reset-before-request", NumRetries: &wrapperspb.UInt32Value{ diff --git a/pilot/pkg/networking/core/route/route_cache.go b/pilot/pkg/networking/core/route/route_cache.go index e56d02b57a..b4e78ca99f 100644 --- a/pilot/pkg/networking/core/route/route_cache.go +++ b/pilot/pkg/networking/core/route/route_cache.go @@ -147,6 +147,12 @@ func (r *Cache) Key() any { h.Write(Slash) h.WriteString(svc.Attributes.Namespace) h.Write(Separator) + for _, alias := range svc.Attributes.Aliases { + h.WriteString(string(alias.Hostname)) + h.Write(Slash) + h.WriteString(alias.Namespace) + h.Write(Separator) + } } h.Write(Separator) diff --git a/pilot/pkg/networking/core/route/route_test.go b/pilot/pkg/networking/core/route/route_test.go index 055bc40cb8..b3f43c8051 100644 --- a/pilot/pkg/networking/core/route/route_test.go +++ b/pilot/pkg/networking/core/route/route_test.go @@ -2988,11 +2988,13 @@ func TestInboundHTTPRoute(t *testing.T) { testCases := []struct { name string enableRetry bool + protocol protocol.Instance expected *envoyroute.Route }{ { - name: "enable retry", + name: "enable retry, http protocol", enableRetry: true, + protocol: protocol.HTTP, expected: &envoyroute.Route{ Name: "default", Match: route.TranslateRouteMatch(config.Config{}, nil), @@ -3017,9 +3019,32 @@ func TestInboundHTTPRoute(t *testing.T) { }, }, }, + { + name: "enable retry, grpc protocol", + enableRetry: true, + protocol: protocol.GRPC, + expected: &envoyroute.Route{ + Name: "default", + Match: route.TranslateRouteMatch(config.Config{}, nil), + Action: &envoyroute.Route_Route{ + Route: &envoyroute.RouteAction{ + ClusterSpecifier: &envoyroute.RouteAction_Cluster{Cluster: "cluster"}, + Timeout: route.Notimeout, + MaxStreamDuration: &envoyroute.RouteAction_MaxStreamDuration{ + MaxStreamDuration: route.Notimeout, + GrpcTimeoutHeaderMax: route.Notimeout, + }, + }, + }, + Decorator: &envoyroute.Decorator{ + Operation: "operation", + }, + }, + }, { name: "disable retry", enableRetry: false, + protocol: protocol.HTTP, expected: &envoyroute.Route{ Name: "default", Match: route.TranslateRouteMatch(config.Config{}, nil), @@ -3043,7 +3068,7 @@ func TestInboundHTTPRoute(t *testing.T) { t.Run(tc.name, func(t *testing.T) { test.SetForTest(t, &features.EnableInboundRetryPolicy, tc.enableRetry) inroute := route.BuildDefaultHTTPInboundRoute(&model.Proxy{IstioVersion: &model.IstioVersion{Major: 1, Minor: 24, Patch: -1}}, - "cluster", "operation") + "cluster", "operation", tc.protocol) if !reflect.DeepEqual(tc.expected, inroute) { t.Errorf("error in inbound routes. Got: %v, Want: %v", inroute, tc.expected) } diff --git a/pkg/log/options.go b/pkg/log/options.go index 49c69c06de..d2f0c373c6 100644 --- a/pkg/log/options.go +++ b/pkg/log/options.go @@ -130,6 +130,15 @@ func (o *Options) WithTeeToUDS(addr, path string) *Options { }) } +// WithTeeToRolling configures a parallel logging pipeline that writes logs to a local rolling log of fixed size. +// This is mainly used by the CNI plugin, and so the size and rollover is intentionally kept small. +// rollingPath is the path the rolling log(s) will be written to. +func (o *Options) WithTeeToRollingLocal(rollingPath string, maxSizeInMB int) *Options { + return o.WithExtension(func(c zapcore.Core) (zapcore.Core, func() error, error) { + return teeToRollingLocal(c, rollingPath, maxSizeInMB), func() error { return nil }, nil + }) +} + // Extension provides an extension mechanism for logs. // This is essentially like https://pkg.go.dev/golang.org/x/exp/slog#Handler. // This interface should be considered unstable; we will likely swap it for slog in the future and not expose zap internals. diff --git a/pkg/log/uds.go b/pkg/log/uds.go index 9a143b7ce3..b985879d00 100644 --- a/pkg/log/uds.go +++ b/pkg/log/uds.go @@ -24,8 +24,10 @@ import ( "sync" "time" + "go.uber.org/zap" "go.uber.org/zap/buffer" "go.uber.org/zap/zapcore" + lj "gopkg.in/natefinch/lumberjack.v2" ) // An udsCore write entries to an UDS server with HTTP Post. Log messages will be encoded into a JSON array. @@ -60,9 +62,30 @@ func teeToUDSServer(baseCore zapcore.Core, address, path string) zapcore.Core { break } } + return zapcore.NewTee(baseCore, uc) } +// Creates a small/fixed rolling log on the node's local FS. +// This can be useful as a backup/fallback in case the node agent is down +// and the UDS logging consequently fails (losing logs). +func teeToRollingLocal(baseCore zapcore.Core, path string, maxSizeMB int) zapcore.Core { + w := zapcore.AddSync(&lj.Logger{ + Filename: path, + MaxSize: maxSizeMB, + MaxBackups: 1, + MaxAge: 2, // days + }) + + core := zapcore.NewCore( + zapcore.NewJSONEncoder(defaultEncoderConfig), + w, + zap.InfoLevel, + ) + + return zapcore.NewTee(baseCore, core) +} + // Enabled implements zapcore.Core. func (u *udsCore) Enabled(l zapcore.Level) bool { return l >= u.minimumLevel diff --git a/releasenotes/notes/55304.yaml b/releasenotes/notes/55304.yaml new file mode 100644 index 0000000000..c2c07ce111 --- /dev/null +++ b/releasenotes/notes/55304.yaml @@ -0,0 +1,8 @@ +apiVersion: release-notes/v2 +kind: bug-fix +area: traffic-management +issue: + - 55215 +releaseNotes: + - | + **Fixed** Corner cases where `istio-cni` might block its own upgrade. Added fallback logging (in case agent is down) to a fixed-size node-local logfile. diff --git a/releasenotes/notes/grpc-inbound-retry.yaml b/releasenotes/notes/grpc-inbound-retry.yaml new file mode 100644 index 0000000000..221f484223 --- /dev/null +++ b/releasenotes/notes/grpc-inbound-retry.yaml @@ -0,0 +1,6 @@ +apiVersion: release-notes/v2 +kind: bug-fix +area: traffic-management +releaseNotes: + - | + **Fixed** an issue where proxy memory goes up with gRPC streaming services. \ No newline at end of file diff --git a/releasenotes/notes/rds-cache-alias.yaml b/releasenotes/notes/rds-cache-alias.yaml new file mode 100644 index 0000000000..960c98bbbc --- /dev/null +++ b/releasenotes/notes/rds-cache-alias.yaml @@ -0,0 +1,6 @@ +apiVersion: release-notes/v2 +kind: bug-fix +area: traffic-management +releaseNotes: + - | + **Fixed** an issue causing changes to ExternalName services to sometimes be skipped due to a cache eviction bug. \ No newline at end of file diff --git a/security/pkg/server/ca/server.go b/security/pkg/server/ca/server.go index c5f3bd9451..168b9aa17d 100644 --- a/security/pkg/server/ca/server.go +++ b/security/pkg/server/ca/server.go @@ -137,10 +137,6 @@ func (s *Server) CreateCertificate(ctx context.Context, request *pb.IstioCertifi serverCaLog.Debugf("Append cert chain to response, %s", string(certChainBytes)) } } - if len(rootCertBytes) != 0 { - respCertChain = append(respCertChain, string(rootCertBytes)) - } - // expand `respCertChain` since each element might be a concatenated multi-cert PEM // the expanded structure (one cert per `string` in `certChain`) is specifically expected by `ztunnel` response := &pb.IstioCertificateResponse{} @@ -151,6 +147,13 @@ func (s *Server) CreateCertificate(ctx context.Context, request *pb.IstioCertifi response.CertChain = append(response.CertChain, cert+"\n") } } + // Per the spec: "... the root cert is the last element." so we do not want to flatten the root cert. + // If we did, the client cannot distinguish the root. + // A better API would put the root in a separate field entirely... + if len(rootCertBytes) != 0 { + response.CertChain = append(response.CertChain, string(rootCertBytes)) + } + serverCaLog.Debugf("Responding with cert chain, %q", response.CertChain) s.monitoring.Success.Increment() serverCaLog.Debugf("CSR successfully signed, sans %v.", sans)