Merge pull request #323 from DirectXMan12/tmp/webhook-decouple

⚠️ webhook redesign for generic case

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.

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.
 //

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 {

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
 }

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
 }

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
 	}

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

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 \

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) {

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.

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
 }

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))
 }