Prototype fix for PATCH without tier prefix

apiserver/pkg/registry/projectcalico/globalpolicy/storage.go

@@ -128,7 +128,8 @@ func (r *REST) Create(ctx context.Context, obj runtime.Object, val rest.Validate
 }
 
 func (r *REST) Update(ctx context.Context, name string, objInfo rest.UpdatedObjectInfo, createValidation rest.ValidateObjectFunc,
-	updateValidation rest.ValidateObjectUpdateFunc, forceAllowCreate bool, options *metav1.UpdateOptions) (runtime.Object, bool, error) {
+	updateValidation rest.ValidateObjectUpdateFunc, forceAllowCreate bool, options *metav1.UpdateOptions,
+) (runtime.Object, bool, error) {
 	tierName, _ := util.GetTierFromPolicyName(name)
 	err := r.authorizer.AuthorizeTierOperation(ctx, name, tierName)
 	if err != nil {
@@ -145,7 +146,6 @@ func (r *REST) Get(ctx context.Context, name string, options *metav1.GetOptions)
 	if err != nil {
 		return nil, err
 	}
-
 	return r.Store.Get(ctx, name, options)
 }
 

apiserver/pkg/registry/projectcalico/networkpolicy/strategy.go

@@ -47,7 +47,10 @@ func (policyStrategy) PrepareForCreate(ctx context.Context, obj runtime.Object)
 }
 
 func (policyStrategy) PrepareForUpdate(ctx context.Context, obj, old runtime.Object) {
-	obj.(*calico.NetworkPolicy).Name = canonicalizePolicyName(old)
+	// Canonicalize the old object's name as well to make sure it's consistent.
+	// This typically shouldn't be needed for update requests, but is necessary in PATCH
+	// requests because we may need to strip the tier earlier in the pipeline to pass earlier validation.
+	old.(*calico.NetworkPolicy).Name = canonicalizePolicyName(old)
 }
 
 func canonicalizePolicyName(obj runtime.Object) string {

apiserver/pkg/storage/calico/resource.go

@@ -9,6 +9,7 @@ import (
 	"os"
 	"reflect"
 	"strconv"
+	"strings"
 	"time"
 
 	aapierrors "k8s.io/apimachinery/pkg/api/errors"
@@ -22,6 +23,7 @@ import (
 	"k8s.io/apiserver/pkg/storage"
 	"k8s.io/klog/v2"
 
+	v3 "github.com/projectcalico/api/pkg/apis/projectcalico/v3"
 	"github.com/projectcalico/calico/libcalico-go/lib/apiconfig"
 	"github.com/projectcalico/calico/libcalico-go/lib/clientv3"
 	"github.com/projectcalico/calico/libcalico-go/lib/errors"
@@ -417,11 +419,27 @@ func (rs *resourceStore) GuaranteedUpdate(
 			klog.Errorf("checking preconditions (%s)", err)
 			return err
 		}
+
 		// update the object by applying the userUpdate func & encode it
 		updatedObj, ttl, err := userUpdate(curState.obj, *curState.meta)
 		if err != nil {
-			klog.Errorf("applying user update: (%s)", err)
-			return err
+			// Try updating the object name to remove the tier - it's possible that the user send the
+			// object without the tier in the name, in which case we need to retry the update against the
+			// object without the tier in the name.
+			switch curState.obj.(type) {
+			case *v3.GlobalNetworkPolicy:
+				tier := curState.obj.(*v3.GlobalNetworkPolicy).Spec.Tier
+				curState.obj.(*v3.GlobalNetworkPolicy).Name = strings.TrimPrefix(curState.obj.(*v3.GlobalNetworkPolicy).Name, tier+".")
+				updatedObj, ttl, err = userUpdate(curState.obj, *curState.meta)
+			case *v3.NetworkPolicy:
+				tier := curState.obj.(*v3.NetworkPolicy).Spec.Tier
+				curState.obj.(*v3.NetworkPolicy).Name = strings.TrimPrefix(curState.obj.(*v3.NetworkPolicy).Name, tier+".")
+				updatedObj, ttl, err = userUpdate(curState.obj, *curState.meta)
+			}
+			if err != nil {
+				klog.Errorf("applying user update: (%s)", err)
+				return err
+			}
 		}
 
 		updatedData, err := runtime.Encode(rs.codec, updatedObj)

apiserver/test/integration/clientset_test.go

@@ -33,6 +33,7 @@ import (
 	"github.com/projectcalico/api/pkg/lib/numorstring"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/apimachinery/pkg/watch"
 
 	"github.com/projectcalico/calico/libcalico-go/lib/apiconfig"
@@ -199,6 +200,16 @@ func testNetworkPolicyClient(client calicoclient.Interface, name string) error {
 		return fmt.Errorf("policy name prefix wasn't defaulted by the apiserver on update: %v", policyServer)
 	}
 
+	// Patch the policy. We should be able to use the same name that we used to create it (i.e., without the "default" prefix).
+	patch := []byte(`{"metadata": {"labels": {"foo": "baz"}}}`)
+	policyServer, err = policyClient.Patch(ctx, name, types.MergePatchType, patch, metav1.PatchOptions{})
+	if err != nil {
+		return fmt.Errorf("error patching the policy without the tier prefix '%v' (%v)", policyServer, err)
+	}
+	if defaultTierPolicyName != policyServer.Name {
+		return fmt.Errorf("policy name prefix wasn't defaulted by the apiserver on patch: %v", policyServer)
+	}
+
 	// Delete that policy. We should be able to use the same name that we used to create it (i.e., without the "default" prefix).
 	err = policyClient.Delete(ctx, name, metav1.DeleteOptions{})
 	if err != nil {
@@ -224,6 +235,13 @@ func testNetworkPolicyClient(client calicoclient.Interface, name string) error {
 		return fmt.Errorf("didn't get the same policy back from the server \n%+v\n%+v", policy, policyServer)
 	}
 
+	// Patch the policy with the tiered prefix.
+	patch = []byte(`{"metadata": {"labels": {"foo": "baz"}}}`)
+	policyServer, err = policyClient.Patch(ctx, defaultTierPolicyName, types.MergePatchType, patch, metav1.PatchOptions{})
+	if err != nil {
+		return fmt.Errorf("error patching the policy with the tier prefix '%v' (%v)", policyServer, err)
+	}
+
 	// For testing out Tiered Policy
 	tierClient := client.ProjectcalicoV3().Tiers()
 	order := float64(100.0)