diff --git a/cmd/kube-proxy/app/server.go b/cmd/kube-proxy/app/server.go index e2c84ca7af4..73489b3f18c 100644 --- a/cmd/kube-proxy/app/server.go +++ b/cmd/kube-proxy/app/server.go @@ -661,6 +661,11 @@ func newProxyServer(logger klog.Logger, config *kubeproxyconfig.KubeProxyConfigu return nil, err } + err = checkBadConfig(s) + if err != nil { + logger.Error(err, "Kube-proxy configuration may be incomplete or incorrect") + } + ipv4Supported, ipv6Supported, dualStackSupported, err := s.platformCheckSupported() if err != nil { return nil, err @@ -672,7 +677,7 @@ func newProxyServer(logger klog.Logger, config *kubeproxyconfig.KubeProxyConfigu logger.Info("kube-proxy running in single-stack mode", "ipFamily", s.PrimaryIPFamily) } - err, fatal := checkIPConfig(s, dualStackSupported) + err, fatal := checkBadIPConfig(s, dualStackSupported) if err != nil { if fatal { return nil, fmt.Errorf("kube-proxy configuration is incorrect: %v", err) @@ -688,8 +693,42 @@ func newProxyServer(logger klog.Logger, config *kubeproxyconfig.KubeProxyConfigu return s, nil } -// checkIPConfig confirms that s has proper configuration for its primary IP family. -func checkIPConfig(s *ProxyServer, dualStackSupported bool) (error, bool) { +// checkBadConfig checks for bad/deprecated configuation +func checkBadConfig(s *ProxyServer) error { + var errors []error + + // At this point we haven't seen any actual Services or EndpointSlices, so we + // don't really know if the cluster is expected to be single- or dual-stack. But + // we can at least take note of whether there is any explicitly-dual-stack + // configuration. + anyDualStackConfig := false + clusterCIDRs := strings.Split(s.Config.ClusterCIDR, ",") + for _, config := range [][]string{clusterCIDRs, s.Config.NodePortAddresses, s.Config.IPVS.ExcludeCIDRs, s.podCIDRs} { + if dual, _ := netutils.IsDualStackCIDRStrings(config); dual { + anyDualStackConfig = true + break + } + } + + // Warn if NodePortAddresses does not limit connections on all IP families that + // seem to be in use. + cidrsByFamily := proxyutil.MapCIDRsByIPFamily(s.Config.NodePortAddresses) + if len(s.Config.NodePortAddresses) == 0 { + errors = append(errors, fmt.Errorf("nodePortAddresses is unset; NodePort connections will be accepted on all local IPs. Consider using `--nodeport-addresses primary`")) + } else if anyDualStackConfig && len(cidrsByFamily[s.PrimaryIPFamily]) == len(s.Config.NodePortAddresses) { + errors = append(errors, fmt.Errorf("cluster appears to be dual-stack but nodePortAddresses contains only %s addresses; NodePort connections will be accepted on all local %s IPs", s.PrimaryIPFamily, proxyutil.OtherIPFamily(s.PrimaryIPFamily))) + } else if len(cidrsByFamily[s.PrimaryIPFamily]) == 0 { + errors = append(errors, fmt.Errorf("cluster appears to be %s-primary but nodePortAddresses contains only %s addresses; NodePort connections will be accepted on all local %s IPs", s.PrimaryIPFamily, proxyutil.OtherIPFamily(s.PrimaryIPFamily), s.PrimaryIPFamily)) + } + + return utilerrors.NewAggregate(errors) +} + +// checkBadIPConfig checks for bad configuration relative to s.PrimaryIPFamily. +// Historically, we did not check most of the config options, so we cannot retroactively +// make IP family mismatches in those options be fatal. When we add new options to check +// here, we should make problems with those options be fatal. +func checkBadIPConfig(s *ProxyServer, dualStackSupported bool) (err error, fatal bool) { var errors []error var badFamily netutils.IPFamily @@ -706,11 +745,6 @@ func checkIPConfig(s *ProxyServer, dualStackSupported bool) (error, bool) { clusterType = fmt.Sprintf("%s-only", s.PrimaryIPFamily) } - // Historically, we did not check most of the config options, so we cannot - // retroactively make IP family mismatches in those options be fatal. When we add - // new options to check here, we should make problems with those options be fatal. - fatal := false - if s.Config.ClusterCIDR != "" { clusterCIDRs := strings.Split(s.Config.ClusterCIDR, ",") if badCIDRs(clusterCIDRs, badFamily) { @@ -722,10 +756,6 @@ func checkIPConfig(s *ProxyServer, dualStackSupported bool) (error, bool) { } } - if badCIDRs(s.Config.NodePortAddresses, badFamily) { - errors = append(errors, fmt.Errorf("cluster is %s but nodePortAddresses contains only IPv%s addresses", clusterType, badFamily)) - } - if badCIDRs(s.podCIDRs, badFamily) { errors = append(errors, fmt.Errorf("cluster is %s but node.spec.podCIDRs contains only IPv%s addresses", clusterType, badFamily)) if s.Config.DetectLocalMode == kubeproxyconfig.LocalModeNodeCIDR { @@ -753,6 +783,9 @@ func checkIPConfig(s *ProxyServer, dualStackSupported bool) (error, bool) { } } + // Note that s.Config.NodePortAddresses gets checked as part of checkBadConfig() + // so it doesn't need to be checked here. + return utilerrors.NewAggregate(errors), fatal } diff --git a/cmd/kube-proxy/app/server_test.go b/cmd/kube-proxy/app/server_test.go index 37559f10c1c..b8963dca8c5 100644 --- a/cmd/kube-proxy/app/server_test.go +++ b/cmd/kube-proxy/app/server_test.go @@ -849,7 +849,81 @@ func Test_detectNodeIPs(t *testing.T) { } } -func Test_checkIPConfig(t *testing.T) { +func Test_checkBadConfig(t *testing.T) { + cases := []struct { + name string + proxy *ProxyServer + err bool + }{ + { + name: "single-stack NodePortAddresses with single-stack config", + proxy: &ProxyServer{ + Config: &kubeproxyconfig.KubeProxyConfiguration{ + ClusterCIDR: "10.0.0.0/8", + NodePortAddresses: []string{"192.168.0.0/24"}, + }, + PrimaryIPFamily: v1.IPv4Protocol, + }, + err: false, + }, + { + name: "dual-stack NodePortAddresses with dual-stack config", + proxy: &ProxyServer{ + Config: &kubeproxyconfig.KubeProxyConfiguration{ + ClusterCIDR: "10.0.0.0/8,fd09::/64", + NodePortAddresses: []string{"192.168.0.0/24", "fd03::/64"}, + }, + PrimaryIPFamily: v1.IPv4Protocol, + }, + err: false, + }, + { + name: "empty NodePortAddresses", + proxy: &ProxyServer{ + Config: &kubeproxyconfig.KubeProxyConfiguration{ + NodePortAddresses: []string{}, + }, + PrimaryIPFamily: v1.IPv4Protocol, + }, + err: true, + }, + { + name: "single-stack NodePortAddresses with dual-stack config", + proxy: &ProxyServer{ + Config: &kubeproxyconfig.KubeProxyConfiguration{ + ClusterCIDR: "10.0.0.0/8,fd09::/64", + NodePortAddresses: []string{"192.168.0.0/24"}, + }, + PrimaryIPFamily: v1.IPv4Protocol, + }, + err: true, + }, + { + name: "wrong-single-stack NodePortAddresses", + proxy: &ProxyServer{ + Config: &kubeproxyconfig.KubeProxyConfiguration{ + ClusterCIDR: "fd09::/64", + NodePortAddresses: []string{"192.168.0.0/24"}, + }, + PrimaryIPFamily: v1.IPv6Protocol, + }, + err: true, + }, + } + + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + err := checkBadConfig(c.proxy) + if err != nil && !c.err { + t.Errorf("unexpected error: %v", err) + } else if err == nil && c.err { + t.Errorf("unexpected lack of error") + } + }) + } +} + +func Test_checkBadIPConfig(t *testing.T) { cases := []struct { name string proxy *ProxyServer @@ -929,53 +1003,6 @@ func Test_checkIPConfig(t *testing.T) { dsFatal: false, }, - { - name: "ok single-stack nodePortAddresses", - proxy: &ProxyServer{ - Config: &kubeproxyconfig.KubeProxyConfiguration{ - NodePortAddresses: []string{"10.0.0.0/8", "192.168.0.0/24"}, - }, - PrimaryIPFamily: v1.IPv4Protocol, - }, - ssErr: false, - dsErr: false, - }, - { - name: "ok dual-stack nodePortAddresses", - proxy: &ProxyServer{ - Config: &kubeproxyconfig.KubeProxyConfiguration{ - NodePortAddresses: []string{"10.0.0.0/8", "fd01:2345::/64", "fd01:abcd::/64"}, - }, - PrimaryIPFamily: v1.IPv4Protocol, - }, - ssErr: false, - dsErr: false, - }, - { - name: "ok reversed dual-stack nodePortAddresses", - proxy: &ProxyServer{ - Config: &kubeproxyconfig.KubeProxyConfiguration{ - NodePortAddresses: []string{"fd01:2345::/64", "fd01:abcd::/64", "10.0.0.0/8"}, - }, - PrimaryIPFamily: v1.IPv4Protocol, - }, - ssErr: false, - dsErr: false, - }, - { - name: "wrong-family nodePortAddresses", - proxy: &ProxyServer{ - Config: &kubeproxyconfig.KubeProxyConfiguration{ - NodePortAddresses: []string{"10.0.0.0/8"}, - }, - PrimaryIPFamily: v1.IPv6Protocol, - }, - ssErr: true, - ssFatal: false, - dsErr: true, - dsFatal: false, - }, - { name: "ok single-stack node.spec.podCIDRs", proxy: &ProxyServer{ @@ -1133,7 +1160,7 @@ func Test_checkIPConfig(t *testing.T) { for _, c := range cases { t.Run(c.name, func(t *testing.T) { - err, fatal := checkIPConfig(c.proxy, false) + err, fatal := checkBadIPConfig(c.proxy, false) if err != nil && !c.ssErr { t.Errorf("unexpected error in single-stack case: %v", err) } else if err == nil && c.ssErr { @@ -1142,7 +1169,7 @@ func Test_checkIPConfig(t *testing.T) { t.Errorf("expected fatal=%v, got %v", c.ssFatal, fatal) } - err, fatal = checkIPConfig(c.proxy, true) + err, fatal = checkBadIPConfig(c.proxy, true) if err != nil && !c.dsErr { t.Errorf("unexpected error in dual-stack case: %v", err) } else if err == nil && c.dsErr {