From c58b8f9687dd6ea8b245fa8901bc0ab3cc99d439 Mon Sep 17 00:00:00 2001
From: Antonio Murdaca <runcom@linux.com>
Date: Wed, 29 Jan 2020 23:11:35 +0100
Subject: [PATCH] pkg/server: serve config only to master in bootstrap server

The new cluster etcd operator flow is:

1) start bootstrap mcs
2) start etcd on bootstrap
3) wait for bootstrapping to finish i.e. atleast one control-plane is ready and there is MCS running on cluster
4) turn down bootstrap mcs

What the above does is giving a chance to workers to grab
the ignition config from the bootstap server which now stays up longer.
However, by the time they attempt to create a CSR the kube-apiserver has
rotated that bootstrap chain of trust out which causes the workers to error out with:

Jan 29 19:55:20 ip-10-0-130-205 hyperkube[2623]: E0129 19:55:20.869251    2623 certificate_manager.go:421] Failed while requesting a signed certificate from the master: cannot create certificate signing request: Unauthorized

The above results in workers not being able to join the cluster eventually.

What this patch does is denying serving the configuration to all pools but master
within the bootstrap server, effectively delaying workers to grab the wrong config
from the wrong server. Workers will keep polling for configuration and they'll
eventually grab the correct one from the server running within the new cluster.

Signed-off-by: Antonio Murdaca <runcom@linux.com>
---
 pkg/server/bootstrap_server.go                |  4 +++-
 pkg/server/server_test.go                     | 10 +++++++-
 pkg/server/testdata/machine-pools/master.yaml | 24 +++++++++++++++++++
 3 files changed, 36 insertions(+), 2 deletions(-)
 create mode 100644 pkg/server/testdata/machine-pools/master.yaml

diff --git a/pkg/server/bootstrap_server.go b/pkg/server/bootstrap_server.go
index 16db9d09f9..82559cb580 100644
--- a/pkg/server/bootstrap_server.go
+++ b/pkg/server/bootstrap_server.go
@@ -56,7 +56,9 @@ func NewBootstrapServer(dir, kubeconfig string) (Server, error) {
 // 4. Append the machine annotations file.
 // 5. Append the KubeConfig file.
 func (bsc *bootstrapServer) GetConfig(cr poolRequest) (*igntypes.Config, error) {
-
+	if cr.machineConfigPool != "master" {
+		return nil, fmt.Errorf("refusing to serve bootstrap configuration to pool %q", cr.machineConfigPool)
+	}
 	// 1. Read the Machine Config Pool object.
 	fileName := path.Join(bsc.serverBaseDir, "machine-pools", cr.machineConfigPool+".yaml")
 	glog.Infof("reading file %q", fileName)
diff --git a/pkg/server/server_test.go b/pkg/server/server_test.go
index 22ae2612e8..27f84ae71b 100644
--- a/pkg/server/server_test.go
+++ b/pkg/server/server_test.go
@@ -120,7 +120,7 @@ func TestBootstrapServer(t *testing.T) {
 		t.Fatal(err)
 	}
 	res, err := bs.GetConfig(poolRequest{
-		machineConfigPool: testPool,
+		machineConfigPool: "master",
 	})
 	if err != nil {
 		t.Fatalf("expected err to be nil, received: %v", err)
@@ -129,6 +129,14 @@ func TestBootstrapServer(t *testing.T) {
 	// assert on the output.
 	validateIgnitionFiles(t, mc.Spec.Config.Storage.Files, res.Storage.Files)
 	validateIgnitionSystemd(t, mc.Spec.Config.Systemd.Units, res.Systemd.Units)
+
+	// verify bootstrap cannot serve ignition to other pool than master
+	res, err = bs.GetConfig(poolRequest{
+		machineConfigPool: testPool,
+	})
+	if err == nil {
+		t.Fatalf("expected bootstrap server to not serve ignition to non-master pools")
+	}
 }
 
 // TestClusterServer tests the behavior of the machine config server
diff --git a/pkg/server/testdata/machine-pools/master.yaml b/pkg/server/testdata/machine-pools/master.yaml
new file mode 100644
index 0000000000..45bdeb645a
--- /dev/null
+++ b/pkg/server/testdata/machine-pools/master.yaml
@@ -0,0 +1,24 @@
+apiVersion: machineconfiguration.openshift.io/v1
+kind: MachineConfigPool
+metadata:
+  creationTimestamp: null
+  name: master
+spec:
+  configuration:
+    name: test-config
+  machineConfigSelector:
+    matchLabels:
+      machineconfiguration.openshift.io/role: test
+  nodeSelector:
+    matchLabels:
+      node-role.kubernetes.io/test: ""
+  maxUnavailable: null
+  paused: false
+status:
+  conditions: null
+  configuration:
+    name: test-config
+  machineCount: 0
+  readyMachineCount: 0
+  unavailableMachineCount: 0
+  updatedMachineCount: 0