Merge pull request #65572 from yue9944882/fixes-admission-operation-mismatch-for-create-on-update

Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

fixes operation for "create on update"

**What this PR does / why we need it**:

Set operation to `admission.Create` for create-on-update requests.

**Which issue(s) this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close the issue(s) when PR gets merged)*:
Fixes #65553

**Special notes for your reviewer**:

**Release note**:

```release-note
Checks CREATE admission for create-on-update requests instead of UPDATE admission
```

pkg/controller/resourcequota/resource_quota_monitor.go

@@ -223,7 +223,7 @@ func (qm *QuotaMonitor) SyncMonitors(resources map[schema.GroupVersionResource]s
 		if evaluator == nil {
 			listerFunc := generic.ListerFuncForResourceFunc(qm.informerFactory.ForResource)
 			listResourceFunc := generic.ListResourceUsingListerFunc(listerFunc, resource)
-			evaluator = generic.NewObjectCountEvaluator(false, resource.GroupResource(), listResourceFunc, "")
+			evaluator = generic.NewObjectCountEvaluator(resource.GroupResource(), listResourceFunc, "")
 			qm.registry.Add(evaluator)
 			glog.Infof("QuotaMonitor created object count evaluator for %s", resource.GroupResource())
 		}

pkg/quota/evaluator/core/registry.go

@@ -44,7 +44,7 @@ func NewEvaluators(f quota.ListerForResourceFunc) []quota.Evaluator {
 	// these evaluators require an alias for backwards compatibility
 	for gvr, alias := range legacyObjectCountAliases {
 		result = append(result,
-			generic.NewObjectCountEvaluator(false, gvr.GroupResource(), generic.ListResourceUsingListerFunc(f, gvr), alias))
+			generic.NewObjectCountEvaluator(gvr.GroupResource(), generic.ListResourceUsingListerFunc(f, gvr), alias))
 	}
 	return result
 }

pkg/quota/generic/evaluator.go

@@ -167,9 +167,6 @@ func CalculateUsageStats(options quota.UsageStatsOptions,
 // that associates usage of the specified resource based on the number of items
 // returned by the specified listing function.
 type objectCountEvaluator struct {
-	// allowCreateOnUpdate if true will ensure the evaluator tracks create
-	// and update operations.
-	allowCreateOnUpdate bool
 	// GroupResource that this evaluator tracks.
 	// It is used to construct a generic object count quota name
 	groupResource schema.GroupResource
@@ -189,7 +186,7 @@ func (o *objectCountEvaluator) Constraints(required []api.ResourceName, item run
 // Handles returns true if the object count evaluator needs to track this attributes.
 func (o *objectCountEvaluator) Handles(a admission.Attributes) bool {
 	operation := a.GetOperation()
-	return operation == admission.Create || (o.allowCreateOnUpdate && operation == admission.Update)
+	return operation == admission.Create
 }
 
 // Matches returns true if the evaluator matches the specified quota with the provided input item
@@ -241,7 +238,6 @@ var _ quota.Evaluator = &objectCountEvaluator{}
 // purposes for the legacy object counting names in quota.  Unless its supporting
 // backward compatibility, alias should not be used.
 func NewObjectCountEvaluator(
-	allowCreateOnUpdate bool,
 	groupResource schema.GroupResource, listFuncByNamespace ListFuncByNamespace,
 	alias api.ResourceName) quota.Evaluator {
 
@@ -251,7 +247,6 @@ func NewObjectCountEvaluator(
 	}
 
 	return &objectCountEvaluator{
-		allowCreateOnUpdate: allowCreateOnUpdate,
 		groupResource:       groupResource,
 		listFuncByNamespace: listFuncByNamespace,
 		resourceNames:       resourceNames,

plugin/pkg/admission/resourcequota/controller.go

@@ -592,7 +592,7 @@ func (e *quotaEvaluator) Evaluate(a admission.Attributes) error {
 	if evaluator == nil {
 		// create an object count evaluator if no evaluator previously registered
 		// note, we do not need aggregate usage here, so we pass a nil informer func
-		evaluator = generic.NewObjectCountEvaluator(false, gr, nil, "")
+		evaluator = generic.NewObjectCountEvaluator(gr, nil, "")
 		e.registry.Add(evaluator)
 		glog.Infof("quota admission added evaluator for: %s", gr)
 	}

staging/src/k8s.io/apiserver/pkg/endpoints/handlers/patch.go

@@ -127,6 +127,7 @@ func PatchResource(r rest.Patcher, scope RequestScope, admit admission.Interface
 			userInfo,
 		)
 		admissionCheck := func(updatedObject runtime.Object, currentObject runtime.Object) error {
+			// if we allow create-on-patch, we have this TODO: call the mutating admission chain with the CREATE verb instead of UPDATE
 			if mutatingAdmission, ok := admit.(admission.MutationInterface); ok && admit.Handles(admission.Update) {
 				return mutatingAdmission.Admit(admission.NewAttributesRecord(
 					updatedObject,

staging/src/k8s.io/apiserver/pkg/endpoints/handlers/update.go

@@ -105,12 +105,24 @@ func UpdateResource(r rest.Updater, scope RequestScope, admit admission.Interfac
 		}
 
 		userInfo, _ := request.UserFrom(ctx)
-		staticAdmissionAttributes := admission.NewAttributesRecord(nil, nil, scope.Kind, namespace, name, scope.Resource, scope.Subresource, admission.Update, userInfo)
 		var transformers []rest.TransformFunc
-		if mutatingAdmission, ok := admit.(admission.MutationInterface); ok && mutatingAdmission.Handles(admission.Update) {
+		if mutatingAdmission, ok := admit.(admission.MutationInterface); ok {
 			transformers = append(transformers, func(ctx context.Context, newObj, oldObj runtime.Object) (runtime.Object, error) {
-				return newObj, mutatingAdmission.Admit(admission.NewAttributesRecord(newObj, oldObj, scope.Kind, namespace, name, scope.Resource, scope.Subresource, admission.Update, userInfo))
+				isNotZeroObject, err := hasUID(oldObj)
+				if err != nil {
+					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, userInfo))
+					}
+				} else {
+					if mutatingAdmission.Handles(admission.Update) {
+						return newObj, mutatingAdmission.Admit(admission.NewAttributesRecord(newObj, oldObj, scope.Kind, namespace, name, scope.Resource, scope.Subresource, admission.Update, userInfo))
+					}
+				}
+				return newObj, nil
 			})
+
 		}
 
 		createAuthorizerAttributes := authorizer.AttributesRecord{
@@ -133,8 +145,13 @@ func UpdateResource(r rest.Updater, scope RequestScope, admit admission.Interfac
 				ctx,
 				name,
 				rest.DefaultUpdatedObjectInfo(obj, transformers...),
-				withAuthorization(rest.AdmissionToValidateObjectFunc(admit, staticAdmissionAttributes), scope.Authorizer, createAuthorizerAttributes),
+				withAuthorization(rest.AdmissionToValidateObjectFunc(
-				rest.AdmissionToValidateObjectUpdateFunc(admit, staticAdmissionAttributes),
+					admit,
+					admission.NewAttributesRecord(nil, nil, scope.Kind, namespace, name, scope.Resource, scope.Subresource, admission.Create, userInfo)),
+					scope.Authorizer, createAuthorizerAttributes),
+				rest.AdmissionToValidateObjectUpdateFunc(
+					admit,
+					admission.NewAttributesRecord(nil, nil, scope.Kind, namespace, name, scope.Resource, scope.Subresource, admission.Update, userInfo)),
 				false,
 				options,
 			)