Merge pull request #41371 from jcbsmpsn/split-request-node-certificate

Automatic merge from submit-queue (batch tested with PRs 41649, 41658, 41266, 41371, 41626)

Split request node certificate

Split the `RequestNodeCertificate` function so the behavior can also be used by non-node callers.

pkg/kubelet/util/csr/BUILD

@@ -5,6 +5,7 @@ licenses(["notice"])
 load(
     "@io_bazel_rules_go//go:def.bzl",
     "go_library",
+    "go_test",
 )
 
 go_library(
@@ -34,3 +35,17 @@ filegroup(
     srcs = [":package-srcs"],
     tags = ["automanaged"],
 )
+
+go_test(
+    name = "go_default_test",
+    srcs = ["csr_test.go"],
+    library = ":go_default_library",
+    tags = ["automanaged"],
+    deps = [
+        "//vendor:k8s.io/apimachinery/pkg/apis/meta/v1",
+        "//vendor:k8s.io/apimachinery/pkg/watch",
+        "//vendor:k8s.io/client-go/kubernetes/typed/certificates/v1beta1",
+        "//vendor:k8s.io/client-go/pkg/apis/certificates/v1beta1",
+        "//vendor:k8s.io/client-go/util/cert",
+    ],
+)

pkg/kubelet/util/csr/csr.go

@@ -29,10 +29,13 @@ import (
 	certutil "k8s.io/client-go/util/cert"
 )
 
-// RequestNodeCertificate will create a certificate signing request and send it to API server,
-// then it will watch the object's status, once approved by API server, it will return the API
-// server's issued certificate (pem-encoded). If there is any errors, or the watch timeouts,
-// it will return an error. This is intended for use on nodes (kubelet and kubeadm).
+// RequestNodeCertificate will create a certificate signing request for a node
+// (Organization and CommonName for the CSR will be set as expected for node
+// certificates) and send it to API server, then it will watch the object's
+// status, once approved by API server, it will return the API server's issued
+// certificate (pem-encoded). If there is any errors, or the watch timeouts, it
+// will return an error. This is intended for use on nodes (kubelet and
+// kubeadm).
 func RequestNodeCertificate(client certificatesclient.CertificateSigningRequestInterface, privateKeyData []byte, nodeName types.NodeName) (certData []byte, err error) {
 	subject := &pkix.Name{
 		Organization: []string{"system:nodes"},
@@ -43,23 +46,31 @@ func RequestNodeCertificate(client certificatesclient.CertificateSigningRequestI
 	if err != nil {
 		return nil, fmt.Errorf("invalid private key for certificate request: %v", err)
 	}
-	csr, err := certutil.MakeCSR(privateKey, subject, nil, nil)
+	csrData, err := certutil.MakeCSR(privateKey, subject, nil, nil)
 	if err != nil {
 		return nil, fmt.Errorf("unable to generate certificate request: %v", err)
 	}
+	return RequestCertificate(client, csrData, []certificates.KeyUsage{
+		certificates.UsageDigitalSignature,
+		certificates.UsageKeyEncipherment,
+		certificates.UsageClientAuth,
+	})
+}
 
+// RequestCertificate will create a certificate signing request using the PEM
+// encoded CSR and send it to API server, then it will watch the object's
+// status, once approved by API server, it will return the API server's issued
+// certificate (pem-encoded). If there is any errors, or the watch timeouts, it
+// will return an error.
+func RequestCertificate(client certificatesclient.CertificateSigningRequestInterface, csrData []byte, usages []certificates.KeyUsage) (certData []byte, err error) {
 	req, err := client.Create(&certificates.CertificateSigningRequest{
 		// Username, UID, Groups will be injected by API server.
 		TypeMeta:   metav1.TypeMeta{Kind: "CertificateSigningRequest"},
 		ObjectMeta: metav1.ObjectMeta{GenerateName: "csr-"},
 
 		Spec: certificates.CertificateSigningRequestSpec{
-			Request: csr,
-			Usages: []certificates.KeyUsage{
-				certificates.UsageDigitalSignature,
-				certificates.UsageKeyEncipherment,
-				certificates.UsageClientAuth,
-			},
+			Request: csrData,
+			Usages:  usages,
 		},
 	})
 	if err != nil {

pkg/kubelet/util/csr/csr_test.go

@@ -0,0 +1,132 @@
+/*
+Copyright 2017 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package csr
+
+import (
+	"fmt"
+	"testing"
+
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	watch "k8s.io/apimachinery/pkg/watch"
+	certificatesclient "k8s.io/client-go/kubernetes/typed/certificates/v1beta1"
+	certificates "k8s.io/client-go/pkg/apis/certificates/v1beta1"
+	certutil "k8s.io/client-go/util/cert"
+)
+
+func TestRequestNodeCertificateNoKeyData(t *testing.T) {
+	certData, err := RequestNodeCertificate(&fakeClient{}, []byte{}, "fake-node-name")
+	if err == nil {
+		t.Errorf("Got no error, wanted error an error because there was an empty private key passed in.")
+	}
+	if certData != nil {
+		t.Errorf("Got cert data, wanted nothing as there should have been an error.")
+	}
+}
+
+func TestRequestNodeCertificateErrorCreatingCSR(t *testing.T) {
+	client := &fakeClient{
+		failureType: createError,
+	}
+	privateKeyData, err := certutil.MakeEllipticPrivateKeyPEM()
+	if err != nil {
+		t.Fatalf("Unable to generate a new private key: %v", err)
+	}
+
+	certData, err := RequestNodeCertificate(client, privateKeyData, "fake-node-name")
+	if err == nil {
+		t.Errorf("Got no error, wanted error an error because client.Create failed.")
+	}
+	if certData != nil {
+		t.Errorf("Got cert data, wanted nothing as there should have been an error.")
+	}
+}
+
+func TestRequestNodeCertificate(t *testing.T) {
+	privateKeyData, err := certutil.MakeEllipticPrivateKeyPEM()
+	if err != nil {
+		t.Fatalf("Unable to generate a new private key: %v", err)
+	}
+
+	certData, err := RequestNodeCertificate(&fakeClient{}, privateKeyData, "fake-node-name")
+	if err != nil {
+		t.Errorf("Got %v, wanted no error.", err)
+	}
+	if certData == nil {
+		t.Errorf("Got nothing, expected a CSR.")
+	}
+}
+
+type FailureType int
+
+const (
+	noError FailureType = iota
+	createError
+	certificateSigningRequestDenied
+)
+
+type fakeClient struct {
+	certificatesclient.CertificateSigningRequestInterface
+	watch       *watch.FakeWatcher
+	failureType FailureType
+}
+
+func (c *fakeClient) Create(*certificates.CertificateSigningRequest) (*certificates.CertificateSigningRequest, error) {
+	if c.failureType == createError {
+		return nil, fmt.Errorf("fakeClient failed creating request")
+	}
+	csr := certificates.CertificateSigningRequest{
+		ObjectMeta: metav1.ObjectMeta{
+			UID:  "fake-uid",
+			Name: "fake-certificate-signing-request-name",
+		},
+	}
+	return &csr, nil
+}
+
+func (c *fakeClient) Watch(opts v1.ListOptions) (watch.Interface, error) {
+	c.watch = watch.NewFakeWithChanSize(1, false)
+	c.watch.Add(c.generateCSR())
+	c.watch.Stop()
+	return c.watch, nil
+}
+
+func (c *fakeClient) generateCSR() *certificates.CertificateSigningRequest {
+	var condition certificates.CertificateSigningRequestCondition
+	if c.failureType == certificateSigningRequestDenied {
+		condition = certificates.CertificateSigningRequestCondition{
+			Type: certificates.CertificateDenied,
+		}
+	} else {
+		condition = certificates.CertificateSigningRequestCondition{
+			Type: certificates.CertificateApproved,
+		}
+	}
+
+	csr := certificates.CertificateSigningRequest{
+		ObjectMeta: metav1.ObjectMeta{
+			UID: "fake-uid",
+		},
+		Status: certificates.CertificateSigningRequestStatus{
+			Conditions: []certificates.CertificateSigningRequestCondition{
+				condition,
+			},
+			Certificate: []byte{},
+		},
+	}
+	return &csr
+}