Skip to content

Commit

Permalink
Split apart defaulting and validation webhooks
Browse files Browse the repository at this point in the history
  • Loading branch information
mattmoor committed Nov 9, 2019
1 parent 2776fb0 commit 22d08eb
Show file tree
Hide file tree
Showing 60 changed files with 801 additions and 2,096 deletions.
11 changes: 7 additions & 4 deletions Gopkg.lock

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

5 changes: 4 additions & 1 deletion Gopkg.toml
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,10 @@ required = [

[[override]]
name = "knative.dev/pkg"
branch = "master"
#TODO(mattmoor): DO NOT SUBMIT
source = "github.com/mattmoor/pkg-1"
revision = "6058c9300faf3145ed64c7936f9a53c0d3b1fe9d"
# branch = "master"

[[constraint]]
name = "knative.dev/caching"
Expand Down
88 changes: 58 additions & 30 deletions cmd/webhook/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -51,49 +53,74 @@ 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",

// 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.
// TODO(mattmoor): We must leave this as '/' because on downgrade the
// validating webhook configuration will be left around and it should exist
// at a path that is configured to handle requests or all mutations will be
// denied.
"/",

// 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,
Expand Down Expand Up @@ -136,7 +163,8 @@ func main() {

sharedmain.MainWithContext(ctx, "webhook",
certificates.NewController,
NewResourceAdmissionController,
NewDefaultingAdmissionController,
NewValidationAdmissionController,
NewConfigValidationController,
)
}
16 changes: 16 additions & 0 deletions config/500-webhook-configuration.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,22 @@ webhooks:
---
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
metadata:
name: config.webhook.serving.knative.dev
labels:
Expand Down
4 changes: 4 additions & 0 deletions test/e2e-upgrade-tests.sh
Original file line number Diff line number Diff line change
Expand Up @@ -43,12 +43,16 @@ function install_latest_release() {
"${url}/serving.yaml" \
|| fail_test "Knative latest release installation failed"
wait_until_pods_running knative-serving
# Give it 30s to settle.
sleep 30
}

function install_head() {
header "Installing Knative head release"
install_knative_serving || fail_test "Knative head release installation failed"
wait_until_pods_running knative-serving
# Give it 30s to settle.
sleep 30
}

function knative_setup() {
Expand Down
1 change: 0 additions & 1 deletion vendor/knative.dev/pkg/Gopkg.lock

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

9 changes: 0 additions & 9 deletions vendor/knative.dev/pkg/OWNERS_ALIASES
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,10 @@ aliases:
pkg-approvers:
- evankanderson
- mattmoor
- vaikas-google
- vaikas

apis-approvers:
- mattmoor
- vaikas-google
- vaikas
- n3wscott

Expand All @@ -16,12 +14,6 @@ aliases:

apis-duck-approvers:
- mattmoor
- vaikas-google
- vaikas

cloudevents-approvers:
- n3wscott
- vaikas-google
- vaikas

configmap-approvers:
Expand Down Expand Up @@ -68,7 +60,6 @@ aliases:

source-approvers:
- n3wscott
- vaikas-google
- vaikas

webhook-approvers:
Expand Down
10 changes: 10 additions & 0 deletions vendor/knative.dev/pkg/RELEASING.md
Original file line number Diff line number Diff line change
Expand Up @@ -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"
```
1 change: 0 additions & 1 deletion vendor/knative.dev/pkg/apis/condition_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,6 @@ const (
// Conditions defines a readiness condition for a Knative resource.
// See: https://github.com/kubernetes/community/blob/master/contributors/devel/sig-architecture/api-conventions.md#typical-status-properties
// +k8s:deepcopy-gen=true
// +k8s:openapi-gen=true
type Condition struct {
// Type of condition.
// +required
Expand Down
93 changes: 93 additions & 0 deletions vendor/knative.dev/pkg/apis/duck/v1/destination.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
/*
Copyright 2019 The Knative 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 v1

import (
"context"

corev1 "k8s.io/api/core/v1"
"knative.dev/pkg/apis"
)

// Destination represents a target of an invocation over HTTP.
type Destination struct {
// Ref points to an Addressable.
// +optional
Ref *corev1.ObjectReference `json:"ref,omitempty"`

// URI can be an absolute URL(non-empty scheme and non-empty host) pointing to the target or a relative URI. Relative URIs will be resolved using the base URI retrieved from Ref.
// +optional
URI *apis.URL `json:"uri,omitempty"`
}

func (dest *Destination) Validate(ctx context.Context) *apis.FieldError {
if dest == nil {
return nil
}
return ValidateDestination(*dest).ViaField(apis.CurrentField)
}

// ValidateDestination validates Destination.
func ValidateDestination(dest Destination) *apis.FieldError {
var ref *corev1.ObjectReference
if dest.Ref != nil {
ref = dest.Ref
}
if ref == nil && dest.URI == nil {
return apis.ErrGeneric("expected at least one, got none", "ref", "uri")
}

if ref != nil && dest.URI != nil && dest.URI.URL().IsAbs() {
return apis.ErrGeneric("Absolute URI is not allowed when Ref or [apiVersion, kind, name] is present", "[apiVersion, kind, name]", "ref", "uri")
}
// IsAbs() check whether the URL has a non-empty scheme. Besides the non-empty scheme, we also require dest.URI has a non-empty host
if ref == nil && dest.URI != nil && (!dest.URI.URL().IsAbs() || dest.URI.Host == "") {
return apis.ErrInvalidValue("Relative URI is not allowed when Ref and [apiVersion, kind, name] is absent", "uri")
}
if ref != nil && dest.URI == nil {
if dest.Ref != nil {
return validateDestinationRef(*ref).ViaField("ref")
}
}
return nil
}

// GetRef gets the ObjectReference from this Destination, if one is present. If no ref is present,
// then nil is returned.
func (dest *Destination) GetRef() *corev1.ObjectReference {
if dest == nil {
return nil
}
return dest.Ref
}

func validateDestinationRef(ref corev1.ObjectReference) *apis.FieldError {
// Check the object.
var errs *apis.FieldError
// Required Fields
if ref.Name == "" {
errs = errs.Also(apis.ErrMissingField("name"))
}
if ref.APIVersion == "" {
errs = errs.Also(apis.ErrMissingField("apiVersion"))
}
if ref.Kind == "" {
errs = errs.Also(apis.ErrMissingField("kind"))
}

return errs
}
Loading

0 comments on commit 22d08eb

Please sign in to comment.