From 13ffe2cf4ea447a0d2da26fba93a3632deadcab0 Mon Sep 17 00:00:00 2001
From: Alexander Kanevskiy <alexander.kanevskiy@intel.com>
Date: Mon, 6 Nov 2017 16:00:11 +0200
Subject: [PATCH] kubeadm: don't create duplicate volume/mount

If certificates for etcd are located in the same directory or
subdirectories of kubernetes pki directory, don't create separate
volumes and mounts in manifests.

Fixes kubernetes/kubeadm#522
---
 .../app/phases/controlplane/volumes.go        |  9 ++++----
 .../app/phases/controlplane/volumes_test.go   | 21 +++++++++++++------
 2 files changed, 20 insertions(+), 10 deletions(-)

diff --git a/cmd/kubeadm/app/phases/controlplane/volumes.go b/cmd/kubeadm/app/phases/controlplane/volumes.go
index 27faa874374..690cd01a0ef 100644
--- a/cmd/kubeadm/app/phases/controlplane/volumes.go
+++ b/cmd/kubeadm/app/phases/controlplane/volumes.go
@@ -57,7 +57,7 @@ func getHostPathVolumesForTheControlPlane(cfg *kubeadmapi.MasterConfiguration) c
 
 	// If external etcd is specified, mount the directories needed for accessing the CA/serving certs and the private key
 	if len(cfg.Etcd.Endpoints) != 0 {
-		etcdVols, etcdVolMounts := getEtcdCertVolumes(cfg.Etcd)
+		etcdVols, etcdVolMounts := getEtcdCertVolumes(cfg.Etcd, cfg.CertificatesDir)
 		mounts.AddHostPathMounts(kubeadmconstants.KubeAPIServer, etcdVols, etcdVolMounts)
 	}
 
@@ -166,14 +166,15 @@ func (c *controlPlaneHostPathMounts) addComponentVolumeMount(component string, v
 }
 
 // getEtcdCertVolumes returns the volumes/volumemounts needed for talking to an external etcd cluster
-func getEtcdCertVolumes(etcdCfg kubeadmapi.Etcd) ([]v1.Volume, []v1.VolumeMount) {
+func getEtcdCertVolumes(etcdCfg kubeadmapi.Etcd, k8sCertificatesDir string) ([]v1.Volume, []v1.VolumeMount) {
 	certPaths := []string{etcdCfg.CAFile, etcdCfg.CertFile, etcdCfg.KeyFile}
 	certDirs := sets.NewString()
 	for _, certPath := range certPaths {
 		certDir := filepath.Dir(certPath)
 		// Ignore ".", which is the result of passing an empty path.
-		// Also ignore the cert directories that already may be mounted; /etc/ssl/certs and /etc/pki. If the etcd certs are in there, it's okay, we don't have to do anything
-		if certDir == "." || strings.HasPrefix(certDir, caCertsVolumePath) || strings.HasPrefix(certDir, caCertsPkiVolumePath) {
+		// Also ignore the cert directories that already may be mounted; /etc/ssl/certs, /etc/pki or Kubernetes CertificatesDir
+		// If the etcd certs are in there, it's okay, we don't have to do anything
+		if certDir == "." || strings.HasPrefix(certDir, caCertsVolumePath) || strings.HasPrefix(certDir, caCertsPkiVolumePath) || strings.HasPrefix(certDir, k8sCertificatesDir) {
 			continue
 		}
 		// Filter out any existing hostpath mounts in the list that contains a subset of the path
diff --git a/cmd/kubeadm/app/phases/controlplane/volumes_test.go b/cmd/kubeadm/app/phases/controlplane/volumes_test.go
index da403383593..e0032bdafa0 100644
--- a/cmd/kubeadm/app/phases/controlplane/volumes_test.go
+++ b/cmd/kubeadm/app/phases/controlplane/volumes_test.go
@@ -30,6 +30,7 @@ import (
 
 func TestGetEtcdCertVolumes(t *testing.T) {
 	hostPathDirectoryOrCreate := v1.HostPathDirectoryOrCreate
+	k8sCertifcatesDir := "/etc/kubernetes/pki"
 	var tests = []struct {
 		ca, cert, key string
 		vol           []v1.Volume
@@ -59,6 +60,14 @@ func TestGetEtcdCertVolumes(t *testing.T) {
 			vol:      []v1.Volume{},
 			volMount: []v1.VolumeMount{},
 		},
+		{
+			// Should ignore files in Kubernetes PKI directory (and subdirs)
+			ca:       k8sCertifcatesDir + "/ca/my-etcd-ca.crt",
+			cert:     k8sCertifcatesDir + "/my-etcd.crt",
+			key:      k8sCertifcatesDir + "/my-etcd.key",
+			vol:      []v1.Volume{},
+			volMount: []v1.VolumeMount{},
+		},
 		{
 			// All in the same dir
 			ca:   "/var/lib/certs/etcd/my-etcd-ca.crt",
@@ -228,7 +237,7 @@ func TestGetEtcdCertVolumes(t *testing.T) {
 			CAFile:   rt.ca,
 			CertFile: rt.cert,
 			KeyFile:  rt.key,
-		})
+		}, k8sCertifcatesDir)
 		if !reflect.DeepEqual(actualVol, rt.vol) {
 			t.Errorf(
 				"failed getEtcdCertVolumes:\n\texpected: %v\n\t  actual: %v",
@@ -389,7 +398,7 @@ func TestGetHostPathVolumesForTheControlPlane(t *testing.T) {
 		Name: "etcd-certs-1",
 		VolumeSource: v1.VolumeSource{
 			HostPath: &v1.HostPathVolumeSource{
-				Path: "/var/lib/certs/etcd",
+				Path: "/var/lib/etcd/certs",
 				Type: &hostPathDirectoryOrCreate,
 			},
 		},
@@ -460,7 +469,7 @@ func TestGetHostPathVolumesForTheControlPlane(t *testing.T) {
 	}
 	volMountMap2[kubeadmconstants.KubeAPIServer]["etcd-certs-1"] = v1.VolumeMount{
 		Name:      "etcd-certs-1",
-		MountPath: "/var/lib/certs/etcd",
+		MountPath: "/var/lib/etcd/certs",
 		ReadOnly:  true,
 	}
 	volMountMap2[kubeadmconstants.KubeControllerManager] = map[string]v1.VolumeMount{}
@@ -505,14 +514,14 @@ func TestGetHostPathVolumesForTheControlPlane(t *testing.T) {
 			volMount: volMountMap,
 		},
 		{
-			// Should ignore files in /etc/ssl/certs
+			// Should ignore files in /etc/ssl/certs and in CertificatesDir
 			cfg: &kubeadmapi.MasterConfiguration{
 				CertificatesDir: testCertsDir,
 				Etcd: kubeadmapi.Etcd{
 					Endpoints: []string{"foo"},
 					CAFile:    "/etc/certs/etcd/my-etcd-ca.crt",
-					CertFile:  "/var/lib/certs/etcd/my-etcd.crt",
-					KeyFile:   "/var/lib/certs/etcd/my-etcd.key",
+					CertFile:  testCertsDir + "/etcd/my-etcd.crt",
+					KeyFile:   "/var/lib/etcd/certs/my-etcd.key",
 				},
 			},
 			vol:      volMap2,