Merge pull request #4545 from RaphaelFakhri/fix-bind-accept-all-identity-hash

Keep identity hash when binding with --accept-all-permission-claims

Author: kcp-ci-bot

staging/src/github.com/kcp-dev/cli/pkg/bind/plugin/bind.go

@@ -213,35 +213,7 @@ func (b *BindOptions) Run(ctx context.Context) error {
 			return fmt.Errorf("failed to get APIExport %q: %w", apiExportName, err)
 		}
 
-		// Iterate over all permission claims defined in the APIExport
-		for _, claim := range apiExport.Spec.PermissionClaims {
-			// Build an AcceptablePermissionClaim using the SDK structs
-			parsedClaim := apisv1alpha2.AcceptablePermissionClaim{
-				ScopedPermissionClaim: apisv1alpha2.ScopedPermissionClaim{
-					PermissionClaim: apisv1alpha2.PermissionClaim{
-						GroupResource: apisv1alpha2.GroupResource{
-							Group:    claim.Group,
-							Resource: claim.Resource,
-						},
-						Verbs: []string{"*"},
-					},
-					Selector: apisv1alpha2.PermissionClaimSelector{MatchAll: true},
-				},
-				State: apisv1alpha2.ClaimAccepted, // default; may be overridden following
-			}
-
-			// Add to accepted claims if user requested accept-all
-			if b.AcceptAllPermissionClaims {
-				parsedClaim.State = apisv1alpha2.ClaimAccepted
-				b.acceptedPermissionClaims = append(b.acceptedPermissionClaims, parsedClaim)
-			}
-
-			// Add to rejected claims if user requested reject-all
-			if b.RejectAllPermissionClaims {
-				parsedClaim.State = apisv1alpha2.ClaimRejected
-				b.rejectedPermissionClaims = append(b.rejectedPermissionClaims, parsedClaim)
-			}
-		}
+		b.addAllPermissionClaims(apiExport.Spec.PermissionClaims)
 	}
 
 	apiBinding, err := b.newAPIBinding(preferredAPIBindingVersion)
@@ -277,6 +249,40 @@ func (b *BindOptions) Run(ctx context.Context) error {
 	return nil
 }
 
+// addAllPermissionClaims accepts or rejects every given APIExport permission claim,
+// depending on whether --accept-all-permission-claims or --reject-all-permission-claims is set.
+func (b *BindOptions) addAllPermissionClaims(claims []apisv1alpha1.PermissionClaim) {
+	for _, claim := range claims {
+		// Build an AcceptablePermissionClaim using the SDK structs
+		parsedClaim := apisv1alpha2.AcceptablePermissionClaim{
+			ScopedPermissionClaim: apisv1alpha2.ScopedPermissionClaim{
+				PermissionClaim: apisv1alpha2.PermissionClaim{
+					GroupResource: apisv1alpha2.GroupResource{
+						Group:    claim.Group,
+						Resource: claim.Resource,
+					},
+					Verbs:        []string{"*"},
+					IdentityHash: claim.IdentityHash,
+				},
+				Selector: apisv1alpha2.PermissionClaimSelector{MatchAll: true},
+			},
+			State: apisv1alpha2.ClaimAccepted, // default; may be overridden following
+		}
+
+		// Add to accepted claims if user requested accept-all
+		if b.AcceptAllPermissionClaims {
+			parsedClaim.State = apisv1alpha2.ClaimAccepted
+			b.acceptedPermissionClaims = append(b.acceptedPermissionClaims, parsedClaim)
+		}
+
+		// Add to rejected claims if user requested reject-all
+		if b.RejectAllPermissionClaims {
+			parsedClaim.State = apisv1alpha2.ClaimRejected
+			b.rejectedPermissionClaims = append(b.rejectedPermissionClaims, parsedClaim)
+		}
+	}
+}
+
 func (b *BindOptions) newAPIBinding(preferredAPIBindingVersion string) (apishelpers.APIBinding, error) {
 	path, apiExportName := logicalcluster.NewPath(b.APIExportRef).Split()
 

staging/src/github.com/kcp-dev/cli/pkg/bind/plugin/bind_test.go

@@ -19,6 +19,7 @@ package plugin
 import (
 	"testing"
 
+	apisv1alpha1 "github.com/kcp-dev/sdk/apis/apis/v1alpha1"
 	apisv1alpha2 "github.com/kcp-dev/sdk/apis/apis/v1alpha2"
 )
 
@@ -262,3 +263,56 @@ func TestBindOptionsAcceptRejectAll(t *testing.T) {
 		}
 	})
 }
+
+// TestAddAllPermissionClaims verifies that accept-all and reject-all copy the
+// group, resource and identity hash of every APIExport permission claim.
+func TestAddAllPermissionClaims(t *testing.T) {
+	t.Parallel()
+
+	exportClaims := []apisv1alpha1.PermissionClaim{
+		{GroupResource: apisv1alpha1.GroupResource{Group: "", Resource: "configmaps"}},
+		{GroupResource: apisv1alpha1.GroupResource{Group: "example.kcp.io", Resource: "widgets"}, IdentityHash: "abc123"},
+	}
+
+	testCases := []struct {
+		description string
+		acceptAll   bool
+		wantState   apisv1alpha2.AcceptablePermissionClaimState
+	}{
+		{description: "accept-all", acceptAll: true, wantState: apisv1alpha2.ClaimAccepted},
+		{description: "reject-all", acceptAll: false, wantState: apisv1alpha2.ClaimRejected},
+	}
+
+	for _, c := range testCases {
+		t.Run(c.description, func(t *testing.T) {
+			t.Parallel()
+			b := &BindOptions{
+				AcceptAllPermissionClaims: c.acceptAll,
+				RejectAllPermissionClaims: !c.acceptAll,
+			}
+			b.addAllPermissionClaims(exportClaims)
+
+			got := b.rejectedPermissionClaims
+			other := b.acceptedPermissionClaims
+			if c.acceptAll {
+				got, other = other, got
+			}
+			if len(other) != 0 {
+				t.Errorf("expected no claims in the opposite list, got %d", len(other))
+			}
+			if len(got) != len(exportClaims) {
+				t.Fatalf("expected %d claims, got %d", len(exportClaims), len(got))
+			}
+			for i, claim := range got {
+				want := exportClaims[i]
+				if claim.Group != want.Group || claim.Resource != want.Resource || claim.IdentityHash != want.IdentityHash {
+					t.Errorf("claim %d: expected %s.%s with identity hash %q, got %s.%s with identity hash %q",
+						i, want.Resource, want.Group, want.IdentityHash, claim.Resource, claim.Group, claim.IdentityHash)
+				}
+				if claim.State != c.wantState {
+					t.Errorf("claim %d: expected state %s, got %s", i, c.wantState, claim.State)
+				}
+			}
+		})
+	}
+}