Skip to content

Commit e86d128

Browse files
xmudriixmudrii
committed
Allow using label selectors in PermissionClaims
On-behalf-of: @SAP [email protected] Signed-off-by: Marko Mudrinić <[email protected]>
1 parent 059a31b commit e86d128

File tree

5 files changed

+207
-114
lines changed

5 files changed

+207
-114
lines changed

‎config/crds/apis.kcp.io_apibindings.yaml‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -557,9 +557,6 @@ spec:
557557
x-kubernetes-validations:
558558
- message: Permission claim selector is immutable
559559
rule: self == oldSelf
560-
- message: a selector is required. Only "matchAll" is currently
561-
implemented
562-
rule: (has(self.matchAll) && self.matchAll)
563560
state:
564561
enum:
565562
- Accepted
@@ -710,9 +707,6 @@ spec:
710707
x-kubernetes-validations:
711708
- message: Permission claim selector is immutable
712709
rule: self == oldSelf
713-
- message: a selector is required. Only "matchAll" is currently
714-
implemented
715-
rule: (has(self.matchAll) && self.matchAll)
716710
verbs:
717711
description: |-
718712
verbs is a list of supported API operation types (this includes

‎sdk/apis/apis/v1alpha2/types_apibinding.go‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -117,8 +117,6 @@ const (
117117

118118
// PermissionClaimSelector configures scoped access to objects
119119
// of a claimed resource.
120-
//
121-
// +kubebuilder:validation:XValidation:rule="(has(self.matchAll) && self.matchAll)",message="a selector is required. Only \"matchAll\" is currently implemented"
122120
type PermissionClaimSelector struct {
123121
metav1.LabelSelector `json:",inline"`
124122

‎sdk/apis/apis/v1alpha2/types_apibinding_test.go‎

Lines changed: 0 additions & 106 deletions
Original file line numberDiff line numberDiff line change
@@ -105,109 +105,3 @@ func TestAPIBindingAPIExportReferenceCELValidation(t *testing.T) {
105105
})
106106
}
107107
}
108-
109-
// TestAPIBindingAPIExportReferenceCELValidation will validate the APIExport reference for an otherwise valid APIBinding.
110-
func TestAPIBindingPermissionClaimSelectorCELValidation(t *testing.T) {
111-
testCases := []struct {
112-
name string
113-
current, old map[string]any
114-
wantErrs []string
115-
}{
116-
{
117-
name: "no change",
118-
current: map[string]any{
119-
"matchAll": true,
120-
},
121-
old: map[string]any{
122-
"matchAll": true,
123-
},
124-
},
125-
{
126-
name: "nothing set",
127-
current: map[string]any{},
128-
old: map[string]any{},
129-
wantErrs: []string{
130-
"openAPIV3Schema.properties.spec.properties.permissionClaims.items.properties.selector: Invalid value: \"object\": a selector is required. Only \"matchAll\" is currently implemented",
131-
},
132-
},
133-
{
134-
name: "using matchLabels",
135-
current: map[string]any{
136-
"matchLabels": map[string]any{
137-
"kcp.io/fake": "test",
138-
},
139-
},
140-
old: map[string]any{
141-
"matchLabels": map[string]any{
142-
"kcp.io/fake": "test",
143-
},
144-
},
145-
wantErrs: []string{
146-
"openAPIV3Schema.properties.spec.properties.permissionClaims.items.properties.selector: Invalid value: \"object\": a selector is required. Only \"matchAll\" is currently implemented",
147-
},
148-
},
149-
/*
150-
// TODO(embik): for future updates to unlock other matchers
151-
{
152-
name: "change from matchAll to matchLabels",
153-
current: map[string]any{
154-
"matchLabels": map[string]any{
155-
"kcp.io/fake": "test",
156-
},
157-
},
158-
old: map[string]any{
159-
"matchAll": true,
160-
},
161-
wantErrs: []string{
162-
"openAPIV3Schema.properties.spec.properties.permissionClaims.items.properties.selector: Invalid value: \"object\": Permission claim selector must not be changed",
163-
},
164-
},
165-
{
166-
name: "multiple matchers configured",
167-
current: map[string]any{
168-
"matchLabels": map[string]any{
169-
"kcp.io/fake": "test",
170-
},
171-
"matchAll": true,
172-
},
173-
old: map[string]any{
174-
"matchLabels": map[string]any{
175-
"kcp.io/fake": "test",
176-
},
177-
"matchAll": true,
178-
},
179-
wantErrs: []string{
180-
"openAPIV3Schema.properties.spec.properties.permissionClaims.items.properties.selector: Invalid value: \"object\": either \"matchAll\", \"matchLabels\" or \"matchExpressions\" must be set",
181-
},
182-
},
183-
*/
184-
}
185-
186-
validators := apitest.FieldValidatorsFromFile(t, "../../../../config/crds/apis.kcp.io_apibindings.yaml")
187-
188-
for _, tc := range testCases {
189-
pth := "openAPIV3Schema.properties.spec.properties.permissionClaims.items.properties.selector"
190-
validator, found := validators["v1alpha2"][pth]
191-
require.True(t, found, "failed to find validator for %s", pth)
192-
193-
t.Run(tc.name, func(t *testing.T) {
194-
errs := validator(tc.current, tc.old)
195-
if len(errs) > 0 {
196-
for _, err := range errs {
197-
t.Logf("'%s': observed validation error: %s", tc.name, err)
198-
}
199-
}
200-
201-
if got := len(errs); got != len(tc.wantErrs) {
202-
t.Fatalf("'%s': expected %d errors, got %d", tc.name, len(tc.wantErrs), len(errs))
203-
}
204-
205-
for i := range tc.wantErrs {
206-
got := errs[i].Error()
207-
if got != tc.wantErrs[i] {
208-
t.Errorf("'%s': want error %q, got %q", tc.name, tc.wantErrs[i], got)
209-
}
210-
}
211-
})
212-
}
213-
}

‎sdk/apis/apis/v1alpha2/validation.go‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ func ValidateAPIBinding(apiBinding *APIBinding) field.ErrorList {
2727
allErrs := field.ErrorList{}
2828

2929
allErrs = append(allErrs, ValidateAPIBindingReference(apiBinding.Spec.Reference, field.NewPath("spec", "reference"))...)
30+
allErrs = append(allErrs, ValidateAPIBindingPermissionClaims(apiBinding.Spec.PermissionClaims, field.NewPath("spec", "permissionClaims"))...)
3031

3132
return allErrs
3233
}
@@ -61,3 +62,25 @@ func ValidateAPIBindingReference(reference BindingReference, path *field.Path) f
6162

6263
return allErrs
6364
}
65+
66+
// ValidateAPIBindingPermissionClaims validates an APIBinding's PermissionClaims.
67+
func ValidateAPIBindingPermissionClaims(permissionClaims []AcceptablePermissionClaim, path *field.Path) field.ErrorList {
68+
allErrs := field.ErrorList{}
69+
70+
for i := range permissionClaims {
71+
claimPath := path.Index(i)
72+
73+
if permissionClaims[i].Selector.MatchAll {
74+
if len(permissionClaims[i].Selector.MatchLabels) > 0 {
75+
allErrs = append(allErrs, field.Invalid(claimPath.Child("selector").Child("matchLabels"), permissionClaims[i].Selector, "matchLabels cannot be used with matchAll"))
76+
}
77+
if len(permissionClaims[i].Selector.MatchExpressions) > 0 {
78+
allErrs = append(allErrs, field.Invalid(claimPath.Child("selector").Child("matchExpressions"), permissionClaims[i].Selector, "matchExpressions cannot be used with matchAll"))
79+
}
80+
} else if len(permissionClaims[i].Selector.MatchLabels) == 0 && len(permissionClaims[i].Selector.MatchExpressions) == 0 {
81+
allErrs = append(allErrs, field.Required(claimPath.Child("selector"), "either one of matchAll, matchLabels, or matchExpressions must be set"))
82+
}
83+
}
84+
85+
return allErrs
86+
}

‎sdk/apis/apis/v1alpha2/validation_test.go‎

Lines changed: 184 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,184 @@
1+
/*
2+
Copyright 2025 The KCP Authors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package v1alpha2
18+
19+
import (
20+
"slices"
21+
"testing"
22+
23+
"k8s.io/apimachinery/pkg/api/equality"
24+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
25+
"k8s.io/apimachinery/pkg/util/validation/field"
26+
)
27+
28+
func TestValidateAPIBindingPermissionClaims(t *testing.T) {
29+
tests := map[string]struct {
30+
permissionClaims []AcceptablePermissionClaim
31+
wantErrs []string
32+
}{
33+
"matchAll": {
34+
permissionClaims: []AcceptablePermissionClaim{
35+
{
36+
ScopedPermissionClaim: ScopedPermissionClaim{
37+
Selector: PermissionClaimSelector{
38+
MatchAll: true,
39+
},
40+
},
41+
},
42+
},
43+
wantErrs: nil,
44+
},
45+
"matchLabels": {
46+
permissionClaims: []AcceptablePermissionClaim{
47+
{
48+
ScopedPermissionClaim: ScopedPermissionClaim{
49+
Selector: PermissionClaimSelector{
50+
LabelSelector: metav1.LabelSelector{
51+
MatchLabels: map[string]string{
52+
"test": "test",
53+
},
54+
},
55+
},
56+
},
57+
},
58+
},
59+
wantErrs: nil,
60+
},
61+
"matchExpressions": {
62+
permissionClaims: []AcceptablePermissionClaim{
63+
{
64+
ScopedPermissionClaim: ScopedPermissionClaim{
65+
Selector: PermissionClaimSelector{
66+
LabelSelector: metav1.LabelSelector{
67+
MatchExpressions: []metav1.LabelSelectorRequirement{
68+
{
69+
Key: "test",
70+
Operator: metav1.LabelSelectorOpIn,
71+
Values: []string{"test"},
72+
},
73+
},
74+
},
75+
},
76+
},
77+
},
78+
},
79+
wantErrs: nil,
80+
},
81+
"none": {
82+
permissionClaims: []AcceptablePermissionClaim{
83+
{
84+
ScopedPermissionClaim: ScopedPermissionClaim{
85+
Selector: PermissionClaimSelector{},
86+
},
87+
},
88+
},
89+
wantErrs: []string{"spec.permissionClaims[0].selector: Required value: either one of matchAll, matchLabels, or matchExpressions must be set"},
90+
},
91+
"empty": {
92+
permissionClaims: []AcceptablePermissionClaim{
93+
{
94+
ScopedPermissionClaim: ScopedPermissionClaim{
95+
Selector: PermissionClaimSelector{
96+
MatchAll: false,
97+
LabelSelector: metav1.LabelSelector{
98+
MatchLabels: map[string]string{},
99+
MatchExpressions: []metav1.LabelSelectorRequirement{},
100+
},
101+
},
102+
},
103+
},
104+
},
105+
wantErrs: []string{"spec.permissionClaims[0].selector: Required value: either one of matchAll, matchLabels, or matchExpressions must be set"},
106+
},
107+
"matchAll+matchLabels+matchExpressions": {
108+
permissionClaims: []AcceptablePermissionClaim{
109+
{
110+
ScopedPermissionClaim: ScopedPermissionClaim{
111+
Selector: PermissionClaimSelector{
112+
MatchAll: true,
113+
LabelSelector: metav1.LabelSelector{
114+
MatchLabels: map[string]string{
115+
"test": "test",
116+
},
117+
},
118+
},
119+
},
120+
},
121+
{
122+
ScopedPermissionClaim: ScopedPermissionClaim{
123+
Selector: PermissionClaimSelector{
124+
MatchAll: true,
125+
LabelSelector: metav1.LabelSelector{
126+
MatchExpressions: []metav1.LabelSelectorRequirement{
127+
{
128+
Key: "test",
129+
Operator: metav1.LabelSelectorOpIn,
130+
Values: []string{"test"},
131+
},
132+
},
133+
},
134+
},
135+
},
136+
},
137+
{
138+
ScopedPermissionClaim: ScopedPermissionClaim{
139+
Selector: PermissionClaimSelector{
140+
MatchAll: true,
141+
LabelSelector: metav1.LabelSelector{
142+
MatchLabels: map[string]string{
143+
"test": "test",
144+
},
145+
MatchExpressions: []metav1.LabelSelectorRequirement{
146+
{
147+
Key: "test",
148+
Operator: metav1.LabelSelectorOpIn,
149+
Values: []string{"test"},
150+
},
151+
},
152+
},
153+
},
154+
},
155+
},
156+
},
157+
wantErrs: []string{
158+
"spec.permissionClaims[0].selector.matchLabels: Invalid value: v1alpha2.PermissionClaimSelector{LabelSelector:v1.LabelSelector{MatchLabels:map[string]string{\"test\":\"test\"}, MatchExpressions:[]v1.LabelSelectorRequirement(nil)}, MatchAll:true}: matchLabels cannot be used with matchAll",
159+
"spec.permissionClaims[1].selector.matchExpressions: Invalid value: v1alpha2.PermissionClaimSelector{LabelSelector:v1.LabelSelector{MatchLabels:map[string]string(nil), MatchExpressions:[]v1.LabelSelectorRequirement{v1.LabelSelectorRequirement{Key:\"test\", Operator:\"In\", Values:[]string{\"test\"}}}}, MatchAll:true}: matchExpressions cannot be used with matchAll",
160+
"spec.permissionClaims[2].selector.matchExpressions: Invalid value: v1alpha2.PermissionClaimSelector{LabelSelector:v1.LabelSelector{MatchLabels:map[string]string{\"test\":\"test\"}, MatchExpressions:[]v1.LabelSelectorRequirement{v1.LabelSelectorRequirement{Key:\"test\", Operator:\"In\", Values:[]string{\"test\"}}}}, MatchAll:true}: matchExpressions cannot be used with matchAll",
161+
"spec.permissionClaims[2].selector.matchLabels: Invalid value: v1alpha2.PermissionClaimSelector{LabelSelector:v1.LabelSelector{MatchLabels:map[string]string{\"test\":\"test\"}, MatchExpressions:[]v1.LabelSelectorRequirement{v1.LabelSelectorRequirement{Key:\"test\", Operator:\"In\", Values:[]string{\"test\"}}}}, MatchAll:true}: matchLabels cannot be used with matchAll",
162+
},
163+
},
164+
}
165+
166+
for name, tc := range tests {
167+
t.Run(name, func(t *testing.T) {
168+
got := ValidateAPIBindingPermissionClaims(tc.permissionClaims, field.NewPath("spec", "permissionClaims"))
169+
170+
// Convert FieldErrors into a string slice
171+
errs := []string{}
172+
for _, err := range got {
173+
errs = append(errs, err.Error())
174+
}
175+
176+
slices.Sort(errs)
177+
slices.Sort(tc.wantErrs)
178+
179+
if !equality.Semantic.DeepEqual(errs, tc.wantErrs) {
180+
t.Errorf("ValidateAPIBindingPermissionClaims() = %v, want %v", errs, tc.wantErrs)
181+
}
182+
})
183+
}
184+
}

0 commit comments

Comments
 (0)