Skip to content

Commit 8bd5bea

Browse files
karmada-botkarmada-bot
authored
Merge pull request #6945 from XiShanYongYe-Chang/validate-dependentobjectreference-labelselector
Enhances validation for dependency verification in resource interpreter
2 parents 90c05cd + a3e4b9f commit 8bd5bea

File tree

2 files changed

+198
-19
lines changed

2 files changed

+198
-19
lines changed

‎pkg/util/interpreter/validation/validation.go‎

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -17,24 +17,30 @@ limitations under the License.
1717
package validation
1818

1919
import (
20-
"errors"
21-
22-
utilerrors "k8s.io/apimachinery/pkg/util/errors"
20+
metav1validation "k8s.io/apimachinery/pkg/apis/meta/v1/validation"
21+
"k8s.io/apimachinery/pkg/util/validation/field"
2322

2423
configv1alpha1 "github.com/karmada-io/karmada/pkg/apis/config/v1alpha1"
2524
)
2625

2726
// VerifyDependencies verifies dependencies.
2827
func VerifyDependencies(dependencies []configv1alpha1.DependentObjectReference) error {
29-
var errs []error
30-
for _, dependency := range dependencies {
31-
if len(dependency.APIVersion) == 0 || len(dependency.Kind) == 0 {
32-
errs = append(errs, errors.New("dependency missing required apiVersion or kind"))
33-
continue
28+
allErrs := field.ErrorList{}
29+
fldPath := field.NewPath("dependencies")
30+
for i, dependency := range dependencies {
31+
fldPath := fldPath.Index(i)
32+
if len(dependency.APIVersion) == 0 {
33+
allErrs = append(allErrs, field.Invalid(fldPath.Child("apiVersion"), dependency.APIVersion, "missing required apiVersion"))
34+
}
35+
if len(dependency.Kind) == 0 {
36+
allErrs = append(allErrs, field.Invalid(fldPath.Child("kind"), dependency.Kind, "missing required kind"))
3437
}
3538
if len(dependency.Name) == 0 && dependency.LabelSelector == nil {
36-
errs = append(errs, errors.New("dependency can not leave name and labelSelector all empty"))
39+
allErrs = append(allErrs, field.Invalid(fldPath, dependencies[i], "dependency can not leave name and labelSelector all empty"))
3740
}
41+
allErrs = append(allErrs, metav1validation.ValidateLabelSelector(dependency.LabelSelector, metav1validation.LabelSelectorValidationOptions{
42+
AllowInvalidLabelValueInSelector: false,
43+
}, fldPath.Child("labelSelector"))...)
3844
}
39-
return utilerrors.NewAggregate(errs)
45+
return allErrs.ToAggregate()
4046
}

‎pkg/util/interpreter/validation/validation_test.go‎

Lines changed: 182 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ limitations under the License.
1717
package validation
1818

1919
import (
20+
"strings"
2021
"testing"
2122

2223
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -29,15 +30,44 @@ func TestVerifyDependencies(t *testing.T) {
2930
dependencies []configv1alpha1.DependentObjectReference
3031
}
3132
tests := []struct {
32-
name string
33-
args args
34-
wantErr bool
33+
name string
34+
args args
35+
wantErr bool
36+
wantErrContains string
3537
}{
3638
{
37-
name: "normal case",
39+
name: "normal case with name only",
3840
args: args{dependencies: []configv1alpha1.DependentObjectReference{
3941
{APIVersion: "v1", Kind: "Foo", Name: "test"},
40-
{APIVersion: "v2", Kind: "Hu", Namespace: "default", LabelSelector: &metav1.LabelSelector{MatchLabels: map[string]string{"bar": "foo"}}},
42+
}},
43+
wantErr: false,
44+
},
45+
{
46+
name: "normal case with labelSelector only",
47+
args: args{dependencies: []configv1alpha1.DependentObjectReference{
48+
{APIVersion: "v1", Kind: "Foo", LabelSelector: &metav1.LabelSelector{MatchLabels: map[string]string{"app": "nginx"}}},
49+
}},
50+
wantErr: false,
51+
},
52+
{
53+
name: "normal case with both name and labelSelector",
54+
args: args{dependencies: []configv1alpha1.DependentObjectReference{
55+
{APIVersion: "v2", Kind: "Hu", Namespace: "default", Name: "test", LabelSelector: &metav1.LabelSelector{MatchLabels: map[string]string{"bar": "foo"}}},
56+
}},
57+
wantErr: false,
58+
},
59+
{
60+
name: "normal case with matchExpressions",
61+
args: args{dependencies: []configv1alpha1.DependentObjectReference{
62+
{
63+
APIVersion: "v1",
64+
Kind: "ConfigMap",
65+
LabelSelector: &metav1.LabelSelector{
66+
MatchExpressions: []metav1.LabelSelectorRequirement{
67+
{Key: "env", Operator: metav1.LabelSelectorOpIn, Values: []string{"prod", "staging"}},
68+
},
69+
},
70+
},
4171
}},
4272
wantErr: false,
4373
},
@@ -46,27 +76,170 @@ func TestVerifyDependencies(t *testing.T) {
4676
args: args{dependencies: []configv1alpha1.DependentObjectReference{
4777
{Kind: "Foo", Name: "test"},
4878
}},
49-
wantErr: true,
79+
wantErr: true,
80+
wantErrContains: "missing required apiVersion",
5081
},
5182
{
5283
name: "empty kind",
5384
args: args{dependencies: []configv1alpha1.DependentObjectReference{
5485
{APIVersion: "v1", Name: "test"},
5586
}},
56-
wantErr: true,
87+
wantErr: true,
88+
wantErrContains: "missing required kind",
5789
},
5890
{
5991
name: "empty Name and LabelSelector at the same time",
6092
args: args{dependencies: []configv1alpha1.DependentObjectReference{
6193
{APIVersion: "v1", Kind: "Foo"},
6294
}},
63-
wantErr: true,
95+
wantErr: true,
96+
wantErrContains: "dependency can not leave name and labelSelector all empty",
97+
},
98+
{
99+
name: "invalid label key in matchLabels",
100+
args: args{dependencies: []configv1alpha1.DependentObjectReference{
101+
{
102+
APIVersion: "v1",
103+
Kind: "Pod",
104+
LabelSelector: &metav1.LabelSelector{
105+
MatchLabels: map[string]string{
106+
"-invalid-key": "value",
107+
},
108+
},
109+
},
110+
}},
111+
wantErr: true,
112+
wantErrContains: "dependencies[0].labelSelector.matchLabels",
113+
},
114+
{
115+
name: "invalid label value in matchLabels",
116+
args: args{dependencies: []configv1alpha1.DependentObjectReference{
117+
{
118+
APIVersion: "v1",
119+
Kind: "Pod",
120+
LabelSelector: &metav1.LabelSelector{
121+
MatchLabels: map[string]string{
122+
"app": "invalid@value",
123+
},
124+
},
125+
},
126+
}},
127+
wantErr: true,
128+
wantErrContains: "dependencies[0].labelSelector.matchLabels",
129+
},
130+
{
131+
name: "invalid operator in matchExpressions",
132+
args: args{dependencies: []configv1alpha1.DependentObjectReference{
133+
{
134+
APIVersion: "v1",
135+
Kind: "Pod",
136+
LabelSelector: &metav1.LabelSelector{
137+
MatchExpressions: []metav1.LabelSelectorRequirement{
138+
{Key: "env", Operator: "InvalidOp", Values: []string{"prod"}},
139+
},
140+
},
141+
},
142+
}},
143+
wantErr: true,
144+
wantErrContains: "not a valid selector operator",
145+
},
146+
{
147+
name: "matchExpressions with In operator but no values",
148+
args: args{dependencies: []configv1alpha1.DependentObjectReference{
149+
{
150+
APIVersion: "v1",
151+
Kind: "Pod",
152+
LabelSelector: &metav1.LabelSelector{
153+
MatchExpressions: []metav1.LabelSelectorRequirement{
154+
{Key: "env", Operator: metav1.LabelSelectorOpIn, Values: []string{}},
155+
},
156+
},
157+
},
158+
}},
159+
wantErr: true,
160+
wantErrContains: "must be specified when `operator` is 'In' or 'NotIn'",
161+
},
162+
{
163+
name: "matchExpressions with Exists operator and values",
164+
args: args{dependencies: []configv1alpha1.DependentObjectReference{
165+
{
166+
APIVersion: "v1",
167+
Kind: "Pod",
168+
LabelSelector: &metav1.LabelSelector{
169+
MatchExpressions: []metav1.LabelSelectorRequirement{
170+
{Key: "env", Operator: metav1.LabelSelectorOpExists, Values: []string{"prod"}},
171+
},
172+
},
173+
},
174+
}},
175+
wantErr: true,
176+
wantErrContains: "may not be specified when `operator` is 'Exists' or 'DoesNotExist'",
177+
},
178+
{
179+
name: "invalid label key in matchExpressions",
180+
args: args{dependencies: []configv1alpha1.DependentObjectReference{
181+
{
182+
APIVersion: "v1",
183+
Kind: "Pod",
184+
LabelSelector: &metav1.LabelSelector{
185+
MatchExpressions: []metav1.LabelSelectorRequirement{
186+
{Key: "@invalid", Operator: metav1.LabelSelectorOpIn, Values: []string{"value"}},
187+
},
188+
},
189+
},
190+
}},
191+
wantErr: true,
192+
wantErrContains: "dependencies[0].labelSelector.matchExpressions[0].key",
193+
},
194+
{
195+
name: "multiple dependencies with mixed errors",
196+
args: args{dependencies: []configv1alpha1.DependentObjectReference{
197+
{APIVersion: "v1", Kind: "Foo", Name: "valid"},
198+
{Kind: "Bar", Name: "missing-apiversion"},
199+
}},
200+
wantErr: true,
201+
wantErrContains: "dependencies[1].apiVersion",
202+
},
203+
{
204+
name: "multiple errors in single dependency",
205+
args: args{dependencies: []configv1alpha1.DependentObjectReference{
206+
{
207+
// Missing both apiVersion and kind
208+
Name: "test",
209+
},
210+
}},
211+
wantErr: true,
212+
wantErrContains: "missing required",
213+
},
214+
{
215+
name: "label value too long",
216+
args: args{dependencies: []configv1alpha1.DependentObjectReference{
217+
{
218+
APIVersion: "v1",
219+
Kind: "Pod",
220+
LabelSelector: &metav1.LabelSelector{
221+
MatchLabels: map[string]string{
222+
"app": "this-is-a-very-long-label-value-that-exceeds-the-maximum-allowed-length-of-63-characters-for-kubernetes-labels",
223+
},
224+
},
225+
},
226+
}},
227+
wantErr: true,
228+
wantErrContains: "must be no more than 63 characters",
64229
},
65230
}
66231
for _, tt := range tests {
67232
t.Run(tt.name, func(t *testing.T) {
68-
if err := VerifyDependencies(tt.args.dependencies); (err != nil) != tt.wantErr {
233+
err := VerifyDependencies(tt.args.dependencies)
234+
if (err != nil) != tt.wantErr {
69235
t.Errorf("VerifyDependencies() error = %v, wantErr %v", err, tt.wantErr)
236+
return
237+
}
238+
239+
if tt.wantErr && tt.wantErrContains != "" {
240+
if !strings.Contains(err.Error(), tt.wantErrContains) {
241+
t.Errorf("VerifyDependencies() error = %v, want error containing %q", err, tt.wantErrContains)
242+
}
70243
}
71244
})
72245
}

0 commit comments

Comments
 (0)