Skip to content

Commit

Permalink
fix: limit binding name length to 63 (#575)
Browse files Browse the repository at this point in the history
  • Loading branch information
ryanzhang-oss authored Oct 23, 2023
1 parent d0d5688 commit 385f830
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 15 deletions.
16 changes: 8 additions & 8 deletions pkg/scheduler/framework/uniquename/uniquename.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import (
)

const (
uuidLength = 6
uuidLength = 8
)

// minInt returns the smaller one of two integers.
Expand All @@ -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]
Expand All @@ -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
}
9 changes: 3 additions & 6 deletions pkg/scheduler/framework/uniquename/uniquename_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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"
Expand Down

0 comments on commit 385f830

Please sign in to comment.