From 29582b012709cc0e7f76c796595c895dddd0c656 Mon Sep 17 00:00:00 2001 From: Alejandro Pedraza Date: Mon, 11 Mar 2024 14:20:52 -0500 Subject: [PATCH 1/4] Add IPv6/dual-stack support ## Flags Changes This replaces the proxy-init flags `--firewall-bin-path` and `--firewall-save-bin-path` with the flag `--iptables-mode` (with possible values `legacy` and `nft`). Also the `--ipv6` flag has been added (default `true`). Proxy-init won't be relying just on the iptables commands family (iptables-legacy, iptables-legacy-save, iptables-nft, iptable-nft-save), but also on the ip6tables command family, so it's better to know the mode we're in (legacy or nft) and whether ipv6 is enabled, to determine all the commands that need to be used instead of directly passing them as arguments. After the set of rules run via iptables are processed, if `--ipv6` is true (which is the default), the same set of rules will be run via ip6tables. Analog changes were applied to linkerd-cni as well. ## Backwards-Compatibility This is backwards-compatible with older control planes as long as the older flags are not used. If those flags are used, an explanatory error is thrown (better than showing a deprecation message and failing later when legacy/nft iptables don't work). If `--ipv6` is not passed (and thus defaults to true), this doesn't impact operation even if the cluster doesn't support IPv6; the ip6tables rules are applied but they're innocuous. OTOH if there's no kernel support for IPv6 then the ip6tables command will fail but we'll just log the failure and not fail the linkerd-init container (nor the `add` command for linkerd-cni). This avoids having to explicitly set `--ipv6=false`, but it can be set if the user is aware of such limitations and wants to get rid of the errors. ## Linkerd IPv6 Support This allows routing IPv6 traffic to the proxy, but is just the first step towards IPv6/dual-stack support. Control plane and proxy changes will come up next. --- .../manifests/calico/linkerd-cni.yaml | 4 +- .../manifests/cilium/linkerd-cni.yaml | 4 +- .../manifests/flannel/linkerd-cni.yaml | 4 +- cni-plugin/integration/testutil/test_util.go | 3 + cni-plugin/main.go | 40 ++++++-- justfile | 1 + proxy-init/cmd/root.go | 99 ++++++++++++++++--- proxy-init/cmd/root_test.go | 12 ++- 8 files changed, 140 insertions(+), 27 deletions(-) diff --git a/cni-plugin/integration/manifests/calico/linkerd-cni.yaml b/cni-plugin/integration/manifests/calico/linkerd-cni.yaml index 05f110bd..df861381 100644 --- a/cni-plugin/integration/manifests/calico/linkerd-cni.yaml +++ b/cni-plugin/integration/manifests/calico/linkerd-cni.yaml @@ -80,7 +80,9 @@ data: "ports-to-redirect": [], "inbound-ports-to-ignore": ["4191","4190"], "simulate": false, - "use-wait-flag": false + "use-wait-flag": false, + "iptables-mode": "legacy", + "ipv6": true } } --- diff --git a/cni-plugin/integration/manifests/cilium/linkerd-cni.yaml b/cni-plugin/integration/manifests/cilium/linkerd-cni.yaml index 05f110bd..df861381 100644 --- a/cni-plugin/integration/manifests/cilium/linkerd-cni.yaml +++ b/cni-plugin/integration/manifests/cilium/linkerd-cni.yaml @@ -80,7 +80,9 @@ data: "ports-to-redirect": [], "inbound-ports-to-ignore": ["4191","4190"], "simulate": false, - "use-wait-flag": false + "use-wait-flag": false, + "iptables-mode": "legacy", + "ipv6": true } } --- diff --git a/cni-plugin/integration/manifests/flannel/linkerd-cni.yaml b/cni-plugin/integration/manifests/flannel/linkerd-cni.yaml index 5b9166ad..d7216f4d 100644 --- a/cni-plugin/integration/manifests/flannel/linkerd-cni.yaml +++ b/cni-plugin/integration/manifests/flannel/linkerd-cni.yaml @@ -84,7 +84,9 @@ data: "ports-to-redirect": [], "inbound-ports-to-ignore": ["4191","4190"], "simulate": false, - "use-wait-flag": false + "use-wait-flag": false, + "iptables-mode": "legacy", + "ipv6": true } } --- diff --git a/cni-plugin/integration/testutil/test_util.go b/cni-plugin/integration/testutil/test_util.go index 3d2429e9..1691e11d 100644 --- a/cni-plugin/integration/testutil/test_util.go +++ b/cni-plugin/integration/testutil/test_util.go @@ -49,8 +49,11 @@ type ProxyInit struct { PortsToRedirect []int `json:"ports-to-redirect"` InboundPortsToIgnore []string `json:"inbound-ports-to-ignore"` OutboundPortsToIgnore []string `json:"outbound-ports-to-ignore"` + SubnetsToIgnore []string `json:"subnets-to-ignore"` Simulate bool `json:"simulate"` UseWaitFlag bool `json:"use-wait-flag"` + IPTablesMode string `json:"iptables-mode"` + IPv6 bool `json:"ipv6"` } // LinkerdPlugin is what we use for CNI configuration in the plugins section diff --git a/cni-plugin/main.go b/cni-plugin/main.go index 9eb68980..10ca2e68 100644 --- a/cni-plugin/main.go +++ b/cni-plugin/main.go @@ -52,6 +52,8 @@ type ProxyInit struct { SubnetsToIgnore []string `json:"subnets-to-ignore"` Simulate bool `json:"simulate"` UseWaitFlag bool `json:"use-wait-flag"` + IPTablesMode string `json:"iptables-mode"` + IPv6 bool `json:"ipv6"` } // Kubernetes a K8s specific struct to hold config @@ -219,8 +221,8 @@ func cmdAdd(args *skel.CmdArgs) error { SimulateOnly: conf.ProxyInit.Simulate, NetNs: args.Netns, UseWaitFlag: conf.ProxyInit.UseWaitFlag, - FirewallBinPath: "iptables-legacy", - FirewallSaveBinPath: "iptables-legacy-save", + IPTablesMode: conf.ProxyInit.IPTablesMode, + IPv6: conf.ProxyInit.IPv6, } // Check if there are any overridden ports to be skipped @@ -292,17 +294,19 @@ func cmdAdd(args *skel.CmdArgs) error { options.OutboundPortsToIgnore = append(options.OutboundPortsToIgnore, skippedPorts...) } - firewallConfiguration, err := cmd.BuildFirewallConfiguration(&options) - if err != nil { - logEntry.Errorf("linkerd-cni: could not create a Firewall Configuration from the options: %v", options) - return err + // This ensures BC against linkerd2-cni older versions not yet passing this flag + if options.IPTablesMode == "" { + options.IPTablesMode = cmd.IPTablesModeLegacy } - err = iptables.ConfigureFirewall(*firewallConfiguration) - if err != nil { - logEntry.Errorf("linkerd-cni: could not configure firewall: %s", err) + if err := buildAndConfigure(logEntry, &options, false); err != nil { return err } + if options.IPv6 { + if err := buildAndConfigure(logEntry, &options, true); err != nil { + return err + } + } } else { if containsInitContainer { logEntry.Debug("linkerd-cni: linkerd-init initContainer is present, skipping.") @@ -353,6 +357,24 @@ func getAPIServerPorts(ctx context.Context, api *kubernetes.Clientset) ([]string return ports, nil } +func buildAndConfigure(logEntry *logrus.Entry, options *cmd.RootOptions, ipv6 bool) error { + firewallConfiguration, err := cmd.BuildFirewallConfiguration(options, ipv6) + if err != nil { + logEntry.Errorf("linkerd-cni: could not create a Firewall Configuration from the options: %v", options) + return err + } + + err = iptables.ConfigureFirewall(*firewallConfiguration) + // We couldn't find a robust way of checking IPv6 support besides trying to just call ip6tables-save. + // If IPv4 rules worked but not IPv6, let's not fail the container (the actual problem will get logged). + if !ipv6 && err != nil { + logEntry.Errorf("linkerd-cni: could not configure firewall: %s", err) + return err + } + + return nil +} + func getAnnotationOverride(ctx context.Context, api *kubernetes.Clientset, pod *v1.Pod, key string) (string, error) { // Check if the annotation is present on the pod if override := pod.GetObjectMeta().GetAnnotations()[key]; override != "" { diff --git a/justfile b/justfile index ba3cb506..b279fd34 100644 --- a/justfile +++ b/justfile @@ -224,6 +224,7 @@ _cni-plugin-setup-cilium: echo "Mounted /sys/fs/bpf to cilium-test-server cluster" helm repo add cilium https://helm.cilium.io/ helm install cilium cilium/cilium --version 1.13.0 \ + --kube-context k3d-l5d-cilium-test \ --namespace kube-system \ --set kubeProxyReplacement=partial \ --set hostServices.enabled=false \ diff --git a/proxy-init/cmd/root.go b/proxy-init/cmd/root.go index 3c8fa8f7..5c90dd6a 100644 --- a/proxy-init/cmd/root.go +++ b/proxy-init/cmd/root.go @@ -1,6 +1,7 @@ package cmd import ( + "errors" "fmt" "net" "os/exec" @@ -13,6 +14,22 @@ import ( "github.com/linkerd/linkerd2-proxy-init/internal/util" ) +const ( + // IPTablesModeLegacy signals the usage of the iptables-legacy commands + IPTablesModeLegacy = "legacy" + // ipTablesModeNFT signals the usage of the iptables-nft commands + ipTablesModeNFT = "nft" + + 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" +) + // RootOptions provides the information that will be used to build a firewall configuration. type RootOptions struct { IncomingProxyPort int @@ -28,8 +45,12 @@ type RootOptions struct { TimeoutCloseWaitSecs int LogFormat string LogLevel string - FirewallBinPath string - FirewallSaveBinPath string + IPTablesMode string + IPv6 bool + + // No longer supported + FirewallBinPath string + FirewallSaveBinPath string } func newRootOptions() *RootOptions { @@ -47,8 +68,8 @@ func newRootOptions() *RootOptions { TimeoutCloseWaitSecs: 0, LogFormat: "plain", LogLevel: "info", - FirewallBinPath: "iptables-legacy", - FirewallSaveBinPath: "iptables-legacy-save", + IPTablesMode: IPTablesModeLegacy, + IPv6: true, } } @@ -61,7 +82,7 @@ func NewRootCmd() *cobra.Command { Use: "proxy-init", Short: "proxy-init adds a Kubernetes pod to the Linkerd service mesh", Long: "proxy-init adds a Kubernetes pod to the Linkerd service mesh.", - RunE: func(cmd *cobra.Command, args []string) error { + RunE: func(_ *cobra.Command, _ []string) error { if options.TimeoutCloseWaitSecs != 0 { sysctl := exec.Command("sysctl", "-w", @@ -75,16 +96,35 @@ func NewRootCmd() *cobra.Command { log.Info(string(out)) } - config, err := BuildFirewallConfiguration(options) + log.SetFormatter(getFormatter(options.LogFormat)) + err := setLogLevel(options.LogLevel) if err != nil { return err } - log.SetFormatter(getFormatter(options.LogFormat)) - err = setLogLevel(options.LogLevel) + + config, err := BuildFirewallConfiguration(options, false) if err != nil { return err } - return iptables.ConfigureFirewall(*config) + + if err = iptables.ConfigureFirewall(*config); err != nil { + return err + } + + if !options.IPv6 { + return nil + } + + config, err = BuildFirewallConfiguration(options, true) + if err != nil { + return err + } + + // We couldn't find a robust way of checking IPv6 support besides trying to just call ip6tables-save. + // If IPv4 rules worked but not IPv6, let's not fail the container (the actual problem will get logged). + _ = iptables.ConfigureFirewall(*config) + + return nil }, } @@ -101,13 +141,31 @@ func NewRootCmd() *cobra.Command { cmd.PersistentFlags().IntVar(&options.TimeoutCloseWaitSecs, "timeout-close-wait-secs", options.TimeoutCloseWaitSecs, "Sets nf_conntrack_tcp_timeout_close_wait") cmd.PersistentFlags().StringVar(&options.LogFormat, "log-format", options.LogFormat, "Configure log format ('plain' or 'json')") cmd.PersistentFlags().StringVar(&options.LogLevel, "log-level", options.LogLevel, "Configure log level") + cmd.PersistentFlags().StringVar(&options.IPTablesMode, "iptables-mode", options.IPTablesMode, "Variant of iptables command to use (\"legacy\" or \"nft\")") + cmd.PersistentFlags().BoolVar(&options.IPv6, "ipv6", options.IPv6, "Set rules both via iptables and ip6tables to support dual-stack networking") cmd.PersistentFlags().StringVar(&options.FirewallBinPath, "firewall-bin-path", options.FirewallBinPath, "Path to iptables binary") cmd.PersistentFlags().StringVar(&options.FirewallSaveBinPath, "firewall-save-bin-path", options.FirewallSaveBinPath, "Path to iptables-save binary") + + if err := cmd.PersistentFlags().MarkHidden("firewall-bin-path"); err != nil { + log.Fatal(err) + } + if err := cmd.PersistentFlags().MarkHidden("firewall-save-bin-path"); err != nil { + log.Fatal(err) + } + return cmd } // BuildFirewallConfiguration returns an iptables FirewallConfiguration suitable to use to configure iptables. -func BuildFirewallConfiguration(options *RootOptions) (*iptables.FirewallConfiguration, error) { +func BuildFirewallConfiguration(options *RootOptions, ipv6 bool) (*iptables.FirewallConfiguration, error) { + if options.FirewallBinPath != "" || options.FirewallSaveBinPath != "" { + return nil, errors.New("--firewal-bin-path and firewall-save-bin-path are no longer supported; please use --iptables-mode instead") + } + + if options.IPTablesMode != IPTablesModeLegacy && options.IPTablesMode != ipTablesModeNFT { + return nil, errors.New("--iptables-mode valid values are only \"legacy\" and \"nft\"") + } + if !util.IsValidPort(options.IncomingProxyPort) { return nil, fmt.Errorf("--incoming-proxy-port must be a valid TCP port number") } @@ -116,6 +174,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.IPTablesMode, ipv6) + sanitizedSubnets := []string{} for _, subnet := range options.SubnetsToIgnore { subnet := strings.TrimSpace(subnet) @@ -138,8 +198,8 @@ func BuildFirewallConfiguration(options *RootOptions) (*iptables.FirewallConfigu SimulateOnly: options.SimulateOnly, NetNs: options.NetNs, UseWaitFlag: options.UseWaitFlag, - BinPath: options.FirewallBinPath, - SaveBinPath: options.FirewallSaveBinPath, + BinPath: cmd, + SaveBinPath: cmdSave, } if len(options.PortsToRedirect) > 0 { @@ -160,6 +220,21 @@ func getFormatter(format string) log.Formatter { } } +func getCommands(mode string, ipv6 bool) (string, string) { + if mode == IPTablesModeLegacy { + if ipv6 { + return cmdLegacyIPv6, cmdLegacyIPv6Save + } + return cmdLegacy, cmdLegacySave + } + + if ipv6 { + return cmdNFTIPv6, cmdNFTIPv6Save + } + + return cmdNFT, cmdNFTSave +} + func setLogLevel(logLevel string) error { level, err := log.ParseLevel(logLevel) if err != nil { diff --git a/proxy-init/cmd/root_test.go b/proxy-init/cmd/root_test.go index d1b31b28..49d79d66 100644 --- a/proxy-init/cmd/root_test.go +++ b/proxy-init/cmd/root_test.go @@ -32,7 +32,7 @@ func TestBuildFirewallConfiguration(t *testing.T) { options.OutgoingProxyPort = expectedOutgoingProxyPort options.ProxyUserID = expectedProxyUserID - config, err := BuildFirewallConfiguration(options) + config, err := BuildFirewallConfiguration(options, false) if err != nil { t.Fatalf("Unexpected error: %s", err) } @@ -51,6 +51,7 @@ func TestBuildFirewallConfiguration(t *testing.T) { options: &RootOptions{ IncomingProxyPort: -1, OutgoingProxyPort: 1234, + IPTablesMode: IPTablesModeLegacy, }, errorMessage: "--incoming-proxy-port must be a valid TCP port number", }, @@ -58,6 +59,7 @@ func TestBuildFirewallConfiguration(t *testing.T) { options: &RootOptions{ IncomingProxyPort: 100000, OutgoingProxyPort: 1234, + IPTablesMode: IPTablesModeLegacy, }, errorMessage: "--incoming-proxy-port must be a valid TCP port number", }, @@ -65,6 +67,7 @@ func TestBuildFirewallConfiguration(t *testing.T) { options: &RootOptions{ IncomingProxyPort: 1234, OutgoingProxyPort: -1, + IPTablesMode: IPTablesModeLegacy, }, errorMessage: "--outgoing-proxy-port must be a valid TCP port number", }, @@ -72,17 +75,19 @@ func TestBuildFirewallConfiguration(t *testing.T) { options: &RootOptions{ IncomingProxyPort: 1234, OutgoingProxyPort: 100000, + IPTablesMode: IPTablesModeLegacy, }, errorMessage: "--outgoing-proxy-port must be a valid TCP port number", }, { options: &RootOptions{ SubnetsToIgnore: []string{"1.1.1.1/24", "0.0.0.0"}, + IPTablesMode: IPTablesModeLegacy, }, errorMessage: "0.0.0.0 is not a valid CIDR address", }, } { - _, err := BuildFirewallConfiguration(tt.options) + _, err := BuildFirewallConfiguration(tt.options, false) if err == nil { t.Fatalf("Expected error for config [%v], got nil", tt.options) } @@ -102,11 +107,12 @@ func TestBuildFirewallConfiguration(t *testing.T) { // Tests that subnets are parsed properly and trimmed of excess whitespace options: &RootOptions{ SubnetsToIgnore: []string{"1.1.1.1/24 "}, + IPTablesMode: IPTablesModeLegacy, }, errorMessage: "", }, } { - _, err := BuildFirewallConfiguration(tt.options) + _, err := BuildFirewallConfiguration(tt.options, false) if err != nil { t.Fatalf("Got error error for config [%v]", tt.options) } From 61633371e754ff3008c3adbb86b60c5cddb39961 Mon Sep 17 00:00:00 2001 From: Alejandro Pedraza Date: Thu, 14 Mar 2024 09:51:57 -0500 Subject: [PATCH 2/4] matrix test for legacy/nft --- .github/workflows/cni-plugin-integration.yml | 29 +++++-------------- .../manifests/calico/linkerd-cni.yaml | 2 +- .../manifests/cilium/linkerd-cni.yaml | 2 +- .../manifests/flannel/linkerd-cni.yaml | 2 +- cni-plugin/integration/run.sh | 8 +++-- 5 files changed, 16 insertions(+), 27 deletions(-) diff --git a/.github/workflows/cni-plugin-integration.yml b/.github/workflows/cni-plugin-integration.yml index 4e80b8e4..d08c7826 100644 --- a/.github/workflows/cni-plugin-integration.yml +++ b/.github/workflows/cni-plugin-integration.yml @@ -12,33 +12,20 @@ on: - justfile* jobs: - cni-flannel-test: - continue-on-error: true - timeout-minutes: 15 - runs-on: ubuntu-latest - steps: - - uses: linkerd/dev/actions/setup-tools@v43 - - uses: actions/checkout@9bb56186c3b09b4f86b1c65136769dd318469633 - - name: Run CNI integration tests - run: just cni-plugin-test-integration-flannel - cni-calico-test: - continue-on-error: true - timeout-minutes: 15 - runs-on: ubuntu-latest - steps: - - uses: linkerd/dev/actions/setup-tools@v43 - - uses: actions/checkout@9bb56186c3b09b4f86b1c65136769dd318469633 - - name: Run CNI integration tests - run: just cni-plugin-test-integration-calico - cni-cilium-test: - continue-on-error: true + cni-test: + strategy: + matrix: + cni: [flannel, calico, cilium] + iptables-mode: [legacy, nft] timeout-minutes: 15 runs-on: ubuntu-latest steps: - uses: linkerd/dev/actions/setup-tools@v43 - uses: actions/checkout@9bb56186c3b09b4f86b1c65136769dd318469633 - name: Run CNI integration tests - run: just cni-plugin-test-integration-cilium + env: + IPTABLES_MODE: ${{ matrix.iptables-mode }} + run: just cni-plugin-test-integration-${{ matrix.cni }} ordering-test: continue-on-error: true timeout-minutes: 15 diff --git a/cni-plugin/integration/manifests/calico/linkerd-cni.yaml b/cni-plugin/integration/manifests/calico/linkerd-cni.yaml index df861381..ec009421 100644 --- a/cni-plugin/integration/manifests/calico/linkerd-cni.yaml +++ b/cni-plugin/integration/manifests/calico/linkerd-cni.yaml @@ -81,7 +81,7 @@ data: "inbound-ports-to-ignore": ["4191","4190"], "simulate": false, "use-wait-flag": false, - "iptables-mode": "legacy", + "iptables-mode": "$IPTABLES_MODE", "ipv6": true } } diff --git a/cni-plugin/integration/manifests/cilium/linkerd-cni.yaml b/cni-plugin/integration/manifests/cilium/linkerd-cni.yaml index df861381..ec009421 100644 --- a/cni-plugin/integration/manifests/cilium/linkerd-cni.yaml +++ b/cni-plugin/integration/manifests/cilium/linkerd-cni.yaml @@ -81,7 +81,7 @@ data: "inbound-ports-to-ignore": ["4191","4190"], "simulate": false, "use-wait-flag": false, - "iptables-mode": "legacy", + "iptables-mode": "$IPTABLES_MODE", "ipv6": true } } diff --git a/cni-plugin/integration/manifests/flannel/linkerd-cni.yaml b/cni-plugin/integration/manifests/flannel/linkerd-cni.yaml index d7216f4d..3774ca7a 100644 --- a/cni-plugin/integration/manifests/flannel/linkerd-cni.yaml +++ b/cni-plugin/integration/manifests/flannel/linkerd-cni.yaml @@ -85,7 +85,7 @@ data: "inbound-ports-to-ignore": ["4191","4190"], "simulate": false, "use-wait-flag": false, - "iptables-mode": "legacy", + "iptables-mode": "$IPTABLES_MODE", "ipv6": true } } diff --git a/cni-plugin/integration/run.sh b/cni-plugin/integration/run.sh index 29414462..3b51339a 100755 --- a/cni-plugin/integration/run.sh +++ b/cni-plugin/integration/run.sh @@ -4,9 +4,8 @@ set -euxo pipefail cd "${BASH_SOURCE[0]%/*}" -# Integration tests to run. Scenario is passed in as an environment variable. -# Default is 'flannel' SCENARIO=${CNI_TEST_SCENARIO:-flannel} +IPTABLES_MODE=${IPTABLES_MODE:-legacy} # Run kubectl with the correct context. function k() { @@ -25,7 +24,10 @@ function create_test_lab() { # can enable a testing matrix? # Apply all files in scenario directory. For non-flannel CNIs, this will # include the CNI manifest itself. - k apply -f "manifests/$SCENARIO/" + for f in ./manifests/"$SCENARIO"/*.yaml + do + envsubst < "$f" | k apply -f - + done } function cleanup() { From 03a7172d39c655c906f8b34bb70d7e3c30768655 Mon Sep 17 00:00:00 2001 From: Alejandro Pedraza Date: Thu, 14 Mar 2024 15:48:28 -0500 Subject: [PATCH 3/4] Avoid passing extra ipv6 flag around --- cni-plugin/main.go | 15 ++++++++++----- proxy-init/cmd/root.go | 26 +++++++++++++++----------- proxy-init/cmd/root_test.go | 7 ++++--- 3 files changed, 29 insertions(+), 19 deletions(-) diff --git a/cni-plugin/main.go b/cni-plugin/main.go index 10ca2e68..ab441e1d 100644 --- a/cni-plugin/main.go +++ b/cni-plugin/main.go @@ -299,11 +299,16 @@ func cmdAdd(args *skel.CmdArgs) error { options.IPTablesMode = cmd.IPTablesModeLegacy } - if err := buildAndConfigure(logEntry, &options, false); err != nil { + // always trigger the IPv4 rules + optIPv4 := options + optIPv4.IPv6 = false + if err := buildAndConfigure(logEntry, &optIPv4); err != nil { return err } + + // trigger the IPv6 rules if options.IPv6 { - if err := buildAndConfigure(logEntry, &options, true); err != nil { + if err := buildAndConfigure(logEntry, &options); err != nil { return err } } @@ -357,8 +362,8 @@ func getAPIServerPorts(ctx context.Context, api *kubernetes.Clientset) ([]string return ports, nil } -func buildAndConfigure(logEntry *logrus.Entry, options *cmd.RootOptions, ipv6 bool) error { - firewallConfiguration, err := cmd.BuildFirewallConfiguration(options, ipv6) +func buildAndConfigure(logEntry *logrus.Entry, options *cmd.RootOptions) error { + firewallConfiguration, err := cmd.BuildFirewallConfiguration(options) if err != nil { logEntry.Errorf("linkerd-cni: could not create a Firewall Configuration from the options: %v", options) return err @@ -367,7 +372,7 @@ func buildAndConfigure(logEntry *logrus.Entry, options *cmd.RootOptions, ipv6 bo err = iptables.ConfigureFirewall(*firewallConfiguration) // We couldn't find a robust way of checking IPv6 support besides trying to just call ip6tables-save. // If IPv4 rules worked but not IPv6, let's not fail the container (the actual problem will get logged). - if !ipv6 && err != nil { + if !options.IPv6 && err != nil { logEntry.Errorf("linkerd-cni: could not configure firewall: %s", err) return err } diff --git a/proxy-init/cmd/root.go b/proxy-init/cmd/root.go index 5c90dd6a..a3c8dc5a 100644 --- a/proxy-init/cmd/root.go +++ b/proxy-init/cmd/root.go @@ -17,8 +17,8 @@ import ( const ( // IPTablesModeLegacy signals the usage of the iptables-legacy commands IPTablesModeLegacy = "legacy" - // ipTablesModeNFT signals the usage of the iptables-nft commands - ipTablesModeNFT = "nft" + // IPTablesModeNFT signals the usage of the iptables-nft commands + IPTablesModeNFT = "nft" cmdLegacy = "iptables-legacy" cmdLegacySave = "iptables-legacy-save" @@ -102,7 +102,10 @@ func NewRootCmd() *cobra.Command { return err } - config, err := BuildFirewallConfiguration(options, false) + // always trigger the IPv4 rules + optIPv4 := *options + optIPv4.IPv6 = false + config, err := BuildFirewallConfiguration(&optIPv4) if err != nil { return err } @@ -115,7 +118,8 @@ func NewRootCmd() *cobra.Command { return nil } - config, err = BuildFirewallConfiguration(options, true) + // trigger the IPv6 rules + config, err = BuildFirewallConfiguration(options) if err != nil { return err } @@ -157,12 +161,12 @@ func NewRootCmd() *cobra.Command { } // BuildFirewallConfiguration returns an iptables FirewallConfiguration suitable to use to configure iptables. -func BuildFirewallConfiguration(options *RootOptions, ipv6 bool) (*iptables.FirewallConfiguration, error) { +func BuildFirewallConfiguration(options *RootOptions) (*iptables.FirewallConfiguration, error) { if options.FirewallBinPath != "" || options.FirewallSaveBinPath != "" { return nil, errors.New("--firewal-bin-path and firewall-save-bin-path are no longer supported; please use --iptables-mode instead") } - if options.IPTablesMode != IPTablesModeLegacy && options.IPTablesMode != ipTablesModeNFT { + if options.IPTablesMode != IPTablesModeLegacy && options.IPTablesMode != IPTablesModeNFT { return nil, errors.New("--iptables-mode valid values are only \"legacy\" and \"nft\"") } @@ -174,7 +178,7 @@ func BuildFirewallConfiguration(options *RootOptions, ipv6 bool) (*iptables.Fire return nil, fmt.Errorf("--outgoing-proxy-port must be a valid TCP port number") } - cmd, cmdSave := getCommands(options.IPTablesMode, ipv6) + cmd, cmdSave := getCommands(options) sanitizedSubnets := []string{} for _, subnet := range options.SubnetsToIgnore { @@ -220,15 +224,15 @@ func getFormatter(format string) log.Formatter { } } -func getCommands(mode string, ipv6 bool) (string, string) { - if mode == IPTablesModeLegacy { - if ipv6 { +func getCommands(options *RootOptions) (string, string) { + if options.IPTablesMode == IPTablesModeLegacy { + if options.IPv6 { return cmdLegacyIPv6, cmdLegacyIPv6Save } return cmdLegacy, cmdLegacySave } - if ipv6 { + if options.IPv6 { return cmdNFTIPv6, cmdNFTIPv6Save } diff --git a/proxy-init/cmd/root_test.go b/proxy-init/cmd/root_test.go index 49d79d66..5a2ca97b 100644 --- a/proxy-init/cmd/root_test.go +++ b/proxy-init/cmd/root_test.go @@ -31,8 +31,9 @@ func TestBuildFirewallConfiguration(t *testing.T) { options.IncomingProxyPort = expectedIncomingProxyPort options.OutgoingProxyPort = expectedOutgoingProxyPort options.ProxyUserID = expectedProxyUserID + options.IPv6 = false - config, err := BuildFirewallConfiguration(options, false) + config, err := BuildFirewallConfiguration(options) if err != nil { t.Fatalf("Unexpected error: %s", err) } @@ -87,7 +88,7 @@ func TestBuildFirewallConfiguration(t *testing.T) { errorMessage: "0.0.0.0 is not a valid CIDR address", }, } { - _, err := BuildFirewallConfiguration(tt.options, false) + _, err := BuildFirewallConfiguration(tt.options) if err == nil { t.Fatalf("Expected error for config [%v], got nil", tt.options) } @@ -112,7 +113,7 @@ func TestBuildFirewallConfiguration(t *testing.T) { errorMessage: "", }, } { - _, err := BuildFirewallConfiguration(tt.options, false) + _, err := BuildFirewallConfiguration(tt.options) if err != nil { t.Fatalf("Got error error for config [%v]", tt.options) } From d156e1a6361c77b8754f6b4c1b173056f5d0df1e Mon Sep 17 00:00:00 2001 From: Alejandro Pedraza Date: Tue, 19 Mar 2024 16:56:22 -0500 Subject: [PATCH 4/4] Re-add support for firewall-bin-path, firewall-save-bin-path --- proxy-init/cmd/root.go | 38 +++++++++++++++++++------------------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/proxy-init/cmd/root.go b/proxy-init/cmd/root.go index a3c8dc5a..6df5852b 100644 --- a/proxy-init/cmd/root.go +++ b/proxy-init/cmd/root.go @@ -1,7 +1,6 @@ package cmd import ( - "errors" "fmt" "net" "os/exec" @@ -45,12 +44,10 @@ type RootOptions struct { TimeoutCloseWaitSecs int LogFormat string LogLevel string + FirewallBinPath string + FirewallSaveBinPath string IPTablesMode string IPv6 bool - - // No longer supported - FirewallBinPath string - FirewallSaveBinPath string } func newRootOptions() *RootOptions { @@ -68,7 +65,9 @@ func newRootOptions() *RootOptions { TimeoutCloseWaitSecs: 0, LogFormat: "plain", LogLevel: "info", - IPTablesMode: IPTablesModeLegacy, + FirewallBinPath: "", + FirewallSaveBinPath: "", + IPTablesMode: "", IPv6: true, } } @@ -145,29 +144,30 @@ func NewRootCmd() *cobra.Command { cmd.PersistentFlags().IntVar(&options.TimeoutCloseWaitSecs, "timeout-close-wait-secs", options.TimeoutCloseWaitSecs, "Sets nf_conntrack_tcp_timeout_close_wait") cmd.PersistentFlags().StringVar(&options.LogFormat, "log-format", options.LogFormat, "Configure log format ('plain' or 'json')") cmd.PersistentFlags().StringVar(&options.LogLevel, "log-level", options.LogLevel, "Configure log level") - cmd.PersistentFlags().StringVar(&options.IPTablesMode, "iptables-mode", options.IPTablesMode, "Variant of iptables command to use (\"legacy\" or \"nft\")") + cmd.PersistentFlags().StringVar(&options.IPTablesMode, "iptables-mode", options.IPTablesMode, "Variant of iptables command to use (\"legacy\" or \"nft\"); overrides --firewall-bin-path and --firewall-save-bin-path") cmd.PersistentFlags().BoolVar(&options.IPv6, "ipv6", options.IPv6, "Set rules both via iptables and ip6tables to support dual-stack networking") + + // these two flags are kept for backwards-compatibility, but --iptables-mode is preferred cmd.PersistentFlags().StringVar(&options.FirewallBinPath, "firewall-bin-path", options.FirewallBinPath, "Path to iptables binary") cmd.PersistentFlags().StringVar(&options.FirewallSaveBinPath, "firewall-save-bin-path", options.FirewallSaveBinPath, "Path to iptables-save binary") - - if err := cmd.PersistentFlags().MarkHidden("firewall-bin-path"); err != nil { - log.Fatal(err) - } - if err := cmd.PersistentFlags().MarkHidden("firewall-save-bin-path"); err != nil { - log.Fatal(err) - } - return cmd } // BuildFirewallConfiguration returns an iptables FirewallConfiguration suitable to use to configure iptables. func BuildFirewallConfiguration(options *RootOptions) (*iptables.FirewallConfiguration, error) { - if options.FirewallBinPath != "" || options.FirewallSaveBinPath != "" { - return nil, errors.New("--firewal-bin-path and firewall-save-bin-path are no longer supported; please use --iptables-mode instead") + if options.IPTablesMode != "" && options.IPTablesMode != IPTablesModeLegacy && options.IPTablesMode != IPTablesModeNFT { + return nil, fmt.Errorf("--iptables-mode valid values are only \"%s\" and \"%s\"", IPTablesModeLegacy, IPTablesModeNFT) } - if options.IPTablesMode != IPTablesModeLegacy && options.IPTablesMode != IPTablesModeNFT { - return nil, errors.New("--iptables-mode valid values are only \"legacy\" and \"nft\"") + if options.IPTablesMode == "" { + switch options.FirewallBinPath { + case "", cmdLegacy: + options.IPTablesMode = IPTablesModeLegacy + case cmdNFT: + options.IPTablesMode = IPTablesModeNFT + default: + return nil, fmt.Errorf("--firewall-bin-path valid values are only \"%s\" and \"%s\"", cmdLegacy, cmdNFT) + } } if !util.IsValidPort(options.IncomingProxyPort) {