From f335812719cfa5d5fb235320fb63af3669ffdcb1 Mon Sep 17 00:00:00 2001 From: Claudiu Belu Date: Tue, 14 Jun 2022 17:27:50 +0300 Subject: [PATCH] unittests: Fixes unit tests for Windows (part 5) Currently, there are some unit tests that are failing on Windows due to various reasons: - getHostDNSConfig is reading a resolv.conf file. However, we don't have that on Windows. Instead, we can get the DNS server list and the DNS suffix list from Windows itself. On Windows, getHostDNSConfig will now return the host's DNS configuration if the given resolverConfig is "Host". If it's not "Host" or an empty string, an error will be returned. Based on the code from kubernetes/test/images/agnhost/dns/dns_windows.go --- pkg/kubelet/network/dns/dns.go | 44 ++--- pkg/kubelet/network/dns/dns_other.go | 53 ++++++ pkg/kubelet/network/dns/dns_other_test.go | 57 ++++++ pkg/kubelet/network/dns/dns_test.go | 36 +--- pkg/kubelet/network/dns/dns_windows.go | 199 ++++++++++++++++++++ pkg/kubelet/network/dns/dns_windows_test.go | 42 +++++ 6 files changed, 372 insertions(+), 59 deletions(-) create mode 100644 pkg/kubelet/network/dns/dns_other.go create mode 100644 pkg/kubelet/network/dns/dns_other_test.go create mode 100644 pkg/kubelet/network/dns/dns_windows.go create mode 100644 pkg/kubelet/network/dns/dns_windows_test.go diff --git a/pkg/kubelet/network/dns/dns.go b/pkg/kubelet/network/dns/dns.go index 81f1927cc86..d08c7a6cf2b 100644 --- a/pkg/kubelet/network/dns/dns.go +++ b/pkg/kubelet/network/dns/dns.go @@ -59,9 +59,10 @@ const ( // Configurer is used for setting up DNS resolver configuration when launching pods. type Configurer struct { - recorder record.EventRecorder - nodeRef *v1.ObjectReference - nodeIPs []net.IP + recorder record.EventRecorder + getHostDNSConfig func(string) (*runtimeapi.DNSConfig, error) + nodeRef *v1.ObjectReference + nodeIPs []net.IP // If non-nil, use this for container DNS server. clusterDNS []net.IP @@ -76,12 +77,13 @@ type Configurer struct { // NewConfigurer returns a DNS configurer for launching pods. func NewConfigurer(recorder record.EventRecorder, nodeRef *v1.ObjectReference, nodeIPs []net.IP, clusterDNS []net.IP, clusterDomain, resolverConfig string) *Configurer { return &Configurer{ - recorder: recorder, - nodeRef: nodeRef, - nodeIPs: nodeIPs, - clusterDNS: clusterDNS, - ClusterDomain: clusterDomain, - ResolverConfig: resolverConfig, + recorder: recorder, + getHostDNSConfig: getHostDNSConfig, + nodeRef: nodeRef, + nodeIPs: nodeIPs, + clusterDNS: clusterDNS, + ClusterDomain: clusterDomain, + ResolverConfig: resolverConfig, } } @@ -279,28 +281,6 @@ func parseResolvConf(reader io.Reader) (nameservers []string, searches []string, return nameservers, searches, options, utilerrors.NewAggregate(allErrors) } -func (c *Configurer) getHostDNSConfig() (*runtimeapi.DNSConfig, error) { - var hostDNS, hostSearch, hostOptions []string - // Get host DNS settings - if c.ResolverConfig != "" { - f, err := os.Open(c.ResolverConfig) - if err != nil { - return nil, err - } - defer f.Close() - - hostDNS, hostSearch, hostOptions, err = parseResolvConf(f) - if err != nil { - return nil, err - } - } - return &runtimeapi.DNSConfig{ - Servers: hostDNS, - Searches: hostSearch, - Options: hostOptions, - }, nil -} - func getPodDNSType(pod *v1.Pod) (podDNSType, error) { dnsPolicy := pod.Spec.DNSPolicy switch dnsPolicy { @@ -384,7 +364,7 @@ func appendDNSConfig(existingDNSConfig *runtimeapi.DNSConfig, dnsConfig *v1.PodD // GetPodDNS returns DNS settings for the pod. func (c *Configurer) GetPodDNS(pod *v1.Pod) (*runtimeapi.DNSConfig, error) { - dnsConfig, err := c.getHostDNSConfig() + dnsConfig, err := c.getHostDNSConfig(c.ResolverConfig) if err != nil { return nil, err } diff --git a/pkg/kubelet/network/dns/dns_other.go b/pkg/kubelet/network/dns/dns_other.go new file mode 100644 index 00000000000..40c18c289e2 --- /dev/null +++ b/pkg/kubelet/network/dns/dns_other.go @@ -0,0 +1,53 @@ +//go:build !windows +// +build !windows + +/* +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 dns + +import ( + "fmt" + "os" + + runtimeapi "k8s.io/cri-api/pkg/apis/runtime/v1" + "k8s.io/klog/v2" +) + +func getHostDNSConfig(resolverConfig string) (*runtimeapi.DNSConfig, error) { + var hostDNS, hostSearch, hostOptions []string + // Get host DNS settings + if resolverConfig != "" { + f, err := os.Open(resolverConfig) + if err != nil { + klog.ErrorS(err, "Could not open resolv conf file.") + return nil, err + } + defer f.Close() + + hostDNS, hostSearch, hostOptions, err = parseResolvConf(f) + if err != nil { + err := fmt.Errorf("Encountered error while parsing resolv conf file. Error: %w", err) + klog.ErrorS(err, "Could not parse resolv conf file.") + return nil, err + } + } + return &runtimeapi.DNSConfig{ + Servers: hostDNS, + Searches: hostSearch, + Options: hostOptions, + }, nil +} diff --git a/pkg/kubelet/network/dns/dns_other_test.go b/pkg/kubelet/network/dns/dns_other_test.go new file mode 100644 index 00000000000..daa615ee133 --- /dev/null +++ b/pkg/kubelet/network/dns/dns_other_test.go @@ -0,0 +1,57 @@ +//go:build !windows +// +build !windows + +/* +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 dns + +import ( + "fmt" + "os" + "testing" +) + +var ( + defaultResolvConf = "/etc/resolv.conf" + // configurer.getHostDNSConfig is faked on Windows, while it is not faked on Linux. + fakeGetHostDNSConfigCustom = getHostDNSConfig +) + +// getResolvConf returns a temporary resolv.conf file containing the testHostNameserver nameserver and +// testHostDomain search field, and a cleanup function for the temporary file. +func getResolvConf(t *testing.T) (string, func()) { + resolvConfContent := []byte(fmt.Sprintf("nameserver %s\nsearch %s\n", testHostNameserver, testHostDomain)) + tmpfile, err := os.CreateTemp("", "tmpResolvConf") + if err != nil { + t.Fatal(err) + } + + cleanup := func() { + os.Remove(tmpfile.Name()) + } + + if _, err := tmpfile.Write(resolvConfContent); err != nil { + cleanup() + t.Fatal(err) + } + if err := tmpfile.Close(); err != nil { + cleanup() + t.Fatal(err) + } + + return tmpfile.Name(), cleanup +} diff --git a/pkg/kubelet/network/dns/dns_test.go b/pkg/kubelet/network/dns/dns_test.go index 2832881a66f..7f279624c4d 100644 --- a/pkg/kubelet/network/dns/dns_test.go +++ b/pkg/kubelet/network/dns/dns_test.go @@ -19,8 +19,6 @@ package dns import ( "fmt" "net" - "os" - goruntime "runtime" "strconv" "strings" "testing" @@ -42,7 +40,10 @@ import ( ) var ( - fetchEvent = func(recorder *record.FakeRecorder) string { + // testHostNameserver and testHostDomain are also being used in dns_windows_test.go. + testHostNameserver = "8.8.8.8" + testHostDomain = "host.domain" + fetchEvent = func(recorder *record.FakeRecorder) string { select { case event := <-recorder.Events: return event @@ -457,26 +458,19 @@ func TestGetPodDNS(t *testing.T) { testCases := []struct { desc string expandedDNSConfig bool - skipOnWindows bool }{ { desc: "Not ExpandedDNSConfig", expandedDNSConfig: false, - skipOnWindows: true, }, { desc: "ExpandedDNSConfig", expandedDNSConfig: true, - skipOnWindows: true, }, } for _, tc := range testCases { t.Run(tc.desc, func(t *testing.T) { - // Skip tests that fail on Windows, as discussed during the SIG Testing meeting from January 10, 2023 - if tc.skipOnWindows && goruntime.GOOS == "windows" { - t.Skip("Skipping test that fails on Windows") - } defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.ExpandedDNSConfig, tc.expandedDNSConfig)() testGetPodDNS(t) }) @@ -541,8 +535,7 @@ func testGetPodDNS(t *testing.T) { t.Errorf("expected search \".\", got %+v", options[3].DNSSearch) } - testResolverConfig := "/etc/resolv.conf" - configurer = NewConfigurer(recorder, nodeRef, nil, testClusterDNS, testClusterDNSDomain, testResolverConfig) + configurer = NewConfigurer(recorder, nodeRef, nil, testClusterDNS, testClusterDNSDomain, defaultResolvConf) for i, pod := range pods { var err error dnsConfig, err := configurer.GetPodDNS(pod) @@ -599,8 +592,6 @@ func TestGetPodDNSCustom(t *testing.T) { testSvcDomain := fmt.Sprintf("svc.%s", testClusterDNSDomain) testNsSvcDomain := fmt.Sprintf("%s.svc.%s", testPodNamespace, testClusterDNSDomain) testNdotsOptionValue := "3" - testHostNameserver := "8.8.8.8" - testHostDomain := "host.domain" testPod := &v1.Pod{ ObjectMeta: metav1.ObjectMeta{ @@ -609,20 +600,11 @@ func TestGetPodDNSCustom(t *testing.T) { }, } - resolvConfContent := []byte(fmt.Sprintf("nameserver %s\nsearch %s\n", testHostNameserver, testHostDomain)) - tmpfile, err := os.CreateTemp("", "tmpResolvConf") - if err != nil { - t.Fatal(err) - } - defer os.Remove(tmpfile.Name()) - if _, err := tmpfile.Write(resolvConfContent); err != nil { - t.Fatal(err) - } - if err := tmpfile.Close(); err != nil { - t.Fatal(err) - } + resolvConf, cleanup := getResolvConf(t) + defer cleanup() - configurer := NewConfigurer(recorder, nodeRef, nil, []net.IP{netutils.ParseIPSloppy(testClusterNameserver)}, testClusterDNSDomain, tmpfile.Name()) + configurer := NewConfigurer(recorder, nodeRef, nil, []net.IP{netutils.ParseIPSloppy(testClusterNameserver)}, testClusterDNSDomain, resolvConf) + configurer.getHostDNSConfig = fakeGetHostDNSConfigCustom testCases := []struct { desc string diff --git a/pkg/kubelet/network/dns/dns_windows.go b/pkg/kubelet/network/dns/dns_windows.go new file mode 100644 index 00000000000..052c4e0eaee --- /dev/null +++ b/pkg/kubelet/network/dns/dns_windows.go @@ -0,0 +1,199 @@ +//go:build windows +// +build windows + +/* +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 dns + +import ( + "fmt" + "strings" + "syscall" + "unsafe" + + "golang.org/x/sys/windows" + "golang.org/x/sys/windows/registry" + runtimeapi "k8s.io/cri-api/pkg/apis/runtime/v1" + "k8s.io/klog/v2" +) + +const ( + netRegistry = `System\CurrentControlSet\Services\TCPIP\Parameters` + netIfacesRegistry = `System\CurrentControlSet\Services\TCPIP\Parameters\Interfaces` + maxHostnameLen = 128 + maxDomainNameLen = 128 + maxScopeIDLen = 256 + + // getHostDNSConfig will return the host's DNS configuration if the given resolverConfig argument + // is set to "Host". + hostResolvConf = "Host" +) + +// FixedInfo information: https://docs.microsoft.com/en-us/windows/win32/api/iptypes/ns-iptypes-fixed_info_w2ksp1 +type FixedInfo struct { + HostName [maxHostnameLen + 4]byte + DomainName [maxDomainNameLen + 4]byte + CurrentDNSServer *syscall.IpAddrString + DNSServerList syscall.IpAddrString + NodeType uint32 + ScopeID [maxScopeIDLen + 4]byte + EnableRouting uint32 + EnableProxy uint32 + EnableDNS uint32 +} + +var ( + // GetNetworkParams can be found in iphlpapi.dll + // see: https://docs.microsoft.com/windows/win32/api/iphlpapi/nf-iphlpapi-getnetworkparams + iphlpapidll = windows.MustLoadDLL("iphlpapi.dll") + procGetNetworkParams = iphlpapidll.MustFindProc("GetNetworkParams") +) + +func getHostDNSConfig(resolverConfig string) (*runtimeapi.DNSConfig, error) { + if resolverConfig != "" && resolverConfig != hostResolvConf { + err := fmt.Errorf(`Unexpected resolver config value: "%s". Expected "" or "%s".`, resolverConfig, hostResolvConf) + klog.ErrorS(err, "Cannot get host DNS Configuration.") + return nil, err + } + + var ( + hostDNS, hostSearch []string + err error + ) + // Get host DNS settings + if resolverConfig == hostResolvConf { + hostDNS, err = getDNSServerList() + if err != nil { + err = fmt.Errorf("Could not get the host's DNS Server List. Error: %w", err) + klog.ErrorS(err, "Encountered error while getting host's DNS Server List.") + return nil, err + } + hostSearch, err = getDNSSuffixList() + if err != nil { + err = fmt.Errorf("Could not get the host's DNS Suffix List. Error: %w", err) + klog.ErrorS(err, "Encountered error while getting host's DNS Suffix List.") + return nil, err + } + } + return &runtimeapi.DNSConfig{ + Servers: hostDNS, + Searches: hostSearch, + }, nil +} + +func elemInList(elem string, list []string) bool { + for _, e := range list { + if e == elem { + return true + } + } + return false +} + +func getRegistryStringValue(reg, key string) (string, error) { + regKey, err := registry.OpenKey(registry.LOCAL_MACHINE, reg, registry.QUERY_VALUE) + if err != nil { + return "", err + } + defer regKey.Close() + + regValue, _, err := regKey.GetStringValue(key) + return regValue, err +} + +// getDNSSuffixList reads DNS config file and returns the list of configured DNS suffixes +func getDNSSuffixList() ([]string, error) { + // We start with the general suffix list that apply to all network connections. + allSuffixes := []string{} + suffixes, err := getRegistryStringValue(netRegistry, "SearchList") + if err != nil { + return nil, err + } + allSuffixes = strings.Split(suffixes, ",") + + // Then we append the network-specific DNS suffix lists. + regKey, err := registry.OpenKey(registry.LOCAL_MACHINE, netIfacesRegistry, registry.ENUMERATE_SUB_KEYS) + if err != nil { + return nil, err + } + defer regKey.Close() + + ifaces, err := regKey.ReadSubKeyNames(0) + if err != nil { + return nil, err + } + for _, iface := range ifaces { + suffixes, err := getRegistryStringValue(fmt.Sprintf("%s\\%s", netIfacesRegistry, iface), "SearchList") + if err != nil { + return nil, err + } + if suffixes == "" { + continue + } + for _, suffix := range strings.Split(suffixes, ",") { + if !elemInList(suffix, allSuffixes) { + allSuffixes = append(allSuffixes, suffix) + } + } + } + + return allSuffixes, nil +} + +func getNetworkParams() (*FixedInfo, error) { + // We don't know how big we should make the byte buffer, but the call will tell us by + // setting the size afterwards. + var size int = 1 + buffer := make([]byte, 1) + ret, _, err := procGetNetworkParams.Call( + uintptr(unsafe.Pointer(&buffer[0])), + uintptr(unsafe.Pointer(&size)), + ) + if ret != uintptr(syscall.ERROR_BUFFER_OVERFLOW) { + err = fmt.Errorf("Unexpected return value %d from GetNetworkParams. Expected: %d. Error: %w", ret, syscall.ERROR_BUFFER_OVERFLOW, err) + return nil, err + } + + buffer = make([]byte, size) + ret, _, err = procGetNetworkParams.Call( + uintptr(unsafe.Pointer(&buffer[0])), + uintptr(unsafe.Pointer(&size)), + ) + if ret != 0 { + err = fmt.Errorf("Unexpected return value %d from GetNetworkParams. Expected: 0. Error: %w", ret, err) + return nil, err + } + + info := (*FixedInfo)(unsafe.Pointer(&buffer[0])) + return info, nil +} + +func getDNSServerList() ([]string, error) { + dnsServerList := []string{} + fixedInfo, err := getNetworkParams() + if err != nil { + return nil, err + } + + list := &(fixedInfo.DNSServerList) + for list != nil { + dnsServer := strings.TrimRight(string(list.IpAddress.String[:]), "\x00") + dnsServerList = append(dnsServerList, dnsServer) + list = list.Next + } + return dnsServerList, nil +} diff --git a/pkg/kubelet/network/dns/dns_windows_test.go b/pkg/kubelet/network/dns/dns_windows_test.go new file mode 100644 index 00000000000..5a415c02c5a --- /dev/null +++ b/pkg/kubelet/network/dns/dns_windows_test.go @@ -0,0 +1,42 @@ +//go:build windows +// +build windows + +/* +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 dns + +import ( + "testing" + + runtimeapi "k8s.io/cri-api/pkg/apis/runtime/v1" +) + +var ( + defaultResolvConf = hostResolvConf +) + +func fakeGetHostDNSConfigCustom(resolverConfig string) (*runtimeapi.DNSConfig, error) { + return &runtimeapi.DNSConfig{ + Servers: []string{testHostNameserver}, + Searches: []string{testHostDomain}, + }, nil +} + +// getResolvConf returns the hostResolvConf string, which will be used to get the Host's DNS configuration. +func getResolvConf(t *testing.T) (string, func()) { + return hostResolvConf, func() {} +}