Split apart defaulting and validation webhooks

Related: knative/pkg#848

Gopkg.toml

@@ -31,7 +31,10 @@ required = [
 
 [[override]]
   name = "knative.dev/pkg"
-  branch = "master"
+  #TODO(mattmoor): DO NOT SUBMIT
+  source = "github.com/mattmoor/pkg-1"
+  revision = "21993fe00e9295c06a00cc992fb209f6834f2ef8"
+  # branch = "master"
 
 [[constraint]]
   name = "knative.dev/caching"

cmd/webhook/main.go

@@ -30,6 +30,8 @@ import (
 	"knative.dev/pkg/webhook/certificates"
 	"knative.dev/pkg/webhook/configmaps"
 	"knative.dev/pkg/webhook/resourcesemantics"
+	"knative.dev/pkg/webhook/resourcesemantics/defaulting"
+	"knative.dev/pkg/webhook/resourcesemantics/validation"
 
 	// resource validation types
 	autoscalingv1alpha1 "knative.dev/serving/pkg/apis/autoscaling/v1alpha1"
@@ -51,49 +53,70 @@ import (
 	domainconfig "knative.dev/serving/pkg/reconciler/route/config"
 )
 
-func NewResourceAdmissionController(ctx context.Context, cmw configmap.Watcher) *controller.Impl {
+var types = map[schema.GroupVersionKind]resourcesemantics.GenericCRD{
+	v1alpha1.SchemeGroupVersion.WithKind("Revision"):      &v1alpha1.Revision{},
+	v1alpha1.SchemeGroupVersion.WithKind("Configuration"): &v1alpha1.Configuration{},
+	v1alpha1.SchemeGroupVersion.WithKind("Route"):         &v1alpha1.Route{},
+	v1alpha1.SchemeGroupVersion.WithKind("Service"):       &v1alpha1.Service{},
+	v1beta1.SchemeGroupVersion.WithKind("Revision"):       &v1beta1.Revision{},
+	v1beta1.SchemeGroupVersion.WithKind("Configuration"):  &v1beta1.Configuration{},
+	v1beta1.SchemeGroupVersion.WithKind("Route"):          &v1beta1.Route{},
+	v1beta1.SchemeGroupVersion.WithKind("Service"):        &v1beta1.Service{},
+	v1.SchemeGroupVersion.WithKind("Revision"):            &v1.Revision{},
+	v1.SchemeGroupVersion.WithKind("Configuration"):       &v1.Configuration{},
+	v1.SchemeGroupVersion.WithKind("Route"):               &v1.Route{},
+	v1.SchemeGroupVersion.WithKind("Service"):             &v1.Service{},
+
+	autoscalingv1alpha1.SchemeGroupVersion.WithKind("PodAutoscaler"): &autoscalingv1alpha1.PodAutoscaler{},
+	autoscalingv1alpha1.SchemeGroupVersion.WithKind("Metric"):        &autoscalingv1alpha1.Metric{},
+
+	net.SchemeGroupVersion.WithKind("Certificate"):       &net.Certificate{},
+	net.SchemeGroupVersion.WithKind("Ingress"):           &net.Ingress{},
+	net.SchemeGroupVersion.WithKind("ServerlessService"): &net.ServerlessService{},
+}
+
+func NewDefaultingAdmissionController(ctx context.Context, cmw configmap.Watcher) *controller.Impl {
 	// Decorate contexts with the current state of the config.
 	store := defaultconfig.NewStore(logging.FromContext(ctx).Named("config-store"))
 	store.WatchConfigs(cmw)
-	ctxFunc := func(ctx context.Context) context.Context {
-		return v1.WithUpgradeViaDefaulting(store.ToContext(ctx))
-	}
 
-	return resourcesemantics.NewAdmissionController(ctx,
+	return defaulting.NewAdmissionController(ctx,
 
 		// Name of the resource webhook.
-		// TODO(mattmoor): This can be changed after 0.10, once the lifecycle of
-		// this object is not managed by OwnerReferences.
-		"webhook.serving.knative.dev",
+		"defaulting.webhook.serving.knative.dev",
 
 		// The path on which to serve the webhook.
-		"/",
+		"/defaulting",
 
 		// The resources to validate and default.
-		map[schema.GroupVersionKind]resourcesemantics.GenericCRD{
-			v1alpha1.SchemeGroupVersion.WithKind("Revision"):      &v1alpha1.Revision{},
-			v1alpha1.SchemeGroupVersion.WithKind("Configuration"): &v1alpha1.Configuration{},
-			v1alpha1.SchemeGroupVersion.WithKind("Route"):         &v1alpha1.Route{},
-			v1alpha1.SchemeGroupVersion.WithKind("Service"):       &v1alpha1.Service{},
-			v1beta1.SchemeGroupVersion.WithKind("Revision"):       &v1beta1.Revision{},
-			v1beta1.SchemeGroupVersion.WithKind("Configuration"):  &v1beta1.Configuration{},
-			v1beta1.SchemeGroupVersion.WithKind("Route"):          &v1beta1.Route{},
-			v1beta1.SchemeGroupVersion.WithKind("Service"):        &v1beta1.Service{},
-			v1.SchemeGroupVersion.WithKind("Revision"):            &v1.Revision{},
-			v1.SchemeGroupVersion.WithKind("Configuration"):       &v1.Configuration{},
-			v1.SchemeGroupVersion.WithKind("Route"):               &v1.Route{},
-			v1.SchemeGroupVersion.WithKind("Service"):             &v1.Service{},
-
-			autoscalingv1alpha1.SchemeGroupVersion.WithKind("PodAutoscaler"): &autoscalingv1alpha1.PodAutoscaler{},
-			autoscalingv1alpha1.SchemeGroupVersion.WithKind("Metric"):        &autoscalingv1alpha1.Metric{},
-
-			net.SchemeGroupVersion.WithKind("Certificate"):       &net.Certificate{},
-			net.SchemeGroupVersion.WithKind("Ingress"):           &net.Ingress{},
-			net.SchemeGroupVersion.WithKind("ServerlessService"): &net.ServerlessService{},
+		types,
+
+		// A function that infuses the context passed to Validate/SetDefaults with custom metadata.
+		func(ctx context.Context) context.Context {
+			return v1.WithUpgradeViaDefaulting(store.ToContext(ctx))
 		},
 
+		// Whether to disallow unknown fields.
+		true,
+	)
+}
+
+func NewValidationAdmissionController(ctx context.Context, cmw configmap.Watcher) *controller.Impl {
+	return validation.NewAdmissionController(ctx,
+
+		// Name of the resource webhook.
+		"validation.webhook.serving.knative.dev",
+
+		// The path on which to serve the webhook.
+		"/",
+
+		// The resources to validate and default.
+		types,
+
 		// A function that infuses the context passed to Validate/SetDefaults with custom metadata.
-		ctxFunc,
+		func(ctx context.Context) context.Context {
+			return ctx
+		},
 
 		// Whether to disallow unknown fields.
 		true,
@@ -136,7 +159,8 @@ func main() {
 
 	sharedmain.MainWithContext(ctx, "webhook",
 		certificates.NewController,
-		NewResourceAdmissionController,
+		NewDefaultingAdmissionController,
+		NewValidationAdmissionController,
 		NewConfigValidationController,
 	)
 }

config/500-webhook-configuration.yaml

@@ -15,7 +15,7 @@
 apiVersion: admissionregistration.k8s.io/v1beta1
 kind: MutatingWebhookConfiguration
 metadata:
-  name: webhook.serving.knative.dev
+  name: defaulting.webhook.serving.knative.dev
   labels:
     serving.knative.dev/release: devel
 webhooks:
@@ -26,7 +26,23 @@ webhooks:
       name: webhook
       namespace: knative-serving
   failurePolicy: Fail
-  name: webhook.serving.knative.dev
+  name: defaulting.webhook.serving.knative.dev
+---
+apiVersion: admissionregistration.k8s.io/v1beta1
+kind: ValidatingWebhookConfiguration
+metadata:
+  name: validation.webhook.serving.knative.dev
+  labels:
+    serving.knative.dev/release: devel
+webhooks:
+- admissionReviewVersions:
+  - v1beta1
+  clientConfig:
+    service:
+      name: webhook
+      namespace: knative-serving
+  failurePolicy: Fail
+  name: validation.webhook.serving.knative.dev
 ---
 apiVersion: admissionregistration.k8s.io/v1beta1
 kind: ValidatingWebhookConfiguration

vendor/knative.dev/pkg/OWNERS_ALIASES

@@ -19,11 +19,6 @@ aliases:
   - vaikas-google
   - vaikas
 
-  cloudevents-approvers:
-  - n3wscott
-  - vaikas-google
-  - vaikas
-
   configmap-approvers:
   - mattmoor
   - mdemirhan

vendor/knative.dev/pkg/RELEASING.md

@@ -88,3 +88,13 @@ their own release branches, so to update the `knative/pkg` dependency we run:
 dep ensure -update knative.dev/pkg
 ./hack/update-deps.sh
 ```
+
+## Revert to Master
+
+Post release, reverse the process. `Gopkg.toml` should look like:
+
+```toml
+[[override]]
+  name = "knative.dev/pkg"
+  branch = "master"
+```

vendor/knative.dev/pkg/apis/interfaces.go

@@ -44,15 +44,6 @@ type Convertible interface {
 	ConvertDown(ctx context.Context, from Convertible) error
 }
 
-// Immutable indicates that a particular type has fields that should
-// not change after creation.
-// DEPRECATED: Use WithinUpdate / GetBaseline from within Validatable instead.
-type Immutable interface {
-	// CheckImmutableFields checks that the current instance's immutable
-	// fields haven't changed from the provided original.
-	CheckImmutableFields(ctx context.Context, original Immutable) *FieldError
-}
-
 // Listable indicates that a particular type can be returned via the returned
 // list type by the API server.
 type Listable interface {