karmada-io
/
karmada
mirror of https://github.com/karmada-io/karmada.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #3164 from XiShanYongYe-Chang/add-validation-for-orderedClusterAffinities 

add validation for ClusterAffinities
This commit is contained in:
karmada-bot 2023-02-18 17:08:59 +08:00 committed by GitHub
parent eb77336ba0 42c2e3c643
commit 4641e88fd2
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 262 additions and 196 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

67
pkg/util/validation/validation.go
View File

@ -16,6 +16,58 @@ import (
// LabelValueMaxLength is a label's max length
const LabelValueMaxLength int = 63
// ValidatePropagationSpec validates a PropagationSpec before creation or update.
func ValidatePropagationSpec(spec policyv1alpha1.PropagationSpec) field.ErrorList {
var allErrs field.ErrorList
allErrs = append(allErrs, ValidatePlacement(spec.Placement, field.NewPath("spec").Child("placement"))...)
return allErrs
}
// ValidatePlacement validates a PropagationSpec before creation or update.
func ValidatePlacement(placement policyv1alpha1.Placement, fldPath *field.Path) field.ErrorList {
var allErrs field.ErrorList
if placement.ClusterAffinity != nil && placement.ClusterAffinities != nil {
allErrs = append(allErrs, field.Invalid(fldPath, placement, "clusterAffinities can not co-exist with clusterAffinity"))
}
allErrs = append(allErrs, ValidateClusterAffinity(placement.ClusterAffinity, fldPath.Child("clusterAffinity"))...)
allErrs = append(allErrs, ValidateClusterAffinities(placement.ClusterAffinities, fldPath.Child("clusterAffinities"))...)
allErrs = append(allErrs, ValidateSpreadConstraint(placement.SpreadConstraints, fldPath.Child("spreadConstraints"))...)
return allErrs
}
// ValidateClusterAffinity validates a PropagationSpec before creation or update.
func ValidateClusterAffinity(affinity *policyv1alpha1.ClusterAffinity, fldPath *field.Path) field.ErrorList {
if affinity == nil {
return nil
}
var allErrs field.ErrorList
err := ValidatePolicyFieldSelector(affinity.FieldSelector)
if err != nil {
allErrs = append(allErrs, field.Invalid(fldPath.Child("fieldSelector"), affinity.FieldSelector, err.Error()))
}
return allErrs
}
// ValidateClusterAffinities validates a PropagationSpec before creation or update.
func ValidateClusterAffinities(affinities []policyv1alpha1.ClusterAffinityTerm, fldPath *field.Path) field.ErrorList {
var allErrs field.ErrorList
affinityNames := make(map[string]bool)
for index, term := range affinities {
if _, exist := affinityNames[term.AffinityName]; exist {
allErrs = append(allErrs, field.Invalid(fldPath, affinities, "each affinity term in a policy must have a unique name"))
} else {
affinityNames[term.AffinityName] = true
}
allErrs = append(allErrs, ValidateClusterAffinity(&term.ClusterAffinity, fldPath.Index(index))...)
}
return allErrs
}
// ValidatePolicyFieldSelector tests if the fieldSelector of propagation policy is valid.
func ValidatePolicyFieldSelector(fieldSelector *policyv1alpha1.FieldSelector) error {
if fieldSelector == nil {
@ -40,18 +92,19 @@ func ValidatePolicyFieldSelector(fieldSelector *policyv1alpha1.FieldSelector) er
}
// ValidateSpreadConstraint tests if the constraints is valid.
func ValidateSpreadConstraint(spreadConstraints []policyv1alpha1.SpreadConstraint) error {
spreadByFields := sets.NewString()
func ValidateSpreadConstraint(spreadConstraints []policyv1alpha1.SpreadConstraint, fldPath *field.Path) field.ErrorList {
var allErrs field.ErrorList
for _, constraint := range spreadConstraints {
spreadByFields := sets.New[string]()
for index, constraint := range spreadConstraints {
// SpreadByField and SpreadByLabel should not co-exist
if len(constraint.SpreadByField) > 0 && len(constraint.SpreadByLabel) > 0 {
return fmt.Errorf("invalid constraints: SpreadByLabel(%s) should not co-exist with spreadByField(%s)", constraint.SpreadByLabel, constraint.SpreadByField)
allErrs = append(allErrs, field.Invalid(fldPath.Index(index), constraint, "spreadByLabel should not co-exist with spreadByField"))
}
// If MaxGroups provided, it should greater or equal than MinGroups.
if constraint.MaxGroups > 0 && constraint.MaxGroups < constraint.MinGroups {
return fmt.Errorf("maxGroups(%d) lower than minGroups(%d) is not allowed", constraint.MaxGroups, constraint.MinGroups)
allErrs = append(allErrs, field.Invalid(fldPath.Index(index), constraint, "maxGroups lower than minGroups is not allowed"))
}
if len(constraint.SpreadByField) > 0 {
@ -64,11 +117,11 @@ func ValidateSpreadConstraint(spreadConstraints []policyv1alpha1.SpreadConstrain
// For example, when using 'SpreadByFieldRegion' to specify region groups, at the meantime, you must use
// 'SpreadByFieldCluster' to specify how many clusters should be selected.
if !spreadByFields.Has(string(policyv1alpha1.SpreadByFieldCluster)) {
return fmt.Errorf("the cluster spread constraint must be enabled in one of the constraints in case of SpreadByField is enabled")
allErrs = append(allErrs, field.Invalid(fldPath, spreadConstraints, "the cluster spread constraint must be enabled in one of the constraints in case of SpreadByField is enabled"))
}
}
return nil
return allErrs
}
// ValidateOverrideSpec validates that the overrider specification is correctly defined.

359
pkg/util/validation/validation_test.go
View File

@ -1,8 +1,7 @@
package validation
import (
"fmt"
"reflect"
"strings"
"testing"
corev1 "k8s.io/api/core/v1"
@ -161,169 +160,6 @@ func TestValidateOverrideSpec(t *testing.T) {
}
}
func TestValidateSpreadConstraint(t *testing.T) {
tests := []struct {
name string
spreadConstraint []policyv1alpha1.SpreadConstraint
expectedErr error
}{
{
name: "spreadByLabel co-exist with spreadByField",
spreadConstraint: []policyv1alpha1.SpreadConstraint{
{
SpreadByField: policyv1alpha1.SpreadByFieldCluster,
SpreadByLabel: "foo",
},
},
expectedErr: fmt.Errorf("invalid constraints: SpreadByLabel(foo) should not co-exist with spreadByField(cluster)"),
},
{
name: "maxGroups lower than minGroups",
spreadConstraint: []policyv1alpha1.SpreadConstraint{
{
MaxGroups: 1,
MinGroups: 2,
},
},
expectedErr: fmt.Errorf("maxGroups(1) lower than minGroups(2) is not allowed"),
},
{
name: "spreadByFieldCluster must be included if using spreadByField",
spreadConstraint: []policyv1alpha1.SpreadConstraint{
{
SpreadByField: policyv1alpha1.SpreadByFieldRegion,
},
},
expectedErr: fmt.Errorf("the cluster spread constraint must be enabled in one of the constraints in case of SpreadByField is enabled"),
},
{
name: "validate success",
spreadConstraint: []policyv1alpha1.SpreadConstraint{
{
MaxGroups: 2,
MinGroups: 1,
SpreadByField: policyv1alpha1.SpreadByFieldRegion,
},
{
SpreadByField: policyv1alpha1.SpreadByFieldCluster,
},
},
expectedErr: nil,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
err := ValidateSpreadConstraint(tt.spreadConstraint)
if !reflect.DeepEqual(err, tt.expectedErr) {
t.Errorf("expected: %v, but got %v", tt.expectedErr, err)
}
})
}
}
func TestValidatePolicyFieldSelector(t *testing.T) {
fakeProvider := []string{"fooCloud"}
fakeRegion := []string{"fooRegion"}
fakeZone := []string{"fooZone"}
tests := []struct {
name string
filedSelector *policyv1alpha1.FieldSelector
expectError bool
}{
{
name: "supported key",
filedSelector: &policyv1alpha1.FieldSelector{
MatchExpressions: []corev1.NodeSelectorRequirement{
{
Key: util.ProviderField,
Operator: corev1.NodeSelectorOpIn,
Values: fakeProvider,
},
{
Key: util.RegionField,
Operator: corev1.NodeSelectorOpNotIn,
Values: fakeRegion,
},
{
Key: util.ZoneField,
Operator: corev1.NodeSelectorOpNotIn,
Values: fakeZone,
},
},
},
expectError: false,
},
{
name: "unsupported key",
filedSelector: &policyv1alpha1.FieldSelector{
MatchExpressions: []corev1.NodeSelectorRequirement{
{
Key: "foo",
Operator: corev1.NodeSelectorOpIn,
Values: fakeProvider,
},
{
Key: util.RegionField,
Operator: corev1.NodeSelectorOpNotIn,
Values: fakeRegion,
},
},
},
expectError: true,
},
{
name: "supported operator",
filedSelector: &policyv1alpha1.FieldSelector{
MatchExpressions: []corev1.NodeSelectorRequirement{
{
Key: util.ProviderField,
Operator: corev1.NodeSelectorOpIn,
Values: fakeProvider,
},
{
Key: util.RegionField,
Operator: corev1.NodeSelectorOpNotIn,
Values: fakeRegion,
},
},
},
expectError: false,
},
{
name: "unsupported operator",
filedSelector: &policyv1alpha1.FieldSelector{
MatchExpressions: []corev1.NodeSelectorRequirement{
{
Key: util.ProviderField,
Operator: corev1.NodeSelectorOpExists,
Values: fakeProvider,
},
{
Key: util.RegionField,
Operator: corev1.NodeSelectorOpNotIn,
Values: fakeRegion,
},
},
},
expectError: true,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
err := ValidatePolicyFieldSelector(tt.filedSelector)
if err != nil && tt.expectError != true {
t.Fatalf("expect no error but got: %v", err)
}
if err == nil && tt.expectError == true {
t.Fatalf("expect an error but got none")
}
})
}
}
func TestEmptyOverrides(t *testing.T) {
tests := []struct {
name string
@ -380,3 +216,196 @@ func TestEmptyOverrides(t *testing.T) {
})
}
}
func TestValidatePropagationSpec(t *testing.T) {
tests := []struct {
name string
spec policyv1alpha1.PropagationSpec
expectedErr string
}{
{
name: "valid spec",
spec: policyv1alpha1.PropagationSpec{
Placement: policyv1alpha1.Placement{
ClusterAffinity: &policyv1alpha1.ClusterAffinity{
FieldSelector: &policyv1alpha1.FieldSelector{
MatchExpressions: []corev1.NodeSelectorRequirement{
{
Key: util.ProviderField,
Operator: corev1.NodeSelectorOpIn,
Values: []string{"fooCloud"},
},
{
Key: util.RegionField,
Operator: corev1.NodeSelectorOpNotIn,
Values: []string{"fooCloud"},
},
{
Key: util.ZoneField,
Operator: corev1.NodeSelectorOpNotIn,
Values: []string{"fooCloud"},
},
}}},
SpreadConstraints: []policyv1alpha1.SpreadConstraint{
{
MaxGroups: 2,
MinGroups: 1,
SpreadByField: policyv1alpha1.SpreadByFieldRegion,
},
{
SpreadByField: policyv1alpha1.SpreadByFieldCluster,
}}}},
expectedErr: "",
},
{
name: "clusterAffinity.fieldSelector has unsupported key",
spec: policyv1alpha1.PropagationSpec{
Placement: policyv1alpha1.Placement{
ClusterAffinity: &policyv1alpha1.ClusterAffinity{
FieldSelector: &policyv1alpha1.FieldSelector{
MatchExpressions: []corev1.NodeSelectorRequirement{
{
Key: "foo",
Operator: corev1.NodeSelectorOpIn,
Values: []string{"fooCloud"},
}}}}}},
expectedErr: "unsupported key \"foo\", must be provider, region, or zone",
},
{
name: "clusterAffinity.fieldSelector has unsupported operator",
spec: policyv1alpha1.PropagationSpec{
Placement: policyv1alpha1.Placement{
ClusterAffinity: &policyv1alpha1.ClusterAffinity{
FieldSelector: &policyv1alpha1.FieldSelector{
MatchExpressions: []corev1.NodeSelectorRequirement{
{
Key: util.ProviderField,
Operator: corev1.NodeSelectorOpExists,
Values: []string{"fooCloud"},
}}}}}},
expectedErr: "unsupported operator \"Exists\", must be In or NotIn",
},
{
name: "clusterAffinities can not co-exist with clusterAffinity",
spec: policyv1alpha1.PropagationSpec{
Placement: policyv1alpha1.Placement{
ClusterAffinity: &policyv1alpha1.ClusterAffinity{
ClusterNames: []string{"m1"},
},
ClusterAffinities: []policyv1alpha1.ClusterAffinityTerm{
{
AffinityName: "group1",
ClusterAffinity: policyv1alpha1.ClusterAffinity{
ClusterNames: []string{"m1"},
}}}}},
expectedErr: "clusterAffinities can not co-exist with clusterAffinity",
},
{
name: "clusterAffinities different affinities have the same affinityName",
spec: policyv1alpha1.PropagationSpec{
Placement: policyv1alpha1.Placement{
ClusterAffinities: []policyv1alpha1.ClusterAffinityTerm{
{
AffinityName: "group1",
ClusterAffinity: policyv1alpha1.ClusterAffinity{
ClusterNames: []string{"m1"},
}},
{
AffinityName: "group1",
ClusterAffinity: policyv1alpha1.ClusterAffinity{
ClusterNames: []string{"m2"},
}}}}},
expectedErr: "each affinity term in a policy must have a unique name",
},
{
name: "clusterAffinities.[index].clusterAffinity.fieldSelector has unsupported key",
spec: policyv1alpha1.PropagationSpec{
Placement: policyv1alpha1.Placement{
ClusterAffinities: []policyv1alpha1.ClusterAffinityTerm{
{
AffinityName: "group1",
ClusterAffinity: policyv1alpha1.ClusterAffinity{
FieldSelector: &policyv1alpha1.FieldSelector{
MatchExpressions: []corev1.NodeSelectorRequirement{
{
Key: "foo",
Operator: corev1.NodeSelectorOpIn,
Values: []string{"fooCloud"},
}}}}}}}},
expectedErr: "unsupported key \"foo\", must be provider, region, or zone",
},
{
name: "clusterAffinities.[index].clusterAffinity.fieldSelector has unsupported operator",
spec: policyv1alpha1.PropagationSpec{
Placement: policyv1alpha1.Placement{
ClusterAffinities: []policyv1alpha1.ClusterAffinityTerm{
{
AffinityName: "group1",
ClusterAffinity: policyv1alpha1.ClusterAffinity{
FieldSelector: &policyv1alpha1.FieldSelector{
MatchExpressions: []corev1.NodeSelectorRequirement{
{
Key: util.ProviderField,
Operator: corev1.NodeSelectorOpExists,
Values: []string{"fooCloud"},
}}}}}}}},
expectedErr: "unsupported operator \"Exists\", must be In or NotIn",
},
{
name: "spreadConstraint spreadByLabel co-exist with spreadByField",
spec: policyv1alpha1.PropagationSpec{
Placement: policyv1alpha1.Placement{
SpreadConstraints: []policyv1alpha1.SpreadConstraint{
{
SpreadByField: policyv1alpha1.SpreadByFieldCluster,
SpreadByLabel: "foo",
},
},
}},
expectedErr: "spreadByLabel should not co-exist with spreadByField",
},
{
name: "spreadConstraint maxGroups lower than minGroups",
spec: policyv1alpha1.PropagationSpec{
Placement: policyv1alpha1.Placement{
SpreadConstraints: []policyv1alpha1.SpreadConstraint{
{
MaxGroups: 1,
MinGroups: 2,
},
},
}},
expectedErr: "maxGroups lower than minGroups is not allowed",
},
{
name: "spreadConstraint spreadByFieldCluster must be included if using spreadByField",
spec: policyv1alpha1.PropagationSpec{
Placement: policyv1alpha1.Placement{
SpreadConstraints: []policyv1alpha1.SpreadConstraint{
{
SpreadByField: policyv1alpha1.SpreadByFieldRegion,
},
},
}},
expectedErr: "the cluster spread constraint must be enabled in one of the constraints in case of SpreadByField is enabled",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
errs := ValidatePropagationSpec(tt.spec)
err := errs.ToAggregate()
if err != nil {
errStr := err.Error()
if tt.expectedErr == "" {
t.Errorf("expected no error:\n but got:\n %s", errStr)
} else if !strings.Contains(errStr, tt.expectedErr) {
t.Errorf("expected to contain:\n %s\ngot:\n %s", tt.expectedErr, errStr)
}
} else {
if tt.expectedErr != "" {
t.Errorf("unexpected no error, expected to contain:\n %s", tt.expectedErr)
}
}
})
}
}

16
pkg/webhook/clusterpropagationpolicy/validating.go
View File

@ -47,19 +47,11 @@ func (v *ValidatingAdmission) Handle(ctx context.Context, req admission.Request)
}
}
if err := validation.ValidateSpreadConstraint(policy.Spec.Placement.SpreadConstraints); err != nil {
klog.Error(err)
return admission.Denied(err.Error())
errs := validation.ValidatePropagationSpec(policy.Spec)
if len(errs) != 0 {
klog.Error(errs)
return admission.Denied(errs.ToAggregate().Error())
}
if policy.Spec.Placement.ClusterAffinity != nil && policy.Spec.Placement.ClusterAffinity.FieldSelector != nil {
err := validation.ValidatePolicyFieldSelector(policy.Spec.Placement.ClusterAffinity.FieldSelector)
if err != nil {
klog.Error(err)
return admission.Denied(err.Error())
}
}
return admission.Allowed("")
}

16
pkg/webhook/propagationpolicy/validating.go
View File

@ -47,19 +47,11 @@ func (v *ValidatingAdmission) Handle(ctx context.Context, req admission.Request)
}
}
if err := validation.ValidateSpreadConstraint(policy.Spec.Placement.SpreadConstraints); err != nil {
klog.Error(err)
return admission.Denied(err.Error())
errs := validation.ValidatePropagationSpec(policy.Spec)
if len(errs) != 0 {
klog.Error(errs)
return admission.Denied(errs.ToAggregate().Error())
}
if policy.Spec.Placement.ClusterAffinity != nil && policy.Spec.Placement.ClusterAffinity.FieldSelector != nil {
err := validation.ValidatePolicyFieldSelector(policy.Spec.Placement.ClusterAffinity.FieldSelector)
if err != nil {
klog.Error(err)
return admission.Denied(err.Error())
}
}
return admission.Allowed("")
}