From 3cc2eaf8b894cf7256c1ce05db89b8cf4d0f1396 Mon Sep 17 00:00:00 2001 From: Nont Date: Wed, 10 Sep 2025 16:32:21 -0500 Subject: [PATCH 1/2] Add validation for PickFixed cluster names Signed-off-by: Nont --- pkg/utils/validator/clusterresourceplacement.go | 4 ++++ pkg/utils/validator/clusterresourceplacement_test.go | 8 ++++++++ 2 files changed, 12 insertions(+) diff --git a/pkg/utils/validator/clusterresourceplacement.go b/pkg/utils/validator/clusterresourceplacement.go index bb906c1ab..0cac2a25f 100644 --- a/pkg/utils/validator/clusterresourceplacement.go +++ b/pkg/utils/validator/clusterresourceplacement.go @@ -194,6 +194,10 @@ func validatePolicyForPickFixedPlacementType(policy *placementv1beta1.PlacementP } uniqueClusterNames := make(map[string]bool) for _, name := range policy.ClusterNames { + nameErr := validation.IsDNS1123Label(name) + if nameErr != nil { + allErr = append(allErr, fmt.Errorf("PickFixed cluster name %s is not a valid member name: %s", name, strings.Join(nameErr, "; "))) + } if _, ok := uniqueClusterNames[name]; ok { allErr = append(allErr, fmt.Errorf("cluster names must be unique for policy type %s", placementv1beta1.PickFixedPlacementType)) break diff --git a/pkg/utils/validator/clusterresourceplacement_test.go b/pkg/utils/validator/clusterresourceplacement_test.go index 3e8b7d5d5..da9a2b911 100644 --- a/pkg/utils/validator/clusterresourceplacement_test.go +++ b/pkg/utils/validator/clusterresourceplacement_test.go @@ -514,6 +514,14 @@ func TestValidateClusterResourcePlacement_PickFixedPlacementPolicy(t *testing.T) wantErr: true, wantErrMsg: "cluster names must be unique for policy type PickFixed", }, + "invalid placement policy - PickFixed with invalid cluster names": { + policy: &placementv1beta1.PlacementPolicy{ + PlacementType: placementv1beta1.PickFixedPlacementType, + ClusterNames: []string{"test@,cluster1", "test$cluster2"}, + }, + wantErr: true, + wantErrMsg: "PickFixed cluster name test@,cluster1 is not a valid member name: a lowercase RFC 1123 label must consist of lower case alphanumeric characters or '-', and must start and end with an alphanumeric character (e.g. 'my-name', or '123-abc', regex used for validation is '[a-z0-9]([-a-z0-9]*[a-z0-9])?'), PickFixed cluster name test$cluster2 is not a valid member name: a lowercase RFC 1123 label must consist of lower case alphanumeric characters or '-', and must start and end with an alphanumeric character (e.g. 'my-name', or '123-abc', regex used for validation is '[a-z0-9]([-a-z0-9]*[a-z0-9])?')", + }, "invalid placement policy - PickFixed with non nil number of clusters": { policy: &placementv1beta1.PlacementPolicy{ PlacementType: placementv1beta1.PickFixedPlacementType, From a7dfad38604309e109e1fa6608ef0ef270722013 Mon Sep 17 00:00:00 2001 From: Nont Date: Thu, 11 Sep 2025 21:51:17 -0500 Subject: [PATCH 2/2] Change to using 1123 subdomain validation Signed-off-by: Nont --- pkg/utils/validator/clusterresourceplacement.go | 5 ++++- pkg/utils/validator/clusterresourceplacement_test.go | 12 ++++++++++-- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/pkg/utils/validator/clusterresourceplacement.go b/pkg/utils/validator/clusterresourceplacement.go index 0cac2a25f..e46ccf130 100644 --- a/pkg/utils/validator/clusterresourceplacement.go +++ b/pkg/utils/validator/clusterresourceplacement.go @@ -194,10 +194,13 @@ func validatePolicyForPickFixedPlacementType(policy *placementv1beta1.PlacementP } uniqueClusterNames := make(map[string]bool) for _, name := range policy.ClusterNames { - nameErr := validation.IsDNS1123Label(name) + nameErr := validation.IsDNS1123Subdomain(name) if nameErr != nil { allErr = append(allErr, fmt.Errorf("PickFixed cluster name %s is not a valid member name: %s", name, strings.Join(nameErr, "; "))) } + if len(name) > validation.DNS1035LabelMaxLength { + allErr = append(allErr, fmt.Errorf("PickFixed cluster name %s cannot have length exceeding %d", name, validation.DNS1035LabelMaxLength)) + } if _, ok := uniqueClusterNames[name]; ok { allErr = append(allErr, fmt.Errorf("cluster names must be unique for policy type %s", placementv1beta1.PickFixedPlacementType)) break diff --git a/pkg/utils/validator/clusterresourceplacement_test.go b/pkg/utils/validator/clusterresourceplacement_test.go index da9a2b911..c3adfd970 100644 --- a/pkg/utils/validator/clusterresourceplacement_test.go +++ b/pkg/utils/validator/clusterresourceplacement_test.go @@ -517,10 +517,18 @@ func TestValidateClusterResourcePlacement_PickFixedPlacementPolicy(t *testing.T) "invalid placement policy - PickFixed with invalid cluster names": { policy: &placementv1beta1.PlacementPolicy{ PlacementType: placementv1beta1.PickFixedPlacementType, - ClusterNames: []string{"test@,cluster1", "test$cluster2"}, + ClusterNames: []string{"test@,cluster1"}, }, wantErr: true, - wantErrMsg: "PickFixed cluster name test@,cluster1 is not a valid member name: a lowercase RFC 1123 label must consist of lower case alphanumeric characters or '-', and must start and end with an alphanumeric character (e.g. 'my-name', or '123-abc', regex used for validation is '[a-z0-9]([-a-z0-9]*[a-z0-9])?'), PickFixed cluster name test$cluster2 is not a valid member name: a lowercase RFC 1123 label must consist of lower case alphanumeric characters or '-', and must start and end with an alphanumeric character (e.g. 'my-name', or '123-abc', regex used for validation is '[a-z0-9]([-a-z0-9]*[a-z0-9])?')", + wantErrMsg: "PickFixed cluster name test@,cluster1 is not a valid member name: 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])?)*')", + }, + "invalid placement policy - PickFixed with too long cluster name": { + policy: &placementv1beta1.PlacementPolicy{ + PlacementType: placementv1beta1.PickFixedPlacementType, + ClusterNames: []string{"this-is-a-very-long-cluster-name-that-exceeds-the-maximum-allowed-length-for-dns-labels"}, + }, + wantErr: true, + wantErrMsg: "PickFixed cluster name this-is-a-very-long-cluster-name-that-exceeds-the-maximum-allowed-length-for-dns-labels cannot have length exceeding 63", }, "invalid placement policy - PickFixed with non nil number of clusters": { policy: &placementv1beta1.PlacementPolicy{