From 140c8c73a64deb102b528109138ca9fb7dbb2392 Mon Sep 17 00:00:00 2001 From: Joe Betz Date: Tue, 7 May 2019 13:34:18 -0700 Subject: [PATCH] Pass {Operation}Option to Webhooks --- pkg/apis/admission/types.go | 10 ++- .../src/k8s.io/api/admission/v1beta1/types.go | 10 ++- .../pkg/apis/apiextensions/v1beta1/types.go | 2 +- .../apiserver/pkg/admission/attributes.go | 8 +- .../apiserver/pkg/admission/interfaces.go | 2 + .../plugin/webhook/request/admissionreview.go | 3 + .../apiserver/pkg/endpoints/handlers/BUILD | 1 + .../pkg/endpoints/handlers/create.go | 3 +- .../pkg/endpoints/handlers/delete.go | 8 +- .../apiserver/pkg/endpoints/handlers/patch.go | 51 +++++++---- .../apiserver/pkg/endpoints/handlers/rest.go | 4 +- .../pkg/endpoints/handlers/rest_test.go | 90 ++++++++++++++++++- .../pkg/endpoints/handlers/update.go | 22 ++++- .../apiserver/pkg/registry/rest/create.go | 1 + .../apiserver/pkg/registry/rest/update.go | 1 + 15 files changed, 186 insertions(+), 30 deletions(-) diff --git a/pkg/apis/admission/types.go b/pkg/apis/admission/types.go index fb704abdf9..f874013e39 100644 --- a/pkg/apis/admission/types.go +++ b/pkg/apis/admission/types.go @@ -63,7 +63,8 @@ type AdmissionRequest struct { // Namespace is the namespace associated with the request (if any). // +optional Namespace string - // Operation is the operation being performed + // Operation is the operation being performed. This may be different than the operation + // requested. e.g. a patch can result in either a CREATE or UPDATE Operation. Operation Operation // UserInfo is information about the requesting user UserInfo authentication.UserInfo @@ -78,6 +79,13 @@ type AdmissionRequest struct { // Defaults to false. // +optional DryRun *bool + // Options is the operation option structure of the operation being performed. + // e.g. `meta.k8s.io/v1.DeleteOptions` or `meta.k8s.io/v1.CreateOptions`. This may be + // different than the options the caller provided. e.g. for a patch request the performed + // Operation might be a CREATE, in which case the Options will a + // `meta.k8s.io/v1.CreateOptions` even though the caller provided `meta.k8s.io/v1.PatchOptions`. + // +optional + Options runtime.Object } // AdmissionResponse describes an admission response. diff --git a/staging/src/k8s.io/api/admission/v1beta1/types.go b/staging/src/k8s.io/api/admission/v1beta1/types.go index 653e847107..9d2884e66f 100644 --- a/staging/src/k8s.io/api/admission/v1beta1/types.go +++ b/staging/src/k8s.io/api/admission/v1beta1/types.go @@ -61,7 +61,8 @@ type AdmissionRequest struct { // Namespace is the namespace associated with the request (if any). // +optional Namespace string `json:"namespace,omitempty" protobuf:"bytes,6,opt,name=namespace"` - // Operation is the operation being performed + // Operation is the operation being performed. This may be different than the operation + // requested. e.g. a patch can result in either a CREATE or UPDATE Operation. Operation Operation `json:"operation" protobuf:"bytes,7,opt,name=operation"` // UserInfo is information about the requesting user UserInfo authenticationv1.UserInfo `json:"userInfo" protobuf:"bytes,8,opt,name=userInfo"` @@ -75,6 +76,13 @@ type AdmissionRequest struct { // Defaults to false. // +optional DryRun *bool `json:"dryRun,omitempty" protobuf:"varint,11,opt,name=dryRun"` + // Options is the operation option structure of the operation being performed. + // e.g. `meta.k8s.io/v1.DeleteOptions` or `meta.k8s.io/v1.CreateOptions`. This may be + // different than the options the caller provided. e.g. for a patch request the performed + // Operation might be a CREATE, in which case the Options will a + // `meta.k8s.io/v1.CreateOptions` even though the caller provided `meta.k8s.io/v1.PatchOptions`. + // +optional + Options runtime.RawExtension `json:"options,omitempty" protobuf:"bytes,12,opt,name=options"` } // AdmissionResponse describes an admission response. diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1/types.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1/types.go index 220a494bce..0ca6673bf4 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1/types.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1/types.go @@ -443,7 +443,7 @@ type ConversionRequest struct { // ConversionResponse describes a conversion response. type ConversionResponse struct { // `uid` is an identifier for the individual request/response. - // This should be copied over from the corresponding AdmissionRequest. + // This should be copied over from the corresponding ConversionRequest. UID types.UID `json:"uid" protobuf:"bytes,1,name=uid"` // `convertedObjects` is the list of converted version of `request.objects` if the `result` is successful otherwise empty. // The webhook is expected to set apiVersion of these objects to the ConversionRequest.desiredAPIVersion. The list diff --git a/staging/src/k8s.io/apiserver/pkg/admission/attributes.go b/staging/src/k8s.io/apiserver/pkg/admission/attributes.go index c8973cc629..ad6ca6ba6f 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/attributes.go +++ b/staging/src/k8s.io/apiserver/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/staging/src/k8s.io/apiserver/pkg/admission/interfaces.go b/staging/src/k8s.io/apiserver/pkg/admission/interfaces.go index 866777cc70..a8853fdd73 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/interfaces.go +++ b/staging/src/k8s.io/apiserver/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/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/request/admissionreview.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/request/admissionreview.go index cec41315c2..51a45a36a7 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/request/admissionreview.go +++ b/staging/src/k8s.io/apiserver/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/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/BUILD b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/BUILD index 85695ff863..87846313d4 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/BUILD +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/BUILD @@ -27,6 +27,7 @@ go_test( "//staging/src/k8s.io/apimachinery/pkg/runtime/serializer:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/types:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/util/diff:go_default_library", + "//staging/src/k8s.io/apimachinery/pkg/util/json:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/util/runtime:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/util/strategicpatch:go_default_library", "//staging/src/k8s.io/apiserver/pkg/admission:go_default_library", diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/create.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/create.go index 40c53efae3..d4a31b7e7e 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/create.go +++ b/staging/src/k8s.io/apiserver/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/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/delete.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/delete.go index 8f1d1d2179..7375c575eb 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/delete.go +++ b/staging/src/k8s.io/apiserver/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/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/patch.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/patch.go index 23644bd340..c33f181661 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/patch.go +++ b/staging/src/k8s.io/apiserver/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/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/rest.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/rest.go index 4db0c06762..cc82a8df8d 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/rest.go +++ b/staging/src/k8s.io/apiserver/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/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/rest_test.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/rest_test.go index 2c9dab5b94..4c33f05e79 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/rest_test.go +++ b/staging/src/k8s.io/apiserver/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/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/update.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/update.go index 4117c0e168..f3aec87b39 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/update.go +++ b/staging/src/k8s.io/apiserver/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/staging/src/k8s.io/apiserver/pkg/registry/rest/create.go b/staging/src/k8s.io/apiserver/pkg/registry/rest/create.go index cc3c9ce4bc..7750cb7bfd 100644 --- a/staging/src/k8s.io/apiserver/pkg/registry/rest/create.go +++ b/staging/src/k8s.io/apiserver/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/staging/src/k8s.io/apiserver/pkg/registry/rest/update.go b/staging/src/k8s.io/apiserver/pkg/registry/rest/update.go index d214e7e6a8..21719b015e 100644 --- a/staging/src/k8s.io/apiserver/pkg/registry/rest/update.go +++ b/staging/src/k8s.io/apiserver/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(), )