Fix CoreDNS config if version is greater than or equal to 1.7

traefik · Jul 27, 2020 · a54e9b3 · a54e9b3
1 parent e28f35b
commit a54e9b3
Show file tree

Hide file tree

Showing 13 changed files with 161 additions and 55 deletions.
diff --git a/integration/coredns_test.go b/integration/coredns_test.go
@@ -21,6 +21,7 @@ func (s *CoreDNSSuite) SetUpSuite(c *check.C) {
 		"coredns/coredns:1.7.0",
 		"giantswarm/tiny-tools:3.9",
 	}
+
 	s.startk3s(c, requiredImages)
 	s.startWhoami(c)
 	s.installTinyToolsMaesh(c)
@@ -54,12 +55,11 @@ func (s *CoreDNSSuite) TestCoreDNSVersionSafe(c *check.C) {
 		},
 	}
 
-	s.createResources(c, "testdata/coredns/corednssafe.yaml")
-	defer s.deleteResources(c, "testdata/coredns/corednssafe.yaml")
-
 	for _, test := range testCases {
+		s.createResources(c, "testdata/coredns/corednssafe.yaml")
 		s.WaitForCoreDNS(c)
-		c.Log("Testing compatibility with " + test.desc)
+
+		c.Logf("Testing compatibility with %s", test.desc)
 		s.setCoreDNSVersion(c, test.version)
 
 		cmd := s.maeshPrepareWithArgs()
@@ -73,6 +73,8 @@ func (s *CoreDNSSuite) TestCoreDNSVersionSafe(c *check.C) {
 		} else {
 			c.Assert(err, checker.IsNil)
 		}
+
+		s.deleteResources(c, "testdata/coredns/corednssafe.yaml")
 	}
 }
 
@@ -95,12 +97,11 @@ func (s *CoreDNSSuite) TestCoreDNSVersion(c *check.C) {
 		},
 	}
 
-	s.createResources(c, "testdata/coredns/coredns.yaml")
-	defer s.deleteResources(c, "testdata/coredns/coredns.yaml")
-
 	for _, test := range testCases {
+		s.createResources(c, "testdata/coredns/coredns.yaml")
 		s.WaitForCoreDNS(c)
-		c.Log("Testing compatibility with " + test.desc)
+
+		c.Logf("Testing compatibility with %s", test.desc)
 		s.setCoreDNSVersion(c, test.version)
 
 		cmd := s.maeshPrepareWithArgs()
@@ -109,22 +110,44 @@ func (s *CoreDNSSuite) TestCoreDNSVersion(c *check.C) {
 
 		c.Log(string(output))
 		c.Assert(err, checker.IsNil)
+
+		s.deleteResources(c, "testdata/coredns/coredns.yaml")
 	}
 }
 
 func (s *CoreDNSSuite) TestCoreDNSDig(c *check.C) {
-	s.createResources(c, "testdata/coredns/coredns.yaml")
-	defer s.deleteResources(c, "testdata/coredns/coredns.yaml")
-	s.WaitForCoreDNS(c)
+	testCases := []struct {
+		desc    string
+		version string
+	}{
+		{
+			desc:    "CoreDNS 1.6.3",
+			version: "1.6.3",
+		},
+		{
+			desc:    "CoreDNS 1.7.0",
+			version: "1.7.0",
+		},
+	}
+
+	for _, test := range testCases {
+		s.createResources(c, "testdata/coredns/coredns.yaml")
+		s.WaitForCoreDNS(c)
 
-	cmd := s.startMaeshBinaryCmd(c, false, false)
-	err := cmd.Start()
+		c.Logf("Testing dig with %s", test.desc)
+		s.setCoreDNSVersion(c, test.version)
 
-	c.Assert(err, checker.IsNil)
-	defer s.stopMaeshBinary(c, cmd.Process)
+		cmd := s.startMaeshBinaryCmd(c, false, false)
 
-	pod := s.getToolsPodMaesh(c)
-	c.Assert(pod, checker.NotNil)
+		err := cmd.Start()
+		c.Assert(err, checker.IsNil)
 
-	s.digHost(c, pod.Name, pod.Namespace, "whoami.whoami.maesh")
+		pod := s.getToolsPodMaesh(c)
+		c.Assert(pod, checker.NotNil)
+
+		s.digHost(c, pod.Name, pod.Namespace, "whoami.whoami.maesh")
+		s.stopMaeshBinary(c, cmd.Process)
+
+		s.deleteResources(c, "testdata/coredns/coredns.yaml")
+	}
 }
diff --git a/integration/testdata/coredns/coredns.yaml b/integration/testdata/coredns/coredns.yaml
@@ -58,7 +58,6 @@ data:
         ready
         kubernetes cluster.local in-addr.arpa ip6.arpa {
           pods insecure
-          upstream
           fallthrough in-addr.arpa ip6.arpa
         }
         hosts /etc/coredns/NodeHosts {

diff --git a/integration/testdata/coredns/corednssafe.yaml b/integration/testdata/coredns/corednssafe.yaml
@@ -57,7 +57,6 @@ data:
         health
         kubernetes cluster.local in-addr.arpa ip6.arpa {
           pods insecure
-          upstream
           fallthrough in-addr.arpa ip6.arpa
         }
         hosts /etc/coredns/NodeHosts {

diff --git a/pkg/dns/dns.go b/pkg/dns/dns.go
@@ -10,6 +10,7 @@ import (
 	"github.com/cenkalti/backoff/v4"
 	"github.com/containous/traefik/v2/pkg/safe"
 	"github.com/google/uuid"
+	goversion "github.com/hashicorp/go-version"
 	"github.com/sirupsen/logrus"
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
@@ -31,15 +32,7 @@ const (
 	coreFileTrailer = "#### End Maesh Block"
 )
 
-var (
-	supportedCoreDNSVersions = []string{
-		"1.3",
-		"1.4",
-		"1.5",
-		"1.6",
-		"1.7",
-	}
-)
+var versionCoreDNS17 = goversion.Must(goversion.NewVersion("1.7"))
 
 // Client holds the client for interacting with the k8s DNS system.
 type Client struct {
@@ -84,6 +77,7 @@ func (c *Client) coreDNSMatch() (bool, error) {
 	c.logger.Info("Checking CoreDNS")
 
 	deployment, err := c.kubeClient.AppsV1().Deployments(metav1.NamespaceSystem).Get("coredns", metav1.GetOptions{})
+
 	if kerrors.IsNotFound(err) {
 		c.logger.Debugf("CoreDNS deployment does not exist in namespace %q", metav1.NamespaceSystem)
 		return false, nil
@@ -93,36 +87,25 @@ func (c *Client) coreDNSMatch() (bool, error) {
 		return false, fmt.Errorf("unable to get CoreDNS deployment in namespace %q: %w", metav1.NamespaceSystem, err)
 	}
 
-	var version string
-
-	for _, container := range deployment.Spec.Template.Spec.Containers {
-		if container.Name != "coredns" {
-			continue
-		}
+	version, err := c.getCoreDNSVersion(deployment)
+	if err != nil {
+		return false, err
+	}
 
-		sp := strings.Split(container.Image, ":")
-		version = sp[len(sp)-1]
+	versionConstraint, err := goversion.NewConstraint(">= 1.3, < 1.8")
+	if err != nil {
+		return false, err
 	}
 
-	if !isCoreDNSVersionSupported(version) {
-		return false, fmt.Errorf("unsupported CoreDNS version %q, (supported versions are: %s)", version, strings.Join(supportedCoreDNSVersions, ","))
+	if !versionConstraint.Check(version) {
+		return false, fmt.Errorf("unsupported CoreDNS version %q", version)
 	}
 
 	c.logger.Info("CoreDNS match")
 
 	return true, nil
 }
 
-func isCoreDNSVersionSupported(versionLine string) bool {
-	for _, v := range supportedCoreDNSVersions {
-		if strings.Contains(versionLine, v) {
-			return true
-		}
-	}
-
-	return false
-}
-
 func (c *Client) kubeDNSMatch() (bool, error) {
 	c.logger.Info("Checking KubeDNS")
 
@@ -169,6 +152,11 @@ func (c *Client) ConfigureCoreDNS(coreDNSNamespace, clusterDomain, maeshNamespac
 }
 
 func (c *Client) patchCoreDNSConfig(deployment *appsv1.Deployment, clusterDomain, maeshNamespace string) (*corev1.ConfigMap, error) {
+	coreDNSVersion, err := c.getCoreDNSVersion(deployment)
+	if err != nil {
+		return nil, err
+	}
+
 	customConfigMap, err := c.getConfigMap(deployment, "coredns-custom")
 
 	// For AKS the CoreDNS config have to be added to the coredns-custom ConfigMap.
@@ -178,6 +166,7 @@ func (c *Client) patchCoreDNSConfig(deployment *appsv1.Deployment, clusterDomain
 			clusterDomain,
 			maeshNamespace,
 			"",
+			coreDNSVersion,
 		)
 
 		return customConfigMap, nil
@@ -192,12 +181,18 @@ func (c *Client) patchCoreDNSConfig(deployment *appsv1.Deployment, clusterDomain
 		clusterDomain,
 		maeshNamespace,
 		coreDNSConfigMap.Data["Corefile"],
+		coreDNSVersion,
 	)
 
 	return coreDNSConfigMap, nil
 }
 
-func (c *Client) addMaeshStubDomain(clusterDomain, maeshNamespace, coreDNSConfig string) string {
+func (c *Client) addMaeshStubDomain(clusterDomain, maeshNamespace, coreDNSConfig string, coreDNSVersion *goversion.Version) string {
+	// config already contains the maesh block.
+	if strings.Contains(coreDNSConfig, coreFileHeader) {
+		return coreDNSConfig
+	}
+
 	stubDomainFormat := `
 %[4]s
 maesh:53 {
@@ -208,7 +203,7 @@ maesh:53 {
     }
     kubernetes %[1]s in-addr.arpa ip6.arpa {
         pods insecure
-        upstream
+        %[6]s
         fallthrough in-addr.arpa ip6.arpa
     }
     forward . /etc/resolv.conf
@@ -219,21 +214,36 @@ maesh:53 {
 }
 %[5]s
 `
+	upstream := ""
+
+	if coreDNSVersion.LessThan(versionCoreDNS17) {
+		upstream = "upstream"
+	}
 
 	stubDomain := fmt.Sprintf(stubDomainFormat,
 		clusterDomain,
 		strings.Replace(clusterDomain, ".", "\\.", -1),
 		maeshNamespace,
 		coreFileHeader,
 		coreFileTrailer,
+		upstream,
 	)
 
-	// CoreDNS config already contains the maesh block.
-	if strings.Contains(coreDNSConfig, coreFileHeader) {
-		return coreDNSConfig
+	return coreDNSConfig + stubDomain
+}
+
+func (c *Client) getCoreDNSVersion(deployment *appsv1.Deployment) (*goversion.Version, error) {
+	for _, container := range deployment.Spec.Template.Spec.Containers {
+		if container.Name != "coredns" {
+			continue
+		}
+
+		parts := strings.Split(container.Image, ":")
+
+		return goversion.NewVersion(parts[len(parts)-1])
 	}
 
-	return coreDNSConfig + stubDomain
+	return nil, fmt.Errorf("unable to get CoreDNS container in deployment %q/%q", deployment.Namespace, deployment.Name)
 }
 
 // ConfigureKubeDNS patches the KubeDNS configuration for Maesh.

diff --git a/pkg/dns/dns_test.go b/pkg/dns/dns_test.go
@@ -110,6 +110,12 @@ func TestConfigureCoreDNS(t *testing.T) {
 			expectedCorefile: ".:53 {\n    errors\n    health {\n        lameduck 5s\n    }\n    ready\n    kubernetes {{ pillar['dns_domain'] }} in-addr.arpa ip6.arpa {\n        pods insecure\n        fallthrough in-addr.arpa ip6.arpa\n        ttl 30\n    }\n    prometheus :9153\n    forward . /etc/resolv.conf\n    cache 30\n    loop\n    reload\n    loadbalance\n}\n",
 			expectedCustom:   "\n#### Begin Maesh Block\nmaesh:53 {\n    errors\n    rewrite continue {\n        name regex ([a-zA-Z0-9-_]*)\\.([a-zv0-9-_]*)\\.maesh toto-{1}-6d61657368-{2}.toto.svc.titi\n        answer name toto-([a-zA-Z0-9-_]*)-6d61657368-([a-zA-Z0-9-_]*)\\.toto\\.svc\\.titi {1}.{2}.maesh\n    }\n    kubernetes titi in-addr.arpa ip6.arpa {\n        pods insecure\n        upstream\n        fallthrough in-addr.arpa ip6.arpa\n    }\n    forward . /etc/resolv.conf\n    cache 30\n    loop\n    reload\n    loadbalance\n}\n#### End Maesh Block\n",
 		},
+		{
+			desc:             "Config of CoreDNS 1.7",
+			mockFile:         "configurecoredns_17.yaml",
+			expectedErr:      false,
+			expectedCorefile: ".:53 {\n    errors\n    health {\n        lameduck 5s\n    }\n    ready\n    kubernetes {{ pillar['dns_domain'] }} in-addr.arpa ip6.arpa {\n        pods insecure\n        fallthrough in-addr.arpa ip6.arpa\n        ttl 30\n    }\n    prometheus :9153\n    forward . /etc/resolv.conf\n    cache 30\n    loop\n    reload\n    loadbalance\n}\n\n#### Begin Maesh Block\nmaesh:53 {\n    errors\n    rewrite continue {\n        name regex ([a-zA-Z0-9-_]*)\\.([a-zv0-9-_]*)\\.maesh toto-{1}-6d61657368-{2}.toto.svc.titi\n        answer name toto-([a-zA-Z0-9-_]*)-6d61657368-([a-zA-Z0-9-_]*)\\.toto\\.svc\\.titi {1}.{2}.maesh\n    }\n    kubernetes titi in-addr.arpa ip6.arpa {\n        pods insecure\n        \n        fallthrough in-addr.arpa ip6.arpa\n    }\n    forward . /etc/resolv.conf\n    cache 30\n    loop\n    reload\n    loadbalance\n}\n#### End Maesh Block\n",
+		},
 		{
 			desc:        "Missing CoreDNS deployment",
 			mockFile:    "configurecoredns_missing_deployment.yaml",

diff --git a/pkg/dns/testdata/configurecoredns_17.yaml b/pkg/dns/testdata/configurecoredns_17.yaml
@@ -0,0 +1,48 @@
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: coredns
+  namespace: kube-system
+spec:
+  template:
+    spec:
+      containers:
+        - name: coredns
+          image: coredns:1.7.0
+      volumes:
+        - configMap:
+            name: "other-cfgmap"
+        - configMap:
+            name: "coredns"
+---
+apiVersion: v1
+kind: ConfigMap
+metadata:
+  name: other-cfgmap
+  namespace: kube-system
+---
+apiVersion: v1
+kind: ConfigMap
+metadata:
+  name: coredns
+  namespace: kube-system
+data:
+  Corefile: |
+    .:53 {
+        errors
+        health {
+            lameduck 5s
+        }
+        ready
+        kubernetes {{ pillar['dns_domain'] }} in-addr.arpa ip6.arpa {
+            pods insecure
+            fallthrough in-addr.arpa ip6.arpa
+            ttl 30
+        }
+        prometheus :9153
+        forward . /etc/resolv.conf
+        cache 30
+        loop
+        reload
+        loadbalance
+    }
diff --git a/pkg/dns/testdata/configurecoredns_already_patched.yaml b/pkg/dns/testdata/configurecoredns_already_patched.yaml
@@ -6,6 +6,9 @@ metadata:
 spec:
   template:
     spec:
+      containers:
+        - name: coredns
+          image: coredns:1.6.0
       volumes:
         - configMap:
             name: "other-cfgmap"

diff --git a/pkg/dns/testdata/configurecoredns_custom_already_patched.yaml b/pkg/dns/testdata/configurecoredns_custom_already_patched.yaml
@@ -6,6 +6,9 @@ metadata:
 spec:
   template:
     spec:
+      containers:
+        - name: coredns
+          image: coredns:1.6.0
       volumes:
         - configMap:
             name: "coredns"

diff --git a/pkg/dns/testdata/configurecoredns_custom_not_patched.yaml b/pkg/dns/testdata/configurecoredns_custom_not_patched.yaml
@@ -6,6 +6,9 @@ metadata:
 spec:
   template:
     spec:
+      containers:
+        - name: coredns
+          image: coredns:1.6.0
       volumes:
         - configMap:
             name: "coredns"

diff --git a/pkg/dns/testdata/configurecoredns_not_patched.yaml b/pkg/dns/testdata/configurecoredns_not_patched.yaml
@@ -6,6 +6,9 @@ metadata:
 spec:
   template:
     spec:
+      containers:
+        - name: coredns
+          image: coredns:1.6.0
       volumes:
         - configMap:
             name: "other-cfgmap"