Merge pull request #9356 from johngmyers/refactor-kubelet-api

Issue kubelet-api cert in nodeup

nodeup/pkg/model/etcd_manager_tls.go

@@ -81,7 +81,5 @@ func (b *EtcdManagerTLSBuilder) buildKubeAPIServerKeypair(c *fi.ModelBuilderCont
 		},
 	}
 	c.AddTask(issueCert)
-	issueCert.AddFileTasks(c, "/etc/kubernetes/pki/kube-apiserver", name, "etcd-ca", nil)
-
-	return nil
+	return issueCert.AddFileTasks(c, "/etc/kubernetes/pki/kube-apiserver", name, "etcd-ca", nil)
 }

nodeup/pkg/model/kube_apiserver.go

@@ -109,11 +109,15 @@ func (b *KubeAPIServerBuilder) Build(c *fi.ModelBuilderContext) error {
 
 	// @check if we are using secure client certificates for kubelet and grab the certificates
 	if b.UseSecureKubelet() {
-		name := "kubelet-api"
-		if err := b.BuildCertificateTask(c, name, name+".pem"); err != nil {
-			return err
+		issueCert := &nodetasks.IssueCert{
+			Name:    "kubelet-api",
+			Signer:  fi.CertificateIDCA,
+			Type:    "client",
+			Subject: nodetasks.PKIXName{CommonName: "kubelet-api"},
 		}
-		if err := b.BuildPrivateKeyTask(c, name, name+"-key.pem"); err != nil {
+		c.AddTask(issueCert)
+		err := issueCert.AddFileTasks(c, b.PathSrvKubernetes(), "kubelet-api", "", nil)
+		if err != nil {
 			return err
 		}
 	}
@@ -331,8 +335,8 @@ func (b *KubeAPIServerBuilder) buildPod() (*v1.Pod, error) {
 	// @check if we are using secure kubelet client certificates
 	if b.UseSecureKubelet() {
 		// @note we are making assumption were using the ones created by the pki model, not custom defined ones
-		kubeAPIServer.KubeletClientCertificate = filepath.Join(b.PathSrvKubernetes(), "kubelet-api.pem")
-		kubeAPIServer.KubeletClientKey = filepath.Join(b.PathSrvKubernetes(), "kubelet-api-key.pem")
+		kubeAPIServer.KubeletClientCertificate = filepath.Join(b.PathSrvKubernetes(), "kubelet-api.crt")
+		kubeAPIServer.KubeletClientKey = filepath.Join(b.PathSrvKubernetes(), "kubelet-api.key")
 	}
 
 	{

nodeup/pkg/model/kube_apiserver_healthcheck.go

@@ -90,7 +90,5 @@ func (b *KubeAPIServerBuilder) addHealthcheckSidecarTasks(c *fi.ModelBuilderCont
 		},
 	}
 	c.AddTask(issueCert)
-	issueCert.AddFileTasks(c, secretsDir, "client", "ca", s(userName))
-
-	return nil
+	return issueCert.AddFileTasks(c, secretsDir, "client", "ca", s(userName))
 }

nodeup/pkg/model/networking/cilium.go

@@ -132,7 +132,5 @@ func (b *CiliumBuilder) buildCiliumEtcdSecrets(c *fi.ModelBuilderContext) error
 		},
 	}
 	c.AddTask(issueCert)
-	issueCert.AddFileTasks(c, "/etc/kubernetes/pki/cilium", name, "etcd-ca", nil)
-
-	return nil
+	return issueCert.AddFileTasks(c, "/etc/kubernetes/pki/cilium", name, "etcd-ca", nil)
 }

nodeup/pkg/model/secrets.go

@@ -79,16 +79,6 @@ func (b *SecretBuilder) Build(c *fi.ModelBuilderContext) error {
 		}
 	}
 
-	{
-		// TODO: Remove - we use the apiserver-aggregator keypair instead (which is signed by a different CA)
-		if err := b.BuildCertificateTask(c, "apiserver-proxy-client", "proxy-client.cert"); err != nil {
-			return err
-		}
-		if err := b.BuildPrivateKeyTask(c, "apiserver-proxy-client", "proxy-client.key"); err != nil {
-			return err
-		}
-	}
-
 	{
 		if err := b.BuildCertificateTask(c, "apiserver-aggregator", "apiserver-aggregator.cert"); err != nil {
 			return err

nodeup/pkg/model/tests/golden/minimal/cluster.yaml

@@ -27,6 +27,8 @@ spec:
     provider: Manager
     backups:
       backupStore: memfs://clusters.example.com/minimal.example.com/backups/etcd-events
+  kubelet:
+    anonymousAuth: false
   kubernetesVersion: v1.18.0
   masterInternalName: api.internal.minimal.example.com
   masterPublicName: api.minimal.example.com

nodeup/pkg/model/tests/golden/minimal/tasks-kube-apiserver.yaml

@@ -29,6 +29,8 @@ contents: |
       - --etcd-servers=https://127.0.0.1:4001
       - --insecure-bind-address=127.0.0.1
       - --insecure-port=0
+      - --kubelet-client-certificate=/srv/kubernetes/kubelet-api.crt
+      - --kubelet-client-key=/srv/kubernetes/kubelet-api.key
       - --kubelet-preferred-address-types=InternalIP,Hostname,ExternalIP
       - --proxy-client-cert-file=/srv/kubernetes/apiserver-aggregator.cert
       - --proxy-client-key-file=/srv/kubernetes/apiserver-aggregator.key
@@ -153,8 +155,40 @@ contents: |
 path: /etc/kubernetes/manifests/kube-apiserver.manifest
 type: file
 ---
+mode: "0755"
+path: /srv/kubernetes
+type: directory
+---
+contents:
+  task:
+    Name: kubelet-api
+    signer: ca
+    subject:
+      CommonName: kubelet-api
+    type: client
+mode: "0644"
+path: /srv/kubernetes/kubelet-api.crt
+type: file
+---
+contents:
+  task:
+    Name: kubelet-api
+    signer: ca
+    subject:
+      CommonName: kubelet-api
+    type: client
+mode: "0600"
+path: /srv/kubernetes/kubelet-api.key
+type: file
+---
 contents: ""
 ifNotExists: true
 mode: "0400"
 path: /var/log/kube-apiserver.log
 type: file
+---
+Name: kubelet-api
+signer: ca
+subject:
+  CommonName: kubelet-api
+type: client

pkg/bundle/builder.go

@@ -239,37 +239,18 @@ func (b *Builder) buildPKIFiles(cluster *kops.Cluster, ig *kops.InstanceGroup, k
 	keys = append(keys, "kube-proxy")
 
 	if ig.IsMaster() {
-		// Used by e.g. protokube
-		certs = append(certs, "kops")
-		keys = append(keys, "kops")
-
 		// Used by apiserver-aggregator
 		certs = append(certs, "apiserver-aggregator")
 		keys = append(keys, "apiserver-aggregator")
 		certs = append(certs, "apiserver-aggregator-ca")
 
-		certs = append(certs, "apiserver-proxy-client")
-		keys = append(keys, "apiserver-proxy-client")
-
 		// Used by k-c-m, for example
 		//certs = append(certs, "ca")
 		keys = append(keys, "ca")
 
-		// Used by kube-controller-manager to auth to API
-		certs = append(certs, "kube-controller-manager")
-		keys = append(keys, "kube-controller-manager")
-
-		// Used by kube-scheduler to auth to API
-		certs = append(certs, "kube-scheduler")
-		keys = append(keys, "kube-scheduler")
-
 		// key for the apiserver
 		certs = append(certs, "master")
 		keys = append(keys, "master")
-
-		// We store kubecfg on the master
-		certs = append(certs, "kubecfg")
-		keys = append(keys, "kubecfg")
 	}
 
 	for _, name := range certs {

pkg/model/pki.go

@@ -60,18 +60,6 @@ func (b *PKIModelBuilder) Build(c *fi.ModelBuilderContext) error {
 			})
 		}
 	}
-	{
-		// Generate a kubelet client certificate for api to speak securely to kubelets. This change was first
-		// introduced in https://github.com/kubernetes/kops/pull/2831 where server.cert/key were used. With kubernetes >= 1.7
-		// the certificate usage is being checked (obviously the above was server not client certificate) and so now fails
-		c.AddTask(&fitasks.Keypair{
-			Name:      fi.String("kubelet-api"),
-			Lifecycle: b.Lifecycle,
-			Subject:   "cn=kubelet-api",
-			Type:      "client",
-			Signer:    defaultCA,
-		})
-	}
 
 	{
 		t := &fitasks.Keypair{
@@ -173,17 +161,6 @@ func (b *PKIModelBuilder) Build(c *fi.ModelBuilderContext) error {
 		c.AddTask(t)
 	}
 
-	{
-		t := &fitasks.Keypair{
-			Name:      fi.String("apiserver-proxy-client"),
-			Lifecycle: b.Lifecycle,
-			Subject:   "cn=apiserver-proxy-client",
-			Type:      "client",
-			Signer:    defaultCA,
-		}
-		c.AddTask(t)
-	}
-
 	{
 		aggregatorCA := &fitasks.Keypair{
 			Name:      fi.String("apiserver-aggregator-ca"),

upup/pkg/fi/nodeup/nodetasks/issue_cert.go

@@ -76,13 +76,16 @@ func (i *IssueCert) GetResources() (certResource, keyResource, caResource *fi.Ta
 	return i.cert, i.key, i.ca
 }
 
-func (i *IssueCert) AddFileTasks(c *fi.ModelBuilderContext, dir string, name string, caName string, owner *string) {
+func (i *IssueCert) AddFileTasks(c *fi.ModelBuilderContext, dir string, name string, caName string, owner *string) error {
 	certResource, keyResource, caResource := i.GetResources()
-	c.AddTask(&File{
+	err := c.EnsureTask(&File{
 		Path: dir,
 		Type: FileType_Directory,
 		Mode: fi.String("0755"),
 	})
+	if err != nil {
+		return err
+	}
 
 	c.AddTask(&File{
 		Path:     filepath.Join(dir, name+".crt"),
@@ -101,14 +104,19 @@ func (i *IssueCert) AddFileTasks(c *fi.ModelBuilderContext, dir string, name str
 	})
 
 	if caName != "" {
-		c.AddTask(&File{
+		err = c.EnsureTask(&File{
 			Path:     filepath.Join(dir, caName+".crt"),
 			Contents: caResource,
 			Type:     FileType_File,
 			Mode:     fi.String("0644"),
 			Owner:    owner,
 		})
+		if err != nil {
+			return nil
+		}
 	}
+
+	return nil
 }
 
 func (e *IssueCert) Run(c *fi.Context) error {

upup/pkg/fi/nodeup/nodetasks/issue_cert_test.go

@@ -30,7 +30,8 @@ func TestIssueCertFileDependencies(t *testing.T) {
 
 	issue := &IssueCert{Name: "testCert"}
 	context.AddTask(issue)
-	issue.AddFileTasks(context, "/tmp", "testCert", "testCa", nil)
+	err := issue.AddFileTasks(context, "/tmp", "testCert", "testCa", nil)
+	assert.NoError(t, err)
 	var taskNames []string
 	for name := range context.Tasks {
 		taskNames = append(taskNames, name)