Merge pull request #2175 from openshift-cherrypick-robot/cherry-pick-…

…2174-to-release-0.9

[release-0.9] 🐛 Fix permissionclaim patch thrashing

config/crds/apis.kcp.dev_apibindings.yaml

@@ -53,6 +53,7 @@ spec:
                     records if the user accepts or rejects it.
                   properties:
                     group:
+                      default: ""
                       description: group is the name of an API group. For core groups
                         this is the empty string '""'.
                       pattern: ^(|[a-z0-9]([-a-z0-9]*[a-z0-9](\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*)?)$
@@ -125,6 +126,7 @@ spec:
                     allow the service provider access to.
                   properties:
                     group:
+                      default: ""
                       description: group is the name of an API group. For core groups
                         this is the empty string '""'.
                       pattern: ^(|[a-z0-9]([-a-z0-9]*[a-z0-9](\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*)?)$
@@ -146,6 +148,10 @@ spec:
                   - resource
                   type: object
                 type: array
+                x-kubernetes-list-map-keys:
+                - group
+                - resource
+                x-kubernetes-list-type: map
               boundExport:
                 description: "boundExport records the export this binding is bound
                   to currently. It can differ from the export that was specified in
@@ -291,6 +297,7 @@ spec:
                     allow the service provider access to.
                   properties:
                     group:
+                      default: ""
                       description: group is the name of an API group. For core groups
                         this is the empty string '""'.
                       pattern: ^(|[a-z0-9]([-a-z0-9]*[a-z0-9](\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*)?)$
@@ -312,6 +319,10 @@ spec:
                   - resource
                   type: object
                 type: array
+                x-kubernetes-list-map-keys:
+                - group
+                - resource
+                x-kubernetes-list-type: map
               phase:
                 description: 'phase is the current phase of the APIBinding: - "":
                   the APIBinding has just been created, waiting to be bound. - Binding:

config/crds/workload.kcp.dev_synctargets.yaml

@@ -195,6 +195,7 @@ spec:
                 items:
                   properties:
                     group:
+                      default: ""
                       description: group is the name of an API group. For core groups
                         this is the empty string '""'.
                       pattern: ^(|[a-z0-9]([-a-z0-9]*[a-z0-9](\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*)?)$

config/root-phase0/apiexport-workload.kcp.dev.yaml

@@ -5,5 +5,5 @@ metadata:
   name: workload.kcp.dev
 spec:
   latestResourceSchemas:
-  - v220923-836dfac8.synctargets.workload.kcp.dev
+  - v221011-1af9d713.synctargets.workload.kcp.dev
 status: {}

config/root-phase0/apiresourceschema-synctargets.workload.kcp.dev.yaml

@@ -2,7 +2,7 @@ apiVersion: apis.kcp.dev/v1alpha1
 kind: APIResourceSchema
 metadata:
   creationTimestamp: null
-  name: v220923-836dfac8.synctargets.workload.kcp.dev
+  name: v221011-1af9d713.synctargets.workload.kcp.dev
 spec:
   group: workload.kcp.dev
   names:
@@ -189,6 +189,7 @@ spec:
               items:
                 properties:
                   group:
+                    default: ""
                     description: group is the name of an API group. For core groups
                       this is the empty string '""'.
                     pattern: ^(|[a-z0-9]([-a-z0-9]*[a-z0-9](\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*)?)$

pkg/apis/apis/v1alpha1/types_apibinding.go

@@ -174,11 +174,17 @@ type APIBindingStatus struct {
 	// state in spec.permissionClaims.
 	//
 	// +optional
+	// +listType=map
+	// +listMapKey=group
+	// +listMapKey=resource
 	AppliedPermissionClaims []PermissionClaim `json:"appliedPermissionClaims,omitempty"`
 
 	// exportPermissionClaims records the permissions that the export provider is asking for
 	// the binding to grant.
 	// +optional
+	// +listType=map
+	// +listMapKey=group
+	// +listMapKey=resource
 	ExportPermissionClaims []PermissionClaim `json:"exportPermissionClaims,omitempty"`
 }
 

pkg/apis/apis/v1alpha1/types_apiexport.go

@@ -220,6 +220,7 @@ type GroupResource struct {
 	//
 	// +kubebuilder:validation:Pattern=`^(|[a-z0-9]([-a-z0-9]*[a-z0-9](\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*)?)$`
 	// +optional
+	// +kubebuilder:default=""
 	Group string `json:"group,omitempty"`
 
 	// resource is the name of the resource.

pkg/reconciler/apis/permissionclaimlabel/permissionclaimlabel_reconcile.go

@@ -132,6 +132,14 @@ func (c *controller) reconcile(ctx context.Context, apiBinding *apisv1alpha1.API
 			}
 
 			logger := logging.WithObject(logger, u)
+
+			if gvr == apisv1alpha1.SchemeGroupVersion.WithResource("apibindings") && logicalcluster.From(u) == clusterName && u.GetName() == apiBinding.Name {
+				// Don't issue a generic patch when obj == the APIBinding being reconciled. That will be covered when
+				// this call to reconcile exits and the controller patches this APIBinding. Otherwise, the generic patch
+				// here will conflict with the controller's attempt to update the APIBinding's status.
+				continue
+			}
+
 			logger.V(4).Info("patching to get claim labels updated")
 
 			// Empty patch, allowing the admission plugin to update the resource to the correct labels
@@ -180,7 +188,7 @@ func (c *controller) reconcile(ctx context.Context, apiBinding *apisv1alpha1.API
 
 	fullyApplied := expectedClaims.Difference(applyErrors)
 	apiBinding.Status.AppliedPermissionClaims = []apisv1alpha1.PermissionClaim{}
-	for _, s := range fullyApplied.UnsortedList() {
+	for _, s := range fullyApplied.List() {
 		claim := claimFromSetKey(s)
 		apiBinding.Status.AppliedPermissionClaims = append(apiBinding.Status.AppliedPermissionClaims, claim)
 	}