Avoid passing extra ipv6 flag around

linkerd · Mar 14, 2024 · edc6086 · edc6086
1 parent 94489ef
commit edc6086
Show file tree

Hide file tree

Showing 3 changed files with 29 additions and 19 deletions.
diff --git 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
@@ -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
@@ -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)
 			}