This creates a way for clients of the webhook to decorate the request…

… context. (#342)

Clients of webhook can now decorate the request context with additional metadata
via:
```
  ac := &AdmissionController{
    ... // As before
    WithContext: func(ctx context.Context) context.Context {
      // logic to attach stuff to ctx
    }
  }
```

This metadata can then be accessed off of the context in methods like
`SetDefaults` and `Validate` on types registered as webhook handlers.

Fixes: #306

testing/resource.go

@@ -55,6 +55,7 @@ var _ apis.Listable = (*Resource)(nil)
 // ResourceSpec represents test resource spec.
 type ResourceSpec struct {
 	FieldWithDefault               string `json:"fieldWithDefault,omitempty"`
+	FieldWithContextDefault        string `json:"fieldWithContextDefault,omitempty"`
 	FieldWithValidation            string `json:"fieldWithValidation,omitempty"`
 	FieldThatsImmutable            string `json:"fieldThatsImmutable,omitempty"`
 	FieldThatsImmutableWithDefault string `json:"fieldThatsImmutableWithDefault,omitempty"`
@@ -65,11 +66,29 @@ func (c *Resource) SetDefaults(ctx context.Context) {
 	c.Spec.SetDefaults(ctx)
 }
 
+type onContextKey struct{}
+
+// WithValue returns a WithContext for attaching an OnContext with the given value.
+func WithValue(ctx context.Context, val string) context.Context {
+	return context.WithValue(ctx, onContextKey{}, &OnContext{Value: val})
+}
+
+// OnContext is a struct for holding a value attached to a context.
+type OnContext struct {
+	Value string
+}
+
 // SetDefaults sets the defaults on the spec.
 func (cs *ResourceSpec) SetDefaults(ctx context.Context) {
 	if cs.FieldWithDefault == "" {
 		cs.FieldWithDefault = "I'm a default."
 	}
+	if cs.FieldWithContextDefault == "" {
+		oc, ok := ctx.Value(onContextKey{}).(*OnContext)
+		if ok {
+			cs.FieldWithContextDefault = oc.Value
+		}
+	}
 	if cs.FieldThatsImmutableWithDefault == "" {
 		cs.FieldThatsImmutableWithDefault = "this is another default value"
 	}

webhook/webhook.go

@@ -121,9 +121,14 @@ type AdmissionController struct {
 	Handlers map[schema.GroupVersionKind]GenericCRD
 	Logger   *zap.SugaredLogger
 
+	WithContext           func(context.Context) context.Context
 	DisallowUnknownFields bool
 }
 
+func nop(ctx context.Context) context.Context {
+	return ctx
+}
+
 // GenericCRD is the interface definition that allows us to perform the generic
 // CRD actions like deciding whether to increment generation and so forth.
 type GenericCRD interface {
@@ -205,32 +210,32 @@ func getOrGenerateKeyCertsFromSecret(ctx context.Context, client kubernetes.Inte
 
 // validate checks whether "new" and "old" implement HasImmutableFields and checks them,
 // it then delegates validation to apis.Validatable on "new".
-func validate(old GenericCRD, new GenericCRD) error {
+func validate(ctx context.Context, old, new GenericCRD) error {
 	if immutableNew, ok := new.(apis.Immutable); ok && old != nil {
 		// Copy the old object and set defaults so that we don't reject our own
 		// defaulting done earlier in the webhook.
 		old = old.DeepCopyObject().(GenericCRD)
 		// TODO(mattmoor): Plumb through a real context
-		old.SetDefaults(context.TODO())
+		old.SetDefaults(ctx)
 
 		immutableOld, ok := old.(apis.Immutable)
 		if !ok {
 			return fmt.Errorf("unexpected type mismatch %T vs. %T", old, new)
 		}
 		// TODO(mattmoor): Plumb through a real context
-		if err := immutableNew.CheckImmutableFields(context.TODO(), immutableOld); err != nil {
+		if err := immutableNew.CheckImmutableFields(ctx, immutableOld); err != nil {
 			return err
 		}
 	}
 	// Can't just `return new.Validate()` because it doesn't properly nil-check.
 	// TODO(mattmoor): Plumb through a real context
-	if err := new.Validate(context.TODO()); err != nil {
+	if err := new.Validate(ctx); err != nil {
 		return err
 	}
 	return nil
 }
 
-func setAnnotations(patches duck.JSONPatch, new, old GenericCRD, ui *authenticationv1.UserInfo) (duck.JSONPatch, error) {
+func setAnnotations(ctx context.Context, patches duck.JSONPatch, new, old GenericCRD, ui *authenticationv1.UserInfo) (duck.JSONPatch, error) {
 	// Nowhere to set the annotations.
 	if new == nil {
 		return patches, nil
@@ -246,7 +251,7 @@ func setAnnotations(patches duck.JSONPatch, new, old GenericCRD, ui *authenticat
 	b, a := new.DeepCopyObject().(apis.Annotatable), na
 
 	// TODO(mattmoor): Plumb through a real context
-	a.AnnotateUserInfo(context.TODO(), oa, ui)
+	a.AnnotateUserInfo(ctx, oa, ui)
 	patch, err := duck.CreatePatch(b, a)
 	if err != nil {
 		return nil, err
@@ -255,10 +260,10 @@ func setAnnotations(patches duck.JSONPatch, new, old GenericCRD, ui *authenticat
 }
 
 // setDefaults simply leverages apis.Defaultable to set defaults.
-func setDefaults(patches duck.JSONPatch, crd GenericCRD) (duck.JSONPatch, error) {
+func setDefaults(ctx context.Context, patches duck.JSONPatch, crd GenericCRD) (duck.JSONPatch, error) {
 	before, after := crd.DeepCopyObject(), crd
 	// TODO(mattmoor): Plumb through a real context
-	after.SetDefaults(context.TODO())
+	after.SetDefaults(ctx)
 
 	patch, err := duck.CreatePatch(before, after)
 	if err != nil {
@@ -456,7 +461,13 @@ func (ac *AdmissionController) ServeHTTP(w http.ResponseWriter, r *http.Request)
 		zap.String(logkey.Resource, fmt.Sprint(review.Request.Resource)),
 		zap.String(logkey.SubResource, fmt.Sprint(review.Request.SubResource)),
 		zap.String(logkey.UserInfo, fmt.Sprint(review.Request.UserInfo)))
-	reviewResponse := ac.admit(logging.WithLogger(r.Context(), logger), review.Request)
+	ctx := logging.WithLogger(r.Context(), logger)
+
+	if ac.WithContext != nil {
+		ctx = ac.WithContext(ctx)
+	}
+
+	reviewResponse := ac.admit(ctx, review.Request)
 	var response admissionv1beta1.AdmissionReview
 	if reviewResponse != nil {
 		response.Response = reviewResponse
@@ -561,14 +572,14 @@ func (ac *AdmissionController) mutate(ctx context.Context, req *admissionv1beta1
 		patches = append(patches, rtp...)
 	}
 
-	if patches, err = setDefaults(patches, newObj); err != nil {
+	if patches, err = setDefaults(ctx, patches, newObj); err != nil {
 		logger.Errorw("Failed the resource specific defaulter", zap.Error(err))
 		// Return the error message as-is to give the defaulter callback
 		// discretion over (our portion of) the message that the user sees.
 		return nil, err
 	}
 
-	if patches, err = setAnnotations(patches, newObj, oldObj, &req.UserInfo); err != nil {
+	if patches, err = setAnnotations(ctx, patches, newObj, oldObj, &req.UserInfo); err != nil {
 		logger.Errorw("Failed the resource annotator", zap.Error(err))
 		return nil, perrors.Wrap(err, "error setting annotations")
 	}
@@ -577,7 +588,7 @@ func (ac *AdmissionController) mutate(ctx context.Context, req *admissionv1beta1
 	if newObj == nil {
 		return nil, errMissingNewObject
 	}
-	if err := validate(oldObj, newObj); err != nil {
+	if err := validate(ctx, oldObj, newObj); err != nil {
 		logger.Errorw("Failed the resource specific validation", zap.Error(err))
 		// Return the error message as-is to give the validation callback
 		// discretion over (our portion of) the message that the user sees.

webhook/webhook_integration_test.go

@@ -18,6 +18,7 @@ package webhook
 
 import (
 	"bytes"
+	"context"
 	"crypto/tls"
 	"encoding/json"
 	"fmt"
@@ -27,8 +28,10 @@ import (
 	"testing"
 	"time"
 
+	. "github.com/knative/pkg/testing"
 	"github.com/mattbaird/jsonpatch"
 	admissionv1beta1 "k8s.io/api/admission/v1beta1"
+	authenticationv1 "k8s.io/api/authentication/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
@@ -216,6 +219,104 @@ func TestValidResponseForResource(t *testing.T) {
 	}
 }
 
+func TestValidResponseForResourceWithContextDefault(t *testing.T) {
+	ac, serverURL, err := testSetup(t)
+	if err != nil {
+		t.Fatalf("testSetup() = %v", err)
+	}
+	theDefault := "Some default value"
+	ac.WithContext = func(ctx context.Context) context.Context {
+		return WithValue(ctx, theDefault)
+	}
+
+	stopCh := make(chan struct{})
+	defer close(stopCh)
+
+	go func() {
+		err := ac.Run(stopCh)
+		if err != nil {
+			t.Fatalf("Unable to run controller: %s", err)
+		}
+	}()
+
+	pollErr := waitForServerAvailable(t, serverURL, testTimeout)
+	if pollErr != nil {
+		t.Fatalf("waitForServerAvailable() = %v", err)
+	}
+	tlsClient, err := createSecureTLSClient(t, ac.Client, &ac.Options)
+	if err != nil {
+		t.Fatalf("createSecureTLSClient() = %v", err)
+	}
+
+	admissionreq := &admissionv1beta1.AdmissionRequest{
+		Operation: admissionv1beta1.Create,
+		Kind: metav1.GroupVersionKind{
+			Group:   "pkg.knative.dev",
+			Version: "v1alpha1",
+			Kind:    "Resource",
+		},
+	}
+	testRev := createResource("testrev")
+	marshaled, err := json.Marshal(testRev)
+	if err != nil {
+		t.Fatalf("Failed to marshal resource: %s", err)
+	}
+
+	admissionreq.Object.Raw = marshaled
+	rev := &admissionv1beta1.AdmissionReview{
+		Request: admissionreq,
+	}
+
+	reqBuf := new(bytes.Buffer)
+	err = json.NewEncoder(reqBuf).Encode(&rev)
+	if err != nil {
+		t.Fatalf("Failed to marshal admission review: %v", err)
+	}
+
+	req, err := http.NewRequest("GET", fmt.Sprintf("https://%s", serverURL), reqBuf)
+	if err != nil {
+		t.Fatalf("http.NewRequest() = %v", err)
+	}
+	req.Header.Add("Content-Type", "application/json")
+
+	response, err := tlsClient.Do(req)
+	if err != nil {
+		t.Fatalf("Failed to get response %v", err)
+	}
+
+	if got, want := response.StatusCode, http.StatusOK; got != want {
+		t.Errorf("Response status code = %v, wanted %v", got, want)
+	}
+
+	defer response.Body.Close()
+	responseBody, err := ioutil.ReadAll(response.Body)
+	if err != nil {
+		t.Fatalf("Failed to read response body %v", err)
+	}
+
+	reviewResponse := admissionv1beta1.AdmissionReview{}
+
+	err = json.NewDecoder(bytes.NewReader(responseBody)).Decode(&reviewResponse)
+	if err != nil {
+		t.Fatalf("Failed to decode response: %v", err)
+	}
+
+	expectPatches(t, reviewResponse.Response.Patch, []jsonpatch.JsonPatchOperation{{
+		Operation: "add",
+		Path:      "/spec/fieldThatsImmutableWithDefault",
+		Value:     "this is another default value",
+	}, {
+		Operation: "add",
+		Path:      "/spec/fieldWithDefault",
+		Value:     "I'm a default.",
+	}, {
+		Operation: "add",
+		Path:      "/spec/fieldWithContextDefault",
+		Value:     theDefault,
+	}, setUserAnnotation("", ""),
+	})
+}
+
 func TestInvalidResponseForResource(t *testing.T) {
 	ac, serverURL, err := testSetup(t)
 	if err != nil {
@@ -256,6 +357,9 @@ func TestInvalidResponseForResource(t *testing.T) {
 			Version: "v1alpha1",
 			Kind:    "Resource",
 		},
+		UserInfo: authenticationv1.UserInfo{
+			Username: user1,
+		},
 	}
 
 	admissionreq.Object.Raw = marshaled

webhook/webhook_test.go

@@ -179,17 +179,41 @@ func TestValidCreateResourceSucceedsWithDefaultPatch(t *testing.T) {
 	_, ac := newNonRunningTestAdmissionController(t, newDefaultOptions())
 	resp := ac.admit(TestContextWithLogger(t), createCreateResource(r))
 	expectAllowed(t, resp)
-	expectPatches(t, resp.Patch, []jsonpatch.JsonPatchOperation{
-		{
-			Operation: "add",
-			Path:      "/spec/fieldThatsImmutableWithDefault",
-			Value:     "this is another default value",
-		}, {
-			Operation: "add",
-			Path:      "/spec/fieldWithDefault",
-			Value:     "I'm a default.",
-		},
-		setUserAnnotation(user1, user1),
+	expectPatches(t, resp.Patch, []jsonpatch.JsonPatchOperation{{
+		Operation: "add",
+		Path:      "/spec/fieldThatsImmutableWithDefault",
+		Value:     "this is another default value",
+	}, {
+		Operation: "add",
+		Path:      "/spec/fieldWithDefault",
+		Value:     "I'm a default.",
+	}, setUserAnnotation(user1, user1),
+	})
+}
+
+func TestValidCreateResourceSucceedsWithDefaultPatchWithContext(t *testing.T) {
+	r := createResource("a name")
+	_, ac := newNonRunningTestAdmissionController(t, newDefaultOptions())
+	ctx := TestContextWithLogger(t)
+
+	contextDefault := "I came from context y'all"
+	ctx = WithValue(ctx, contextDefault)
+
+	resp := ac.admit(ctx, createCreateResource(r))
+	expectAllowed(t, resp)
+	expectPatches(t, resp.Patch, []jsonpatch.JsonPatchOperation{{
+		Operation: "add",
+		Path:      "/spec/fieldThatsImmutableWithDefault",
+		Value:     "this is another default value",
+	}, {
+		Operation: "add",
+		Path:      "/spec/fieldWithDefault",
+		Value:     "I'm a default.",
+	}, {
+		Operation: "add",
+		Path:      "/spec/fieldWithContextDefault",
+		Value:     contextDefault,
+	}, setUserAnnotation(user1, user1),
 	})
 }