diff --git a/pkg/builder/build.go b/pkg/builder/build.go index 8160501b16..7debf14743 100644 --- a/pkg/builder/build.go +++ b/pkg/builder/build.go @@ -21,6 +21,7 @@ import ( "strings" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/client-go/rest" "sigs.k8s.io/controller-runtime/pkg/client/apiutil" "sigs.k8s.io/controller-runtime/pkg/client/config" @@ -249,9 +250,6 @@ func (blder *Builder) doWebhook() error { return err } - partialPath := strings.Replace(gvk.Group, ".", "-", -1) + "-" + - gvk.Version + "-" + strings.ToLower(gvk.Kind) - // TODO: When the conversion webhook lands, we need to handle all registered versions of a given group-kind. // A potential workflow for defaulting webhook // 1) a bespoke (non-hub) version comes in @@ -267,7 +265,7 @@ func (blder *Builder) doWebhook() error { if defaulter, isDefaulter := blder.apiType.(admission.Defaulter); isDefaulter { mwh := admission.DefaultingWebhookFor(defaulter) if mwh != nil { - path := "/mutate-" + partialPath + path := generateMutatePath(gvk) log.Info("Registering a mutating webhook", "GVK", gvk, "path", path) @@ -279,7 +277,7 @@ func (blder *Builder) doWebhook() error { if validator, isValidator := blder.apiType.(admission.Validator); isValidator { vwh := admission.ValidatingWebhookFor(validator) if vwh != nil { - path := "/validate-" + partialPath + path := generateValidatePath(gvk) log.Info("Registering a validating webhook", "GVK", gvk, "path", path) @@ -289,3 +287,13 @@ func (blder *Builder) doWebhook() error { return err } + +func generateMutatePath(gvk schema.GroupVersionKind) string { + return "/mutate-" + strings.Replace(gvk.Group, ".", "-", -1) + "-" + + gvk.Version + "-" + strings.ToLower(gvk.Kind) +} + +func generateValidatePath(gvk schema.GroupVersionKind) string { + return "/validate-" + strings.Replace(gvk.Group, ".", "-", -1) + "-" + + gvk.Version + "-" + strings.ToLower(gvk.Kind) +} diff --git a/pkg/builder/build_test.go b/pkg/builder/build_test.go index 75c1795e98..a65890afa9 100644 --- a/pkg/builder/build_test.go +++ b/pkg/builder/build_test.go @@ -18,12 +18,13 @@ package builder import ( "context" + "errors" "fmt" + "net/http" + "net/http/httptest" + "os" "strings" - "sigs.k8s.io/controller-runtime/pkg/handler" - "sigs.k8s.io/controller-runtime/pkg/source" - . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" appsv1 "k8s.io/api/apps/v1" @@ -33,10 +34,13 @@ import ( "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/rest" - "sigs.k8s.io/controller-runtime/pkg/client/apiutil" "sigs.k8s.io/controller-runtime/pkg/controller" + "sigs.k8s.io/controller-runtime/pkg/handler" "sigs.k8s.io/controller-runtime/pkg/manager" "sigs.k8s.io/controller-runtime/pkg/reconcile" + "sigs.k8s.io/controller-runtime/pkg/scheme" + "sigs.k8s.io/controller-runtime/pkg/source" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" ) var _ = Describe("application", func() { @@ -47,7 +51,6 @@ var _ = Describe("application", func() { getConfig = func() (*rest.Config, error) { return cfg, nil } newController = controller.New newManager = manager.New - getGvk = apiutil.GVKForObject }) AfterEach(func() { @@ -121,6 +124,232 @@ var _ = Describe("application", func() { Expect(err.Error()).To(ContainSubstring("expected error")) Expect(instance).To(BeNil()) }) + + It("should scaffold a defaulting webhook if the type implements the Defaulter interface", func() { + By("creating a controller manager") + m, err := manager.New(cfg, manager.Options{}) + Expect(err).NotTo(HaveOccurred()) + + By("registering the type in the Scheme") + builder := scheme.Builder{GroupVersion: testDefaulterGVK.GroupVersion()} + builder.Register(&TestDefaulter{}, &TestDefaulterList{}) + err = builder.AddToScheme(m.GetScheme()) + Expect(err).NotTo(HaveOccurred()) + + instance, err := ControllerManagedBy(m). + For(&TestDefaulter{}). + Owns(&appsv1.ReplicaSet{}). + Build(noop) + Expect(err).NotTo(HaveOccurred()) + Expect(instance).NotTo(BeNil()) + svr := m.GetWebhookServer() + Expect(svr).NotTo(BeNil()) + + reader := strings.NewReader(`{ + "kind":"AdmissionReview", + "apiVersion":"admission.k8s.io/v1beta1", + "request":{ + "uid":"07e52e8d-4513-11e9-a716-42010a800270", + "kind":{ + "group":"", + "version":"v1", + "kind":"TestDefaulter" + }, + "resource":{ + "group":"", + "version":"v1", + "resource":"testdefaulter" + }, + "namespace":"default", + "operation":"CREATE", + "object":{ + "replica":1 + }, + "oldObject":null + } +}`) + + stopCh := make(chan struct{}) + close(stopCh) + // TODO: we may want to improve it to make it be able to inject dependencies, + // but not always try to load certs and return not found error. + err = svr.Start(stopCh) + if err != nil && !os.IsNotExist(err) { + Expect(err).NotTo(HaveOccurred()) + } + + By("sending a request to a mutating webhook path") + path := generateMutatePath(testDefaulterGVK) + req := httptest.NewRequest("POST", "http://svc-name.svc-ns.svc"+path, reader) + req.Header.Add(http.CanonicalHeaderKey("Content-Type"), "application/json") + w := httptest.NewRecorder() + svr.WebhookMux.ServeHTTP(w, req) + Expect(w.Code).To(Equal(http.StatusOK)) + By("sanity checking the response contains reasonable fields") + Expect(w.Body).To(ContainSubstring(`"allowed":true`)) + Expect(w.Body).To(ContainSubstring(`"patch":`)) + Expect(w.Body).To(ContainSubstring(`"code":200`)) + + By("sending a request to a validating webhook path that doesn't exist") + path = generateValidatePath(testDefaulterGVK) + req = httptest.NewRequest("POST", "http://svc-name.svc-ns.svc"+path, reader) + req.Header.Add(http.CanonicalHeaderKey("Content-Type"), "application/json") + w = httptest.NewRecorder() + svr.WebhookMux.ServeHTTP(w, req) + Expect(w.Code).To(Equal(http.StatusNotFound)) + }) + + It("should scaffold a validating webhook if the type implements the Validator interface", func() { + By("creating a controller manager") + m, err := manager.New(cfg, manager.Options{}) + Expect(err).NotTo(HaveOccurred()) + + By("registering the type in the Scheme") + builder := scheme.Builder{GroupVersion: testValidatorGVK.GroupVersion()} + builder.Register(&TestValidator{}, &TestValidatorList{}) + err = builder.AddToScheme(m.GetScheme()) + Expect(err).NotTo(HaveOccurred()) + + instance, err := ControllerManagedBy(m). + For(&TestValidator{}). + Owns(&appsv1.ReplicaSet{}). + Build(noop) + Expect(err).NotTo(HaveOccurred()) + Expect(instance).NotTo(BeNil()) + svr := m.GetWebhookServer() + Expect(svr).NotTo(BeNil()) + + reader := strings.NewReader(`{ + "kind":"AdmissionReview", + "apiVersion":"admission.k8s.io/v1beta1", + "request":{ + "uid":"07e52e8d-4513-11e9-a716-42010a800270", + "kind":{ + "group":"", + "version":"v1", + "kind":"TestValidator" + }, + "resource":{ + "group":"", + "version":"v1", + "resource":"testvalidator" + }, + "namespace":"default", + "operation":"UPDATE", + "object":{ + "replica":1 + }, + "oldObject":{ + "replica":2 + } + } +}`) + + stopCh := make(chan struct{}) + close(stopCh) + // TODO: we may want to improve it to make it be able to inject dependencies, + // but not always try to load certs and return not found error. + err = svr.Start(stopCh) + if err != nil && !os.IsNotExist(err) { + Expect(err).NotTo(HaveOccurred()) + } + + By("sending a request to a mutating webhook path that doesn't exist") + path := generateMutatePath(testValidatorGVK) + req := httptest.NewRequest("POST", "http://svc-name.svc-ns.svc"+path, reader) + req.Header.Add(http.CanonicalHeaderKey("Content-Type"), "application/json") + w := httptest.NewRecorder() + svr.WebhookMux.ServeHTTP(w, req) + Expect(w.Code).To(Equal(http.StatusNotFound)) + + By("sending a request to a validating webhook path") + path = generateValidatePath(testValidatorGVK) + req = httptest.NewRequest("POST", "http://svc-name.svc-ns.svc"+path, reader) + req.Header.Add(http.CanonicalHeaderKey("Content-Type"), "application/json") + w = httptest.NewRecorder() + svr.WebhookMux.ServeHTTP(w, req) + Expect(w.Code).To(Equal(http.StatusOK)) + By("sanity checking the response contains reasonable field") + Expect(w.Body).To(ContainSubstring(`"allowed":false`)) + Expect(w.Body).To(ContainSubstring(`"code":200`)) + }) + + It("should scaffold defaulting and validating webhooks if the type implements both Defaulter and Validator interfaces", func() { + By("creating a controller manager") + m, err := manager.New(cfg, manager.Options{}) + Expect(err).NotTo(HaveOccurred()) + + By("registering the type in the Scheme") + builder := scheme.Builder{GroupVersion: testDefaultValidatorGVK.GroupVersion()} + builder.Register(&TestDefaultValidator{}, &TestDefaultValidatorList{}) + err = builder.AddToScheme(m.GetScheme()) + Expect(err).NotTo(HaveOccurred()) + + instance, err := ControllerManagedBy(m). + For(&TestDefaultValidator{}). + Owns(&appsv1.ReplicaSet{}). + Build(noop) + Expect(err).NotTo(HaveOccurred()) + Expect(instance).NotTo(BeNil()) + svr := m.GetWebhookServer() + Expect(svr).NotTo(BeNil()) + + reader := strings.NewReader(`{ + "kind":"AdmissionReview", + "apiVersion":"admission.k8s.io/v1beta1", + "request":{ + "uid":"07e52e8d-4513-11e9-a716-42010a800270", + "kind":{ + "group":"", + "version":"v1", + "kind":"TestDefaultValidator" + }, + "resource":{ + "group":"", + "version":"v1", + "resource":"testdefaultvalidator" + }, + "namespace":"default", + "operation":"CREATE", + "object":{ + "replica":1 + }, + "oldObject":null + } +}`) + + stopCh := make(chan struct{}) + close(stopCh) + // TODO: we may want to improve it to make it be able to inject dependencies, + // but not always try to load certs and return not found error. + err = svr.Start(stopCh) + if err != nil && !os.IsNotExist(err) { + Expect(err).NotTo(HaveOccurred()) + } + + By("sending a request to a mutating webhook path") + path := generateMutatePath(testDefaultValidatorGVK) + req := httptest.NewRequest("POST", "http://svc-name.svc-ns.svc"+path, reader) + req.Header.Add(http.CanonicalHeaderKey("Content-Type"), "application/json") + w := httptest.NewRecorder() + svr.WebhookMux.ServeHTTP(w, req) + Expect(w.Code).To(Equal(http.StatusOK)) + By("sanity checking the response contains reasonable field") + Expect(w.Body).To(ContainSubstring(`"allowed":true`)) + Expect(w.Body).To(ContainSubstring(`"patch":`)) + Expect(w.Body).To(ContainSubstring(`"code":200`)) + + By("sending a request to a validating webhook path") + path = generateValidatePath(testDefaultValidatorGVK) + req = httptest.NewRequest("POST", "http://svc-name.svc-ns.svc"+path, reader) + req.Header.Add(http.CanonicalHeaderKey("Content-Type"), "application/json") + w = httptest.NewRecorder() + svr.WebhookMux.ServeHTTP(w, req) + Expect(w.Code).To(Equal(http.StatusOK)) + By("sanity checking the response contains reasonable field") + Expect(w.Body).To(ContainSubstring(`"allowed":true`)) + Expect(w.Body).To(ContainSubstring(`"code":200`)) + }) }) Describe("Start with SimpleController", func() { @@ -281,3 +510,121 @@ type fakeType struct{} func (*fakeType) GetObjectKind() schema.ObjectKind { return nil } func (*fakeType) DeepCopyObject() runtime.Object { return nil } + +// TestDefaulter +var _ runtime.Object = &TestDefaulter{} + +type TestDefaulter struct { + Replica int `json:"replica,omitempty"` +} + +var testDefaulterGVK = schema.GroupVersionKind{Group: "foo.test.org", Version: "v1", Kind: "TestDefaulter"} + +func (*TestDefaulter) GetObjectKind() schema.ObjectKind { return nil } +func (d *TestDefaulter) DeepCopyObject() runtime.Object { + return &TestDefaulter{ + Replica: d.Replica, + } +} + +var _ runtime.Object = &TestDefaulterList{} + +type TestDefaulterList struct{} + +func (*TestDefaulterList) GetObjectKind() schema.ObjectKind { return nil } +func (*TestDefaulterList) DeepCopyObject() runtime.Object { return nil } + +func (d *TestDefaulter) Default() { + if d.Replica < 2 { + d.Replica = 2 + } +} + +// TestValidator +var _ runtime.Object = &TestValidator{} + +type TestValidator struct { + Replica int `json:"replica,omitempty"` +} + +var testValidatorGVK = schema.GroupVersionKind{Group: "foo.test.org", Version: "v1", Kind: "TestValidator"} + +func (*TestValidator) GetObjectKind() schema.ObjectKind { return nil } +func (v *TestValidator) DeepCopyObject() runtime.Object { + return &TestValidator{ + Replica: v.Replica, + } +} + +var _ runtime.Object = &TestValidatorList{} + +type TestValidatorList struct{} + +func (*TestValidatorList) GetObjectKind() schema.ObjectKind { return nil } +func (*TestValidatorList) DeepCopyObject() runtime.Object { return nil } + +var _ admission.Validator = &TestValidator{} + +func (v *TestValidator) ValidateCreate() error { + if v.Replica < 0 { + return errors.New("number of replica should be greater than or equal to 0") + } + return nil +} + +func (v *TestValidator) ValidateUpdate(old runtime.Object) error { + if v.Replica < 0 { + return errors.New("number of replica should be greater than or equal to 0") + } + if oldObj, ok := old.(*TestValidator); !ok { + return fmt.Errorf("the old object is expected to be %T", oldObj) + } else if v.Replica < oldObj.Replica { + return fmt.Errorf("new replica %v should not be fewer than old replica %v", v.Replica, oldObj.Replica) + } + return nil +} + +// TestDefaultValidator +var _ runtime.Object = &TestDefaultValidator{} + +type TestDefaultValidator struct { + Replica int `json:"replica,omitempty"` +} + +var testDefaultValidatorGVK = schema.GroupVersionKind{Group: "foo.test.org", Version: "v1", Kind: "TestDefaultValidator"} + +func (*TestDefaultValidator) GetObjectKind() schema.ObjectKind { return nil } +func (dv *TestDefaultValidator) DeepCopyObject() runtime.Object { + return &TestDefaultValidator{ + Replica: dv.Replica, + } +} + +var _ runtime.Object = &TestDefaultValidatorList{} + +type TestDefaultValidatorList struct{} + +func (*TestDefaultValidatorList) GetObjectKind() schema.ObjectKind { return nil } +func (*TestDefaultValidatorList) DeepCopyObject() runtime.Object { return nil } + +func (dv *TestDefaultValidator) Default() { + if dv.Replica < 2 { + dv.Replica = 2 + } +} + +var _ admission.Validator = &TestDefaultValidator{} + +func (dv *TestDefaultValidator) ValidateCreate() error { + if dv.Replica < 0 { + return errors.New("number of replica should be greater than or equal to 0") + } + return nil +} + +func (dv *TestDefaultValidator) ValidateUpdate(old runtime.Object) error { + if dv.Replica < 0 { + return errors.New("number of replica should be greater than or equal to 0") + } + return nil +} diff --git a/pkg/builder/builder_suite_test.go b/pkg/builder/builder_suite_test.go index 39098ce934..e8e8f0ae66 100644 --- a/pkg/builder/builder_suite_test.go +++ b/pkg/builder/builder_suite_test.go @@ -22,6 +22,10 @@ import ( . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" + apiextensionsv1beta1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1" + "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/client-go/rest" "sigs.k8s.io/controller-runtime/pkg/envtest" logf "sigs.k8s.io/controller-runtime/pkg/log" @@ -43,6 +47,10 @@ var _ = BeforeSuite(func(done Done) { logf.SetLogger(zap.LoggerTo(GinkgoWriter, true)) testenv = &envtest.Environment{} + addCRDToEnvironment(testenv, + testDefaulterGVK, + testValidatorGVK, + testDefaultValidatorGVK) var err error cfg, err = testenv.Start() @@ -66,3 +74,35 @@ var _ = AfterSuite(func() { // Change the webhook.DefaultPort back to the original default. webhook.DefaultPort = 443 }) + +func addCRDToEnvironment(env *envtest.Environment, gvks ...schema.GroupVersionKind) { + for _, gvk := range gvks { + plural, singlar := meta.UnsafeGuessKindToResource(gvk) + crd := &apiextensionsv1beta1.CustomResourceDefinition{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "apiextensions.k8s.io", + Kind: "CustomResourceDefinition", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: plural.Resource + "." + gvk.Group, + }, + Spec: apiextensionsv1beta1.CustomResourceDefinitionSpec{ + Group: gvk.Group, + Version: gvk.Version, + Names: apiextensionsv1beta1.CustomResourceDefinitionNames{ + Plural: plural.Resource, + Singular: singlar.Resource, + Kind: gvk.Kind, + }, + Versions: []apiextensionsv1beta1.CustomResourceDefinitionVersion{ + { + Name: gvk.Version, + Served: true, + Storage: true, + }, + }, + }, + } + env.CRDs = append(env.CRDs, crd) + } +} diff --git a/pkg/manager/manager_test.go b/pkg/manager/manager_test.go index d46ff8cc18..dcf0ee425a 100644 --- a/pkg/manager/manager_test.go +++ b/pkg/manager/manager_test.go @@ -121,6 +121,22 @@ var _ = Describe("manger.Manager", func() { close(done) }) + + It("should lazily initialize a webhook server if needed", func(done Done) { + By("creating a manager with options") + m, err := New(cfg, Options{Port: 9443, Host: "foo.com"}) + Expect(err).NotTo(HaveOccurred()) + Expect(m).NotTo(BeNil()) + + By("checking options are passed to the webhook server") + svr := m.GetWebhookServer() + Expect(svr).NotTo(BeNil()) + Expect(svr.Port).To(Equal(9443)) + Expect(svr.Host).To(Equal("foo.com")) + + close(done) + }) + Context("with leader election enabled", func() { It("should default ID to controller-runtime if ID is not set", func() { var rl resourcelock.Interface diff --git a/pkg/webhook/admission/decode_test.go b/pkg/webhook/admission/decode_test.go index efba146d09..a9ad0875be 100644 --- a/pkg/webhook/admission/decode_test.go +++ b/pkg/webhook/admission/decode_test.go @@ -51,7 +51,25 @@ var _ = Describe("Admission Webhook Decoder", func() { "spec": { "containers": [ { - "image": "bar", + "image": "bar:v2", + "name": "bar" + } + ] + } +}`), + }, + OldObject: runtime.RawExtension{ + Raw: []byte(`{ + "apiVersion": "v1", + "kind": "Pod", + "metadata": { + "name": "foo", + "namespace": "default" + }, + "spec": { + "containers": [ + { + "image": "bar:v1", "name": "bar" } ] @@ -78,19 +96,45 @@ var _ = Describe("Admission Webhook Decoder", func() { }, Spec: corev1.PodSpec{ Containers: []corev1.Container{ - {Image: "bar", Name: "bar"}, + {Image: "bar:v2", Name: "bar"}, + }, + }, + })) + }) + + It("should decode a valid RawExtension object", func() { + By("decoding the RawExtension object") + var actualObj corev1.Pod + Expect(decoder.DecodeRaw(req.OldObject, &actualObj)).To(Succeed()) + + By("verifying that all data is present in the object") + Expect(actualObj).To(Equal(corev1.Pod{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "v1", + Kind: "Pod", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + Namespace: "default", + }, + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + {Image: "bar:v1", Name: "bar"}, }, }, })) }) It("should fail to decode if the object in the request doesn't match the passed-in type", func() { - By("trying to extract a pod into a node") + By("trying to extract a pod from the quest into a node") Expect(decoder.Decode(req, &corev1.Node{})).NotTo(Succeed()) + + By("trying to extract a pod in RawExtension format into a node") + Expect(decoder.DecodeRaw(req.OldObject, &corev1.Node{})).NotTo(Succeed()) }) It("should be able to decode into an unstructured object", func() { - By("decoding into an unstructured object") + By("decoding the request into an unstructured object") var target unstructured.Unstructured Expect(decoder.Decode(req, &target)).To(Succeed()) @@ -99,5 +143,15 @@ var _ = Describe("Admission Webhook Decoder", func() { "name": "foo", "namespace": "default", })) + + By("decoding the RawExtension object into an unstructured object") + var target2 unstructured.Unstructured + Expect(decoder.DecodeRaw(req.Object, &target2)).To(Succeed()) + + By("sanity-checking the metadata on the output object") + Expect(target2.Object["metadata"]).To(Equal(map[string]interface{}{ + "name": "foo", + "namespace": "default", + })) }) }) diff --git a/pkg/webhook/server.go b/pkg/webhook/server.go index c55e249c1e..19a9888391 100644 --- a/pkg/webhook/server.go +++ b/pkg/webhook/server.go @@ -57,10 +57,9 @@ type Server struct { // the user is responsible to mount the secret to the this location for the server to consume. CertDir string - // TODO(directxman12): should we make the mux configurable? + // WebhookMux is the multiplexer that handles different webhooks. + WebhookMux *http.ServeMux - // webhookMux is the multiplexer that handles different webhooks. - webhookMux *http.ServeMux // webhooks keep track of all registered webhooks for dependency injection, // and to provide better panic messages on duplicate webhook registration. webhooks map[string]http.Handler @@ -75,9 +74,11 @@ type Server struct { // setDefaults does defaulting for the Server. func (s *Server) setDefaults() { s.webhooks = map[string]http.Handler{} - s.webhookMux = http.NewServeMux() + if s.WebhookMux == nil { + s.WebhookMux = http.NewServeMux() + } - if s.Port <= 0 { + if s.Port < 0 { s.Port = DefaultPort } @@ -96,7 +97,7 @@ func (s *Server) Register(path string, hook http.Handler) { } // TODO(directxman12): call setfields if we've already started the server s.webhooks[path] = hook - s.webhookMux.Handle(path, instrumentedHook(path, hook)) + s.WebhookMux.Handle(path, instrumentedHook(path, hook)) } // instrumentedHook adds some instrumentation on top of the given webhook. @@ -147,7 +148,7 @@ func (s *Server) Start(stop <-chan struct{}) error { } srv := &http.Server{ - Handler: s.webhookMux, + Handler: s.WebhookMux, } idleConnsClosed := make(chan struct{})