From a6260c7448c6313a73e79fb822891eea7aff949c Mon Sep 17 00:00:00 2001 From: Richard Hansen Date: Mon, 5 May 2025 03:05:36 -0400 Subject: [PATCH] flannel: Use custom type for network mode (IPv4, IPv6, dual-stack) Move the `ipv4` and `ipv6` constants to their own constant declaration. This ensures that the `iota` expression for the `ipv4` constant evaluates to 0, not some arbitrary value. (`iota` evaluates to N for the Nth constant in the constant declaration; see .) This is also more idiomatic, which improves readability. Also switch from incremental integers to bit flags, and use bitwise operators for checking. This is more idiomatic (the integer is treated like a set of booleans), it avoids some code duplication, and it is necessary to avoid ambiguity. Consider the following: const ( ipv4 = iota ipv6 ) In the above, `ipv4` would have the value 0 and `ipv6` would have the value 1. This would make it impossible to distinguish an IPv6-only stack from a dual-stack configuration because `ipv6` would equal `ipv4 + ipv6`. With bit flags this problem doesn't exist. And put the integer holding the bit flags in a custom type with convenience methods to improve readability. Signed-off-by: Richard Hansen (cherry picked from commit 925726c84dfef4be24872961f229429840a17923) Signed-off-by: Brad Davidson --- pkg/agent/flannel/flannel.go | 47 +++++++---------- pkg/agent/flannel/setup.go | 89 +++++++++++++++++---------------- pkg/agent/flannel/setup_test.go | 23 +++++---- 3 files changed, 78 insertions(+), 81 deletions(-) diff --git a/pkg/agent/flannel/flannel.go b/pkg/agent/flannel/flannel.go index 22dc66e162..e90456da43 100644 --- a/pkg/agent/flannel/flannel.go +++ b/pkg/agent/flannel/flannel.go @@ -51,8 +51,8 @@ var ( FlannelExternalIPv6Annotation = FlannelBaseAnnotation + "/public-ipv6-overwrite" ) -func flannel(ctx context.Context, flannelIface *net.Interface, flannelConf, kubeConfigFile string, flannelIPv6Masq bool, netMode int) error { - extIface, err := LookupExtInterface(flannelIface, netMode) +func flannel(ctx context.Context, flannelIface *net.Interface, flannelConf, kubeConfigFile string, flannelIPv6Masq bool, nm netMode) error { + extIface, err := LookupExtInterface(flannelIface, nm) if err != nil { return pkgerrors.WithMessage(err, "failed to find the interface") } @@ -90,10 +90,8 @@ func flannel(ctx context.Context, flannelIface *net.Interface, flannelConf, kube return pkgerrors.WithMessage(err, "failed to initialize flannel ipTables manager") } - if netMode == (ipv4+ipv6) || netMode == ipv4 { - if config.Network.Empty() { - return errors.New("ipv4 mode requested but no ipv4 network provided") - } + if nm.IPv4Enabled() && config.Network.Empty() { + return errors.New("ipv4 mode requested but no ipv4 network provided") } //setup masq rules @@ -115,7 +113,7 @@ func flannel(ctx context.Context, flannelIface *net.Interface, flannelConf, kube //setup forward rules trafficMngr.SetupAndEnsureForwardRules(ctx, config.Network, config.IPv6Network, 50) - if err := WriteSubnetFile(subnetFile, config.Network, config.IPv6Network, true, bn, netMode); err != nil { + if err := WriteSubnetFile(subnetFile, config.Network, config.IPv6Network, true, bn, nm); err != nil { // Continue, even though it failed. logrus.Warningf("Failed to write flannel subnet file: %s", err) } else { @@ -128,14 +126,14 @@ func flannel(ctx context.Context, flannelIface *net.Interface, flannelConf, kube return nil } -func LookupExtInterface(iface *net.Interface, netMode int) (*backend.ExternalInterface, error) { +func LookupExtInterface(iface *net.Interface, nm netMode) (*backend.ExternalInterface, error) { var ifaceAddr []net.IP var ifacev6Addr []net.IP var err error if iface == nil { logrus.Debug("No interface defined for flannel in the config. Fetching the default gateway interface") - if netMode == ipv4 || netMode == (ipv4+ipv6) { + if nm.IPv4Enabled() { if iface, err = ip.GetDefaultGatewayInterface(); err != nil { return nil, pkgerrors.WithMessage(err, "failed to get default interface") } @@ -147,33 +145,22 @@ func LookupExtInterface(iface *net.Interface, netMode int) (*backend.ExternalInt } logrus.Debugf("The interface %s will be used by flannel", iface.Name) - switch netMode { - case ipv4: + if nm.IPv4Enabled() { ifaceAddr, err = ip.GetInterfaceIP4Addrs(iface) if err != nil { - return nil, pkgerrors.WithMessage(err, "failed to find IPv4 address for interface") + return nil, pkgerrors.WithMessagef(err, "failed to find IPv4 address for interface %s", iface.Name) } logrus.Infof("The interface %s with ipv4 address %s will be used by flannel", iface.Name, ifaceAddr[0]) - ifacev6Addr = append(ifacev6Addr, nil) - case ipv6: + } else { + ifaceAddr = append(ifaceAddr, nil) + } + if nm.IPv6Enabled() { ifacev6Addr, err = ip.GetInterfaceIP6Addrs(iface) if err != nil { - return nil, pkgerrors.WithMessage(err, "failed to find IPv6 address for interface") + return nil, pkgerrors.WithMessagef(err, "failed to find IPv6 address for interface %s", iface.Name) } logrus.Infof("The interface %s with ipv6 address %s will be used by flannel", iface.Name, ifacev6Addr[0]) - ifaceAddr = append(ifaceAddr, nil) - case (ipv4 + ipv6): - ifaceAddr, err = ip.GetInterfaceIP4Addrs(iface) - if err != nil { - return nil, fmt.Errorf("failed to find IPv4 address for interface %s", iface.Name) - } - ifacev6Addr, err = ip.GetInterfaceIP6Addrs(iface) - if err != nil { - return nil, fmt.Errorf("failed to find IPv6 address for interface %s", iface.Name) - } - logrus.Infof("Using dual-stack mode. The interface %s with ipv4 address %s and ipv6 address %s will be used by flannel", iface.Name, ifaceAddr[0], ifacev6Addr[0]) - default: - ifaceAddr = append(ifaceAddr, nil) + } else { ifacev6Addr = append(ifacev6Addr, nil) } @@ -190,7 +177,7 @@ func LookupExtInterface(iface *net.Interface, netMode int) (*backend.ExternalInt }, nil } -func WriteSubnetFile(path string, nw ip.IP4Net, nwv6 ip.IP6Net, ipMasq bool, bn backend.Network, netMode int) error { +func WriteSubnetFile(path string, nw ip.IP4Net, nwv6 ip.IP6Net, ipMasq bool, bn backend.Network, nm netMode) error { dir, name := filepath.Split(path) os.MkdirAll(dir, 0755) @@ -204,7 +191,7 @@ func WriteSubnetFile(path string, nw ip.IP4Net, nwv6 ip.IP6Net, ipMasq bool, bn // sn.IP by one sn := bn.Lease().Subnet sn.IP++ - if netMode == ipv4 || netMode == (ipv4+ipv6) { + if nm.IPv4Enabled() { fmt.Fprintf(f, "FLANNEL_NETWORK=%s\n", nw) fmt.Fprintf(f, "FLANNEL_SUBNET=%s\n", sn) } diff --git a/pkg/agent/flannel/setup.go b/pkg/agent/flannel/setup.go index a13072afac..62a7232a9a 100644 --- a/pkg/agent/flannel/setup.go +++ b/pkg/agent/flannel/setup.go @@ -53,8 +53,15 @@ const ( }` emptyIPv6Network = "::/0" +) - ipv4 = iota +type netMode int + +func (nm netMode) IPv4Enabled() bool { return nm&ipv4 != 0 } +func (nm netMode) IPv6Enabled() bool { return nm&ipv6 != 0 } + +const ( + ipv4 netMode = 1 << iota ipv6 ) @@ -92,12 +99,12 @@ func Run(ctx context.Context, nodeConfig *config.Node) error { return pkgerrors.WithMessage(err, "flannel failed to wait for PodCIDR assignment") } - netMode, err := findNetMode(nodeConfig.AgentConfig.ClusterCIDRs) + nm, err := findNetMode(nodeConfig.AgentConfig.ClusterCIDRs) if err != nil { return pkgerrors.WithMessage(err, "failed to check netMode for flannel") } go func() { - err := flannel(ctx, nodeConfig.FlannelIface, nodeConfig.FlannelConfFile, kubeConfig, nodeConfig.FlannelIPv6Masq, netMode) + err := flannel(ctx, nodeConfig.FlannelIface, nodeConfig.FlannelConfFile, kubeConfig, nodeConfig.FlannelIPv6Masq, nm) if err != nil && !errors.Is(err, context.Canceled) { logrus.Errorf("flannel exited: %v", err) os.Exit(1) @@ -164,7 +171,6 @@ func createCNIConf(dir string, nodeConfig *config.Node) error { } func createFlannelConf(nodeConfig *config.Node) error { - var ipv4Enabled string logrus.Debugf("Creating the flannel configuration for backend %s in file %s", nodeConfig.FlannelBackend, nodeConfig.FlannelConfFile) if nodeConfig.FlannelConfFile == "" { return errors.New("Flannel configuration not defined") @@ -173,40 +179,40 @@ func createFlannelConf(nodeConfig *config.Node) error { logrus.Infof("Using custom flannel conf defined at %s", nodeConfig.FlannelConfFile) return nil } - netMode, err := findNetMode(nodeConfig.AgentConfig.ClusterCIDRs) + nm, err := findNetMode(nodeConfig.AgentConfig.ClusterCIDRs) if err != nil { logrus.Fatalf("Flannel error checking netMode: %v", err) return err } - if netMode == ipv4 || netMode == (ipv4+ipv6) { - ipv4Enabled = "true" + confJSON := flannelConf + if nm.IPv4Enabled() { + confJSON = strings.ReplaceAll(confJSON, "%IPV4_ENABLED%", "true") + if nm.IPv6Enabled() { + for _, cidr := range nodeConfig.AgentConfig.ClusterCIDRs { + if utilsnet.IsIPv4(cidr.IP) { + confJSON = strings.ReplaceAll(confJSON, "%CIDR%", cidr.String()) + break + } + } + } else { + confJSON = strings.ReplaceAll(confJSON, "%CIDR%", nodeConfig.AgentConfig.ClusterCIDR.String()) + } } else { - ipv4Enabled = "false" + confJSON = strings.ReplaceAll(confJSON, "%IPV4_ENABLED%", "false") + confJSON = strings.ReplaceAll(confJSON, "%CIDR%", "0.0.0.0/0") } - confJSON := strings.ReplaceAll(flannelConf, "%IPV4_ENABLED%", ipv4Enabled) - if netMode == ipv4 { - confJSON = strings.ReplaceAll(confJSON, "%CIDR%", nodeConfig.AgentConfig.ClusterCIDR.String()) + if nm.IPv6Enabled() { + confJSON = strings.ReplaceAll(confJSON, "%IPV6_ENABLED%", "true") + for _, cidr := range nodeConfig.AgentConfig.ClusterCIDRs { + if utilsnet.IsIPv6(cidr.IP) { + // Only one ipv6 range available. This might change in future: https://github.com/kubernetes/enhancements/issues/2593 + confJSON = strings.ReplaceAll(confJSON, "%CIDR_IPV6%", cidr.String()) + break + } + } + } else { confJSON = strings.ReplaceAll(confJSON, "%IPV6_ENABLED%", "false") confJSON = strings.ReplaceAll(confJSON, "%CIDR_IPV6%", emptyIPv6Network) - } else if netMode == (ipv4 + ipv6) { - confJSON = strings.ReplaceAll(confJSON, "%IPV6_ENABLED%", "true") - for _, cidr := range nodeConfig.AgentConfig.ClusterCIDRs { - if utilsnet.IsIPv6(cidr.IP) { - // Only one ipv6 range available. This might change in future: https://github.com/kubernetes/enhancements/issues/2593 - confJSON = strings.ReplaceAll(confJSON, "%CIDR_IPV6%", cidr.String()) - } else { - confJSON = strings.ReplaceAll(confJSON, "%CIDR%", cidr.String()) - } - } - } else { - confJSON = strings.ReplaceAll(confJSON, "%CIDR%", "0.0.0.0/0") - confJSON = strings.ReplaceAll(confJSON, "%IPV6_ENABLED%", "true") - for _, cidr := range nodeConfig.AgentConfig.ClusterCIDRs { - if utilsnet.IsIPv6(cidr.IP) { - // Only one ipv6 range available. This might change in future: https://github.com/kubernetes/enhancements/issues/2593 - confJSON = strings.ReplaceAll(confJSON, "%CIDR_IPV6%", cidr.String()) - } - } } var backendConf string @@ -227,18 +233,17 @@ func createFlannelConf(nodeConfig *config.Node) error { case config.FlannelBackendHostGW: backendConf = hostGWBackend case config.FlannelBackendTailscale: - var routes string - switch netMode { - case ipv4: - routes = "$SUBNET" - case (ipv4 + ipv6): - routes = "$SUBNET,$IPV6SUBNET" - case ipv6: - routes = "$IPV6SUBNET" - default: + var routes []string + if nm.IPv4Enabled() { + routes = append(routes, "$SUBNET") + } + if nm.IPv6Enabled() { + routes = append(routes, "$IPV6SUBNET") + } + if len(routes) == 0 { return fmt.Errorf("incorrect netMode for flannel tailscale backend") } - backendConf = strings.ReplaceAll(tailscaledBackend, "%Routes%", routes) + backendConf = strings.ReplaceAll(tailscaledBackend, "%Routes%", strings.Join(routes, ",")) case config.FlannelBackendWireguardNative: backendConf = wireguardNativeBackend default: @@ -251,13 +256,13 @@ func createFlannelConf(nodeConfig *config.Node) error { } // fundNetMode returns the mode (ipv4, ipv6 or dual-stack) in which flannel is operating -func findNetMode(cidrs []*net.IPNet) (int, error) { +func findNetMode(cidrs []*net.IPNet) (netMode, error) { dualStack, err := utilsnet.IsDualStackCIDRs(cidrs) if err != nil { return 0, err } if dualStack { - return ipv4 + ipv6, nil + return ipv4 | ipv6, nil } for _, cidr := range cidrs { diff --git a/pkg/agent/flannel/setup_test.go b/pkg/agent/flannel/setup_test.go index 46473649a9..2c712f09f5 100644 --- a/pkg/agent/flannel/setup_test.go +++ b/pkg/agent/flannel/setup_test.go @@ -23,13 +23,16 @@ func Test_findNetMode(t *testing.T) { tests := []struct { name string args string - want int + wantv4 bool + wantv6 bool wantErr bool }{ - {"dual-stack", "10.42.0.0/16,2001:cafe:22::/56", ipv4 + ipv6, false}, - {"ipv4 only", "10.42.0.0/16", ipv4, false}, - {"ipv6 only", "2001:cafe:42:0::/56", ipv6, false}, - {"wrong input", "wrong", 0, true}, + {"dual-stack", "10.42.0.0/16,2001:cafe:22::/56", true, true, false}, + {"dual-stack ipv6 first", "2001:cafe:22::/56,10.42.0.0/16", true, true, false}, + {"ipv4 only", "10.42.0.0/16", true, false, false}, + {"ipv6 only", "2001:cafe:42:0::/56", false, true, false}, + {"empty", "", false, false, true}, + {"wrong input", "wrong", false, false, true}, } for _, tt := range tests { @@ -37,11 +40,13 @@ func Test_findNetMode(t *testing.T) { netCidrs := stringToCIDR(tt.args) got, err := findNetMode(netCidrs) if (err != nil) != tt.wantErr { - t.Errorf("findNetMode() error = %v, wantErr %v", err, tt.wantErr) - return + t.Fatalf("got error %v, want %v", err, tt.wantErr) } - if got != tt.want { - t.Errorf("findNetMode() = %v, want %v", got, tt.want) + if gotv4 := got.IPv4Enabled(); gotv4 != tt.wantv4 { + t.Errorf("got ipv4 %v, want %v", gotv4, tt.wantv4) + } + if gotv6 := got.IPv6Enabled(); gotv6 != tt.wantv6 { + t.Errorf("got ipv6 %v, want %v", gotv6, tt.wantv6) } }) }