Merge pull request #75969 from rajansandeep/translationcheck

Fix translation from kube-dns to CoreDNS Config to skip invalid values

cmd/kubeadm/app/phases/addons/dns/BUILD

@@ -48,6 +48,7 @@ go_library(
         "//staging/src/k8s.io/client-go/kubernetes/scheme:go_default_library",
         "//vendor/github.com/mholt/caddy/caddyfile:go_default_library",
         "//vendor/github.com/pkg/errors:go_default_library",
+        "//vendor/k8s.io/klog:go_default_library",
     ],
 )
 

cmd/kubeadm/app/phases/addons/dns/dns.go

@@ -19,6 +19,7 @@ package dns
 import (
 	"encoding/json"
 	"fmt"
+	"net"
 	"strings"
 
 	"github.com/mholt/caddy/caddyfile"
@@ -32,6 +33,7 @@ import (
 	kuberuntime "k8s.io/apimachinery/pkg/runtime"
 	clientset "k8s.io/client-go/kubernetes"
 	clientsetscheme "k8s.io/client-go/kubernetes/scheme"
+	"k8s.io/klog"
 	kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm"
 	kubeadmconstants "k8s.io/kubernetes/cmd/kubeadm/app/constants"
 	"k8s.io/kubernetes/cmd/kubeadm/app/images"
@@ -178,7 +180,7 @@ func coreDNSAddon(cfg *kubeadmapi.ClusterConfiguration, client clientset.Interfa
 		return err
 	}
 
-	upstreamNameserver, err := translateUpstreamNameServerOfKubeDNSToUpstreamProxyCoreDNS(kubeDNSUpstreamNameservers, kubeDNSConfigMap)
+	upstreamNameserver, err := translateUpstreamNameServerOfKubeDNSToUpstreamForwardCoreDNS(kubeDNSUpstreamNameservers, kubeDNSConfigMap)
 	if err != nil {
 		return err
 	}
@@ -310,7 +312,15 @@ func translateStubDomainOfKubeDNSToForwardCoreDNS(dataField string, kubeDNSConfi
 		}
 
 		var proxyStanza []interface{}
-		for domain, proxyIP := range stubDomainData {
+		for domain, proxyHosts := range stubDomainData {
+			proxyIP, err := omitHostnameInTranslation(proxyHosts)
+			if err != nil {
+				return "", errors.Wrap(err, "invalid format to parse for proxy")
+			}
+			if len(proxyIP) == 0 {
+				break
+			}
+
 			pStanza := map[string]interface{}{}
 			pStanza["keys"] = []string{domain + ":53"}
 			pStanza["body"] = [][]string{
@@ -336,22 +346,27 @@ func translateStubDomainOfKubeDNSToForwardCoreDNS(dataField string, kubeDNSConfi
 	return "", nil
 }
 
-// translateUpstreamNameServerOfKubeDNSToUpstreamProxyCoreDNS translates UpstreamNameServer Data in kube-dns ConfigMap
+// translateUpstreamNameServerOfKubeDNSToUpstreamForwardCoreDNS translates UpstreamNameServer Data in kube-dns ConfigMap
 // in the form of Proxy for the CoreDNS Corefile.
-func translateUpstreamNameServerOfKubeDNSToUpstreamProxyCoreDNS(dataField string, kubeDNSConfigMap *v1.ConfigMap) (string, error) {
+func translateUpstreamNameServerOfKubeDNSToUpstreamForwardCoreDNS(dataField string, kubeDNSConfigMap *v1.ConfigMap) (string, error) {
 	if kubeDNSConfigMap == nil {
 		return "", nil
 	}
 
 	if upstreamValues, ok := kubeDNSConfigMap.Data[dataField]; ok {
-		var upstreamProxyIP []string
+		var upstreamProxyValues []string
 
-		err := json.Unmarshal([]byte(upstreamValues), &upstreamProxyIP)
+		err := json.Unmarshal([]byte(upstreamValues), &upstreamProxyValues)
 		if err != nil {
 			return "", errors.Wrap(err, "failed to parse JSON from 'kube-dns ConfigMap")
 		}
 
-		coreDNSProxyStanzaList := strings.Join(upstreamProxyIP, " ")
+		upstreamProxyValues, err = omitHostnameInTranslation(upstreamProxyValues)
+		if err != nil {
+			return "", errors.Wrap(err, "invalid format to parse for proxy")
+		}
+
+		coreDNSProxyStanzaList := strings.Join(upstreamProxyValues, " ")
 		return coreDNSProxyStanzaList, nil
 	}
 	return "/etc/resolv.conf", nil
@@ -401,7 +416,10 @@ func translateFederationsofKubeDNSToCoreDNS(dataField, coreDNSDomain string, kub
 // prepCorefileFormat indents the output of the Corefile caddytext and replaces tabs with spaces
 // to neatly format the configmap, making it readable.
 func prepCorefileFormat(s string, indentation int) string {
-	r := []string{}
+	var r []string
+	if s == "" {
+		return ""
+	}
 	for _, line := range strings.Split(s, "\n") {
 		indented := strings.Repeat(" ", indentation) + line
 		r = append(r, indented)
@@ -409,3 +427,26 @@ func prepCorefileFormat(s string, indentation int) string {
 	corefile := strings.Join(r, "\n")
 	return "\n" + strings.Replace(corefile, "\t", "   ", -1)
 }
+
+// omitHostnameInTranslation checks if the data extracted from the kube-dns ConfigMap contains a valid
+// IP address. Hostname to nameservers is not supported on CoreDNS and will
+// skip that particular instance, if there is any hostname present.
+func omitHostnameInTranslation(forwardIPs []string) ([]string, error) {
+	index := 0
+	for _, value := range forwardIPs {
+		proxyHost, _, err := kubeadmutil.ParseHostPort(value)
+		if err != nil {
+			return nil, err
+		}
+		parseIP := net.ParseIP(proxyHost)
+		if parseIP == nil {
+			klog.Warningf("your kube-dns configuration contains a hostname %v. It will be omitted in the translation to CoreDNS as hostnames are unsupported", proxyHost)
+		} else {
+			forwardIPs[index] = value
+			index++
+		}
+	}
+	forwardIPs = forwardIPs[:index]
+
+	return forwardIPs, nil
+}

cmd/kubeadm/app/phases/addons/dns/dns_test.go

@@ -190,7 +190,7 @@ func TestTranslateStubDomainKubeDNSToCoreDNS(t *testing.T) {
 		expectTwo string
 	}{
 		{
-			name: "valid call 1",
+			name: "valid call with multiple IPs",
 			configMap: &v1.ConfigMap{
 				ObjectMeta: metav1.ObjectMeta{
 					Name:      "kube-dns",
@@ -243,7 +243,7 @@ func TestTranslateStubDomainKubeDNSToCoreDNS(t *testing.T) {
 			expectOne: "",
 		},
 		{
-			name: "valid call 2",
+			name: "valid call",
 			configMap: &v1.ConfigMap{
 				ObjectMeta: metav1.ObjectMeta{
 					Name:      "kube-dns",
@@ -284,6 +284,64 @@ func TestTranslateStubDomainKubeDNSToCoreDNS(t *testing.T) {
        forward . 1.2.3.4:5300
     }`,
 		},
+		{
+			name: "If Hostname present: Omit Hostname",
+			configMap: &v1.ConfigMap{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "kube-dns",
+					Namespace: "kube-system",
+				},
+				Data: map[string]string{
+					"stubDomains":         `{"bar.com" : ["1.2.3.4:5300","service.consul"], "my.cluster.local" : ["2.3.4.5"], "foo.com" : ["service.consul"]}`,
+					"upstreamNameservers": `["8.8.8.8", "8.8.4.4"]`,
+				},
+			},
+
+			expectOne: `
+    bar.com:53 {
+       errors
+       cache 30
+       loop
+       forward . 1.2.3.4:5300
+    }
+    
+    my.cluster.local:53 {
+       errors
+       cache 30
+       loop
+       forward . 2.3.4.5
+    }`,
+			expectTwo: `
+    my.cluster.local:53 {
+       errors
+       cache 30
+       loop
+       forward . 2.3.4.5
+    }
+    
+    bar.com:53 {
+       errors
+       cache 30
+       loop
+       forward . 1.2.3.4:5300
+    }`,
+		},
+		{
+			name: "All hostname: return empty",
+			configMap: &v1.ConfigMap{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "kube-dns",
+					Namespace: "kube-system",
+				},
+				Data: map[string]string{
+					"stubDomains":         `{"foo.com" : ["service.consul"], "my.cluster.local" : ["ns.foo.com"]}`,
+					"upstreamNameservers": `["8.8.8.8", "8.8.4.4"]`,
+				},
+			},
+
+			expectOne: "",
+			expectTwo: "",
+		},
 		{
 			name: "missing stubDomains",
 			configMap: &v1.ConfigMap{
@@ -305,7 +363,7 @@ func TestTranslateStubDomainKubeDNSToCoreDNS(t *testing.T) {
 			if err != nil {
 				t.Errorf("unexpected error: %v", err)
 			}
-			if !strings.Contains(out, testCase.expectOne) && !strings.Contains(out, testCase.expectTwo) {
+			if !strings.EqualFold(out, testCase.expectOne) && !strings.EqualFold(out, testCase.expectTwo) {
 				t.Errorf("expected to find %q or %q in output: %q", testCase.expectOne, testCase.expectTwo, out)
 			}
 		})
@@ -358,14 +416,56 @@ func TestTranslateUpstreamKubeDNSToCoreDNS(t *testing.T) {
 
 			expect: "8.8.8.8 8.8.4.4",
 		},
+		{
+			name: "Hostname present: expect NameServer to omit the hostname",
+			configMap: &v1.ConfigMap{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "kubedns",
+					Namespace: "kube-system",
+				},
+				Data: map[string]string{
+					"upstreamNameservers": `["service.consul", "ns.foo.com", "8.8.4.4", "ns.moo.com", "ns.bar.com"]`,
+				},
+			},
+
+			expect: "8.8.4.4",
+		},
+		{
+			name: "All hostnames: return empty",
+			configMap: &v1.ConfigMap{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "kube-dns",
+					Namespace: "kube-system",
+				},
+				Data: map[string]string{
+					"upstreamNameservers": `["service.consul", "ns.foo.com"]`,
+				},
+			},
+
+			expect: "",
+		},
+		{
+			name: "IPv6: expect list of Name Server IP addresses",
+			configMap: &v1.ConfigMap{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "kubedns",
+					Namespace: "kube-system",
+				},
+				Data: map[string]string{
+					"upstreamNameservers": `["[2003::1]:53", "8.8.4.4"]`,
+				},
+			},
+
+			expect: "[2003::1]:53 8.8.4.4",
+		},
 	}
 	for _, testCase := range testCases {
 		t.Run(testCase.name, func(t *testing.T) {
-			out, err := translateUpstreamNameServerOfKubeDNSToUpstreamProxyCoreDNS(kubeDNSUpstreamNameservers, testCase.configMap)
+			out, err := translateUpstreamNameServerOfKubeDNSToUpstreamForwardCoreDNS(kubeDNSUpstreamNameservers, testCase.configMap)
 			if err != nil {
 				t.Errorf("unexpected error: %v", err)
 			}
-			if !strings.Contains(out, testCase.expect) {
+			if !strings.EqualFold(out, testCase.expect) {
 				t.Errorf("expected to find %q in output: %q", testCase.expect, out)
 			}
 		})
@@ -414,6 +514,7 @@ func TestTranslateFederationKubeDNSToCoreDNS(t *testing.T) {
 			},
 
 			expectOne: "",
+			expectTwo: "",
 		},
 		{
 			name: "missing federations data",
@@ -429,6 +530,7 @@ func TestTranslateFederationKubeDNSToCoreDNS(t *testing.T) {
 			},
 
 			expectOne: "",
+			expectTwo: "",
 		},
 	}
 	for _, testCase := range testCases {
@@ -437,7 +539,7 @@ func TestTranslateFederationKubeDNSToCoreDNS(t *testing.T) {
 			if err != nil {
 				t.Errorf("unexpected error: %v", err)
 			}
-			if !strings.Contains(out, testCase.expectOne) && !strings.Contains(out, testCase.expectTwo) {
+			if !strings.EqualFold(out, testCase.expectOne) && !strings.EqualFold(out, testCase.expectTwo) {
 				t.Errorf("expected to find %q or %q in output: %q", testCase.expectOne, testCase.expectTwo, out)
 			}
 		})