diff --git a/Gopkg.lock b/Gopkg.lock index 5eecfcca9..e11a07670 100644 --- a/Gopkg.lock +++ b/Gopkg.lock @@ -1356,7 +1356,6 @@ "k8s.io/client-go/kubernetes", "k8s.io/client-go/kubernetes/fake", "k8s.io/client-go/kubernetes/scheme", - "k8s.io/client-go/kubernetes/typed/admissionregistration/v1beta1", "k8s.io/client-go/kubernetes/typed/core/v1", "k8s.io/client-go/plugin/pkg/client/auth/gcp", "k8s.io/client-go/rest", diff --git a/webhook/resource_admission_controller.go b/webhook/resource_admission_controller.go new file mode 100644 index 000000000..cf0ec50cc --- /dev/null +++ b/webhook/resource_admission_controller.go @@ -0,0 +1,325 @@ +/* +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 webhook + +import ( + "bytes" + "context" + "encoding/json" + "fmt" + "sort" + "strings" + + "github.com/markbates/inflect" + "github.com/mattbaird/jsonpatch" + "go.uber.org/zap" + admissionv1beta1 "k8s.io/api/admission/v1beta1" + admissionregistrationv1beta1 "k8s.io/api/admissionregistration/v1beta1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/client-go/kubernetes" + + "knative.dev/pkg/apis" + "knative.dev/pkg/apis/duck" + "knative.dev/pkg/kmp" + "knative.dev/pkg/logging" +) + +// ResourceCallback defines a signature for resource specific (Route, Configuration, etc.) +// handlers that can validate and mutate an object. If non-nil error is returned, object mutation +// is denied. Mutations should be appended to the patches operations. +type ResourceCallback func(patches *[]jsonpatch.JsonPatchOperation, old GenericCRD, new GenericCRD) error + +// ResourceDefaulter defines a signature for resource specific (Route, Configuration, etc.) +// handlers that can set defaults on an object. If non-nil error is returned, object mutation +// is denied. Mutations should be appended to the patches operations. +type ResourceDefaulter func(patches *[]jsonpatch.JsonPatchOperation, crd GenericCRD) error + +// 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 +} + +type ResourceAdmissionController struct { + Handlers map[schema.GroupVersionKind]GenericCRD + Options ControllerOptions + + DisallowUnknownFields bool +} + +func (ac *ResourceAdmissionController) Admit(ctx context.Context, request *admissionv1beta1.AdmissionRequest) *admissionv1beta1.AdmissionResponse { + 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} + } + + patchBytes, err := ac.mutate(ctx, request) + if err != nil { + return makeErrorStatus("mutation failed: %v", err) + } + logger.Infof("Kind: %q PatchBytes: %v", request.Kind, string(patchBytes)) + + return &admissionv1beta1.AdmissionResponse{ + Patch: patchBytes, + Allowed: true, + PatchType: func() *admissionv1beta1.PatchType { + pt := admissionv1beta1.PatchTypeJSONPatch + return &pt + }(), + } +} + +func (ac *ResourceAdmissionController) Register(ctx context.Context, kubeClient kubernetes.Interface, caCert []byte) error { + client := kubeClient.AdmissionregistrationV1beta1().MutatingWebhookConfigurations() + logger := logging.FromContext(ctx) + failurePolicy := admissionregistrationv1beta1.Fail + + 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] + }) + + webhook := &admissionregistrationv1beta1.MutatingWebhookConfiguration{ + ObjectMeta: metav1.ObjectMeta{ + Name: ac.Options.WebhookName, + }, + Webhooks: []admissionregistrationv1beta1.Webhook{{ + Name: ac.Options.WebhookName, + Rules: rules, + ClientConfig: admissionregistrationv1beta1.WebhookClientConfig{ + Service: &admissionregistrationv1beta1.ServiceReference{ + Namespace: ac.Options.Namespace, + Name: ac.Options.ServiceName, + }, + CABundle: caCert, + }, + FailurePolicy: &failurePolicy, + }}, + } + + // Set the owner to our deployment. + deployment, err := kubeClient.Apps().Deployments(ac.Options.Namespace).Get(ac.Options.DeploymentName, metav1.GetOptions{}) + if err != nil { + return fmt.Errorf("failed to fetch our deployment: %v", err) + } + deploymentRef := metav1.NewControllerRef(deployment, deploymentKind) + webhook.OwnerReferences = append(webhook.OwnerReferences, *deploymentRef) + + // Try to create the webhook and if it already exists validate webhook rules. + _, err = client.Create(webhook) + if err != nil { + if !apierrors.IsAlreadyExists(err) { + return fmt.Errorf("failed to create a webhook: %v", err) + } + logger.Info("Webhook already exists") + configuredWebhook, err := client.Get(ac.Options.WebhookName, metav1.GetOptions{}) + if err != nil { + return fmt.Errorf("error retrieving webhook: %v", err) + } + if ok, err := kmp.SafeEqual(configuredWebhook.Webhooks, webhook.Webhooks); err != nil { + return fmt.Errorf("error diffing webhooks: %v", err) + } else if !ok { + logger.Info("Updating webhook") + // Set the ResourceVersion as required by update. + webhook.ObjectMeta.ResourceVersion = configuredWebhook.ObjectMeta.ResourceVersion + if _, err := client.Update(webhook); err != nil { + return fmt.Errorf("failed to update webhook: %s", err) + } + } else { + logger.Info("Webhook is already valid") + } + } else { + logger.Info("Created a webhook") + } + return nil +} + +func (ac *ResourceAdmissionController) mutate(ctx context.Context, req *admissionv1beta1.AdmissionRequest) ([]byte, 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 nil, fmt.Errorf("unhandled kind: %v", gvk) + } + + // nil values denote absence of `old` (create) or `new` (delete) objects. + var oldObj, newObj GenericCRD + + if len(newBytes) != 0 { + newObj = handler.DeepCopyObject().(GenericCRD) + newDecoder := json.NewDecoder(bytes.NewBuffer(newBytes)) + if ac.DisallowUnknownFields { + newDecoder.DisallowUnknownFields() + } + if err := newDecoder.Decode(&newObj); err != nil { + return nil, fmt.Errorf("cannot decode incoming new object: %v", err) + } + } + if len(oldBytes) != 0 { + oldObj = handler.DeepCopyObject().(GenericCRD) + oldDecoder := json.NewDecoder(bytes.NewBuffer(oldBytes)) + if ac.DisallowUnknownFields { + oldDecoder.DisallowUnknownFields() + } + if err := oldDecoder.Decode(&oldObj); err != nil { + return nil, fmt.Errorf("cannot decode incoming old object: %v", err) + } + } + var patches duck.JSONPatch + + var err error + // Skip this step if the type we're dealing with is a duck type, since it is inherently + // incomplete and this will patch away all of the unspecified fields. + if _, ok := newObj.(duck.Populatable); !ok { + // Add these before defaulting fields, otherwise defaulting may cause an illegal patch + // because it expects the round tripped through Golang fields to be present already. + rtp, err := roundTripPatch(newBytes, newObj) + if err != nil { + return nil, fmt.Errorf("cannot create patch for round tripped newBytes: %v", err) + } + patches = append(patches, rtp...) + } + + // Set up the context for defaulting and validation + 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.SetDefaults(ctx) + + s, ok := oldObj.(apis.HasSpec) + if ok { + SetUserInfoAnnotations(s, ctx, req.Resource.Group) + } + + 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) + + // Default the new object. + 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 = ac.setUserInfoAnnotations(ctx, patches, newObj, req.Resource.Group); err != nil { + logger.Errorw("Failed the resource user info annotator", zap.Error(err)) + return nil, err + } + + // None of the validators will accept a nil value for newObj. + 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 *ResourceAdmissionController) setUserInfoAnnotations(ctx context.Context, patches duck.JSONPatch, new GenericCRD, groupName string) (duck.JSONPatch, error) { + if new == nil { + return patches, nil + } + nh, ok := new.(apis.HasSpec) + if !ok { + return patches, nil + } + + b, a := new.DeepCopyObject().(apis.HasSpec), nh + + SetUserInfoAnnotations(nh, ctx, groupName) + + patch, err := duck.CreatePatch(b, a) + if err != nil { + return nil, err + } + return append(patches, patch...), nil +} + +// roundTripPatch generates the JSONPatch that corresponds to round tripping the given bytes through +// the Golang type (JSON -> Golang type -> JSON). Because it is not always true that +// bytes == json.Marshal(json.Unmarshal(bytes)). +// +// For example, if bytes did not contain a 'spec' field and the Golang type specifies its 'spec' +// field without omitempty, then by round tripping through the Golang type, we would have added +// `'spec': {}`. +func roundTripPatch(bytes []byte, unmarshalled interface{}) (duck.JSONPatch, error) { + if unmarshalled == nil { + return duck.JSONPatch{}, nil + } + marshaledBytes, err := json.Marshal(unmarshalled) + if err != nil { + return nil, fmt.Errorf("cannot marshal interface: %v", err) + } + return jsonpatch.CreatePatch(bytes, marshaledBytes) +} diff --git a/webhook/webhook_deprecated_test.go b/webhook/resource_admission_controller_deprecated_test.go similarity index 97% rename from webhook/webhook_deprecated_test.go rename to webhook/resource_admission_controller_deprecated_test.go index 6ef63308e..8ade4bcc1 100644 --- a/webhook/webhook_deprecated_test.go +++ b/webhook/resource_admission_controller_deprecated_test.go @@ -504,8 +504,8 @@ func TestStrictValidation(t *testing.T) { ctx = apis.DisallowDeprecated(ctx) } - _, ac := newNonRunningTestAdmissionController(t, newDefaultOptions()) - resp := ac.admit(ctx, tc.req) + _, ac := newNonRunningTestResourceAdmissionController(t, newDefaultOptions()) + resp := ac.Admit(ctx, tc.req) if len(tc.wantErrs) > 0 { for _, err := range tc.wantErrs { @@ -534,8 +534,8 @@ func TestStrictValidation_Spec_Create(t *testing.T) { ctx := apis.DisallowDeprecated(TestContextWithLogger(t)) - _, ac := newNonRunningTestAdmissionController(t, newDefaultOptions()) - resp := ac.admit(ctx, req) + _, ac := newNonRunningTestResourceAdmissionController(t, newDefaultOptions()) + resp := ac.Admit(ctx, req) expectFailsWith(t, resp, "must not set") expectFailsWith(t, resp, "spec.field") @@ -558,8 +558,8 @@ func TestStrictValidation_Spec_Update(t *testing.T) { ctx := apis.DisallowDeprecated(TestContextWithLogger(t)) - _, ac := newNonRunningTestAdmissionController(t, newDefaultOptions()) - resp := ac.admit(ctx, req) + _, ac := newNonRunningTestResourceAdmissionController(t, newDefaultOptions()) + resp := ac.Admit(ctx, req) expectFailsWith(t, resp, "must not update") expectFailsWith(t, resp, "spec.field") diff --git a/webhook/resource_admission_controller_test.go b/webhook/resource_admission_controller_test.go new file mode 100644 index 000000000..213c91ad7 --- /dev/null +++ b/webhook/resource_admission_controller_test.go @@ -0,0 +1,626 @@ +/* +Copyright 2017 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 webhook + +import ( + "context" + "encoding/json" + "fmt" + "reflect" + "sort" + "strings" + "testing" + + // "knative.dev/pkg/apis/duck" + + "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" + "github.com/mattbaird/jsonpatch" + admissionv1beta1 "k8s.io/api/admission/v1beta1" + admissionregistrationv1beta1 "k8s.io/api/admissionregistration/v1beta1" + appsv1 "k8s.io/api/apps/v1" + authenticationv1 "k8s.io/api/authentication/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/kubernetes" + fakekubeclientset "k8s.io/client-go/kubernetes/fake" + "knative.dev/pkg/apis" + + . "knative.dev/pkg/logging/testing" + . "knative.dev/pkg/testing" +) + +func newNonRunningTestResourceAdmissionController(t *testing.T, options ControllerOptions) ( + kubeClient *fakekubeclientset.Clientset, + ac *ResourceAdmissionController) { + t.Helper() + // Create fake clients + kubeClient = fakekubeclientset.NewSimpleClientset() + + ac = NewTestResourceAdmissionController(options) + return +} + +func TestDeleteAllowed(t *testing.T) { + _, ac := newNonRunningTestResourceAdmissionController(t, newDefaultOptions()) + + 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, newDefaultOptions()) + + 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, newDefaultOptions()) + + 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, newDefaultOptions()) + 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, newDefaultOptions()) + 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), + `mutation 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 + patches []jsonpatch.JsonPatchOperation + }{{ + 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, + } + }, + patches: []jsonpatch.JsonPatchOperation{}, + }, { + 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, + } + }, + patches: []jsonpatch.JsonPatchOperation{}, + }, { + name: "test simple creation (with defaults)", + setup: func(ctx context.Context, r *Resource) { + }, + patches: []jsonpatch.JsonPatchOperation{{ + Operation: "add", + Path: "/metadata/annotations", + Value: map[string]interface{}{ + "pkg.knative.dev/creator": user1, + "pkg.knative.dev/lastModifier": user1, + }, + }, { + Operation: "add", + Path: "/spec/fieldThatsImmutableWithDefault", + Value: "this is another default value", + }, { + Operation: "add", + Path: "/spec/fieldWithDefault", + Value: "I'm a default.", + }}, + }, { + name: "test simple creation (with defaults around annotations)", + setup: func(ctx context.Context, r *Resource) { + r.Annotations = map[string]string{ + "foo": "bar", + } + }, + patches: []jsonpatch.JsonPatchOperation{{ + Operation: "add", + Path: "/metadata/annotations/pkg.knative.dev~1creator", + Value: user1, + }, { + Operation: "add", + Path: "/metadata/annotations/pkg.knative.dev~1lastModifier", + Value: user1, + }, { + Operation: "add", + Path: "/spec/fieldThatsImmutableWithDefault", + Value: "this is another default value", + }, { + Operation: "add", + Path: "/spec/fieldWithDefault", + Value: "I'm a default.", + }}, + }, { + name: "test simple creation (with partially overridden defaults)", + setup: func(ctx context.Context, r *Resource) { + r.Spec.FieldThatsImmutableWithDefault = "not the default" + }, + patches: []jsonpatch.JsonPatchOperation{{ + Operation: "add", + Path: "/metadata/annotations", + Value: map[string]interface{}{ + "pkg.knative.dev/creator": user1, + "pkg.knative.dev/lastModifier": user1, + }, + }, { + Operation: "add", + Path: "/spec/fieldWithDefault", + Value: "I'm a default.", + }}, + }, { + name: "test simple creation (webhook corrects user annotation)", + setup: func(ctx context.Context, r *Resource) { + r.SetDefaults(ctx) + // THIS IS NOT WHO IS CREATING IT, IT IS LIES! + r.Annotations = map[string]string{ + "pkg.knative.dev/lastModifier": user2, + } + }, + patches: []jsonpatch.JsonPatchOperation{{ + Operation: "replace", + Path: "/metadata/annotations/pkg.knative.dev~1lastModifier", + Value: user1, + }, { + Operation: "add", + 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 { + 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, newDefaultOptions()) + resp := ac.Admit(ctx, createCreateResource(ctx, r)) + + if tc.rejection == "" { + expectAllowed(t, resp) + expectPatches(t, resp.Patch, tc.patches) + } 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 + patches []jsonpatch.JsonPatchOperation + }{{ + 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. + }, + patches: []jsonpatch.JsonPatchOperation{}, + }, { + name: "test simple update (update updater annotation)", + setup: func(ctx context.Context, r *Resource) { + r.SetDefaults(ctx) + }, + mutate: func(ctx context.Context, r *Resource) { + // When we change the spec, the updater + // annotation changes. + r.Spec.FieldWithDefault = "not the default" + }, + patches: []jsonpatch.JsonPatchOperation{{ + Operation: "replace", + Path: "/metadata/annotations/pkg.knative.dev~1lastModifier", + Value: user2, + }}, + }, { + name: "test simple update (annotation change doesn't change updater)", + setup: func(ctx context.Context, r *Resource) { + r.SetDefaults(ctx) + }, + mutate: func(ctx context.Context, r *Resource) { + // When we change an annotation, the updater doesn't change. + r.Annotations["foo"] = "bar" + }, + patches: []jsonpatch.JsonPatchOperation{}, + }, { + name: "test that updates dropping immutable defaults are filled back in", + setup: func(ctx context.Context, r *Resource) { + r.SetDefaults(ctx) + r.Spec.FieldThatsImmutableWithDefault = "" + }, + mutate: func(ctx context.Context, r *Resource) { + r.Spec.FieldThatsImmutableWithDefault = "" + }, + patches: []jsonpatch.JsonPatchOperation{{ + Operation: "add", + 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 { + 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, newDefaultOptions()) + resp := ac.Admit(ctx, createUpdateResource(ctx, old, new)) + + if tc.rejection == "" { + expectAllowed(t, resp) + expectPatches(t, resp.Patch, tc.patches) + } 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 TestValidCreateResourceSucceedsWithRoundTripAndDefaultPatch(t *testing.T) { + req := &admissionv1beta1.AdmissionRequest{ + Operation: admissionv1beta1.Create, + Kind: metav1.GroupVersionKind{ + Group: "pkg.knative.dev", + Version: "v1alpha1", + Kind: "InnerDefaultResource", + }, + } + req.Object.Raw = createInnerDefaultResourceWithoutSpec(t) + + _, ac := newNonRunningTestResourceAdmissionController(t, newDefaultOptions()) + resp := ac.Admit(TestContextWithLogger(t), req) + expectAllowed(t, resp) + expectPatches(t, resp.Patch, []jsonpatch.JsonPatchOperation{{ + Operation: "add", + Path: "/spec", + Value: map[string]interface{}{}, + }, { + Operation: "add", + Path: "/spec/fieldWithDefault", + Value: "I'm a default.", + }}) +} + +func createInnerDefaultResourceWithoutSpec(t *testing.T) []byte { + t.Helper() + r := InnerDefaultResource{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: testNamespace, + 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: testNamespace, + 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 TestValidWebhook(t *testing.T) { + kubeClient, ac := newNonRunningTestResourceAdmissionController(t, newDefaultOptions()) + createDeployment(kubeClient) + err := ac.Register(TestContextWithLogger(t), kubeClient, []byte{}) + if err != nil { + t.Fatalf("Failed to create webhook: %s", err) + } +} + +func TestUpdatingWebhook(t *testing.T) { + kubeClient, ac := newNonRunningTestResourceAdmissionController(t, newDefaultOptions()) + webhook := &admissionregistrationv1beta1.MutatingWebhookConfiguration{ + ObjectMeta: metav1.ObjectMeta{ + Name: ac.Options.WebhookName, + }, + Webhooks: []admissionregistrationv1beta1.Webhook{{ + Name: ac.Options.WebhookName, + Rules: []admissionregistrationv1beta1.RuleWithOperations{{}}, + ClientConfig: admissionregistrationv1beta1.WebhookClientConfig{}, + }}, + } + + createDeployment(kubeClient) + createWebhook(kubeClient, webhook) + err := ac.Register(TestContextWithLogger(t), kubeClient, []byte{}) + if err != nil { + t.Fatalf("Failed to create webhook: %s", err) + } + + currentWebhook, _ := kubeClient.AdmissionregistrationV1beta1().MutatingWebhookConfigurations().Get(ac.Options.WebhookName, metav1.GetOptions{}) + if reflect.DeepEqual(currentWebhook.Webhooks, webhook.Webhooks) { + t.Fatalf("Expected webhook to be updated") + } +} + +func createDeployment(kubeClient kubernetes.Interface) { + deployment := &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: "whatever", + Namespace: "knative-something", + }, + } + kubeClient.Apps().Deployments("knative-something").Create(deployment) +} + +func createWebhook(kubeClient kubernetes.Interface, webhook *admissionregistrationv1beta1.MutatingWebhookConfiguration) { + client := kubeClient.AdmissionregistrationV1beta1().MutatingWebhookConfigurations() + _, err := client.Create(webhook) + if err != nil { + panic(fmt.Sprintf("failed to create test webhook: %s", err)) + } +} + +func expectAllowed(t *testing.T, resp *admissionv1beta1.AdmissionResponse) { + t.Helper() + if !resp.Allowed { + t.Errorf("Expected allowed, but failed with %+v", resp.Result) + } +} + +func expectFailsWith(t *testing.T, resp *admissionv1beta1.AdmissionResponse, contains string) { + t.Helper() + if resp.Allowed { + t.Error("Expected denial, got allowed") + return + } + if !strings.Contains(resp.Result.Message, contains) { + t.Errorf("Expected failure containing %q got %q", contains, resp.Result.Message) + } +} + +func expectPatches(t *testing.T, a []byte, e []jsonpatch.JsonPatchOperation) { + t.Helper() + var got []jsonpatch.JsonPatchOperation + + err := json.Unmarshal(a, &got) + if err != nil { + t.Errorf("Failed to unmarshal patches: %s", err) + return + } + + // Give the patch a deterministic ordering. + // Technically this can change the meaning, but the ordering is otherwise unstable + // and difficult to test. + sort.Slice(e, func(i, j int) bool { + lhs, rhs := e[i], e[j] + if lhs.Operation != rhs.Operation { + return lhs.Operation < rhs.Operation + } + return lhs.Path < rhs.Path + }) + sort.Slice(got, func(i, j int) bool { + lhs, rhs := got[i], got[j] + if lhs.Operation != rhs.Operation { + return lhs.Operation < rhs.Operation + } + return lhs.Path < rhs.Path + }) + + // Even though diff is useful, seeing the whole objects + // one under another helps a lot. + t.Logf("Got Patches: %#v", got) + t.Logf("Want Patches: %#v", e) + if diff := cmp.Diff(e, got, cmpopts.EquateEmpty()); diff != "" { + t.Logf("diff Patches: %v", diff) + t.Errorf("expectPatches (-want, +got) = %s", diff) + } +} + +func setUserAnnotation(userC, userU string) jsonpatch.JsonPatchOperation { + return jsonpatch.JsonPatchOperation{ + Operation: "add", + Path: "/metadata/annotations", + Value: map[string]interface{}{ + "pkg.knative.dev/creator": userC, + "pkg.knative.dev/lastModifier": userU, + }, + } +} + +func NewTestResourceAdmissionController(options ControllerOptions) *ResourceAdmissionController { + // Use different versions and domains, for coverage. + handlers := newHandlers() + return &ResourceAdmissionController{ + Handlers: handlers, + Options: options, + DisallowUnknownFields: true, + } +} diff --git a/webhook/webhook.go b/webhook/webhook.go index 15d90e5ea..2efdaefb6 100644 --- a/webhook/webhook.go +++ b/webhook/webhook.go @@ -17,7 +17,6 @@ limitations under the License. package webhook import ( - "bytes" "context" "crypto/tls" "crypto/x509" @@ -25,30 +24,22 @@ import ( "errors" "fmt" "net/http" - "sort" - "strings" "time" "go.uber.org/zap" "knative.dev/pkg/apis" "knative.dev/pkg/apis/duck" - "knative.dev/pkg/kmp" "knative.dev/pkg/logging" "knative.dev/pkg/logging/logkey" - "github.com/markbates/inflect" - "github.com/mattbaird/jsonpatch" admissionv1beta1 "k8s.io/api/admission/v1beta1" - admissionregistrationv1beta1 "k8s.io/api/admissionregistration/v1beta1" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/client-go/kubernetes" - clientadmissionregistrationv1beta1 "k8s.io/client-go/kubernetes/typed/admissionregistration/v1beta1" ) const ( @@ -105,34 +96,15 @@ type ControllerOptions struct { StatsReporter StatsReporter } -// ResourceCallback defines a signature for resource specific (Route, Configuration, etc.) -// handlers that can validate and mutate an object. If non-nil error is returned, object creation -// is denied. Mutations should be appended to the patches operations. -type ResourceCallback func(patches *[]jsonpatch.JsonPatchOperation, old GenericCRD, new GenericCRD) error - -// ResourceDefaulter defines a signature for resource specific (Route, Configuration, etc.) -// handlers that can set defaults on an object. If non-nil error is returned, object creation -// is denied. Mutations should be appended to the patches operations. -type ResourceDefaulter func(patches *[]jsonpatch.JsonPatchOperation, crd GenericCRD) error - // AdmissionController implements the external admission webhook for validation of // pilot configuration. type AdmissionController struct { - Client kubernetes.Interface - Options ControllerOptions - Handlers map[schema.GroupVersionKind]GenericCRD - Logger *zap.SugaredLogger + Client kubernetes.Interface + Options ControllerOptions + Logger *zap.SugaredLogger + resourceAdmissionController ResourceAdmissionController - WithContext func(context.Context) context.Context - DisallowUnknownFields bool -} - -// 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 + WithContext func(context.Context) context.Context } // NewAdmissionController constructs an AdmissionController @@ -153,12 +125,15 @@ func NewAdmissionController( } return &AdmissionController{ - Client: client, - Options: opts, - Handlers: handlers, - Logger: logger, - WithContext: ctx, - DisallowUnknownFields: disallowUnknownFields, + Client: client, + Options: opts, + resourceAdmissionController: ResourceAdmissionController{ + Handlers: handlers, + Options: opts, + DisallowUnknownFields: disallowUnknownFields, + }, + Logger: logger, + WithContext: ctx, }, nil } @@ -238,7 +213,7 @@ func getOrGenerateKeyCertsFromSecret(ctx context.Context, client kubernetes.Inte // 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 GenericCRD) error { +func validate(ctx context.Context, new apis.Validatable) error { if apis.IsInUpdate(ctx) { old := apis.GetBaseline(ctx) if immutableNew, ok := new.(apis.Immutable); ok { @@ -317,8 +292,7 @@ func (ac *AdmissionController) Run(stop <-chan struct{}) error { select { case <-time.After(ac.Options.RegistrationDelay): - cl := ac.Client.AdmissionregistrationV1beta1().MutatingWebhookConfigurations() - if err := ac.register(ctx, cl, caCert); err != nil { + if err := ac.resourceAdmissionController.Register(ctx, ac.Client, caCert); err != nil { logger.Errorw("failed to register webhook", zap.Error(err)) return err } @@ -343,97 +317,6 @@ func (ac *AdmissionController) Run(stop <-chan struct{}) error { } } -// Register registers the external admission webhook for pilot -// configuration types. -func (ac *AdmissionController) register( - ctx context.Context, client clientadmissionregistrationv1beta1.MutatingWebhookConfigurationInterface, caCert []byte) error { // nolint: lll - logger := logging.FromContext(ctx) - failurePolicy := admissionregistrationv1beta1.Fail - - 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] - }) - - webhook := &admissionregistrationv1beta1.MutatingWebhookConfiguration{ - ObjectMeta: metav1.ObjectMeta{ - Name: ac.Options.WebhookName, - }, - Webhooks: []admissionregistrationv1beta1.Webhook{{ - Name: ac.Options.WebhookName, - Rules: rules, - ClientConfig: admissionregistrationv1beta1.WebhookClientConfig{ - Service: &admissionregistrationv1beta1.ServiceReference{ - Namespace: ac.Options.Namespace, - Name: ac.Options.ServiceName, - }, - CABundle: caCert, - }, - FailurePolicy: &failurePolicy, - }}, - } - - // Set the owner to our deployment. - deployment, err := ac.Client.Apps().Deployments(ac.Options.Namespace).Get(ac.Options.DeploymentName, metav1.GetOptions{}) - if err != nil { - return fmt.Errorf("failed to fetch our deployment: %v", err) - } - deploymentRef := metav1.NewControllerRef(deployment, deploymentKind) - webhook.OwnerReferences = append(webhook.OwnerReferences, *deploymentRef) - - // Try to create the webhook and if it already exists validate webhook rules. - _, err = client.Create(webhook) - if err != nil { - if !apierrors.IsAlreadyExists(err) { - return fmt.Errorf("failed to create a webhook: %v", err) - } - logger.Info("Webhook already exists") - configuredWebhook, err := client.Get(ac.Options.WebhookName, metav1.GetOptions{}) - if err != nil { - return fmt.Errorf("error retrieving webhook: %v", err) - } - if ok, err := kmp.SafeEqual(configuredWebhook.Webhooks, webhook.Webhooks); err != nil { - return fmt.Errorf("error diffing webhooks: %v", err) - } else if !ok { - logger.Info("Updating webhook") - // Set the ResourceVersion as required by update. - webhook.ObjectMeta.ResourceVersion = configuredWebhook.ObjectMeta.ResourceVersion - if _, err := client.Update(webhook); err != nil { - return fmt.Errorf("failed to update webhook: %s", err) - } - } else { - logger.Info("Webhook is already valid") - } - } else { - logger.Info("Created a webhook") - } - return nil -} - // ServeHTTP implements the external admission webhook for mutating // serving resources. func (ac *AdmissionController) ServeHTTP(w http.ResponseWriter, r *http.Request) { @@ -468,7 +351,7 @@ func (ac *AdmissionController) ServeHTTP(w http.ResponseWriter, r *http.Request) ctx = ac.WithContext(ctx) } - reviewResponse := ac.admit(ctx, review.Request) + reviewResponse := ac.resourceAdmissionController.Admit(ctx, review.Request) var response admissionv1beta1.AdmissionReview if reviewResponse != nil { response.Response = reviewResponse @@ -497,174 +380,6 @@ func makeErrorStatus(reason string, args ...interface{}) *admissionv1beta1.Admis } } -func (ac *AdmissionController) admit(ctx context.Context, request *admissionv1beta1.AdmissionRequest) *admissionv1beta1.AdmissionResponse { - 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} - } - - patchBytes, err := ac.mutate(ctx, request) - if err != nil { - return makeErrorStatus("mutation failed: %v", err) - } - logger.Infof("Kind: %q PatchBytes: %v", request.Kind, string(patchBytes)) - - return &admissionv1beta1.AdmissionResponse{ - Patch: patchBytes, - Allowed: true, - PatchType: func() *admissionv1beta1.PatchType { - pt := admissionv1beta1.PatchTypeJSONPatch - return &pt - }(), - } -} - -func (ac *AdmissionController) mutate(ctx context.Context, req *admissionv1beta1.AdmissionRequest) ([]byte, 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 nil, fmt.Errorf("unhandled kind: %v", gvk) - } - - // nil values denote absence of `old` (create) or `new` (delete) objects. - var oldObj, newObj GenericCRD - - if len(newBytes) != 0 { - newObj = handler.DeepCopyObject().(GenericCRD) - newDecoder := json.NewDecoder(bytes.NewBuffer(newBytes)) - if ac.DisallowUnknownFields { - newDecoder.DisallowUnknownFields() - } - if err := newDecoder.Decode(&newObj); err != nil { - return nil, fmt.Errorf("cannot decode incoming new object: %v", err) - } - } - if len(oldBytes) != 0 { - oldObj = handler.DeepCopyObject().(GenericCRD) - oldDecoder := json.NewDecoder(bytes.NewBuffer(oldBytes)) - if ac.DisallowUnknownFields { - oldDecoder.DisallowUnknownFields() - } - if err := oldDecoder.Decode(&oldObj); err != nil { - return nil, fmt.Errorf("cannot decode incoming old object: %v", err) - } - } - var patches duck.JSONPatch - - var err error - // Skip this step if the type we're dealing with is a duck type, since it is inherently - // incomplete and this will patch away all of the unspecified fields. - if _, ok := newObj.(duck.Populatable); !ok { - // Add these before defaulting fields, otherwise defaulting may cause an illegal patch - // because it expects the round tripped through Golang fields to be present already. - rtp, err := roundTripPatch(newBytes, newObj) - if err != nil { - return nil, fmt.Errorf("cannot create patch for round tripped newBytes: %v", err) - } - patches = append(patches, rtp...) - } - - // Set up the context for defaulting and validation - 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.SetDefaults(ctx) - - s, ok := oldObj.(apis.HasSpec) - if ok { - SetUserInfoAnnotations(s, ctx, req.Resource.Group) - } - - 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) - - // Default the new object. - 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 = ac.setUserInfoAnnotations(ctx, patches, newObj, req.Resource.Group); err != nil { - logger.Errorw("Failed the resource user info annotator", zap.Error(err)) - return nil, err - } - - // None of the validators will accept a nil value for newObj. - 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 *AdmissionController) setUserInfoAnnotations(ctx context.Context, patches duck.JSONPatch, new GenericCRD, groupName string) (duck.JSONPatch, error) { - if new == nil { - return patches, nil - } - nh, ok := new.(apis.HasSpec) - if !ok { - return patches, nil - } - - b, a := new.DeepCopyObject().(apis.HasSpec), nh - - SetUserInfoAnnotations(nh, ctx, groupName) - - patch, err := duck.CreatePatch(b, a) - if err != nil { - return nil, err - } - return append(patches, patch...), nil -} - -// roundTripPatch generates the JSONPatch that corresponds to round tripping the given bytes through -// the Golang type (JSON -> Golang type -> JSON). Because it is not always true that -// bytes == json.Marshal(json.Unmarshal(bytes)). -// -// For example, if bytes did not contain a 'spec' field and the Golang type specifies its 'spec' -// field without omitempty, then by round tripping through the Golang type, we would have added -// `'spec': {}`. -func roundTripPatch(bytes []byte, unmarshalled interface{}) (duck.JSONPatch, error) { - if unmarshalled == nil { - return duck.JSONPatch{}, nil - } - marshaledBytes, err := json.Marshal(unmarshalled) - if err != nil { - return nil, fmt.Errorf("cannot marshal interface: %v", err) - } - return jsonpatch.CreatePatch(bytes, marshaledBytes) -} - func generateSecret(ctx context.Context, options *ControllerOptions) (*corev1.Secret, error) { serverKey, serverCert, caCert, err := CreateCerts(ctx, options.ServiceName, options.Namespace) if err != nil { diff --git a/webhook/webhook_integration_test.go b/webhook/webhook_integration_test.go index 55d06b20e..1aeb817f3 100644 --- a/webhook/webhook_integration_test.go +++ b/webhook/webhook_integration_test.go @@ -464,19 +464,19 @@ func testSetup(t *testing.T) (*AdmissionController, string, error) { defaultOpts := newDefaultOptions() defaultOpts.Port = port - _, ac := newNonRunningTestAdmissionController(t, defaultOpts) + kubeClient, ac := newNonRunningTestAdmissionController(t, defaultOpts) - nsErr := createNamespace(t, ac.Client, metav1.NamespaceSystem) + nsErr := createNamespace(t, kubeClient, metav1.NamespaceSystem) if nsErr != nil { return nil, "", nsErr } - cMapsErr := createTestConfigMap(t, ac.Client) + cMapsErr := createTestConfigMap(t, kubeClient) if cMapsErr != nil { return nil, "", cMapsErr } - createDeployment(ac) + createDeployment(kubeClient) resetMetrics() return ac, fmt.Sprintf("0.0.0.0:%d", port), nil } @@ -484,17 +484,17 @@ func testSetup(t *testing.T) (*AdmissionController, string, error) { func TestSetupWebhookHTTPServerError(t *testing.T) { defaultOpts := newDefaultOptions() defaultOpts.Port = -1 // invalid port - _, ac := newNonRunningTestAdmissionController(t, defaultOpts) + kubeClient, ac := newNonRunningTestAdmissionController(t, defaultOpts) - nsErr := createNamespace(t, ac.Client, metav1.NamespaceSystem) + nsErr := createNamespace(t, kubeClient, metav1.NamespaceSystem) if nsErr != nil { t.Fatalf("testSetup() = %v", nsErr) } - cMapsErr := createTestConfigMap(t, ac.Client) + cMapsErr := createTestConfigMap(t, kubeClient) if cMapsErr != nil { t.Fatalf("testSetup() = %v", cMapsErr) } - createDeployment(ac) + createDeployment(kubeClient) stopCh := make(chan struct{}) errCh := make(chan error) diff --git a/webhook/webhook_test.go b/webhook/webhook_test.go index b6b7bd847..b263f8604 100644 --- a/webhook/webhook_test.go +++ b/webhook/webhook_test.go @@ -1,5 +1,5 @@ /* -Copyright 2017 The Knative Authors +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. @@ -17,34 +17,22 @@ limitations under the License. package webhook import ( - "context" "crypto/tls" - "encoding/json" "encoding/pem" "fmt" "net" - "reflect" - "sort" "strings" "testing" "time" - // "knative.dev/pkg/apis/duck" - "golang.org/x/sync/errgroup" - "github.com/google/go-cmp/cmp" - "github.com/google/go-cmp/cmp/cmpopts" - "github.com/mattbaird/jsonpatch" "go.uber.org/zap" - admissionv1beta1 "k8s.io/api/admission/v1beta1" + "golang.org/x/sync/errgroup" admissionregistrationv1beta1 "k8s.io/api/admissionregistration/v1beta1" - appsv1 "k8s.io/api/apps/v1" - authenticationv1 "k8s.io/api/authentication/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/client-go/kubernetes" fakekubeclientset "k8s.io/client-go/kubernetes/fake" - "knative.dev/pkg/apis" . "knative.dev/pkg/logging/testing" . "knative.dev/pkg/testing" @@ -60,6 +48,31 @@ func newDefaultOptions() ControllerOptions { } } +func newHandlers() map[schema.GroupVersionKind]GenericCRD { + return map[schema.GroupVersionKind]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{}, + } +} + const ( testNamespace = "test-namespace" testResourceName = "test-resource" @@ -81,482 +94,9 @@ func newNonRunningTestAdmissionController(t *testing.T, options ControllerOption return } -func TestDeleteAllowed(t *testing.T) { - _, ac := newNonRunningTestAdmissionController(t, newDefaultOptions()) - - 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 := newNonRunningTestAdmissionController(t, newDefaultOptions()) - - 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 := newNonRunningTestAdmissionController(t, newDefaultOptions()) - - 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 := newNonRunningTestAdmissionController(t, newDefaultOptions()) - 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 := newNonRunningTestAdmissionController(t, newDefaultOptions()) - 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), - `mutation 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 - patches []jsonpatch.JsonPatchOperation - }{{ - 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, - } - }, - patches: []jsonpatch.JsonPatchOperation{}, - }, { - 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, - } - }, - patches: []jsonpatch.JsonPatchOperation{}, - }, { - name: "test simple creation (with defaults)", - setup: func(ctx context.Context, r *Resource) { - }, - patches: []jsonpatch.JsonPatchOperation{{ - Operation: "add", - Path: "/metadata/annotations", - Value: map[string]interface{}{ - "pkg.knative.dev/creator": user1, - "pkg.knative.dev/lastModifier": user1, - }, - }, { - Operation: "add", - Path: "/spec/fieldThatsImmutableWithDefault", - Value: "this is another default value", - }, { - Operation: "add", - Path: "/spec/fieldWithDefault", - Value: "I'm a default.", - }}, - }, { - name: "test simple creation (with defaults around annotations)", - setup: func(ctx context.Context, r *Resource) { - r.Annotations = map[string]string{ - "foo": "bar", - } - }, - patches: []jsonpatch.JsonPatchOperation{{ - Operation: "add", - Path: "/metadata/annotations/pkg.knative.dev~1creator", - Value: user1, - }, { - Operation: "add", - Path: "/metadata/annotations/pkg.knative.dev~1lastModifier", - Value: user1, - }, { - Operation: "add", - Path: "/spec/fieldThatsImmutableWithDefault", - Value: "this is another default value", - }, { - Operation: "add", - Path: "/spec/fieldWithDefault", - Value: "I'm a default.", - }}, - }, { - name: "test simple creation (with partially overridden defaults)", - setup: func(ctx context.Context, r *Resource) { - r.Spec.FieldThatsImmutableWithDefault = "not the default" - }, - patches: []jsonpatch.JsonPatchOperation{{ - Operation: "add", - Path: "/metadata/annotations", - Value: map[string]interface{}{ - "pkg.knative.dev/creator": user1, - "pkg.knative.dev/lastModifier": user1, - }, - }, { - Operation: "add", - Path: "/spec/fieldWithDefault", - Value: "I'm a default.", - }}, - }, { - name: "test simple creation (webhook corrects user annotation)", - setup: func(ctx context.Context, r *Resource) { - r.SetDefaults(ctx) - // THIS IS NOT WHO IS CREATING IT, IT IS LIES! - r.Annotations = map[string]string{ - "pkg.knative.dev/lastModifier": user2, - } - }, - patches: []jsonpatch.JsonPatchOperation{{ - Operation: "replace", - Path: "/metadata/annotations/pkg.knative.dev~1lastModifier", - Value: user1, - }, { - Operation: "add", - 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 { - 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 := newNonRunningTestAdmissionController(t, newDefaultOptions()) - resp := ac.admit(ctx, createCreateResource(ctx, r)) - - if tc.rejection == "" { - expectAllowed(t, resp) - expectPatches(t, resp.Patch, tc.patches) - } 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 - patches []jsonpatch.JsonPatchOperation - }{{ - 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. - }, - patches: []jsonpatch.JsonPatchOperation{}, - }, { - name: "test simple update (update updater annotation)", - setup: func(ctx context.Context, r *Resource) { - r.SetDefaults(ctx) - }, - mutate: func(ctx context.Context, r *Resource) { - // When we change the spec, the updater - // annotation changes. - r.Spec.FieldWithDefault = "not the default" - }, - patches: []jsonpatch.JsonPatchOperation{{ - Operation: "replace", - Path: "/metadata/annotations/pkg.knative.dev~1lastModifier", - Value: user2, - }}, - }, { - name: "test simple update (annotation change doesn't change updater)", - setup: func(ctx context.Context, r *Resource) { - r.SetDefaults(ctx) - }, - mutate: func(ctx context.Context, r *Resource) { - // When we change an annotation, the updater doesn't change. - r.Annotations["foo"] = "bar" - }, - patches: []jsonpatch.JsonPatchOperation{}, - }, { - name: "test that updates dropping immutable defaults are filled back in", - setup: func(ctx context.Context, r *Resource) { - r.SetDefaults(ctx) - r.Spec.FieldThatsImmutableWithDefault = "" - }, - mutate: func(ctx context.Context, r *Resource) { - r.Spec.FieldThatsImmutableWithDefault = "" - }, - patches: []jsonpatch.JsonPatchOperation{{ - Operation: "add", - 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 { - 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 := newNonRunningTestAdmissionController(t, newDefaultOptions()) - resp := ac.admit(ctx, createUpdateResource(ctx, old, new)) - - if tc.rejection == "" { - expectAllowed(t, resp) - expectPatches(t, resp.Patch, tc.patches) - } 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 TestValidCreateResourceSucceedsWithRoundTripAndDefaultPatch(t *testing.T) { - req := &admissionv1beta1.AdmissionRequest{ - Operation: admissionv1beta1.Create, - Kind: metav1.GroupVersionKind{ - Group: "pkg.knative.dev", - Version: "v1alpha1", - Kind: "InnerDefaultResource", - }, - } - req.Object.Raw = createInnerDefaultResourceWithoutSpec(t) - - _, ac := newNonRunningTestAdmissionController(t, newDefaultOptions()) - resp := ac.admit(TestContextWithLogger(t), req) - expectAllowed(t, resp) - expectPatches(t, resp.Patch, []jsonpatch.JsonPatchOperation{{ - Operation: "add", - Path: "/spec", - Value: map[string]interface{}{}, - }, { - Operation: "add", - Path: "/spec/fieldWithDefault", - Value: "I'm a default.", - }}) -} - -func createInnerDefaultResourceWithoutSpec(t *testing.T) []byte { - t.Helper() - r := InnerDefaultResource{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: testNamespace, - 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: testNamespace, - 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 TestValidWebhook(t *testing.T) { - _, ac := newNonRunningTestAdmissionController(t, newDefaultOptions()) - createDeployment(ac) - ac.register(TestContextWithLogger(t), ac.Client.AdmissionregistrationV1beta1().MutatingWebhookConfigurations(), []byte{}) - _, err := ac.Client.AdmissionregistrationV1beta1().MutatingWebhookConfigurations().Get(ac.Options.WebhookName, metav1.GetOptions{}) - if err != nil { - t.Fatalf("Failed to create webhook: %s", err) - } -} - -func TestUpdatingWebhook(t *testing.T) { - _, ac := newNonRunningTestAdmissionController(t, newDefaultOptions()) - webhook := &admissionregistrationv1beta1.MutatingWebhookConfiguration{ - ObjectMeta: metav1.ObjectMeta{ - Name: ac.Options.WebhookName, - }, - Webhooks: []admissionregistrationv1beta1.Webhook{{ - Name: ac.Options.WebhookName, - Rules: []admissionregistrationv1beta1.RuleWithOperations{{}}, - ClientConfig: admissionregistrationv1beta1.WebhookClientConfig{}, - }}, - } - - createDeployment(ac) - createWebhook(ac, webhook) - ac.register(TestContextWithLogger(t), ac.Client.AdmissionregistrationV1beta1().MutatingWebhookConfigurations(), []byte{}) - currentWebhook, _ := ac.Client.AdmissionregistrationV1beta1().MutatingWebhookConfigurations().Get(ac.Options.WebhookName, metav1.GetOptions{}) - if reflect.DeepEqual(currentWebhook.Webhooks, webhook.Webhooks) { - t.Fatalf("Expected webhook to be updated") - } -} - func TestRegistrationStopChanFire(t *testing.T) { opts := newDefaultOptions() - _, ac := newNonRunningTestAdmissionController(t, opts) + kubeClient, ac := newNonRunningTestAdmissionController(t, opts) webhook := &admissionregistrationv1beta1.MutatingWebhookConfiguration{ ObjectMeta: metav1.ObjectMeta{ Name: ac.Options.WebhookName, @@ -569,7 +109,7 @@ func TestRegistrationStopChanFire(t *testing.T) { }, }, } - createWebhook(ac, webhook) + createWebhook(kubeClient, webhook) ac.Options.RegistrationDelay = 1 * time.Minute stopCh := make(chan struct{}) @@ -591,7 +131,7 @@ func TestRegistrationStopChanFire(t *testing.T) { } func TestRegistrationForAlreadyExistingWebhook(t *testing.T) { - _, ac := newNonRunningTestAdmissionController(t, newDefaultOptions()) + kubeClient, ac := newNonRunningTestAdmissionController(t, newDefaultOptions()) webhook := &admissionregistrationv1beta1.MutatingWebhookConfiguration{ ObjectMeta: metav1.ObjectMeta{ Name: ac.Options.WebhookName, @@ -604,7 +144,7 @@ func TestRegistrationForAlreadyExistingWebhook(t *testing.T) { }, }, } - createWebhook(ac, webhook) + createWebhook(kubeClient, webhook) ac.Options.RegistrationDelay = 1 * time.Millisecond stopCh := make(chan struct{}) @@ -641,8 +181,8 @@ func TestCertConfigurationForAlreadyGeneratedSecret(t *testing.T) { t.Fatalf("Failed to create secret: %v", err) } - createNamespace(t, ac.Client, metav1.NamespaceSystem) - createTestConfigMap(t, ac.Client) + createNamespace(t, kubeClient, metav1.NamespaceSystem) + createTestConfigMap(t, kubeClient) tlsConfig, caCert, err := configureCerts(ctx, kubeClient, &ac.Options) if err != nil { @@ -677,8 +217,8 @@ func TestCertConfigurationForGeneratedSecret(t *testing.T) { kubeClient, ac := newNonRunningTestAdmissionController(t, opts) ctx := TestContextWithLogger(t) - createNamespace(t, ac.Client, metav1.NamespaceSystem) - createTestConfigMap(t, ac.Client) + createNamespace(t, kubeClient, metav1.NamespaceSystem) + createTestConfigMap(t, kubeClient) tlsConfig, caCert, err := configureCerts(ctx, kubeClient, &ac.Options) if err != nil { @@ -709,115 +249,8 @@ func TestSettingWebhookClientAuth(t *testing.T) { } } -func createDeployment(ac *AdmissionController) { - deployment := &appsv1.Deployment{ - ObjectMeta: metav1.ObjectMeta{ - Name: "whatever", - Namespace: "knative-something", - }, - } - ac.Client.Apps().Deployments("knative-something").Create(deployment) -} - -func createWebhook(ac *AdmissionController, webhook *admissionregistrationv1beta1.MutatingWebhookConfiguration) { - client := ac.Client.AdmissionregistrationV1beta1().MutatingWebhookConfigurations() - _, err := client.Create(webhook) - if err != nil { - panic(fmt.Sprintf("failed to create test webhook: %s", err)) - } -} - -func expectAllowed(t *testing.T, resp *admissionv1beta1.AdmissionResponse) { - t.Helper() - if !resp.Allowed { - t.Errorf("Expected allowed, but failed with %+v", resp.Result) - } -} - -func expectFailsWith(t *testing.T, resp *admissionv1beta1.AdmissionResponse, contains string) { - t.Helper() - if resp.Allowed { - t.Error("Expected denial, got allowed") - return - } - if !strings.Contains(resp.Result.Message, contains) { - t.Errorf("Expected failure containing %q got %q", contains, resp.Result.Message) - } -} - -func expectPatches(t *testing.T, a []byte, e []jsonpatch.JsonPatchOperation) { - t.Helper() - var got []jsonpatch.JsonPatchOperation - - err := json.Unmarshal(a, &got) - if err != nil { - t.Errorf("Failed to unmarshal patches: %s", err) - return - } - - // Give the patch a deterministic ordering. - // Technically this can change the meaning, but the ordering is otherwise unstable - // and difficult to test. - sort.Slice(e, func(i, j int) bool { - lhs, rhs := e[i], e[j] - if lhs.Operation != rhs.Operation { - return lhs.Operation < rhs.Operation - } - return lhs.Path < rhs.Path - }) - sort.Slice(got, func(i, j int) bool { - lhs, rhs := got[i], got[j] - if lhs.Operation != rhs.Operation { - return lhs.Operation < rhs.Operation - } - return lhs.Path < rhs.Path - }) - - // Even though diff is useful, seeing the whole objects - // one under another helps a lot. - t.Logf("Got Patches: %#v", got) - t.Logf("Want Patches: %#v", e) - if diff := cmp.Diff(e, got, cmpopts.EquateEmpty()); diff != "" { - t.Logf("diff Patches: %v", diff) - t.Errorf("expectPatches (-want, +got) = %s", diff) - } -} - -func setUserAnnotation(userC, userU string) jsonpatch.JsonPatchOperation { - return jsonpatch.JsonPatchOperation{ - Operation: "add", - Path: "/metadata/annotations", - Value: map[string]interface{}{ - "pkg.knative.dev/creator": userC, - "pkg.knative.dev/lastModifier": userU, - }, - } -} - func NewTestAdmissionController(client kubernetes.Interface, options ControllerOptions, logger *zap.SugaredLogger) (*AdmissionController, error) { - // Use different versions and domains, for coverage. - handlers := map[schema.GroupVersionKind]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{}, - } + handlers := newHandlers() return NewAdmissionController(client, options, handlers, logger, nil, true) }