mirror of https://github.com/knative/pkg.git
Split the resource semantic webhooks into separate AdmissionControllers (#848)
By combining our validation logic into our mutating webhook we were previously allowing for mutating webhooks evaluated after our own to modify our resources into invalid shapes. There are no guarantees around ordering of mutating webhooks (that I could find), so the only way to remedy this properly is to split apart the two into separate webhook configurations: - `defaulting`: which runs during the mutating admission webhook phase - `validation`: which runs during the validating admission webhook phase. The diagram in [this post](https://kubernetes.io/blog/2019/03/21/a-guide-to-kubernetes-admission-controllers/) is very helpful in illustrating the flow of webhooks. Fixes: https://github.com/knative/pkg/issues/847
This commit is contained in:
parent
3525756c2a
commit
4836f680bb
|
@ -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 {
|
||||
|
|
|
@ -38,7 +38,6 @@ type Resource struct {
|
|||
// Check that Resource may be validated and defaulted.
|
||||
var _ apis.Validatable = (*Resource)(nil)
|
||||
var _ apis.Defaultable = (*Resource)(nil)
|
||||
var _ apis.Immutable = (*Resource)(nil)
|
||||
var _ apis.Listable = (*Resource)(nil)
|
||||
|
||||
// ResourceSpec represents test resource spec.
|
||||
|
@ -105,12 +104,7 @@ func (cs *ResourceSpec) Validate(ctx context.Context) *apis.FieldError {
|
|||
return nil
|
||||
}
|
||||
|
||||
func (current *Resource) CheckImmutableFields(ctx context.Context, og apis.Immutable) *apis.FieldError {
|
||||
original, ok := og.(*Resource)
|
||||
if !ok {
|
||||
return &apis.FieldError{Message: "The provided original was not a Resource"}
|
||||
}
|
||||
|
||||
func (current *Resource) CheckImmutableFields(ctx context.Context, original *Resource) *apis.FieldError {
|
||||
if original.Spec.FieldThatsImmutable != current.Spec.FieldThatsImmutable {
|
||||
return &apis.FieldError{
|
||||
Message: "Immutable field changed",
|
||||
|
|
|
@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
|
|||
limitations under the License.
|
||||
*/
|
||||
|
||||
package resourcesemantics
|
||||
package defaulting
|
||||
|
||||
import (
|
||||
"context"
|
||||
|
@ -30,13 +30,14 @@ import (
|
|||
"knative.dev/pkg/logging"
|
||||
"knative.dev/pkg/system"
|
||||
"knative.dev/pkg/webhook"
|
||||
"knative.dev/pkg/webhook/resourcesemantics"
|
||||
)
|
||||
|
||||
// NewAdmissionController constructs a reconciler
|
||||
func NewAdmissionController(
|
||||
ctx context.Context,
|
||||
name, path string,
|
||||
handlers map[schema.GroupVersionKind]GenericCRD,
|
||||
handlers map[schema.GroupVersionKind]resourcesemantics.GenericCRD,
|
||||
wc func(context.Context) context.Context,
|
||||
disallowUnknownFields bool,
|
||||
) *controller.Impl {
|
|
@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
|
|||
limitations under the License.
|
||||
*/
|
||||
|
||||
package resourcesemantics
|
||||
package defaulting
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
|
@ -30,7 +30,6 @@ import (
|
|||
"go.uber.org/zap"
|
||||
admissionv1beta1 "k8s.io/api/admission/v1beta1"
|
||||
admissionregistrationv1beta1 "k8s.io/api/admissionregistration/v1beta1"
|
||||
"k8s.io/apimachinery/pkg/runtime"
|
||||
"k8s.io/apimachinery/pkg/runtime/schema"
|
||||
"k8s.io/client-go/kubernetes"
|
||||
admissionlisters "k8s.io/client-go/listers/admissionregistration/v1beta1"
|
||||
|
@ -44,23 +43,16 @@ import (
|
|||
"knative.dev/pkg/system"
|
||||
"knative.dev/pkg/webhook"
|
||||
certresources "knative.dev/pkg/webhook/certificates/resources"
|
||||
"knative.dev/pkg/webhook/resourcesemantics"
|
||||
)
|
||||
|
||||
// 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 {
|
||||
apis.Defaultable
|
||||
apis.Validatable
|
||||
runtime.Object
|
||||
}
|
||||
|
||||
var errMissingNewObject = errors.New("the new object may not be nil")
|
||||
|
||||
// reconciler implements the AdmissionController for resources
|
||||
type reconciler struct {
|
||||
name string
|
||||
path string
|
||||
handlers map[schema.GroupVersionKind]GenericCRD
|
||||
handlers map[schema.GroupVersionKind]resourcesemantics.GenericCRD
|
||||
|
||||
withContext func(context.Context) context.Context
|
||||
|
||||
|
@ -217,10 +209,10 @@ func (ac *reconciler) mutate(ctx context.Context, req *admissionv1beta1.Admissio
|
|||
}
|
||||
|
||||
// nil values denote absence of `old` (create) or `new` (delete) objects.
|
||||
var oldObj, newObj GenericCRD
|
||||
var oldObj, newObj resourcesemantics.GenericCRD
|
||||
|
||||
if len(newBytes) != 0 {
|
||||
newObj = handler.DeepCopyObject().(GenericCRD)
|
||||
newObj = handler.DeepCopyObject().(resourcesemantics.GenericCRD)
|
||||
newDecoder := json.NewDecoder(bytes.NewBuffer(newBytes))
|
||||
if ac.disallowUnknownFields {
|
||||
newDecoder.DisallowUnknownFields()
|
||||
|
@ -230,7 +222,7 @@ func (ac *reconciler) mutate(ctx context.Context, req *admissionv1beta1.Admissio
|
|||
}
|
||||
}
|
||||
if len(oldBytes) != 0 {
|
||||
oldObj = handler.DeepCopyObject().(GenericCRD)
|
||||
oldObj = handler.DeepCopyObject().(resourcesemantics.GenericCRD)
|
||||
oldDecoder := json.NewDecoder(bytes.NewBuffer(oldBytes))
|
||||
if ac.disallowUnknownFields {
|
||||
oldDecoder.DisallowUnknownFields()
|
||||
|
@ -258,7 +250,7 @@ func (ac *reconciler) mutate(ctx context.Context, req *admissionv1beta1.Admissio
|
|||
if oldObj != nil {
|
||||
// Copy the old object and set defaults so that we don't reject our own
|
||||
// defaulting done earlier in the webhook.
|
||||
oldObj = oldObj.DeepCopyObject().(GenericCRD)
|
||||
oldObj = oldObj.DeepCopyObject().(resourcesemantics.GenericCRD)
|
||||
oldObj.SetDefaults(ctx)
|
||||
|
||||
s, ok := oldObj.(apis.HasSpec)
|
||||
|
@ -293,17 +285,10 @@ func (ac *reconciler) mutate(ctx context.Context, req *admissionv1beta1.Admissio
|
|||
if newObj == nil {
|
||||
return nil, errMissingNewObject
|
||||
}
|
||||
if err := validate(ctx, 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.
|
||||
return nil, err
|
||||
}
|
||||
|
||||
return json.Marshal(patches)
|
||||
}
|
||||
|
||||
func (ac *reconciler) setUserInfoAnnotations(ctx context.Context, patches duck.JSONPatch, new GenericCRD, groupName string) (duck.JSONPatch, error) {
|
||||
func (ac *reconciler) setUserInfoAnnotations(ctx context.Context, patches duck.JSONPatch, new resourcesemantics.GenericCRD, groupName string) (duck.JSONPatch, error) {
|
||||
if new == nil {
|
||||
return patches, nil
|
||||
}
|
||||
|
@ -341,33 +326,8 @@ func roundTripPatch(bytes []byte, unmarshalled interface{}) (duck.JSONPatch, err
|
|||
return jsonpatch.CreatePatch(bytes, marshaledBytes)
|
||||
}
|
||||
|
||||
// validate performs validation on the provided "new" CRD.
|
||||
// For legacy purposes, this also does apis.Immutable validation,
|
||||
// which is deprecated and will be removed in a future release.
|
||||
func validate(ctx context.Context, new apis.Validatable) error {
|
||||
if apis.IsInUpdate(ctx) {
|
||||
old := apis.GetBaseline(ctx)
|
||||
if immutableNew, ok := new.(apis.Immutable); ok {
|
||||
immutableOld, ok := old.(apis.Immutable)
|
||||
if !ok {
|
||||
return fmt.Errorf("unexpected type mismatch %T vs. %T", old, new)
|
||||
}
|
||||
if err := immutableNew.CheckImmutableFields(ctx, immutableOld); err != nil {
|
||||
return err
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Can't just `return new.Validate()` because it doesn't properly nil-check.
|
||||
if err := new.Validate(ctx); err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
// setDefaults simply leverages apis.Defaultable to set defaults.
|
||||
func setDefaults(ctx context.Context, patches duck.JSONPatch, crd GenericCRD) (duck.JSONPatch, error) {
|
||||
func setDefaults(ctx context.Context, patches duck.JSONPatch, crd resourcesemantics.GenericCRD) (duck.JSONPatch, error) {
|
||||
before, after := crd.DeepCopyObject(), crd
|
||||
after.SetDefaults(ctx)
|
||||
|
|
@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
|
|||
limitations under the License.
|
||||
*/
|
||||
|
||||
package resourcesemantics
|
||||
package defaulting
|
||||
|
||||
import (
|
||||
"context"
|
||||
|
@ -42,6 +42,7 @@ import (
|
|||
. "knative.dev/pkg/logging/testing"
|
||||
. "knative.dev/pkg/reconciler/testing"
|
||||
. "knative.dev/pkg/testing"
|
||||
"knative.dev/pkg/webhook/resourcesemantics"
|
||||
. "knative.dev/pkg/webhook/testing"
|
||||
)
|
||||
|
||||
|
@ -53,7 +54,7 @@ const (
|
|||
)
|
||||
|
||||
var (
|
||||
handlers = map[schema.GroupVersionKind]GenericCRD{
|
||||
handlers = map[schema.GroupVersionKind]resourcesemantics.GenericCRD{
|
||||
{
|
||||
Group: "pkg.knative.dev",
|
||||
Version: "v1alpha1",
|
||||
|
@ -292,13 +293,6 @@ func TestAdmitCreates(t *testing.T) {
|
|||
Path: "/metadata/annotations/pkg.knative.dev~1creator",
|
||||
Value: user1,
|
||||
}},
|
||||
}, {
|
||||
name: "with bad field",
|
||||
setup: func(ctx context.Context, r *Resource) {
|
||||
// Put a bad value in.
|
||||
r.Spec.FieldWithValidation = "not what's expected"
|
||||
},
|
||||
rejection: "invalid value",
|
||||
}}
|
||||
|
||||
for _, tc := range tests {
|
||||
|
@ -399,24 +393,6 @@ func TestAdmitUpdates(t *testing.T) {
|
|||
Path: "/spec/fieldThatsImmutableWithDefault",
|
||||
Value: "this is another default value",
|
||||
}},
|
||||
}, {
|
||||
name: "bad mutation (immutable)",
|
||||
setup: func(ctx context.Context, r *Resource) {
|
||||
r.SetDefaults(ctx)
|
||||
},
|
||||
mutate: func(ctx context.Context, r *Resource) {
|
||||
r.Spec.FieldThatsImmutableWithDefault = "something different"
|
||||
},
|
||||
rejection: "Immutable field changed",
|
||||
}, {
|
||||
name: "bad mutation (validation)",
|
||||
setup: func(ctx context.Context, r *Resource) {
|
||||
r.SetDefaults(ctx)
|
||||
},
|
||||
mutate: func(ctx context.Context, r *Resource) {
|
||||
r.Spec.FieldWithValidation = "not what's expected"
|
||||
},
|
||||
rejection: "invalid value",
|
||||
}}
|
||||
|
||||
for _, tc := range tests {
|
|
@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
|
|||
limitations under the License.
|
||||
*/
|
||||
|
||||
package resourcesemantics
|
||||
package defaulting
|
||||
|
||||
import (
|
||||
"context"
|
||||
|
@ -35,6 +35,7 @@ import (
|
|||
"knative.dev/pkg/system"
|
||||
"knative.dev/pkg/webhook"
|
||||
certresources "knative.dev/pkg/webhook/certificates/resources"
|
||||
"knative.dev/pkg/webhook/resourcesemantics"
|
||||
|
||||
. "knative.dev/pkg/reconciler/testing"
|
||||
. "knative.dev/pkg/webhook/testing"
|
||||
|
@ -330,7 +331,7 @@ func TestNew(t *testing.T) {
|
|||
ctx = webhook.WithOptions(ctx, webhook.Options{})
|
||||
|
||||
c := NewAdmissionController(ctx, "foo", "/bar",
|
||||
map[schema.GroupVersionKind]GenericCRD{},
|
||||
map[schema.GroupVersionKind]resourcesemantics.GenericCRD{},
|
||||
func(ctx context.Context) context.Context {
|
||||
return ctx
|
||||
}, true /* disallow unknown field */)
|
|
@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
|
|||
limitations under the License.
|
||||
*/
|
||||
|
||||
package resourcesemantics
|
||||
package defaulting
|
||||
|
||||
import (
|
||||
"context"
|
|
@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
|
|||
limitations under the License.
|
||||
*/
|
||||
|
||||
package resourcesemantics
|
||||
package defaulting
|
||||
|
||||
import (
|
||||
"context"
|
|
@ -0,0 +1,30 @@
|
|||
/*
|
||||
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 resourcesemantics
|
||||
|
||||
import (
|
||||
"k8s.io/apimachinery/pkg/runtime"
|
||||
"knative.dev/pkg/apis"
|
||||
)
|
||||
|
||||
// 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 {
|
||||
apis.Defaultable
|
||||
apis.Validatable
|
||||
runtime.Object
|
||||
}
|
|
@ -0,0 +1,84 @@
|
|||
/*
|
||||
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 validation
|
||||
|
||||
import (
|
||||
"context"
|
||||
|
||||
// Injection stuff
|
||||
kubeclient "knative.dev/pkg/client/injection/kube/client"
|
||||
vwhinformer "knative.dev/pkg/client/injection/kube/informers/admissionregistration/v1beta1/validatingwebhookconfiguration"
|
||||
secretinformer "knative.dev/pkg/client/injection/kube/informers/core/v1/secret"
|
||||
|
||||
"k8s.io/apimachinery/pkg/runtime/schema"
|
||||
"k8s.io/client-go/tools/cache"
|
||||
"knative.dev/pkg/controller"
|
||||
"knative.dev/pkg/logging"
|
||||
"knative.dev/pkg/system"
|
||||
"knative.dev/pkg/webhook"
|
||||
"knative.dev/pkg/webhook/resourcesemantics"
|
||||
)
|
||||
|
||||
// NewAdmissionController constructs a reconciler
|
||||
func NewAdmissionController(
|
||||
ctx context.Context,
|
||||
name, path string,
|
||||
handlers map[schema.GroupVersionKind]resourcesemantics.GenericCRD,
|
||||
wc func(context.Context) context.Context,
|
||||
disallowUnknownFields bool,
|
||||
) *controller.Impl {
|
||||
|
||||
client := kubeclient.Get(ctx)
|
||||
vwhInformer := vwhinformer.Get(ctx)
|
||||
secretInformer := secretinformer.Get(ctx)
|
||||
options := webhook.GetOptions(ctx)
|
||||
|
||||
wh := &reconciler{
|
||||
name: name,
|
||||
path: path,
|
||||
handlers: handlers,
|
||||
|
||||
withContext: wc,
|
||||
disallowUnknownFields: disallowUnknownFields,
|
||||
secretName: options.SecretName,
|
||||
|
||||
client: client,
|
||||
vwhlister: vwhInformer.Lister(),
|
||||
secretlister: secretInformer.Lister(),
|
||||
}
|
||||
|
||||
logger := logging.FromContext(ctx)
|
||||
c := controller.NewImpl(wh, logger, "ConfigMapWebhook")
|
||||
|
||||
// Reconcile when the named ValidatingWebhookConfiguration changes.
|
||||
vwhInformer.Informer().AddEventHandler(cache.FilteringResourceEventHandler{
|
||||
FilterFunc: controller.FilterWithName(name),
|
||||
// It doesn't matter what we enqueue because we will always Reconcile
|
||||
// the named VWH resource.
|
||||
Handler: controller.HandleAll(c.Enqueue),
|
||||
})
|
||||
|
||||
// Reconcile when the cert bundle changes.
|
||||
secretInformer.Informer().AddEventHandler(cache.FilteringResourceEventHandler{
|
||||
FilterFunc: controller.FilterWithNameAndNamespace(system.Namespace(), wh.secretName),
|
||||
// It doesn't matter what we enqueue because we will always Reconcile
|
||||
// the named VWH resource.
|
||||
Handler: controller.HandleAll(c.Enqueue),
|
||||
})
|
||||
|
||||
return c
|
||||
}
|
|
@ -0,0 +1,342 @@
|
|||
/*
|
||||
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 validation
|
||||
|
||||
import (
|
||||
"context"
|
||||
"testing"
|
||||
|
||||
kubeclient "knative.dev/pkg/client/injection/kube/client/fake"
|
||||
_ "knative.dev/pkg/client/injection/kube/informers/core/v1/secret/fake"
|
||||
|
||||
admissionregistrationv1beta1 "k8s.io/api/admissionregistration/v1beta1"
|
||||
corev1 "k8s.io/api/core/v1"
|
||||
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
|
||||
"k8s.io/apimachinery/pkg/runtime"
|
||||
"k8s.io/apimachinery/pkg/runtime/schema"
|
||||
clientgotesting "k8s.io/client-go/testing"
|
||||
"knative.dev/pkg/configmap"
|
||||
"knative.dev/pkg/controller"
|
||||
"knative.dev/pkg/ptr"
|
||||
"knative.dev/pkg/system"
|
||||
"knative.dev/pkg/webhook"
|
||||
certresources "knative.dev/pkg/webhook/certificates/resources"
|
||||
"knative.dev/pkg/webhook/resourcesemantics"
|
||||
|
||||
. "knative.dev/pkg/reconciler/testing"
|
||||
. "knative.dev/pkg/webhook/testing"
|
||||
)
|
||||
|
||||
func TestReconcile(t *testing.T) {
|
||||
const name, path = "foo.bar.baz", "/blah"
|
||||
const secretName = "webhook-secret"
|
||||
|
||||
secret := &corev1.Secret{
|
||||
ObjectMeta: metav1.ObjectMeta{
|
||||
Name: secretName,
|
||||
Namespace: system.Namespace(),
|
||||
},
|
||||
Data: map[string][]byte{
|
||||
certresources.ServerKey: []byte("present"),
|
||||
certresources.ServerCert: []byte("present"),
|
||||
certresources.CACert: []byte("present"),
|
||||
},
|
||||
}
|
||||
|
||||
// These are the rules we expect given the context of "handlers".
|
||||
expectedRules := []admissionregistrationv1beta1.RuleWithOperations{{
|
||||
Operations: []admissionregistrationv1beta1.OperationType{"CREATE", "UPDATE"},
|
||||
Rule: admissionregistrationv1beta1.Rule{
|
||||
APIGroups: []string{"pkg.knative.dev"},
|
||||
APIVersions: []string{"v1alpha1"},
|
||||
Resources: []string{"innerdefaultresources/*"},
|
||||
},
|
||||
}, {
|
||||
Operations: []admissionregistrationv1beta1.OperationType{"CREATE", "UPDATE"},
|
||||
Rule: admissionregistrationv1beta1.Rule{
|
||||
APIGroups: []string{"pkg.knative.dev"},
|
||||
APIVersions: []string{"v1alpha1"},
|
||||
Resources: []string{"resources/*"},
|
||||
},
|
||||
}, {
|
||||
Operations: []admissionregistrationv1beta1.OperationType{"CREATE", "UPDATE"},
|
||||
Rule: admissionregistrationv1beta1.Rule{
|
||||
APIGroups: []string{"pkg.knative.dev"},
|
||||
APIVersions: []string{"v1beta1"},
|
||||
Resources: []string{"resources/*"},
|
||||
},
|
||||
}, {
|
||||
Operations: []admissionregistrationv1beta1.OperationType{"CREATE", "UPDATE"},
|
||||
Rule: admissionregistrationv1beta1.Rule{
|
||||
APIGroups: []string{"pkg.knative.io"},
|
||||
APIVersions: []string{"v1alpha1"},
|
||||
Resources: []string{"innerdefaultresources/*"},
|
||||
},
|
||||
}}
|
||||
|
||||
// The key to use, which for this singleton reconciler doesn't matter (although the
|
||||
// namespace matters for namespace validation).
|
||||
key := system.Namespace() + "/does not matter"
|
||||
|
||||
table := TableTest{{
|
||||
Name: "no secret",
|
||||
Key: key,
|
||||
WantErr: true,
|
||||
}, {
|
||||
Name: "secret missing CA Cert",
|
||||
Key: key,
|
||||
Objects: []runtime.Object{&corev1.Secret{
|
||||
ObjectMeta: metav1.ObjectMeta{
|
||||
Name: secretName,
|
||||
Namespace: system.Namespace(),
|
||||
},
|
||||
Data: map[string][]byte{
|
||||
certresources.ServerKey: []byte("present"),
|
||||
certresources.ServerCert: []byte("present"),
|
||||
// certresources.CACert: []byte("missing"),
|
||||
},
|
||||
}},
|
||||
WantErr: true,
|
||||
}, {
|
||||
Name: "secret exists, but VWH does not",
|
||||
Key: key,
|
||||
Objects: []runtime.Object{secret},
|
||||
WantErr: true,
|
||||
}, {
|
||||
Name: "secret and VWH exist, missing service reference",
|
||||
Key: key,
|
||||
Objects: []runtime.Object{secret,
|
||||
&admissionregistrationv1beta1.ValidatingWebhookConfiguration{
|
||||
ObjectMeta: metav1.ObjectMeta{
|
||||
Name: name,
|
||||
},
|
||||
Webhooks: []admissionregistrationv1beta1.ValidatingWebhook{{
|
||||
Name: name,
|
||||
}},
|
||||
},
|
||||
},
|
||||
WantErr: true,
|
||||
}, {
|
||||
Name: "secret and VWH exist, missing other stuff",
|
||||
Key: key,
|
||||
Objects: []runtime.Object{secret,
|
||||
&admissionregistrationv1beta1.ValidatingWebhookConfiguration{
|
||||
ObjectMeta: metav1.ObjectMeta{
|
||||
Name: name,
|
||||
},
|
||||
Webhooks: []admissionregistrationv1beta1.ValidatingWebhook{{
|
||||
Name: name,
|
||||
ClientConfig: admissionregistrationv1beta1.WebhookClientConfig{
|
||||
Service: &admissionregistrationv1beta1.ServiceReference{
|
||||
Namespace: system.Namespace(),
|
||||
Name: "webhook",
|
||||
},
|
||||
},
|
||||
}},
|
||||
},
|
||||
},
|
||||
WantUpdates: []clientgotesting.UpdateActionImpl{{
|
||||
Object: &admissionregistrationv1beta1.ValidatingWebhookConfiguration{
|
||||
ObjectMeta: metav1.ObjectMeta{
|
||||
Name: name,
|
||||
},
|
||||
Webhooks: []admissionregistrationv1beta1.ValidatingWebhook{{
|
||||
Name: name,
|
||||
ClientConfig: admissionregistrationv1beta1.WebhookClientConfig{
|
||||
Service: &admissionregistrationv1beta1.ServiceReference{
|
||||
Namespace: system.Namespace(),
|
||||
Name: "webhook",
|
||||
// Path is added.
|
||||
Path: ptr.String(path),
|
||||
},
|
||||
// CABundle is added.
|
||||
CABundle: []byte("present"),
|
||||
},
|
||||
// Rules are added.
|
||||
Rules: expectedRules,
|
||||
}},
|
||||
},
|
||||
}},
|
||||
}, {
|
||||
Name: "secret and VWH exist, added fields are incorrect",
|
||||
Key: key,
|
||||
Objects: []runtime.Object{secret,
|
||||
&admissionregistrationv1beta1.ValidatingWebhookConfiguration{
|
||||
ObjectMeta: metav1.ObjectMeta{
|
||||
Name: name,
|
||||
},
|
||||
Webhooks: []admissionregistrationv1beta1.ValidatingWebhook{{
|
||||
Name: name,
|
||||
ClientConfig: admissionregistrationv1beta1.WebhookClientConfig{
|
||||
Service: &admissionregistrationv1beta1.ServiceReference{
|
||||
Namespace: system.Namespace(),
|
||||
Name: "webhook",
|
||||
// Incorrect
|
||||
Path: ptr.String("incorrect"),
|
||||
},
|
||||
// Incorrect
|
||||
CABundle: []byte("incorrect"),
|
||||
},
|
||||
// Incorrect (really just incomplete)
|
||||
Rules: []admissionregistrationv1beta1.RuleWithOperations{{
|
||||
Operations: []admissionregistrationv1beta1.OperationType{"CREATE", "UPDATE"},
|
||||
Rule: admissionregistrationv1beta1.Rule{
|
||||
APIGroups: []string{"pkg.knative.dev"},
|
||||
APIVersions: []string{"v1alpha1"},
|
||||
Resources: []string{"innerdefaultresources/*"},
|
||||
},
|
||||
}},
|
||||
}},
|
||||
},
|
||||
},
|
||||
WantUpdates: []clientgotesting.UpdateActionImpl{{
|
||||
Object: &admissionregistrationv1beta1.ValidatingWebhookConfiguration{
|
||||
ObjectMeta: metav1.ObjectMeta{
|
||||
Name: name,
|
||||
},
|
||||
Webhooks: []admissionregistrationv1beta1.ValidatingWebhook{{
|
||||
Name: name,
|
||||
ClientConfig: admissionregistrationv1beta1.WebhookClientConfig{
|
||||
Service: &admissionregistrationv1beta1.ServiceReference{
|
||||
Namespace: system.Namespace(),
|
||||
Name: "webhook",
|
||||
// Path is fixed.
|
||||
Path: ptr.String(path),
|
||||
},
|
||||
// CABundle is fixed.
|
||||
CABundle: []byte("present"),
|
||||
},
|
||||
// Rules are fixed.
|
||||
Rules: expectedRules,
|
||||
}},
|
||||
},
|
||||
}},
|
||||
}, {
|
||||
Name: "failure updating VWH",
|
||||
Key: key,
|
||||
WantErr: true,
|
||||
WithReactors: []clientgotesting.ReactionFunc{
|
||||
InduceFailure("update", "validatingwebhookconfigurations"),
|
||||
},
|
||||
Objects: []runtime.Object{secret,
|
||||
&admissionregistrationv1beta1.ValidatingWebhookConfiguration{
|
||||
ObjectMeta: metav1.ObjectMeta{
|
||||
Name: name,
|
||||
},
|
||||
Webhooks: []admissionregistrationv1beta1.ValidatingWebhook{{
|
||||
Name: name,
|
||||
ClientConfig: admissionregistrationv1beta1.WebhookClientConfig{
|
||||
Service: &admissionregistrationv1beta1.ServiceReference{
|
||||
Namespace: system.Namespace(),
|
||||
Name: "webhook",
|
||||
// Incorrect
|
||||
Path: ptr.String("incorrect"),
|
||||
},
|
||||
// Incorrect
|
||||
CABundle: []byte("incorrect"),
|
||||
},
|
||||
// Incorrect (really just incomplete)
|
||||
Rules: []admissionregistrationv1beta1.RuleWithOperations{{
|
||||
Operations: []admissionregistrationv1beta1.OperationType{"CREATE", "UPDATE"},
|
||||
Rule: admissionregistrationv1beta1.Rule{
|
||||
APIGroups: []string{"pkg.knative.dev"},
|
||||
APIVersions: []string{"v1alpha1"},
|
||||
Resources: []string{"innerdefaultresources/*"},
|
||||
},
|
||||
}},
|
||||
}},
|
||||
},
|
||||
},
|
||||
WantUpdates: []clientgotesting.UpdateActionImpl{{
|
||||
Object: &admissionregistrationv1beta1.ValidatingWebhookConfiguration{
|
||||
ObjectMeta: metav1.ObjectMeta{
|
||||
Name: name,
|
||||
},
|
||||
Webhooks: []admissionregistrationv1beta1.ValidatingWebhook{{
|
||||
Name: name,
|
||||
ClientConfig: admissionregistrationv1beta1.WebhookClientConfig{
|
||||
Service: &admissionregistrationv1beta1.ServiceReference{
|
||||
Namespace: system.Namespace(),
|
||||
Name: "webhook",
|
||||
// Path is fixed.
|
||||
Path: ptr.String(path),
|
||||
},
|
||||
// CABundle is fixed.
|
||||
CABundle: []byte("present"),
|
||||
},
|
||||
// Rules are fixed.
|
||||
Rules: expectedRules,
|
||||
}},
|
||||
},
|
||||
}},
|
||||
}, {
|
||||
Name: ":fire: everything is fine :fire:",
|
||||
Key: key,
|
||||
Objects: []runtime.Object{secret,
|
||||
&admissionregistrationv1beta1.ValidatingWebhookConfiguration{
|
||||
ObjectMeta: metav1.ObjectMeta{
|
||||
Name: name,
|
||||
},
|
||||
Webhooks: []admissionregistrationv1beta1.ValidatingWebhook{{
|
||||
Name: name,
|
||||
ClientConfig: admissionregistrationv1beta1.WebhookClientConfig{
|
||||
Service: &admissionregistrationv1beta1.ServiceReference{
|
||||
Namespace: system.Namespace(),
|
||||
Name: "webhook",
|
||||
// Path is fine.
|
||||
Path: ptr.String(path),
|
||||
},
|
||||
// CABundle is fine.
|
||||
CABundle: []byte("present"),
|
||||
},
|
||||
// Rules are fine.
|
||||
Rules: expectedRules,
|
||||
}},
|
||||
},
|
||||
},
|
||||
}}
|
||||
|
||||
table.Test(t, MakeFactory(func(ctx context.Context, listers *Listers, cmw configmap.Watcher) controller.Reconciler {
|
||||
return &reconciler{
|
||||
name: name,
|
||||
path: path,
|
||||
|
||||
handlers: handlers,
|
||||
|
||||
client: kubeclient.Get(ctx),
|
||||
vwhlister: listers.GetValidatingWebhookConfigurationLister(),
|
||||
secretlister: listers.GetSecretLister(),
|
||||
|
||||
secretName: secretName,
|
||||
}
|
||||
}))
|
||||
}
|
||||
|
||||
func TestNew(t *testing.T) {
|
||||
ctx, cancel, _ := SetupFakeContextWithCancel(t)
|
||||
defer cancel()
|
||||
ctx = webhook.WithOptions(ctx, webhook.Options{})
|
||||
|
||||
c := NewAdmissionController(ctx, "foo", "/bar",
|
||||
map[schema.GroupVersionKind]resourcesemantics.GenericCRD{},
|
||||
func(ctx context.Context) context.Context {
|
||||
return ctx
|
||||
}, true /* disallow unknown field */)
|
||||
if c == nil {
|
||||
t.Fatal("Expected NewController to return a non-nil value")
|
||||
}
|
||||
}
|
|
@ -0,0 +1,264 @@
|
|||
/*
|
||||
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 validation
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"context"
|
||||
"encoding/json"
|
||||
"errors"
|
||||
"fmt"
|
||||
"sort"
|
||||
"strings"
|
||||
|
||||
"github.com/markbates/inflect"
|
||||
"go.uber.org/zap"
|
||||
admissionv1beta1 "k8s.io/api/admission/v1beta1"
|
||||
admissionregistrationv1beta1 "k8s.io/api/admissionregistration/v1beta1"
|
||||
"k8s.io/apimachinery/pkg/runtime/schema"
|
||||
"k8s.io/client-go/kubernetes"
|
||||
admissionlisters "k8s.io/client-go/listers/admissionregistration/v1beta1"
|
||||
corelisters "k8s.io/client-go/listers/core/v1"
|
||||
"knative.dev/pkg/apis"
|
||||
"knative.dev/pkg/controller"
|
||||
"knative.dev/pkg/kmp"
|
||||
"knative.dev/pkg/logging"
|
||||
"knative.dev/pkg/ptr"
|
||||
"knative.dev/pkg/system"
|
||||
"knative.dev/pkg/webhook"
|
||||
certresources "knative.dev/pkg/webhook/certificates/resources"
|
||||
"knative.dev/pkg/webhook/resourcesemantics"
|
||||
)
|
||||
|
||||
var errMissingNewObject = errors.New("the new object may not be nil")
|
||||
|
||||
// reconciler implements the AdmissionController for resources
|
||||
type reconciler struct {
|
||||
name string
|
||||
path string
|
||||
handlers map[schema.GroupVersionKind]resourcesemantics.GenericCRD
|
||||
|
||||
withContext func(context.Context) context.Context
|
||||
|
||||
client kubernetes.Interface
|
||||
vwhlister admissionlisters.ValidatingWebhookConfigurationLister
|
||||
secretlister corelisters.SecretLister
|
||||
|
||||
disallowUnknownFields bool
|
||||
secretName string
|
||||
}
|
||||
|
||||
var _ controller.Reconciler = (*reconciler)(nil)
|
||||
var _ webhook.AdmissionController = (*reconciler)(nil)
|
||||
|
||||
// Reconcile implements controller.Reconciler
|
||||
func (ac *reconciler) Reconcile(ctx context.Context, key string) error {
|
||||
logger := logging.FromContext(ctx)
|
||||
|
||||
// Look up the webhook secret, and fetch the CA cert bundle.
|
||||
secret, err := ac.secretlister.Secrets(system.Namespace()).Get(ac.secretName)
|
||||
if err != nil {
|
||||
logger.Errorw("Error fetching secret", zap.Error(err))
|
||||
return err
|
||||
}
|
||||
caCert, ok := secret.Data[certresources.CACert]
|
||||
if !ok {
|
||||
return fmt.Errorf("secret %q is missing %q key", ac.secretName, certresources.CACert)
|
||||
}
|
||||
|
||||
// Reconcile the webhook configuration.
|
||||
return ac.reconcileValidatingWebhook(ctx, caCert)
|
||||
}
|
||||
|
||||
// Path implements AdmissionController
|
||||
func (ac *reconciler) Path() string {
|
||||
return ac.path
|
||||
}
|
||||
|
||||
// Admit implements AdmissionController
|
||||
func (ac *reconciler) Admit(ctx context.Context, request *admissionv1beta1.AdmissionRequest) *admissionv1beta1.AdmissionResponse {
|
||||
if ac.withContext != nil {
|
||||
ctx = ac.withContext(ctx)
|
||||
}
|
||||
|
||||
logger := logging.FromContext(ctx)
|
||||
switch request.Operation {
|
||||
case admissionv1beta1.Create, admissionv1beta1.Update:
|
||||
default:
|
||||
logger.Infof("Unhandled webhook operation, letting it through %v", request.Operation)
|
||||
return &admissionv1beta1.AdmissionResponse{Allowed: true}
|
||||
}
|
||||
|
||||
if err := ac.validate(ctx, request); err != nil {
|
||||
return webhook.MakeErrorStatus("validation failed: %v", err)
|
||||
}
|
||||
|
||||
return &admissionv1beta1.AdmissionResponse{Allowed: true}
|
||||
}
|
||||
|
||||
func (ac *reconciler) reconcileValidatingWebhook(ctx context.Context, caCert []byte) error {
|
||||
logger := logging.FromContext(ctx)
|
||||
|
||||
var rules []admissionregistrationv1beta1.RuleWithOperations
|
||||
for gvk := range ac.handlers {
|
||||
plural := strings.ToLower(inflect.Pluralize(gvk.Kind))
|
||||
|
||||
rules = append(rules, admissionregistrationv1beta1.RuleWithOperations{
|
||||
Operations: []admissionregistrationv1beta1.OperationType{
|
||||
admissionregistrationv1beta1.Create,
|
||||
admissionregistrationv1beta1.Update,
|
||||
},
|
||||
Rule: admissionregistrationv1beta1.Rule{
|
||||
APIGroups: []string{gvk.Group},
|
||||
APIVersions: []string{gvk.Version},
|
||||
Resources: []string{plural + "/*"},
|
||||
},
|
||||
})
|
||||
}
|
||||
|
||||
// Sort the rules by Group, Version, Kind so that things are deterministically ordered.
|
||||
sort.Slice(rules, func(i, j int) bool {
|
||||
lhs, rhs := rules[i], rules[j]
|
||||
if lhs.APIGroups[0] != rhs.APIGroups[0] {
|
||||
return lhs.APIGroups[0] < rhs.APIGroups[0]
|
||||
}
|
||||
if lhs.APIVersions[0] != rhs.APIVersions[0] {
|
||||
return lhs.APIVersions[0] < rhs.APIVersions[0]
|
||||
}
|
||||
return lhs.Resources[0] < rhs.Resources[0]
|
||||
})
|
||||
|
||||
configuredWebhook, err := ac.vwhlister.Get(ac.name)
|
||||
if err != nil {
|
||||
return fmt.Errorf("error retrieving webhook: %v", err)
|
||||
}
|
||||
|
||||
webhook := configuredWebhook.DeepCopy()
|
||||
|
||||
// Clear out any previous (bad) OwnerReferences.
|
||||
// See: https://github.com/knative/serving/issues/5845
|
||||
webhook.OwnerReferences = nil
|
||||
|
||||
for i, wh := range webhook.Webhooks {
|
||||
if wh.Name != webhook.Name {
|
||||
continue
|
||||
}
|
||||
webhook.Webhooks[i].Rules = rules
|
||||
webhook.Webhooks[i].ClientConfig.CABundle = caCert
|
||||
if webhook.Webhooks[i].ClientConfig.Service == nil {
|
||||
return fmt.Errorf("missing service reference for webhook: %s", wh.Name)
|
||||
}
|
||||
webhook.Webhooks[i].ClientConfig.Service.Path = ptr.String(ac.Path())
|
||||
}
|
||||
|
||||
if ok, err := kmp.SafeEqual(configuredWebhook, webhook); err != nil {
|
||||
return fmt.Errorf("error diffing webhooks: %v", err)
|
||||
} else if !ok {
|
||||
logger.Info("Updating webhook")
|
||||
vwhclient := ac.client.AdmissionregistrationV1beta1().ValidatingWebhookConfigurations()
|
||||
if _, err := vwhclient.Update(webhook); err != nil {
|
||||
return fmt.Errorf("failed to update webhook: %v", err)
|
||||
}
|
||||
} else {
|
||||
logger.Info("Webhook is valid")
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
func (ac *reconciler) validate(ctx context.Context, req *admissionv1beta1.AdmissionRequest) error {
|
||||
kind := req.Kind
|
||||
newBytes := req.Object.Raw
|
||||
oldBytes := req.OldObject.Raw
|
||||
// Why, oh why are these different types...
|
||||
gvk := schema.GroupVersionKind{
|
||||
Group: kind.Group,
|
||||
Version: kind.Version,
|
||||
Kind: kind.Kind,
|
||||
}
|
||||
|
||||
logger := logging.FromContext(ctx)
|
||||
handler, ok := ac.handlers[gvk]
|
||||
if !ok {
|
||||
logger.Errorf("Unhandled kind: %v", gvk)
|
||||
return fmt.Errorf("unhandled kind: %v", gvk)
|
||||
}
|
||||
|
||||
// nil values denote absence of `old` (create) or `new` (delete) objects.
|
||||
var oldObj, newObj resourcesemantics.GenericCRD
|
||||
|
||||
if len(newBytes) != 0 {
|
||||
newObj = handler.DeepCopyObject().(resourcesemantics.GenericCRD)
|
||||
newDecoder := json.NewDecoder(bytes.NewBuffer(newBytes))
|
||||
if ac.disallowUnknownFields {
|
||||
newDecoder.DisallowUnknownFields()
|
||||
}
|
||||
if err := newDecoder.Decode(&newObj); err != nil {
|
||||
return fmt.Errorf("cannot decode incoming new object: %v", err)
|
||||
}
|
||||
}
|
||||
if len(oldBytes) != 0 {
|
||||
oldObj = handler.DeepCopyObject().(resourcesemantics.GenericCRD)
|
||||
oldDecoder := json.NewDecoder(bytes.NewBuffer(oldBytes))
|
||||
if ac.disallowUnknownFields {
|
||||
oldDecoder.DisallowUnknownFields()
|
||||
}
|
||||
if err := oldDecoder.Decode(&oldObj); err != nil {
|
||||
return fmt.Errorf("cannot decode incoming old object: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
// Set up the context for defaulting and validation
|
||||
if oldObj != nil {
|
||||
// TODO(mattmoor): Remove this after 0.11 cuts.
|
||||
oldObj.SetDefaults(ctx)
|
||||
if req.SubResource == "" {
|
||||
ctx = apis.WithinUpdate(ctx, oldObj)
|
||||
} else {
|
||||
ctx = apis.WithinSubResourceUpdate(ctx, oldObj, req.SubResource)
|
||||
}
|
||||
} else {
|
||||
ctx = apis.WithinCreate(ctx)
|
||||
}
|
||||
ctx = apis.WithUserInfo(ctx, &req.UserInfo)
|
||||
|
||||
// None of the validators will accept a nil value for newObj.
|
||||
if newObj == nil {
|
||||
return errMissingNewObject
|
||||
}
|
||||
|
||||
// TODO(mattmoor): Remove this after 0.11 cuts.
|
||||
newObj.SetDefaults(ctx)
|
||||
|
||||
if err := validate(ctx, 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.
|
||||
return err
|
||||
}
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
// validate performs validation on the provided "new" CRD.
|
||||
func validate(ctx context.Context, new apis.Validatable) error {
|
||||
// Can't just `return new.Validate()` because it doesn't properly nil-check.
|
||||
if err := new.Validate(ctx); err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
return nil
|
||||
}
|
|
@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
|
|||
limitations under the License.
|
||||
*/
|
||||
|
||||
package resourcesemantics
|
||||
package validation
|
||||
|
||||
import (
|
||||
"testing"
|
||||
|
@ -178,18 +178,6 @@ func TestStrictValidation(t *testing.T) {
|
|||
},
|
||||
},
|
||||
|
||||
"update, strict": {
|
||||
strict: true,
|
||||
req: newUpdateReq(
|
||||
createInnerDefaultResourceWithoutSpec(t),
|
||||
createInnerDefaultResourceWithSpecAndStatus(t, &InnerDefaultSpec{
|
||||
DeprecatedField: "fail setting.",
|
||||
}, nil)),
|
||||
wantErrs: []string{
|
||||
"must not update",
|
||||
"spec.field",
|
||||
},
|
||||
},
|
||||
"update strict, spec.sub.string": {
|
||||
strict: true,
|
||||
req: newUpdateReq(
|
||||
|
@ -498,6 +486,7 @@ func TestStrictValidation(t *testing.T) {
|
|||
}, nil)),
|
||||
},
|
||||
}
|
||||
|
||||
for n, tc := range testCases {
|
||||
t.Run(n, func(t *testing.T) {
|
||||
defer ClearAll()
|
||||
|
@ -542,28 +531,3 @@ func TestStrictValidation_Spec_Create(t *testing.T) {
|
|||
ExpectFailsWith(t, resp, "must not set")
|
||||
ExpectFailsWith(t, resp, "spec.field")
|
||||
}
|
||||
|
||||
// In strict mode, you are not allowed to update a deprecated filed when doing a Update.
|
||||
func TestStrictValidation_Spec_Update(t *testing.T) {
|
||||
req := &admissionv1beta1.AdmissionRequest{
|
||||
Operation: admissionv1beta1.Update,
|
||||
Kind: metav1.GroupVersionKind{
|
||||
Group: "pkg.knative.dev",
|
||||
Version: "v1alpha1",
|
||||
Kind: "InnerDefaultResource",
|
||||
},
|
||||
}
|
||||
req.OldObject.Raw = createInnerDefaultResourceWithoutSpec(t)
|
||||
req.Object.Raw = createInnerDefaultResourceWithSpecAndStatus(t, &InnerDefaultSpec{
|
||||
DeprecatedField: "fail setting.",
|
||||
}, nil)
|
||||
|
||||
ctx := apis.DisallowDeprecated(TestContextWithLogger(t))
|
||||
|
||||
_, ac := newNonRunningTestResourceAdmissionController(t)
|
||||
resp := ac.Admit(ctx, req)
|
||||
|
||||
ExpectFailsWith(t, resp, "must not update")
|
||||
ExpectFailsWith(t, resp, "spec.field")
|
||||
|
||||
}
|
|
@ -0,0 +1,411 @@
|
|||
/*
|
||||
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 validation
|
||||
|
||||
import (
|
||||
"context"
|
||||
"encoding/json"
|
||||
"testing"
|
||||
|
||||
// Injection stuff
|
||||
_ "knative.dev/pkg/client/injection/kube/client/fake"
|
||||
_ "knative.dev/pkg/client/injection/kube/informers/admissionregistration/v1beta1/validatingwebhookconfiguration/fake"
|
||||
_ "knative.dev/pkg/client/injection/kube/informers/core/v1/secret/fake"
|
||||
|
||||
admissionv1beta1 "k8s.io/api/admission/v1beta1"
|
||||
admissionregistrationv1beta1 "k8s.io/api/admissionregistration/v1beta1"
|
||||
authenticationv1 "k8s.io/api/authentication/v1"
|
||||
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
|
||||
"k8s.io/apimachinery/pkg/runtime/schema"
|
||||
fakekubeclientset "k8s.io/client-go/kubernetes/fake"
|
||||
"knative.dev/pkg/apis"
|
||||
"knative.dev/pkg/system"
|
||||
"knative.dev/pkg/webhook"
|
||||
|
||||
_ "knative.dev/pkg/system/testing"
|
||||
|
||||
. "knative.dev/pkg/logging/testing"
|
||||
. "knative.dev/pkg/reconciler/testing"
|
||||
. "knative.dev/pkg/testing"
|
||||
"knative.dev/pkg/webhook/resourcesemantics"
|
||||
. "knative.dev/pkg/webhook/testing"
|
||||
)
|
||||
|
||||
const (
|
||||
testResourceValidationPath = "/foo"
|
||||
testResourceValidationName = "webhook.knative.dev"
|
||||
user1 = "brutto@knative.dev"
|
||||
user2 = "arrabbiato@knative.dev"
|
||||
)
|
||||
|
||||
var (
|
||||
handlers = map[schema.GroupVersionKind]resourcesemantics.GenericCRD{
|
||||
{
|
||||
Group: "pkg.knative.dev",
|
||||
Version: "v1alpha1",
|
||||
Kind: "Resource",
|
||||
}: &Resource{},
|
||||
{
|
||||
Group: "pkg.knative.dev",
|
||||
Version: "v1beta1",
|
||||
Kind: "Resource",
|
||||
}: &Resource{},
|
||||
{
|
||||
Group: "pkg.knative.dev",
|
||||
Version: "v1alpha1",
|
||||
Kind: "InnerDefaultResource",
|
||||
}: &InnerDefaultResource{},
|
||||
{
|
||||
Group: "pkg.knative.io",
|
||||
Version: "v1alpha1",
|
||||
Kind: "InnerDefaultResource",
|
||||
}: &InnerDefaultResource{},
|
||||
}
|
||||
|
||||
initialResourceWebhook = &admissionregistrationv1beta1.ValidatingWebhookConfiguration{
|
||||
ObjectMeta: metav1.ObjectMeta{
|
||||
Name: "webhook.knative.dev",
|
||||
OwnerReferences: []metav1.OwnerReference{{
|
||||
Name: "asdf",
|
||||
}},
|
||||
},
|
||||
Webhooks: []admissionregistrationv1beta1.ValidatingWebhook{{
|
||||
Name: "webhook.knative.dev",
|
||||
ClientConfig: admissionregistrationv1beta1.WebhookClientConfig{
|
||||
Service: &admissionregistrationv1beta1.ServiceReference{
|
||||
Namespace: system.Namespace(),
|
||||
Name: "webhook",
|
||||
},
|
||||
},
|
||||
}},
|
||||
}
|
||||
)
|
||||
|
||||
func newNonRunningTestResourceAdmissionController(t *testing.T) (
|
||||
kubeClient *fakekubeclientset.Clientset,
|
||||
ac *reconciler) {
|
||||
|
||||
t.Helper()
|
||||
// Create fake clients
|
||||
kubeClient = fakekubeclientset.NewSimpleClientset(initialResourceWebhook)
|
||||
|
||||
ac = NewTestResourceAdmissionController(t)
|
||||
return
|
||||
}
|
||||
|
||||
func TestDeleteAllowed(t *testing.T) {
|
||||
_, ac := newNonRunningTestResourceAdmissionController(t)
|
||||
|
||||
req := &admissionv1beta1.AdmissionRequest{
|
||||
Operation: admissionv1beta1.Delete,
|
||||
}
|
||||
|
||||
if resp := ac.Admit(TestContextWithLogger(t), req); !resp.Allowed {
|
||||
t.Fatal("Unexpected denial of delete")
|
||||
}
|
||||
}
|
||||
|
||||
func TestConnectAllowed(t *testing.T) {
|
||||
_, ac := newNonRunningTestResourceAdmissionController(t)
|
||||
|
||||
req := &admissionv1beta1.AdmissionRequest{
|
||||
Operation: admissionv1beta1.Connect,
|
||||
}
|
||||
|
||||
resp := ac.Admit(TestContextWithLogger(t), req)
|
||||
if !resp.Allowed {
|
||||
t.Fatalf("Unexpected denial of connect")
|
||||
}
|
||||
}
|
||||
|
||||
func TestUnknownKindFails(t *testing.T) {
|
||||
_, ac := newNonRunningTestResourceAdmissionController(t)
|
||||
|
||||
req := &admissionv1beta1.AdmissionRequest{
|
||||
Operation: admissionv1beta1.Create,
|
||||
Kind: metav1.GroupVersionKind{
|
||||
Group: "pkg.knative.dev",
|
||||
Version: "v1alpha1",
|
||||
Kind: "Garbage",
|
||||
},
|
||||
}
|
||||
|
||||
ExpectFailsWith(t, ac.Admit(TestContextWithLogger(t), req), "unhandled kind")
|
||||
}
|
||||
|
||||
func TestUnknownVersionFails(t *testing.T) {
|
||||
_, ac := newNonRunningTestResourceAdmissionController(t)
|
||||
req := &admissionv1beta1.AdmissionRequest{
|
||||
Operation: admissionv1beta1.Create,
|
||||
Kind: metav1.GroupVersionKind{
|
||||
Group: "pkg.knative.dev",
|
||||
Version: "v1beta2",
|
||||
Kind: "Resource",
|
||||
},
|
||||
}
|
||||
ExpectFailsWith(t, ac.Admit(TestContextWithLogger(t), req), "unhandled kind")
|
||||
}
|
||||
|
||||
func TestUnknownFieldFails(t *testing.T) {
|
||||
_, ac := newNonRunningTestResourceAdmissionController(t)
|
||||
req := &admissionv1beta1.AdmissionRequest{
|
||||
Operation: admissionv1beta1.Create,
|
||||
Kind: metav1.GroupVersionKind{
|
||||
Group: "pkg.knative.dev",
|
||||
Version: "v1alpha1",
|
||||
Kind: "Resource",
|
||||
},
|
||||
}
|
||||
|
||||
marshaled, err := json.Marshal(map[string]interface{}{
|
||||
"spec": map[string]interface{}{
|
||||
"foo": "bar",
|
||||
},
|
||||
})
|
||||
if err != nil {
|
||||
panic("failed to marshal resource")
|
||||
}
|
||||
req.Object.Raw = marshaled
|
||||
|
||||
ExpectFailsWith(t, ac.Admit(TestContextWithLogger(t), req),
|
||||
`validation failed: cannot decode incoming new object: json: unknown field "foo"`)
|
||||
}
|
||||
|
||||
func TestAdmitCreates(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
setup func(context.Context, *Resource)
|
||||
rejection string
|
||||
}{{
|
||||
name: "test simple creation (alpha, no diff)",
|
||||
setup: func(ctx context.Context, r *Resource) {
|
||||
r.TypeMeta.APIVersion = "v1alpha1"
|
||||
r.SetDefaults(ctx)
|
||||
r.Annotations = map[string]string{
|
||||
"pkg.knative.dev/creator": user1,
|
||||
"pkg.knative.dev/lastModifier": user1,
|
||||
}
|
||||
},
|
||||
}, {
|
||||
name: "test simple creation (beta, no diff)",
|
||||
setup: func(ctx context.Context, r *Resource) {
|
||||
r.TypeMeta.APIVersion = "v1beta1"
|
||||
r.SetDefaults(ctx)
|
||||
r.Annotations = map[string]string{
|
||||
"pkg.knative.dev/creator": user1,
|
||||
"pkg.knative.dev/lastModifier": user1,
|
||||
}
|
||||
},
|
||||
}, {
|
||||
name: "with bad field",
|
||||
setup: func(ctx context.Context, r *Resource) {
|
||||
// Put a bad value in.
|
||||
r.Spec.FieldWithValidation = "not what's expected"
|
||||
},
|
||||
rejection: "invalid value",
|
||||
}}
|
||||
|
||||
for _, tc := range tests {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
r := CreateResource("a name")
|
||||
ctx := apis.WithinCreate(apis.WithUserInfo(
|
||||
TestContextWithLogger(t),
|
||||
&authenticationv1.UserInfo{Username: user1}))
|
||||
|
||||
// Setup the resource.
|
||||
tc.setup(ctx, r)
|
||||
|
||||
_, ac := newNonRunningTestResourceAdmissionController(t)
|
||||
resp := ac.Admit(ctx, createCreateResource(ctx, r))
|
||||
|
||||
if tc.rejection == "" {
|
||||
ExpectAllowed(t, resp)
|
||||
} else {
|
||||
ExpectFailsWith(t, resp, tc.rejection)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func createCreateResource(ctx context.Context, r *Resource) *admissionv1beta1.AdmissionRequest {
|
||||
req := &admissionv1beta1.AdmissionRequest{
|
||||
Operation: admissionv1beta1.Create,
|
||||
Kind: metav1.GroupVersionKind{
|
||||
Group: "pkg.knative.dev",
|
||||
Version: "v1alpha1",
|
||||
Kind: "Resource",
|
||||
},
|
||||
UserInfo: *apis.GetUserInfo(ctx),
|
||||
}
|
||||
marshaled, err := json.Marshal(r)
|
||||
if err != nil {
|
||||
panic("failed to marshal resource")
|
||||
}
|
||||
req.Object.Raw = marshaled
|
||||
req.Resource.Group = "pkg.knative.dev"
|
||||
return req
|
||||
}
|
||||
|
||||
func TestAdmitUpdates(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
setup func(context.Context, *Resource)
|
||||
mutate func(context.Context, *Resource)
|
||||
rejection string
|
||||
}{{
|
||||
name: "test simple update (no diff)",
|
||||
setup: func(ctx context.Context, r *Resource) {
|
||||
r.SetDefaults(ctx)
|
||||
},
|
||||
mutate: func(ctx context.Context, r *Resource) {
|
||||
// If we don't change anything, the updater
|
||||
// annotation doesn't change.
|
||||
},
|
||||
}, {
|
||||
name: "bad mutation (immutable)",
|
||||
setup: func(ctx context.Context, r *Resource) {
|
||||
r.SetDefaults(ctx)
|
||||
},
|
||||
mutate: func(ctx context.Context, r *Resource) {
|
||||
r.Spec.FieldThatsImmutableWithDefault = "something different"
|
||||
},
|
||||
rejection: "Immutable field changed",
|
||||
}, {
|
||||
name: "bad mutation (validation)",
|
||||
setup: func(ctx context.Context, r *Resource) {
|
||||
r.SetDefaults(ctx)
|
||||
},
|
||||
mutate: func(ctx context.Context, r *Resource) {
|
||||
r.Spec.FieldWithValidation = "not what's expected"
|
||||
},
|
||||
rejection: "invalid value",
|
||||
}}
|
||||
|
||||
for _, tc := range tests {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
old := CreateResource("a name")
|
||||
ctx := TestContextWithLogger(t)
|
||||
|
||||
old.Annotations = map[string]string{
|
||||
"pkg.knative.dev/creator": user1,
|
||||
"pkg.knative.dev/lastModifier": user1,
|
||||
}
|
||||
|
||||
tc.setup(ctx, old)
|
||||
|
||||
new := old.DeepCopy()
|
||||
|
||||
// Mutate the resource using the update context as user2
|
||||
ctx = apis.WithUserInfo(apis.WithinUpdate(ctx, old),
|
||||
&authenticationv1.UserInfo{Username: user2})
|
||||
tc.mutate(ctx, new)
|
||||
|
||||
_, ac := newNonRunningTestResourceAdmissionController(t)
|
||||
resp := ac.Admit(ctx, createUpdateResource(ctx, old, new))
|
||||
|
||||
if tc.rejection == "" {
|
||||
ExpectAllowed(t, resp)
|
||||
} else {
|
||||
ExpectFailsWith(t, resp, tc.rejection)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func createUpdateResource(ctx context.Context, old, new *Resource) *admissionv1beta1.AdmissionRequest {
|
||||
req := &admissionv1beta1.AdmissionRequest{
|
||||
Operation: admissionv1beta1.Update,
|
||||
Kind: metav1.GroupVersionKind{
|
||||
Group: "pkg.knative.dev",
|
||||
Version: "v1alpha1",
|
||||
Kind: "Resource",
|
||||
},
|
||||
UserInfo: *apis.GetUserInfo(ctx),
|
||||
}
|
||||
marshaled, err := json.Marshal(new)
|
||||
if err != nil {
|
||||
panic("failed to marshal resource")
|
||||
}
|
||||
req.Object.Raw = marshaled
|
||||
marshaledOld, err := json.Marshal(old)
|
||||
if err != nil {
|
||||
panic("failed to marshal resource")
|
||||
}
|
||||
req.OldObject.Raw = marshaledOld
|
||||
req.Resource.Group = "pkg.knative.dev"
|
||||
return req
|
||||
}
|
||||
|
||||
func createInnerDefaultResourceWithoutSpec(t *testing.T) []byte {
|
||||
t.Helper()
|
||||
r := InnerDefaultResource{
|
||||
ObjectMeta: metav1.ObjectMeta{
|
||||
Namespace: system.Namespace(),
|
||||
Name: "a name",
|
||||
},
|
||||
}
|
||||
// Remove the 'spec' field of the generated JSON by marshaling it to JSON, parsing that as a
|
||||
// generic map[string]interface{}, removing 'spec', and marshaling it again.
|
||||
origBytes, err := json.Marshal(r)
|
||||
if err != nil {
|
||||
t.Fatalf("Error marshaling origBytes: %v", err)
|
||||
}
|
||||
var q map[string]interface{}
|
||||
if err := json.Unmarshal(origBytes, &q); err != nil {
|
||||
t.Fatalf("Error unmarshaling origBytes: %v", err)
|
||||
}
|
||||
delete(q, "spec")
|
||||
b, err := json.Marshal(q)
|
||||
if err != nil {
|
||||
t.Fatalf("Error marshaling q: %v", err)
|
||||
}
|
||||
return b
|
||||
}
|
||||
|
||||
func createInnerDefaultResourceWithSpecAndStatus(t *testing.T, spec *InnerDefaultSpec, status *InnerDefaultStatus) []byte {
|
||||
t.Helper()
|
||||
r := InnerDefaultResource{
|
||||
ObjectMeta: metav1.ObjectMeta{
|
||||
Namespace: system.Namespace(),
|
||||
Name: "a name",
|
||||
},
|
||||
}
|
||||
if spec != nil {
|
||||
r.Spec = *spec
|
||||
}
|
||||
if status != nil {
|
||||
r.Status = *status
|
||||
}
|
||||
|
||||
b, err := json.Marshal(r)
|
||||
if err != nil {
|
||||
t.Fatalf("Error marshaling bytes: %v", err)
|
||||
}
|
||||
return b
|
||||
}
|
||||
|
||||
func NewTestResourceAdmissionController(t *testing.T) *reconciler {
|
||||
ctx, _ := SetupFakeContext(t)
|
||||
ctx = webhook.WithOptions(ctx, webhook.Options{
|
||||
SecretName: "webhook-secret",
|
||||
})
|
||||
return NewAdmissionController(
|
||||
ctx, testResourceValidationName, testResourceValidationPath,
|
||||
handlers, func(ctx context.Context) context.Context {
|
||||
return ctx
|
||||
}, true).Reconciler.(*reconciler)
|
||||
}
|
|
@ -80,7 +80,7 @@ type Webhook struct {
|
|||
Client kubernetes.Interface
|
||||
Options Options
|
||||
Logger *zap.SugaredLogger
|
||||
admissionControllers map[string]AdmissionController
|
||||
admissionControllers map[string][]AdmissionController
|
||||
secretlister corelisters.SecretLister
|
||||
}
|
||||
|
||||
|
@ -114,12 +114,9 @@ func New(
|
|||
}
|
||||
|
||||
// Build up a map of paths to admission controllers for routing handlers.
|
||||
acs := map[string]AdmissionController{}
|
||||
acs := map[string][]AdmissionController{}
|
||||
for _, ac := range admissionControllers {
|
||||
if _, ok := acs[ac.Path()]; ok {
|
||||
return nil, fmt.Errorf("duplicate admission controller path %q", ac.Path())
|
||||
}
|
||||
acs[ac.Path()] = ac
|
||||
acs[ac.Path()] = append(acs[ac.Path()], ac)
|
||||
}
|
||||
|
||||
return &Webhook{
|
||||
|
@ -212,23 +209,33 @@ func (ac *Webhook) ServeHTTP(w http.ResponseWriter, r *http.Request) {
|
|||
zap.String(logkey.UserInfo, fmt.Sprint(review.Request.UserInfo)))
|
||||
ctx := logging.WithLogger(r.Context(), logger)
|
||||
|
||||
c, ok := ac.admissionControllers[r.URL.Path]
|
||||
cs, ok := ac.admissionControllers[r.URL.Path]
|
||||
if !ok {
|
||||
http.Error(w, fmt.Sprintf("no admission controller registered for: %s", r.URL.Path), http.StatusBadRequest)
|
||||
return
|
||||
}
|
||||
|
||||
// Where the magic happens.
|
||||
reviewResponse := c.Admit(ctx, review.Request)
|
||||
|
||||
// TODO(mattmoor): Remove support for multiple AdmissionControllers at
|
||||
// the same path after 0.11 cuts.
|
||||
// We only TEMPORARILY support multiple AdmissionControllers at the same path because of
|
||||
// the issue described here: https://github.com/knative/serving/pull/5947
|
||||
// So we only support a single AdmissionController per path returning Patches.
|
||||
var response admissionv1beta1.AdmissionReview
|
||||
if reviewResponse != nil {
|
||||
response.Response = reviewResponse
|
||||
response.Response.UID = review.Request.UID
|
||||
}
|
||||
for _, c := range cs {
|
||||
reviewResponse := c.Admit(ctx, review.Request)
|
||||
logger.Infof("AdmissionReview for %#v: %s/%s response=%#v",
|
||||
review.Request.Kind, review.Request.Namespace, review.Request.Name, reviewResponse)
|
||||
|
||||
logger.Infof("AdmissionReview for %#v: %s/%s response=%#v",
|
||||
review.Request.Kind, review.Request.Namespace, review.Request.Name, reviewResponse)
|
||||
if !reviewResponse.Allowed {
|
||||
response.Response = reviewResponse
|
||||
break
|
||||
}
|
||||
|
||||
if reviewResponse.PatchType != nil || response.Response == nil {
|
||||
response.Response = reviewResponse
|
||||
}
|
||||
}
|
||||
response.Response.UID = review.Request.UID
|
||||
|
||||
if err := json.NewEncoder(w).Encode(response); err != nil {
|
||||
http.Error(w, fmt.Sprintf("could encode response: %v", err), http.StatusInternalServerError)
|
||||
|
|
Loading…
Reference in New Issue