From 8a1f9f43e63401ab45e023ea6d258dd2a7a1a783 Mon Sep 17 00:00:00 2001 From: Alex Wang Date: Tue, 14 Mar 2023 11:29:12 +0800 Subject: [PATCH] feat: validate matchLabelKeys when labelSelector isn't set Signed-off-by: Alex Wang --- pkg/apis/core/validation/validation.go | 5 ++++- pkg/apis/core/validation/validation_test.go | 17 +++++++++++++++-- 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/pkg/apis/core/validation/validation.go b/pkg/apis/core/validation/validation.go index 61d0fb3af42..4d910e9c5b9 100644 --- a/pkg/apis/core/validation/validation.go +++ b/pkg/apis/core/validation/validation.go @@ -6964,7 +6964,9 @@ func validateMatchLabelKeys(fldPath *field.Path, matchLabelKeys []string, labelS return nil } + var allErrs field.ErrorList labelSelectorKeys := sets.String{} + if labelSelector != nil { for key := range labelSelector.MatchLabels { labelSelectorKeys.Insert(key) @@ -6972,9 +6974,10 @@ func validateMatchLabelKeys(fldPath *field.Path, matchLabelKeys []string, labelS for _, matchExpression := range labelSelector.MatchExpressions { labelSelectorKeys.Insert(matchExpression.Key) } + } else { + allErrs = append(allErrs, field.Forbidden(fldPath, "must not be specified when labelSelector is not set")) } - allErrs := field.ErrorList{} for i, key := range matchLabelKeys { allErrs = append(allErrs, unversionedvalidation.ValidateLabelName(key, fldPath.Index(i))...) if labelSelectorKeys.Has(key) { diff --git a/pkg/apis/core/validation/validation_test.go b/pkg/apis/core/validation/validation_test.go index d20f0ffa532..793b067a2ad 100644 --- a/pkg/apis/core/validation/validation_test.go +++ b/pkg/apis/core/validation/validation_test.go @@ -22091,11 +22091,12 @@ func TestValidateTopologySpreadConstraints(t *testing.T) { { MaxSkew: 1, TopologyKey: "k8s.io/zone", + LabelSelector: &metav1.LabelSelector{}, WhenUnsatisfiable: core.DoNotSchedule, MatchLabelKeys: []string{"/simple"}, }, }, - wantFieldErrors: []*field.Error{field.Invalid(fieldPathMatchLabelKeys.Index(0), "/simple", "prefix part must be non-empty")}, + wantFieldErrors: field.ErrorList{field.Invalid(fieldPathMatchLabelKeys.Index(0), "/simple", "prefix part must be non-empty")}, }, { name: "key exists in both matchLabelKeys and labelSelector", @@ -22116,7 +22117,19 @@ func TestValidateTopologySpreadConstraints(t *testing.T) { }, }, }, - wantFieldErrors: []*field.Error{field.Invalid(fieldPathMatchLabelKeys.Index(0), "foo", "exists in both matchLabelKeys and labelSelector")}, + wantFieldErrors: field.ErrorList{field.Invalid(fieldPathMatchLabelKeys.Index(0), "foo", "exists in both matchLabelKeys and labelSelector")}, + }, + { + name: "key in MatchLabelKeys is forbidden to be specified when labelSelector is not set", + constraints: []core.TopologySpreadConstraint{ + { + MaxSkew: 1, + TopologyKey: "k8s.io/zone", + WhenUnsatisfiable: core.DoNotSchedule, + MatchLabelKeys: []string{"foo"}, + }, + }, + wantFieldErrors: field.ErrorList{field.Forbidden(fieldPathMatchLabelKeys, "must not be specified when labelSelector is not set")}, }, { name: "invalid matchLabels set on labelSelector when AllowInvalidTopologySpreadConstraintLabelSelector is false",