restructure certwriter and certprovisioner packages

Author: Mengqi Yu

pkg/webhook/bootstrap.go

@@ -64,7 +64,6 @@ Start the server by starting the manager
 	if err != nil {
 		// handle error
 	}
-
 */
 package webhook
 

pkg/webhook/doc.go

@@ -15,88 +15,22 @@ limitations under the License.
 */
 
 /*
-Package cert provides functions to manage webhooks certificates.
+Package cert provides functions to manage certificates for webhookClientConfiguration.
 
-There are 3 typical ways to use this library:
+Create a Provisioner with a CertWriter.
 
-* Using the Sync function as a Reconciler function.
-
-* Invoking it directly from the webhook server at startup.
-
-* Deploying it as an init container along with the webhook server.
-
-Webhook Configuration
-
-The following is an example MutatingWebhookConfiguration in yaml.
-
-	apiVersion: admissionregistration.k8s.io/v1beta1
-	kind: MutatingWebhookConfiguration
-	metadata:
-	  name: myMutatingWebhookConfiguration
-	  annotations:
-	    secret.certprovisioner.kubernetes.io/webhook-1: namespace-bar/secret-foo
-	    secret.certprovisioner.kubernetes.io/webhook-2: default/secret-baz
-	webhooks:
-	- name: webhook-1
-	  rules:
-	  - apiGroups:
-		- ""
-		apiVersions:
-		- v1
-		operations:
-		- "*"
-		resources:
-		- pods
-	  clientConfig:
-		service:
-		  namespace: service-ns-1
-		  name: service-foo
-		  path: "/mutating-pods"
-		caBundle: [] # CA bundle here
-	- name: webhook-2
-	  rules:
-	  - apiGroups:
-		- apps
-		apiVersions:
-		- v1
-		operations:
-		- "*"
-		resources:
-		- deployments
-	  clientConfig:
-		service:
-		  namespace: service-ns-2
-		  name: service-bar
-		  path: "/mutating-deployment"
-		caBundle: [] # CA bundle here
-
-Build the Provisioner
-
-You can choose to provide your own CertGenerator and CertWriter.
-An easier way is to use an empty Options the package will default it with reasonable values.
-The package will write self-signed certificates to secrets.
-
-	// Build a client. You can also create a client with your own config.Config.
-	cl, err := client.New(config.GetConfigOrDie(), client.Options)
-	if err != nil {
-		// handle error
+	provisioner := Provisioner{
+		CertWriter: admission.NewSecretCertWriter(admission.SecretCertWriterOptions{...}),
 	}
 
-	// Build a Provisioner with unspecified CertGenerator and CertWriter.
-	cp := &Provisioner{client: cl}
-
-Provision certificates
+Provision the certificates for the webhookClientConfig
 
-Provision certificates for webhook configuration objects' by calling Sync method.
-
-	err = cp.Sync(mwc)
+	err := provisioner.Provision(Options{
+		ClientConfig: webhookClientConfig,
+		Objects: []runtime.Object{mutatingWebhookConfiguration, validatingWebhookConfiguration}
+	})
 	if err != nil {
-		// handler error
+		// handle error
 	}
-
-When the above MutatingWebhookConfiguration is processed, the cert provisioner will create
-the certificate and create a secret named "secret-foo" in namespace "namespace-bar" for webhook "webhook-1".
-Similarly, it will create an secret named "secret-baz" in namespace "default" for webhook "webhook-2".
-And it will also write the CA back to the WebhookConfiguration.
 */
 package cert

pkg/webhook/internal/cert/doc.go

@@ -17,7 +17,7 @@ limitations under the License.
 /*
 Package generator provides an interface and implementation to provision certificates.
 
-Create an instance of CertGenerator.
+Create an instance of certGenerator.
 
 	cg := SelfSignedCertGenerator{}
 

pkg/webhook/internal/cert/generator/doc.go

@@ -22,7 +22,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/webhook/internal/cert/generator"
 )
 
-// CertGenerator is a CertGenerator for testing.
+// CertGenerator is a certGenerator for testing.
 type CertGenerator struct {
 	DNSNameToCertArtifacts map[string]*generator.Artifacts
 }
@@ -33,7 +33,7 @@ var _ generator.CertGenerator = &CertGenerator{}
 func (cp *CertGenerator) Generate(commonName string) (*generator.Artifacts, error) {
 	certs, found := cp.DNSNameToCertArtifacts[commonName]
 	if !found {
-		return nil, fmt.Errorf("failed to find common name %q in the CertGenerator", commonName)
+		return nil, fmt.Errorf("failed to find common name %q in the certGenerator", commonName)
 	}
 	return certs, nil
 }

pkg/webhook/internal/cert/generator/fake/certgenerator.go

@@ -28,7 +28,7 @@ func ServiceToCommonName(serviceNamespace, serviceName string) string {
 	return fmt.Sprintf("%s.%s.svc", serviceName, serviceNamespace)
 }
 
-// SelfSignedCertGenerator implements the CertGenerator interface.
+// SelfSignedCertGenerator implements the certGenerator interface.
 // It provisions self-signed certificates.
 type SelfSignedCertGenerator struct{}
 

pkg/webhook/internal/cert/generator/selfsigned.go

@@ -17,13 +17,13 @@ limitations under the License.
 package cert
 
 import (
+	"bytes"
+	"errors"
 	"fmt"
-	"reflect"
-	"sync"
+	"net/url"
 
+	admissionregistrationv1beta1 "k8s.io/api/admissionregistration/v1beta1"
 	"k8s.io/apimachinery/pkg/runtime"
-	"sigs.k8s.io/controller-runtime/pkg/client"
-	"sigs.k8s.io/controller-runtime/pkg/client/config"
 	"sigs.k8s.io/controller-runtime/pkg/webhook/internal/cert/generator"
 	"sigs.k8s.io/controller-runtime/pkg/webhook/internal/cert/writer"
 )
@@ -32,62 +32,91 @@ import (
 // destination - such as a Secret or local file. Provisioner can update the CA field of
 // certain resources with the CA of the certs.
 type Provisioner struct {
-	Client client.Client
-	// CertGenerator generates certificate for a given common name.
-	CertGenerator generator.CertGenerator
 	// CertWriter knows how to persist the certificate.
 	CertWriter writer.CertWriter
+}
 
-	once sync.Once
+// Options are options for provisioning the certificate.
+type Options struct {
+	// ClientConfig is the WebhookClientCert that contains the information to generate
+	// the certificate. The CA Certificate will be updated in the ClientConfig.
+	// The updated ClientConfig will be used to inject into other runtime.Objects,
+	// e.g. MutatingWebhookConfiguration and ValidatingWebhookConfiguration.
+	ClientConfig *admissionregistrationv1beta1.WebhookClientConfig
+	// Objects are the objects that will use the ClientConfig above.
+	Objects []runtime.Object
+	// Dryrun controls if the objects are sent to the API server or output in the io.Writer
+	Dryrun bool
 }
 
-// Sync takes a runtime.Object which is expected to be either a MutatingWebhookConfiguration or
-// a ValidatingWebhookConfiguration.
-// It provisions certificate for each webhook in the webhookConfiguration, ensures the cert and CA are valid,
-// and not expiring. It updates the CABundle in the webhook configuration if necessary.
-func (cp *Provisioner) Sync(webhookConfiguration runtime.Object) error {
-	var err error
-	// Do the initialization for CertInput only once.
-	cp.once.Do(func() {
-		if cp.CertGenerator == nil {
-			cp.CertGenerator = &generator.SelfSignedCertGenerator{}
-		}
-		if cp.Client == nil {
-			cp.Client, err = client.New(config.GetConfigOrDie(), client.Options{})
-			if err != nil {
-				return
-			}
-		}
-		if cp.CertWriter == nil {
-			cp.CertWriter, err = writer.NewCertWriter(
-				writer.Options{
-					Client:        cp.Client,
-					CertGenerator: cp.CertGenerator,
-				})
-			if err != nil {
-				return
-			}
-		}
-	})
+// Provision provisions certificates for for the WebhookClientConfig.
+// It ensures the cert and CA are valid and not expiring.
+// It updates the CABundle in the webhookClientConfig if necessary.
+// It inject the WebhookClientConfig into options.Objects.
+func (cp *Provisioner) Provision(options Options) (bool, error) {
+	if cp.CertWriter == nil {
+		return false, errors.New("CertWriter need to be set")
+	}
+
+	dnsName, err := dnsNameFromClientConfig(options.ClientConfig)
 	if err != nil {
-		return fmt.Errorf("failed to default the CertProvision: %v", err)
+		return false, err
 	}
 
-	// Deepcopy the webhook configuration object before invoking EnsureCerts,
-	// since EnsureCerts will modify the provided object.
-	cloned := webhookConfiguration.DeepCopyObject()
-	err = cp.CertWriter.EnsureCerts(webhookConfiguration)
+	certs, changed, err := cp.CertWriter.EnsureCert(dnsName, options.Dryrun)
 	if err != nil {
-		return err
+		return false, err
 	}
 
-	// If some fields have been changed, we will update the object.
-	// Mostly this is because of the CABundle field has been updated.
-	if reflect.DeepEqual(webhookConfiguration, cloned) {
+	caBundle := options.ClientConfig.CABundle
+	caCert := certs.CACert
+	if !bytes.Contains(caBundle, caCert) {
+		// Ensure the CA bundle in the webhook configuration has the signing CA.
+		options.ClientConfig.CABundle = append(caBundle, caCert...)
+		changed = true
+	}
+	return changed, cp.inject(options.ClientConfig, options.Objects)
+}
+
+// Inject the ClientConfig to the objects.
+// It supports MutatingWebhookConfiguration and ValidatingWebhookConfiguration.
+func (cp *Provisioner) inject(cc *admissionregistrationv1beta1.WebhookClientConfig, objs []runtime.Object) error {
+	if cc == nil {
 		return nil
 	}
-	return nil
-	// TODO: figure what we want to do with this.
-	// Disable the auto-updating in this function.
-	//return cp.Client.Update(context.Background(), cloned)
+	for i := range objs {
+		switch typed := objs[i].(type) {
+		case *admissionregistrationv1beta1.MutatingWebhookConfiguration:
+			injectForEachWebhook(cc, typed.Webhooks)
+		case *admissionregistrationv1beta1.ValidatingWebhookConfiguration:
+			injectForEachWebhook(cc, typed.Webhooks)
+		}
+	}
+	return cp.CertWriter.Inject(objs...)
+}
+
+func injectForEachWebhook(
+	cc *admissionregistrationv1beta1.WebhookClientConfig,
+	webhooks []admissionregistrationv1beta1.Webhook) {
+	for i := range webhooks {
+		// only replacing the CA bundle to preserve the path in the WebhookClientConfig
+		webhooks[i].ClientConfig.CABundle = cc.CABundle
+	}
+}
+
+func dnsNameFromClientConfig(config *admissionregistrationv1beta1.WebhookClientConfig) (string, error) {
+	if config.Service != nil && config.URL != nil {
+		return "", fmt.Errorf("service and URL can't be set at the same time in a webhook: %v", config)
+	}
+	if config.Service == nil && config.URL == nil {
+		return "", fmt.Errorf("one of service and URL need to be set in a webhook: %v", config)
+	}
+	if config.Service != nil {
+		return generator.ServiceToCommonName(config.Service.Namespace, config.Service.Name), nil
+	}
+
+	// TODO
+	// config.URL != nil
+	u, err := url.Parse(*config.URL)
+	return u.Host, err
 }

pkg/webhook/internal/cert/provisioner.go

@@ -28,9 +28,9 @@ import (
 	"strings"
 	"testing"
 
+	log "github.com/go-logr/logr/testing"
 	"k8s.io/apimachinery/pkg/util/sets"
 	utiltesting "k8s.io/client-go/util/testing"
-	log "github.com/go-logr/logr/testing"
 )
 
 func TestNewAtomicWriter(t *testing.T) {