Skip to content

Commit

Permalink
Envtest should modify CRDs appropriately when using webhooks
Browse files Browse the repository at this point in the history 
Before this change, the webhook code was patching mutation or admission
webhooks to point to the temporary URL and CA bundle generated when the
webhook server would start.

Conversion webhooks are handled at the CRD level in Kubernetes, and we
need to make sure to patch those client configurations as well.

This patch also unexports a lot of methods that should have been private
from the very beginning, so it's marked as a breaking change.

Signed-off-by: Vince Prignano <vincepri@vmware.com>
  • Loading branch information
@vincepri
vincepri committed May 14, 2021
1 parent ce2f0c9 commit 0080db8
Show file tree
Hide file tree
Showing 4 changed files with 75 additions and 36 deletions.
37 changes: 36 additions & 1 deletion pkg/envtest/crd.go
View file
Expand Up @@ -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"
Expand All @@ -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"
)
Expand Down Expand Up @@ -66,14 +70,18 @@ const defaultPollInterval = 100 * time.Millisecond
const defaultMaxWait = 10 * time.Second

// InstallCRDs installs a collection of CRDs into a cluster by reading the crd yaml files from a directory
func InstallCRDs(config *rest.Config, options CRDInstallOptions) ([]client.Object, error) {
func InstallCRDs(config *rest.Config, options CRDInstallOptions, webhookOptions WebhookInstallOptions) ([]client.Object, error) {
defaultCRDOptions(&options)

// Read the CRD yamls into options.CRDs
if err := readCRDFiles(&options); err != nil {
return nil, err
}

if err := modifyConversionWebhooks(options.CRDs, webhookOptions); err != nil {
return nil, err
}

// Create the CRDs in the apiserver
if err := CreateCRDs(config, options.CRDs); err != nil {
return options.CRDs, err
Expand Down Expand Up @@ -340,6 +348,33 @@ func renderCRDs(options *CRDInstallOptions) ([]client.Object, error) {
return res, nil
}

func modifyConversionWebhooks(crds []client.Object, webhookOptions WebhookInstallOptions) error {
// 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
Expand Down
28 changes: 14 additions & 14 deletions pkg/envtest/envtest_test.go
View file
Expand Up @@ -84,7 +84,7 @@ var _ = Describe("Test", func() {
It("should install the unserved CRDs into the cluster", func() {
crds, err = InstallCRDs(env.Config, CRDInstallOptions{
Paths: []string{filepath.Join(".", "testdata", "crds", "examplecrd_unserved.yaml")},
})
}, WebhookInstallOptions{})
Expect(err).NotTo(HaveOccurred())

// Expect to find the CRDs
Expand Down Expand Up @@ -122,7 +122,7 @@ var _ = Describe("Test", func() {
It("should install the CRDs into the cluster using directory", func(done Done) {
crds, err = InstallCRDs(env.Config, CRDInstallOptions{
Paths: []string{validDirectory},
})
}, WebhookInstallOptions{})
Expect(err).NotTo(HaveOccurred())

// Expect to find the CRDs
Expand Down Expand Up @@ -226,7 +226,7 @@ var _ = Describe("Test", func() {
It("should install the CRDs into the cluster using file", func(done Done) {
crds, err = InstallCRDs(env.Config, CRDInstallOptions{
Paths: []string{filepath.Join(".", "testdata", "crds", "examplecrd3.yaml")},
})
}, WebhookInstallOptions{})
Expect(err).NotTo(HaveOccurred())

crd := &v1beta1.CustomResourceDefinition{}
Expand Down Expand Up @@ -257,7 +257,7 @@ var _ = Describe("Test", func() {
filepath.Join(".", "testdata", "examplecrd.yaml"),
filepath.Join(".", "testdata", "examplecrd_v1.yaml"),
},
})
}, WebhookInstallOptions{})
Expect(err).NotTo(HaveOccurred())
Expect(crds).To(HaveLen(2))

Expand All @@ -270,7 +270,7 @@ var _ = Describe("Test", func() {
filepath.Join(".", "testdata"),
filepath.Join(".", "testdata", "examplecrd1.yaml"),
},
})
}, WebhookInstallOptions{})
Expect(err).NotTo(HaveOccurred())

crd := &apiextensionsv1.CustomResourceDefinition{}
Expand Down Expand Up @@ -307,7 +307,7 @@ var _ = Describe("Test", func() {
}, 10)

It("should not return an not error if the directory doesn't exist", func(done Done) {
crds, err = InstallCRDs(env.Config, CRDInstallOptions{Paths: []string{invalidDirectory}})
crds, err = InstallCRDs(env.Config, CRDInstallOptions{Paths: []string{invalidDirectory}}, WebhookInstallOptions{})
Expect(err).NotTo(HaveOccurred())

close(done)
Expand All @@ -316,7 +316,7 @@ var _ = Describe("Test", func() {
It("should return an error if the directory doesn't exist", func(done Done) {
crds, err = InstallCRDs(env.Config, CRDInstallOptions{
Paths: []string{invalidDirectory}, ErrorIfPathMissing: true,
})
}, WebhookInstallOptions{})
Expect(err).To(HaveOccurred())

close(done)
Expand All @@ -325,7 +325,7 @@ var _ = Describe("Test", func() {
It("should return an error if the file doesn't exist", func(done Done) {
crds, err = InstallCRDs(env.Config, CRDInstallOptions{Paths: []string{
filepath.Join(".", "testdata", "fake.yaml")}, ErrorIfPathMissing: true,
})
}, WebhookInstallOptions{})
Expect(err).To(HaveOccurred())

close(done)
Expand Down Expand Up @@ -353,7 +353,7 @@ var _ = Describe("Test", func() {
It("should return an error if the resource isn't found in the group version", func(done Done) {
crds, err = InstallCRDs(env.Config, CRDInstallOptions{
Paths: []string{"."},
})
}, WebhookInstallOptions{})
Expect(err).NotTo(HaveOccurred())

// Wait for a CRD that doesn't exist, but the Group and Version do
Expand Down Expand Up @@ -385,7 +385,7 @@ var _ = Describe("Test", func() {

crds, err = InstallCRDs(env.Config, CRDInstallOptions{
Paths: []string{filepath.Join(".", "testdata")},
})
}, WebhookInstallOptions{})
Expect(err).NotTo(HaveOccurred())

// Expect to find the CRDs
Expand Down Expand Up @@ -487,7 +487,7 @@ var _ = Describe("Test", func() {

crds, err = InstallCRDs(env.Config, CRDInstallOptions{
Paths: []string{filepath.Join(".", "testdata")},
})
}, WebhookInstallOptions{})
Expect(err).NotTo(HaveOccurred())

// Expect to find the CRDs
Expand Down Expand Up @@ -594,7 +594,7 @@ var _ = Describe("Test", func() {
// Install only the CRDv1 multi-version example
crds, err = InstallCRDs(env.Config, CRDInstallOptions{
Paths: []string{filepath.Join(".", "testdata")},
})
}, WebhookInstallOptions{})
Expect(err).NotTo(HaveOccurred())

// Expect to find the CRDs
Expand Down Expand Up @@ -636,7 +636,7 @@ var _ = Describe("Test", func() {
// Add one more version and update
_, err = InstallCRDs(env.Config, CRDInstallOptions{
Paths: []string{filepath.Join(".", "testdata", "crdv1_updated")},
})
}, WebhookInstallOptions{})
Expect(err).NotTo(HaveOccurred())

// Expect to find updated CRD
Expand Down Expand Up @@ -686,7 +686,7 @@ var _ = Describe("Test", func() {

crds, err = InstallCRDs(env.Config, CRDInstallOptions{
Paths: []string{validDirectory},
})
}, WebhookInstallOptions{})
Expect(err).NotTo(HaveOccurred())

// Expect to find the CRDs
Expand Down
15 changes: 11 additions & 4 deletions pkg/envtest/server.go
View file
Expand Up @@ -263,20 +263,27 @@ 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)
te.CRDInstallOptions.ErrorIfPathMissing = te.ErrorIfCRDPathMissing
crds, err := InstallCRDs(te.Config, te.CRDInstallOptions)
crds, err := InstallCRDs(te.Config, te.CRDInstallOptions, te.WebhookInstallOptions)
if err != nil {
return te.Config, err
}
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 {
Expand Down
31 changes: 14 additions & 17 deletions pkg/envtest/webhook.go
View file
Expand Up @@ -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
Expand Down Expand Up @@ -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
}

Expand All @@ -179,10 +180,6 @@ 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 err := createWebhooks(config, o.MutatingWebhooks, o.ValidatingWebhooks); err != nil {
return err
}
Expand Down Expand Up @@ -269,38 +266,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 {
Expand Down

0 comments on commit 0080db8

Please sign in to comment.