Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

⚠️ webhook redesign for generic case #323

Merged
merged 19 commits into from
Feb 21, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Gopkg.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 3 additions & 3 deletions alias.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion doc.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
//
Expand Down
29 changes: 6 additions & 23 deletions example/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -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 {
Expand Down
38 changes: 11 additions & 27 deletions example/mutatingwebhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 on readability admission.{Request, Response} as http.{Request, 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 {
mengqiy marked this conversation as resolved.
Show resolved Hide resolved
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{}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why removing this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't really add much, and necessitates an extra import. I can add it back in if we really want it, but I think the comment suffices here.


// 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
}
39 changes: 11 additions & 28 deletions example/validatingwebhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
mengqiy marked this conversation as resolved.
Show resolved Hide resolved
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 {
Expand All @@ -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
}
2 changes: 1 addition & 1 deletion example_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
6 changes: 3 additions & 3 deletions hack/test-all.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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
6 changes: 3 additions & 3 deletions hack/verify.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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 \
Expand Down
2 changes: 1 addition & 1 deletion pkg/builder/builder_suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion pkg/builder/example_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
4 changes: 2 additions & 2 deletions pkg/client/client_cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -141,5 +141,5 @@ type objMeta struct {
*resourceMeta

// Object contains meta data for the object instance
v1.Object
metav1.Object
}
12 changes: 6 additions & 6 deletions pkg/client/example_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
)

Expand Down Expand Up @@ -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",
},
Expand Down Expand Up @@ -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",
},
Expand Down Expand Up @@ -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))
}
Loading