Truncate long resource names when provisioning (#4387)

sjberman · web-flow · commit a1afc9d5c408 · 2025-12-04T18:18:32.000Z
Problem: When provisioning the NGINX Service, if the Gateway name was very long, it could result in a failure to create the Service. A Service name must be 63 characters or less. This also could affect the shadow Service that we create for InferencePools if the InferencePool name is long.

Solution: If the name of the resource that we create is going to be over the max, create a hash of the name and truncate to ensure that it is less than the max. The suffix is still included, since we rely on that for certain resources. Just the prefix is shortened as necessary to include the hash.
diff --git a/internal/controller/provisioner/handler.go b/internal/controller/provisioner/handler.go
@@ -67,8 +67,10 @@ func (h *eventHandler) HandleEventBatch(ctx context.Context, logger logr.Logger,
 				objLabels := labels.Set(obj.GetLabels())
 				if h.labelSelector.Matches(objLabels) {
 					gatewayName := objLabels.Get(controller.GatewayLabel)
+					if gatewayName == "" {
+						gatewayName = obj.GetAnnotations()[controller.GatewayLabel]
+					}
 					gatewayNSName := types.NamespacedName{Namespace: obj.GetNamespace(), Name: gatewayName}
-
 					if err := h.updateOrDeleteResources(ctx, logger, obj, gatewayNSName); err != nil {
 						logger.Error(err, "error handling resource update")
 					}
@@ -77,12 +79,13 @@ func (h *eventHandler) HandleEventBatch(ctx context.Context, logger logr.Logger,
 				objLabels := labels.Set(obj.GetLabels())
 				if h.labelSelector.Matches(objLabels) {
 					gatewayName := objLabels.Get(controller.GatewayLabel)
+					if gatewayName == "" {
+						gatewayName = obj.GetAnnotations()[controller.GatewayLabel]
+					}
 					gatewayNSName := types.NamespacedName{Namespace: obj.GetNamespace(), Name: gatewayName}
-
 					if err := h.updateOrDeleteResources(ctx, logger, obj, gatewayNSName); err != nil {
 						logger.Error(err, "error handling resource update")
 					}
-
 					statusUpdate := &status.QueueObject{
 						Deployment:     client.ObjectKeyFromObject(obj),
 						UpdateType:     status.UpdateGateway,
@@ -94,8 +97,10 @@ func (h *eventHandler) HandleEventBatch(ctx context.Context, logger logr.Logger,
 				objLabels := labels.Set(obj.GetLabels())
 				if h.labelSelector.Matches(objLabels) {
 					gatewayName := objLabels.Get(controller.GatewayLabel)
+					if gatewayName == "" {
+						gatewayName = obj.GetAnnotations()[controller.GatewayLabel]
+					}
 					gatewayNSName := types.NamespacedName{Namespace: obj.GetNamespace(), Name: gatewayName}
-
 					if err := h.updateOrDeleteResources(ctx, logger, obj, gatewayNSName); err != nil {
 						logger.Error(err, "error handling resource update")
 					}
diff --git a/internal/controller/provisioner/objects.go b/internal/controller/provisioner/objects.go
@@ -90,12 +90,17 @@ func (p *NginxProvisioner) buildNginxResourceObjects(
 
 	selectorLabels := make(map[string]string)
 	maps.Copy(selectorLabels, p.baseLabelSelector.MatchLabels)
-	selectorLabels[controller.GatewayLabel] = gateway.GetName()
 	selectorLabels[controller.AppNameLabel] = resourceName
 
 	labels := make(map[string]string)
 	annotations := make(map[string]string)
 
+	if len(gateway.GetName()) > controller.MaxServiceNameLen {
+		annotations[controller.GatewayLabel] = gateway.GetName()
+	} else {
+		selectorLabels[controller.GatewayLabel] = gateway.GetName()
+	}
+
 	maps.Copy(labels, selectorLabels)
 
 	if gateway.Spec.Infrastructure != nil {
diff --git a/internal/controller/state/graph/backend_refs.go b/internal/controller/state/graph/backend_refs.go
@@ -14,7 +14,6 @@ import (
 	ngfAPIv1alpha2 "github.com/nginx/nginx-gateway-fabric/v2/apis/v1alpha2"
 	"github.com/nginx/nginx-gateway-fabric/v2/internal/controller/sort"
 	"github.com/nginx/nginx-gateway-fabric/v2/internal/controller/state/conditions"
-	"github.com/nginx/nginx-gateway-fabric/v2/internal/framework/controller"
 	"github.com/nginx/nginx-gateway-fabric/v2/internal/framework/helpers"
 	"github.com/nginx/nginx-gateway-fabric/v2/internal/framework/kinds"
 )
@@ -116,7 +115,7 @@ func addBackendRefsToRules(
 				}
 
 				poolName := types.NamespacedName{
-					Name:      controller.GetInferencePoolName(string(ref.Name)),
+					Name:      ref.InferencePoolName,
 					Namespace: namespace,
 				}
 
@@ -596,7 +595,7 @@ func validateBackendRefHTTPRoute(
 			inferencePool = true
 			inferencePoolName = types.NamespacedName{
 				Namespace: string(*ref.Namespace),
-				Name:      controller.GetInferencePoolName(string(ref.Name)),
+				Name:      ref.InferencePoolName,
 			}
 		default:
 			refNsName := types.NamespacedName{Namespace: string(*ref.Namespace), Name: string(ref.Name)}
diff --git a/internal/controller/state/graph/backend_refs_test.go b/internal/controller/state/graph/backend_refs_test.go
@@ -106,7 +106,8 @@ func TestValidateRouteBackendRef(t *testing.T) {
 					backend.Name = gatewayv1.ObjectName(controller.CreateInferencePoolServiceName("ipool"))
 					return backend
 				}),
-				IsInferencePool: true,
+				IsInferencePool:   true,
+				InferencePoolName: "ipool",
 			},
 			expectedValid: true,
 		},
@@ -393,7 +394,8 @@ func TestValidateBackendRefHTTPRoute(t *testing.T) {
 					backend.Namespace = helpers.GetPointer[gatewayv1.Namespace]("invalid")
 					return backend
 				}),
-				IsInferencePool: true,
+				IsInferencePool:   true,
+				InferencePoolName: "ipool",
 			},
 			refGrantResolver: alwaysFalseRefGrantResolver,
 			expectedValid:    false,
@@ -1220,6 +1222,7 @@ func TestAddBackendRefsToRules(t *testing.T) {
 				route := createRoute("hr-inference", RouteTypeHTTP, "Service", 1, svcInferenceName)
 				// Mark the backend ref as IsInferencePool and set the port to nil (simulate InferencePool logic)
 				route.Spec.Rules[0].RouteBackendRefs[0].IsInferencePool = true
+				route.Spec.Rules[0].RouteBackendRefs[0].InferencePoolName = "ipool"
 				route.Spec.Rules[0].RouteBackendRefs[0].Port = nil
 				return route
 			}(),
diff --git a/internal/controller/state/graph/graph_test.go b/internal/controller/state/graph/graph_test.go
@@ -239,7 +239,8 @@ func TestBuildGraph(t *testing.T) {
 		}
 		rbrs := []RouteBackendRef{
 			{
-				IsInferencePool: true,
+				IsInferencePool:   true,
+				InferencePoolName: "ipool",
 				BackendRef: gatewayv1.BackendRef{
 					BackendObjectReference: gatewayv1.BackendObjectReference{
 						Group:     helpers.GetPointer[gatewayv1.Group](""),
diff --git a/internal/controller/state/graph/httproute.go b/internal/controller/state/graph/httproute.go
@@ -224,7 +224,7 @@ func processHTTPRouteRule(
 		// If route specifies an InferencePool backend, we need to convert it to its associated
 		// headless Service backend (that we created), so nginx config can be built properly.
 		// Only do this if the InferencePool actually exists.
-		if inferencePoolBackend(b, routeNamespace, inferencePools) {
+		if ok, key := inferencePoolBackend(b, routeNamespace, inferencePools); ok {
 			// We don't support traffic splitting at the Route level for
 			// InferencePool backends, so if there's more than one backendRef, and one of them
 			// is an InferencePool, we mark the rule as invalid.
@@ -239,7 +239,8 @@ func processHTTPRouteRule(
 
 			svcName := controller.CreateInferencePoolServiceName(string(b.Name))
 			rbr = RouteBackendRef{
-				IsInferencePool: true,
+				IsInferencePool:   true,
+				InferencePoolName: key.Name,
 				BackendRef: v1.BackendRef{
 					BackendObjectReference: v1.BackendObjectReference{
 						Group:     helpers.GetPointer[v1.Group](""),
@@ -346,12 +347,12 @@ func processHTTPRouteRules(
 }
 
 // inferencePoolBackend returns if a Route references an InferencePool backend
-// and that InferencePool exists.
+// and that InferencePool exists. Also returns the NamespacedName of the InferencePool.
 func inferencePoolBackend(
 	backendRef v1.HTTPBackendRef,
 	routeNamespace string,
 	inferencePools map[types.NamespacedName]*inference.InferencePool,
-) bool {
+) (bool, types.NamespacedName) {
 	if backendRef.Group != nil &&
 		*backendRef.Group == inferenceAPIGroup &&
 		*backendRef.Kind == kinds.InferencePool {
@@ -364,11 +365,11 @@ func inferencePoolBackend(
 			Namespace: namespace,
 		}
 		if _, exists := inferencePools[key]; exists {
-			return true
+			return true, key
 		}
 	}
 
-	return false
+	return false, types.NamespacedName{}
 }
 
 func validateMatch(
diff --git a/internal/controller/state/graph/httproute_test.go b/internal/controller/state/graph/httproute_test.go
@@ -1038,7 +1038,8 @@ func TestBuildHTTPRoute(t *testing.T) {
 							Matches: hrInferencePool.Spec.Rules[0].Matches,
 							RouteBackendRefs: []RouteBackendRef{
 								{
-									IsInferencePool: true,
+									IsInferencePool:   true,
+									InferencePoolName: "ipool",
 									BackendRef: gatewayv1.BackendRef{
 										BackendObjectReference: gatewayv1.BackendObjectReference{
 											Group:     helpers.GetPointer[gatewayv1.Group](""),
diff --git a/internal/controller/state/graph/inferencepools.go b/internal/controller/state/graph/inferencepools.go
@@ -10,7 +10,6 @@ import (
 	apiv1 "sigs.k8s.io/gateway-api/apis/v1"
 
 	"github.com/nginx/nginx-gateway-fabric/v2/internal/controller/state/conditions"
-	"github.com/nginx/nginx-gateway-fabric/v2/internal/framework/controller"
 	"github.com/nginx/nginx-gateway-fabric/v2/internal/framework/kinds"
 )
 
@@ -101,7 +100,7 @@ func processInferencePoolsForGateway(
 				}
 
 				poolName := types.NamespacedName{
-					Name:      controller.GetInferencePoolName(string(ref.Name)),
+					Name:      ref.InferencePoolName,
 					Namespace: namespace,
 				}
 
diff --git a/internal/controller/state/graph/inferencepools_test.go b/internal/controller/state/graph/inferencepools_test.go
@@ -50,7 +50,8 @@ func TestBuildReferencedInferencePools(t *testing.T) {
 					{
 						RouteBackendRefs: []RouteBackendRef{
 							{
-								IsInferencePool: true,
+								IsInferencePool:   true,
+								InferencePoolName: "pool",
 								BackendRef: gatewayv1.BackendRef{
 									BackendObjectReference: gatewayv1.BackendObjectReference{
 										Namespace: helpers.GetPointer[gatewayv1.Namespace]("test"),
@@ -111,7 +112,8 @@ func TestBuildReferencedInferencePools(t *testing.T) {
 			{
 				RouteBackendRefs: []RouteBackendRef{
 					{
-						IsInferencePool: true,
+						IsInferencePool:   true,
+						InferencePoolName: "pool",
 						BackendRef: gatewayv1.BackendRef{
 							BackendObjectReference: gatewayv1.BackendObjectReference{
 								Kind:      helpers.GetPointer[gatewayv1.Kind](kinds.Service),
diff --git a/internal/controller/state/graph/route_common.go b/internal/controller/state/graph/route_common.go
@@ -169,6 +169,9 @@ type RouteBackendRef struct {
 	// EndpointPickerConfig is the configuration for the EndpointPicker, if this backendRef is for an InferencePool.
 	EndpointPickerConfig EndpointPickerConfig
 
+	// InferencePoolName is the name of the InferencePool, if this backendRef is for an InferencePool.
+	InferencePoolName string
+
 	Filters []any
 
 	// IsInferencePool indicates if this backend is an InferencePool disguised as a Service.
diff --git a/internal/framework/controller/resource.go b/internal/framework/controller/resource.go
@@ -1,32 +1,46 @@
 package controller
 
 import (
-	"fmt"
-	"strings"
+	"crypto/sha256"
+	"encoding/hex"
 )
 
-// inferencePoolServiceSuffix is the suffix of the headless Service name for an InferencePool.
-const inferencePoolServiceSuffix = "-pool-svc"
+const (
+	// inferencePoolServiceSuffix is the suffix of the headless Service name for an InferencePool.
+	inferencePoolServiceSuffix = "pool-svc"
+	MaxServiceNameLen          = 63
+	hashLen                    = 8
+)
 
 // CreateNginxResourceName creates the base resource name for all nginx resources
 // created by the control plane.
 func CreateNginxResourceName(prefix, suffix string) string {
-	return fmt.Sprintf("%s-%s", prefix, suffix)
+	return truncateAndHashName(prefix, suffix)
 }
 
 // CreateInferencePoolServiceName creates the name for a headless Service that
 // we create for an InferencePool.
 func CreateInferencePoolServiceName(name string) string {
-	svcName := fmt.Sprintf("%s%s", name, inferencePoolServiceSuffix)
-	// if InferencePool name is already at or near max length, just use that name
-	if len(svcName) > 253 {
-		return name
+	return truncateAndHashName(name, inferencePoolServiceSuffix)
+}
+
+// truncateAndHashName truncates the input name to fit within maxLen,
+// appending a hash for uniqueness if needed.
+func truncateAndHashName(name string, suffix string) string {
+	sep := "-"
+	full := name + sep + suffix
+	if len(full) <= MaxServiceNameLen {
+		return full
 	}
 
-	return svcName
-}
+	// Always include the suffix, truncate name as needed
+	hash := sha256.Sum256([]byte(full))
+	hashStr := hex.EncodeToString(hash[:])[:hashLen]
+	maxNameLen := MaxServiceNameLen - (len(sep) * 2) - hashLen - len(suffix)
+	truncName := name
+	if len(name) > maxNameLen {
+		truncName = name[:maxNameLen]
+	}
 
-// GetInferencePoolName returns the name of the InferencePool for a given headless Service name.
-func GetInferencePoolName(serviceName string) string {
-	return strings.TrimSuffix(serviceName, inferencePoolServiceSuffix)
+	return truncName + sep + hashStr + sep + suffix
 }
diff --git a/internal/framework/controller/resource_test.go b/internal/framework/controller/resource_test.go
@@ -0,0 +1,86 @@
+package controller
+
+import (
+	"strings"
+	"testing"
+
+	. "github.com/onsi/gomega"
+)
+
+func TestCreateNginxResourceName(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		prefix   string
+		suffix   string
+		expected string
+		msg      string
+	}{
+		{
+			prefix:   "shortprefix",
+			suffix:   "shortsuffix",
+			expected: "shortprefix-shortsuffix",
+			msg:      "short names",
+		},
+		{
+			prefix:   strings.Repeat("a", 64),
+			suffix:   "suffix",
+			expected: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa-b9a00a3c-suffix",
+			msg:      "prefix is longer than max",
+		},
+		{
+			prefix:   strings.Repeat("b", 60),
+			suffix:   "suffix",
+			expected: "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb-1930ffb3-suffix",
+			msg:      "prefix + suffix is longer than max",
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.msg, func(t *testing.T) {
+			t.Parallel()
+			g := NewWithT(t)
+
+			name := CreateNginxResourceName(test.prefix, test.suffix)
+			g.Expect(len(name)).To(BeNumerically("<=", MaxServiceNameLen))
+			g.Expect(name).To(Equal(test.expected), "expected %q, got %q", test.expected, name)
+		})
+	}
+}
+
+func TestCreateInferencePoolServiceName(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name     string
+		expected string
+		msg      string
+	}{
+		{
+			name:     "pool",
+			expected: "pool-pool-svc",
+			msg:      "short name",
+		},
+		{
+			name:     strings.Repeat("a", 64),
+			expected: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa-b33d7b99-pool-svc",
+			msg:      "prefix is longer than max",
+		},
+		{
+			name:     strings.Repeat("b", 60),
+			expected: "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb-af3f976c-pool-svc",
+			msg:      "prefix + suffix is longer than max",
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.msg, func(t *testing.T) {
+			t.Parallel()
+			g := NewWithT(t)
+
+			serviceName := CreateInferencePoolServiceName(test.name)
+			g.Expect(len(serviceName)).To(BeNumerically("<=", MaxServiceNameLen))
+			g.Expect(serviceName).To(Equal(test.expected), "expected %q, got %q", test.expected, serviceName)
+		})
+	}
+}

Original file line number	Diff line number	Diff line change
`@@ -14,7 +14,6 @@ import (`
`14`	`14`	`ngfAPIv1alpha2 "github.com/nginx/nginx-gateway-fabric/v2/apis/v1alpha2"`
`15`	`15`	`"github.com/nginx/nginx-gateway-fabric/v2/internal/controller/sort"`
`16`	`16`	`"github.com/nginx/nginx-gateway-fabric/v2/internal/controller/state/conditions"`
`17`		`- "github.com/nginx/nginx-gateway-fabric/v2/internal/framework/controller"`
`18`	`17`	`"github.com/nginx/nginx-gateway-fabric/v2/internal/framework/helpers"`
`19`	`18`	`"github.com/nginx/nginx-gateway-fabric/v2/internal/framework/kinds"`
`20`	`19`	`)`
`@@ -116,7 +115,7 @@ func addBackendRefsToRules(`
`116`	`115`	`}`
`117`	`116`
`118`	`117`	`poolName := types.NamespacedName{`
`119`		`- Name: controller.GetInferencePoolName(string(ref.Name)),`
	`118`	`+ Name: ref.InferencePoolName,`
`120`	`119`	`Namespace: namespace,`
`121`	`120`	`}`
`122`	`121`
`@@ -596,7 +595,7 @@ func validateBackendRefHTTPRoute(`
`596`	`595`	`inferencePool = true`
`597`	`596`	`inferencePoolName = types.NamespacedName{`
`598`	`597`	`Namespace: string(*ref.Namespace),`
`599`		`- Name: controller.GetInferencePoolName(string(ref.Name)),`
	`598`	`+ Name: ref.InferencePoolName,`
`600`	`599`	`}`
`601`	`600`	`default:`
`602`	`601`	`refNsName := types.NamespacedName{Namespace: string(*ref.Namespace), Name: string(ref.Name)}`
Original file line number	Diff line number	Diff line change
`@@ -239,7 +239,8 @@ func TestBuildGraph(t *testing.T) {`
`239`	`239`	`}`
`240`	`240`	`rbrs := []RouteBackendRef{`
`241`	`241`	`{`
`242`		`- IsInferencePool: true,`
	`242`	`+ IsInferencePool: true,`
	`243`	`+ InferencePoolName: "ipool",`
`243`	`244`	`BackendRef: gatewayv1.BackendRef{`
`244`	`245`	`BackendObjectReference: gatewayv1.BackendObjectReference{`
`245`	`246`	`Group: helpers.GetPointer[gatewayv1.Group](""),`
Original file line number	Diff line number	Diff line change
`@@ -10,7 +10,6 @@ import (`
`10`	`10`	`apiv1 "sigs.k8s.io/gateway-api/apis/v1"`
`11`	`11`
`12`	`12`	`"github.com/nginx/nginx-gateway-fabric/v2/internal/controller/state/conditions"`
`13`		`- "github.com/nginx/nginx-gateway-fabric/v2/internal/framework/controller"`
`14`	`13`	`"github.com/nginx/nginx-gateway-fabric/v2/internal/framework/kinds"`
`15`	`14`	`)`
`16`	`15`
`@@ -101,7 +100,7 @@ func processInferencePoolsForGateway(`
`101`	`100`	`}`
`102`	`101`
`103`	`102`	`poolName := types.NamespacedName{`
`104`		`- Name: controller.GetInferencePoolName(string(ref.Name)),`
	`103`	`+ Name: ref.InferencePoolName,`
`105`	`104`	`Namespace: namespace,`
`106`	`105`	`}`
`107`	`106`
Original file line number	Diff line number	Diff line change
`@@ -50,7 +50,8 @@ func TestBuildReferencedInferencePools(t *testing.T) {`
`50`	`50`	`{`
`51`	`51`	`RouteBackendRefs: []RouteBackendRef{`
`52`	`52`	`{`
`53`		`- IsInferencePool: true,`
	`53`	`+ IsInferencePool: true,`
	`54`	`+ InferencePoolName: "pool",`
`54`	`55`	`BackendRef: gatewayv1.BackendRef{`
`55`	`56`	`BackendObjectReference: gatewayv1.BackendObjectReference{`
`56`	`57`	`Namespace: helpers.GetPointer[gatewayv1.Namespace]("test"),`
`@@ -111,7 +112,8 @@ func TestBuildReferencedInferencePools(t *testing.T) {`
`111`	`112`	`{`
`112`	`113`	`RouteBackendRefs: []RouteBackendRef{`
`113`	`114`	`{`
`114`		`- IsInferencePool: true,`
	`115`	`+ IsInferencePool: true,`
	`116`	`+ InferencePoolName: "pool",`
`115`	`117`	`BackendRef: gatewayv1.BackendRef{`
`116`	`118`	`BackendObjectReference: gatewayv1.BackendObjectReference{`
`117`	`119`	`Kind: helpers.GetPointer[gatewayv1.Kind](kinds.Service),`