diff --git a/cmd/kubelet/app/server.go b/cmd/kubelet/app/server.go index 68af6aaba7b..b755d4e5bfc 100644 --- a/cmd/kubelet/app/server.go +++ b/cmd/kubelet/app/server.go @@ -1120,24 +1120,9 @@ func RunKubelet(kubeServer *options.KubeletServer, kubeDeps *kubelet.Dependencie // Setup event recorder if required. makeEventRecorder(kubeDeps, nodeName) - var nodeIPs []net.IP - if kubeServer.NodeIP != "" { - for _, ip := range strings.Split(kubeServer.NodeIP, ",") { - parsedNodeIP := netutils.ParseIPSloppy(strings.TrimSpace(ip)) - if parsedNodeIP == nil { - klog.InfoS("Could not parse --node-ip ignoring", "IP", ip) - } else { - nodeIPs = append(nodeIPs, parsedNodeIP) - } - } - } - - if len(nodeIPs) > 2 || (len(nodeIPs) == 2 && netutils.IsIPv6(nodeIPs[0]) == netutils.IsIPv6(nodeIPs[1])) { - return fmt.Errorf("bad --node-ip %q; must contain either a single IP or a dual-stack pair of IPs", kubeServer.NodeIP) - } else if len(nodeIPs) == 2 && kubeServer.CloudProvider != "" { - return fmt.Errorf("dual-stack --node-ip %q not supported when using a cloud provider", kubeServer.NodeIP) - } else if len(nodeIPs) == 2 && (nodeIPs[0].IsUnspecified() || nodeIPs[1].IsUnspecified()) { - return fmt.Errorf("dual-stack --node-ip %q cannot include '0.0.0.0' or '::'", kubeServer.NodeIP) + nodeIPs, err := nodeutil.ParseNodeIPArgument(kubeServer.NodeIP, kubeServer.CloudProvider) + if err != nil { + return fmt.Errorf("bad --node-ip %q: %v", kubeServer.NodeIP, err) } capabilities.Initialize(capabilities.Capabilities{ diff --git a/staging/src/k8s.io/cloud-provider/controllers/node/node_controller.go b/staging/src/k8s.io/cloud-provider/controllers/node/node_controller.go index 878ca1194a6..e77b45e89d6 100644 --- a/staging/src/k8s.io/cloud-provider/controllers/node/node_controller.go +++ b/staging/src/k8s.io/cloud-provider/controllers/node/node_controller.go @@ -45,7 +45,6 @@ import ( controllersmetrics "k8s.io/component-base/metrics/prometheus/controllers" nodeutil "k8s.io/component-helpers/node/util" "k8s.io/klog/v2" - netutils "k8s.io/utils/net" ) // labelReconcileInfo lists Node labels to reconcile, and how to reconcile them. @@ -756,9 +755,9 @@ func getNodeProvidedIP(node *v1.Node) (net.IP, error) { return nil, nil } - nodeIP := netutils.ParseIPSloppy(providedIP) - if nodeIP == nil { - return nil, fmt.Errorf("failed to parse node IP %q for node %q", providedIP, node.Name) + nodeIP, err := nodeutil.ParseNodeIPAnnotation(providedIP) + if err != nil { + return nil, fmt.Errorf("failed to parse node IP %q for node %q: %v", providedIP, node.Name, err) } return nodeIP, nil diff --git a/staging/src/k8s.io/component-helpers/node/util/ips.go b/staging/src/k8s.io/component-helpers/node/util/ips.go new file mode 100644 index 00000000000..d0edd8c0532 --- /dev/null +++ b/staging/src/k8s.io/component-helpers/node/util/ips.go @@ -0,0 +1,81 @@ +/* +Copyright 2023 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package util + +import ( + "fmt" + "net" + "strings" + + "k8s.io/klog/v2" + netutils "k8s.io/utils/net" +) + +const ( + cloudProviderNone = "" + cloudProviderExternal = "external" +) + +// parseNodeIP implements ParseNodeIPArgument and ParseNodeIPAnnotation +func parseNodeIP(nodeIP string, allowDual, sloppy bool) ([]net.IP, error) { + var nodeIPs []net.IP + if nodeIP != "" || !sloppy { + for _, ip := range strings.Split(nodeIP, ",") { + if sloppy { + ip = strings.TrimSpace(ip) + } + parsedNodeIP := netutils.ParseIPSloppy(ip) + if parsedNodeIP == nil { + if sloppy { + klog.InfoS("Could not parse node IP. Ignoring", "IP", ip) + } else { + return nil, fmt.Errorf("could not parse %q", ip) + } + } else { + nodeIPs = append(nodeIPs, parsedNodeIP) + } + } + } + + if len(nodeIPs) > 2 || (len(nodeIPs) == 2 && netutils.IsIPv6(nodeIPs[0]) == netutils.IsIPv6(nodeIPs[1])) { + return nil, fmt.Errorf("must contain either a single IP or a dual-stack pair of IPs") + } else if len(nodeIPs) == 2 && !allowDual { + return nil, fmt.Errorf("dual-stack not supported in this configuration") + } else if len(nodeIPs) == 2 && (nodeIPs[0].IsUnspecified() || nodeIPs[1].IsUnspecified()) { + return nil, fmt.Errorf("dual-stack node IP cannot include '0.0.0.0' or '::'") + } + + return nodeIPs, nil +} + +// ParseNodeIPArgument parses kubelet's --node-ip argument. If nodeIP contains invalid +// values, they will be logged and ignored. Dual-stack node IPs are only supported if +// cloudProvider is unset. +func ParseNodeIPArgument(nodeIP, cloudProvider string) ([]net.IP, error) { + return parseNodeIP(nodeIP, cloudProvider == cloudProviderNone, true) +} + +// ParseNodeIPAnnotation parses the `alpha.kubernetes.io/provided-node-ip` annotation, +// which should be a single IP address. Unlike with ParseNodeIPArgument, invalid values +// are considered an error. +func ParseNodeIPAnnotation(nodeIP string) (net.IP, error) { + ips, err := parseNodeIP(nodeIP, false, false) + if err != nil || len(ips) == 0 { + return nil, err + } + return ips[0], nil +} diff --git a/staging/src/k8s.io/component-helpers/node/util/ips_test.go b/staging/src/k8s.io/component-helpers/node/util/ips_test.go new file mode 100644 index 00000000000..250aae91c59 --- /dev/null +++ b/staging/src/k8s.io/component-helpers/node/util/ips_test.go @@ -0,0 +1,330 @@ +/* +Copyright 2023 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package util + +import ( + "fmt" + "net" + "reflect" + "strings" + "testing" + + netutils "k8s.io/utils/net" +) + +func TestParseNodeIPArgument(t *testing.T) { + testCases := []struct { + desc string + in string + out []net.IP + err string + ssErr string + }{ + { + desc: "empty --node-ip", + in: "", + out: nil, + }, + { + desc: "just whitespace (ignored)", + in: " ", + out: nil, + }, + { + desc: "garbage (ignored)", + in: "blah", + out: nil, + }, + { + desc: "single IPv4", + in: "1.2.3.4", + out: []net.IP{ + netutils.ParseIPSloppy("1.2.3.4"), + }, + }, + { + desc: "single IPv4 with whitespace", + in: " 1.2.3.4 ", + out: []net.IP{ + netutils.ParseIPSloppy("1.2.3.4"), + }, + }, + { + desc: "single IPv4 non-canonical", + in: "01.2.3.004", + out: []net.IP{ + netutils.ParseIPSloppy("1.2.3.4"), + }, + }, + { + desc: "single IPv4 invalid (ignored)", + in: "1.2.3", + out: nil, + }, + { + desc: "single IPv4 CIDR (ignored)", + in: "1.2.3.0/24", + out: nil, + }, + { + desc: "single IPv4 unspecified", + in: "0.0.0.0", + out: []net.IP{ + net.IPv4zero, + }, + }, + { + desc: "single IPv4 plus ignored garbage", + in: "1.2.3.4,not-an-IPv6-address", + out: []net.IP{ + netutils.ParseIPSloppy("1.2.3.4"), + }, + }, + { + desc: "single IPv6", + in: "abcd::ef01", + out: []net.IP{ + netutils.ParseIPSloppy("abcd::ef01"), + }, + }, + { + desc: "single IPv6 non-canonical", + in: "abcd:0abc:00ab:0000:0000::1", + out: []net.IP{ + netutils.ParseIPSloppy("abcd:abc:ab::1"), + }, + }, + { + desc: "simple dual-stack", + in: "1.2.3.4,abcd::ef01", + out: []net.IP{ + netutils.ParseIPSloppy("1.2.3.4"), + netutils.ParseIPSloppy("abcd::ef01"), + }, + ssErr: "not supported in this configuration", + }, + { + desc: "dual-stack with whitespace", + in: "abcd::ef01 , 1.2.3.4", + out: []net.IP{ + netutils.ParseIPSloppy("abcd::ef01"), + netutils.ParseIPSloppy("1.2.3.4"), + }, + ssErr: "not supported in this configuration", + }, + { + desc: "double IPv4", + in: "1.2.3.4,5.6.7.8", + err: "either a single IP or a dual-stack pair of IPs", + }, + { + desc: "double IPv6", + in: "abcd::1,abcd::2", + err: "either a single IP or a dual-stack pair of IPs", + }, + { + desc: "dual-stack with unspecified", + in: "1.2.3.4,::", + err: "cannot include '0.0.0.0' or '::'", + ssErr: "not supported in this configuration", + }, + { + desc: "dual-stack with unspecified", + in: "0.0.0.0,abcd::1", + err: "cannot include '0.0.0.0' or '::'", + ssErr: "not supported in this configuration", + }, + { + desc: "dual-stack plus ignored garbage", + in: "abcd::ef01 , 1.2.3.4, something else", + out: []net.IP{ + netutils.ParseIPSloppy("abcd::ef01"), + netutils.ParseIPSloppy("1.2.3.4"), + }, + ssErr: "not supported in this configuration", + }, + { + desc: "triple stack!", + in: "1.2.3.4,abcd::1,5.6.7.8", + err: "either a single IP or a dual-stack pair of IPs", + }, + } + + for _, tc := range testCases { + for _, cloudProvider := range []string{cloudProviderNone, cloudProviderExternal, "gce"} { + desc := fmt.Sprintf("%s, cloudProvider=%q", tc.desc, cloudProvider) + t.Run(desc, func(t *testing.T) { + parsed, err := ParseNodeIPArgument(tc.in, cloudProvider) + + expectedOut := tc.out + expectedErr := tc.err + + // Dual-stack is only supported with no cloudProvider + if cloudProvider != "" { + if len(tc.out) == 2 { + expectedOut = nil + } + if tc.ssErr != "" { + expectedErr = tc.ssErr + } + } + + if !reflect.DeepEqual(parsed, expectedOut) { + t.Errorf("expected %#v, got %#v", expectedOut, parsed) + } + if err != nil { + if expectedErr == "" { + t.Errorf("unexpected error %v", err) + } else if !strings.Contains(err.Error(), expectedErr) { + t.Errorf("expected error with %q, got %v", expectedErr, err) + } + } else if expectedErr != "" { + t.Errorf("expected error with %q, got no error", expectedErr) + } + }) + } + } +} + +func TestParseNodeIPAnnotation(t *testing.T) { + testCases := []struct { + desc string + in string + out net.IP + err string + }{ + { + desc: "empty --node-ip", + in: "", + err: "could not parse", + }, + { + desc: "just whitespace", + in: " ", + err: "could not parse", + }, + { + desc: "garbage", + in: "blah", + err: "could not parse", + }, + { + desc: "single IPv4", + in: "1.2.3.4", + out: netutils.ParseIPSloppy("1.2.3.4"), + }, + { + desc: "single IPv4 with whitespace", + in: " 1.2.3.4 ", + err: "could not parse", + }, + { + desc: "single IPv4 non-canonical", + in: "01.2.3.004", + out: netutils.ParseIPSloppy("1.2.3.4"), + }, + { + desc: "single IPv4 invalid", + in: "1.2.3", + err: "could not parse", + }, + { + desc: "single IPv4 CIDR", + in: "1.2.3.0/24", + err: "could not parse", + }, + { + desc: "single IPv4 unspecified", + in: "0.0.0.0", + out: net.IPv4zero, + }, + { + desc: "single IPv4 plus garbage", + in: "1.2.3.4,not-an-IPv6-address", + err: "could not parse", + }, + { + desc: "single IPv6", + in: "abcd::ef01", + out: netutils.ParseIPSloppy("abcd::ef01"), + }, + { + desc: "single IPv6 non-canonical", + in: "abcd:0abc:00ab:0000:0000::1", + out: netutils.ParseIPSloppy("abcd:abc:ab::1"), + }, + { + desc: "simple dual-stack", + in: "1.2.3.4,abcd::ef01", + err: "not supported in this configuration", + }, + { + desc: "dual-stack with whitespace", + in: "abcd::ef01 , 1.2.3.4", + err: "could not parse", + }, + { + desc: "double IPv4", + in: "1.2.3.4,5.6.7.8", + err: "either a single IP or a dual-stack pair of IPs", + }, + { + desc: "double IPv6", + in: "abcd::1,abcd::2", + err: "either a single IP or a dual-stack pair of IPs", + }, + { + desc: "dual-stack with unspecified", + in: "1.2.3.4,::", + err: "not supported in this configuration", + }, + { + desc: "dual-stack with unspecified", + in: "0.0.0.0,abcd::1", + err: "not supported in this configuration", + }, + { + desc: "dual-stack plus garbage", + in: "abcd::ef01 , 1.2.3.4, something else", + err: "could not parse", + }, + { + desc: "triple stack!", + in: "1.2.3.4,abcd::1,5.6.7.8", + err: "either a single IP or a dual-stack pair of IPs", + }, + } + + for _, tc := range testCases { + t.Run(tc.desc, func(t *testing.T) { + parsed, err := ParseNodeIPAnnotation(tc.in) + + if !reflect.DeepEqual(parsed, tc.out) { + t.Errorf("expected %#v, got %#v", tc.out, parsed) + } + if err != nil { + if tc.err == "" { + t.Errorf("unexpected error %v", err) + } else if !strings.Contains(err.Error(), tc.err) { + t.Errorf("expected error with %q, got %v", tc.err, err) + } + } else if tc.err != "" { + t.Errorf("expected error with %q, got no error", tc.err) + } + }) + } +}