address comments

Author: Mengqi Yu

pkg/webhook/certprovisioner/certprovisioner.go

@@ -16,8 +16,16 @@ limitations under the License.
 
 package certprovisioner
 
+// Certs hosts a private key, its corresponding serving certificate and
+// the CA certificate that signs the serving certificate.
+type Certs struct {
+	Key    []byte
+	Cert   []byte
+	CACert []byte
+}
+
 // CertProvisioner is an interface to provision the serving certificate.
 type CertProvisioner interface {
-	// ProvisionServingCert returns the key, serving certificate and the CA certificate.
-	ProvisionServingCert() (key []byte, cert []byte, caCert []byte, err error)
+	// ProvisionServingCert returns a Certs struct.
+	ProvisionServingCert() (*Certs, error)
 }

pkg/webhook/certprovisioner/certprovisioner_test.go

@@ -0,0 +1,24 @@
+/*
+Copyright 2018 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 certprovisioner
+
+import "fmt"
+
+func ExampleServiceToCommonName() {
+	fmt.Println(ServiceToCommonName("myservicenamespace", "myservicename"))
+	// Output: myservicename.myservicenamespace.svc
+}

pkg/webhook/certprovisioner/doc.go

@@ -24,7 +24,7 @@ Create a implementation instance of certprovisioner.
 	}
 
 Provision the certificates.
-	key, cert, caCert, err := cp.ProvisionServingCert()
+	certs, err := cp.ProvisionServingCert()
 	if err != nil {
 		// handle error
 	}

pkg/webhook/certprovisioner/selfsignedcertprovisioner.go

@@ -23,6 +23,11 @@ import (
 	"k8s.io/client-go/util/cert"
 )
 
+// ServiceToCommonName generates the CommonName for the certificate when using a k8s service.
+func ServiceToCommonName(serviceNamespace, serviceName string) string {
+	return fmt.Sprintf("%s.%s.svc", serviceName, serviceNamespace)
+}
+
 // SelfSignedCertProvisioner implements the CertProvisioner interface.
 // It provisions self-signed certificates.
 type SelfSignedCertProvisioner struct {
@@ -32,26 +37,23 @@ type SelfSignedCertProvisioner struct {
 
 var _ CertProvisioner = &SelfSignedCertProvisioner{}
 
-// ProvisionServingCert creates and returns a CA certificate and certificate and
+// ProvisionServingCert creates and returns a CA certificate, certificate and
 // key for the server. serverKey and serverCert are used by the server
 // to establish trust for clients, CA certificate is used by the
 // client to verify the server authentication chain.
 // The cert will be valid for 365 days.
-func (cp *SelfSignedCertProvisioner) ProvisionServingCert() (serverKey, serverCert, caCert []byte, err error) {
+func (cp *SelfSignedCertProvisioner) ProvisionServingCert() (*Certs, error) {
 	signingKey, err := cert.NewPrivateKey()
 	if err != nil {
-		return nil, nil, nil,
-			fmt.Errorf("failed to create the CA private key: %v", err)
+		return nil, fmt.Errorf("failed to create the CA private key: %v", err)
 	}
 	signingCert, err := cert.NewSelfSignedCACert(cert.Config{CommonName: "webhook-cert-ca"}, signingKey)
 	if err != nil {
-		return nil, nil, nil,
-			fmt.Errorf("failed to create the CA cert: %v", err)
+		return nil, fmt.Errorf("failed to create the CA cert: %v", err)
 	}
 	key, err := cert.NewPrivateKey()
 	if err != nil {
-		return nil, nil, nil,
-			fmt.Errorf("failed to create the private key: %v", err)
+		return nil, fmt.Errorf("failed to create the private key: %v", err)
 	}
 	signedCert, err := cert.NewSignedCert(
 		cert.Config{
@@ -61,8 +63,11 @@ func (cp *SelfSignedCertProvisioner) ProvisionServingCert() (serverKey, serverCe
 		key, signingCert, signingKey,
 	)
 	if err != nil {
-		return nil, nil, nil,
-			fmt.Errorf("failed to create the cert: %v", err)
+		return nil, fmt.Errorf("failed to create the cert: %v", err)
 	}
-	return cert.EncodePrivateKeyPEM(key), cert.EncodeCertPEM(signedCert), cert.EncodeCertPEM(signingCert), nil
+	return &Certs{
+		Key:    cert.EncodePrivateKeyPEM(key),
+		Cert:   cert.EncodeCertPEM(signedCert),
+		CACert: cert.EncodeCertPEM(signingCert),
+	}, nil
 }

pkg/webhook/certprovisioner/selfsignedcertprovisioner_test.go

@@ -23,30 +23,30 @@ import (
 )
 
 func TestProvisionServingCert(t *testing.T) {
-	CN := "mysvc.myns.svc"
-	cp := SelfSignedCertProvisioner{CommonName: CN}
-	_, certPEM, caPEM, err := cp.ProvisionServingCert()
+	cn := "mysvc.myns.svc"
+	cp := SelfSignedCertProvisioner{CommonName: cn}
+	certs, err := cp.ProvisionServingCert()
 
 	// First, create the set of root certificates. For this example we only
 	// have one. It's also possible to omit this in order to use the
 	// default root set of the current operating system.
 	roots := x509.NewCertPool()
-	ok := roots.AppendCertsFromPEM([]byte(caPEM))
+	ok := roots.AppendCertsFromPEM(certs.CACert)
 	if !ok {
-		t.Fatalf("failed to parse root certificate: %s", caPEM)
+		t.Fatalf("failed to parse root certificate: %s", certs.CACert)
 	}
 
-	block, _ := pem.Decode(certPEM)
+	block, _ := pem.Decode(certs.Cert)
 	if block == nil {
-		t.Fatalf("failed to parse certificate PEM: %s", certPEM)
+		t.Fatalf("failed to parse certificate PEM: %s", certs.Cert)
 	}
 	cert, err := x509.ParseCertificate(block.Bytes)
 	if err != nil {
 		t.Fatalf("failed to parse certificate: %v", err)
 	}
 
 	opts := x509.VerifyOptions{
-		DNSName: CN,
+		DNSName: cn,
 		Roots:   roots,
 	}