diff --git a/pkg/utils/namer.go b/pkg/utils/namer.go index a95a180b32..210c41b448 100644 --- a/pkg/utils/namer.go +++ b/pkg/utils/namer.go @@ -69,6 +69,12 @@ const ( // avoid terminating on an invalid character ('-'). nameLenLimit = 62 + // clusterNameEvalThreshold is the minimum required length of the clusterName section + // in the suffix of a GCE resource in order to qualify for evaluating if a given name + // belong to the current cluster. This is for minimizing chances of cluster name collision due + // matching uid prefix. + clusterNameEvalThreshold = 4 + // maxNEGDescriptiveLabel is the max length for namespace, name and // port for neg name. 63 - 5 (k8s and naming schema version prefix) // - 8 (truncated cluster id) - 8 (suffix hash) - 4 (hyphen connector) = 38 @@ -226,7 +232,7 @@ func (n *Namer) ParseName(name string) *NameComponents { if resource == urlMapPrefix { // It is possible for the loadbalancer name to have dashes in it - so we // join the remaining name parts. - lbName = strings.Join(c[2:len(c)], "-") + lbName = strings.Join(c[2:], "-") } return &NameComponents{ @@ -248,9 +254,34 @@ func (n *Namer) NameBelongsToCluster(name string) bool { if !strings.HasPrefix(name, n.prefix+"-") { return false } - clusterName := n.UID() + fullClusterName := n.UID() components := n.ParseName(name) - return components.ClusterName == clusterName + componentClusterName := components.ClusterName + + // if exact match is found, then return true immediately + // otherwise, do best effort matching as follows + if componentClusterName == fullClusterName { + return true + } + + // if the name is longer or equal to 63 charactors and the last character of the resource matches alphaNumericChar, + // it is likely that the name was truncated. + if len(name) > nameLenLimit && len(componentClusterName) > 0 && componentClusterName[len(componentClusterName)-1:] == alphaNumericChar { + componentClusterName = componentClusterName[0 : len(componentClusterName)-1] + } + + // If the name is longer or equal to 63 characters and the length of the + // cluster name parsed from the resource name is too short, ignore the resource and do + // not consider the resource managed by this cluster. This is to prevent cluster A + // accidentally GC resources from cluster B due to both clusters share the same prefix + // uid. + // For example: + // Case 1: k8s-fws-test-sandbox-50a6f22a4cd34e91-ingress-1--16a1467191ad30 + // The cluster name is 16a1467191ad30 which is longer than clusterNameEvalThreshold. + // Case 2: k8s-fws-test-sandbox-50a6f22a4cd34e91-ingress-1111111111111--10 + // The cluster name is 10 which is shorter than clusterNameEvalThreshold. + return len(componentClusterName) > clusterNameEvalThreshold && strings.HasPrefix(fullClusterName, componentClusterName) + } // IGBackend constructs the name for a backend service targeting instance groups. diff --git a/pkg/utils/namer_test.go b/pkg/utils/namer_test.go index 5af57581eb..78e33d319f 100644 --- a/pkg/utils/namer_test.go +++ b/pkg/utils/namer_test.go @@ -119,24 +119,35 @@ func TestNamerParseName(t *testing.T) { } func TestNameBelongsToCluster(t *testing.T) { - const uid = "uid1" + const uid = "0123456789abcdef" + // string with 10 characters + longKey := "0123456789" secretHash := fmt.Sprintf("%x", sha256.Sum256([]byte("test123")))[:16] - for _, prefix := range []string{defaultPrefix, "mci"} { namer := NewNamerWithPrefix(prefix, uid, "fw1") lbName := namer.LoadBalancer("key1") + // longLBName with 40 characters. Triggers truncation + longLBName := namer.LoadBalancer(strings.Repeat(longKey, 4)) // Positive cases. for _, tc := range []string{ + // short names namer.IGBackend(80), namer.InstanceGroup(), namer.TargetProxy(lbName, HTTPProtocol), namer.TargetProxy(lbName, HTTPSProtocol), namer.SSLCertName("default/my-ing", secretHash), - namer.SSLCertName("default/my-ing", secretHash), namer.ForwardingRule(lbName, HTTPProtocol), namer.ForwardingRule(lbName, HTTPSProtocol), namer.UrlMap(lbName), namer.NEG("ns", "n", int32(80)), + // long names that are truncated + namer.TargetProxy(longLBName, HTTPProtocol), + namer.TargetProxy(longLBName, HTTPSProtocol), + namer.SSLCertName(longLBName, secretHash), + namer.ForwardingRule(longLBName, HTTPProtocol), + namer.ForwardingRule(longLBName, HTTPSProtocol), + namer.UrlMap(longLBName), + namer.NEG(strings.Repeat(longKey, 3), strings.Repeat(longKey, 3), int32(88888)), } { if !namer.NameBelongsToCluster(tc) { t.Errorf("namer.NameBelongsToCluster(%q) = false, want true", tc) @@ -146,7 +157,18 @@ func TestNameBelongsToCluster(t *testing.T) { // Negative cases. namer := NewNamer(uid, "fw1") - for _, tc := range []string{"", "invalid", "not--the-right-uid"} { + // longLBName with 60 characters. Triggers truncation to eliminate cluster name suffix + longLBName := namer.LoadBalancer(strings.Repeat(longKey, 6)) + for _, tc := range []string{ + "", + "invalid", + "not--the-right-uid", + namer.TargetProxy(longLBName, HTTPProtocol), + namer.TargetProxy(longLBName, HTTPSProtocol), + namer.ForwardingRule(longLBName, HTTPProtocol), + namer.ForwardingRule(longLBName, HTTPSProtocol), + namer.UrlMap(longLBName), + } { if namer.NameBelongsToCluster(tc) { t.Errorf("namer.NameBelongsToCluster(%q) = true, want false", tc) }