From 2a87b1dce57c1d3e08c92a4da2e0cb560f0eccaf Mon Sep 17 00:00:00 2001 From: Sameer Kulkarni Date: Wed, 15 Jul 2026 07:18:04 -0700 Subject: [PATCH] fix(operator): cap discovery RBAC label values at 63 chars Keep k8s discovery ServiceAccount, Role, and RoleBinding resource names unchanged while capping only app.kubernetes.io/name label values with a deterministic hash suffix when needed. Add boundary tests to cover unchanged <=63 values and capped values across all three resources. Signed-off-by: Sameer Kulkarni Signed-off-by: Sameer Kulkarni --- .../operator/internal/discovery/resource.go | 33 ++++++- .../internal/discovery/resource_test.go | 91 +++++++++++++++++++ 2 files changed, 121 insertions(+), 3 deletions(-) create mode 100644 deploy/operator/internal/discovery/resource_test.go diff --git a/deploy/operator/internal/discovery/resource.go b/deploy/operator/internal/discovery/resource.go index fa0d7745a60c..d87c09322746 100644 --- a/deploy/operator/internal/discovery/resource.go +++ b/deploy/operator/internal/discovery/resource.go @@ -6,6 +6,8 @@ package discovery import ( + "crypto/sha256" + "encoding/hex" "fmt" corev1 "k8s.io/api/core/v1" @@ -18,6 +20,8 @@ const ( apiGroupRBAC = "rbac.authorization.k8s.io" apiGroupCore = "" apiGroupNvidia = "nvidia.com" + maxLabelValueLen = 63 + hashLen = 8 ) func GetK8sDiscoveryServiceAccountName(dgdName string) string { @@ -26,6 +30,7 @@ func GetK8sDiscoveryServiceAccountName(dgdName string) string { func GetK8sDiscoveryServiceAccount(dgdName string, namespace string) *corev1.ServiceAccount { name := GetK8sDiscoveryServiceAccountName(dgdName) + labelValue := getK8sDiscoveryLabelValue(name) return &corev1.ServiceAccount{ ObjectMeta: metav1.ObjectMeta{ Name: name, @@ -33,7 +38,7 @@ func GetK8sDiscoveryServiceAccount(dgdName string, namespace string) *corev1.Ser Labels: map[string]string{ "app.kubernetes.io/managed-by": "dynamo-operator", "app.kubernetes.io/component": "rbac", - "app.kubernetes.io/name": name, + "app.kubernetes.io/name": labelValue, }, }, } @@ -41,6 +46,7 @@ func GetK8sDiscoveryServiceAccount(dgdName string, namespace string) *corev1.Ser func GetK8sDiscoveryRole(dgdName string, namespace string) *rbacv1.Role { name := GetK8sDiscoveryServiceAccountName(dgdName) + labelValue := getK8sDiscoveryLabelValue(name) roleName := name + "-role" return &rbacv1.Role{ ObjectMeta: metav1.ObjectMeta{ @@ -49,7 +55,7 @@ func GetK8sDiscoveryRole(dgdName string, namespace string) *rbacv1.Role { Labels: map[string]string{ "app.kubernetes.io/managed-by": "dynamo-operator", "app.kubernetes.io/component": "rbac", - "app.kubernetes.io/name": name, + "app.kubernetes.io/name": labelValue, }, }, Rules: []rbacv1.PolicyRule{ @@ -74,6 +80,7 @@ func GetK8sDiscoveryRole(dgdName string, namespace string) *rbacv1.Role { func GetK8sDiscoveryRoleBinding(dgdName, namespace string) *rbacv1.RoleBinding { name := GetK8sDiscoveryServiceAccountName(dgdName) + labelValue := getK8sDiscoveryLabelValue(name) roleName := name + "-role" bindingName := name + "-binding" return &rbacv1.RoleBinding{ @@ -83,7 +90,7 @@ func GetK8sDiscoveryRoleBinding(dgdName, namespace string) *rbacv1.RoleBinding { Labels: map[string]string{ "app.kubernetes.io/managed-by": "dynamo-operator", "app.kubernetes.io/component": "rbac", - "app.kubernetes.io/name": name, + "app.kubernetes.io/name": labelValue, }, }, Subjects: []rbacv1.Subject{ @@ -100,3 +107,23 @@ func GetK8sDiscoveryRoleBinding(dgdName, namespace string) *rbacv1.RoleBinding { }, } } + +// getK8sDiscoveryLabelValue returns the app.kubernetes.io/name value shared by +// the discovery ServiceAccount, Role, and RoleBinding. It is derived from the +// SA name (not each resource's own name) so all three carry the same "app" +// identifier, and is capped to Kubernetes' 63-char label-value limit via +// deterministic SHA-256 suffix truncation when necessary. +func getK8sDiscoveryLabelValue(serviceAccountName string) string { + if len(serviceAccountName) <= maxLabelValueLen { + return serviceAccountName + } + + sum := sha256.Sum256([]byte(serviceAccountName)) + hash := hex.EncodeToString(sum[:hashLen/2]) + keep := maxLabelValueLen - len(hash) - 1 + if keep <= 0 { + return hash + } + + return serviceAccountName[:keep] + "-" + hash +} diff --git a/deploy/operator/internal/discovery/resource_test.go b/deploy/operator/internal/discovery/resource_test.go new file mode 100644 index 000000000000..73c98264ddc8 --- /dev/null +++ b/deploy/operator/internal/discovery/resource_test.go @@ -0,0 +1,91 @@ +/* + * SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +package discovery + +import ( + "strings" + "testing" +) + +func TestGetK8sDiscoveryLabelValue(t *testing.T) { + t.Parallel() + + tests := map[string]struct { + input string + want string + wantSame bool + wantMaxLen int + wantHashPart bool + }{ + "short value unchanged": { + input: "short-name", + wantSame: true, + wantMaxLen: maxLabelValueLen, + }, + "boundary value unchanged": { + input: strings.Repeat("a", maxLabelValueLen), + wantSame: true, + wantMaxLen: maxLabelValueLen, + }, + "over limit truncated with hash": { + // sha256(64 * 'b') starts with a0fab137; expected value is + // 54 'b' chars + '-' + first 8 hex chars of that digest. + input: strings.Repeat("b", maxLabelValueLen+1), + want: strings.Repeat("b", maxLabelValueLen-hashLen-1) + "-a0fab137", + wantSame: false, + wantMaxLen: maxLabelValueLen, + wantHashPart: true, + }, + } + + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + t.Parallel() + + got := getK8sDiscoveryLabelValue(tc.input) + + if tc.want != "" && got != tc.want { + t.Fatalf("getK8sDiscoveryLabelValue(%q) = %q, want %q", tc.input, got, tc.want) + } + if tc.wantSame && got != tc.input { + t.Fatalf("getK8sDiscoveryLabelValue(%q) = %q, want unchanged", tc.input, got) + } + if !tc.wantSame && got == tc.input { + t.Fatalf("getK8sDiscoveryLabelValue(%q) should change for over-limit values", tc.input) + } + if len(got) > tc.wantMaxLen { + t.Fatalf("len(%q) = %d, want <= %d", got, len(got), tc.wantMaxLen) + } + if tc.wantHashPart && !strings.Contains(got, "-") { + t.Fatalf("getK8sDiscoveryLabelValue(%q) = %q, want hash separator", tc.input, got) + } + if again := getK8sDiscoveryLabelValue(tc.input); again != got { + t.Fatalf("getK8sDiscoveryLabelValue(%q) not deterministic: first=%q second=%q", tc.input, got, again) + } + }) + } +} + +func TestGetK8sDiscoveryResourcesUseCappedLabel(t *testing.T) { + t.Parallel() + + dgdName := strings.Repeat("a", 80) + namespace := "default" + + sa := GetK8sDiscoveryServiceAccount(dgdName, namespace) + role := GetK8sDiscoveryRole(dgdName, namespace) + rb := GetK8sDiscoveryRoleBinding(dgdName, namespace) + + for _, label := range []string{ + sa.Labels["app.kubernetes.io/name"], + role.Labels["app.kubernetes.io/name"], + rb.Labels["app.kubernetes.io/name"], + } { + if len(label) > maxLabelValueLen { + t.Fatalf("label %q length=%d, want <= %d", label, len(label), maxLabelValueLen) + } + } +}