From a92ec96100ac4f2020767ade02d82db8767bd982 Mon Sep 17 00:00:00 2001 From: Alex Leong Date: Thu, 23 Oct 2025 00:50:10 +0000 Subject: [PATCH 1/7] add iptables binary fallback logic Signed-off-by: Alex Leong --- cni-plugin/main.go | 33 ++++++++++++++++++-- pkg/iptables/iptables.go | 67 ++++++++++++++++++++++++++++++++++++++-- 2 files changed, 96 insertions(+), 4 deletions(-) diff --git a/cni-plugin/main.go b/cni-plugin/main.go index 1032a17d..6c353e97 100644 --- a/cni-plugin/main.go +++ b/cni-plugin/main.go @@ -22,7 +22,9 @@ import ( "context" "encoding/json" "fmt" + "io" "os" + "path/filepath" "strconv" "strings" @@ -81,6 +83,7 @@ type PluginConf struct { PrevResult *cniv1.Result `json:"-"` LogLevel string `json:"log_level"` + LogFile string `json:"log_file"` ProxyInit ProxyInit `json:"linkerd"` Kubernetes Kubernetes `json:"kubernetes"` } @@ -99,7 +102,9 @@ func main() { ) } -func configureLoggingLevel(logLevel string) { +// configureLogging sets log level and configures outputs to both stderr and a file. +// If logFilePath is empty, a sensible default is used. +func configureLogging(logLevel, logFilePath string) { switch strings.ToLower(logLevel) { case "debug": logrus.SetLevel(logrus.DebugLevel) @@ -108,6 +113,29 @@ func configureLoggingLevel(logLevel string) { default: logrus.SetLevel(logrus.WarnLevel) } + + // Default log file path + if strings.TrimSpace(logFilePath) == "" { + logFilePath = "/var/log/linkerd-cni-plugin.log" + } + + // Ensure directory exists + if dir := filepath.Dir(logFilePath); dir != "" && dir != "." { + _ = os.MkdirAll(dir, 0o755) + } + + // Open file for append, create if missing + f, err := os.OpenFile(logFilePath, os.O_CREATE|os.O_APPEND|os.O_WRONLY, 0o644) + if err != nil { + // If we cannot open the log file, continue logging only to stderr + logrus.SetOutput(os.Stderr) + logrus.WithError(err).Warnf("linkerd-cni: failed to open log file %q; continuing with stderr only", logFilePath) + return + } + + // Tee logs to both stderr and the file + mw := io.MultiWriter(os.Stderr, f) + logrus.SetOutput(mw) } // parseConfig parses the supplied configuration (and prevResult) from stdin. @@ -147,7 +175,8 @@ func cmdAdd(args *skel.CmdArgs) error { logrus.Errorf("error parsing config: %e", err) return err } - configureLoggingLevel(conf.LogLevel) + // Configure logging level and outputs + configureLogging(conf.LogLevel, conf.LogFile) if conf.PrevResult != nil { logrus.WithFields(logrus.Fields{ diff --git a/pkg/iptables/iptables.go b/pkg/iptables/iptables.go index 9cf9ff75..8f74c618 100644 --- a/pkg/iptables/iptables.go +++ b/pkg/iptables/iptables.go @@ -67,8 +67,9 @@ type FirewallConfiguration struct { // https://github.com/istio/istio/blob/e83411e/pilot/docker/prepare_proxy.sh func ConfigureFirewall(firewallConfiguration FirewallConfiguration) error { log.Debugf("tracing script execution as [%s]", executionTraceID) - log.Debugf("using '%s' to set-up firewall rules", firewallConfiguration.BinPath) - log.Debugf("using '%s' to list all available rules", firewallConfiguration.SaveBinPath) + + // Before executing, ensure the configured iptables binaries exist; if not, attempt a fallback. + resolveBinFallback(&firewallConfiguration) existingRules, err := executeCommand(firewallConfiguration, firewallConfiguration.makeShowAllRules()) if err != nil { @@ -112,6 +113,9 @@ func CleanupFirewallConfig(firewallConfiguration FirewallConfiguration) error { log.Debugf("using '%s' to clean-up firewall rules", firewallConfiguration.BinPath) log.Debugf("using '%s' to list all available rules", firewallConfiguration.SaveBinPath) + // Ensure binaries exist before attempting cleanup as well + resolveBinFallback(&firewallConfiguration) + commands := make([]*exec.Cmd, 0) commands = firewallConfiguration.cleanupRules(commands) @@ -448,3 +452,62 @@ func asDestination(portRange util.PortRange) string { return fmt.Sprintf("%d:%d", portRange.LowerBound, portRange.UpperBound) } + +// resolveBinFallback ensures the configured BinPath and SaveBinPath exist on PATH; if not, it +// tries reasonable alternatives of the same family (ip6tables vs iptables). Returns true if a +// fallback was applied. +func resolveBinFallback(fc *FirewallConfiguration) { + // helper to check presence + has := func(name string) bool { + _, err := exec.LookPath(name) + return err == nil + } + + // Both present? nothing to do + if has(fc.BinPath) && has(fc.SaveBinPath) { + log.WithFields(log.Fields{ + "requestedBin": fc.BinPath, + "requestedSaveBin": fc.SaveBinPath, + }).Debug("iptables: using configured binaries") + return + } + + // Decide family based on current name + ipv6 := strings.Contains(fc.BinPath, "ip6tables") || strings.Contains(fc.SaveBinPath, "ip6tables") + + // Candidate orders: prefer nft, then plain, then legacy + var candidates [][2]string + if ipv6 { + candidates = [][2]string{ + {"ip6tables-nft", "ip6tables-nft-save"}, + {"ip6tables", "ip6tables-save"}, + {"ip6tables-legacy", "ip6tables-legacy-save"}, + } + } else { + candidates = [][2]string{ + {"iptables-nft", "iptables-nft-save"}, + {"iptables", "iptables-save"}, + {"iptables-legacy", "iptables-legacy-save"}, + } + } + + // Use first candidate where both exist + for _, pair := range candidates { + if has(pair[0]) && has(pair[1]) { + if pair[0] != fc.BinPath || pair[1] != fc.SaveBinPath { + log.WithFields(log.Fields{ + "requestedBin": fc.BinPath, + "requestedSaveBin": fc.SaveBinPath, + "fallbackBin": pair[0], + "fallbackSaveBin": pair[1], + }).Warn("iptables: configured binaries not found; applying fallback to available binaries") + } + fc.BinPath = pair[0] + fc.SaveBinPath = pair[1] + return + } + } + + // No candidates found; keep as-is and let execution fail with a clear error later + log.WithFields(log.Fields{"binPath": fc.BinPath, "saveBinPath": fc.SaveBinPath}).Error("iptables: no suitable binaries found on PATH; commands may fail") +} From eba2d973e7181f3c19625dcb5a0c188a0124639a Mon Sep 17 00:00:00 2001 From: Alex Leong Date: Thu, 23 Oct 2025 23:21:09 +0000 Subject: [PATCH 2/7] Add log rotation Signed-off-by: Alex Leong --- cni-plugin/main.go | 50 ++++++++++++++++++++++++++++++---------------- go.mod | 1 + go.sum | 2 ++ 3 files changed, 36 insertions(+), 17 deletions(-) diff --git a/cni-plugin/main.go b/cni-plugin/main.go index 6c353e97..7bcb9378 100644 --- a/cni-plugin/main.go +++ b/cni-plugin/main.go @@ -36,6 +36,7 @@ import ( "github.com/linkerd/linkerd2-proxy-init/proxy-init/cmd" "github.com/sirupsen/logrus" + "gopkg.in/natefinch/lumberjack.v2" v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/intstr" @@ -82,10 +83,13 @@ type PluginConf struct { RawPrevResult *map[string]interface{} `json:"prevResult"` PrevResult *cniv1.Result `json:"-"` - LogLevel string `json:"log_level"` - LogFile string `json:"log_file"` - ProxyInit ProxyInit `json:"linkerd"` - Kubernetes Kubernetes `json:"kubernetes"` + LogLevel string `json:"log_level"` + LogFile string `json:"log_file"` + LogFileMaxSizeMB int `json:"log_file_max_size_mb"` + LogFileMaxAgeDays int `json:"log_file_max_age_days"` + LogFileMaxCount int `json:"log_file_max_count"` + ProxyInit ProxyInit `json:"linkerd"` + Kubernetes Kubernetes `json:"kubernetes"` } func main() { @@ -102,9 +106,10 @@ func main() { ) } -// configureLogging sets log level and configures outputs to both stderr and a file. +// configureLogging sets log level and configures outputs to both stderr and a file with rotation. // If logFilePath is empty, a sensible default is used. -func configureLogging(logLevel, logFilePath string) { +// maxSizeMB, maxAgeDays, and maxCount configure the log rotation behavior. +func configureLogging(logLevel, logFilePath string, maxSizeMB, maxAgeDays, maxCount int) { switch strings.ToLower(logLevel) { case "debug": logrus.SetLevel(logrus.DebugLevel) @@ -119,22 +124,33 @@ func configureLogging(logLevel, logFilePath string) { logFilePath = "/var/log/linkerd-cni-plugin.log" } + // Default rotation parameters if not specified (matching Calico CNI defaults) + if maxSizeMB <= 0 { + maxSizeMB = 100 // 100 MB default + } + if maxAgeDays <= 0 { + maxAgeDays = 30 // 30 days default + } + if maxCount <= 0 { + maxCount = 10 // 10 files default + } + // Ensure directory exists if dir := filepath.Dir(logFilePath); dir != "" && dir != "." { _ = os.MkdirAll(dir, 0o755) } - // Open file for append, create if missing - f, err := os.OpenFile(logFilePath, os.O_CREATE|os.O_APPEND|os.O_WRONLY, 0o644) - if err != nil { - // If we cannot open the log file, continue logging only to stderr - logrus.SetOutput(os.Stderr) - logrus.WithError(err).Warnf("linkerd-cni: failed to open log file %q; continuing with stderr only", logFilePath) - return + // Configure log rotation with lumberjack + logger := &lumberjack.Logger{ + Filename: logFilePath, + MaxSize: maxSizeMB, // megabytes + MaxBackups: maxCount, // number of backups + MaxAge: maxAgeDays, // days + Compress: true, // compress rotated files } - // Tee logs to both stderr and the file - mw := io.MultiWriter(os.Stderr, f) + // Tee logs to both stderr and the rotating log file + mw := io.MultiWriter(os.Stderr, logger) logrus.SetOutput(mw) } @@ -175,8 +191,8 @@ func cmdAdd(args *skel.CmdArgs) error { logrus.Errorf("error parsing config: %e", err) return err } - // Configure logging level and outputs - configureLogging(conf.LogLevel, conf.LogFile) + // Configure logging level and outputs with rotation + configureLogging(conf.LogLevel, conf.LogFile, conf.LogFileMaxSizeMB, conf.LogFileMaxAgeDays, conf.LogFileMaxCount) if conf.PrevResult != nil { logrus.WithFields(logrus.Fields{ diff --git a/go.mod b/go.mod index 1a630212..17635650 100644 --- a/go.mod +++ b/go.mod @@ -45,6 +45,7 @@ require ( google.golang.org/protobuf v1.36.5 // indirect gopkg.in/evanphx/json-patch.v4 v4.12.0 // indirect gopkg.in/inf.v0 v0.9.1 // indirect + gopkg.in/natefinch/lumberjack.v2 v2.2.1 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect k8s.io/klog/v2 v2.130.1 // indirect k8s.io/kube-openapi v0.0.0-20250710124328-f3f2b991d03b // indirect diff --git a/go.sum b/go.sum index 30b91586..b09ebf65 100644 --- a/go.sum +++ b/go.sum @@ -148,6 +148,8 @@ gopkg.in/evanphx/json-patch.v4 v4.12.0 h1:n6jtcsulIzXPJaxegRbvFNNrZDjbij7ny3gmSP gopkg.in/evanphx/json-patch.v4 v4.12.0/go.mod h1:p8EYWUEYMpynmqDbY58zCKCFZw8pRWMG4EsWvDvM72M= gopkg.in/inf.v0 v0.9.1 h1:73M5CoZyi3ZLMOyDlQh031Cx6N9NDJ2Vvfl76EDAgDc= gopkg.in/inf.v0 v0.9.1/go.mod h1:cWUDdTG/fYaXco+Dcufb5Vnc6Gp2YChqWtbxRZE0mXw= +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.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= From 13138e5dbdbeaef1f806dd3d913eb708efa1dcef Mon Sep 17 00:00:00 2001 From: Alex Leong Date: Fri, 24 Oct 2025 18:45:05 +0000 Subject: [PATCH 3/7] update detection logic Signed-off-by: Alex Leong --- pkg/iptables/detect.go | 145 +++++++++++++++++++++++++++++++++++++++ pkg/iptables/iptables.go | 65 +----------------- proxy-init/cmd/root.go | 56 +++++---------- 3 files changed, 165 insertions(+), 101 deletions(-) create mode 100644 pkg/iptables/detect.go diff --git a/pkg/iptables/detect.go b/pkg/iptables/detect.go new file mode 100644 index 00000000..22842113 --- /dev/null +++ b/pkg/iptables/detect.go @@ -0,0 +1,145 @@ +// Copyright (c) Linkerd authors +// +// Portions of the iptables backend detection code are derived from: +// https://github.com/projectcalico/calico/blob/master/felix/environment/feature_detect_linux.go +// Copyright (c) 2018-2025 Tigera, Inc. All rights reserved. +// Licensed under the Apache License, Version 2.0 + +package iptables + +import ( + "bytes" + "os/exec" + "strings" + + log "github.com/sirupsen/logrus" +) + +func countRulesInIptableOutput(in []byte) int { + count := 0 + for _, x := range bytes.Split(in, []byte("\n")) { + if len(x) >= 1 && x[0] == '-' { + count++ + } + } + return count +} + +// hasKubernetesChains tries to find in the output of the binary if the Kubernetes +// chains exists +func hasKubernetesChains(output []byte) bool { + return strings.Contains(string(output), "KUBE-IPTABLES-HINT") || strings.Contains(string(output), "KUBE-KUBELET-CANARY") +} + +// DetectBackend attempts to detect the iptables backend (nft or legacy) and the +// appropriate iptables commands to use. If the detected backend does not match +// the specifiedBackend, a warning is logged but the specifiedBackend is used. +// The specifiedBackend can be "nft", "legacy", "plain", or "auto" (to use the +// detected backend). +// +// Based on the backend selected, the appropriate iptables command paths are set +// in the FirewallConfiguration. +// +// This logic is adapted from the Calico CNI plugin: +// https://github.com/projectcalico/calico/blob/master/felix/environment/feature_detect_linux.go +func DetectBackend(fc *FirewallConfiguration, lookPath func(file string) (string, error), ipv6 bool, specifiedBackend string) { + prefix := "iptables" + if ipv6 { + prefix = "ip6tables" + } + + nftSave := FindBestBinary(lookPath, prefix, "nft", "save") + lgcySave := FindBestBinary(lookPath, prefix, "legacy", "save") + nftCmd := strings.TrimSuffix(nftSave, "-save") + lgcyCmd := strings.TrimSuffix(lgcySave, "-save") + + // Check kubelet canary chains in the mangle table for nft first. + nftMangle, _ := executeCommand(*fc, exec.Command(nftSave, "-t", "mangle")) + + var detectedBackend string + if hasKubernetesChains(nftMangle) { + detectedBackend = "nft" + } else { + // Check legacy mangle next. + lgcyMangle, _ := executeCommand(*fc, exec.Command(lgcySave, "-t", "mangle")) + + if hasKubernetesChains(lgcyMangle) { + detectedBackend = "legacy" + } else { + // Fall back to comparing total rule counts between full legacy vs nft saves. + lgcyAll, _ := executeCommand(*fc, exec.Command(lgcySave)) + nftAll, _ := executeCommand(*fc, exec.Command(nftSave)) + + legacyLines := countRulesInIptableOutput(lgcyAll) + nftLines := countRulesInIptableOutput(nftAll) + if legacyLines >= nftLines { + detectedBackend = "legacy" // default to legacy when tied or more legacy rules + } else { + detectedBackend = "nft" + } + } + } + + // Decide which backend to use, honoring a specific request if provided. + requested := strings.ToLower(specifiedBackend) + backendToUse := requested + if requested == "auto" { + log.WithField("detectedBackend", detectedBackend).Debug("Detected Iptables backend") + backendToUse = detectedBackend + } else if requested != detectedBackend { + log.WithFields(log.Fields{ + "detectedBackend": detectedBackend, + "specifiedBackend": requested, + }).Warn("Iptables backend specified does not match the detected backend; honoring specified backend") + } else { + log.WithField("specifiedBackend", specifiedBackend).Debug("Iptables backend specified matches detected backend") + } + + switch backendToUse { + case "legacy": + fc.BinPath = lgcyCmd + fc.SaveBinPath = lgcySave + case "nft": + fc.BinPath = nftCmd + fc.SaveBinPath = nftSave + case "plain": + fc.BinPath = prefix + fc.SaveBinPath = prefix + "-save" + } + + log.WithFields(log.Fields{ + "binPath": fc.BinPath, + "saveBinPath": fc.SaveBinPath, + }).Debug("Using iptables commands") +} + +// FindBestBinary tries to find an iptables binary for the specific variant (legacy/nftables mode) and returns the name +// of the binary. Falls back on iptables-restore/iptables-save if the specific variant isn't available. +// Panics if no binary can be found. +func FindBestBinary(lookPath func(file string) (string, error), prefix, backendMode, saveOrRestore string) string { + if lookPath == nil { + lookPath = exec.LookPath + } + candidates := []string{ + prefix + "-" + backendMode + "-" + saveOrRestore, + prefix + "-" + saveOrRestore, + } + + logCxt := log.WithFields(log.Fields{ + "prefix": prefix, + "backendMode": backendMode, + "saveOrRestore": saveOrRestore, + "candidates": candidates, + }) + + for _, candidate := range candidates { + _, err := lookPath(candidate) + if err == nil { + logCxt.WithField("command", candidate).Debug("Looked up iptables command") + return candidate + } + } + + logCxt.Panic("Failed to find iptables command") + return "" +} diff --git a/pkg/iptables/iptables.go b/pkg/iptables/iptables.go index 8f74c618..ca892dc2 100644 --- a/pkg/iptables/iptables.go +++ b/pkg/iptables/iptables.go @@ -68,8 +68,7 @@ type FirewallConfiguration struct { func ConfigureFirewall(firewallConfiguration FirewallConfiguration) error { log.Debugf("tracing script execution as [%s]", executionTraceID) - // Before executing, ensure the configured iptables binaries exist; if not, attempt a fallback. - resolveBinFallback(&firewallConfiguration) + // Using configured iptables binaries as-is. existingRules, err := executeCommand(firewallConfiguration, firewallConfiguration.makeShowAllRules()) if err != nil { @@ -113,8 +112,7 @@ func CleanupFirewallConfig(firewallConfiguration FirewallConfiguration) error { log.Debugf("using '%s' to clean-up firewall rules", firewallConfiguration.BinPath) log.Debugf("using '%s' to list all available rules", firewallConfiguration.SaveBinPath) - // Ensure binaries exist before attempting cleanup as well - resolveBinFallback(&firewallConfiguration) + // Using configured iptables binaries as-is for cleanup as well. commands := make([]*exec.Cmd, 0) commands = firewallConfiguration.cleanupRules(commands) @@ -453,61 +451,4 @@ func asDestination(portRange util.PortRange) string { return fmt.Sprintf("%d:%d", portRange.LowerBound, portRange.UpperBound) } -// resolveBinFallback ensures the configured BinPath and SaveBinPath exist on PATH; if not, it -// tries reasonable alternatives of the same family (ip6tables vs iptables). Returns true if a -// fallback was applied. -func resolveBinFallback(fc *FirewallConfiguration) { - // helper to check presence - has := func(name string) bool { - _, err := exec.LookPath(name) - return err == nil - } - - // Both present? nothing to do - if has(fc.BinPath) && has(fc.SaveBinPath) { - log.WithFields(log.Fields{ - "requestedBin": fc.BinPath, - "requestedSaveBin": fc.SaveBinPath, - }).Debug("iptables: using configured binaries") - return - } - - // Decide family based on current name - ipv6 := strings.Contains(fc.BinPath, "ip6tables") || strings.Contains(fc.SaveBinPath, "ip6tables") - - // Candidate orders: prefer nft, then plain, then legacy - var candidates [][2]string - if ipv6 { - candidates = [][2]string{ - {"ip6tables-nft", "ip6tables-nft-save"}, - {"ip6tables", "ip6tables-save"}, - {"ip6tables-legacy", "ip6tables-legacy-save"}, - } - } else { - candidates = [][2]string{ - {"iptables-nft", "iptables-nft-save"}, - {"iptables", "iptables-save"}, - {"iptables-legacy", "iptables-legacy-save"}, - } - } - - // Use first candidate where both exist - for _, pair := range candidates { - if has(pair[0]) && has(pair[1]) { - if pair[0] != fc.BinPath || pair[1] != fc.SaveBinPath { - log.WithFields(log.Fields{ - "requestedBin": fc.BinPath, - "requestedSaveBin": fc.SaveBinPath, - "fallbackBin": pair[0], - "fallbackSaveBin": pair[1], - }).Warn("iptables: configured binaries not found; applying fallback to available binaries") - } - fc.BinPath = pair[0] - fc.SaveBinPath = pair[1] - return - } - } - - // No candidates found; keep as-is and let execution fail with a clear error later - log.WithFields(log.Fields{"binPath": fc.BinPath, "saveBinPath": fc.SaveBinPath}).Error("iptables: no suitable binaries found on PATH; commands may fail") -} +// resolveBinFallback removed: binaries are assumed to be correctly set by callers. diff --git a/proxy-init/cmd/root.go b/proxy-init/cmd/root.go index b31dbf8a..fc8b695c 100644 --- a/proxy-init/cmd/root.go +++ b/proxy-init/cmd/root.go @@ -21,6 +21,8 @@ const ( // IPTablesModePlain signals the usage of the iptables commands, which // can be either legacy or nft IPTablesModePlain = "plain" + // IPTablesModeAuto signals automatic detection of the iptables backend + IPTablesModeAuto = "auto" cmdLegacy = "iptables-legacy" cmdLegacySave = "iptables-legacy-save" @@ -165,21 +167,11 @@ func NewRootCmd() *cobra.Command { // BuildFirewallConfiguration returns an iptables FirewallConfiguration suitable to use to configure iptables. func BuildFirewallConfiguration(options *RootOptions) (*iptables.FirewallConfiguration, error) { - if options.IPTablesMode != "" && options.IPTablesMode != IPTablesModeLegacy && options.IPTablesMode != IPTablesModeNFT && options.IPTablesMode != IPTablesModePlain { - return nil, fmt.Errorf("--iptables-mode valid values are only \"%s\", \"%s\" and \"%s\"", IPTablesModeLegacy, IPTablesModeNFT, IPTablesModePlain) - } - - if options.IPTablesMode == "" { - switch options.FirewallBinPath { - case "", cmdLegacy: - options.IPTablesMode = IPTablesModeLegacy - case cmdNFT: - options.IPTablesMode = IPTablesModeNFT - case cmdPlain: - options.IPTablesMode = IPTablesModePlain - default: - return nil, fmt.Errorf("--firewall-bin-path valid values are only \"%s\", \"%s\" and \"%s\"", cmdLegacy, cmdNFT, cmdPlain) - } + if options.IPTablesMode != "" && + options.IPTablesMode != IPTablesModeLegacy && + options.IPTablesMode != IPTablesModeNFT && + options.IPTablesMode != IPTablesModePlain { + return nil, fmt.Errorf("--iptables-mode valid values are only \"%s\", \"%s\", \"%s\", and \"%s\"", IPTablesModeLegacy, IPTablesModeNFT, IPTablesModeAuto, IPTablesModePlain) } if !util.IsValidPort(options.IncomingProxyPort) { @@ -190,8 +182,6 @@ func BuildFirewallConfiguration(options *RootOptions) (*iptables.FirewallConfigu return nil, fmt.Errorf("--outgoing-proxy-port must be a valid TCP port number") } - cmd, cmdSave := getCommands(options) - sanitizedSubnets := []string{} for _, subnet := range options.SubnetsToIgnore { subnet := strings.TrimSpace(subnet) @@ -215,8 +205,6 @@ func BuildFirewallConfiguration(options *RootOptions) (*iptables.FirewallConfigu SimulateOnly: options.SimulateOnly, NetNs: options.NetNs, UseWaitFlag: options.UseWaitFlag, - BinPath: cmd, - SaveBinPath: cmdSave, } if len(options.PortsToRedirect) > 0 { @@ -225,6 +213,16 @@ func BuildFirewallConfiguration(options *RootOptions) (*iptables.FirewallConfigu firewallConfiguration.Mode = iptables.RedirectAllMode } + // For backwards-compatibility, if IPTablesMode is not set, use the FirewallBinPath + // explicitly set by the user. + if options.IPTablesMode == "" { + firewallConfiguration.BinPath = options.FirewallBinPath + firewallConfiguration.SaveBinPath = options.FirewallSaveBinPath + } else { + // Otherwise, detect and set the appropriate backend. + iptables.DetectBackend(firewallConfiguration, exec.LookPath, options.IPv6, options.IPTablesMode) + } + return firewallConfiguration, nil } @@ -237,26 +235,6 @@ func getFormatter(format string) log.Formatter { } } -func getCommands(options *RootOptions) (string, string) { - switch options.IPTablesMode { - case IPTablesModeLegacy: - if options.IPv6 { - return cmdLegacyIPv6, cmdLegacyIPv6Save - } - return cmdLegacy, cmdLegacySave - case IPTablesModeNFT: - if options.IPv6 { - return cmdNFTIPv6, cmdNFTIPv6Save - } - return cmdNFT, cmdNFTSave - default: - if options.IPv6 { - return cmdPlainIPv6, cmdPlainIPv6Save - } - return cmdPlain, cmdPlainSave - } -} - func setLogLevel(logLevel string) error { level, err := log.ParseLevel(logLevel) if err != nil { From a85e19d27d7edd19c7516b97373aca5bdb36198a Mon Sep 17 00:00:00 2001 From: Alex Leong Date: Fri, 24 Oct 2025 19:18:24 +0000 Subject: [PATCH 4/7] lints Signed-off-by: Alex Leong --- cni-plugin/main.go | 2 +- proxy-init/cmd/root.go | 13 ------------- 2 files changed, 1 insertion(+), 14 deletions(-) diff --git a/cni-plugin/main.go b/cni-plugin/main.go index 7bcb9378..e0d246e6 100644 --- a/cni-plugin/main.go +++ b/cni-plugin/main.go @@ -137,7 +137,7 @@ func configureLogging(logLevel, logFilePath string, maxSizeMB, maxAgeDays, maxCo // Ensure directory exists if dir := filepath.Dir(logFilePath); dir != "" && dir != "." { - _ = os.MkdirAll(dir, 0o755) + _ = os.MkdirAll(dir, 0o750) } // Configure log rotation with lumberjack diff --git a/proxy-init/cmd/root.go b/proxy-init/cmd/root.go index fc8b695c..f99319af 100644 --- a/proxy-init/cmd/root.go +++ b/proxy-init/cmd/root.go @@ -23,19 +23,6 @@ const ( IPTablesModePlain = "plain" // IPTablesModeAuto signals automatic detection of the iptables backend IPTablesModeAuto = "auto" - - cmdLegacy = "iptables-legacy" - cmdLegacySave = "iptables-legacy-save" - cmdLegacyIPv6 = "ip6tables-legacy" - cmdLegacyIPv6Save = "ip6tables-legacy-save" - cmdNFT = "iptables-nft" - cmdNFTSave = "iptables-nft-save" - cmdNFTIPv6 = "ip6tables-nft" - cmdNFTIPv6Save = "ip6tables-nft-save" - cmdPlain = "iptables" - cmdPlainSave = "iptables-save" - cmdPlainIPv6 = "ip6tables" - cmdPlainIPv6Save = "ip6tables-save" ) // RootOptions provides the information that will be used to build a firewall configuration. From 451c21fdacfc127a81b785ed9038375f37b33587 Mon Sep 17 00:00:00 2001 From: Alex Leong Date: Fri, 24 Oct 2025 21:11:40 +0000 Subject: [PATCH 5/7] Revert "lints" This reverts commit a85e19d27d7edd19c7516b97373aca5bdb36198a. --- cni-plugin/main.go | 2 +- proxy-init/cmd/root.go | 13 +++++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/cni-plugin/main.go b/cni-plugin/main.go index e0d246e6..7bcb9378 100644 --- a/cni-plugin/main.go +++ b/cni-plugin/main.go @@ -137,7 +137,7 @@ func configureLogging(logLevel, logFilePath string, maxSizeMB, maxAgeDays, maxCo // Ensure directory exists if dir := filepath.Dir(logFilePath); dir != "" && dir != "." { - _ = os.MkdirAll(dir, 0o750) + _ = os.MkdirAll(dir, 0o755) } // Configure log rotation with lumberjack diff --git a/proxy-init/cmd/root.go b/proxy-init/cmd/root.go index f99319af..fc8b695c 100644 --- a/proxy-init/cmd/root.go +++ b/proxy-init/cmd/root.go @@ -23,6 +23,19 @@ const ( IPTablesModePlain = "plain" // IPTablesModeAuto signals automatic detection of the iptables backend IPTablesModeAuto = "auto" + + cmdLegacy = "iptables-legacy" + cmdLegacySave = "iptables-legacy-save" + cmdLegacyIPv6 = "ip6tables-legacy" + cmdLegacyIPv6Save = "ip6tables-legacy-save" + cmdNFT = "iptables-nft" + cmdNFTSave = "iptables-nft-save" + cmdNFTIPv6 = "ip6tables-nft" + cmdNFTIPv6Save = "ip6tables-nft-save" + cmdPlain = "iptables" + cmdPlainSave = "iptables-save" + cmdPlainIPv6 = "ip6tables" + cmdPlainIPv6Save = "ip6tables-save" ) // RootOptions provides the information that will be used to build a firewall configuration. From 7205a44abd0c7bb85572bfd6f8fc06b9db533fdc Mon Sep 17 00:00:00 2001 From: Alex Leong Date: Fri, 24 Oct 2025 21:11:46 +0000 Subject: [PATCH 6/7] Revert "update detection logic" This reverts commit 13138e5dbdbeaef1f806dd3d913eb708efa1dcef. --- pkg/iptables/detect.go | 145 --------------------------------------- pkg/iptables/iptables.go | 65 +++++++++++++++++- proxy-init/cmd/root.go | 56 ++++++++++----- 3 files changed, 101 insertions(+), 165 deletions(-) delete mode 100644 pkg/iptables/detect.go diff --git a/pkg/iptables/detect.go b/pkg/iptables/detect.go deleted file mode 100644 index 22842113..00000000 --- a/pkg/iptables/detect.go +++ /dev/null @@ -1,145 +0,0 @@ -// Copyright (c) Linkerd authors -// -// Portions of the iptables backend detection code are derived from: -// https://github.com/projectcalico/calico/blob/master/felix/environment/feature_detect_linux.go -// Copyright (c) 2018-2025 Tigera, Inc. All rights reserved. -// Licensed under the Apache License, Version 2.0 - -package iptables - -import ( - "bytes" - "os/exec" - "strings" - - log "github.com/sirupsen/logrus" -) - -func countRulesInIptableOutput(in []byte) int { - count := 0 - for _, x := range bytes.Split(in, []byte("\n")) { - if len(x) >= 1 && x[0] == '-' { - count++ - } - } - return count -} - -// hasKubernetesChains tries to find in the output of the binary if the Kubernetes -// chains exists -func hasKubernetesChains(output []byte) bool { - return strings.Contains(string(output), "KUBE-IPTABLES-HINT") || strings.Contains(string(output), "KUBE-KUBELET-CANARY") -} - -// DetectBackend attempts to detect the iptables backend (nft or legacy) and the -// appropriate iptables commands to use. If the detected backend does not match -// the specifiedBackend, a warning is logged but the specifiedBackend is used. -// The specifiedBackend can be "nft", "legacy", "plain", or "auto" (to use the -// detected backend). -// -// Based on the backend selected, the appropriate iptables command paths are set -// in the FirewallConfiguration. -// -// This logic is adapted from the Calico CNI plugin: -// https://github.com/projectcalico/calico/blob/master/felix/environment/feature_detect_linux.go -func DetectBackend(fc *FirewallConfiguration, lookPath func(file string) (string, error), ipv6 bool, specifiedBackend string) { - prefix := "iptables" - if ipv6 { - prefix = "ip6tables" - } - - nftSave := FindBestBinary(lookPath, prefix, "nft", "save") - lgcySave := FindBestBinary(lookPath, prefix, "legacy", "save") - nftCmd := strings.TrimSuffix(nftSave, "-save") - lgcyCmd := strings.TrimSuffix(lgcySave, "-save") - - // Check kubelet canary chains in the mangle table for nft first. - nftMangle, _ := executeCommand(*fc, exec.Command(nftSave, "-t", "mangle")) - - var detectedBackend string - if hasKubernetesChains(nftMangle) { - detectedBackend = "nft" - } else { - // Check legacy mangle next. - lgcyMangle, _ := executeCommand(*fc, exec.Command(lgcySave, "-t", "mangle")) - - if hasKubernetesChains(lgcyMangle) { - detectedBackend = "legacy" - } else { - // Fall back to comparing total rule counts between full legacy vs nft saves. - lgcyAll, _ := executeCommand(*fc, exec.Command(lgcySave)) - nftAll, _ := executeCommand(*fc, exec.Command(nftSave)) - - legacyLines := countRulesInIptableOutput(lgcyAll) - nftLines := countRulesInIptableOutput(nftAll) - if legacyLines >= nftLines { - detectedBackend = "legacy" // default to legacy when tied or more legacy rules - } else { - detectedBackend = "nft" - } - } - } - - // Decide which backend to use, honoring a specific request if provided. - requested := strings.ToLower(specifiedBackend) - backendToUse := requested - if requested == "auto" { - log.WithField("detectedBackend", detectedBackend).Debug("Detected Iptables backend") - backendToUse = detectedBackend - } else if requested != detectedBackend { - log.WithFields(log.Fields{ - "detectedBackend": detectedBackend, - "specifiedBackend": requested, - }).Warn("Iptables backend specified does not match the detected backend; honoring specified backend") - } else { - log.WithField("specifiedBackend", specifiedBackend).Debug("Iptables backend specified matches detected backend") - } - - switch backendToUse { - case "legacy": - fc.BinPath = lgcyCmd - fc.SaveBinPath = lgcySave - case "nft": - fc.BinPath = nftCmd - fc.SaveBinPath = nftSave - case "plain": - fc.BinPath = prefix - fc.SaveBinPath = prefix + "-save" - } - - log.WithFields(log.Fields{ - "binPath": fc.BinPath, - "saveBinPath": fc.SaveBinPath, - }).Debug("Using iptables commands") -} - -// FindBestBinary tries to find an iptables binary for the specific variant (legacy/nftables mode) and returns the name -// of the binary. Falls back on iptables-restore/iptables-save if the specific variant isn't available. -// Panics if no binary can be found. -func FindBestBinary(lookPath func(file string) (string, error), prefix, backendMode, saveOrRestore string) string { - if lookPath == nil { - lookPath = exec.LookPath - } - candidates := []string{ - prefix + "-" + backendMode + "-" + saveOrRestore, - prefix + "-" + saveOrRestore, - } - - logCxt := log.WithFields(log.Fields{ - "prefix": prefix, - "backendMode": backendMode, - "saveOrRestore": saveOrRestore, - "candidates": candidates, - }) - - for _, candidate := range candidates { - _, err := lookPath(candidate) - if err == nil { - logCxt.WithField("command", candidate).Debug("Looked up iptables command") - return candidate - } - } - - logCxt.Panic("Failed to find iptables command") - return "" -} diff --git a/pkg/iptables/iptables.go b/pkg/iptables/iptables.go index ca892dc2..8f74c618 100644 --- a/pkg/iptables/iptables.go +++ b/pkg/iptables/iptables.go @@ -68,7 +68,8 @@ type FirewallConfiguration struct { func ConfigureFirewall(firewallConfiguration FirewallConfiguration) error { log.Debugf("tracing script execution as [%s]", executionTraceID) - // Using configured iptables binaries as-is. + // Before executing, ensure the configured iptables binaries exist; if not, attempt a fallback. + resolveBinFallback(&firewallConfiguration) existingRules, err := executeCommand(firewallConfiguration, firewallConfiguration.makeShowAllRules()) if err != nil { @@ -112,7 +113,8 @@ func CleanupFirewallConfig(firewallConfiguration FirewallConfiguration) error { log.Debugf("using '%s' to clean-up firewall rules", firewallConfiguration.BinPath) log.Debugf("using '%s' to list all available rules", firewallConfiguration.SaveBinPath) - // Using configured iptables binaries as-is for cleanup as well. + // Ensure binaries exist before attempting cleanup as well + resolveBinFallback(&firewallConfiguration) commands := make([]*exec.Cmd, 0) commands = firewallConfiguration.cleanupRules(commands) @@ -451,4 +453,61 @@ func asDestination(portRange util.PortRange) string { return fmt.Sprintf("%d:%d", portRange.LowerBound, portRange.UpperBound) } -// resolveBinFallback removed: binaries are assumed to be correctly set by callers. +// resolveBinFallback ensures the configured BinPath and SaveBinPath exist on PATH; if not, it +// tries reasonable alternatives of the same family (ip6tables vs iptables). Returns true if a +// fallback was applied. +func resolveBinFallback(fc *FirewallConfiguration) { + // helper to check presence + has := func(name string) bool { + _, err := exec.LookPath(name) + return err == nil + } + + // Both present? nothing to do + if has(fc.BinPath) && has(fc.SaveBinPath) { + log.WithFields(log.Fields{ + "requestedBin": fc.BinPath, + "requestedSaveBin": fc.SaveBinPath, + }).Debug("iptables: using configured binaries") + return + } + + // Decide family based on current name + ipv6 := strings.Contains(fc.BinPath, "ip6tables") || strings.Contains(fc.SaveBinPath, "ip6tables") + + // Candidate orders: prefer nft, then plain, then legacy + var candidates [][2]string + if ipv6 { + candidates = [][2]string{ + {"ip6tables-nft", "ip6tables-nft-save"}, + {"ip6tables", "ip6tables-save"}, + {"ip6tables-legacy", "ip6tables-legacy-save"}, + } + } else { + candidates = [][2]string{ + {"iptables-nft", "iptables-nft-save"}, + {"iptables", "iptables-save"}, + {"iptables-legacy", "iptables-legacy-save"}, + } + } + + // Use first candidate where both exist + for _, pair := range candidates { + if has(pair[0]) && has(pair[1]) { + if pair[0] != fc.BinPath || pair[1] != fc.SaveBinPath { + log.WithFields(log.Fields{ + "requestedBin": fc.BinPath, + "requestedSaveBin": fc.SaveBinPath, + "fallbackBin": pair[0], + "fallbackSaveBin": pair[1], + }).Warn("iptables: configured binaries not found; applying fallback to available binaries") + } + fc.BinPath = pair[0] + fc.SaveBinPath = pair[1] + return + } + } + + // No candidates found; keep as-is and let execution fail with a clear error later + log.WithFields(log.Fields{"binPath": fc.BinPath, "saveBinPath": fc.SaveBinPath}).Error("iptables: no suitable binaries found on PATH; commands may fail") +} diff --git a/proxy-init/cmd/root.go b/proxy-init/cmd/root.go index fc8b695c..b31dbf8a 100644 --- a/proxy-init/cmd/root.go +++ b/proxy-init/cmd/root.go @@ -21,8 +21,6 @@ const ( // IPTablesModePlain signals the usage of the iptables commands, which // can be either legacy or nft IPTablesModePlain = "plain" - // IPTablesModeAuto signals automatic detection of the iptables backend - IPTablesModeAuto = "auto" cmdLegacy = "iptables-legacy" cmdLegacySave = "iptables-legacy-save" @@ -167,11 +165,21 @@ func NewRootCmd() *cobra.Command { // BuildFirewallConfiguration returns an iptables FirewallConfiguration suitable to use to configure iptables. func BuildFirewallConfiguration(options *RootOptions) (*iptables.FirewallConfiguration, error) { - if options.IPTablesMode != "" && - options.IPTablesMode != IPTablesModeLegacy && - options.IPTablesMode != IPTablesModeNFT && - options.IPTablesMode != IPTablesModePlain { - return nil, fmt.Errorf("--iptables-mode valid values are only \"%s\", \"%s\", \"%s\", and \"%s\"", IPTablesModeLegacy, IPTablesModeNFT, IPTablesModeAuto, IPTablesModePlain) + if options.IPTablesMode != "" && options.IPTablesMode != IPTablesModeLegacy && options.IPTablesMode != IPTablesModeNFT && options.IPTablesMode != IPTablesModePlain { + return nil, fmt.Errorf("--iptables-mode valid values are only \"%s\", \"%s\" and \"%s\"", IPTablesModeLegacy, IPTablesModeNFT, IPTablesModePlain) + } + + if options.IPTablesMode == "" { + switch options.FirewallBinPath { + case "", cmdLegacy: + options.IPTablesMode = IPTablesModeLegacy + case cmdNFT: + options.IPTablesMode = IPTablesModeNFT + case cmdPlain: + options.IPTablesMode = IPTablesModePlain + default: + return nil, fmt.Errorf("--firewall-bin-path valid values are only \"%s\", \"%s\" and \"%s\"", cmdLegacy, cmdNFT, cmdPlain) + } } if !util.IsValidPort(options.IncomingProxyPort) { @@ -182,6 +190,8 @@ func BuildFirewallConfiguration(options *RootOptions) (*iptables.FirewallConfigu return nil, fmt.Errorf("--outgoing-proxy-port must be a valid TCP port number") } + cmd, cmdSave := getCommands(options) + sanitizedSubnets := []string{} for _, subnet := range options.SubnetsToIgnore { subnet := strings.TrimSpace(subnet) @@ -205,6 +215,8 @@ func BuildFirewallConfiguration(options *RootOptions) (*iptables.FirewallConfigu SimulateOnly: options.SimulateOnly, NetNs: options.NetNs, UseWaitFlag: options.UseWaitFlag, + BinPath: cmd, + SaveBinPath: cmdSave, } if len(options.PortsToRedirect) > 0 { @@ -213,16 +225,6 @@ func BuildFirewallConfiguration(options *RootOptions) (*iptables.FirewallConfigu firewallConfiguration.Mode = iptables.RedirectAllMode } - // For backwards-compatibility, if IPTablesMode is not set, use the FirewallBinPath - // explicitly set by the user. - if options.IPTablesMode == "" { - firewallConfiguration.BinPath = options.FirewallBinPath - firewallConfiguration.SaveBinPath = options.FirewallSaveBinPath - } else { - // Otherwise, detect and set the appropriate backend. - iptables.DetectBackend(firewallConfiguration, exec.LookPath, options.IPv6, options.IPTablesMode) - } - return firewallConfiguration, nil } @@ -235,6 +237,26 @@ func getFormatter(format string) log.Formatter { } } +func getCommands(options *RootOptions) (string, string) { + switch options.IPTablesMode { + case IPTablesModeLegacy: + if options.IPv6 { + return cmdLegacyIPv6, cmdLegacyIPv6Save + } + return cmdLegacy, cmdLegacySave + case IPTablesModeNFT: + if options.IPv6 { + return cmdNFTIPv6, cmdNFTIPv6Save + } + return cmdNFT, cmdNFTSave + default: + if options.IPv6 { + return cmdPlainIPv6, cmdPlainIPv6Save + } + return cmdPlain, cmdPlainSave + } +} + func setLogLevel(logLevel string) error { level, err := log.ParseLevel(logLevel) if err != nil { From 5c06f70585a36e2bf6c3b7c9edc9e380a3f6c39c Mon Sep 17 00:00:00 2001 From: Alex Leong Date: Fri, 24 Oct 2025 21:47:01 +0000 Subject: [PATCH 7/7] add test Signed-off-by: Alex Leong --- pkg/iptables/iptables.go | 11 ++-- pkg/iptables/resolve_fallback_test.go | 89 +++++++++++++++++++++++++++ 2 files changed, 94 insertions(+), 6 deletions(-) create mode 100644 pkg/iptables/resolve_fallback_test.go diff --git a/pkg/iptables/iptables.go b/pkg/iptables/iptables.go index 8f74c618..a3d0b900 100644 --- a/pkg/iptables/iptables.go +++ b/pkg/iptables/iptables.go @@ -69,7 +69,7 @@ func ConfigureFirewall(firewallConfiguration FirewallConfiguration) error { log.Debugf("tracing script execution as [%s]", executionTraceID) // Before executing, ensure the configured iptables binaries exist; if not, attempt a fallback. - resolveBinFallback(&firewallConfiguration) + resolveBinFallback(&firewallConfiguration, exec.LookPath) existingRules, err := executeCommand(firewallConfiguration, firewallConfiguration.makeShowAllRules()) if err != nil { @@ -114,7 +114,7 @@ func CleanupFirewallConfig(firewallConfiguration FirewallConfiguration) error { log.Debugf("using '%s' to list all available rules", firewallConfiguration.SaveBinPath) // Ensure binaries exist before attempting cleanup as well - resolveBinFallback(&firewallConfiguration) + resolveBinFallback(&firewallConfiguration, exec.LookPath) commands := make([]*exec.Cmd, 0) commands = firewallConfiguration.cleanupRules(commands) @@ -454,12 +454,11 @@ func asDestination(portRange util.PortRange) string { } // resolveBinFallback ensures the configured BinPath and SaveBinPath exist on PATH; if not, it -// tries reasonable alternatives of the same family (ip6tables vs iptables). Returns true if a -// fallback was applied. -func resolveBinFallback(fc *FirewallConfiguration) { +// tries reasonable alternatives of the same family (ip6tables vs iptables). +func resolveBinFallback(fc *FirewallConfiguration, lookPath func(string) (string, error)) { // helper to check presence has := func(name string) bool { - _, err := exec.LookPath(name) + _, err := lookPath(name) return err == nil } diff --git a/pkg/iptables/resolve_fallback_test.go b/pkg/iptables/resolve_fallback_test.go new file mode 100644 index 00000000..505af390 --- /dev/null +++ b/pkg/iptables/resolve_fallback_test.go @@ -0,0 +1,89 @@ +package iptables + +import ( + "errors" + "testing" +) + +// fakeLookPath returns a LookPath-like function backed by a set of available names. +func fakeLookPath(available []string) func(string) (string, error) { + return func(name string) (string, error) { + for _, a := range available { + if a == name { + return "/fake/" + name, nil + } + } + return "", errors.New("not found") + } +} + +func TestResolveBinFallback_KeepWhenPresent(t *testing.T) { + fc := &FirewallConfiguration{BinPath: "iptables-nft", SaveBinPath: "iptables-nft-save"} + lp := fakeLookPath([]string{ + "iptables-nft", + "iptables-nft-save", + }) + + resolveBinFallback(fc, lp) + + if fc.BinPath != "iptables-nft" || fc.SaveBinPath != "iptables-nft-save" { + t.Fatalf("expected to keep configured binaries, got bin=%q save=%q", fc.BinPath, fc.SaveBinPath) + } +} + +func TestResolveBinFallback_FallbackToNFT_IPv4(t *testing.T) { + fc := &FirewallConfiguration{BinPath: "iptables-notreal", SaveBinPath: "iptables-notreal-save"} + lp := fakeLookPath([]string{ + // Only nft pair is available + "iptables-nft", + "iptables-nft-save", + }) + + resolveBinFallback(fc, lp) + + if fc.BinPath != "iptables-nft" || fc.SaveBinPath != "iptables-nft-save" { + t.Fatalf("expected fallback to iptables-nft, got bin=%q save=%q", fc.BinPath, fc.SaveBinPath) + } +} + +func TestResolveBinFallback_FallbackOrder_Plain(t *testing.T) { + fc := &FirewallConfiguration{BinPath: "iptables-missing", SaveBinPath: "iptables-missing-save"} + lp := fakeLookPath([]string{ + // Only plain iptables present + "iptables", + "iptables-save", + }) + + resolveBinFallback(fc, lp) + + if fc.BinPath != "iptables" || fc.SaveBinPath != "iptables-save" { + t.Fatalf("expected fallback to iptables/iptable-save, got bin=%q save=%q", fc.BinPath, fc.SaveBinPath) + } +} + +func TestResolveBinFallback_IPv6_FallbackLegacy(t *testing.T) { + fc := &FirewallConfiguration{BinPath: "ip6tables-missing", SaveBinPath: "ip6tables-missing-save"} + lp := fakeLookPath([]string{ + // Only legacy pair present for IPv6 + "ip6tables-legacy", + "ip6tables-legacy-save", + }) + + resolveBinFallback(fc, lp) + + if fc.BinPath != "ip6tables-legacy" || fc.SaveBinPath != "ip6tables-legacy-save" { + t.Fatalf("expected fallback to ip6tables-legacy, got bin=%q save=%q", fc.BinPath, fc.SaveBinPath) + } +} + +func TestResolveBinFallback_NoCandidates(t *testing.T) { + origBin, origSave := "iptables-missing", "iptables-missing-save" + fc := &FirewallConfiguration{BinPath: origBin, SaveBinPath: origSave} + lp := fakeLookPath([]string{}) + + resolveBinFallback(fc, lp) + + if fc.BinPath != origBin || fc.SaveBinPath != origSave { + t.Fatalf("expected no change when no candidates found, got bin=%q save=%q", fc.BinPath, fc.SaveBinPath) + } +}