Only generate certs not provided by the user (#1958)

* wip adding tls encryption to etcd communication

* remove unwanted tab

* fix etcd cert name

* wip update args

* rename etcd server cert

* rename etcdServerPair

* updated args

* Use CSE to generate etcd certs

* fix var replacement in apiserver.yaml

* small fixes

* WIP working for 1 master

* add different certs per peer

* fix peer cert and race condition

* added certs for each peer and make peer certs arrays

* peer array of certs and keys

* remove duplicate line

* only generate right number of peer certs

* Use master_index to keep track of host

* fix typo

* Removing todo since CoreOS is broken

* Prevent race condition in go routine

* Make sure etcd certs generated before etcd restart

* update etcd certs extended key usages

* simplify array secret parameter encoding func

* added etcdctl env variables

* fix typo and wait for all go routines

* typo fix

* improve code readability

* change client key access permissions

* wip only generate certs not provided

* add kubeconfig cert to verification

* fix missing > 0

* fix logic with etcd certs

* wip add tests

* Simplified logic and added tests

* wip only generate certs not provided

* add kubeconfig cert to verification

* fix missing > 0

* fix logic with etcd certs

* wip add tests

* Simplified logic and added tests

* fix missed conflict

pkg/acsengine/defaults.go

@@ -638,7 +638,14 @@ func setStorageDefaults(a *api.Properties) {
 }
 
 func setDefaultCerts(a *api.Properties) (bool, error) {
-	if !certGenerationRequired(a) {
+
+	if a.MasterProfile == nil || a.OrchestratorProfile.OrchestratorType != api.Kubernetes {
+		return false, nil
+	}
+
+	provided := certsAlreadyPresent(a.CertificateProfile, a.MasterProfile.Count)
+
+	if areAllTrue(provided) {
 		return false, nil
 	}
 
@@ -666,7 +673,7 @@ func setDefaultCerts(a *api.Properties) (bool, error) {
 
 	// use the specified Certificate Authority pair, or generate a new pair
 	var caPair *PkiKeyCertPair
-	if len(a.CertificateProfile.CaCertificate) != 0 && len(a.CertificateProfile.CaPrivateKey) != 0 {
+	if provided["ca"] {
 		caPair = &PkiKeyCertPair{CertificatePem: a.CertificateProfile.CaCertificate, PrivateKeyPem: a.CertificateProfile.CaPrivateKey}
 	} else {
 		caCertificate, caPrivateKey, err := createCertificate("ca", nil, nil, false, false, nil, nil, nil)
@@ -689,59 +696,71 @@ func setDefaultCerts(a *api.Properties) (bool, error) {
 		return false, err
 	}
 
-	a.CertificateProfile.APIServerCertificate = apiServerPair.CertificatePem
-	a.CertificateProfile.APIServerPrivateKey = apiServerPair.PrivateKeyPem
-	a.CertificateProfile.ClientCertificate = clientPair.CertificatePem
-	a.CertificateProfile.ClientPrivateKey = clientPair.PrivateKeyPem
-	a.CertificateProfile.KubeConfigCertificate = kubeConfigPair.CertificatePem
-	a.CertificateProfile.KubeConfigPrivateKey = kubeConfigPair.PrivateKeyPem
-	a.CertificateProfile.EtcdServerCertificate = etcdServerPair.CertificatePem
-	a.CertificateProfile.EtcdServerPrivateKey = etcdServerPair.PrivateKeyPem
-	a.CertificateProfile.EtcdClientCertificate = etcdClientPair.CertificatePem
-	a.CertificateProfile.EtcdClientPrivateKey = etcdClientPair.PrivateKeyPem
-	a.CertificateProfile.EtcdPeerCertificates = make([]string, a.MasterProfile.Count)
-	a.CertificateProfile.EtcdPeerPrivateKeys = make([]string, a.MasterProfile.Count)
-	for i, v := range etcdPeerPairs {
-		a.CertificateProfile.EtcdPeerCertificates[i] = v.CertificatePem
-		a.CertificateProfile.EtcdPeerPrivateKeys[i] = v.PrivateKeyPem
+	// If no Certificate Authority pair or no cert/key pair was provided, use generated cert/key pairs signed by provided Certificate Authority pair
+	if !provided["apiserver"] || !provided["ca"] {
+		a.CertificateProfile.APIServerCertificate = apiServerPair.CertificatePem
+		a.CertificateProfile.APIServerPrivateKey = apiServerPair.PrivateKeyPem
 	}
-	return true, nil
-}
-
-func certGenerationRequired(a *api.Properties) bool {
-	if certAlreadyPresent(a.CertificateProfile) {
-		return false
+	if !provided["client"] || !provided["ca"] {
+		a.CertificateProfile.ClientCertificate = clientPair.CertificatePem
+		a.CertificateProfile.ClientPrivateKey = clientPair.PrivateKeyPem
 	}
-	if a.MasterProfile == nil {
-		return false
+	if !provided["kubeconfig"] || !provided["ca"] {
+		a.CertificateProfile.KubeConfigCertificate = kubeConfigPair.CertificatePem
+		a.CertificateProfile.KubeConfigPrivateKey = kubeConfigPair.PrivateKeyPem
+	}
+	if !provided["etcd"] || !provided["ca"] {
+		a.CertificateProfile.EtcdServerCertificate = etcdServerPair.CertificatePem
+		a.CertificateProfile.EtcdServerPrivateKey = etcdServerPair.PrivateKeyPem
+		a.CertificateProfile.EtcdClientCertificate = etcdClientPair.CertificatePem
+		a.CertificateProfile.EtcdClientPrivateKey = etcdClientPair.PrivateKeyPem
+		a.CertificateProfile.EtcdPeerCertificates = make([]string, a.MasterProfile.Count)
+		a.CertificateProfile.EtcdPeerPrivateKeys = make([]string, a.MasterProfile.Count)
+		for i, v := range etcdPeerPairs {
+			a.CertificateProfile.EtcdPeerCertificates[i] = v.CertificatePem
+			a.CertificateProfile.EtcdPeerPrivateKeys[i] = v.PrivateKeyPem
+		}
 	}
 
-	switch a.OrchestratorProfile.OrchestratorType {
-	case api.Kubernetes:
-		return true
-	default:
-		return false
+	return true, nil
+}
+
+func areAllTrue(m map[string]bool) bool {
+	for _, v := range m {
+		if !v {
+			return false
+		}
 	}
+	return true
 }
 
-// certAlreadyPresent determines if the passed in CertificateProfile includes certificate data
-// TODO actually verify valid/useable certificate data
-func certAlreadyPresent(c *api.CertificateProfile) bool {
+// certsAlreadyPresent already present returns a map where each key is a type of cert and each value is true if that cert/key pair is user-provided
+func certsAlreadyPresent(c *api.CertificateProfile, m int) map[string]bool {
+	g := map[string]bool{
+		"ca":         false,
+		"apiserver":  false,
+		"kubeconfig": false,
+		"client":     false,
+		"etcd":       false,
+	}
 	if c != nil {
-		switch {
-		case len(c.APIServerCertificate) > 0:
-			return true
-		case len(c.APIServerPrivateKey) > 0:
-			return true
-		case len(c.ClientCertificate) > 0:
-			return true
-		case len(c.ClientPrivateKey) > 0:
-			return true
-		default:
-			return false
+		etcdPeer := true
+		if len(c.EtcdPeerCertificates) != m || len(c.EtcdPeerPrivateKeys) != m {
+			etcdPeer = false
+		} else {
+			for i, p := range c.EtcdPeerCertificates {
+				if !(len(p) > 0) || !(len(c.EtcdPeerPrivateKeys[i]) > 0) {
+					etcdPeer = false
+				}
+			}
 		}
+		g["ca"] = len(c.CaCertificate) > 0 && len(c.CaPrivateKey) > 0
+		g["apiserver"] = len(c.APIServerCertificate) > 0 && len(c.APIServerPrivateKey) > 0
+		g["kubeconfig"] = len(c.KubeConfigCertificate) > 0 && len(c.KubeConfigPrivateKey) > 0
+		g["client"] = len(c.ClientCertificate) > 0 && len(c.ClientPrivateKey) > 0
+		g["etcd"] = etcdPeer && len(c.EtcdClientCertificate) > 0 && len(c.EtcdClientPrivateKey) > 0 && len(c.EtcdServerCertificate) > 0 && len(c.EtcdServerPrivateKey) > 0
 	}
-	return false
+	return g
 }
 
 // getFirstConsecutiveStaticIPAddress returns the first static IP address of the given subnet.

pkg/acsengine/defaults_test.go

@@ -1,40 +1,109 @@
 package acsengine
 
 import (
+	"reflect"
 	"testing"
 
 	"github.com/Azure/acs-engine/pkg/api"
-	. "github.com/onsi/gomega"
 )
 
-func TestCertAlreadyPresent(t *testing.T) {
-	RegisterTestingT(t)
+func TestCertsAlreadyPresent(t *testing.T) {
 	var cert *api.CertificateProfile
 
-	Expect(certAlreadyPresent(nil)).To(BeFalse())
+	result := certsAlreadyPresent(nil, 1)
+	expected := map[string]bool{
+		"ca":         false,
+		"apiserver":  false,
+		"client":     false,
+		"kubeconfig": false,
+		"etcd":       false,
+	}
 
+	if !reflect.DeepEqual(result, expected) {
+		t.Fatalf("certsAlreadyPresent() did not return false for all certs for a non-existent CertificateProfile")
+	}
 	cert = &api.CertificateProfile{}
-	Expect(certAlreadyPresent(cert)).To(BeFalse())
+	result = certsAlreadyPresent(cert, 1)
+	expected = map[string]bool{
+		"ca":         false,
+		"apiserver":  false,
+		"client":     false,
+		"kubeconfig": false,
+		"etcd":       false,
+	}
 
+	if !reflect.DeepEqual(result, expected) {
+		t.Fatalf("certsAlreadyPresent() did not return false for all certs for empty CertificateProfile")
+	}
 	cert = &api.CertificateProfile{
 		APIServerCertificate: "a",
 	}
-	Expect(certAlreadyPresent(cert)).To(BeTrue())
+	result = certsAlreadyPresent(cert, 1)
+	expected = map[string]bool{
+		"ca":         false,
+		"apiserver":  false,
+		"client":     false,
+		"kubeconfig": false,
+		"etcd":       false,
+	}
 
-	cert = &api.CertificateProfile{
-		APIServerPrivateKey: "b",
+	if !reflect.DeepEqual(result, expected) {
+		t.Fatalf("certsAlreadyPresent() did not return false for all certs for 1 cert in CertificateProfile")
 	}
-	Expect(certAlreadyPresent(cert)).To(BeTrue())
 
 	cert = &api.CertificateProfile{
-		ClientCertificate: "c",
+		APIServerCertificate:  "a",
+		CaCertificate:         "c",
+		CaPrivateKey:          "d",
+		ClientCertificate:     "e",
+		ClientPrivateKey:      "f",
+		KubeConfigCertificate: "g",
+		KubeConfigPrivateKey:  "h",
+		EtcdClientCertificate: "i",
+		EtcdClientPrivateKey:  "j",
+		EtcdServerCertificate: "k",
+		EtcdServerPrivateKey:  "l",
+	}
+	result = certsAlreadyPresent(cert, 3)
+	expected = map[string]bool{
+		"ca":         true,
+		"apiserver":  false,
+		"client":     true,
+		"kubeconfig": true,
+		"etcd":       false,
 	}
-	Expect(certAlreadyPresent(cert)).To(BeTrue())
 
+	if !reflect.DeepEqual(result, expected) {
+		t.Fatalf("certsAlreadyPresent() did not return expected result for some certs in CertificateProfile")
+	}
 	cert = &api.CertificateProfile{
-		ClientPrivateKey: "d",
+		APIServerCertificate:  "a",
+		APIServerPrivateKey:   "b",
+		CaCertificate:         "c",
+		CaPrivateKey:          "d",
+		ClientCertificate:     "e",
+		ClientPrivateKey:      "f",
+		KubeConfigCertificate: "g",
+		KubeConfigPrivateKey:  "h",
+		EtcdClientCertificate: "i",
+		EtcdClientPrivateKey:  "j",
+		EtcdServerCertificate: "k",
+		EtcdServerPrivateKey:  "l",
+		EtcdPeerCertificates:  []string{"0", "1", "2"},
+		EtcdPeerPrivateKeys:   []string{"0", "1", "2"},
+	}
+	result = certsAlreadyPresent(cert, 3)
+	expected = map[string]bool{
+		"ca":         true,
+		"apiserver":  true,
+		"client":     true,
+		"kubeconfig": true,
+		"etcd":       true,
+	}
+
+	if !reflect.DeepEqual(result, expected) {
+		t.Fatalf("certsAlreadyPresent() did not return expected result for all certs in CertificateProfile")
 	}
-	Expect(certAlreadyPresent(cert)).To(BeTrue())
 }
 
 func TestSetMissingKubeletValues(t *testing.T) {

pkg/acsengine/engine_test.go

@@ -211,6 +211,12 @@ func addTestCertificateProfile(api *api.CertificateProfile) {
 	api.ClientPrivateKey = "clientPrivateKey"
 	api.KubeConfigCertificate = "kubeConfigCertificate"
 	api.KubeConfigPrivateKey = "kubeConfigPrivateKey"
+	api.EtcdClientCertificate = "etcdClientCertificate"
+	api.EtcdClientPrivateKey = "etcdClientPrivateKey"
+	api.EtcdServerCertificate = "etcdServerCertificate"
+	api.EtcdServerPrivateKey = "etcdServerPrivateKey"
+	api.EtcdPeerCertificates = []string{"etcdPeerCertificate0"}
+	api.EtcdPeerPrivateKeys = []string{"etcdPeerPrivateKey0"}
 }
 
 func TestGetStorageAccountType(t *testing.T) {

pkg/acsengine/testdata/disks-managed/kubernetes-vmas.json

@@ -57,12 +57,23 @@
     },
     "certificateProfile": {
       "caCertificate": "caCertificate",
+      "caPrivateKey": "caPrivateKey",
       "apiServerCertificate": "apiServerCertificate",
       "apiServerPrivateKey": "apiServerPrivateKey",
       "clientCertificate": "clientCertificate",
       "clientPrivateKey": "clientPrivateKey",
       "kubeConfigCertificate": "kubeConfigCertificate",
-      "kubeConfigPrivateKey": "kubeConfigPrivateKey"
+      "kubeConfigPrivateKey": "kubeConfigPrivateKey",
+      "etcdClientCertificate": "etcdClientCertificate",
+      "etcdClientPrivateKey": "etcdClientPrivateKey",
+      "etcdServerCertificate": "etcdServerCertificate",
+      "etcdServerPrivateKey": "etcdServerPrivateKey",
+      "etcdPeerCertificates": [
+        "etcdPeerCertificate0"
+      ],
+      "etcdPeerPrivateKeys": [
+        "etcdPeerPrivateKey0"
+      ]
     }
   }
-}
+}

pkg/acsengine/testdata/disks-storageaccount/kubernetes.json

@@ -43,12 +43,23 @@
     },
     "certificateProfile": {
       "caCertificate": "caCertificate",
+      "caPrivateKey": "caPrivateKey",      
       "apiServerCertificate": "apiServerCertificate",
       "apiServerPrivateKey": "apiServerPrivateKey",
       "clientCertificate": "clientCertificate",
       "clientPrivateKey": "clientPrivateKey",
       "kubeConfigCertificate": "kubeConfigCertificate",
-      "kubeConfigPrivateKey": "kubeConfigPrivateKey"
+      "kubeConfigPrivateKey": "kubeConfigPrivateKey",
+      "etcdClientCertificate": "etcdClientCertificate",
+      "etcdClientPrivateKey": "etcdClientPrivateKey",
+      "etcdServerCertificate": "etcdServerCertificate",
+      "etcdServerPrivateKey": "etcdServerPrivateKey",
+      "etcdPeerCertificates": [
+        "etcdPeerCertificate0"
+      ],
+      "etcdPeerPrivateKeys": [
+        "etcdPeerPrivateKey0"
+      ]
     }
   }
 }

pkg/acsengine/testdata/etcd-versions/kubernetes.json

@@ -42,12 +42,23 @@
     },
     "certificateProfile": {
       "caCertificate": "caCertificate",
+      "caPrivateKey": "caPrivateKey",
       "apiServerCertificate": "apiServerCertificate",
       "apiServerPrivateKey": "apiServerPrivateKey",
       "clientCertificate": "clientCertificate",
       "clientPrivateKey": "clientPrivateKey",
       "kubeConfigCertificate": "kubeConfigCertificate",
-      "kubeConfigPrivateKey": "kubeConfigPrivateKey"
+      "kubeConfigPrivateKey": "kubeConfigPrivateKey",
+      "etcdClientCertificate": "etcdClientCertificate",
+      "etcdClientPrivateKey": "etcdClientPrivateKey",
+      "etcdServerCertificate": "etcdServerCertificate",
+      "etcdServerPrivateKey": "etcdServerPrivateKey",
+      "etcdPeerCertificates": [
+        "etcdPeerCertificate0"
+      ],
+      "etcdPeerPrivateKeys": [
+        "etcdPeerPrivateKey0"
+      ]
     }
   }
 }

pkg/acsengine/testdata/extensions/kubernetes.json

@@ -58,12 +58,23 @@
     },
     "certificateProfile": {
       "caCertificate": "caCertificate",
+      "caPrivateKey": "caPrivateKey",
       "apiServerCertificate": "apiServerCertificate",
       "apiServerPrivateKey": "apiServerPrivateKey",
       "clientCertificate": "clientCertificate",
       "clientPrivateKey": "clientPrivateKey",
       "kubeConfigCertificate": "kubeConfigCertificate",
-      "kubeConfigPrivateKey": "kubeConfigPrivateKey"
+      "kubeConfigPrivateKey": "kubeConfigPrivateKey",
+      "etcdClientCertificate": "etcdClientCertificate",
+      "etcdClientPrivateKey": "etcdClientPrivateKey",
+      "etcdServerCertificate": "etcdServerCertificate",
+      "etcdServerPrivateKey": "etcdServerPrivateKey",
+      "etcdPeerCertificates": [
+        "etcdPeerCertificate0"
+      ],
+      "etcdPeerPrivateKeys": [
+        "etcdPeerPrivateKey0"
+      ]
     }
   }
 }