diff --git a/cni/pkg/cmd/root.go b/cni/pkg/cmd/root.go index 0882841332d..df36b4af9df 100644 --- a/cni/pkg/cmd/root.go +++ b/cni/pkg/cmd/root.go @@ -165,6 +165,7 @@ func init() { registerBooleanParameter(constants.UpdateCNIBinaries, true, "Whether to refresh existing binaries when installing CNI") registerStringArrayParameter(constants.SkipCNIBinaries, []string{}, "Binaries that should not be installed. Currently Istio only installs one binary `istio-cni`") + registerStringParameter(constants.CNIBinariesPrefix, "", "The filename prefix to add to each binary when copying") registerIntegerParameter(constants.MonitoringPort, 15014, "HTTP port to serve prometheus metrics") registerStringParameter(constants.LogUDSAddress, "/var/run/istio-cni/log.sock", "The UDS server address which CNI plugin will copy log ouptut to") registerBooleanParameter(constants.AmbientEnabled, false, "Whether ambient controller is enabled") @@ -253,6 +254,7 @@ func constructConfig() (*config.Config, error) { CNIBinSourceDir: constants.CNIBinDir, CNIBinTargetDirs: []string{constants.HostCNIBinDir, constants.SecondaryBinDir}, + CNIBinariesPrefix: viper.GetString(constants.CNIBinariesPrefix), UpdateCNIBinaries: viper.GetBool(constants.UpdateCNIBinaries), SkipCNIBinaries: viper.GetStringSlice(constants.SkipCNIBinaries), MonitoringPort: viper.GetInt(constants.MonitoringPort), diff --git a/cni/pkg/config/config.go b/cni/pkg/config/config.go index 8b8494d869e..a8f6a2f31d3 100644 --- a/cni/pkg/config/config.go +++ b/cni/pkg/config/config.go @@ -68,6 +68,8 @@ type InstallConfig struct { CNIBinSourceDir string // Directories into which to copy the CNI binaries CNIBinTargetDirs []string + // The prefix to add to the name of each CNI binary + CNIBinariesPrefix string // Whether to override existing CNI binaries UpdateCNIBinaries bool diff --git a/cni/pkg/constants/constants.go b/cni/pkg/constants/constants.go index a77bfc92070..4307f0675e5 100644 --- a/cni/pkg/constants/constants.go +++ b/cni/pkg/constants/constants.go @@ -32,6 +32,7 @@ const ( SkipTLSVerify = "skip-tls-verify" SkipCNIBinaries = "skip-cni-binaries" UpdateCNIBinaries = "update-cni-binaries" + CNIBinariesPrefix = "cni-binaries-prefix" MonitoringPort = "monitoring-port" LogUDSAddress = "log-uds-address" AmbientEnabled = "ambient-enabled" diff --git a/cni/pkg/install/binaries.go b/cni/pkg/install/binaries.go index f99a9153876..c3e09a55802 100644 --- a/cni/pkg/install/binaries.go +++ b/cni/pkg/install/binaries.go @@ -22,8 +22,9 @@ import ( "istio.io/istio/pkg/util/sets" ) -func copyBinaries(srcDir string, targetDirs []string, updateBinaries bool, skipBinaries []string) error { +func copyBinaries(srcDir string, targetDirs []string, updateBinaries bool, skipBinaries []string, binariesPrefix string) error { skipBinariesSet := sets.New(skipBinaries...) + for _, targetDir := range targetDirs { if err := file.IsDirWriteable(targetDir); err != nil { installLog.Infof("Directory %s is not writable, skipping.", targetDir) @@ -46,18 +47,19 @@ func copyBinaries(srcDir string, targetDirs []string, updateBinaries bool, skipB continue } - targetFilepath := filepath.Join(targetDir, filename) + targetFilename := binariesPrefix + filename + targetFilepath := filepath.Join(targetDir, targetFilename) if _, err := os.Stat(targetFilepath); err == nil && !updateBinaries { installLog.Infof("%s is already here and UPDATE_CNI_BINARIES isn't true, skipping", targetFilepath) continue } srcFilepath := filepath.Join(srcDir, filename) - err := file.AtomicCopy(srcFilepath, targetDir, filename) + err := file.AtomicCopy(srcFilepath, targetDir, targetFilename) if err != nil { return err } - installLog.Infof("Copied %s to %s.", filename, targetDir) + installLog.Infof("Copied %s to %s.", filename, targetFilepath) } } diff --git a/cni/pkg/install/binaries_test.go b/cni/pkg/install/binaries_test.go index 638a36813d0..3423cf59a4a 100644 --- a/cni/pkg/install/binaries_test.go +++ b/cni/pkg/install/binaries_test.go @@ -28,6 +28,7 @@ func TestCopyBinaries(t *testing.T) { expectedFiles map[string]string // {filename: contents. ...} updateBinaries bool skipBinaries []string + prefix string }{ { name: "basic", @@ -54,6 +55,12 @@ func TestCopyBinaries(t *testing.T) { srcFiles: map[string]string{"istio-cni": "cni111", "istio-iptables": "iptables111"}, expectedFiles: map[string]string{"istio-cni": "cni111"}, }, + { + name: "binaries prefix", + prefix: "prefix-", + srcFiles: map[string]string{"istio-cni": "cni111", "istio-iptables": "iptables111"}, + expectedFiles: map[string]string{"prefix-istio-cni": "cni111", "prefix-istio-iptables": "iptables111"}, + }, } for _, c := range cases { @@ -74,7 +81,7 @@ func TestCopyBinaries(t *testing.T) { } } - err := copyBinaries(srcDir, []string{targetDir}, c.updateBinaries, c.skipBinaries) + err := copyBinaries(srcDir, []string{targetDir}, c.updateBinaries, c.skipBinaries, c.prefix) if err != nil { t.Fatal(err) } diff --git a/cni/pkg/install/cniconfig.go b/cni/pkg/install/cniconfig.go index 302bbf4c344..3f46c9a30e7 100644 --- a/cni/pkg/install/cniconfig.go +++ b/cni/pkg/install/cniconfig.go @@ -180,7 +180,7 @@ func getCNIConfigFilepath(ctx context.Context, cfg pluginConfig) (string, error) return filepath.Join(cfg.mountedCNINetDir, filename), nil } - watcher, fileModified, errChan, err := util.CreateFileWatcher(cfg.mountedCNINetDir) + watcher, fileModified, errChan, err := util.CreateFileWatcher("", []string{cfg.mountedCNINetDir}) if err != nil { return "", err } diff --git a/cni/pkg/install/install.go b/cni/pkg/install/install.go index f55f7f63e6f..5dd041dfc70 100644 --- a/cni/pkg/install/install.go +++ b/cni/pkg/install/install.go @@ -53,7 +53,7 @@ func NewInstaller(cfg *config.InstallConfig, isReady *atomic.Value) *Installer { func (in *Installer) install(ctx context.Context) (err error) { if err = copyBinaries( in.cfg.CNIBinSourceDir, in.cfg.CNIBinTargetDirs, - in.cfg.UpdateCNIBinaries, in.cfg.SkipCNIBinaries); err != nil { + in.cfg.UpdateCNIBinaries, in.cfg.SkipCNIBinaries, in.cfg.CNIBinariesPrefix); err != nil { cniInstalls.With(resultLabel.Value(resultCopyBinariesFailure)).Increment() return } @@ -107,6 +107,8 @@ func (in *Installer) Run(ctx context.Context) (err error) { // Cleanup remove Istio CNI's config, kubeconfig file, and binaries. func (in *Installer) Cleanup() error { + istioCniExecutableName := in.cfg.CNIBinariesPrefix + "istio-cni" + installLog.Info("Cleaning up.") if len(in.cniConfigFilepath) > 0 && file.Exists(in.cniConfigFilepath) { if in.cfg.ChainedCNIPlugin { @@ -127,7 +129,7 @@ func (in *Installer) Cleanup() error { if err != nil { return fmt.Errorf("%s: %w", in.cniConfigFilepath, err) } - if plugin["type"] == "istio-cni" { + if plugin["type"] == istioCniExecutableName { cniConfigMap["plugins"] = append(plugins[:i], plugins[i+1:]...) break } @@ -156,7 +158,7 @@ func (in *Installer) Cleanup() error { } for _, targetDir := range in.cfg.CNIBinTargetDirs { - if istioCNIBin := filepath.Join(targetDir, "istio-cni"); file.Exists(istioCNIBin) { + if istioCNIBin := filepath.Join(targetDir, istioCniExecutableName); file.Exists(istioCNIBin) { installLog.Infof("Removing binary: %s", istioCNIBin) if err := os.Remove(istioCNIBin); err != nil { return err @@ -185,7 +187,7 @@ func readServiceAccountToken(saToken string) (string, error) { func (in *Installer) sleepCheckInstall(ctx context.Context) error { // Create file watcher before checking for installation // so that no file modifications are missed while and after checking - watcher, fileModified, errChan, err := util.CreateFileWatcher(append(in.cfg.CNIBinTargetDirs, in.cfg.MountedCNINetDir)...) + watcher, fileModified, errChan, err := util.CreateFileWatcher(in.cfg.CNIBinariesPrefix, append(in.cfg.CNIBinTargetDirs, in.cfg.MountedCNINetDir)) if err != nil { return err } @@ -227,6 +229,7 @@ func checkInstall(cfg *config.InstallConfig, cniConfigFilepath string) error { if !cfg.CNIEnableInstall { return nil } + istioCniExecutableName := cfg.CNIBinariesPrefix + "istio-cni" defaultCNIConfigFilename, err := getDefaultCNINetwork(cfg.MountedCNINetDir) if err != nil { return err @@ -261,7 +264,7 @@ func checkInstall(cfg *config.InstallConfig, cniConfigFilepath string) error { if err != nil { return fmt.Errorf("%s: %w", cniConfigFilepath, err) } - if plugin["type"] == "istio-cni" { + if plugin["type"] == istioCniExecutableName { return nil } } @@ -274,7 +277,7 @@ func checkInstall(cfg *config.InstallConfig, cniConfigFilepath string) error { return err } - if cniConfigMap["type"] != "istio-cni" { + if cniConfigMap["type"] != istioCniExecutableName { return fmt.Errorf("istio-cni CNI config file modified: %s", cniConfigFilepath) } return nil diff --git a/cni/pkg/install/install_test.go b/cni/pkg/install/install_test.go index 4a28d214cd8..3a55de20220 100644 --- a/cni/pkg/install/install_test.go +++ b/cni/pkg/install/install_test.go @@ -37,6 +37,7 @@ func TestCheckInstall(t *testing.T) { chainedCNIPlugin bool skipInstall bool existingConfFiles map[string]string // {srcFilename: targetFilename, ...} + cniBinariesPrefix string }{ { name: "preempted config", @@ -95,6 +96,12 @@ func TestCheckInstall(t *testing.T) { cniConfigFilename: "istio-cni.conf", existingConfFiles: map[string]string{"istio-cni.conf": "istio-cni.conf"}, }, + { + name: "custom binaries prefix", + cniConfigFilename: "istio-cni.conf", + cniBinariesPrefix: "prefix-", + existingConfFiles: map[string]string{"istio-cni-prefixed.conf": "istio-cni.conf"}, + }, } for _, c := range cases { @@ -110,10 +117,11 @@ func TestCheckInstall(t *testing.T) { } cfg := &config.InstallConfig{ - MountedCNINetDir: tempDir, - CNIConfName: c.cniConfName, - ChainedCNIPlugin: c.chainedCNIPlugin, - CNIEnableInstall: !c.skipInstall, + MountedCNINetDir: tempDir, + CNIConfName: c.cniConfName, + ChainedCNIPlugin: c.chainedCNIPlugin, + CNIEnableInstall: !c.skipInstall, + CNIBinariesPrefix: c.cniBinariesPrefix, } err := checkInstall(cfg, filepath.Join(tempDir, c.cniConfigFilename)) if (c.expectedFailure && err == nil) || (!c.expectedFailure && err != nil) { @@ -299,6 +307,7 @@ func TestCleanup(t *testing.T) { configFilename string existingConfigFilename string expectedConfigFilename string + cniBinariesPrefix string }{ { name: "chained CNI plugin", @@ -312,6 +321,13 @@ func TestCleanup(t *testing.T) { configFilename: "istio-cni.conf", existingConfigFilename: "istio-cni.conf", }, + { + name: "prefix", + cniBinariesPrefix: "prefix-", + configFilename: "list-cni-prefixed.conf", + existingConfigFilename: "list-with-istio.conflist", + expectedConfigFilename: "list-no-istio.conflist", + }, } for _, c := range cases { @@ -327,7 +343,8 @@ func TestCleanup(t *testing.T) { } // Create existing binary files - if err := os.WriteFile(filepath.Join(cniBinDir, "istio-cni"), []byte{1, 2, 3}, 0o755); err != nil { + filename := c.cniBinariesPrefix + "istio-cni" + if err := os.WriteFile(filepath.Join(cniBinDir, filename), []byte{1, 2, 3}, 0o755); err != nil { t.Fatal(err) } @@ -338,9 +355,10 @@ func TestCleanup(t *testing.T) { } cfg := &config.InstallConfig{ - MountedCNINetDir: cniNetDir, - ChainedCNIPlugin: c.chainedCNIPlugin, - CNIBinTargetDirs: []string{cniBinDir}, + MountedCNINetDir: cniNetDir, + ChainedCNIPlugin: c.chainedCNIPlugin, + CNIBinariesPrefix: c.cniBinariesPrefix, + CNIBinTargetDirs: []string{cniBinDir}, } isReady := &atomic.Value{} diff --git a/cni/pkg/install/kubeconfig.go b/cni/pkg/install/kubeconfig.go index 6f45e8dead5..c469e388526 100644 --- a/cni/pkg/install/kubeconfig.go +++ b/cni/pkg/install/kubeconfig.go @@ -120,6 +120,11 @@ func createKubeconfigFile(cfg *config.InstallConfig, saToken string) (kubeconfig return "", err } + // When using Multus, the net.d dir might not exist yet, so we must create it + if err := os.MkdirAll(cfg.MountedCNINetDir, os.FileMode(0o755)); err != nil { + return "", err + } + kubeconfigFilepath = filepath.Join(cfg.MountedCNINetDir, cfg.KubeconfigFilename) installLog.Infof("write kubeconfig file %s with: \n%+v", kubeconfigFilepath, kcbbToPrint.String()) if err = file.AtomicWrite(kubeconfigFilepath, kcbb.Bytes(), os.FileMode(cfg.KubeconfigMode)); err != nil { diff --git a/cni/pkg/install/kubeconfig_test.go b/cni/pkg/install/kubeconfig_test.go index a4d83280fd6..efb57da21a6 100644 --- a/cni/pkg/install/kubeconfig_test.go +++ b/cni/pkg/install/kubeconfig_test.go @@ -45,6 +45,7 @@ func TestCreateKubeconfigFile(t *testing.T) { saToken string kubeCAFilepath string skipTLSVerify bool + cniNetDir string }{ { name: "k8s service host not set", @@ -73,12 +74,25 @@ func TestCreateKubeconfigFile(t *testing.T) { saToken: saToken, kubeCAFilepath: kubeCAFilepath, }, + { + name: "nonexistent net.d dir", + kubeconfigFilename: "istio-cni-kubeconfig", + kubeconfigMode: constants.DefaultKubeconfigMode, + k8sServiceHost: k8sServiceHost, + k8sServicePort: k8sServicePort, + saToken: saToken, + skipTLSVerify: true, + cniNetDir: filepath.Join(t.TempDir(), "nonexistent-dir"), + }, } for _, c := range cases { t.Run(c.name, func(t *testing.T) { - // Create temp directory for files - tempDir := t.TempDir() + tempDir := c.cniNetDir + // Create temp directory for files if not provided in test case + if tempDir == "" { + tempDir = t.TempDir() + } cfg := &config.InstallConfig{ MountedCNINetDir: tempDir, diff --git a/cni/pkg/plugin/iptables_linux.go b/cni/pkg/plugin/iptables_linux.go index c8a0b4c10c2..ae9fe0e991f 100644 --- a/cni/pkg/plugin/iptables_linux.go +++ b/cni/pkg/plugin/iptables_linux.go @@ -39,6 +39,7 @@ func (ipt *iptables) Program(podName, netns string, rdrct *Redirect) error { viper.Set(constants.NetworkNamespace, netns) viper.Set(constants.EnvoyPort, rdrct.targetPort) viper.Set(constants.ProxyUID, rdrct.noRedirectUID) + viper.Set(constants.ProxyGID, rdrct.noRedirectGID) viper.Set(constants.InboundInterceptionMode, rdrct.redirectMode) viper.Set(constants.ServiceCidr, rdrct.includeIPCidrs) viper.Set(constants.LocalExcludePorts, rdrct.excludeInboundPorts) diff --git a/cni/pkg/plugin/kubernetes.go b/cni/pkg/plugin/kubernetes.go index fdaa1f7e6b5..c57f60aaa95 100644 --- a/cni/pkg/plugin/kubernetes.go +++ b/cni/pkg/plugin/kubernetes.go @@ -22,6 +22,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/kubernetes" + meshconfig "istio.io/api/mesh/v1alpha1" "istio.io/istio/pkg/kube" "istio.io/pkg/log" ) @@ -38,6 +39,9 @@ type PodInfo struct { Labels map[string]string Annotations map[string]string ProxyEnvironments map[string]string + ProxyConfig *meshconfig.ProxyConfig + ProxyUID *int64 + ProxyGID *int64 } // newK8sClient returns a Kubernetes client @@ -84,6 +88,10 @@ func getK8sPodInfo(client *kubernetes.Clientset, podName, podNamespace string) ( for _, e := range container.Env { pi.ProxyEnvironments[e.Name] = e.Value } + if container.SecurityContext != nil { + pi.ProxyUID = container.SecurityContext.RunAsUser + pi.ProxyGID = container.SecurityContext.RunAsGroup + } continue } } diff --git a/cni/pkg/plugin/redirect.go b/cni/pkg/plugin/redirect.go index e99d86f6bb3..963175011c5 100644 --- a/cni/pkg/plugin/redirect.go +++ b/cni/pkg/plugin/redirect.go @@ -32,6 +32,7 @@ const ( defaultProxyStatusPort = "15020" defaultRedirectToPort = "15001" defaultNoRedirectUID = "1337" + defaultNoRedirectGID = "1337" defaultRedirectMode = redirectModeREDIRECT defaultRedirectIPCidr = "*" defaultRedirectExcludeIPCidr = "" @@ -77,6 +78,7 @@ type Redirect struct { targetPort string redirectMode string noRedirectUID string + noRedirectGID string includeIPCidrs string excludeIPCidrs string excludeInboundPorts string @@ -216,6 +218,13 @@ func NewRedirect(pi *PodInfo) (*Redirect, error) { "redirectMode", isFound, valErr) } redir.noRedirectUID = defaultNoRedirectUID + if pi.ProxyUID != nil { + redir.noRedirectUID = fmt.Sprintf("%d", *pi.ProxyUID) + } + redir.noRedirectGID = defaultNoRedirectGID + if pi.ProxyGID != nil { + redir.noRedirectGID = fmt.Sprintf("%d", *pi.ProxyGID) + } isFound, redir.includeIPCidrs, valErr = getAnnotationOrDefault("includeIPCidrs", pi.Annotations) if valErr != nil { return nil, fmt.Errorf("annotation value error for value %s; annotationFound = %t: %v", diff --git a/cni/pkg/util/util.go b/cni/pkg/util/util.go index ce76d84f13d..05e9a835fe1 100644 --- a/cni/pkg/util/util.go +++ b/cni/pkg/util/util.go @@ -19,21 +19,30 @@ import ( "encoding/json" "fmt" "os" + "path/filepath" + "strings" "github.com/fsnotify/fsnotify" "istio.io/istio/pkg/file" ) +type watchPrefix struct { + watcher *fsnotify.Watcher + prefix string +} + // Creates a file watcher that watches for any changes to the directory -func CreateFileWatcher(dirs ...string) (watcher *fsnotify.Watcher, fileModified chan bool, errChan chan error, err error) { +// If `prefix` is non-empty, it will only notify of changes for files whose filename starts with `prefix` +func CreateFileWatcher(prefix string, dirs []string) (watcher *fsnotify.Watcher, fileModified chan bool, errChan chan error, err error) { watcher, err = fsnotify.NewWatcher() if err != nil { return } fileModified, errChan = make(chan bool), make(chan error) - go watchFiles(watcher, fileModified, errChan) + wp := &watchPrefix{watcher: watcher, prefix: prefix} + go watchFiles(wp, fileModified, errChan) for _, dir := range dirs { if file.IsDirWriteable(dir) != nil { @@ -50,17 +59,17 @@ func CreateFileWatcher(dirs ...string) (watcher *fsnotify.Watcher, fileModified return } -func watchFiles(watcher *fsnotify.Watcher, fileModified chan bool, errChan chan error) { +func watchFiles(wp *watchPrefix, fileModified chan bool, errChan chan error) { for { select { - case event, ok := <-watcher.Events: + case event, ok := <-wp.watcher.Events: if !ok { return } - if event.Op&(fsnotify.Create|fsnotify.Write|fsnotify.Remove) != 0 { + if event.Op&(fsnotify.Create|fsnotify.Write|fsnotify.Remove) != 0 && strings.HasPrefix(filepath.Base(event.Name), wp.prefix) { fileModified <- true } - case err, ok := <-watcher.Errors: + case err, ok := <-wp.watcher.Errors: if !ok { return }