Skip to content

Commit 708f3e4

Browse files
karmada-botkarmada-bot
authored
Merge pull request #6667 from XiShanYongYe-Chang/add-validate-for-aliasLabelName
add validation for policyv1alpha1.StatePreservation.Rules[*].AliasLabelName
2 parents 017379f + 9edc1b4 commit 708f3e4

File tree

2 files changed

+253
-0
lines changed

2 files changed

+253
-0
lines changed

‎pkg/util/validation/validation.go‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -211,6 +211,7 @@ func ValidateFailover(failoverBehavior *policyv1alpha1.FailoverBehavior, fldPath
211211
}
212212

213213
allErrs = append(allErrs, ValidateApplicationFailover(failoverBehavior.Application, fldPath.Child("application"))...)
214+
allErrs = append(allErrs, validateClusterFailover(failoverBehavior.Cluster, fldPath.Child("cluster"))...)
214215
return allErrs
215216
}
216217

@@ -246,9 +247,45 @@ func ValidateApplicationFailover(applicationFailoverBehavior *policyv1alpha1.App
246247
allErrs = append(allErrs, field.Invalid(fldPath.Child("gracePeriodSeconds"), *applicationFailoverBehavior.GracePeriodSeconds, "must be greater than 0"))
247248
}
248249

250+
if applicationFailoverBehavior.StatePreservation != nil {
251+
allErrs = append(allErrs, validateStatePreservation(applicationFailoverBehavior.StatePreservation, fldPath.Child("statePreservation"))...)
252+
}
253+
254+
return allErrs
255+
}
256+
257+
func validateClusterFailover(clusterFailoverBehavior *policyv1alpha1.ClusterFailoverBehavior, fldPath *field.Path) field.ErrorList {
258+
var allErrs field.ErrorList
259+
260+
if clusterFailoverBehavior == nil {
261+
return nil
262+
}
263+
264+
if clusterFailoverBehavior.StatePreservation != nil {
265+
allErrs = append(allErrs, validateStatePreservation(clusterFailoverBehavior.StatePreservation, fldPath.Child("statePreservation"))...)
266+
}
267+
249268
return allErrs
250269
}
251270

271+
func validateStatePreservation(statePreservation *policyv1alpha1.StatePreservation, fldPath *field.Path) field.ErrorList {
272+
var allErrs field.ErrorList
273+
274+
if statePreservation == nil {
275+
return nil
276+
}
277+
278+
for index, rule := range statePreservation.Rules {
279+
allErrs = append(allErrs, validateStatePreservationRule(rule, fldPath.Child("rules").Index(index))...)
280+
}
281+
282+
return allErrs
283+
}
284+
285+
func validateStatePreservationRule(rule policyv1alpha1.StatePreservationRule, fldPath *field.Path) field.ErrorList {
286+
return metav1validation.ValidateLabelName(rule.AliasLabelName, fldPath.Child("aliasLabelName"))
287+
}
288+
252289
// ValidateOverrideSpec validates that the overrider specification is correctly defined.
253290
func ValidateOverrideSpec(overrideSpec *policyv1alpha1.OverrideSpec) field.ErrorList {
254291
var allErrs field.ErrorList

‎pkg/util/validation/validation_test.go‎

Lines changed: 216 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -707,6 +707,160 @@ func TestValidateApplicationFailover(t *testing.T) {
707707
},
708708
expectedErr: "",
709709
},
710+
{
711+
name: "statePreservation is nil",
712+
applicationFailoverBehavior: &policyv1alpha1.ApplicationFailoverBehavior{
713+
DecisionConditions: policyv1alpha1.DecisionConditions{
714+
TolerationSeconds: ptr.To[int32](100),
715+
},
716+
StatePreservation: nil,
717+
},
718+
expectedErr: "",
719+
},
720+
{
721+
name: "statePreservation with valid rules",
722+
applicationFailoverBehavior: &policyv1alpha1.ApplicationFailoverBehavior{
723+
DecisionConditions: policyv1alpha1.DecisionConditions{
724+
TolerationSeconds: ptr.To[int32](100),
725+
},
726+
StatePreservation: &policyv1alpha1.StatePreservation{
727+
Rules: []policyv1alpha1.StatePreservationRule{
728+
{
729+
AliasLabelName: "valid-label-name",
730+
JSONPath: "{.availableReplicas}",
731+
},
732+
{
733+
AliasLabelName: "example.com/state-key",
734+
JSONPath: "{.readyReplicas}",
735+
},
736+
},
737+
},
738+
},
739+
expectedErr: "",
740+
},
741+
{
742+
name: "statePreservation with invalid aliasLabelName - starts with dash",
743+
applicationFailoverBehavior: &policyv1alpha1.ApplicationFailoverBehavior{
744+
DecisionConditions: policyv1alpha1.DecisionConditions{
745+
TolerationSeconds: ptr.To[int32](100),
746+
},
747+
StatePreservation: &policyv1alpha1.StatePreservation{
748+
Rules: []policyv1alpha1.StatePreservationRule{
749+
{
750+
AliasLabelName: "-invalid-label",
751+
JSONPath: "{.availableReplicas}",
752+
},
753+
},
754+
},
755+
},
756+
expectedErr: "spec.failover.application.statePreservation.rules[0].aliasLabelName: Invalid value: \"-invalid-label\": name part must consist of alphanumeric characters, '-', '_' or '.', and must start and end with an alphanumeric character (e.g. 'MyName', or 'my.name', or '123-abc', regex used for validation is '([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9]')",
757+
},
758+
{
759+
name: "statePreservation with invalid aliasLabelName - ends with dash",
760+
applicationFailoverBehavior: &policyv1alpha1.ApplicationFailoverBehavior{
761+
DecisionConditions: policyv1alpha1.DecisionConditions{
762+
TolerationSeconds: ptr.To[int32](100),
763+
},
764+
StatePreservation: &policyv1alpha1.StatePreservation{
765+
Rules: []policyv1alpha1.StatePreservationRule{
766+
{
767+
AliasLabelName: "invalid-label-",
768+
JSONPath: "{.availableReplicas}",
769+
},
770+
},
771+
},
772+
},
773+
expectedErr: "spec.failover.application.statePreservation.rules[0].aliasLabelName: Invalid value: \"invalid-label-\": name part must consist of alphanumeric characters, '-', '_' or '.', and must start and end with an alphanumeric character (e.g. 'MyName', or 'my.name', or '123-abc', regex used for validation is '([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9]')",
774+
},
775+
{
776+
name: "statePreservation with invalid aliasLabelName - contains invalid characters",
777+
applicationFailoverBehavior: &policyv1alpha1.ApplicationFailoverBehavior{
778+
DecisionConditions: policyv1alpha1.DecisionConditions{
779+
TolerationSeconds: ptr.To[int32](100),
780+
},
781+
StatePreservation: &policyv1alpha1.StatePreservation{
782+
Rules: []policyv1alpha1.StatePreservationRule{
783+
{
784+
AliasLabelName: "invalid@label",
785+
JSONPath: "{.availableReplicas}",
786+
},
787+
},
788+
},
789+
},
790+
expectedErr: "spec.failover.application.statePreservation.rules[0].aliasLabelName: Invalid value: \"invalid@label\": name part must consist of alphanumeric characters, '-', '_' or '.', and must start and end with an alphanumeric character (e.g. 'MyName', or 'my.name', or '123-abc', regex used for validation is '([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9]')",
791+
},
792+
{
793+
name: "statePreservation with too long aliasLabelName",
794+
applicationFailoverBehavior: &policyv1alpha1.ApplicationFailoverBehavior{
795+
DecisionConditions: policyv1alpha1.DecisionConditions{
796+
TolerationSeconds: ptr.To[int32](100),
797+
},
798+
StatePreservation: &policyv1alpha1.StatePreservation{
799+
Rules: []policyv1alpha1.StatePreservationRule{
800+
{
801+
AliasLabelName: strings.Repeat("a", 64),
802+
JSONPath: "{.availableReplicas}",
803+
},
804+
},
805+
},
806+
},
807+
expectedErr: "spec.failover.application.statePreservation.rules[0].aliasLabelName: Invalid value: \"" + strings.Repeat("a", 64) + "\": name part must be no more than 63 characters",
808+
},
809+
{
810+
name: "statePreservation with multiple invalid rules",
811+
applicationFailoverBehavior: &policyv1alpha1.ApplicationFailoverBehavior{
812+
DecisionConditions: policyv1alpha1.DecisionConditions{
813+
TolerationSeconds: ptr.To[int32](100),
814+
},
815+
StatePreservation: &policyv1alpha1.StatePreservation{
816+
Rules: []policyv1alpha1.StatePreservationRule{
817+
{
818+
AliasLabelName: "-invalid1",
819+
JSONPath: "{.availableReplicas}",
820+
},
821+
{
822+
AliasLabelName: "invalid2@",
823+
JSONPath: "{.readyReplicas}",
824+
},
825+
},
826+
},
827+
},
828+
expectedErr: "[spec.failover.application.statePreservation.rules[0].aliasLabelName: Invalid value: \"-invalid1\": name part must consist of alphanumeric characters, '-', '_' or '.', and must start and end with an alphanumeric character (e.g. 'MyName', or 'my.name', or '123-abc', regex used for validation is '([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9]'), spec.failover.application.statePreservation.rules[1].aliasLabelName: Invalid value: \"invalid2@\": name part must consist of alphanumeric characters, '-', '_' or '.', and must start and end with an alphanumeric character (e.g. 'MyName', or 'my.name', or '123-abc', regex used for validation is '([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9]')]",
829+
},
830+
{
831+
name: "statePreservation with valid prefix in aliasLabelName",
832+
applicationFailoverBehavior: &policyv1alpha1.ApplicationFailoverBehavior{
833+
DecisionConditions: policyv1alpha1.DecisionConditions{
834+
TolerationSeconds: ptr.To[int32](100),
835+
},
836+
StatePreservation: &policyv1alpha1.StatePreservation{
837+
Rules: []policyv1alpha1.StatePreservationRule{
838+
{
839+
AliasLabelName: "example.com/my-state",
840+
JSONPath: "{.availableReplicas}",
841+
},
842+
},
843+
},
844+
},
845+
expectedErr: "",
846+
},
847+
{
848+
name: "statePreservation with invalid prefix in aliasLabelName",
849+
applicationFailoverBehavior: &policyv1alpha1.ApplicationFailoverBehavior{
850+
DecisionConditions: policyv1alpha1.DecisionConditions{
851+
TolerationSeconds: ptr.To[int32](100),
852+
},
853+
StatePreservation: &policyv1alpha1.StatePreservation{
854+
Rules: []policyv1alpha1.StatePreservationRule{
855+
{
856+
AliasLabelName: "INVALID.COM/my-state",
857+
JSONPath: "{.availableReplicas}",
858+
},
859+
},
860+
},
861+
},
862+
expectedErr: "spec.failover.application.statePreservation.rules[0].aliasLabelName: Invalid value: \"INVALID.COM/my-state\": prefix part a lowercase RFC 1123 subdomain must consist of lower case alphanumeric characters, '-' or '.', and must start and end with an alphanumeric character (e.g. 'example.com', regex used for validation is '[a-z0-9]([-a-z0-9]*[a-z0-9])?(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*')",
863+
},
710864
}
711865
for _, tt := range tests {
712866
t.Run(tt.name, func(t *testing.T) {
@@ -721,6 +875,68 @@ func TestValidateApplicationFailover(t *testing.T) {
721875
}
722876
}
723877

878+
func TestValidateClusterFailover(t *testing.T) {
879+
tests := []struct {
880+
name string
881+
clusterFailoverBehavior *policyv1alpha1.ClusterFailoverBehavior
882+
expectedErr string
883+
}{
884+
{
885+
name: "cluster failover is nil",
886+
clusterFailoverBehavior: nil,
887+
expectedErr: "",
888+
},
889+
{
890+
name: "cluster failover with valid statePreservation",
891+
clusterFailoverBehavior: &policyv1alpha1.ClusterFailoverBehavior{
892+
PurgeMode: policyv1alpha1.PurgeModeGracefully,
893+
StatePreservation: &policyv1alpha1.StatePreservation{
894+
Rules: []policyv1alpha1.StatePreservationRule{
895+
{
896+
AliasLabelName: "cluster-state",
897+
JSONPath: "{.status.nodeCount}",
898+
},
899+
},
900+
},
901+
},
902+
expectedErr: "",
903+
},
904+
{
905+
name: "cluster failover with invalid statePreservation",
906+
clusterFailoverBehavior: &policyv1alpha1.ClusterFailoverBehavior{
907+
PurgeMode: policyv1alpha1.PurgeModeDirectly,
908+
StatePreservation: &policyv1alpha1.StatePreservation{
909+
Rules: []policyv1alpha1.StatePreservationRule{
910+
{
911+
AliasLabelName: "-invalid-cluster-label",
912+
JSONPath: "{.status.nodeCount}",
913+
},
914+
},
915+
},
916+
},
917+
expectedErr: "spec.failover.cluster.statePreservation.rules[0].aliasLabelName: Invalid value: \"-invalid-cluster-label\": name part must consist of alphanumeric characters, '-', '_' or '.', and must start and end with an alphanumeric character (e.g. 'MyName', or 'my.name', or '123-abc', regex used for validation is '([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9]')",
918+
},
919+
{
920+
name: "cluster failover without statePreservation",
921+
clusterFailoverBehavior: &policyv1alpha1.ClusterFailoverBehavior{
922+
PurgeMode: policyv1alpha1.PurgeModeGracefully,
923+
},
924+
expectedErr: "",
925+
},
926+
}
927+
for _, tt := range tests {
928+
t.Run(tt.name, func(t *testing.T) {
929+
errs := validateClusterFailover(tt.clusterFailoverBehavior, field.NewPath("spec").Child("failover").Child("cluster"))
930+
err := errs.ToAggregate()
931+
if err != nil && err.Error() != tt.expectedErr {
932+
t.Errorf("expected error:\n %s, but got:\n %s", tt.expectedErr, err.Error())
933+
} else if err == nil && tt.expectedErr != "" {
934+
t.Errorf("expected error:\n %s, but got no error\n", tt.expectedErr)
935+
}
936+
})
937+
}
938+
}
939+
724940
func TestValidateCrdsTarBall(t *testing.T) {
725941
testItems := []struct {
726942
name string

0 commit comments

Comments
 (0)