Merge pull request #1525 from vincepri/envtest-conversion-webhook

⚠️ Envtest should modify CRDs appropriately when using webhooks

pkg/envtest/crd.go

@@ -20,12 +20,15 @@ import (
 	"bufio"
 	"bytes"
 	"context"
+	"fmt"
 	"io"
 	"io/ioutil"
 	"os"
 	"path/filepath"
 	"time"
 
+	apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
+	apiextensionsv1beta1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1"
 	"k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
@@ -35,6 +38,7 @@ import (
 	k8syaml "k8s.io/apimachinery/pkg/util/yaml"
 	"k8s.io/client-go/rest"
 	"k8s.io/client-go/util/retry"
+	"k8s.io/utils/pointer"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/yaml"
 )
@@ -60,6 +64,13 @@ type CRDInstallOptions struct {
 	// uninstalled when terminating the test environment.
 	// Defaults to false.
 	CleanUpAfterUse bool
+
+	// WebhookOptions contains the conversion webhook information to install
+	// on the CRDs. This field is usually inherited by the EnvTest options.
+	//
+	// If you're passing this field manually, you need to make sure that
+	// the CA information and host port is filled in properly.
+	WebhookOptions WebhookInstallOptions
 }
 
 const defaultPollInterval = 100 * time.Millisecond
@@ -74,6 +85,10 @@ func InstallCRDs(config *rest.Config, options CRDInstallOptions) ([]client.Objec
 		return nil, err
 	}
 
+	if err := modifyConversionWebhooks(options.CRDs, options.WebhookOptions); err != nil {
+		return nil, err
+	}
+
 	// Create the CRDs in the apiserver
 	if err := CreateCRDs(config, options.CRDs); err != nil {
 		return options.CRDs, err
@@ -340,6 +355,37 @@ func renderCRDs(options *CRDInstallOptions) ([]client.Object, error) {
 	return res, nil
 }
 
+func modifyConversionWebhooks(crds []client.Object, webhookOptions WebhookInstallOptions) error {
+	if len(webhookOptions.LocalServingCAData) == 0 {
+		return nil
+	}
+
+	// generate host port.
+	hostPort, err := webhookOptions.generateHostPort()
+	if err != nil {
+		return err
+	}
+	url := pointer.StringPtr(fmt.Sprintf("https://%s/convert", hostPort))
+
+	for _, crd := range crds {
+		switch c := crd.(type) {
+		case *apiextensionsv1beta1.CustomResourceDefinition:
+			c.Spec.Conversion.WebhookClientConfig.Service = nil
+			c.Spec.Conversion.WebhookClientConfig = &apiextensionsv1beta1.WebhookClientConfig{
+				CABundle: webhookOptions.LocalServingCAData,
+			}
+		case *apiextensionsv1.CustomResourceDefinition:
+			c.Spec.Conversion.Webhook.ClientConfig.Service = nil
+			c.Spec.Conversion.Webhook.ClientConfig = &apiextensionsv1.WebhookClientConfig{
+				URL:      url,
+				CABundle: webhookOptions.LocalServingCAData,
+			}
+		}
+	}
+
+	return nil
+}
+
 // readCRDs reads the CRDs from files and Unmarshals them into structs
 func readCRDs(basePath string, files []os.FileInfo) ([]*unstructured.Unstructured, error) {
 	var crds []*unstructured.Unstructured

pkg/envtest/server.go

@@ -263,6 +263,12 @@ func (te *Environment) Start() (*rest.Config, error) {
 		}
 	}
 
+	// Call PrepWithoutInstalling to setup certificates first
+	// and have them available to patch CRD conversion webhook as well.
+	if err := te.WebhookInstallOptions.PrepWithoutInstalling(); err != nil {
+		return nil, err
+	}
+
 	log.V(1).Info("installing CRDs")
 	te.CRDInstallOptions.CRDs = mergeCRDs(te.CRDInstallOptions.CRDs, te.CRDs)
 	te.CRDInstallOptions.Paths = mergePaths(te.CRDInstallOptions.Paths, te.CRDDirectoryPaths)
@@ -274,9 +280,10 @@ func (te *Environment) Start() (*rest.Config, error) {
 	te.CRDs = crds
 
 	log.V(1).Info("installing webhooks")
-	err = te.WebhookInstallOptions.Install(te.Config)
-
-	return te.Config, err
+	if err := te.WebhookInstallOptions.Install(te.Config); err != nil {
+		return nil, err
+	}
+	return te.Config, nil
 }
 
 func (te *Environment) startControlPlane() error {

pkg/envtest/webhook.go

@@ -80,7 +80,10 @@ type WebhookInstallOptions struct {
 // ModifyWebhookDefinitions modifies webhook definitions by:
 // - applying CABundle based on the provided tinyca
 // - if webhook client config uses service spec, it's removed and replaced with direct url
-func (o *WebhookInstallOptions) ModifyWebhookDefinitions(caData []byte) error {
+func (o *WebhookInstallOptions) ModifyWebhookDefinitions() error {
+	caData := o.LocalServingCAData
+
+	// generate host port.
 	hostPort, err := o.generateHostPort()
 	if err != nil {
 		return err
@@ -161,16 +164,14 @@ func (o *WebhookInstallOptions) generateHostPort() (string, error) {
 // controller-runtime, where we need a random host-port & caData for webhook
 // tests, but may be useful in similar scenarios.
 func (o *WebhookInstallOptions) PrepWithoutInstalling() error {
-	hookCA, err := o.setupCA()
-	if err != nil {
+	if err := o.setupCA(); err != nil {
 		return err
 	}
 	if err := parseWebhook(o); err != nil {
 		return err
 	}
 
-	err = o.ModifyWebhookDefinitions(hookCA)
-	if err != nil {
+	if err := o.ModifyWebhookDefinitions(); err != nil {
 		return err
 	}
 
@@ -179,8 +180,10 @@ func (o *WebhookInstallOptions) PrepWithoutInstalling() error {
 
 // Install installs specified webhooks to the API server
 func (o *WebhookInstallOptions) Install(config *rest.Config) error {
-	if err := o.PrepWithoutInstalling(); err != nil {
-		return err
+	if len(o.LocalServingCAData) == 0 {
+		if err := o.PrepWithoutInstalling(); err != nil {
+			return err
+		}
 	}
 
 	if err := createWebhooks(config, o.MutatingWebhooks, o.ValidatingWebhooks); err != nil {
@@ -269,38 +272,38 @@ func (p *webhookPoller) poll() (done bool, err error) {
 }
 
 // setupCA creates CA for testing and writes them to disk
-func (o *WebhookInstallOptions) setupCA() ([]byte, error) {
+func (o *WebhookInstallOptions) setupCA() error {
 	hookCA, err := integration.NewTinyCA()
 	if err != nil {
-		return nil, fmt.Errorf("unable to set up webhook CA: %v", err)
+		return fmt.Errorf("unable to set up webhook CA: %v", err)
 	}
 
 	names := []string{"localhost", o.LocalServingHost, o.LocalServingHostExternalName}
 	hookCert, err := hookCA.NewServingCert(names...)
 	if err != nil {
-		return nil, fmt.Errorf("unable to set up webhook serving certs: %v", err)
+		return fmt.Errorf("unable to set up webhook serving certs: %v", err)
 	}
 
 	localServingCertsDir, err := ioutil.TempDir("", "envtest-serving-certs-")
 	o.LocalServingCertDir = localServingCertsDir
 	if err != nil {
-		return nil, fmt.Errorf("unable to create directory for webhook serving certs: %v", err)
+		return fmt.Errorf("unable to create directory for webhook serving certs: %v", err)
 	}
 
 	certData, keyData, err := hookCert.AsBytes()
 	if err != nil {
-		return nil, fmt.Errorf("unable to marshal webhook serving certs: %v", err)
+		return fmt.Errorf("unable to marshal webhook serving certs: %v", err)
 	}
 
 	if err := ioutil.WriteFile(filepath.Join(localServingCertsDir, "tls.crt"), certData, 0640); err != nil {
-		return nil, fmt.Errorf("unable to write webhook serving cert to disk: %v", err)
+		return fmt.Errorf("unable to write webhook serving cert to disk: %v", err)
 	}
 	if err := ioutil.WriteFile(filepath.Join(localServingCertsDir, "tls.key"), keyData, 0640); err != nil {
-		return nil, fmt.Errorf("unable to write webhook serving key to disk: %v", err)
+		return fmt.Errorf("unable to write webhook serving key to disk: %v", err)
 	}
 
 	o.LocalServingCAData = certData
-	return certData, nil
+	return err
 }
 
 func createWebhooks(config *rest.Config, mutHooks []client.Object, valHooks []client.Object) error {