From 2413713c4ea13fb5a66c0597beaa1c3dc2aeb81f Mon Sep 17 00:00:00 2001
From: "Lubomir I. Ivanov" <lubomirivanov@vmware.com>
Date: Fri, 31 May 2019 04:00:41 +0300
Subject: [PATCH] kubeadm: apply deterministic order on certificate phases

The existing logic already creates a proper "tree"
where a CA is always generated before the certs that are signed
by this CA, however the tree is not deterministic.

Always use the default list of certs when generating the
"kubeadm init phase certs" phases. Add a unit test that
makes sure that CA always precede signed certs in the default
lists.

This solves the problem where the help screen for "kubeadm
init" cert sub-phases can have a random order.
---
 cmd/kubeadm/app/cmd/phases/init/certs.go      | 22 ++++++------
 cmd/kubeadm/app/phases/certs/certlist_test.go | 34 +++++++++++++++++++
 2 files changed, 46 insertions(+), 10 deletions(-)

diff --git a/cmd/kubeadm/app/cmd/phases/init/certs.go b/cmd/kubeadm/app/cmd/phases/init/certs.go
index 19f99d69c29..7f2c0951d10 100644
--- a/cmd/kubeadm/app/cmd/phases/init/certs.go
+++ b/cmd/kubeadm/app/cmd/phases/init/certs.go
@@ -86,17 +86,19 @@ func newCertSubPhases() []workflow.Phase {
 
 	subPhases = append(subPhases, allPhase)
 
-	certTree, _ := certsphase.GetDefaultCertList().AsMap().CertTree()
-
-	for ca, certList := range certTree {
-		caPhase := newCertSubPhase(ca, runCAPhase(ca))
-		subPhases = append(subPhases, caPhase)
-
-		for _, cert := range certList {
-			certPhase := newCertSubPhase(cert, runCertPhase(cert, ca))
-			certPhase.LocalFlags = localFlags()
-			subPhases = append(subPhases, certPhase)
+	// This loop assumes that GetDefaultCertList() always returns a list of
+	// certificate that is preceded by the CAs that sign them.
+	var lastCACert *certsphase.KubeadmCert
+	for _, cert := range certsphase.GetDefaultCertList() {
+		var phase workflow.Phase
+		if cert.CAName == "" {
+			phase = newCertSubPhase(cert, runCAPhase(cert))
+			lastCACert = cert
+		} else {
+			phase = newCertSubPhase(cert, runCertPhase(cert, lastCACert))
+			phase.LocalFlags = localFlags()
 		}
+		subPhases = append(subPhases, phase)
 	}
 
 	// SA creates the private/public key pair, which doesn't use x509 at all
diff --git a/cmd/kubeadm/app/phases/certs/certlist_test.go b/cmd/kubeadm/app/phases/certs/certlist_test.go
index 2392dc271b3..345c589cb03 100644
--- a/cmd/kubeadm/app/phases/certs/certlist_test.go
+++ b/cmd/kubeadm/app/phases/certs/certlist_test.go
@@ -29,6 +29,40 @@ import (
 	kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm"
 )
 
+func TestCertListOrder(t *testing.T) {
+	tests := []struct {
+		certs Certificates
+		name  string
+	}{
+		{
+			name:  "Default Certificate List",
+			certs: GetDefaultCertList(),
+		},
+		{
+			name:  "Cert list less etcd",
+			certs: GetCertsWithoutEtcd(),
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			var lastCA *KubeadmCert
+			for i, cert := range test.certs {
+				if i > 0 && lastCA == nil {
+					t.Fatalf("CA not present in list before certificate %q", cert.Name)
+				}
+				if cert.CAName == "" {
+					lastCA = cert
+				} else {
+					if cert.CAName != lastCA.Name {
+						t.Fatalf("expected CA name %q, got %q, for certificate %q", lastCA.Name, cert.CAName, cert.Name)
+					}
+				}
+			}
+		})
+	}
+}
+
 func TestCAPointersValid(t *testing.T) {
 	tests := []struct {
 		certs Certificates