diff --git a/Gopkg.lock b/Gopkg.lock index 897580722e..967d96f23c 100644 --- a/Gopkg.lock +++ b/Gopkg.lock @@ -962,6 +962,7 @@ "k8s.io/apimachinery/pkg/runtime/serializer", "k8s.io/apimachinery/pkg/selection", "k8s.io/apimachinery/pkg/types", + "k8s.io/apimachinery/pkg/util/json", "k8s.io/apimachinery/pkg/util/runtime", "k8s.io/apimachinery/pkg/util/sets", "k8s.io/apimachinery/pkg/util/uuid", diff --git a/alias.go b/alias.go index 2620fab772..b41f51435b 100644 --- a/alias.go +++ b/alias.go @@ -22,11 +22,11 @@ import ( "sigs.k8s.io/controller-runtime/pkg/builder" "sigs.k8s.io/controller-runtime/pkg/client/config" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" + "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/manager" + "sigs.k8s.io/controller-runtime/pkg/manager/signals" "sigs.k8s.io/controller-runtime/pkg/reconcile" - "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/scheme" - "sigs.k8s.io/controller-runtime/pkg/manager/signals" ) // Builder builds an Application ControllerManagedBy (e.g. Operator) and returns a manager.Manager to start it. @@ -47,7 +47,7 @@ type Manager = manager.Manager // Options are the arguments for creating a new Manager type Options = manager.Options -// Builder builds a new Scheme for mapping go types to Kubernetes GroupVersionKinds. +// SchemeBuilder builds a new Scheme for mapping go types to Kubernetes GroupVersionKinds. type SchemeBuilder = scheme.Builder // GroupVersion contains the "group" and the "version", which uniquely identifies the API. diff --git a/doc.go b/doc.go index 5151792e27..253d6556fc 100644 --- a/doc.go +++ b/doc.go @@ -110,7 +110,7 @@ limitations under the License. // controller-runtime. // // Metrics (pkg/metrics) provided by controller-runtime are registered into a -// controller-runtime-specific Prometheus metrics registery. The manager can +// controller-runtime-specific Prometheus metrics registry. The manager can // serve these by an HTTP endpoint, and additional metrics may be registered to // this Registry as normal. // diff --git a/example/main.go b/example/main.go index 55188fc739..aa33c94831 100644 --- a/example/main.go +++ b/example/main.go @@ -32,7 +32,6 @@ import ( "sigs.k8s.io/controller-runtime/pkg/manager/signals" "sigs.k8s.io/controller-runtime/pkg/source" "sigs.k8s.io/controller-runtime/pkg/webhook" - "sigs.k8s.io/controller-runtime/pkg/webhook/admission/builder" ) var log = logf.Log.WithName("example-controller") @@ -78,35 +77,19 @@ func main() { } // Setup webhooks - entryLog.Info("setting up webhooks") - mutatingWebhook := builder.NewWebhookBuilder(). - Path("/mutate-pods"). - Mutating(). - Handlers(&podAnnotator{}). - Build() - - validatingWebhook := builder.NewWebhookBuilder(). - Path("/validate-pods"). - Validating(). - Handlers(&podValidator{}). - Build() - entryLog.Info("setting up webhook server") - as, err := webhook.NewServer(mgr, webhook.ServerOptions{ + hookServer := &webhook.Server{ Port: 9876, CertDir: "/tmp/cert", - }) - if err != nil { - entryLog.Error(err, "unable to create a new webhook server") + } + if err := mgr.Add(hookServer); err != nil { + entryLog.Error(err, "unable register webhook server with manager") os.Exit(1) } entryLog.Info("registering webhooks to the webhook server") - err = as.Register(mutatingWebhook, validatingWebhook) - if err != nil { - entryLog.Error(err, "unable to setup the admission server") - os.Exit(1) - } + hookServer.Register("/mutate-pods", &webhook.Admission{Handler: &podAnnotator{}}) + hookServer.Register("/validate-pods", &webhook.Admission{Handler: &podValidator{}}) entryLog.Info("starting manager") if err := mgr.Start(signals.SetupSignalHandler()); err != nil { diff --git a/example/mutatingwebhook.go b/example/mutatingwebhook.go index 98b0323809..ec361bf811 100644 --- a/example/mutatingwebhook.go +++ b/example/mutatingwebhook.go @@ -23,67 +23,51 @@ import ( corev1 "k8s.io/api/core/v1" "sigs.k8s.io/controller-runtime/pkg/client" - "sigs.k8s.io/controller-runtime/pkg/runtime/inject" "sigs.k8s.io/controller-runtime/pkg/webhook/admission" - "sigs.k8s.io/controller-runtime/pkg/webhook/admission/types" ) // podAnnotator annotates Pods type podAnnotator struct { client client.Client - decoder types.Decoder + decoder *admission.Decoder } -// Implement admission.Handler so the controller can handle admission request. -var _ admission.Handler = &podAnnotator{} - // podAnnotator adds an annotation to every incoming pods. -func (a *podAnnotator) Handle(ctx context.Context, req types.Request) types.Response { +func (a *podAnnotator) Handle(ctx context.Context, req admission.Request) admission.Response { pod := &corev1.Pod{} err := a.decoder.Decode(req, pod) if err != nil { - return admission.ErrorResponse(http.StatusBadRequest, err) + return admission.Errored(http.StatusBadRequest, err) } - err = a.mutatePodsFn(ctx, pod) - if err != nil { - return admission.ErrorResponse(http.StatusInternalServerError, err) + if pod.Annotations == nil { + pod.Annotations = map[string]string{} } + pod.Annotations["example-mutating-admission-webhook"] = "foo" marshaledPod, err := json.Marshal(pod) if err != nil { - return admission.ErrorResponse(http.StatusInternalServerError, err) + return admission.Errored(http.StatusInternalServerError, err) } return admission.PatchResponseFromRaw(req.AdmissionRequest.Object.Raw, marshaledPod) } -// mutatePodsFn add an annotation to the given pod -func (a *podAnnotator) mutatePodsFn(ctx context.Context, pod *corev1.Pod) error { - if pod.Annotations == nil { - pod.Annotations = map[string]string{} - } - pod.Annotations["example-mutating-admission-webhook"] = "foo" - return nil -} - // podAnnotator implements inject.Client. // A client will be automatically injected. -var _ inject.Client = &podAnnotator{} // InjectClient injects the client. -func (v *podAnnotator) InjectClient(c client.Client) error { - v.client = c +func (a *podAnnotator) InjectClient(c client.Client) error { + a.client = c return nil } // podAnnotator implements inject.Decoder. // A decoder will be automatically injected. -var _ inject.Decoder = &podAnnotator{} // InjectDecoder injects the decoder. -func (v *podAnnotator) InjectDecoder(d types.Decoder) error { - v.decoder = d +func (a *podAnnotator) InjectDecoder(d *admission.Decoder) error { + a.decoder = d return nil } diff --git a/example/validatingwebhook.go b/example/validatingwebhook.go index 5019cb611c..6c02616b28 100644 --- a/example/validatingwebhook.go +++ b/example/validatingwebhook.go @@ -23,54 +23,38 @@ import ( corev1 "k8s.io/api/core/v1" "sigs.k8s.io/controller-runtime/pkg/client" - "sigs.k8s.io/controller-runtime/pkg/runtime/inject" "sigs.k8s.io/controller-runtime/pkg/webhook/admission" - "sigs.k8s.io/controller-runtime/pkg/webhook/admission/types" ) // podValidator validates Pods type podValidator struct { client client.Client - decoder types.Decoder + decoder *admission.Decoder } -// Implement admission.Handler so the controller can handle admission request. -var _ admission.Handler = &podValidator{} - // podValidator admits a pod iff a specific annotation exists. -func (v *podValidator) Handle(ctx context.Context, req types.Request) types.Response { +func (v *podValidator) Handle(ctx context.Context, req admission.Request) admission.Response { pod := &corev1.Pod{} err := v.decoder.Decode(req, pod) if err != nil { - return admission.ErrorResponse(http.StatusBadRequest, err) + return admission.Errored(http.StatusBadRequest, err) } - allowed, reason, err := v.validatePodsFn(ctx, pod) - if err != nil { - return admission.ErrorResponse(http.StatusInternalServerError, err) - } - return admission.ValidationResponse(allowed, reason) -} - -func (v *podValidator) validatePodsFn(ctx context.Context, pod *corev1.Pod) (bool, string, error) { key := "example-mutating-admission-webhook" anno, found := pod.Annotations[key] - switch { - case !found: - return found, fmt.Sprintf("failed to find annotation with key: %q", key), nil - case found && anno == "foo": - return found, "", nil - case found && anno != "foo": - return false, - fmt.Sprintf("the value associate with key %q is expected to be %q, but got %q", key, "foo", anno), nil + if !found { + return admission.Denied(fmt.Sprintf("missing annotation %s", key)) + } + if anno != "foo" { + return admission.Denied(fmt.Sprintf("annotation %s did not have value %q", key, "foo")) } - return false, "", nil + + return admission.Allowed("") } // podValidator implements inject.Client. // A client will be automatically injected. -var _ inject.Client = &podValidator{} // InjectClient injects the client. func (v *podValidator) InjectClient(c client.Client) error { @@ -80,10 +64,9 @@ func (v *podValidator) InjectClient(c client.Client) error { // podValidator implements inject.Decoder. // A decoder will be automatically injected. -var _ inject.Decoder = &podValidator{} // InjectDecoder injects the decoder. -func (v *podValidator) InjectDecoder(d types.Decoder) error { +func (v *podValidator) InjectDecoder(d *admission.Decoder) error { v.decoder = d return nil } diff --git a/example_test.go b/example_test.go index dde560cf63..be587f5b0b 100644 --- a/example_test.go +++ b/example_test.go @@ -81,7 +81,7 @@ func (a *ReplicaSetReconciler) Reconcile(req controllers.Request) (controllers.R // List the Pods matching the PodTemplate Labels pods := &corev1.PodList{} - err = a.List(context.TODO(), client.InNamespace(req.Namespace).MatchingLabels(rs.Spec.Template.Labels), pods) + err = a.List(context.TODO(), pods, client.InNamespace(req.Namespace), client.MatchingLabels(rs.Spec.Template.Labels)) if err != nil { return controllers.Result{}, err } diff --git a/hack/test-all.sh b/hack/test-all.sh index cb840b1506..594e7eaa05 100755 --- a/hack/test-all.sh +++ b/hack/test-all.sh @@ -22,16 +22,16 @@ setup_envs header_text "running go test" -go test ./pkg/... -parallel 4 +go test ./... -parallel 4 header_text "running coverage" # Verify no coverage regressions have been introduced. Remove the exception list from here # once the coverage has been brought back up -if [[ ! $(go test ./pkg/... -coverprofile cover.out -parallel 4 | grep -v "coverage: 100.0% of statements" | grep "controller-runtime/pkg " | grep -v "controller-runtime/pkg \|controller-runtime/pkg/recorder \|pkg/admission/certprovisioner \|pkg/internal/admission \|pkg/cache\|pkg/client \|pkg/event \|pkg/client/config \|pkg/controller/controllertest \|pkg/reconcile/reconciletest \|pkg/test ") ]]; then +if [[ ! $(go test ./pkg/... -coverprofile cover.out -parallel 4 | grep -v "coverage: 100.0% of statements" | grep "controller-runtime/pkg " | grep -v "controller-runtime/pkg \|controller-runtime/pkg/recorder \|pkg/cache\|pkg/client \|pkg/event \|pkg/client/config \|pkg/controller/controllertest \|pkg/reconcile/reconciletest \|pkg/test ") ]]; then echo "ok" else -go test ./pkg/... -coverprofile cover.out -parallel 4 | grep -v "coverage: 100.0% of statements" | grep "controller-runtime/pkg " | grep -v "controller-runtime/pkg \|controller-runtime/pkg/recorder \|pkg/admission/certprovisioner \|pkg/internal/admission \|pkg/cache\|pkg/client \|pkg/event \|pkg/client/config \|pkg/controller/controllertest \|pkg/reconcile/reconciletest \|pkg/test " +go test ./pkg/... -coverprofile cover.out -parallel 4 | grep -v "coverage: 100.0% of statements" | grep "controller-runtime/pkg " | grep -v "controller-runtime/pkg \|controller-runtime/pkg/recorder \|pkg/cache\|pkg/client \|pkg/event \|pkg/client/config \|pkg/controller/controllertest \|pkg/reconcile/reconciletest \|pkg/test " echo "missing test coverage" exit 1 fi diff --git a/hack/verify.sh b/hack/verify.sh index 43278b73af..4e9b760495 100755 --- a/hack/verify.sh +++ b/hack/verify.sh @@ -20,7 +20,7 @@ source $(dirname ${BASH_SOURCE})/common.sh header_text "running go vet" -go vet ./pkg/... +go vet ./... # go get is broken for golint. re-enable this once it is fixed. #header_text "running golint" @@ -49,9 +49,9 @@ gometalinter.v2 --disable-all \ --dupl-threshold=400 \ --enable=dupl \ --skip=atomic \ - ./pkg/... + --enable=goimports \ + ./pkg/... ./example/... . # TODO: Enable these as we fix them to make them pass -# --enable=goimports \ # --enable=gosec \ # --enable=maligned \ # --enable=safesql \ diff --git a/pkg/builder/builder_suite_test.go b/pkg/builder/builder_suite_test.go index 46ee514ac5..db6a238e15 100644 --- a/pkg/builder/builder_suite_test.go +++ b/pkg/builder/builder_suite_test.go @@ -23,9 +23,9 @@ import ( . "github.com/onsi/gomega" "k8s.io/client-go/rest" "sigs.k8s.io/controller-runtime/pkg/envtest" - "sigs.k8s.io/controller-runtime/pkg/metrics" logf "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/log/zap" + "sigs.k8s.io/controller-runtime/pkg/metrics" ) func TestSource(t *testing.T) { diff --git a/pkg/builder/example_test.go b/pkg/builder/example_test.go index 25e0facbe4..2197be8af2 100644 --- a/pkg/builder/example_test.go +++ b/pkg/builder/example_test.go @@ -29,8 +29,8 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/config" "sigs.k8s.io/controller-runtime/pkg/manager" - "sigs.k8s.io/controller-runtime/pkg/reconcile" "sigs.k8s.io/controller-runtime/pkg/manager/signals" + "sigs.k8s.io/controller-runtime/pkg/reconcile" ) // This example creates a simple application ControllerManagedBy that is configured for ReplicaSets and Pods. diff --git a/pkg/client/client_cache.go b/pkg/client/client_cache.go index d6452ab62e..2a1ff05d50 100644 --- a/pkg/client/client_cache.go +++ b/pkg/client/client_cache.go @@ -22,7 +22,7 @@ import ( "sync" "k8s.io/apimachinery/pkg/api/meta" - "k8s.io/apimachinery/pkg/apis/meta/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/runtime/serializer" @@ -141,5 +141,5 @@ type objMeta struct { *resourceMeta // Object contains meta data for the object instance - v1.Object + metav1.Object } diff --git a/pkg/client/example_test.go b/pkg/client/example_test.go index c0ab71301f..9e29f5b3ff 100644 --- a/pkg/client/example_test.go +++ b/pkg/client/example_test.go @@ -22,16 +22,16 @@ import ( "os" corev1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/apis/meta/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" - "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/config" ) var ( - c client.Client + c client.Client someIndexer client.FieldIndexer ) @@ -78,7 +78,7 @@ func ExampleClient_get() { func ExampleClient_create() { // Using a typed object. pod := &corev1.Pod{ - ObjectMeta: v1.ObjectMeta{ + ObjectMeta: metav1.ObjectMeta{ Namespace: "namespace", Name: "name", }, @@ -177,7 +177,7 @@ func ExampleClient_update() { func ExampleClient_delete() { // Using a typed object. pod := &corev1.Pod{ - ObjectMeta: v1.ObjectMeta{ + ObjectMeta: metav1.ObjectMeta{ Namespace: "namespace", Name: "name", }, @@ -215,7 +215,7 @@ func ExampleFieldIndexer_secretName() { }) // elsewhere (e.g. in your reconciler) - mySecretName := "someSecret" // derived from the reconcile.Request, for instance + mySecretName := "someSecret" // derived from the reconcile.Request, for instance var podsWithSecrets corev1.PodList _ = c.List(context.Background(), &podsWithSecrets, client.MatchingField("spec.volumes.secret.secretName", mySecretName)) } diff --git a/pkg/controller/controller.go b/pkg/controller/controller.go index 0ad828c477..8d5f8a34cc 100644 --- a/pkg/controller/controller.go +++ b/pkg/controller/controller.go @@ -80,15 +80,15 @@ func New(name string, mgr manager.Manager, options Options) (Controller, error) // Create controller with dependencies set c := &controller.Controller{ - Do: options.Reconciler, - Cache: mgr.GetCache(), - Config: mgr.GetConfig(), - Scheme: mgr.GetScheme(), - Client: mgr.GetClient(), - Recorder: mgr.GetEventRecorderFor(name), - Queue: workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), name), + Do: options.Reconciler, + Cache: mgr.GetCache(), + Config: mgr.GetConfig(), + Scheme: mgr.GetScheme(), + Client: mgr.GetClient(), + Recorder: mgr.GetEventRecorderFor(name), + Queue: workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), name), MaxConcurrentReconciles: options.MaxConcurrentReconciles, - Name: name, + Name: name, } // Add the controller as a Manager components diff --git a/pkg/controller/controller_suite_test.go b/pkg/controller/controller_suite_test.go index ec730a2168..30c7fd6efe 100644 --- a/pkg/controller/controller_suite_test.go +++ b/pkg/controller/controller_suite_test.go @@ -24,9 +24,9 @@ import ( "k8s.io/client-go/kubernetes" "k8s.io/client-go/rest" "sigs.k8s.io/controller-runtime/pkg/envtest" - "sigs.k8s.io/controller-runtime/pkg/metrics" logf "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/log/zap" + "sigs.k8s.io/controller-runtime/pkg/metrics" ) func TestSource(t *testing.T) { diff --git a/pkg/controller/controllerutil/controllerutil.go b/pkg/controller/controllerutil/controllerutil.go index 3edc98e541..320530ce3f 100644 --- a/pkg/controller/controllerutil/controllerutil.go +++ b/pkg/controller/controllerutil/controllerutil.go @@ -22,7 +22,7 @@ import ( "reflect" "k8s.io/apimachinery/pkg/api/errors" - "k8s.io/apimachinery/pkg/apis/meta/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" "sigs.k8s.io/controller-runtime/pkg/client" @@ -33,15 +33,15 @@ import ( // a controller reference is already owned by another controller Object is the // subject and Owner is the reference for the current owner type AlreadyOwnedError struct { - Object v1.Object - Owner v1.OwnerReference + Object metav1.Object + Owner metav1.OwnerReference } func (e *AlreadyOwnedError) Error() string { return fmt.Sprintf("Object %s/%s is already owned by another %s controller %s", e.Object.GetNamespace(), e.Object.GetName(), e.Owner.Kind, e.Owner.Name) } -func newAlreadyOwnedError(Object v1.Object, Owner v1.OwnerReference) *AlreadyOwnedError { +func newAlreadyOwnedError(Object metav1.Object, Owner metav1.OwnerReference) *AlreadyOwnedError { return &AlreadyOwnedError{ Object: Object, Owner: Owner, @@ -53,7 +53,7 @@ func newAlreadyOwnedError(Object v1.Object, Owner v1.OwnerReference) *AlreadyOwn // reconciling the owner object on changes to owned (with a Watch + EnqueueRequestForOwner). // Since only one OwnerReference can be a controller, it returns an error if // there is another OwnerReference with Controller flag set. -func SetControllerReference(owner, object v1.Object, scheme *runtime.Scheme) error { +func SetControllerReference(owner, object metav1.Object, scheme *runtime.Scheme) error { ro, ok := owner.(runtime.Object) if !ok { return fmt.Errorf("is not a %T a runtime.Object, cannot call SetControllerReference", owner) @@ -65,7 +65,7 @@ func SetControllerReference(owner, object v1.Object, scheme *runtime.Scheme) err } // Create a new ref - ref := *v1.NewControllerRef(owner, schema.GroupVersionKind{Group: gvk.Group, Version: gvk.Version, Kind: gvk.Kind}) + ref := *metav1.NewControllerRef(owner, schema.GroupVersionKind{Group: gvk.Group, Version: gvk.Version, Kind: gvk.Kind}) existingRefs := object.GetOwnerReferences() fi := -1 @@ -88,7 +88,7 @@ func SetControllerReference(owner, object v1.Object, scheme *runtime.Scheme) err } // Returns true if a and b point to the same object -func referSameObject(a, b v1.OwnerReference) bool { +func referSameObject(a, b metav1.OwnerReference) bool { aGV, err := schema.ParseGroupVersion(a.APIVersion) if err != nil { return false diff --git a/pkg/controller/example_test.go b/pkg/controller/example_test.go index 6ad7aaa4fa..b6183d7439 100644 --- a/pkg/controller/example_test.go +++ b/pkg/controller/example_test.go @@ -19,15 +19,15 @@ package controller_test import ( "os" - "k8s.io/api/core/v1" + corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime/schema" "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" logf "sigs.k8s.io/controller-runtime/pkg/log" + "sigs.k8s.io/controller-runtime/pkg/manager" "sigs.k8s.io/controller-runtime/pkg/manager/signals" + "sigs.k8s.io/controller-runtime/pkg/reconcile" "sigs.k8s.io/controller-runtime/pkg/source" ) @@ -70,7 +70,7 @@ func ExampleController() { } // Watch for Pod create / update / delete events and call Reconcile - err = c.Watch(&source.Kind{Type: &v1.Pod{}}, &handler.EnqueueRequestForObject{}) + err = c.Watch(&source.Kind{Type: &corev1.Pod{}}, &handler.EnqueueRequestForObject{}) if err != nil { log.Error(err, "unable to watch pods") os.Exit(1) diff --git a/pkg/event/event.go b/pkg/event/event.go index 4874e2e875..6aa50bf301 100644 --- a/pkg/event/event.go +++ b/pkg/event/event.go @@ -17,7 +17,7 @@ limitations under the License. package event import ( - "k8s.io/apimachinery/pkg/apis/meta/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" ) @@ -25,7 +25,7 @@ import ( // by a source.Source and transformed into a reconcile.Request by an handler.EventHandler. type CreateEvent struct { // Meta is the ObjectMeta of the Kubernetes Type that was created - Meta v1.Object + Meta metav1.Object // Object is the object from the event Object runtime.Object @@ -35,13 +35,13 @@ type CreateEvent struct { // by a source.Source and transformed into a reconcile.Request by an handler.EventHandler. type UpdateEvent struct { // MetaOld is the ObjectMeta of the Kubernetes Type that was updated (before the update) - MetaOld v1.Object + MetaOld metav1.Object // ObjectOld is the object from the event ObjectOld runtime.Object // MetaNew is the ObjectMeta of the Kubernetes Type that was updated (after the update) - MetaNew v1.Object + MetaNew metav1.Object // ObjectNew is the object from the event ObjectNew runtime.Object @@ -51,7 +51,7 @@ type UpdateEvent struct { // by a source.Source and transformed into a reconcile.Request by an handler.EventHandler. type DeleteEvent struct { // Meta is the ObjectMeta of the Kubernetes Type that was deleted - Meta v1.Object + Meta metav1.Object // Object is the object from the event Object runtime.Object @@ -66,7 +66,7 @@ type DeleteEvent struct { // handler.EventHandler. type GenericEvent struct { // Meta is the ObjectMeta of a Kubernetes Type this event is for - Meta v1.Object + Meta metav1.Object // Object is the object from the event Object runtime.Object diff --git a/pkg/handler/enqueue.go b/pkg/handler/enqueue.go index 2d0d4a6bda..2464c8b671 100644 --- a/pkg/handler/enqueue.go +++ b/pkg/handler/enqueue.go @@ -20,8 +20,8 @@ import ( "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/util/workqueue" "sigs.k8s.io/controller-runtime/pkg/event" - "sigs.k8s.io/controller-runtime/pkg/reconcile" logf "sigs.k8s.io/controller-runtime/pkg/internal/log" + "sigs.k8s.io/controller-runtime/pkg/reconcile" ) var enqueueLog = logf.RuntimeLog.WithName("eventhandler").WithName("EnqueueRequestForObject") diff --git a/pkg/handler/enqueue_owner.go b/pkg/handler/enqueue_owner.go index 563e0c051b..17d512696c 100644 --- a/pkg/handler/enqueue_owner.go +++ b/pkg/handler/enqueue_owner.go @@ -26,9 +26,9 @@ import ( "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/util/workqueue" "sigs.k8s.io/controller-runtime/pkg/event" + logf "sigs.k8s.io/controller-runtime/pkg/internal/log" "sigs.k8s.io/controller-runtime/pkg/reconcile" "sigs.k8s.io/controller-runtime/pkg/runtime/inject" - logf "sigs.k8s.io/controller-runtime/pkg/internal/log" ) var _ EventHandler = &EnqueueRequestForOwner{} diff --git a/pkg/internal/admission/decode.go b/pkg/internal/admission/decode.go deleted file mode 100644 index e3dae2dbbd..0000000000 --- a/pkg/internal/admission/decode.go +++ /dev/null @@ -1,48 +0,0 @@ -/* -Copyright 2017 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 admission - -import ( - "fmt" - - "k8s.io/api/admission/v1beta1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/runtime/serializer" -) - -var ( - scheme = runtime.NewScheme() - codecs = serializer.NewCodecFactory(scheme) -) - -// Decode reads the Raw data from review and deserializes it into object returning a non-nil response if there was an -// error -func Decode(review v1beta1.AdmissionReview, object runtime.Object, - resourceType metav1.GroupVersionResource) *v1beta1.AdmissionResponse { - if review.Request.Resource != resourceType { - return ErrorResponse(fmt.Errorf("expect resource to be %s", resourceType)) - } - - raw := review.Request.Object.Raw - deserializer := codecs.UniversalDeserializer() - if _, _, err := deserializer.Decode(raw, nil, object); err != nil { - fmt.Printf("%v", err) - return ErrorResponse(err) - } - return nil -} diff --git a/pkg/internal/admission/example_admissionfunc_test.go b/pkg/internal/admission/example_admissionfunc_test.go deleted file mode 100644 index 4d83ec6325..0000000000 --- a/pkg/internal/admission/example_admissionfunc_test.go +++ /dev/null @@ -1,42 +0,0 @@ -/* -Copyright 2017 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 admission_test - -import ( - "fmt" - - "k8s.io/api/admission/v1beta1" - corev1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "sigs.k8s.io/controller-runtime/pkg/internal/admission" -) - -func ExampleFunc() { - var _ admission.Func = func(review v1beta1.AdmissionReview) *v1beta1.AdmissionResponse { - pod := corev1.Pod{} - resourceType := metav1.GroupVersionResource{Group: "", Version: "v1", Resource: "pods"} - if errResp := admission.Decode(review, &pod, resourceType); errResp != nil { - return errResp - } - // Business logic for admission decision - if len(pod.Spec.Containers) != 1 { - return admission.DenyResponse(fmt.Sprintf( - "pod %s/%s may only have 1 container.", pod.Namespace, pod.Name)) - } - return admission.AllowResponse() - } -} diff --git a/pkg/internal/admission/example_decode_test.go b/pkg/internal/admission/example_decode_test.go deleted file mode 100644 index d3ce25d082..0000000000 --- a/pkg/internal/admission/example_decode_test.go +++ /dev/null @@ -1,47 +0,0 @@ -/* -Copyright 2017 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 admission_test - -import ( - "fmt" - - "k8s.io/api/admission/v1beta1" - corev1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "sigs.k8s.io/controller-runtime/pkg/internal/admission" -) - -func ExampleDecode() { - var review v1beta1.AdmissionReview - resourceType := metav1.GroupVersionResource{Group: "", Version: "v1", Resource: "pods"} - pod := corev1.Pod{} - if errResp := admission.Decode(review, &pod, resourceType); errResp != nil { - // Send error resp - } -} - -func ExampleErrorResponse() { - admission.ErrorResponse(fmt.Errorf("some error explanation")) -} - -func ExampleDenyResponse() { - admission.DenyResponse(fmt.Sprintf("some deny explanation")) -} - -func ExampleAllowResponse() { - admission.AllowResponse() -} diff --git a/pkg/internal/admission/example_handlefunc_test.go b/pkg/internal/admission/example_handlefunc_test.go deleted file mode 100644 index 3333bc37c6..0000000000 --- a/pkg/internal/admission/example_handlefunc_test.go +++ /dev/null @@ -1,59 +0,0 @@ -/* -Copyright 2017 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 admission_test - -import ( - "fmt" - - "k8s.io/api/admission/v1beta1" - corev1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "sigs.k8s.io/controller-runtime/pkg/internal/admission" -) - -func ExampleHandleFunc() { - resourceType := metav1.GroupVersionResource{Group: "", Version: "v1", Resource: "pods"} - admission.HandleFunc("/pod", resourceType, func(review v1beta1.AdmissionReview) *v1beta1.AdmissionResponse { - pod := corev1.Pod{} - if errResp := admission.Decode(review, &pod, resourceType); errResp != nil { - return errResp - } - // Business logic for admission decision - if len(pod.Spec.Containers) != 1 { - return admission.DenyResponse(fmt.Sprintf( - "pod %s/%s may only have 1 container.", pod.Namespace, pod.Name)) - } - return admission.AllowResponse() - }) -} - -func ExampleManager_HandleFunc() { - resourceType := metav1.GroupVersionResource{Group: "", Version: "v1", Resource: "pods"} - ah := admission.Manager{} - ah.HandleFunc("/pod", resourceType, func(review v1beta1.AdmissionReview) *v1beta1.AdmissionResponse { - pod := corev1.Pod{} - if errResp := admission.Decode(review, &pod, resourceType); errResp != nil { - return errResp - } - // Business logic for admission decision - if len(pod.Spec.Containers) != 1 { - return admission.DenyResponse(fmt.Sprintf( - "pod %s/%s may only have 1 container.", pod.Namespace, pod.Name)) - } - return admission.AllowResponse() - }) -} diff --git a/pkg/internal/admission/example_test.go b/pkg/internal/admission/example_test.go deleted file mode 100644 index 4fe959bdc3..0000000000 --- a/pkg/internal/admission/example_test.go +++ /dev/null @@ -1,43 +0,0 @@ -/* -Copyright 2017 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 admission_test - -import ( - "fmt" - - "k8s.io/api/admission/v1beta1" - corev1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "sigs.k8s.io/controller-runtime/pkg/internal/admission" -) - -func Example() { - resourceType := metav1.GroupVersionResource{Group: "", Version: "v1", Resource: "pods"} - admission.HandleFunc("/pod", resourceType, func(review v1beta1.AdmissionReview) *v1beta1.AdmissionResponse { - pod := corev1.Pod{} - if errResp := admission.Decode(review, &pod, resourceType); errResp != nil { - return errResp - } - // Business logic for admission decision - if len(pod.Spec.Containers) != 1 { - return admission.DenyResponse(fmt.Sprintf( - "pod %s/%s may only have 1 container.", pod.Namespace, pod.Name)) - } - return admission.AllowResponse() - }) - admission.ListenAndServeTLS("") -} diff --git a/pkg/internal/admission/handler.go b/pkg/internal/admission/handler.go deleted file mode 100644 index 933ff43e34..0000000000 --- a/pkg/internal/admission/handler.go +++ /dev/null @@ -1,72 +0,0 @@ -/* -Copyright 2017 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 admission - -import ( - "net/http" - - "k8s.io/api/admission/v1beta1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" -) - -// Func implements an AdmissionReview operation for a GroupVersionResource -type Func func(review v1beta1.AdmissionReview) *v1beta1.AdmissionResponse - -// HandleEntry -type admissionHandler struct { - GVR metav1.GroupVersionResource - Fn Func -} - -// handle handles an admission request and returns a result -func (ah admissionHandler) handle(review v1beta1.AdmissionReview) *v1beta1.AdmissionResponse { - return ah.handle(review) -} - -// Manager manages admission controllers -type Manager struct { - Entries map[string]admissionHandler - SMux *http.ServeMux -} - -// DefaultAdmissionFns is the default admission control functions registry -var DefaultAdmissionFns = &Manager{ - SMux: http.DefaultServeMux, -} - -// HandleFunc registers fn as an admission control webhook callback for the group,version,resources specified -func (e *Manager) HandleFunc(path string, gvr metav1.GroupVersionResource, fn Func) { - // Register the entry so a Webhook config is created - e.Entries[path] = admissionHandler{gvr, fn} - - // Register the handler path - e.SMux.Handle(path, httpHandler{fn}) -} - -// HandleFunc registers fn as an admission control webhook callback for the group,version,resources specified -func HandleFunc(path string, gvr metav1.GroupVersionResource, fn Func) { - DefaultAdmissionFns.HandleFunc(path, gvr, fn) -} - -// ListenAndServeTLS starts the admission HttpServer. -func ListenAndServeTLS(addr string) error { - server := &http.Server{ - Addr: addr, - TLSConfig: nil, // TODO: Set this - } - return server.ListenAndServeTLS("", "") -} diff --git a/pkg/internal/admission/http.go b/pkg/internal/admission/http.go deleted file mode 100644 index 49ddb4ad16..0000000000 --- a/pkg/internal/admission/http.go +++ /dev/null @@ -1,80 +0,0 @@ -/* -Copyright 2017 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 admission - -import ( - "encoding/json" - "io/ioutil" - "net/http" - - "k8s.io/api/admission/v1beta1" - "k8s.io/apimachinery/pkg/runtime" - logf "sigs.k8s.io/controller-runtime/pkg/internal/log" -) - -var ( - // TODO(directxman12): this shouldn't be a global log - log = logf.RuntimeLog.WithName("admission").WithName("http-handler") -) - -func (h httpHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { - var body []byte - if r.Body != nil { - if data, err := ioutil.ReadAll(r.Body); err == nil { - body = data - } - } - - // verify the content type is accurate - contentType := r.Header.Get("Content-Type") - if contentType != "application/json" { - log.Error(nil, "invalid content type, expected application/json", "content type", contentType) - return - } - - var reviewResponse *v1beta1.AdmissionResponse - ar := v1beta1.AdmissionReview{} - deserializer := codecs.UniversalDeserializer() - if _, _, err := deserializer.Decode(body, nil, &ar); err != nil { - log.Error(err, "unable to decode request body") - reviewResponse = ErrorResponse(err) - } else { - reviewResponse = h.admit(ar) - } - - response := v1beta1.AdmissionReview{} - if reviewResponse != nil { - response.Response = reviewResponse - response.Response.UID = ar.Request.UID - } - // reset the Object and OldObject, they are not needed in a response. - ar.Request.Object = runtime.RawExtension{} - ar.Request.OldObject = runtime.RawExtension{} - - resp, err := json.Marshal(response) - if err != nil { - log.Error(err, "unable to marshal response") - return - } - if _, err := w.Write(resp); err != nil { - log.Error(err, "unable to write response") - } -} - -type httpHandler struct { - admit Func -} diff --git a/pkg/internal/admission/response.go b/pkg/internal/admission/response.go deleted file mode 100644 index 24dc32d9b3..0000000000 --- a/pkg/internal/admission/response.go +++ /dev/null @@ -1,48 +0,0 @@ -/* -Copyright 2017 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 admission - -import ( - "k8s.io/api/admission/v1beta1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" -) - -// ErrorResponse creates a new AdmissionResponse for an error handling the request -func ErrorResponse(err error) *v1beta1.AdmissionResponse { - return &v1beta1.AdmissionResponse{ - Result: &metav1.Status{ - Message: err.Error(), - }, - } -} - -// DenyResponse returns a new response for denying a request -func DenyResponse(msg string) *v1beta1.AdmissionResponse { - return &v1beta1.AdmissionResponse{ - Allowed: false, - Result: &metav1.Status{ - Reason: metav1.StatusReason(msg), - }, - } -} - -// AllowResponse returns a new response for admitting a request -func AllowResponse() *v1beta1.AdmissionResponse { - return &v1beta1.AdmissionResponse{ - Allowed: true, - } -} diff --git a/pkg/internal/admission/tls.go b/pkg/internal/admission/tls.go deleted file mode 100644 index ed22d249b2..0000000000 --- a/pkg/internal/admission/tls.go +++ /dev/null @@ -1,42 +0,0 @@ -/* -Copyright 2017 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 admission - -//type certs struct { -// Cert []byte -// Key []byte -// CACert []byte -//} - -//// MakeTLSConfig makes a TLS configuration suitable for use with the server -//func makeTLSConfig(certs certs) (*tls.Config, error) { -// caCertPool := x509.NewCertPool() -// caCertPool.AppendCertsFromPEM(certs.CACert) -// //cert, err := tls.X509KeyPair(certs.Cert, certs.Key) -// //if err != nil { -// // return nil, err -// //} -// return &tls.Config{ -// //Certificates: []tls.Certificate{cert}, -// ClientCAs: caCertPool, -// ClientAuth: tls.NoClientCert, -// // Note on GKE there apparently is no client cert sent, so this -// // does not work on GKE. -// // TODO: make this into a configuration option. -// // ClientAuth: tls.RequireAndVerifyClientCert, -// }, nil -//} diff --git a/pkg/internal/controller/controller.go b/pkg/internal/controller/controller.go index 18deac0992..cd19332189 100644 --- a/pkg/internal/controller/controller.go +++ b/pkg/internal/controller/controller.go @@ -31,10 +31,10 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/handler" ctrlmetrics "sigs.k8s.io/controller-runtime/pkg/internal/controller/metrics" + logf "sigs.k8s.io/controller-runtime/pkg/internal/log" "sigs.k8s.io/controller-runtime/pkg/predicate" "sigs.k8s.io/controller-runtime/pkg/reconcile" "sigs.k8s.io/controller-runtime/pkg/runtime/inject" - logf "sigs.k8s.io/controller-runtime/pkg/internal/log" "sigs.k8s.io/controller-runtime/pkg/source" ) diff --git a/pkg/internal/controller/controller_test.go b/pkg/internal/controller/controller_test.go index 0faa12c3f6..5bfb261ce3 100644 --- a/pkg/internal/controller/controller_test.go +++ b/pkg/internal/controller/controller_test.go @@ -24,7 +24,7 @@ import ( . "github.com/onsi/gomega" "github.com/prometheus/client_golang/prometheus" dto "github.com/prometheus/client_model/go" - "k8s.io/api/apps/v1" + appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/util/workqueue" @@ -62,9 +62,9 @@ var _ = Describe("controller", func() { informers = &informertest.FakeInformers{} ctrl = &Controller{ MaxConcurrentReconciles: 1, - Do: fakeReconcile, - Queue: queue, - Cache: informers, + Do: fakeReconcile, + Queue: queue, + Cache: informers, } ctrl.InjectFunc(func(interface{}) error { return nil }) }) @@ -103,8 +103,8 @@ var _ = Describe("controller", func() { c, err := cache.New(cfg, cache.Options{}) Expect(err).NotTo(HaveOccurred()) - c.GetInformer(&v1.Deployment{}) - c.GetInformer(&v1.ReplicaSet{}) + c.GetInformer(&appsv1.Deployment{}) + c.GetInformer(&appsv1.ReplicaSet{}) ctrl.Cache = c ctrl.WaitForCacheSync = func(<-chan struct{}) bool { return true } diff --git a/pkg/log/log_test.go b/pkg/log/log_test.go index 1ec5f51e0c..fa5ff23bca 100644 --- a/pkg/log/log_test.go +++ b/pkg/log/log_test.go @@ -17,7 +17,6 @@ limitations under the License. package log import ( - "github.com/go-logr/logr" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" diff --git a/pkg/log/zap/kube_helpers.go b/pkg/log/zap/kube_helpers.go index c88ba56cdc..2c0d88386d 100644 --- a/pkg/log/zap/kube_helpers.go +++ b/pkg/log/zap/kube_helpers.go @@ -125,4 +125,3 @@ func (k *KubeAwareEncoder) EncodeEntry(entry zapcore.Entry, fields []zapcore.Fie return k.Encoder.EncodeEntry(entry, fields) } - diff --git a/pkg/manager/example_test.go b/pkg/manager/example_test.go index 44a2e511bd..093e4f1f0e 100644 --- a/pkg/manager/example_test.go +++ b/pkg/manager/example_test.go @@ -20,8 +20,8 @@ import ( "os" "sigs.k8s.io/controller-runtime/pkg/client/config" - "sigs.k8s.io/controller-runtime/pkg/manager" logf "sigs.k8s.io/controller-runtime/pkg/log" + "sigs.k8s.io/controller-runtime/pkg/manager" "sigs.k8s.io/controller-runtime/pkg/manager/signals" ) diff --git a/pkg/manager/internal.go b/pkg/manager/internal.go index 716702c5ca..230b78f89c 100644 --- a/pkg/manager/internal.go +++ b/pkg/manager/internal.go @@ -33,11 +33,10 @@ import ( "k8s.io/client-go/tools/record" "sigs.k8s.io/controller-runtime/pkg/cache" "sigs.k8s.io/controller-runtime/pkg/client" + logf "sigs.k8s.io/controller-runtime/pkg/internal/log" "sigs.k8s.io/controller-runtime/pkg/metrics" "sigs.k8s.io/controller-runtime/pkg/recorder" "sigs.k8s.io/controller-runtime/pkg/runtime/inject" - logf "sigs.k8s.io/controller-runtime/pkg/internal/log" - "sigs.k8s.io/controller-runtime/pkg/webhook/admission/types" ) var log = logf.RuntimeLog.WithName("manager") @@ -49,8 +48,6 @@ type controllerManager struct { // scheme is the scheme injected into Controllers, EventHandlers, Sources and Predicates. Defaults // to scheme.scheme. scheme *runtime.Scheme - // admissionDecoder is used to decode an admission.Request. - admissionDecoder types.Decoder // runnables is the set of Controllers that the controllerManager injects deps into and Starts. runnables []Runnable @@ -136,9 +133,6 @@ func (cm *controllerManager) SetFields(i interface{}) error { if _, err := inject.StopChannelInto(cm.internalStop, i); err != nil { return err } - if _, err := inject.DecoderInto(cm.admissionDecoder, i); err != nil { - return err - } if _, err := inject.MapperInto(cm.mapper, i); err != nil { return err } @@ -157,10 +151,6 @@ func (cm *controllerManager) GetScheme() *runtime.Scheme { return cm.scheme } -func (cm *controllerManager) GetAdmissionDecoder() types.Decoder { - return cm.admissionDecoder -} - func (cm *controllerManager) GetFieldIndexer() client.FieldIndexer { return cm.fieldIndexes } diff --git a/pkg/manager/manager.go b/pkg/manager/manager.go index ea25e9f509..633091eb78 100644 --- a/pkg/manager/manager.go +++ b/pkg/manager/manager.go @@ -36,8 +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/admission" - "sigs.k8s.io/controller-runtime/pkg/webhook/admission/types" ) // Manager initializes shared dependencies such as Caches and Clients, and provides them to Runnables. @@ -62,9 +60,6 @@ type Manager interface { // GetScheme returns an initialized Scheme GetScheme() *runtime.Scheme - // GetAdmissionDecoder returns the runtime.Decoder based on the scheme. - GetAdmissionDecoder() types.Decoder - // GetClient returns a client configured with the Config GetClient() client.Client @@ -135,7 +130,6 @@ type Options struct { // Dependency injection for testing newRecorderProvider func(config *rest.Config, scheme *runtime.Scheme, logger logr.Logger) (recorder.Provider, error) newResourceLock func(config *rest.Config, recorderProvider recorder.Provider, options leaderelection.Options) (resourcelock.Interface, error) - newAdmissionDecoder func(scheme *runtime.Scheme) (types.Decoder, error) newMetricsListener func(addr string) (net.Listener, error) } @@ -210,11 +204,6 @@ func New(config *rest.Config, options Options) (Manager, error) { return nil, err } - admissionDecoder, err := options.newAdmissionDecoder(options.Scheme) - if err != nil { - return nil, err - } - // Create the mertics listener. This will throw an error if the metrics bind // address is invalid or already in use. metricsListener, err := options.newMetricsListener(options.MetricsBindAddress) @@ -227,7 +216,6 @@ func New(config *rest.Config, options Options) (Manager, error) { return &controllerManager{ config: config, scheme: options.Scheme, - admissionDecoder: admissionDecoder, errChan: make(chan error), cache: cache, fieldIndexes: cache, @@ -290,10 +278,6 @@ func setOptionsDefaults(options Options) Options { options.newResourceLock = leaderelection.NewResourceLock } - if options.newAdmissionDecoder == nil { - options.newAdmissionDecoder = admission.NewDecoder - } - if options.newMetricsListener == nil { options.newMetricsListener = metrics.NewListener } diff --git a/pkg/manager/manager_suite_test.go b/pkg/manager/manager_suite_test.go index a1183f66c0..b544fa8ae6 100644 --- a/pkg/manager/manager_suite_test.go +++ b/pkg/manager/manager_suite_test.go @@ -24,9 +24,9 @@ import ( "k8s.io/client-go/kubernetes" "k8s.io/client-go/rest" "sigs.k8s.io/controller-runtime/pkg/envtest" - "sigs.k8s.io/controller-runtime/pkg/metrics" logf "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/log/zap" + "sigs.k8s.io/controller-runtime/pkg/metrics" ) func TestSource(t *testing.T) { diff --git a/pkg/metrics/client_go_adapter.go b/pkg/metrics/client_go_adapter.go index d02c56356f..5dfc120397 100644 --- a/pkg/metrics/client_go_adapter.go +++ b/pkg/metrics/client_go_adapter.go @@ -110,35 +110,35 @@ var ( workQueueSubsystem = "workqueue" depth = prometheus.NewGaugeVec(prometheus.GaugeOpts{ - Subsystem: workQueueSubsystem, - Name: "depth", - Help: "Current depth of workqueue", + Subsystem: workQueueSubsystem, + Name: "depth", + Help: "Current depth of workqueue", }, []string{"name"}) adds = prometheus.NewCounterVec(prometheus.CounterOpts{ - Subsystem: workQueueSubsystem, - Name: "adds_total", - Help: "Total number of adds handled by workqueue", + Subsystem: workQueueSubsystem, + Name: "adds_total", + Help: "Total number of adds handled by workqueue", }, []string{"name"}) latency = prometheus.NewHistogramVec(prometheus.HistogramOpts{ - Subsystem: workQueueSubsystem, - Name: "queue_latency_seconds", - Help: "How long in seconds an item stays in workqueue before being requested.", - Buckets: prometheus.ExponentialBuckets(10e-9, 10, 10), + Subsystem: workQueueSubsystem, + Name: "queue_latency_seconds", + Help: "How long in seconds an item stays in workqueue before being requested.", + Buckets: prometheus.ExponentialBuckets(10e-9, 10, 10), }, []string{"name"}) workDuration = prometheus.NewHistogramVec(prometheus.HistogramOpts{ - Subsystem: workQueueSubsystem, - Name: "work_duration_seconds", - Help: "How long in seconds processing an item from workqueue takes.", - Buckets: prometheus.ExponentialBuckets(10e-9, 10, 10), + Subsystem: workQueueSubsystem, + Name: "work_duration_seconds", + Help: "How long in seconds processing an item from workqueue takes.", + Buckets: prometheus.ExponentialBuckets(10e-9, 10, 10), }, []string{"name"}) retries = prometheus.NewCounterVec(prometheus.CounterOpts{ - Subsystem: workQueueSubsystem, - Name: "retries_total", - Help: "Total number of retries handled by workqueue", + Subsystem: workQueueSubsystem, + Name: "retries_total", + Help: "Total number of retries handled by workqueue", }, []string{"name"}) longestRunning = prometheus.NewGaugeVec(prometheus.GaugeOpts{ diff --git a/pkg/recorder/example_test.go b/pkg/recorder/example_test.go index 3066c1c269..c900423d77 100644 --- a/pkg/recorder/example_test.go +++ b/pkg/recorder/example_test.go @@ -27,7 +27,7 @@ import ( var ( recorderProvider recorder.Provider - somePod *corev1.Pod // the object you're reconciling, for example + somePod *corev1.Pod // the object you're reconciling, for example ) func Example_event() { @@ -54,7 +54,7 @@ func Example_pastEventf() { recorder := recorderProvider.GetEventRecorderFor("my-controller") // emit a backdated event (potentially with variable message) - recorder.PastEventf(somePod, metav1.Time{Time: time.Now().Add(-5*time.Minute)}, + recorder.PastEventf(somePod, metav1.Time{Time: time.Now().Add(-5 * time.Minute)}, corev1.EventTypeWarning, "ForgottenCrackers", "Crackers, Gromit! We forgot the crackers!") } diff --git a/pkg/runtime/inject/inject.go b/pkg/runtime/inject/inject.go index 7d1fbd4a6d..c006fdb53d 100644 --- a/pkg/runtime/inject/inject.go +++ b/pkg/runtime/inject/inject.go @@ -17,12 +17,13 @@ limitations under the License. package inject import ( + "github.com/go-logr/logr" "k8s.io/apimachinery/pkg/api/meta" "k8s.io/apimachinery/pkg/runtime" "k8s.io/client-go/rest" + "sigs.k8s.io/controller-runtime/pkg/cache" "sigs.k8s.io/controller-runtime/pkg/client" - "sigs.k8s.io/controller-runtime/pkg/webhook/admission/types" ) // Cache is used by the ControllerManager to inject Cache into Sources, EventHandlers, Predicates, and @@ -70,20 +71,6 @@ func ClientInto(client client.Client, i interface{}) (bool, error) { return false, nil } -// Decoder is used by the ControllerManager to inject decoder into webhook handlers. -type Decoder interface { - InjectDecoder(types.Decoder) error -} - -// DecoderInto will set decoder on i and return the result if it implements Decoder. Returns -// false if i does not implement Decoder. -func DecoderInto(decoder types.Decoder, i interface{}) (bool, error) { - if s, ok := i.(Decoder); ok { - return true, s.InjectDecoder(decoder) - } - return false, nil -} - // Scheme is used by the ControllerManager to inject Scheme into Sources, EventHandlers, Predicates, and // Reconciles type Scheme interface { @@ -144,3 +131,18 @@ func InjectorInto(f Func, i interface{}) (bool, error) { } return false, nil } + +// Logger is used to inject Loggers into components that need them +// and don't otherwise have opinions. +type Logger interface { + InjectLogger(l logr.Logger) error +} + +// LoggerInto will set the logger on the given object if it implements inject.Logger, +// returning true if a InjectLogger was called, and false otherwise. +func LoggerInto(l logr.Logger, i interface{}) (bool, error) { + if injectable, wantsLogger := i.(Logger); wantsLogger { + return true, injectable.InjectLogger(l) + } + return false, nil +} diff --git a/pkg/runtime/scheme/scheme.go b/pkg/runtime/scheme/scheme.go index a1d00141b0..889308c1e0 100644 --- a/pkg/runtime/scheme/scheme.go +++ b/pkg/runtime/scheme/scheme.go @@ -20,4 +20,3 @@ limitations under the License. // // Deprecated: use pkg/scheme instead. package scheme - diff --git a/pkg/runtime/signals/signal.go b/pkg/runtime/signals/signal.go index 2b25fe73dd..64bbcef368 100644 --- a/pkg/runtime/signals/signal.go +++ b/pkg/runtime/signals/signal.go @@ -17,7 +17,7 @@ limitations under the License. // Package signals contains libraries for handling signals to gracefully // shutdown the manager in combination with Kubernetes pod graceful termination // policy. -// +// // Deprecated: use pkg/manager/signals instead. package signals diff --git a/pkg/source/example_test.go b/pkg/source/example_test.go index 01a17f89f5..afc40f50d8 100644 --- a/pkg/source/example_test.go +++ b/pkg/source/example_test.go @@ -17,7 +17,7 @@ limitations under the License. package source_test import ( - "k8s.io/api/core/v1" + corev1 "k8s.io/api/core/v1" "sigs.k8s.io/controller-runtime/pkg/controller" "sigs.k8s.io/controller-runtime/pkg/event" "sigs.k8s.io/controller-runtime/pkg/handler" @@ -29,7 +29,7 @@ var ctrl controller.Controller // This example Watches for Pod Events (e.g. Create / Update / Delete) and enqueues a reconcile.Request // with the Name and Namespace of the Pod. func ExampleKind() { - ctrl.Watch(&source.Kind{Type: &v1.Pod{}}, &handler.EnqueueRequestForObject{}) + ctrl.Watch(&source.Kind{Type: &corev1.Pod{}}, &handler.EnqueueRequestForObject{}) } // This example reads GenericEvents from a channel and enqueues a reconcile.Request containing the Name and Namespace diff --git a/pkg/source/source.go b/pkg/source/source.go index 3375616e1d..3d27bc4334 100644 --- a/pkg/source/source.go +++ b/pkg/source/source.go @@ -25,8 +25,8 @@ import ( "k8s.io/client-go/util/workqueue" "sigs.k8s.io/controller-runtime/pkg/event" "sigs.k8s.io/controller-runtime/pkg/handler" - "sigs.k8s.io/controller-runtime/pkg/runtime/inject" logf "sigs.k8s.io/controller-runtime/pkg/internal/log" + "sigs.k8s.io/controller-runtime/pkg/runtime/inject" "sigs.k8s.io/controller-runtime/pkg/source/internal" toolscache "k8s.io/client-go/tools/cache" diff --git a/pkg/webhook/admission/builder/builder.go b/pkg/webhook/admission/builder/builder.go deleted file mode 100644 index 4f7e12e20a..0000000000 --- a/pkg/webhook/admission/builder/builder.go +++ /dev/null @@ -1,101 +0,0 @@ -/* -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 builder - -import ( - "sigs.k8s.io/controller-runtime/pkg/webhook/admission" - "sigs.k8s.io/controller-runtime/pkg/webhook/types" -) - -// WebhookBuilder builds a webhook based on the provided options. -type WebhookBuilder struct { - // name specifies the name of the webhook. It must be unique among all webhooks. - name string - - // path is the URL Path to register this webhook. e.g. "/mutate-pods". - path string - - // handlers handle admission requests. - // A WebhookBuilder may have multiple handlers. - // For example, handlers[0] mutates a pod for feature foo. - // handlers[1] mutates a pod for a different feature bar. - handlers []admission.Handler - - // t specifies the type of the webhook. - // Currently, Mutating and Validating are supported. - t *types.WebhookType -} - -// NewWebhookBuilder creates an empty WebhookBuilder. -func NewWebhookBuilder() *WebhookBuilder { - return &WebhookBuilder{} -} - -// Name sets the name of the webhook. -// This is optional -func (b *WebhookBuilder) Name(name string) *WebhookBuilder { - b.name = name - return b -} - -// Mutating sets the type to mutating admission webhook -// Only one of Mutating and Validating can be invoked. -func (b *WebhookBuilder) Mutating() *WebhookBuilder { - m := types.WebhookTypeMutating - b.t = &m - return b -} - -// Validating sets the type to validating admission webhook -// Only one of Mutating and Validating can be invoked. -func (b *WebhookBuilder) Validating() *WebhookBuilder { - m := types.WebhookTypeValidating - b.t = &m - return b -} - -// Path sets the path for the webhook. -// Path needs to be unique among different webhooks. -// This is required. If not set, it will be built from the type and resource name. -// For example, a webhook that mutates pods has a default path of "/mutate-pods" -// If the defaulting logic can't find a unique path for it, user need to set it manually. -func (b *WebhookBuilder) Path(path string) *WebhookBuilder { - b.path = path - return b -} - -// Handlers sets the handlers of the webhook. -func (b *WebhookBuilder) Handlers(handlers ...admission.Handler) *WebhookBuilder { - b.handlers = handlers - return b -} - -// Build creates the Webhook based on the options provided. -func (b *WebhookBuilder) Build() *admission.Webhook { - if b.t == nil { - b.Mutating() - } - - w := &admission.Webhook{ - Name: b.name, - Type: *b.t, - Path: b.path, - Handlers: b.handlers, - } - - return w -} diff --git a/pkg/webhook/admission/builder/doc.go b/pkg/webhook/admission/builder/doc.go deleted file mode 100644 index 9ef8d65fa0..0000000000 --- a/pkg/webhook/admission/builder/doc.go +++ /dev/null @@ -1,58 +0,0 @@ -/* -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 builder provides methods to build admission webhooks. - -The following are 2 examples for building mutating webhook and validating webhook. - - webhook1 := NewWebhookBuilder(). - Mutating(). - Path("/mutatepods"). - ForType(&corev1.Pod{}). - Handlers(mutatingHandler11, mutatingHandler12). - Build() - - webhook2 := NewWebhookBuilder(). - Validating(). - Path("/validatepods"). - ForType(&appsv1.Deployment{}). - Handlers(validatingHandler21). - Build() - -Note: To build a webhook for a CRD, you need to ensure the manager uses the scheme that understands your CRD. -This is necessary, because if the scheme doesn't understand your CRD types, the decoder won't be able to decode -the CR object from the admission review request. - -The following snippet shows how to register CRD types with manager's scheme. - - mgr, err := manager.New(cfg, manager.Options{}) - if err != nil { - // handle error - } - // SchemeGroupVersion is group version used to register these objects - SchemeGroupVersion = schema.GroupVersion{Group: "crew.k8s.io", Version: "v1"} - // SchemeBuilder is used to add go types to the GroupVersionKind scheme - SchemeBuilder = &scheme.Builder{GroupVersion: SchemeGroupVersion} - // Register your CRD types. - SchemeBuilder.Register(&Kraken{}, &KrakenList{}) - // Register your CRD types with the manager's scheme. - err = SchemeBuilder.AddToScheme(mgr.GetScheme()) - if err != nil { - // handle error - } -*/ -package builder diff --git a/pkg/webhook/admission/decode.go b/pkg/webhook/admission/decode.go index 1aea214cfd..ced04ed060 100644 --- a/pkg/webhook/admission/decode.go +++ b/pkg/webhook/admission/decode.go @@ -17,32 +17,41 @@ limitations under the License. package admission import ( + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/serializer" - "sigs.k8s.io/controller-runtime/pkg/webhook/admission/types" + "k8s.io/apimachinery/pkg/util/json" ) -// DecodeFunc is a function that implements the Decoder interface. -type DecodeFunc func(types.Request, runtime.Object) error - -var _ types.Decoder = DecodeFunc(nil) - -// Decode implements the Decoder interface. -func (f DecodeFunc) Decode(req types.Request, obj runtime.Object) error { - return f(req, obj) -} - -type decoder struct { +// Decoder knows how to decode the contents of an admission +// request into a concrete object. +type Decoder struct { codecs serializer.CodecFactory } // NewDecoder creates a Decoder given the runtime.Scheme -func NewDecoder(scheme *runtime.Scheme) (types.Decoder, error) { - return decoder{codecs: serializer.NewCodecFactory(scheme)}, nil +func NewDecoder(scheme *runtime.Scheme) (*Decoder, error) { + return &Decoder{codecs: serializer.NewCodecFactory(scheme)}, nil } // Decode decodes the inlined object in the AdmissionRequest into the passed-in runtime.Object. -func (d decoder) Decode(req types.Request, into runtime.Object) error { +func (d *Decoder) Decode(req Request, into runtime.Object) error { + // NB(directxman12): there's a bug/weird interaction between decoders and + // the API server where the API server doesn't send a GVK on the embedded + // objects, which means the unstructured decoder refuses to decode. It + // also means we can't pass the unstructured directly in, since it'll try + // and call unstructured's special Unmarshal implementation, which calls + // back into that same decoder :-/ + // See kubernetes/kubernetes#74373. + if unstructuredInto, isUnstructured := into.(*unstructured.Unstructured); isUnstructured { + // unmarshal into unstructured's underlying object to avoid calling the decoder + if err := json.Unmarshal(req.Object.Raw, &unstructuredInto.Object); err != nil { + return err + } + + return nil + } + deserializer := d.codecs.UniversalDeserializer() return runtime.DecodeInto(deserializer, req.AdmissionRequest.Object.Raw, into) } diff --git a/pkg/webhook/admission/decode_test.go b/pkg/webhook/admission/decode_test.go index d0d38c7cb0..efba146d09 100644 --- a/pkg/webhook/admission/decode_test.go +++ b/pkg/webhook/admission/decode_test.go @@ -22,34 +22,26 @@ import ( admissionv1beta1 "k8s.io/api/admission/v1beta1" corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" "k8s.io/client-go/kubernetes/scheme" - "sigs.k8s.io/controller-runtime/pkg/webhook/admission/types" ) -var _ = Describe("admission webhook decoder", func() { - var decoder types.Decoder - BeforeEach(func(done Done) { +var _ = Describe("Admission Webhook Decoder", func() { + var decoder *Decoder + BeforeEach(func() { + By("creating a new decoder for a scheme") var err error decoder, err = NewDecoder(scheme.Scheme) Expect(err).NotTo(HaveOccurred()) Expect(decoder).NotTo(BeNil()) - close(done) }) - Describe("NewDecoder", func() { - It("should return a decoder without an error", func() { - decoder, err := NewDecoder(scheme.Scheme) - Expect(err).NotTo(HaveOccurred()) - Expect(decoder).NotTo(BeNil()) - }) - }) - - Describe("Decode", func() { - req := types.Request{ - AdmissionRequest: &admissionv1beta1.AdmissionRequest{ - Object: runtime.RawExtension{ - Raw: []byte(`{ + req := Request{ + AdmissionRequest: admissionv1beta1.AdmissionRequest{ + Object: runtime.RawExtension{ + Raw: []byte(`{ "apiVersion": "v1", "kind": "Pod", "metadata": { @@ -65,19 +57,47 @@ 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()) + + 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", 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") + Expect(decoder.Decode(req, &corev1.Node{})).NotTo(Succeed()) + }) - It("should be able to decode", func() { - err := decoder.Decode(req, &corev1.Pod{}) - Expect(err).NotTo(HaveOccurred()) - }) + 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()) - It("should return an error if the GVK mismatch", func() { - err := decoder.Decode(req, &corev1.Node{}) - Expect(err).To(HaveOccurred()) - Expect(err.Error()).Should(ContainSubstring("unable to decode")) - }) + By("sanity-checking the metadata on the output object") + Expect(target.Object["metadata"]).To(Equal(map[string]interface{}{ + "name": "foo", + "namespace": "default", + })) }) }) diff --git a/pkg/webhook/admission/doc.go b/pkg/webhook/admission/doc.go index 0126aad03a..0b274dd02b 100644 --- a/pkg/webhook/admission/doc.go +++ b/pkg/webhook/admission/doc.go @@ -17,80 +17,7 @@ limitations under the License. /* Package admission provides implementation for admission webhook and methods to implement admission webhook handlers. -The following snippet is an example implementation of mutating handler. - - type Mutator struct { - client client.Client - decoder types.Decoder - } - - func (m *Mutator) mutatePodsFn(ctx context.Context, pod *corev1.Pod) error { - // your logic to mutate the passed-in pod. - } - - func (m *Mutator) Handle(ctx context.Context, req types.Request) types.Response { - pod := &corev1.Pod{} - err := m.decoder.Decode(req, pod) - if err != nil { - return admission.ErrorResponse(http.StatusBadRequest, err) - } - // Do deepcopy before actually mutate the object. - copy := pod.DeepCopy() - err = m.mutatePodsFn(ctx, copy) - if err != nil { - return admission.ErrorResponse(http.StatusInternalServerError, err) - } - return admission.PatchResponse(pod, copy) - } - - // InjectClient is called by the Manager and provides a client.Client to the Mutator instance. - func (m *Mutator) InjectClient(c client.Client) error { - h.client = c - return nil - } - - // InjectDecoder is called by the Manager and provides a types.Decoder to the Mutator instance. - func (m *Mutator) InjectDecoder(d types.Decoder) error { - h.decoder = d - return nil - } - -The following snippet is an example implementation of validating handler. - - type Handler struct { - client client.Client - decoder types.Decoder - } - - func (v *Validator) validatePodsFn(ctx context.Context, pod *corev1.Pod) (bool, string, error) { - // your business logic - } - - func (v *Validator) Handle(ctx context.Context, req types.Request) types.Response { - pod := &corev1.Pod{} - err := h.decoder.Decode(req, pod) - if err != nil { - return admission.ErrorResponse(http.StatusBadRequest, err) - } - - allowed, reason, err := h.validatePodsFn(ctx, pod) - if err != nil { - return admission.ErrorResponse(http.StatusInternalServerError, err) - } - return admission.ValidationResponse(allowed, reason) - } - - // InjectClient is called by the Manager and provides a client.Client to the Validator instance. - func (v *Validator) InjectClient(c client.Client) error { - h.client = c - return nil - } - - // InjectDecoder is called by the Manager and provides a types.Decoder to the Validator instance. - func (v *Validator) InjectDecoder(d types.Decoder) error { - h.decoder = d - return nil - } +See examples/mutatingwebhook.go and examples/validatingwebhook.go for examples of admission webhooks. */ package admission diff --git a/pkg/webhook/admission/http.go b/pkg/webhook/admission/http.go index 5b91ad2b50..fa60302adc 100644 --- a/pkg/webhook/admission/http.go +++ b/pkg/webhook/admission/http.go @@ -24,49 +24,39 @@ import ( "io" "io/ioutil" "net/http" - "time" "k8s.io/api/admission/v1beta1" admissionv1beta1 "k8s.io/api/admission/v1beta1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/serializer" utilruntime "k8s.io/apimachinery/pkg/util/runtime" - "sigs.k8s.io/controller-runtime/pkg/webhook/admission/types" - "sigs.k8s.io/controller-runtime/pkg/webhook/internal/metrics" ) -var admissionv1beta1scheme = runtime.NewScheme() -var admissionv1beta1schemecodecs = serializer.NewCodecFactory(admissionv1beta1scheme) +var admissionScheme = runtime.NewScheme() +var admissionCodecs = serializer.NewCodecFactory(admissionScheme) func init() { - addToScheme(admissionv1beta1scheme) -} - -func addToScheme(scheme *runtime.Scheme) { - utilruntime.Must(admissionv1beta1.AddToScheme(scheme)) + utilruntime.Must(admissionv1beta1.AddToScheme(admissionScheme)) } var _ http.Handler = &Webhook{} func (wh *Webhook) ServeHTTP(w http.ResponseWriter, r *http.Request) { - startTS := time.Now() - defer metrics.RequestLatency.WithLabelValues(wh.Name).Observe(time.Now().Sub(startTS).Seconds()) - var body []byte var err error - var reviewResponse types.Response + var reviewResponse Response if r.Body != nil { if body, err = ioutil.ReadAll(r.Body); err != nil { - log.Error(err, "unable to read the body from the incoming request") - reviewResponse = ErrorResponse(http.StatusBadRequest, err) + wh.log.Error(err, "unable to read the body from the incoming request") + reviewResponse = Errored(http.StatusBadRequest, err) wh.writeResponse(w, reviewResponse) return } } else { err = errors.New("request body is empty") - log.Error(err, "bad request") - reviewResponse = ErrorResponse(http.StatusBadRequest, err) + wh.log.Error(err, "bad request") + reviewResponse = Errored(http.StatusBadRequest, err) wh.writeResponse(w, reviewResponse) return } @@ -74,42 +64,38 @@ func (wh *Webhook) ServeHTTP(w http.ResponseWriter, r *http.Request) { // verify the content type is accurate contentType := r.Header.Get("Content-Type") if contentType != "application/json" { - err = fmt.Errorf("contentType=%s, expect application/json", contentType) - log.Error(err, "unable to process a request with an unknown content type", "content type", contentType) - reviewResponse = ErrorResponse(http.StatusBadRequest, err) + err = fmt.Errorf("contentType=%s, expected application/json", contentType) + wh.log.Error(err, "unable to process a request with an unknown content type", "content type", contentType) + reviewResponse = Errored(http.StatusBadRequest, err) wh.writeResponse(w, reviewResponse) return } - ar := v1beta1.AdmissionReview{} - if _, _, err := admissionv1beta1schemecodecs.UniversalDeserializer().Decode(body, nil, &ar); err != nil { - log.Error(err, "unable to decode the request") - reviewResponse = ErrorResponse(http.StatusBadRequest, err) + req := Request{} + ar := v1beta1.AdmissionReview{ + // avoid an extra copy + Request: &req.AdmissionRequest, + } + if _, _, err := admissionCodecs.UniversalDeserializer().Decode(body, nil, &ar); err != nil { + wh.log.Error(err, "unable to decode the request") + reviewResponse = Errored(http.StatusBadRequest, err) wh.writeResponse(w, reviewResponse) return } // TODO: add panic-recovery for Handle - reviewResponse = wh.Handle(context.Background(), types.Request{AdmissionRequest: ar.Request}) + reviewResponse = wh.Handle(context.Background(), req) wh.writeResponse(w, reviewResponse) } -func (wh *Webhook) writeResponse(w io.Writer, response types.Response) { - if response.Response.Result.Code != 0 { - if response.Response.Result.Code == http.StatusOK { - metrics.TotalRequests.WithLabelValues(wh.Name, "true").Inc() - } else { - metrics.TotalRequests.WithLabelValues(wh.Name, "false").Inc() - } - } - +func (wh *Webhook) writeResponse(w io.Writer, response Response) { encoder := json.NewEncoder(w) responseAdmissionReview := v1beta1.AdmissionReview{ - Response: response.Response, + Response: &response.AdmissionResponse, } err := encoder.Encode(responseAdmissionReview) if err != nil { - log.Error(err, "unable to encode the response") - wh.writeResponse(w, ErrorResponse(http.StatusInternalServerError, err)) + wh.log.Error(err, "unable to encode the response") + wh.writeResponse(w, Errored(http.StatusInternalServerError, err)) } } diff --git a/pkg/webhook/admission/http_test.go b/pkg/webhook/admission/http_test.go index 8d3c0507b4..975115695f 100644 --- a/pkg/webhook/admission/http_test.go +++ b/pkg/webhook/admission/http_test.go @@ -26,120 +26,73 @@ import ( . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" + "sigs.k8s.io/controller-runtime/pkg/runtime/inject" + admissionv1beta1 "k8s.io/api/admission/v1beta1" - atypes "sigs.k8s.io/controller-runtime/pkg/webhook/admission/types" - "sigs.k8s.io/controller-runtime/pkg/webhook/types" ) -var _ = Describe("admission webhook http handler", func() { - var w *httptest.ResponseRecorder - BeforeEach(func(done Done) { - w = &httptest.ResponseRecorder{ - Body: bytes.NewBuffer(nil), - } - close(done) - }) +var _ = Describe("Admission Webhooks", func() { - Describe("empty request body", func() { - req := &http.Request{Body: nil} - wh := &Webhook{ - Handlers: []Handler{}, + Describe("HTTP Handler", func() { + var respRecorder *httptest.ResponseRecorder + webhook := &Webhook{ + Handler: nil, } - - expected := []byte(`{"response":{"uid":"","allowed":false,"status":{"metadata":{},"message":"request body is empty","code":400}}} -`) - It("should return an error with bad-request status code", func() { - wh.ServeHTTP(w, req) - Expect(w.Body.Bytes()).To(Equal(expected)) + BeforeEach(func() { + respRecorder = &httptest.ResponseRecorder{ + Body: bytes.NewBuffer(nil), + } + _, err := inject.LoggerInto(log.WithName("test-webhook"), webhook) + Expect(err).NotTo(HaveOccurred()) }) - }) - - Describe("wrong content type", func() { - req := &http.Request{ - Header: http.Header{"Content-Type": []string{"application/foo"}}, - Body: nopCloser{Reader: bytes.NewBuffer(nil)}, - } - wh := &Webhook{ - Handlers: []Handler{}, - } - expected := []byte(`{"response":{"uid":"","allowed":false,"status":{"metadata":{},"message":"contentType=application/foo, expect application/json","code":400}}} -`) - It("should return an error with bad-request status code", func() { - wh.ServeHTTP(w, req) - Expect(w.Body.Bytes()).To(Equal(expected)) - }) - }) + It("should return bad-request when given an empty body", func() { + req := &http.Request{Body: nil} - Describe("can't decode body", func() { - req := &http.Request{ - Header: http.Header{"Content-Type": []string{"application/json"}}, - Body: nopCloser{Reader: bytes.NewBufferString("{")}, - } - wh := &Webhook{ - Type: types.WebhookTypeMutating, - Handlers: []Handler{}, - } - expected := []byte( - `{"response":{"uid":"","allowed":false,"status":{"metadata":{},"message":"couldn't get version/kind; json parse error: unexpected end of JSON input","code":400}}} + expected := []byte(`{"response":{"uid":"","allowed":false,"status":{"metadata":{},"message":"request body is empty","code":400}}} `) - It("should return an error with bad-request status code", func() { - wh.ServeHTTP(w, req) - Expect(w.Body.Bytes()).To(Equal(expected)) - + webhook.ServeHTTP(respRecorder, req) + Expect(respRecorder.Body.Bytes()).To(Equal(expected)) }) - }) - Describe("empty body after decoding", func() { - req := &http.Request{ - Header: http.Header{"Content-Type": []string{"application/json"}}, - Body: nopCloser{Reader: bytes.NewBuffer(nil)}, - } - wh := &Webhook{ - Type: types.WebhookTypeMutating, - Handlers: []Handler{}, - } - expected := []byte(`{"response":{"uid":"","allowed":false,"status":{"metadata":{},"message":"got an empty AdmissionRequest","code":400}}} + It("should return bad-request when given the wrong content-type", func() { + req := &http.Request{ + Header: http.Header{"Content-Type": []string{"application/foo"}}, + Body: nopCloser{Reader: bytes.NewBuffer(nil)}, + } + + expected := []byte(`{"response":{"uid":"","allowed":false,"status":{"metadata":{},"message":"contentType=application/foo, expected application/json","code":400}}} `) - It("should return an error with bad-request status code", func() { - wh.ServeHTTP(w, req) - Expect(w.Body.Bytes()).To(Equal(expected)) + webhook.ServeHTTP(respRecorder, req) + Expect(respRecorder.Body.Bytes()).To(Equal(expected)) }) - }) - Describe("no webhook type", func() { - req := &http.Request{ - Header: http.Header{"Content-Type": []string{"application/json"}}, - Body: nopCloser{Reader: bytes.NewBufferString(`{"request":{}}`)}, - } - wh := &Webhook{ - Handlers: []Handler{}, - } - expected := []byte(`{"response":{"uid":"","allowed":false,"status":{"metadata":{},"message":"you must specify your webhook type","code":500}}} -`) - It("should return an error with internal-error status code", func() { - wh.ServeHTTP(w, req) - Expect(w.Body.Bytes()).To(Equal(expected)) + It("should return bad-request when given an undecodable body", func() { + req := &http.Request{ + Header: http.Header{"Content-Type": []string{"application/json"}}, + Body: nopCloser{Reader: bytes.NewBufferString("{")}, + } + expected := []byte( + `{"response":{"uid":"","allowed":false,"status":{"metadata":{},"message":"couldn't get version/kind; json parse error: unexpected end of JSON input","code":400}}} +`) + webhook.ServeHTTP(respRecorder, req) + Expect(respRecorder.Body.Bytes()).To(Equal(expected)) }) - }) - Describe("handler can be invoked", func() { - req := &http.Request{ - Header: http.Header{"Content-Type": []string{"application/json"}}, - Body: nopCloser{Reader: bytes.NewBufferString(`{"request":{}}`)}, - } - h := &fakeHandler{} - wh := &Webhook{ - Type: types.WebhookTypeValidating, - Handlers: []Handler{h}, - } - expected := []byte(`{"response":{"uid":"","allowed":true,"status":{"metadata":{},"code":200}}} + It("should return the response given by the handler", func() { + req := &http.Request{ + Header: http.Header{"Content-Type": []string{"application/json"}}, + Body: nopCloser{Reader: bytes.NewBufferString(`{"request":{}}`)}, + } + webhook := &Webhook{ + Handler: &fakeHandler{}, + } + + expected := []byte(`{"response":{"uid":"","allowed":true,"status":{"metadata":{},"code":200}}} `) - It("should return a response successfully", func() { - wh.ServeHTTP(w, req) - Expect(w.Body.Bytes()).To(Equal(expected)) - Expect(h.invoked).To(BeTrue()) + webhook.ServeHTTP(respRecorder, req) + Expect(respRecorder.Body.Bytes()).To(Equal(expected)) }) }) }) @@ -151,16 +104,28 @@ type nopCloser struct { func (nopCloser) Close() error { return nil } type fakeHandler struct { - invoked bool - fn func(context.Context, atypes.Request) atypes.Response + invoked bool + fn func(context.Context, Request) Response + decoder *Decoder + injectedString string +} + +func (h *fakeHandler) InjectDecoder(d *Decoder) error { + h.decoder = d + return nil +} + +func (h *fakeHandler) InjectString(s string) error { + h.injectedString = s + return nil } -func (h *fakeHandler) Handle(ctx context.Context, req atypes.Request) atypes.Response { +func (h *fakeHandler) Handle(ctx context.Context, req Request) Response { h.invoked = true if h.fn != nil { return h.fn(ctx, req) } - return atypes.Response{Response: &admissionv1beta1.AdmissionResponse{ + return Response{AdmissionResponse: admissionv1beta1.AdmissionResponse{ Allowed: true, }} } diff --git a/pkg/internal/admission/doc.go b/pkg/webhook/admission/inject.go similarity index 51% rename from pkg/internal/admission/doc.go rename to pkg/webhook/admission/inject.go index 841ed3a332..d5af0d598f 100644 --- a/pkg/internal/admission/doc.go +++ b/pkg/webhook/admission/inject.go @@ -1,5 +1,5 @@ /* -Copyright 2017 The Kubernetes Authors. +Copyright 2019 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. @@ -14,5 +14,18 @@ See the License for the specific language governing permissions and limitations under the License. */ -// Package admission provides libraries for creating admission webhooks. package admission + +// DecoderInjector is used by the ControllerManager to inject decoder into webhook handlers. +type DecoderInjector interface { + InjectDecoder(*Decoder) error +} + +// InjectDecoderInto will set decoder on i and return the result if it implements Decoder. Returns +// false if i does not implement Decoder. +func InjectDecoderInto(decoder *Decoder, i interface{}) (bool, error) { + if s, ok := i.(DecoderInjector); ok { + return true, s.InjectDecoder(decoder) + } + return false, nil +} diff --git a/pkg/webhook/admission/multi.go b/pkg/webhook/admission/multi.go new file mode 100644 index 0000000000..a65be69f68 --- /dev/null +++ b/pkg/webhook/admission/multi.go @@ -0,0 +1,126 @@ +/* +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 admission + +import ( + "context" + "encoding/json" + "fmt" + "net/http" + + "github.com/appscode/jsonpatch" + admissionv1beta1 "k8s.io/api/admission/v1beta1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/runtime/inject" +) + +type multiMutating []Handler + +func (hs multiMutating) Handle(ctx context.Context, req Request) Response { + patches := []jsonpatch.JsonPatchOperation{} + for _, handler := range hs { + resp := handler.Handle(ctx, req) + if !resp.Allowed { + return resp + } + if resp.PatchType != nil && *resp.PatchType != admissionv1beta1.PatchTypeJSONPatch { + return Errored(http.StatusInternalServerError, + fmt.Errorf("unexpected patch type returned by the handler: %v, only allow: %v", + resp.PatchType, admissionv1beta1.PatchTypeJSONPatch)) + } + patches = append(patches, resp.Patches...) + } + var err error + marshaledPatch, err := json.Marshal(patches) + if err != nil { + return Errored(http.StatusBadRequest, fmt.Errorf("error when marshaling the patch: %v", err)) + } + return Response{ + AdmissionResponse: admissionv1beta1.AdmissionResponse{ + Allowed: true, + Result: &metav1.Status{ + Code: http.StatusOK, + }, + Patch: marshaledPatch, + PatchType: func() *admissionv1beta1.PatchType { pt := admissionv1beta1.PatchTypeJSONPatch; return &pt }(), + }, + } +} + +// InjectFunc injects the field setter into the handlers. +func (hs multiMutating) InjectFunc(f inject.Func) error { + // inject directly into the handlers. It would be more correct + // to do this in a sync.Once in Handle (since we don't have some + // other start/finalize-type method), but it's more efficient to + // do it here, presumably. + for _, handler := range hs { + if err := f(handler); err != nil { + return err + } + } + + return nil +} + +// MultiMutatingHandler combines multiple mutating webhook handlers into a single +// mutating webhook handler. Handlers are called in sequential order, and the first +// `allowed: false` response may short-circuit the rest. Users must take care to +// ensure patches are disjoint. +func MultiMutatingHandler(handlers ...Handler) Handler { + return multiMutating(handlers) +} + +type multiValidating []Handler + +func (hs multiValidating) Handle(ctx context.Context, req Request) Response { + for _, handler := range hs { + resp := handler.Handle(ctx, req) + if !resp.Allowed { + return resp + } + } + return Response{ + AdmissionResponse: admissionv1beta1.AdmissionResponse{ + Allowed: true, + Result: &metav1.Status{ + Code: http.StatusOK, + }, + }, + } +} + +// MultiValidatingHandler combines multiple validating webhook handlers into a single +// validating webhook handler. Handlers are called in sequential order, and the first +// `allowed: false` response may short-circuit the rest. +func MultiValidatingHandler(handlers ...Handler) Handler { + return multiValidating(handlers) +} + +// InjectFunc injects the field setter into the handlers. +func (hs multiValidating) InjectFunc(f inject.Func) error { + // inject directly into the handlers. It would be more correct + // to do this in a sync.Once in Handle (since we don't have some + // other start/finalize-type method), but it's more efficient to + // do it here, presumably. + for _, handler := range hs { + if err := f(handler); err != nil { + return err + } + } + + return nil +} diff --git a/pkg/webhook/admission/multi_test.go b/pkg/webhook/admission/multi_test.go new file mode 100644 index 0000000000..8501e62f62 --- /dev/null +++ b/pkg/webhook/admission/multi_test.go @@ -0,0 +1,132 @@ +/* +Copyright 2019 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 admission + +import ( + "context" + + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" + + "github.com/appscode/jsonpatch" + admissionv1beta1 "k8s.io/api/admission/v1beta1" +) + +var _ = Describe("Multi-Handler Admission Webhooks", func() { + alwaysAllow := &fakeHandler{ + fn: func(ctx context.Context, req Request) Response { + return Response{ + AdmissionResponse: admissionv1beta1.AdmissionResponse{ + Allowed: true, + }, + } + }, + } + alwaysDeny := &fakeHandler{ + fn: func(ctx context.Context, req Request) Response { + return Response{ + AdmissionResponse: admissionv1beta1.AdmissionResponse{ + Allowed: false, + }, + } + }, + } + + Context("with validating handlers", func() { + It("should deny the request if any handler denies the request", func() { + By("setting up a handler with accept and deny") + handler := MultiValidatingHandler(alwaysAllow, alwaysDeny) + + By("checking that the handler denies the request") + resp := handler.Handle(context.Background(), Request{}) + Expect(resp.Allowed).To(BeFalse()) + }) + + It("should allow the request if all handlers allow the request", func() { + By("setting up a handler with only accept") + handler := MultiValidatingHandler(alwaysAllow, alwaysAllow) + + By("checking that the handler allows the request") + resp := handler.Handle(context.Background(), Request{}) + Expect(resp.Allowed).To(BeTrue()) + }) + }) + + Context("with mutating handlers", func() { + patcher1 := &fakeHandler{ + fn: func(ctx context.Context, req Request) Response { + return Response{ + Patches: []jsonpatch.JsonPatchOperation{ + { + Operation: "add", + Path: "/metadata/annotation/new-key", + Value: "new-value", + }, + { + Operation: "replace", + Path: "/spec/replicas", + Value: "2", + }, + }, + AdmissionResponse: admissionv1beta1.AdmissionResponse{ + Allowed: true, + PatchType: func() *admissionv1beta1.PatchType { pt := admissionv1beta1.PatchTypeJSONPatch; return &pt }(), + }, + } + }, + } + patcher2 := &fakeHandler{ + fn: func(ctx context.Context, req Request) Response { + return Response{ + Patches: []jsonpatch.JsonPatchOperation{ + { + Operation: "add", + Path: "/metadata/annotation/hello", + Value: "world", + }, + }, + AdmissionResponse: admissionv1beta1.AdmissionResponse{ + Allowed: true, + PatchType: func() *admissionv1beta1.PatchType { pt := admissionv1beta1.PatchTypeJSONPatch; return &pt }(), + }, + } + }, + } + + It("should not return any patches if the request is denied", func() { + By("setting up a webhook with some patches and a deny") + handler := MultiMutatingHandler(patcher1, patcher2, alwaysDeny) + + By("checking that the handler denies the request and produces no patches") + resp := handler.Handle(context.Background(), Request{}) + Expect(resp.Allowed).To(BeFalse()) + Expect(resp.Patches).To(BeEmpty()) + }) + + It("should produce all patches if the requests are all allowed", func() { + By("setting up a webhook with some patches") + handler := MultiMutatingHandler(patcher1, patcher2, alwaysAllow) + + By("checking that the handler accepts the request and returns all patches") + resp := handler.Handle(context.Background(), Request{}) + Expect(resp.Allowed).To(BeTrue()) + Expect(resp.Patch).To(Equal([]byte( + `[{"op":"add","path":"/metadata/annotation/new-key","value":"new-value"},` + + `{"op":"replace","path":"/spec/replicas","value":"2"},{"op":"add","path":"/metadata/annotation/hello","value":"world"}]`))) + }) + }) +}) diff --git a/pkg/webhook/admission/response.go b/pkg/webhook/admission/response.go index 74f55a4ca3..7f9b544712 100644 --- a/pkg/webhook/admission/response.go +++ b/pkg/webhook/admission/response.go @@ -23,13 +23,34 @@ import ( admissionv1beta1 "k8s.io/api/admission/v1beta1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "sigs.k8s.io/controller-runtime/pkg/webhook/admission/types" ) -// ErrorResponse creates a new Response for error-handling a request. -func ErrorResponse(code int32, err error) types.Response { - return types.Response{ - Response: &admissionv1beta1.AdmissionResponse{ +// Allowed constructs a response indicating that the given operation +// is allowed (without any patches). +func Allowed(reason string) Response { + return ValidationResponse(true, reason) +} + +// Denied constructs a response indicating that the given operation +// is not allowed. +func Denied(reason string) Response { + return ValidationResponse(false, reason) +} + +// Patched constructs a response indicating that the given operation is +// allowed, and that the target object should be modified by the given +// JSONPatch operations. +func Patched(reason string, patches ...jsonpatch.JsonPatchOperation) Response { + resp := Allowed(reason) + resp.Patches = patches + + return resp +} + +// Errored creates a new Response for error-handling a request. +func Errored(code int32, err error) Response { + return Response{ + AdmissionResponse: admissionv1beta1.AdmissionResponse{ Allowed: false, Result: &metav1.Status{ Code: code, @@ -40,14 +61,14 @@ func ErrorResponse(code int32, err error) types.Response { } // ValidationResponse returns a response for admitting a request. -func ValidationResponse(allowed bool, reason string) types.Response { - resp := types.Response{ - Response: &admissionv1beta1.AdmissionResponse{ +func ValidationResponse(allowed bool, reason string) Response { + resp := Response{ + AdmissionResponse: admissionv1beta1.AdmissionResponse{ Allowed: allowed, }, } if len(reason) > 0 { - resp.Response.Result = &metav1.Status{ + resp.Result = &metav1.Status{ Reason: metav1.StatusReason(reason), } } @@ -57,14 +78,14 @@ func ValidationResponse(allowed bool, reason string) types.Response { // PatchResponseFromRaw takes 2 byte arrays and returns a new response with json patch. // The original object should be passed in as raw bytes to avoid the roundtripping problem // described in https://github.com/kubernetes-sigs/kubebuilder/issues/510. -func PatchResponseFromRaw(original, current []byte) types.Response { +func PatchResponseFromRaw(original, current []byte) Response { patches, err := jsonpatch.CreatePatch(original, current) if err != nil { - return ErrorResponse(http.StatusInternalServerError, err) + return Errored(http.StatusInternalServerError, err) } - return types.Response{ + return Response{ Patches: patches, - Response: &admissionv1beta1.AdmissionResponse{ + AdmissionResponse: admissionv1beta1.AdmissionResponse{ Allowed: true, PatchType: func() *admissionv1beta1.PatchType { pt := admissionv1beta1.PatchTypeJSONPatch; return &pt }(), }, diff --git a/pkg/webhook/admission/response_test.go b/pkg/webhook/admission/response_test.go index fcbecce942..8e2dfde48e 100644 --- a/pkg/webhook/admission/response_test.go +++ b/pkg/webhook/admission/response_test.go @@ -26,15 +26,101 @@ import ( admissionv1beta1 "k8s.io/api/admission/v1beta1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "sigs.k8s.io/controller-runtime/pkg/webhook/admission/types" ) -var _ = Describe("admission webhook response", func() { - Describe("ErrorResponse", func() { - It("should return the response with an error", func() { +var _ = Describe("Admission Webhook Response Helpers", func() { + Describe("Allowed", func() { + It("should return an 'allowed' response", func() { + Expect(Allowed("")).To(Equal( + Response{ + AdmissionResponse: admissionv1beta1.AdmissionResponse{ + Allowed: true, + }, + }, + )) + }) + + It("should populate a status with a reason when a reason is given", func() { + Expect(Allowed("acceptable")).To(Equal( + Response{ + AdmissionResponse: admissionv1beta1.AdmissionResponse{ + Allowed: true, + Result: &metav1.Status{ + Reason: "acceptable", + }, + }, + }, + )) + }) + }) + + Describe("Denied", func() { + It("should return a 'not allowed' response", func() { + Expect(Denied("")).To(Equal( + Response{ + AdmissionResponse: admissionv1beta1.AdmissionResponse{ + Allowed: false, + }, + }, + )) + }) + + It("should populate a status with a reason when a reason is given", func() { + Expect(Denied("UNACCEPTABLE!")).To(Equal( + Response{ + AdmissionResponse: admissionv1beta1.AdmissionResponse{ + Allowed: false, + Result: &metav1.Status{ + Reason: "UNACCEPTABLE!", + }, + }, + }, + )) + }) + }) + + Describe("Patched", func() { + ops := []jsonpatch.JsonPatchOperation{ + { + Operation: "replace", + Path: "/spec/selector/matchLabels", + Value: map[string]string{"foo": "bar"}, + }, + { + Operation: "delete", + Path: "/spec/replicas", + }, + } + It("should return an 'allowed' response with the given patches", func() { + Expect(Patched("", ops...)).To(Equal( + Response{ + AdmissionResponse: admissionv1beta1.AdmissionResponse{ + Allowed: true, + }, + Patches: ops, + }, + )) + }) + It("should populate a status with a reason when a reason is given", func() { + Expect(Patched("some changes", ops...)).To(Equal( + Response{ + AdmissionResponse: admissionv1beta1.AdmissionResponse{ + Allowed: true, + Result: &metav1.Status{ + Reason: "some changes", + }, + }, + Patches: ops, + }, + )) + }) + }) + + Describe("Errored", func() { + It("should return a denied response with an error", func() { err := errors.New("this is an error") - expected := types.Response{ - Response: &admissionv1beta1.AdmissionResponse{ + expected := Response{ + AdmissionResponse: admissionv1beta1.AdmissionResponse{ Allowed: false, Result: &metav1.Status{ Code: http.StatusBadRequest, @@ -42,36 +128,71 @@ var _ = Describe("admission webhook response", func() { }, }, } - resp := ErrorResponse(http.StatusBadRequest, err) + resp := Errored(http.StatusBadRequest, err) Expect(resp).To(Equal(expected)) }) }) Describe("ValidationResponse", func() { - It("should return the response with an admission decision", func() { - expected := types.Response{ - Response: &admissionv1beta1.AdmissionResponse{ - Allowed: true, - Result: &metav1.Status{ - Reason: metav1.StatusReason("allow to admit"), + It("should populate a status with a reason when a reason is given", func() { + By("checking that a message is populated for 'allowed' responses") + Expect(ValidationResponse(true, "acceptable")).To(Equal( + Response{ + AdmissionResponse: admissionv1beta1.AdmissionResponse{ + Allowed: true, + Result: &metav1.Status{ + Reason: "acceptable", + }, }, }, - } - resp := ValidationResponse(true, "allow to admit") - Expect(resp).To(Equal(expected)) + )) + + By("checking that a message is populated for 'denied' responses") + Expect(ValidationResponse(false, "UNACCEPTABLE!")).To(Equal( + Response{ + AdmissionResponse: admissionv1beta1.AdmissionResponse{ + Allowed: false, + Result: &metav1.Status{ + Reason: "UNACCEPTABLE!", + }, + }, + }, + )) + }) + + It("should return an admission decision", func() { + By("checking that it returns a 'denied' response when allowed is false") + Expect(ValidationResponse(true, "")).To(Equal( + Response{ + AdmissionResponse: admissionv1beta1.AdmissionResponse{ + Allowed: true, + }, + }, + )) + + By("checking that it returns an 'allowed' response when allowed is true") + Expect(ValidationResponse(false, "")).To(Equal( + Response{ + AdmissionResponse: admissionv1beta1.AdmissionResponse{ + Allowed: false, + }, + }, + )) }) }) - Describe("PatchResponse", func() { - It("should return the response with patches", func() { - expected := types.Response{ - Patches: []jsonpatch.JsonPatchOperation{}, - Response: &admissionv1beta1.AdmissionResponse{ + Describe("PatchResponseFromRaw", func() { + It("should return an 'allowed' response with a patch of the diff between two sets of serialized JSON", func() { + expected := Response{ + Patches: []jsonpatch.JsonPatchOperation{ + {Operation: "replace", Path: "/a", Value: "bar"}, + }, + AdmissionResponse: admissionv1beta1.AdmissionResponse{ Allowed: true, PatchType: func() *admissionv1beta1.PatchType { pt := admissionv1beta1.PatchTypeJSONPatch; return &pt }(), }, } - resp := PatchResponseFromRaw([]byte(`{}`), []byte(`{}`)) + resp := PatchResponseFromRaw([]byte(`{"a": "foo"}`), []byte(`{"a": "bar"}`)) Expect(resp).To(Equal(expected)) }) }) diff --git a/pkg/webhook/admission/types/types.go b/pkg/webhook/admission/types/types.go deleted file mode 100644 index 75236779ff..0000000000 --- a/pkg/webhook/admission/types/types.go +++ /dev/null @@ -1,44 +0,0 @@ -/* -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 types - -import ( - "github.com/appscode/jsonpatch" - - admissionv1beta1 "k8s.io/api/admission/v1beta1" - "k8s.io/apimachinery/pkg/runtime" -) - -// Request is the input of Handler -type Request struct { - AdmissionRequest *admissionv1beta1.AdmissionRequest -} - -// Response is the output of admission.Handler -type Response struct { - // Patches are the JSON patches for mutating webhooks. - // Using this instead of setting Response.Patch to minimize the overhead of serialization and deserialization. - Patches []jsonpatch.JsonPatchOperation - // Response is the admission response. Don't set the Patch field in it. - Response *admissionv1beta1.AdmissionResponse -} - -// Decoder is used to decode AdmissionRequest. -type Decoder interface { - // Decode decodes the raw byte object from the AdmissionRequest to the passed-in runtime.Object. - Decode(Request, runtime.Object) error -} diff --git a/pkg/webhook/admission/webhook.go b/pkg/webhook/admission/webhook.go index 632160c0cb..2f6cd9a88f 100644 --- a/pkg/webhook/admission/webhook.go +++ b/pkg/webhook/admission/webhook.go @@ -18,205 +18,179 @@ package admission import ( "context" - "encoding/json" "errors" - "fmt" "net/http" - "regexp" - "strings" - "sync" "github.com/appscode/jsonpatch" - + "github.com/go-logr/logr" admissionv1beta1 "k8s.io/api/admission/v1beta1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/util/json" + "sigs.k8s.io/controller-runtime/pkg/runtime/inject" - atypes "sigs.k8s.io/controller-runtime/pkg/webhook/admission/types" - "sigs.k8s.io/controller-runtime/pkg/webhook/types" ) +var ( + errUnableToEncodeResponse = errors.New("unable to encode response") +) + +// Request defines the input for an admission handler. +// It contains information to identify the object in +// question (group, version, kind, resource, subresource, +// name, namespace), as well as the operation in question +// (e.g. Get, Create, etc), and the object itself. +type Request struct { + admissionv1beta1.AdmissionRequest +} + +// Response is the output of an admission handler. +// It contains a response indicating if a given +// operation is allowed, as well as a set of patches +// to mutate the object in the case of a mutating admission handler. +type Response struct { + // Patches are the JSON patches for mutating webhooks. + // Using this instead of setting Response.Patch to minimize + // overhead of serialization and deserialization. + // Patches set here will override any patches in the response, + // so leave this empty if you want to set the patch response directly. + Patches []jsonpatch.JsonPatchOperation + // AdmissionResponse is the raw admission response. + // The Patch field in it will be overwritten by the listed patches. + admissionv1beta1.AdmissionResponse +} + +// Complete populates any fields that are yet to be set in +// the underlying AdmissionResponse, It mutates the response. +func (r *Response) Complete(req Request) error { + r.UID = req.UID + + // ensure that we have a valid status code + if r.Result == nil { + r.Result = &metav1.Status{} + } + if r.Result.Code == 0 { + r.Result.Code = http.StatusOK + } + // TODO(directxman12): do we need to populate this further, and/or + // is code actually necessary (the same webhook doesn't use it) + + if len(r.Patches) == 0 { + return nil + } + + var err error + r.Patch, err = json.Marshal(r.Patches) + if err != nil { + return err + } + patchType := admissionv1beta1.PatchTypeJSONPatch + r.PatchType = &patchType + + return nil +} + // Handler can handle an AdmissionRequest. type Handler interface { - Handle(context.Context, atypes.Request) atypes.Response + Handle(context.Context, Request) Response } // HandlerFunc implements Handler interface using a single function. -type HandlerFunc func(context.Context, atypes.Request) atypes.Response +type HandlerFunc func(context.Context, Request) Response var _ Handler = HandlerFunc(nil) // Handle process the AdmissionRequest by invoking the underlying function. -func (f HandlerFunc) Handle(ctx context.Context, req atypes.Request) atypes.Response { +func (f HandlerFunc) Handle(ctx context.Context, req Request) Response { return f(ctx, req) } // Webhook represents each individual webhook. type Webhook struct { - // Name is the name of the webhook - Name string - // Type is the webhook type, i.e. mutating, validating - Type types.WebhookType - // Path is the path this webhook will serve. - Path string - // Handlers contains a list of handlers. Each handler may only contains the business logic for its own feature. - // For example, feature foo and bar can be in the same webhook if all the other configurations are the same. - // The handler will be invoked sequentially as the order in the list. - // Note: if you are using mutating webhook with multiple handlers, it's your responsibility to - // ensure the handlers are not generating conflicting JSON patches. - Handlers []Handler - - once sync.Once -} + // Handler actually processes an admission request returning whether it was allowed or denied, + // and potentially patches to apply to the handler. + Handler Handler -func (w *Webhook) setDefaults() { - if len(w.Name) == 0 { - reg := regexp.MustCompile("[^a-zA-Z0-9]+") - processedPath := strings.ToLower(reg.ReplaceAllString(w.Path, "")) - w.Name = processedPath + ".example.com" - } -} + // decoder is constructed on receiving a scheme and passed down to then handler + decoder *Decoder -// Add adds additional handler(s) in the webhook -func (w *Webhook) Add(handlers ...Handler) { - w.Handlers = append(w.Handlers, handlers...) + log logr.Logger } -// Webhook implements Handler interface. -var _ Handler = &Webhook{} +// InjectLogger gets a handle to a logging instance, hopefully with more info about this particular webhook. +func (w *Webhook) InjectLogger(l logr.Logger) error { + w.log = l + return nil +} // Handle processes AdmissionRequest. // If the webhook is mutating type, it delegates the AdmissionRequest to each handler and merge the patches. // If the webhook is validating type, it delegates the AdmissionRequest to each handler and // deny the request if anyone denies. -func (w *Webhook) Handle(ctx context.Context, req atypes.Request) atypes.Response { - if req.AdmissionRequest == nil { - return ErrorResponse(http.StatusBadRequest, errors.New("got an empty AdmissionRequest")) +func (w *Webhook) Handle(ctx context.Context, req Request) Response { + resp := w.Handler.Handle(ctx, req) + if err := resp.Complete(req); err != nil { + w.log.Error(err, "unable to encode response") + return Errored(http.StatusInternalServerError, errUnableToEncodeResponse) } - var resp atypes.Response - switch w.Type { - case types.WebhookTypeMutating: - resp = w.handleMutating(ctx, req) - case types.WebhookTypeValidating: - resp = w.handleValidating(ctx, req) - default: - return ErrorResponse(http.StatusInternalServerError, errors.New("you must specify your webhook type")) - } - resp.Response.UID = req.AdmissionRequest.UID + return resp } -func (w *Webhook) handleMutating(ctx context.Context, req atypes.Request) atypes.Response { - patches := []jsonpatch.JsonPatchOperation{} - for _, handler := range w.Handlers { - resp := handler.Handle(ctx, req) - if !resp.Response.Allowed { - setStatusOKInAdmissionResponse(resp.Response) - return resp - } - if resp.Response.PatchType != nil && *resp.Response.PatchType != admissionv1beta1.PatchTypeJSONPatch { - return ErrorResponse(http.StatusInternalServerError, - fmt.Errorf("unexpected patch type returned by the handler: %v, only allow: %v", - resp.Response.PatchType, admissionv1beta1.PatchTypeJSONPatch)) - } - patches = append(patches, resp.Patches...) - } +// InjectScheme injects a scheme into the webhook, in order to construct a Decoder. +func (w *Webhook) InjectScheme(s *runtime.Scheme) error { + // TODO(directxman12): we should have a better way to pass this down + var err error - marshaledPatch, err := json.Marshal(patches) + w.decoder, err = NewDecoder(s) if err != nil { - return ErrorResponse(http.StatusBadRequest, fmt.Errorf("error when marshaling the patch: %v", err)) - } - return atypes.Response{ - Response: &admissionv1beta1.AdmissionResponse{ - Allowed: true, - Result: &metav1.Status{ - Code: http.StatusOK, - }, - Patch: marshaledPatch, - PatchType: func() *admissionv1beta1.PatchType { pt := admissionv1beta1.PatchTypeJSONPatch; return &pt }(), - }, + return err } -} -func (w *Webhook) handleValidating(ctx context.Context, req atypes.Request) atypes.Response { - for _, handler := range w.Handlers { - resp := handler.Handle(ctx, req) - if !resp.Response.Allowed { - setStatusOKInAdmissionResponse(resp.Response) - return resp + // inject the decoder here too, just in case the order of calling this is not + // scheme first, then inject func + if w.Handler != nil { + if _, err := InjectDecoderInto(w.GetDecoder(), w.Handler); err != nil { + return err } } - return atypes.Response{ - Response: &admissionv1beta1.AdmissionResponse{ - Allowed: true, - Result: &metav1.Status{ - Code: http.StatusOK, - }, - }, - } -} - -func setStatusOKInAdmissionResponse(resp *admissionv1beta1.AdmissionResponse) { - if resp == nil { - return - } - if resp.Result == nil { - resp.Result = &metav1.Status{} - } - if resp.Result.Code == 0 { - resp.Result.Code = http.StatusOK - } -} - -// GetName returns the name of the webhook. -func (w *Webhook) GetName() string { - w.once.Do(w.setDefaults) - return w.Name -} - -// GetPath returns the path that the webhook registered. -func (w *Webhook) GetPath() string { - w.once.Do(w.setDefaults) - return w.Path -} -// GetType returns the type of the webhook. -func (w *Webhook) GetType() types.WebhookType { - w.once.Do(w.setDefaults) - return w.Type + return nil } -// Handler returns a http.Handler for the webhook -func (w *Webhook) Handler() http.Handler { - w.once.Do(w.setDefaults) - return w +// GetDecoder returns a decoder to decode the objects embedded in admission requests. +// It may be nil if we haven't received a scheme to use to determine object types yet. +func (w *Webhook) GetDecoder() *Decoder { + return w.decoder } -// Validate validates if the webhook is valid. -func (w *Webhook) Validate() error { - w.once.Do(w.setDefaults) - if len(w.Name) == 0 { - return errors.New("field Name should not be empty") - } - if w.Type != types.WebhookTypeMutating && w.Type != types.WebhookTypeValidating { - return fmt.Errorf("unsupported Type: %v, only WebhookTypeMutating and WebhookTypeValidating are supported", w.Type) - } - if len(w.Path) == 0 { - return errors.New("field Path should not be empty") - } - if len(w.Handlers) == 0 { - return errors.New("field Handler should not be empty") - } - return nil -} +// InjectFunc injects the field setter into the webhook. +func (w *Webhook) InjectFunc(f inject.Func) error { + // inject directly into the handlers. It would be more correct + // to do this in a sync.Once in Handle (since we don't have some + // other start/finalize-type method), but it's more efficient to + // do it here, presumably. + + // also inject a decoder, and wrap this so that we get a setFields + // that injects a decoder (hopefully things don't ignore the duplicate + // InjectorInto call). + + var setFields inject.Func + setFields = func(target interface{}) error { + if err := f(target); err != nil { + return err + } -var _ inject.Injector = &Webhook{} + if _, err := inject.InjectorInto(setFields, target); err != nil { + return err + } -// InjectFunc injects dependencies into the handlers. -func (w *Webhook) InjectFunc(f inject.Func) error { - for _, handler := range w.Handlers { - if err := f(handler); err != nil { + if _, err := InjectDecoderInto(w.GetDecoder(), target); err != nil { return err } + + return nil } - return nil + + return setFields(w.Handler) } diff --git a/pkg/webhook/admission/webhook_test.go b/pkg/webhook/admission/webhook_test.go index 1f6bddbcb3..2097414de0 100644 --- a/pkg/webhook/admission/webhook_test.go +++ b/pkg/webhook/admission/webhook_test.go @@ -17,261 +17,195 @@ limitations under the License. package admission import ( - "bytes" "context" - "encoding/base64" - "encoding/json" - "errors" "net/http" - "net/http/httptest" - "github.com/appscode/jsonpatch" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" + "github.com/appscode/jsonpatch" admissionv1beta1 "k8s.io/api/admission/v1beta1" - atypes "sigs.k8s.io/controller-runtime/pkg/webhook/admission/types" - "sigs.k8s.io/controller-runtime/pkg/webhook/types" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + machinerytypes "k8s.io/apimachinery/pkg/types" + + "sigs.k8s.io/controller-runtime/pkg/runtime/inject" ) -var _ = Describe("admission webhook", func() { - var w *httptest.ResponseRecorder - BeforeEach(func(done Done) { - w = &httptest.ResponseRecorder{ - Body: bytes.NewBuffer(nil), +var _ = Describe("Admission Webhooks", func() { + allowHandler := func() *Webhook { + handler := &fakeHandler{ + fn: func(ctx context.Context, req Request) Response { + return Response{ + AdmissionResponse: admissionv1beta1.AdmissionResponse{ + Allowed: true, + }, + } + }, } - close(done) + webhook := &Webhook{ + Handler: handler, + } + + return webhook + } + + It("should invoke the handler to get a response", func() { + By("setting up a webhook with an allow handler") + webhook := allowHandler() + + By("invoking the webhook") + resp := webhook.Handle(context.Background(), Request{}) + + By("checking that it allowed the request") + Expect(resp.Allowed).To(BeTrue()) }) - Describe("validating webhook", func() { - var alwaysAllow, alwaysDeny *fakeHandler - var req *http.Request - var wh *Webhook - BeforeEach(func(done Done) { - alwaysAllow = &fakeHandler{ - fn: func(ctx context.Context, req atypes.Request) atypes.Response { - return atypes.Response{ - Response: &admissionv1beta1.AdmissionResponse{ - Allowed: true, - }, - } - }, - } - alwaysDeny = &fakeHandler{ - fn: func(ctx context.Context, req atypes.Request) atypes.Response { - return atypes.Response{ - Response: &admissionv1beta1.AdmissionResponse{ - Allowed: false, - }, - } - }, - } - req = &http.Request{ - Header: http.Header{"Content-Type": []string{"application/json"}}, - Body: nopCloser{Reader: bytes.NewBufferString(`{"request":{}}`)}, - } - close(done) - }) - Context("multiple handlers can be invoked", func() { - BeforeEach(func(done Done) { - wh = &Webhook{ - Type: types.WebhookTypeValidating, - Handlers: []Handler{alwaysAllow, alwaysDeny}, - } - close(done) - }) - - It("should deny the request", func() { - expected := []byte(`{"response":{"uid":"","allowed":false,"status":{"metadata":{},"code":200}}} -`) - wh.ServeHTTP(w, req) - Expect(w.Body.Bytes()).To(Equal(expected)) - Expect(alwaysAllow.invoked).To(BeTrue()) - Expect(alwaysDeny.invoked).To(BeTrue()) - }) - }) + It("should ensure that the response's UID is set to the request's UID", func() { + By("setting up a webhook") + webhook := allowHandler() + + By("invoking the webhook") + resp := webhook.Handle(context.Background(), Request{AdmissionRequest: admissionv1beta1.AdmissionRequest{UID: "foobar"}}) + + By("checking that the response share's the request's UID") + Expect(resp.UID).To(Equal(machinerytypes.UID("foobar"))) + }) + + It("should populate the status on a response if one is not provided", func() { + By("setting up a webhook") + webhook := allowHandler() + + By("invoking the webhook") + resp := webhook.Handle(context.Background(), Request{}) - Context("validating webhook should return if one of the handler denies", func() { - BeforeEach(func(done Done) { - wh = &Webhook{ - Type: types.WebhookTypeValidating, - Handlers: []Handler{alwaysDeny, alwaysAllow}, + By("checking that the response share's the request's UID") + Expect(resp.Result).To(Equal(&metav1.Status{Code: http.StatusOK})) + }) + + It("shouldn't overwrite the status on a response", func() { + By("setting up a webhook that sets a status") + webhook := &Webhook{ + Handler: HandlerFunc(func(ctx context.Context, req Request) Response { + return Response{ + AdmissionResponse: admissionv1beta1.AdmissionResponse{ + Allowed: true, + Result: &metav1.Status{Message: "Ground Control to Major Tom"}, + }, } - close(done) - }) - - It("should deny the request", func() { - expected := []byte(`{"response":{"uid":"","allowed":false,"status":{"metadata":{},"code":200}}} -`) - wh.ServeHTTP(w, req) - Expect(w.Body.Bytes()).To(Equal(expected)) - Expect(alwaysDeny.invoked).To(BeTrue()) - Expect(alwaysAllow.invoked).To(BeFalse()) - }) - }) + }), + } + + By("invoking the webhook") + resp := webhook.Handle(context.Background(), Request{}) + + By("checking that the message is intact") + Expect(resp.Result).NotTo(BeNil()) + Expect(resp.Result.Message).To(Equal("Ground Control to Major Tom")) }) - Describe("mutating webhook", func() { - Context("multiple patch handlers", func() { - req := &http.Request{ - Header: http.Header{"Content-Type": []string{"application/json"}}, - Body: nopCloser{Reader: bytes.NewBufferString(`{"request":{}}`)}, - } - patcher1 := &fakeHandler{ - fn: func(ctx context.Context, req atypes.Request) atypes.Response { - return atypes.Response{ - Patches: []jsonpatch.JsonPatchOperation{ - { - Operation: "add", - Path: "/metadata/annotation/new-key", - Value: "new-value", - }, - { - Operation: "replace", - Path: "/spec/replicas", - Value: "2", - }, - }, - Response: &admissionv1beta1.AdmissionResponse{ - Allowed: true, - PatchType: func() *admissionv1beta1.PatchType { pt := admissionv1beta1.PatchTypeJSONPatch; return &pt }(), - }, - } - }, + It("should serialize patch operations into a single jsonpatch blob", func() { + By("setting up a webhook with a patching handler") + webhook := &Webhook{ + Handler: HandlerFunc(func(ctx context.Context, req Request) Response { + return Patched("", jsonpatch.Operation{Operation: "add", Path: "/a", Value: 2}, jsonpatch.Operation{Operation: "replace", Path: "/b", Value: 4}) + }), + } + + By("invoking the webhoook") + resp := webhook.Handle(context.Background(), Request{}) + + By("checking that a JSON patch is populated on the response") + patchType := admissionv1beta1.PatchTypeJSONPatch + Expect(resp.PatchType).To(Equal(&patchType)) + Expect(resp.Patch).To(Equal([]byte(`[{"op":"add","path":"/a","value":2},{"op":"replace","path":"/b","value":4}]`))) + }) + + Describe("dependency injection", func() { + It("should set dependencies passed in on the handler", func() { + By("setting up a webhook and injecting it with a injection func that injects a string") + setFields := func(target interface{}) error { + inj, ok := target.(stringInjector) + if !ok { + return nil + } + + return inj.InjectString("something") } - patcher2 := &fakeHandler{ - fn: func(ctx context.Context, req atypes.Request) atypes.Response { - return atypes.Response{ - Patches: []jsonpatch.JsonPatchOperation{ - { - Operation: "add", - Path: "/metadata/annotation/hello", - Value: "world", - }, - }, - Response: &admissionv1beta1.AdmissionResponse{ - Allowed: true, - PatchType: func() *admissionv1beta1.PatchType { pt := admissionv1beta1.PatchTypeJSONPatch; return &pt }(), - }, - } - }, + handler := &fakeHandler{} + webhook := &Webhook{ + Handler: handler, } - wh := &Webhook{ - Type: types.WebhookTypeMutating, - Handlers: []Handler{patcher1, patcher2}, + Expect(setFields(webhook)).To(Succeed()) + Expect(inject.InjectorInto(setFields, webhook)).To(BeTrue()) + + By("checking that the string was injected") + Expect(handler.injectedString).To(Equal("something")) + }) + + It("should inject a decoder into the handler", func() { + By("setting up a webhook and injecting it with a injection func that injects a scheme") + setFields := func(target interface{}) error { + if _, err := inject.SchemeInto(runtime.NewScheme(), target); err != nil { + return err + } + return nil } - expected := []byte( - `{"response":{"uid":"","allowed":true,"status":{"metadata":{},"code":200},` + - `"patch":"W3sib3AiOiJhZGQiLCJwYXRoIjoiL21ldGFkYXRhL2Fubm90YXRpb2` + - `4vbmV3LWtleSIsInZhbHVlIjoibmV3LXZhbHVlIn0seyJvcCI6InJlcGxhY2UiLCJwYXRoIjoiL3NwZWMvcmVwbGljYXMiLC` + - `J2YWx1ZSI6IjIifSx7Im9wIjoiYWRkIiwicGF0aCI6Ii9tZXRhZGF0YS9hbm5vdGF0aW9uL2hlbGxvIiwidmFsdWUiOiJ3b3JsZCJ9XQ==",` + - `"patchType":"JSONPatch"}} -`) - patches := []jsonpatch.JsonPatchOperation{ - { - Operation: "add", - Path: "/metadata/annotation/new-key", - Value: "new-value", - }, - { - Operation: "replace", - Path: "/spec/replicas", - Value: "2", - }, - { - Operation: "add", - Path: "/metadata/annotation/hello", - Value: "world", - }, + handler := &fakeHandler{} + webhook := &Webhook{ + Handler: handler, } - j, _ := json.Marshal(patches) - base64encoded := base64.StdEncoding.EncodeToString(j) - It("should aggregates patches from multiple handlers", func() { - wh.ServeHTTP(w, req) - Expect(w.Body.Bytes()).To(Equal(expected)) - Expect(w.Body.String()).To(ContainSubstring(base64encoded)) - Expect(patcher1.invoked).To(BeTrue()) - Expect(patcher2.invoked).To(BeTrue()) - }) + Expect(setFields(webhook)).To(Succeed()) + Expect(inject.InjectorInto(setFields, webhook)).To(BeTrue()) + + By("checking that the decoder was injected") + Expect(handler.decoder).NotTo(BeNil()) }) - Context("patch handler denies the request", func() { - req := &http.Request{ - Header: http.Header{"Content-Type": []string{"application/json"}}, - Body: nopCloser{Reader: bytes.NewBufferString(`{"request":{}}`)}, + It("should pass a setFields that also injects a decoder into sub-dependencies", func() { + By("setting up a webhook and injecting it with a injection func that injects a scheme") + setFields := func(target interface{}) error { + if _, err := inject.SchemeInto(runtime.NewScheme(), target); err != nil { + return err + } + return nil } - errPatcher := &fakeHandler{ - fn: func(ctx context.Context, req atypes.Request) atypes.Response { - return atypes.Response{ - Response: &admissionv1beta1.AdmissionResponse{ - Allowed: false, - }, - } - }, + handler := &handlerWithSubDependencies{ + Handler: HandlerFunc(func(ctx context.Context, req Request) Response { + return Response{} + }), + dep: &subDep{}, } - wh := &Webhook{ - Type: types.WebhookTypeMutating, - Handlers: []Handler{errPatcher}, + webhook := &Webhook{ + Handler: handler, } - expected := []byte(`{"response":{"uid":"","allowed":false,"status":{"metadata":{},"code":200}}} -`) - It("should deny the request", func() { - wh.ServeHTTP(w, req) - Expect(w.Body.Bytes()).To(Equal(expected)) - Expect(errPatcher.invoked).To(BeTrue()) - }) + Expect(setFields(webhook)).To(Succeed()) + Expect(inject.InjectorInto(setFields, webhook)).To(BeTrue()) + + By("checking that setFields sets the decoder as well") + Expect(handler.dep.decoder).NotTo(BeNil()) }) }) +}) - Describe("webhook validation", func() { - Context("valid mutating webhook", func() { - wh := &Webhook{ - Type: types.WebhookTypeMutating, - Path: "/mutate-deployments", - Handlers: []Handler{&fakeHandler{}}, - } - It("should pass validation", func() { - err := wh.Validate() - Expect(err).NotTo(HaveOccurred()) - Expect(wh.Name).To(Equal("mutatedeployments.example.com")) - }) - }) +type stringInjector interface { + InjectString(s string) error +} - Context("valid validating webhook", func() { - wh := &Webhook{ - Type: types.WebhookTypeValidating, - Path: "/validate-deployments", - Handlers: []Handler{&fakeHandler{}}, - } - It("should pass validation", func() { - err := wh.Validate() - Expect(err).NotTo(HaveOccurred()) - Expect(wh.Name).To(Equal("validatedeployments.example.com")) - }) - }) +type handlerWithSubDependencies struct { + Handler + dep *subDep +} - Context("missing webhook type", func() { - wh := &Webhook{ - Path: "/mutate-deployments", - Handlers: []Handler{&fakeHandler{}}, - } - It("should fail validation", func() { - err := wh.Validate() - Expect(err.Error()).To(ContainSubstring("only WebhookTypeMutating and WebhookTypeValidating are supported")) - }) - }) +func (h *handlerWithSubDependencies) InjectFunc(f inject.Func) error { + return f(h.dep) +} - Context("missing Handlers", func() { - wh := &Webhook{ - Type: types.WebhookTypeValidating, - Path: "/validate-deployments", - Handlers: []Handler{}, - } - It("should fail validation", func() { - err := wh.Validate() - Expect(err).To(Equal(errors.New("field Handler should not be empty"))) - }) - }) +type subDep struct { + decoder *Decoder +} - }) -}) +func (d *subDep) InjectDecoder(dec *Decoder) error { + d.decoder = dec + return nil +} diff --git a/pkg/webhook/alias.go b/pkg/webhook/alias.go new file mode 100644 index 0000000000..d721878b7f --- /dev/null +++ b/pkg/webhook/alias.go @@ -0,0 +1,67 @@ +/* +Copyright 2019 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 ( + "github.com/appscode/jsonpatch" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" +) + +// define some aliases for common bits of the webhook functionality + +// AdmissionRequest defines the input for an admission handler. +// It contains information to identify the object in +// question (group, version, kind, resource, subresource, +// name, namespace), as well as the operation in question +// (e.g. Get, Create, etc), and the object itself. +type AdmissionRequest = admission.Request + +// AdmissionResponse is the output of an admission handler. +// It contains a response indicating if a given +// operation is allowed, as well as a set of patches +// to mutate the object in the case of a mutating admission handler. +type AdmissionResponse = admission.Response + +// Admission is webhook suitable for registration with the server +// an admission webhook that validates API operations and potentially +// mutates their contents. +type Admission = admission.Webhook + +// AdmissionHandler knows how to process admission requests, validating them, +// and potentially mutating the objects they contain. +type AdmissionHandler = admission.Handler + +// AdmissionDecoder knows how to decode objects from admission requests. +type AdmissionDecoder = admission.Decoder + +// JSONPatchOp represents a single JSONPatch patch operation. +type JSONPatchOp = jsonpatch.Operation + +var ( + // Allowed indicates that the admission request should be allowed for the given reason. + Allowed = admission.Allowed + + // Denied indicates that the admission request should be denied for the given reason. + Denied = admission.Denied + + // Patched indicates that the admission request should be allowed for the given reason, + // and that the contained object should be mutated using the given patches. + Patched = admission.Patched + + // Errored indicates that an error occurred in the admission request. + Errored = admission.Errored +) diff --git a/pkg/webhook/doc.go b/pkg/webhook/doc.go index 6aafff524c..2c93f0d995 100644 --- a/pkg/webhook/doc.go +++ b/pkg/webhook/doc.go @@ -18,52 +18,6 @@ limitations under the License. Package webhook provides methods to build and bootstrap a webhook server. Currently, it only supports admission webhooks. It will support CRD conversion webhooks in the near future. - -Build webhooks - - // mgr is the manager that runs the server. - webhook1, err := NewWebhookBuilder(). - Name("foo.k8s.io"). - Mutating(). - Path("/mutating-pods"). - Handlers(mutatingHandler1, mutatingHandler2). - Build() - if err != nil { - // handle error - } - - webhook2, err := NewWebhookBuilder(). - Name("bar.k8s.io"). - Validating(). - Path("/validating-deployment"). - Handlers(validatingHandler1). - Build() - if err != nil { - // handle error - } - -Create a webhook server. - - as, err := NewServer("baz-admission-server", mgr, ServerOptions{ - CertDir: "/tmp/cert", - }) - if err != nil { - // handle error - } - -Register the webhooks in the server. - - err = as.Register(webhook1, webhook2) - if err != nil { - // handle error - } - -Start the server by starting the manager - - err := mrg.Start(signals.SetupSignalHandler()) - if err != nil { - // handle error - } */ package webhook diff --git a/pkg/webhook/example_test.go b/pkg/webhook/example_test.go new file mode 100644 index 0000000000..b2fc77ec1d --- /dev/null +++ b/pkg/webhook/example_test.go @@ -0,0 +1,67 @@ +/* +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_test + +import ( + "context" + + ctrl "sigs.k8s.io/controller-runtime" + . "sigs.k8s.io/controller-runtime/pkg/webhook" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" +) + +var ( + mgr ctrl.Manager +) + +func Example() { + // Build webhooks + // These handlers could be also be implementations + // of the AdmissionHandler interface for more complex + // implementations. + mutatingHook := &Admission{ + Handler: admission.HandlerFunc(func(ctx context.Context, req AdmissionRequest) AdmissionResponse { + return Patched("some changes", + JSONPatchOp{Operation: "add", Path: "/metadata/annotations/access", Value: "granted"}, + JSONPatchOp{Operation: "add", Path: "/metadata/annotations/reason", Value: "not so secret"}, + ) + }), + } + + validatingHook := &Admission{ + Handler: admission.HandlerFunc(func(ctx context.Context, req AdmissionRequest) AdmissionResponse { + return Denied("none shall pass!") + }), + } + + // Create a webhook server. + hookServer := &Server{ + Port: 8443, + } + mgr.Add(hookServer) + + // Register the webhooks in the server. + hookServer.Register("/mutating", mutatingHook) + hookServer.Register("/validating", validatingHook) + + // Start the server by starting a previously-set-up manager + err := mgr.Start(ctrl.SetupSignalHandler()) + if err != nil { + // handle error + panic(err) + } +} diff --git a/pkg/webhook/server.go b/pkg/webhook/server.go index 38daa91dd1..25b2eb43e1 100644 --- a/pkg/webhook/server.go +++ b/pkg/webhook/server.go @@ -19,14 +19,16 @@ package webhook import ( "context" "crypto/tls" + "fmt" "net" "net/http" "path" "strconv" "sync" + "time" - "sigs.k8s.io/controller-runtime/pkg/manager" "sigs.k8s.io/controller-runtime/pkg/runtime/inject" + "sigs.k8s.io/controller-runtime/pkg/webhook/internal/metrics" ) const ( @@ -34,9 +36,10 @@ const ( keyName = "tls.key" ) -// ServerOptions are options for configuring an admission webhook server. -type ServerOptions struct { - // Address that the server will listen on. +// Server is an admission webhook server that can serve traffic and +// generates related k8s resources for deploying. +type Server struct { + // Host is the address that the server will listen on. // Defaults to "" - all addresses. Host string @@ -50,102 +53,79 @@ type ServerOptions struct { // If using SecretCertWriter in Provisioner, the server will provision the certificate in a secret, // the user is responsible to mount the secret to the this location for the server to consume. CertDir string -} - -// Server is an admission webhook server that can serve traffic and -// generates related k8s resources for deploying. -type Server struct { - // ServerOptions contains options for configuring the admission server. - ServerOptions - sMux *http.ServeMux - // registry maps a path to a http.Handler. - registry map[string]http.Handler + // TODO(directxman12): should we make the mux configurable? - // setFields is used to inject dependencies into webhooks - setFields func(i interface{}) error + // 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 - // manager is the manager that this webhook server will be registered. - manager manager.Manager + // setFields allows injecting dependencies from an external source + setFields inject.Func - once sync.Once + // defaultingOnce ensures that the default fields are only ever set once. + defaultingOnce sync.Once } -// Webhook defines the basics that a webhook should support. -type Webhook interface { - http.Handler - - // GetPath returns the path that the webhook registered. - GetPath() string - // Handler returns a http.Handler for the webhook. - Handler() http.Handler - // Validate validates if the webhook itself is valid. - // If invalid, a non-nil error will be returned. - Validate() error -} - -// NewServer creates a new admission webhook server. -func NewServer(mgr manager.Manager, options ServerOptions) (*Server, error) { - as := &Server{ - sMux: http.NewServeMux(), - registry: map[string]http.Handler{}, - ServerOptions: options, - manager: mgr, - } - - return as, nil -} +// setDefaults does defaulting for the Server. +func (s *Server) setDefaults() { + s.webhooks = map[string]http.Handler{} + s.webhookMux = http.NewServeMux() -// setDefault does defaulting for the Server. -func (s *Server) setDefault() { - if s.registry == nil { - s.registry = map[string]http.Handler{} - } - if s.sMux == nil { - s.sMux = http.NewServeMux() - } if s.Port <= 0 { s.Port = 443 } if len(s.CertDir) == 0 { - s.CertDir = path.Join("k8s-webhook-server", "cert") + s.CertDir = path.Join("/tmp", "k8s-webhook-server", "serving-certs") } } -// Register validates and registers webhook(s) in the server -func (s *Server) Register(webhooks ...Webhook) error { - for i, webhook := range webhooks { - // validate the webhook before registering it. - err := webhook.Validate() - if err != nil { - return err - } - // Handle actually ensures that no duplicate paths are registered. - s.sMux.Handle(webhook.GetPath(), webhook.Handler()) - s.registry[webhook.GetPath()] = webhooks[i] - - // Inject dependencies to each webhook. - if err := s.setFields(webhooks[i]); err != nil { - return err - } +// 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) { + s.defaultingOnce.Do(s.setDefaults) + _, found := s.webhooks[path] + if found { + panic(fmt.Errorf("can't register duplicate path: %v", path)) } - - // Lazily add Server to manager. - // Because the all webhook handlers to be in place, so we can inject the things they need. - return s.manager.Add(s) + // TODO(directxman12): call setfields if we've already started the server + s.webhooks[path] = hook + s.webhookMux.Handle(path, instrumentedHook(path, hook)) } -// Handle registers a http.Handler for the given pattern. -func (s *Server) Handle(pattern string, handler http.Handler) { - s.sMux.Handle(pattern, handler) -} +// instrumentedHook adds some instrumentation on top of the given webhook. +func instrumentedHook(path string, hookRaw http.Handler) http.Handler { + return http.HandlerFunc(func(resp http.ResponseWriter, req *http.Request) { + startTS := time.Now() + defer func() { metrics.RequestLatency.WithLabelValues(path).Observe(time.Now().Sub(startTS).Seconds()) }() + hookRaw.ServeHTTP(resp, req) -var _ manager.Runnable = &Server{} + // TODO(directxman12): add back in metric about total requests broken down by result? + }) +} // Start runs the server. // It will install the webhook related resources depend on the server configuration. func (s *Server) Start(stop <-chan struct{}) error { - s.once.Do(s.setDefault) + s.defaultingOnce.Do(s.setDefaults) + + baseHookLog := log.WithName("webhooks") + // inject fields here as opposed to in Register so that we're certain to have our setFields + // function available. + for hookPath, webhook := range s.webhooks { + if err := s.setFields(webhook); err != nil { + return err + } + + // NB(directxman12): we don't propagate this further by wrapping setFields because it's + // unclear if this is how we want to deal with log propagation. In this specific instance, + // we want to be able to pass a logger to webhooks because they don't know their own path. + if _, err := inject.LoggerInto(baseHookLog.WithValues("webhook", hookPath), webhook); err != nil { + return err + } + } // TODO: watch the cert dir. Reload the cert if it changes cert, err := tls.LoadX509KeyPair(path.Join(s.CertDir, certName), path.Join(s.CertDir, keyName)) @@ -163,7 +143,7 @@ func (s *Server) Start(stop <-chan struct{}) error { } srv := &http.Server{ - Handler: s.sMux, + Handler: s.webhookMux, } idleConnsClosed := make(chan struct{}) @@ -187,9 +167,7 @@ func (s *Server) Start(stop <-chan struct{}) error { return nil } -var _ inject.Injector = &Server{} - -// InjectFunc injects dependencies into the handlers. +// InjectFunc injects the field setter into the server. func (s *Server) InjectFunc(f inject.Func) error { s.setFields = f return nil diff --git a/pkg/webhook/types/webhook.go b/pkg/webhook/types/webhook.go deleted file mode 100644 index 2ad1253f21..0000000000 --- a/pkg/webhook/types/webhook.go +++ /dev/null @@ -1,28 +0,0 @@ -/* -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 types - -// WebhookType defines the type of a webhook -type WebhookType int - -const ( - _ = iota - // WebhookTypeMutating represents mutating type webhook - WebhookTypeMutating WebhookType = iota - // WebhookTypeValidating represents validating type webhook - WebhookTypeValidating -)