diff --git a/pkg/admission/attributes.go b/pkg/admission/attributes.go index c8973cc62..ad6ca6ba6 100644 --- a/pkg/admission/attributes.go +++ b/pkg/admission/attributes.go @@ -34,6 +34,7 @@ type attributesRecord struct { resource schema.GroupVersionResource subresource string operation Operation + options runtime.Object dryRun bool object runtime.Object oldObject runtime.Object @@ -45,7 +46,7 @@ type attributesRecord struct { annotationsLock sync.RWMutex } -func NewAttributesRecord(object runtime.Object, oldObject runtime.Object, kind schema.GroupVersionKind, namespace, name string, resource schema.GroupVersionResource, subresource string, operation Operation, dryRun bool, userInfo user.Info) Attributes { +func NewAttributesRecord(object runtime.Object, oldObject runtime.Object, kind schema.GroupVersionKind, namespace, name string, resource schema.GroupVersionResource, subresource string, operation Operation, operationOptions runtime.Object, dryRun bool, userInfo user.Info) Attributes { return &attributesRecord{ kind: kind, namespace: namespace, @@ -53,6 +54,7 @@ func NewAttributesRecord(object runtime.Object, oldObject runtime.Object, kind s resource: resource, subresource: subresource, operation: operation, + options: operationOptions, dryRun: dryRun, object: object, oldObject: oldObject, @@ -84,6 +86,10 @@ func (record *attributesRecord) GetOperation() Operation { return record.operation } +func (record *attributesRecord) GetOperationOptions() runtime.Object { + return record.options +} + func (record *attributesRecord) IsDryRun() bool { return record.dryRun } diff --git a/pkg/admission/interfaces.go b/pkg/admission/interfaces.go index 866777cc7..a8853fdd7 100644 --- a/pkg/admission/interfaces.go +++ b/pkg/admission/interfaces.go @@ -41,6 +41,8 @@ type Attributes interface { GetSubresource() string // GetOperation is the operation being performed GetOperation() Operation + // GetOperationOptions is the options for the operation being performed + GetOperationOptions() runtime.Object // IsDryRun indicates that modifications will definitely not be persisted for this request. This is to prevent // admission controllers with side effects and a method of reconciliation from being overwhelmed. // However, a value of false for this does not mean that the modification will be persisted, because it diff --git a/pkg/admission/plugin/webhook/request/admissionreview.go b/pkg/admission/plugin/webhook/request/admissionreview.go index cec41315c..51a45a36a 100644 --- a/pkg/admission/plugin/webhook/request/admissionreview.go +++ b/pkg/admission/plugin/webhook/request/admissionreview.go @@ -68,6 +68,9 @@ func CreateAdmissionReview(attr *generic.VersionedAttributes) admissionv1beta1.A Object: attr.VersionedOldObject, }, DryRun: &dryRun, + Options: runtime.RawExtension{ + Object: attr.GetOperationOptions(), + }, }, } } diff --git a/pkg/endpoints/handlers/create.go b/pkg/endpoints/handlers/create.go index 40c53efae..d4a31b7e7 100644 --- a/pkg/endpoints/handlers/create.go +++ b/pkg/endpoints/handlers/create.go @@ -106,6 +106,7 @@ func createHandler(r rest.NamedCreater, scope *RequestScope, admit admission.Int scope.err(err, w, req) return } + options.TypeMeta.SetGroupVersionKind(metav1.SchemeGroupVersion.WithKind("CreateOptions")) defaultGVK := scope.Kind original := r.New() @@ -128,7 +129,7 @@ func createHandler(r rest.NamedCreater, scope *RequestScope, admit admission.Int audit.LogRequestObject(ae, obj, scope.Resource, scope.Subresource, scope.Serializer) userInfo, _ := request.UserFrom(ctx) - admissionAttributes := admission.NewAttributesRecord(obj, nil, scope.Kind, namespace, name, scope.Resource, scope.Subresource, admission.Create, dryrun.IsDryRun(options.DryRun), userInfo) + admissionAttributes := admission.NewAttributesRecord(obj, nil, scope.Kind, namespace, name, scope.Resource, scope.Subresource, admission.Create, options, dryrun.IsDryRun(options.DryRun), userInfo) if mutatingAdmission, ok := admit.(admission.MutationInterface); ok && mutatingAdmission.Handles(admission.Create) { err = mutatingAdmission.Admit(admissionAttributes, scope) if err != nil { diff --git a/pkg/endpoints/handlers/delete.go b/pkg/endpoints/handlers/delete.go index 8f1d1d217..7375c575e 100644 --- a/pkg/endpoints/handlers/delete.go +++ b/pkg/endpoints/handlers/delete.go @@ -113,11 +113,12 @@ func DeleteResource(r rest.GracefulDeleter, allowsOptions bool, scope *RequestSc scope.err(err, w, req) return } + options.TypeMeta.SetGroupVersionKind(metav1.SchemeGroupVersion.WithKind("DeleteOptions")) trace.Step("About to check admission control") if admit != nil && admit.Handles(admission.Delete) { userInfo, _ := request.UserFrom(ctx) - attrs := admission.NewAttributesRecord(nil, nil, scope.Kind, namespace, name, scope.Resource, scope.Subresource, admission.Delete, dryrun.IsDryRun(options.DryRun), userInfo) + attrs := admission.NewAttributesRecord(nil, nil, scope.Kind, namespace, name, scope.Resource, scope.Subresource, admission.Delete, options, dryrun.IsDryRun(options.DryRun), userInfo) if mutatingAdmission, ok := admit.(admission.MutationInterface); ok { if err := mutatingAdmission.Admit(attrs, scope); err != nil { scope.err(err, w, req) @@ -236,6 +237,8 @@ func DeleteCollection(r rest.CollectionDeleter, checkBody bool, scope *RequestSc scope.err(err, w, req) return } + // For backwards compatibility, we need to allow existing clients to submit per group DeleteOptions + // It is also allowed to pass a body with meta.k8s.io/v1.DeleteOptions defaultGVK := scope.Kind.GroupVersion().WithKind("DeleteOptions") obj, _, err := scope.Serializer.DecoderToVersion(s.Serializer, defaultGVK.GroupVersion()).Decode(body, &defaultGVK, options) if err != nil { @@ -262,11 +265,12 @@ func DeleteCollection(r rest.CollectionDeleter, checkBody bool, scope *RequestSc scope.err(err, w, req) return } + options.TypeMeta.SetGroupVersionKind(metav1.SchemeGroupVersion.WithKind("DeleteOptions")) admit = admission.WithAudit(admit, ae) if admit != nil && admit.Handles(admission.Delete) { userInfo, _ := request.UserFrom(ctx) - attrs := admission.NewAttributesRecord(nil, nil, scope.Kind, namespace, "", scope.Resource, scope.Subresource, admission.Delete, dryrun.IsDryRun(options.DryRun), userInfo) + attrs := admission.NewAttributesRecord(nil, nil, scope.Kind, namespace, "", scope.Resource, scope.Subresource, admission.Delete, options, dryrun.IsDryRun(options.DryRun), userInfo) if mutatingAdmission, ok := admit.(admission.MutationInterface); ok { err = mutatingAdmission.Admit(attrs, scope) if err != nil { diff --git a/pkg/endpoints/handlers/patch.go b/pkg/endpoints/handlers/patch.go index 23644bd34..c33f18166 100644 --- a/pkg/endpoints/handlers/patch.go +++ b/pkg/endpoints/handlers/patch.go @@ -23,7 +23,7 @@ import ( "strings" "time" - "github.com/evanphx/json-patch" + jsonpatch "github.com/evanphx/json-patch" "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" metainternalversion "k8s.io/apimachinery/pkg/apis/meta/internalversion" @@ -118,6 +118,7 @@ func PatchResource(r rest.Patcher, scope *RequestScope, admit admission.Interfac scope.err(err, w, req) return } + options.TypeMeta.SetGroupVersionKind(metav1.SchemeGroupVersion.WithKind("PatchOptions")) ae := request.AuditEventFrom(ctx) admit = admission.WithAudit(admit, ae) @@ -151,6 +152,7 @@ func PatchResource(r rest.Patcher, scope *RequestScope, admit admission.Interfac scope.Resource, scope.Subresource, admission.Create, + patchToCreateOptions(options), dryrun.IsDryRun(options.DryRun), userInfo) staticUpdateAttributes := admission.NewAttributesRecord( @@ -162,6 +164,7 @@ func PatchResource(r rest.Patcher, scope *RequestScope, admit admission.Interfac scope.Resource, scope.Subresource, admission.Update, + patchToUpdateOptions(options), dryrun.IsDryRun(options.DryRun), userInfo, ) @@ -489,9 +492,9 @@ func (p *patcher) applyPatch(_ context.Context, _, currentObject runtime.Object) return objToUpdate, nil } -func (p *patcher) admissionAttributes(ctx context.Context, updatedObject runtime.Object, currentObject runtime.Object, operation admission.Operation) admission.Attributes { +func (p *patcher) admissionAttributes(ctx context.Context, updatedObject runtime.Object, currentObject runtime.Object, operation admission.Operation, operationOptions runtime.Object) admission.Attributes { userInfo, _ := request.UserFrom(ctx) - return admission.NewAttributesRecord(updatedObject, currentObject, p.kind, p.namespace, p.name, p.resource, p.subresource, operation, p.dryRun, userInfo) + return admission.NewAttributesRecord(updatedObject, currentObject, p.kind, p.namespace, p.name, p.resource, p.subresource, operation, operationOptions, p.dryRun, userInfo) } // applyAdmission is called every time GuaranteedUpdate asks for the updated object, @@ -500,16 +503,19 @@ func (p *patcher) admissionAttributes(ctx context.Context, updatedObject runtime func (p *patcher) applyAdmission(ctx context.Context, patchedObject runtime.Object, currentObject runtime.Object) (runtime.Object, error) { p.trace.Step("About to check admission control") var operation admission.Operation + var options runtime.Object if hasUID, err := hasUID(currentObject); err != nil { return nil, err } else if !hasUID { operation = admission.Create currentObject = nil + options = patchToCreateOptions(p.options) } else { operation = admission.Update + options = patchToUpdateOptions(p.options) } if p.admissionCheck != nil && p.admissionCheck.Handles(operation) { - attributes := p.admissionAttributes(ctx, patchedObject, currentObject, operation) + attributes := p.admissionAttributes(ctx, patchedObject, currentObject, operation, options) return patchedObject, p.admissionCheck.Admit(attributes, p.objectInterfaces) } return patchedObject, nil @@ -551,11 +557,8 @@ func (p *patcher) patchResource(ctx context.Context, scope *RequestScope) (runti wasCreated := false p.updatedObjectInfo = rest.DefaultUpdatedObjectInfo(nil, p.applyPatch, p.applyAdmission) result, err := finishRequest(p.timeout, func() (runtime.Object, error) { - // TODO: Pass in UpdateOptions to override UpdateStrategy.AllowUpdateOnCreate - options, err := patchToUpdateOptions(p.options) - if err != nil { - return nil, err - } + // Pass in UpdateOptions to override UpdateStrategy.AllowUpdateOnCreate + options := patchToUpdateOptions(p.options) updateObject, created, updateErr := p.restPatcher.Update(ctx, p.name, p.updatedObjectInfo, p.createValidation, p.updateValidation, p.forceAllowCreate, options) wasCreated = created return updateObject, updateErr @@ -600,12 +603,28 @@ func interpretStrategicMergePatchError(err error) error { } } -func patchToUpdateOptions(po *metav1.PatchOptions) (*metav1.UpdateOptions, error) { - b, err := json.Marshal(po) - if err != nil { - return nil, err +// patchToUpdateOptions creates an UpdateOptions with the same field values as the provided PatchOptions. +func patchToUpdateOptions(po *metav1.PatchOptions) *metav1.UpdateOptions { + if po == nil { + return nil } - uo := metav1.UpdateOptions{} - err = json.Unmarshal(b, &uo) - return &uo, err + uo := &metav1.UpdateOptions{ + DryRun: po.DryRun, + FieldManager: po.FieldManager, + } + uo.TypeMeta.SetGroupVersionKind(metav1.SchemeGroupVersion.WithKind("UpdateOptions")) + return uo +} + +// patchToCreateOptions creates an CreateOptions with the same field values as the provided PatchOptions. +func patchToCreateOptions(po *metav1.PatchOptions) *metav1.CreateOptions { + if po == nil { + return nil + } + co := &metav1.CreateOptions{ + DryRun: po.DryRun, + FieldManager: po.FieldManager, + } + co.TypeMeta.SetGroupVersionKind(metav1.SchemeGroupVersion.WithKind("CreateOptions")) + return co } diff --git a/pkg/endpoints/handlers/rest.go b/pkg/endpoints/handlers/rest.go index 4db0c0676..cc82a8df8 100644 --- a/pkg/endpoints/handlers/rest.go +++ b/pkg/endpoints/handlers/rest.go @@ -137,14 +137,14 @@ func ConnectResource(connecter rest.Connecter, scope *RequestScope, admit admiss userInfo, _ := request.UserFrom(ctx) // TODO: remove the mutating admission here as soon as we have ported all plugin that handle CONNECT if mutatingAdmission, ok := admit.(admission.MutationInterface); ok { - err = mutatingAdmission.Admit(admission.NewAttributesRecord(opts, nil, scope.Kind, namespace, name, scope.Resource, scope.Subresource, admission.Connect, false, userInfo), scope) + err = mutatingAdmission.Admit(admission.NewAttributesRecord(opts, nil, scope.Kind, namespace, name, scope.Resource, scope.Subresource, admission.Connect, nil, false, userInfo), scope) if err != nil { scope.err(err, w, req) return } } if validatingAdmission, ok := admit.(admission.ValidationInterface); ok { - err = validatingAdmission.Validate(admission.NewAttributesRecord(opts, nil, scope.Kind, namespace, name, scope.Resource, scope.Subresource, admission.Connect, false, userInfo), scope) + err = validatingAdmission.Validate(admission.NewAttributesRecord(opts, nil, scope.Kind, namespace, name, scope.Resource, scope.Subresource, admission.Connect, nil, false, userInfo), scope) if err != nil { scope.err(err, w, req) return diff --git a/pkg/endpoints/handlers/rest_test.go b/pkg/endpoints/handlers/rest_test.go index 2c9dab5b9..4c33f05e7 100644 --- a/pkg/endpoints/handlers/rest_test.go +++ b/pkg/endpoints/handlers/rest_test.go @@ -26,7 +26,8 @@ import ( "testing" "time" - "github.com/evanphx/json-patch" + jsonpatch "github.com/evanphx/json-patch" + fuzz "github.com/google/gofuzz" apiequality "k8s.io/apimachinery/pkg/api/equality" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -37,6 +38,7 @@ import ( "k8s.io/apimachinery/pkg/runtime/serializer" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/diff" + "k8s.io/apimachinery/pkg/util/json" utilruntime "k8s.io/apimachinery/pkg/util/runtime" "k8s.io/apimachinery/pkg/util/strategicpatch" "k8s.io/apiserver/pkg/admission" @@ -1000,3 +1002,89 @@ func (alwaysErrorTyper) ObjectKinds(runtime.Object) ([]schema.GroupVersionKind, func (alwaysErrorTyper) Recognizes(gvk schema.GroupVersionKind) bool { return false } + +func TestUpdateToCreateOptions(t *testing.T) { + f := fuzz.New() + for i := 0; i < 100; i++ { + t.Run(fmt.Sprintf("Run %d/100", i), func(t *testing.T) { + update := &metav1.UpdateOptions{} + f.Fuzz(update) + create := updateToCreateOptions(update) + + b, err := json.Marshal(create) + if err != nil { + t.Fatalf("failed to marshal CreateOptions (%v): %v", err, create) + } + got := &metav1.UpdateOptions{} + err = json.Unmarshal(b, &got) + if err != nil { + t.Fatalf("failed to unmarshal UpdateOptions: %v", err) + } + got.TypeMeta = metav1.TypeMeta{} + update.TypeMeta = metav1.TypeMeta{} + if !reflect.DeepEqual(*update, *got) { + t.Fatalf(`updateToCreateOptions round-trip failed: +got: %#+v +want: %#+v`, got, update) + } + + }) + } +} + +func TestPatchToUpdateOptions(t *testing.T) { + tests := []struct { + name string + converterFn func(po *metav1.PatchOptions) interface{} + }{ + { + name: "patchToUpdateOptions", + converterFn: func(patch *metav1.PatchOptions) interface{} { + return patchToUpdateOptions(patch) + }, + }, + { + name: "patchToCreateOptions", + converterFn: func(patch *metav1.PatchOptions) interface{} { + return patchToCreateOptions(patch) + }, + }, + } + + f := fuzz.New() + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + for i := 0; i < 100; i++ { + t.Run(fmt.Sprintf("Run %d/100", i), func(t *testing.T) { + patch := &metav1.PatchOptions{} + f.Fuzz(patch) + converted := test.converterFn(patch) + + b, err := json.Marshal(converted) + if err != nil { + t.Fatalf("failed to marshal converted object (%v): %v", err, converted) + } + got := &metav1.PatchOptions{} + err = json.Unmarshal(b, &got) + if err != nil { + t.Fatalf("failed to unmarshal converted object: %v", err) + } + + // Clear TypeMeta because we expect it to be different between the original and converted type + got.TypeMeta = metav1.TypeMeta{} + patch.TypeMeta = metav1.TypeMeta{} + + // clear fields that we know belong in PatchOptions only + patch.Force = nil + + if !reflect.DeepEqual(*patch, *got) { + t.Fatalf(`round-trip failed: +got: %#+v +want: %#+v`, got, converted) + } + + }) + } + }) + } +} diff --git a/pkg/endpoints/handlers/update.go b/pkg/endpoints/handlers/update.go index 4117c0e16..f3aec87b3 100644 --- a/pkg/endpoints/handlers/update.go +++ b/pkg/endpoints/handlers/update.go @@ -87,6 +87,7 @@ func UpdateResource(r rest.Updater, scope *RequestScope, admit admission.Interfa scope.err(err, w, req) return } + options.TypeMeta.SetGroupVersionKind(metav1.SchemeGroupVersion.WithKind("UpdateOptions")) s, err := negotiation.NegotiateInputSerializer(req, false, scope.Serializer) if err != nil { @@ -138,11 +139,11 @@ func UpdateResource(r rest.Updater, scope *RequestScope, admit admission.Interfa return nil, fmt.Errorf("unexpected error when extracting UID from oldObj: %v", err.Error()) } else if !isNotZeroObject { if mutatingAdmission.Handles(admission.Create) { - return newObj, mutatingAdmission.Admit(admission.NewAttributesRecord(newObj, nil, scope.Kind, namespace, name, scope.Resource, scope.Subresource, admission.Create, dryrun.IsDryRun(options.DryRun), userInfo), scope) + return newObj, mutatingAdmission.Admit(admission.NewAttributesRecord(newObj, nil, scope.Kind, namespace, name, scope.Resource, scope.Subresource, admission.Create, updateToCreateOptions(options), dryrun.IsDryRun(options.DryRun), userInfo), scope) } } else { if mutatingAdmission.Handles(admission.Update) { - return newObj, mutatingAdmission.Admit(admission.NewAttributesRecord(newObj, oldObj, scope.Kind, namespace, name, scope.Resource, scope.Subresource, admission.Update, dryrun.IsDryRun(options.DryRun), userInfo), scope) + return newObj, mutatingAdmission.Admit(admission.NewAttributesRecord(newObj, oldObj, scope.Kind, namespace, name, scope.Resource, scope.Subresource, admission.Update, options, dryrun.IsDryRun(options.DryRun), userInfo), scope) } } return newObj, nil @@ -172,11 +173,11 @@ func UpdateResource(r rest.Updater, scope *RequestScope, admit admission.Interfa rest.DefaultUpdatedObjectInfo(obj, transformers...), withAuthorization(rest.AdmissionToValidateObjectFunc( admit, - admission.NewAttributesRecord(nil, nil, scope.Kind, namespace, name, scope.Resource, scope.Subresource, admission.Create, dryrun.IsDryRun(options.DryRun), userInfo), scope), + admission.NewAttributesRecord(nil, nil, scope.Kind, namespace, name, scope.Resource, scope.Subresource, admission.Create, updateToCreateOptions(options), dryrun.IsDryRun(options.DryRun), userInfo), scope), scope.Authorizer, createAuthorizerAttributes), rest.AdmissionToValidateObjectUpdateFunc( admit, - admission.NewAttributesRecord(nil, nil, scope.Kind, namespace, name, scope.Resource, scope.Subresource, admission.Update, dryrun.IsDryRun(options.DryRun), userInfo), scope), + admission.NewAttributesRecord(nil, nil, scope.Kind, namespace, name, scope.Resource, scope.Subresource, admission.Update, options, dryrun.IsDryRun(options.DryRun), userInfo), scope), false, options, ) @@ -229,3 +230,16 @@ func withAuthorization(validate rest.ValidateObjectFunc, a authorizer.Authorizer return errors.NewForbidden(gr, name, err) } } + +// updateToCreateOptions creates a CreateOptions with the same field values as the provided UpdateOptions. +func updateToCreateOptions(uo *metav1.UpdateOptions) *metav1.CreateOptions { + if uo == nil { + return nil + } + co := &metav1.CreateOptions{ + DryRun: uo.DryRun, + FieldManager: uo.FieldManager, + } + co.TypeMeta.SetGroupVersionKind(metav1.SchemeGroupVersion.WithKind("CreateOptions")) + return co +} diff --git a/pkg/registry/rest/create.go b/pkg/registry/rest/create.go index cc3c9ce4b..7750cb7bf 100644 --- a/pkg/registry/rest/create.go +++ b/pkg/registry/rest/create.go @@ -175,6 +175,7 @@ func AdmissionToValidateObjectFunc(admit admission.Interface, staticAttributes a staticAttributes.GetResource(), staticAttributes.GetSubresource(), staticAttributes.GetOperation(), + staticAttributes.GetOperationOptions(), staticAttributes.IsDryRun(), staticAttributes.GetUserInfo(), ) diff --git a/pkg/registry/rest/update.go b/pkg/registry/rest/update.go index d214e7e6a..21719b015 100644 --- a/pkg/registry/rest/update.go +++ b/pkg/registry/rest/update.go @@ -271,6 +271,7 @@ func AdmissionToValidateObjectUpdateFunc(admit admission.Interface, staticAttrib staticAttributes.GetResource(), staticAttributes.GetSubresource(), staticAttributes.GetOperation(), + staticAttributes.GetOperationOptions(), staticAttributes.IsDryRun(), staticAttributes.GetUserInfo(), )