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 #2879 from carlory/validate 

move validation funcs into util/validation
This commit is contained in:
karmada-bot 2022-12-14 10:10:23 +08:00 committed by GitHub
parent 796de421b8 1b33ef882b
commit 0131bffaae
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 98 additions and 100 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

34
pkg/util/helper/policy.go
View File

@ -2,11 +2,9 @@ package helper
import (
"encoding/json"
"fmt"
discoveryv1 "k8s.io/api/discovery/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/sets"
"k8s.io/klog/v2"
"sigs.k8s.io/controller-runtime/pkg/client"
@ -30,38 +28,6 @@ func SetDefaultSpreadConstraints(spreadConstraints []policyv1alpha1.SpreadConstr
}
}
// ValidateSpreadConstraint tests if the constraints is valid.
func ValidateSpreadConstraint(spreadConstraints []policyv1alpha1.SpreadConstraint) error {
spreadByFields := sets.NewString()
for _, 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)
}
// 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)
}
if len(constraint.SpreadByField) > 0 {
spreadByFields.Insert(string(constraint.SpreadByField))
}
}
if spreadByFields.Len() > 0 {
// If one of spread constraints are using 'SpreadByField', the 'SpreadByFieldCluster' must be included.
// 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")
}
}
return nil
}
// IsDependentOverridesPresent checks if a PropagationPolicy's dependent OverridePolicy all exist.
func IsDependentOverridesPresent(c client.Client, policy *policyv1alpha1.PropagationPolicy) (bool, error) {
ns := policy.Namespace

62
pkg/util/helper/policy_test.go
View File

@ -2,7 +2,6 @@ package helper
import (
"context"
"fmt"
"reflect"
"testing"
@ -60,67 +59,6 @@ func TestSetDefaultSpreadConstraints(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 TestIsDependentOverridesPresent(t *testing.T) {
tests := []struct {
name string

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

@ -4,6 +4,7 @@ import (
"fmt"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/util/sets"
policyv1alpha1 "github.com/karmada-io/karmada/pkg/apis/policy/v1alpha1"
"github.com/karmada-io/karmada/pkg/util"
@ -35,6 +36,38 @@ func ValidatePolicyFieldSelector(fieldSelector *policyv1alpha1.FieldSelector) er
return nil
}
// ValidateSpreadConstraint tests if the constraints is valid.
func ValidateSpreadConstraint(spreadConstraints []policyv1alpha1.SpreadConstraint) error {
spreadByFields := sets.NewString()
for _, 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)
}
// 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)
}
if len(constraint.SpreadByField) > 0 {
spreadByFields.Insert(string(constraint.SpreadByField))
}
}
if spreadByFields.Len() > 0 {
// If one of spread constraints are using 'SpreadByField', the 'SpreadByFieldCluster' must be included.
// 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")
}
}
return nil
}
// ValidateOverrideSpec tests if the overrideRules and (overriders or targetCluster) co-exist
func ValidateOverrideSpec(overrideSpec *policyv1alpha1.OverrideSpec) error {
if overrideSpec == nil {

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

@ -1,6 +1,8 @@
package validation
import (
"fmt"
"reflect"
"testing"
corev1 "k8s.io/api/core/v1"
@ -117,6 +119,67 @@ 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"}

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

@ -8,7 +8,6 @@ import (
"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
policyv1alpha1 "github.com/karmada-io/karmada/pkg/apis/policy/v1alpha1"
"github.com/karmada-io/karmada/pkg/util/helper"
"github.com/karmada-io/karmada/pkg/util/validation"
)
@ -32,7 +31,7 @@ func (v *ValidatingAdmission) Handle(ctx context.Context, req admission.Request)
}
klog.V(2).Infof("Validating ClusterPropagationPolicy(%s) for request: %s", policy.Name, req.Operation)
if err := helper.ValidateSpreadConstraint(policy.Spec.Placement.SpreadConstraints); err != nil {
if err := validation.ValidateSpreadConstraint(policy.Spec.Placement.SpreadConstraints); err != nil {
klog.Error(err)
return admission.Denied(err.Error())
}

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

@ -8,7 +8,6 @@ import (
"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
policyv1alpha1 "github.com/karmada-io/karmada/pkg/apis/policy/v1alpha1"
"github.com/karmada-io/karmada/pkg/util/helper"
"github.com/karmada-io/karmada/pkg/util/validation"
)
@ -32,7 +31,7 @@ func (v *ValidatingAdmission) Handle(ctx context.Context, req admission.Request)
}
klog.V(2).Infof("Validating PropagationPolicy(%s/%s) for request: %s", policy.Namespace, policy.Name, req.Operation)
if err := helper.ValidateSpreadConstraint(policy.Spec.Placement.SpreadConstraints); err != nil {
if err := validation.ValidateSpreadConstraint(policy.Spec.Placement.SpreadConstraints); err != nil {
klog.Error(err)
return admission.Denied(err.Error())
}