Skip to content

Commit

Permalink
address comments
Browse files Browse the repository at this point in the history
  • Loading branch information
Mengqi Yu committed Feb 25, 2019
1 parent b51eb44 commit 8c836d7
Show file tree
Hide file tree
Showing 7 changed files with 198 additions and 196 deletions.
55 changes: 39 additions & 16 deletions pkg/builder/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
}
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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 {
Expand Down Expand Up @@ -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
}
7 changes: 0 additions & 7 deletions pkg/manager/internal.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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,
Expand Down
25 changes: 2 additions & 23 deletions pkg/manager/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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),
Expand All @@ -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
Expand Down
6 changes: 3 additions & 3 deletions pkg/webhook/admission/decode_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand All @@ -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{}{
Expand Down
90 changes: 90 additions & 0 deletions pkg/webhook/defaulter.go
Original file line number Diff line number Diff line change
@@ -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)
}
Loading

0 comments on commit 8c836d7

Please sign in to comment.