diff --git a/pkg/builder/build.go b/pkg/builder/build.go index 2ffb91cf8a..51015e0014 100644 --- a/pkg/builder/build.go +++ b/pkg/builder/build.go @@ -30,6 +30,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/predicate" "sigs.k8s.io/controller-runtime/pkg/reconcile" "sigs.k8s.io/controller-runtime/pkg/source" + "sigs.k8s.io/controller-runtime/pkg/webhook" ) // Supporting mocking out functions for testing @@ -65,6 +66,7 @@ func ControllerManagedBy(m manager.Manager) *Builder { // This is the equivalent of calling // Watches(&source.Kind{Type: apiType}, &handler.EnqueueRequestForObject{}) // Deprecated: Use For +// TODO: Update this func (blder *Builder) ForType(apiType runtime.Object) *Builder { return blder.For(apiType) } @@ -73,6 +75,7 @@ func (blder *Builder) ForType(apiType runtime.Object) *Builder { // update events by *reconciling the object*. // This is the equivalent of calling // Watches(&source.Kind{Type: apiType}, &handler.EnqueueRequestForObject{}) +// TODO: Update this func (blder *Builder) For(apiType runtime.Object) *Builder { blder.apiType = apiType return blder @@ -130,7 +133,7 @@ func (blder *Builder) Complete(r reconcile.Reconciler) error { // Deprecated: Use Complete func (blder *Builder) Build(r reconcile.Reconciler) (manager.Manager, error) { if r == nil { - return nil, fmt.Errorf("must call WithReconciler to set Reconciler") + return nil, fmt.Errorf("must provide a non-nil Reconciler") } // Set the Config @@ -148,24 +151,26 @@ func (blder *Builder) Build(r reconcile.Reconciler) (manager.Manager, error) { return nil, err } - if blder.mgr.GetWebhookServer() != nil { - svr := blder.mgr.GetWebhookServer() - err := svr.EnableDefaultsFor(blder.mgr.GetScheme(), blder.apiType) - if err != nil { - return nil, err - } - err = svr.EnableValidationFor(blder.mgr.GetScheme(), blder.apiType) - if err != nil { - return nil, err - } + // Set the Webook if needed + if err := blder.doWebhook(); err != nil { + return nil, err } + // Set the Watch + if err := blder.doWatch(); err != nil { + return nil, err + } + + return blder.mgr, nil +} + +func (blder *Builder) doWatch() error { // Reconcile type src := &source.Kind{Type: blder.apiType} hdler := &handler.EnqueueRequestForObject{} err := blder.ctrl.Watch(src, hdler, blder.predicates...) if err != nil { - return nil, err + return err } // Watches the managed types @@ -176,19 +181,18 @@ func (blder *Builder) Build(r reconcile.Reconciler) (manager.Manager, error) { IsController: true, } if err := blder.ctrl.Watch(src, hdler, blder.predicates...); err != nil { - return nil, err + return err } } // Do the watch requests for _, w := range blder.watchRequest { if err := blder.ctrl.Watch(w.src, w.eventhandler, blder.predicates...); err != nil { - return nil, err + return err } } - - return blder.mgr, nil + return nil } func (blder *Builder) doConfig() error { @@ -230,3 +234,22 @@ func (blder *Builder) doController(r reconcile.Reconciler) error { blder.ctrl, err = newController(name, blder.mgr, controller.Options{Reconciler: r}) return err } + +func (blder *Builder) doWebhook() error { + svr := &webhook.Server{} + err := svr.EnableWebhooksFor(blder.apiType) + // If no error, we add the Server instance to the controller manager. + // If we get a NotImplementDefaulterValidatorInterfacesError, we discord the Server instance. + // If other type of error, surface it. + if err == nil { + e := blder.mgr.Add(svr) + if e != nil { + return e + } + } else { + if _, ok := err.(webhook.NotImplementDefaulterValidatorInterfacesError); !ok { + return err + } + } + return nil +} diff --git a/pkg/manager/internal.go b/pkg/manager/internal.go index ac3105f7b1..230b78f89c 100644 --- a/pkg/manager/internal.go +++ b/pkg/manager/internal.go @@ -37,7 +37,6 @@ import ( "sigs.k8s.io/controller-runtime/pkg/metrics" "sigs.k8s.io/controller-runtime/pkg/recorder" "sigs.k8s.io/controller-runtime/pkg/runtime/inject" - "sigs.k8s.io/controller-runtime/pkg/webhook" ) var log = logf.RuntimeLog.WithName("manager") @@ -91,8 +90,6 @@ type controllerManager struct { internalStopper chan<- struct{} startCache func(stop <-chan struct{}) error - - webhookServer *webhook.Server } // Add sets dependencies on i, and adds it to the list of runnables to start. @@ -170,10 +167,6 @@ func (cm *controllerManager) GetRESTMapper() meta.RESTMapper { return cm.mapper } -func (cm *controllerManager) GetWebhookServer() *webhook.Server { - return cm.webhookServer -} - func (cm *controllerManager) serveMetrics(stop <-chan struct{}) { handler := promhttp.HandlerFor(metrics.Registry, promhttp.HandlerOpts{ ErrorHandling: promhttp.HTTPErrorOnError, diff --git a/pkg/manager/manager.go b/pkg/manager/manager.go index ad5de9c926..633091eb78 100644 --- a/pkg/manager/manager.go +++ b/pkg/manager/manager.go @@ -36,7 +36,6 @@ import ( "sigs.k8s.io/controller-runtime/pkg/leaderelection" "sigs.k8s.io/controller-runtime/pkg/metrics" "sigs.k8s.io/controller-runtime/pkg/recorder" - "sigs.k8s.io/controller-runtime/pkg/webhook" ) // Manager initializes shared dependencies such as Caches and Clients, and provides them to Runnables. @@ -75,9 +74,6 @@ type Manager interface { // GetRESTMapper returns a RESTMapper GetRESTMapper() meta.RESTMapper - - // GetWebhookServer returns a webhook.Server - GetWebhookServer() *webhook.Server } // Options are the arguments for creating a new Manager @@ -120,12 +116,6 @@ type Options struct { // for serving prometheus metrics MetricsBindAddress string - // Port is where the webhook server serves traffic - Port int - - // CertDir is the path of the directory for the serving certificates. - CertDir string - // Functions to all for a user to customize the values that will be injected. // NewCache is the function that will create the cache to be used @@ -223,7 +213,7 @@ func New(config *rest.Config, options Options) (Manager, error) { stop := make(chan struct{}) - cm := &controllerManager{ + return &controllerManager{ config: config, scheme: options.Scheme, errChan: make(chan error), @@ -236,18 +226,7 @@ func New(config *rest.Config, options Options) (Manager, error) { metricsListener: metricsListener, internalStop: stop, internalStopper: stop, - } - - if options.Port > 0 || len(options.CertDir) > 0 { - server := &webhook.Server{ - Port: options.Port, - CertDir: options.CertDir, - } - cm.webhookServer = server - cm.Add(server) - } - - return cm, nil + }, nil } // defaultNewClient creates the default caching client diff --git a/pkg/webhook/admission/decode_test.go b/pkg/webhook/admission/decode_test.go index efba146d09..5c36ba304f 100644 --- a/pkg/webhook/admission/decode_test.go +++ b/pkg/webhook/admission/decode_test.go @@ -64,7 +64,7 @@ var _ = Describe("Admission Webhook Decoder", func() { It("should decode a valid admission request", func() { By("extracting the object from the request") var actualObj corev1.Pod - Expect(decoder.Decode(req, &actualObj)).To(Succeed()) + Expect(decoder.Decode(req.Object, &actualObj)).To(Succeed()) By("verifying that all data is present in the object") Expect(actualObj).To(Equal(corev1.Pod{ @@ -86,13 +86,13 @@ var _ = Describe("Admission Webhook Decoder", func() { 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") - Expect(decoder.Decode(req, &corev1.Node{})).NotTo(Succeed()) + Expect(decoder.Decode(req.Object, &corev1.Node{})).NotTo(Succeed()) }) It("should be able to decode into an unstructured object", func() { By("decoding into an unstructured object") var target unstructured.Unstructured - Expect(decoder.Decode(req, &target)).To(Succeed()) + Expect(decoder.Decode(req.Object, &target)).To(Succeed()) By("sanity-checking the metadata on the output object") Expect(target.Object["metadata"]).To(Equal(map[string]interface{}{ diff --git a/pkg/webhook/defaulter.go b/pkg/webhook/defaulter.go new file mode 100644 index 0000000000..3c1f232686 --- /dev/null +++ b/pkg/webhook/defaulter.go @@ -0,0 +1,90 @@ +/* +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 webhook + +import ( + "context" + "encoding/json" + "net/http" + + "k8s.io/apimachinery/pkg/runtime" + "sigs.k8s.io/controller-runtime/pkg/runtime/inject" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" +) + +// Defaulter defines functions for setting defaults on resources +type Defaulter interface { + runtime.Object + Default() +} + +// newDefaultingWebhookFor creates a new admission.Webhook for Defaulting the provided type. +// Returns nil if o does not implement the Defaulter interface. +func newDefaultingWebhookFor(o runtime.Object) *admission.Webhook { + d, ok := o.(Defaulter) + if !ok { + // Type doesn't implement function for mutating webhook + return nil + } + return &admission.Webhook{ + Handler: &mutatingHandler{defaulter: d}, + } +} + +type mutatingHandler struct { + defaulter Defaulter + scheme *runtime.Scheme + decoder *admission.Decoder +} + +var _ inject.Scheme = &mutatingHandler{} + +// InjectScheme injects the scheme into mutatingHandler and initialize a decoder using the scheme. +func (h *mutatingHandler) InjectScheme(s *runtime.Scheme) error { + h.scheme = s + var err error + h.decoder, err = admission.NewDecoder(h.scheme) + if err != nil { + log.Error(err, "unable to get a decoder from the scheme for mutatingHandler") + return err + } + return nil +} + +// Handle handles admission requests. +func (h *mutatingHandler) Handle(ctx context.Context, req admission.Request) admission.Response { + if h.defaulter == nil { + return admission.Allowed("") + } + + // Get the object in the request + obj := h.defaulter.DeepCopyObject().(Defaulter) + err := h.decoder.Decode(req.Object, obj) + if err != nil { + return admission.Errored(http.StatusBadRequest, err) + } + + // Default the object + obj.Default() + marshalled, err := json.Marshal(obj) + if err != nil { + return admission.Errored(http.StatusInternalServerError, err) + } + + // Create the patch + return admission.PatchResponseFromRaw(req.Object.Raw, marshalled) +} diff --git a/pkg/webhook/server.go b/pkg/webhook/server.go index b6a3eb427e..a7d057fdc0 100644 --- a/pkg/webhook/server.go +++ b/pkg/webhook/server.go @@ -20,9 +20,9 @@ import ( "context" "crypto/tls" "fmt" - "k8s.io/apimachinery/pkg/runtime/schema" "net" "net/http" + "os" "path" "strconv" "strings" @@ -30,8 +30,9 @@ import ( "time" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" + "sigs.k8s.io/controller-runtime/pkg/client/apiutil" "sigs.k8s.io/controller-runtime/pkg/runtime/inject" - "sigs.k8s.io/controller-runtime/pkg/webhook/admission" "sigs.k8s.io/controller-runtime/pkg/webhook/internal/metrics" ) @@ -40,6 +41,16 @@ const ( keyName = "tls.key" ) +// NotImplementDefaulterValidatorInterfacesError is a typed error for cases when an object implements +// neither Defaulter nor Validator interface. +type NotImplementDefaulterValidatorInterfacesError struct { + gvk *schema.GroupVersionKind +} + +func (e NotImplementDefaulterValidatorInterfacesError) Error() string { + return fmt.Sprintf("%s implements neither the Defaulter interface nor the Validator interface", e.gvk.String()) +} + // Server is an admission webhook server that can serve traffic and // generates related k8s resources for deploying. type Server struct { @@ -58,6 +69,9 @@ type Server struct { // the user is responsible to mount the secret to the this location for the server to consume. CertDir string + // Scheme knows the mapping between go struct types and GVKs. + Scheme *runtime.Scheme + // TODO(directxman12): should we make the mux configurable? // webhookMux is the multiplexer that handles different webhooks. @@ -78,82 +92,67 @@ func (s *Server) setDefaults() { s.webhooks = map[string]http.Handler{} s.webhookMux = http.NewServeMux() + // TODO: probably allow set Port by env. + p, err := strconv.Atoi(os.Getenv("WEBHOOKPORT")) + if err != nil { + log.Error(err, "can't convert WEBHOOKPORT environment variable to int") + } else { + s.Port = p + } if s.Port <= 0 { s.Port = 443 } + if len(s.CertDir) == 0 { s.CertDir = path.Join("/tmp", "k8s-webhook-server", "serving-certs") } } -// For registers defaulting and validation webhooks for the provided types that implement either -// webhooktuil.validator or webhookutil.defaulter. -func (s *Server) EnableDefaultsFor(scheme *runtime.Scheme, objects ...runtime.Object) error { +// EnableWebhooksFor registers mutating (defaulting) and validation webhooks for the provided types that +// implement Defaulter and (or) Validator. Passing in core types doesn't work, since the core types implement +// neither the Defaulter nor the Validator interfaces. +// It returns a NotImplementDefaulterValidatorInterfacesError error if a provided object implements +// neither the Defaulter interface nor the Validator interface. +func (s *Server) EnableWebhooksFor(objects ...runtime.Object) error { + if s.Scheme == nil { + return fmt.Errorf("field Scheme must be set or injected before invoking EnableWebhooksFor") + } + // Create a webhook for each type for _, object := range objects { - wh := newMutatingWebhookFor(object) - if wh == nil { - continue - } - - gvks, _, err := scheme.ObjectKinds(object) + gvk, err := apiutil.GVKForObject(object, s.Scheme) if err != nil { - return err - } - if len(gvks) != 1 { - return fmt.Errorf("expected only GVK returned by scheme.ObjectKinds") + return nil } - fmt.Println("/mutate-" + generatePath(gvks[0])) - s.Register("/mutate-"+generatePath(gvks[0]), wh) - } - return nil -} -// For registers defaulting and validation webhooks for the provided types that implement either -// webhooktuil.validator or webhookutil.defaulter. -func (s *Server) EnableValidationFor(scheme *runtime.Scheme, objects ...runtime.Object) error { - // Create a webhook for each type - for _, object := range objects { - wh := newValidatingWebhookFor(object) - if wh == nil { - continue + mwh := newDefaultingWebhookFor(object) + if mwh != nil { + path := "/mutate-" + generatePath(gvk) + log.Info("registering a mutating webhook", "path", path) + s.Register(path, mwh) + if err = s.setFields(mwh); err != nil { + return err + } + } - gvks, _, err := scheme.ObjectKinds(object) - if err != nil { - return err + vwh := newValidatingWebhookFor(object) + if vwh != nil { + path := "/validate-" + generatePath(gvk) + log.Info("registering a validating webhook", "path", path) + s.Register(path, vwh) + if err = s.setFields(vwh); err != nil { + return err + } } - if len(gvks) != 1 { - return fmt.Errorf("expected only GVK returned by scheme.ObjectKinds") + + if mwh == nil && vwh == nil { + return NotImplementDefaulterValidatorInterfacesError{gvk: &gvk} } - fmt.Println("/validate-" + generatePath(gvks[0])) - s.Register("/validate-"+generatePath(gvks[0]), wh) } return nil } -// PathWithMutateFn creates a mutating webhook with the provided handlers and then -// register it with the given path. -func (s *Server) PathWithMutateFn(path string, handlers ...admission.Handler) { - var h http.Handler - h = &admission.Webhook{ - Handler: admission.MultiMutatingHandler(handlers...), - } - // Register all of the types - s.Register(path, h) -} - -// PathWithValidateFn creates a validating webhook with the provided handlers and then -// register it with the given path. -func (s *Server) PathWithValidateFn(path string, handlers ...admission.Handler) { - var h http.Handler - h = &admission.Webhook{ - Handler: admission.MultiValidatingHandler(handlers...), - } - // Register all of the types - s.Register(path, h) -} - // Register marks the given webhook as being served at the given path. // It panics if two hooks are registered on the same path. func (s *Server) Register(path string, hook http.Handler) { diff --git a/pkg/webhook/handlers.go b/pkg/webhook/validator.go similarity index 51% rename from pkg/webhook/handlers.go rename to pkg/webhook/validator.go index ff6ab1b8f8..00453c2119 100644 --- a/pkg/webhook/handlers.go +++ b/pkg/webhook/validator.go @@ -18,7 +18,6 @@ package webhook import ( "context" - "encoding/json" "net/http" "k8s.io/api/admission/v1beta1" @@ -27,12 +26,6 @@ import ( "sigs.k8s.io/controller-runtime/pkg/webhook/admission" ) -// Defaultor defines functions for setting defaults on resources -type Defaulter interface { - runtime.Object - Default() -} - // Validator defines functions for validating an operation type Validator interface { runtime.Object @@ -40,74 +33,8 @@ type Validator interface { ValidateUpdate(old runtime.Object) error } -// newMutatingWebhookFor creates a new WebhookBuilder for Validation and Defaulting the provided type. Returns nil -// if o does not implement either validator or defaulter. -func newMutatingWebhookFor(o runtime.Object) *admission.Webhook { - d, ok := o.(Defaulter) - if !ok { - // Type doesn't implement function for mutating webhook - return nil - } - return &admission.Webhook{ - Handler: &mutatingHandler{defaulter: d}, - } -} - -type mutatingHandler struct { - defaulter Defaulter - scheme *runtime.Scheme - decoder *admission.Decoder -} - -var _ inject.Scheme = &mutatingHandler{} - -// InjectDecoder injects the decoder into the FirstMateDeleteHandler -func (h *mutatingHandler) InjectScheme(s *runtime.Scheme) error { - h.scheme = s - return nil -} - -//var _ admission.DecoderInjector = &mutatingHandler{} -// -//// InjectDecoder injects the decoder into the FirstMateDeleteHandler -//func (h *mutatingHandler) InjectDecoder(d *admission.Decoder) error { -// h.decoder = d -// return nil -//} - -// Handle handles admission requests. -func (h *mutatingHandler) Handle(ctx context.Context, req admission.Request) admission.Response { - var err error - h.decoder, err = admission.NewDecoder(h.scheme) - if err != nil { - log.Error(err, "unable to get a decoder from the scheme") - return admission.Errored(http.StatusInternalServerError, err) - } - - if h.defaulter == nil { - return admission.Allowed("") - } - - // Get the object in the request - obj := h.defaulter.DeepCopyObject().(Defaulter) - err = h.decoder.Decode(req.Object, obj) - if err != nil { - return admission.Errored(http.StatusBadRequest, err) - } - - // Default the object - obj.Default() - marshalled, err := json.Marshal(obj) - if err != nil { - return admission.Errored(http.StatusInternalServerError, err) - } - - // Create the patch - return admission.PatchResponseFromRaw(req.Object.Raw, marshalled) -} - -// newValidatingWebhookFor creates a new WebhookBuilder for Validation and Defaulting the provided type. Returns nil -// if o does not implement either validator or defaulter. +// newValidatingWebhookFor creates a new admission.Webhook for Validating the provided type. +// Returns nil if o does not implement the Validator interface. func newValidatingWebhookFor(o runtime.Object) *admission.Webhook { v, ok := o.(Validator) if !ok { @@ -130,26 +57,17 @@ var _ inject.Scheme = &validatingHandler{} // InjectDecoder injects the decoder into the FirstMateDeleteHandler func (h *validatingHandler) InjectScheme(s *runtime.Scheme) error { h.scheme = s - return nil -} - -//var _ admission.DecoderInjector = &validatingHandler{} -// -//// InjectDecoder injects the decoder into the FirstMateDeleteHandler -//func (h *validatingHandler) InjectDecoder(d *admission.Decoder) error { -// h.decoder = d -// return nil -//} - -// Handle handles admission requests. -func (h *validatingHandler) Handle(ctx context.Context, req admission.Request) admission.Response { var err error h.decoder, err = admission.NewDecoder(h.scheme) if err != nil { - log.Error(err, "unable to get a decoder from the scheme") - return admission.Errored(http.StatusInternalServerError, err) + log.Error(err, "unable to get a decoder from the scheme for validatingHandler") + return err } + return nil +} +// Handle handles admission requests. +func (h *validatingHandler) Handle(ctx context.Context, req admission.Request) admission.Response { if h.validator == nil { return admission.Allowed("") }