From 385f830e48823be37afdf10a9c84ea4324a93a7c Mon Sep 17 00:00:00 2001
From: Ryan Zhang <yangzhangrice@hotmail.com>
Date: Sun, 22 Oct 2023 22:59:24 -0700
Subject: [PATCH] fix: limit binding name length to 63 (#575)

---
 pkg/scheduler/framework/uniquename/uniquename.go | 16 ++++++++--------
 .../framework/uniquename/uniquename_test.go      |  9 +++------
 .../fleetresourcehandler_webhook.go              |  2 +-
 3 files changed, 12 insertions(+), 15 deletions(-)

diff --git a/pkg/scheduler/framework/uniquename/uniquename.go b/pkg/scheduler/framework/uniquename/uniquename.go
index 12b11dcb3..baf63c3c4 100644
--- a/pkg/scheduler/framework/uniquename/uniquename.go
+++ b/pkg/scheduler/framework/uniquename/uniquename.go
@@ -15,7 +15,7 @@ import (
 )
 
 const (
-	uuidLength = 6
+	uuidLength = 8
 )
 
 // minInt returns the smaller one of two integers.
@@ -27,7 +27,7 @@ func minInt(a, b int) int {
 }
 
 // NewClusterResourceBindingName returns a unique name for a cluster resource binding in the
-// format of DNS subdomain names (RFC 1123).
+// format of DNS label names (RFC 1123). It will be used as a label on the work resource.
 //
 // The name is generated using the following format:
 // * [CRP-NAME] - [TARGET-CLUSTER-NAME] - [RANDOM-SUFFIX]
@@ -38,20 +38,20 @@ func minInt(a, b int) int {
 // of name collisions are extremely low.
 //
 // In addition, note that this function assumes that both the CRP name and the cluster name
-// are valid DNS subdomain names (RFC 1123).
+// are valid DNS label names (RFC 1123).
 func NewClusterResourceBindingName(CRPName string, clusterName string) (string, error) {
-	reservedSlots := 2 + uuidLength // 2 dashs + 6 character UUID string
+	reservedSlots := 2 + uuidLength // 2 dashs + 8 character UUID string
 
-	slotsPerSeg := (validation.DNS1123SubdomainMaxLength - reservedSlots) / 2
+	slotsPerSeg := (validation.DNS1123LabelMaxLength - reservedSlots) / 2
 	uniqueName := fmt.Sprintf("%s-%s-%s",
 		CRPName[:minInt(slotsPerSeg, len(CRPName))],
-		clusterName[:minInt(slotsPerSeg, len(clusterName))],
+		clusterName[:minInt(slotsPerSeg+1, len(clusterName))],
 		uuid.NewUUID()[:uuidLength],
 	)
 
-	if errs := validation.IsDNS1123Subdomain(uniqueName); len(errs) != 0 {
+	if errs := validation.IsDNS1123Label(uniqueName); len(errs) != 0 {
 		// Do a sanity check here; normally this would not occur.
-		return "", fmt.Errorf("failed to format a unique RFC 1123 DNS subdomain name with namespace %s, name %s: %v", CRPName, clusterName, errs)
+		return "", fmt.Errorf("failed to format a unique RFC 1123 label name with CRP name %s, cluster name %s: %v", CRPName, clusterName, errs)
 	}
 	return uniqueName, nil
 }
diff --git a/pkg/scheduler/framework/uniquename/uniquename_test.go b/pkg/scheduler/framework/uniquename/uniquename_test.go
index 76b75c4b7..e55e727aa 100644
--- a/pkg/scheduler/framework/uniquename/uniquename_test.go
+++ b/pkg/scheduler/framework/uniquename/uniquename_test.go
@@ -15,10 +15,7 @@ const (
 	crpName     = "app"
 	clusterName = "bravelion"
 
-	longName = "c7t2c6oppjnryqcihwweexeobs7tlmf08ha4qb5htc4cifzpalhb5ec2lbh3" +
-		"j73reciaz2f0jfd2rl5qba6rzuuwgyw6d9e6la19bo89k41lphln4s4dy1gr" +
-		"h1dvua17iu4ro61dxo91ayovns8cgnmshlsflmi68e3najm7dw5dqe17pih7" +
-		"up0dtyvrqxyp90sxedbf"
+	longName = "c7t2c6oppjnryqcihwweexeobs7tlmf08ha4qb5htc4cifzpalhb5ec2lbh3"
 )
 
 // TO-DO (chenyu1): Expand the test cases as development proceeds.
@@ -44,8 +41,8 @@ func TestClusterResourceBindingUniqueName(t *testing.T) {
 			name:        "valid name (truncated)",
 			crpName:     longName,
 			clusterName: longName,
-			wantPrefix:  fmt.Sprintf("%s-%s", longName[:122], longName[:122]),
-			wantLength:  252,
+			wantPrefix:  fmt.Sprintf("%s-%s", longName[:26], longName[:27]),
+			wantLength:  63,
 		},
 		{
 			name:           "invalid name",
diff --git a/pkg/webhook/fleetresourcehandler/fleetresourcehandler_webhook.go b/pkg/webhook/fleetresourcehandler/fleetresourcehandler_webhook.go
index 8911357ec..d8e00cc8b 100644
--- a/pkg/webhook/fleetresourcehandler/fleetresourcehandler_webhook.go
+++ b/pkg/webhook/fleetresourcehandler/fleetresourcehandler_webhook.go
@@ -7,7 +7,6 @@ import (
 	"regexp"
 	"strings"
 
-	fleetnetworkingv1alpha1 "go.goms.io/fleet-networking/api/v1alpha1"
 	admissionv1 "k8s.io/api/admission/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/types"
@@ -17,6 +16,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/webhook"
 	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
 
+	fleetnetworkingv1alpha1 "go.goms.io/fleet-networking/api/v1alpha1"
 	clusterv1beta1 "go.goms.io/fleet/apis/cluster/v1beta1"
 	fleetv1alpha1 "go.goms.io/fleet/apis/v1alpha1"
 	"go.goms.io/fleet/pkg/utils"