Merge pull request #196 from tremes/truncate-proposal-name

fix: truncate proposal name in label values to 63 chars

Author: blublinsky

controller/proposal/bare_pod_manager.go

@@ -76,7 +76,7 @@ func (m *BarePodManager) Claim(ctx context.Context, proposalName, step, _ string
 			Name:      podName,
 			Namespace: m.Namespace,
 			Labels: map[string]string{
-				LabelProposal: proposalName,
+				LabelProposal: truncateK8sName(proposalName),
 				LabelStep:     step,
 			},
 		},

controller/proposal/bare_pod_manager_test.go

@@ -2,6 +2,7 @@ package proposal
 
 import (
 	"context"
+	"strings"
 	"testing"
 	"time"
 
@@ -80,6 +81,32 @@ func TestBarePodManager_Claim_UsesPerProposalSA(t *testing.T) {
 	}
 }
 
+func TestBarePodManager_Claim_TruncatesLongProposalNameInLabel(t *testing.T) {
+	fc := newBarePodClient().Build()
+	builder := &PodSpecBuilder{Image: "quay.io/test/sandbox:latest"}
+	m := NewBarePodManager(fc, builder, "test-ns")
+	m.SetStep(
+		&agenticv1alpha1.Agent{Spec: agenticv1alpha1.AgentSpec{Model: "claude-opus-4-6"}},
+		testLLMProvider(agenticv1alpha1.LLMProviderAnthropic),
+		nil,
+		defaultSandboxSA,
+	)
+
+	longName := strings.Repeat("a", 80)
+	name, err := m.Claim(context.Background(), longName, "analysis", "")
+	if err != nil {
+		t.Fatalf("Claim: %v", err)
+	}
+
+	var pod corev1.Pod
+	if err := fc.Get(context.Background(), types.NamespacedName{Name: name, Namespace: "test-ns"}, &pod); err != nil {
+		t.Fatalf("pod not created: %v", err)
+	}
+	if len(pod.Labels[LabelProposal]) > 63 {
+		t.Fatalf("proposal label length %d exceeds 63", len(pod.Labels[LabelProposal]))
+	}
+}
+
 func TestBarePodManager_Claim_AlreadyExists(t *testing.T) {
 	existing := &corev1.Pod{}
 	existing.Name = "ls-analysis-my-proposal"

controller/proposal/rbac.go

@@ -213,7 +213,7 @@ func rbacTargetNamespaces(proposal *agenticv1alpha1.Proposal, rbacResult *agenti
 
 func truncateK8sName(name string) string {
 	if len(name) > 63 {
-		name = strings.TrimRight(name[:63], "-")
+		name = strings.TrimRight(name[:63], "-._")
 	}
 	return name
 }
@@ -228,7 +228,7 @@ func clusterRoleName(proposalName string) string {
 
 func rbacLabels(proposalName, component string) map[string]string {
 	return map[string]string{
-		LabelProposal:  proposalName,
+		LabelProposal:  truncateK8sName(proposalName),
 		LabelComponent: component,
 	}
 }

controller/proposal/rbac_test.go

@@ -477,6 +477,9 @@ func TestTruncateK8sName(t *testing.T) {
 		{"exactly_63", strings.Repeat("a", 63), strings.Repeat("a", 63)},
 		{"over_63", strings.Repeat("a", 70), strings.Repeat("a", 63)},
 		{"trailing_dash_trimmed", strings.Repeat("a", 60) + "---" + strings.Repeat("b", 5), strings.Repeat("a", 60)},
+		{"trailing_dot_trimmed", strings.Repeat("a", 60) + "..." + strings.Repeat("b", 5), strings.Repeat("a", 60)},
+		{"trailing_underscore_trimmed", strings.Repeat("a", 60) + "___" + strings.Repeat("b", 5), strings.Repeat("a", 60)},
+		{"trailing_mixed_trimmed", strings.Repeat("a", 58) + "-._.-" + strings.Repeat("b", 5), strings.Repeat("a", 58)},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
@@ -778,3 +781,14 @@ func TestRBACLabels(t *testing.T) {
 		t.Fatalf("expected 2 labels, got %d", len(labels))
 	}
 }
+
+func TestRBACLabels_TruncatesLongProposalName(t *testing.T) {
+	longName := strings.Repeat("a", 80)
+	labels := rbacLabels(longName, "execution-rbac")
+	if len(labels[LabelProposal]) > 63 {
+		t.Fatalf("proposal label length %d exceeds 63", len(labels[LabelProposal]))
+	}
+	if labels[LabelProposal] != strings.Repeat("a", 63) {
+		t.Errorf("proposal label = %q, want %q", labels[LabelProposal], strings.Repeat("a", 63))
+	}
+}

controller/proposal/results.go

@@ -36,7 +36,7 @@ func proposalOwnerRef(proposal *agenticv1alpha1.Proposal) metav1.OwnerReference
 
 func resultLabels(proposalName, step string) map[string]string {
 	return map[string]string{
-		LabelProposal: proposalName,
+		LabelProposal: truncateK8sName(proposalName),
 		LabelStep:     step,
 	}
 }

controller/proposal/results_test.go

@@ -2,6 +2,7 @@ package proposal
 
 import (
 	"context"
+	"strings"
 	"testing"
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -11,6 +12,20 @@ import (
 	agenticv1alpha1 "github.com/openshift/lightspeed-agentic-operator/api/v1alpha1"
 )
 
+func TestResultLabels_TruncatesLongProposalName(t *testing.T) {
+	longName := strings.Repeat("a", 80)
+	labels := resultLabels(longName, "analysis")
+	if len(labels[LabelProposal]) > 63 {
+		t.Fatalf("proposal label length %d exceeds 63", len(labels[LabelProposal]))
+	}
+	if labels[LabelProposal] != strings.Repeat("a", 63) {
+		t.Errorf("proposal label = %q, want %q", labels[LabelProposal], strings.Repeat("a", 63))
+	}
+	if labels[LabelStep] != "analysis" {
+		t.Errorf("step label = %q, want analysis", labels[LabelStep])
+	}
+}
+
 func TestCreateIdempotent_StatusFieldsWritten(t *testing.T) {
 	scheme := testScheme()
 	fc := fake.NewClientBuilder().WithScheme(scheme).

controller/proposal/sandbox.go

@@ -80,7 +80,7 @@ func (m *SandboxManager) buildClaim(claimName, proposalName, step, templateName
 				"name":      claimName,
 				"namespace": m.Namespace,
 				"labels": map[string]any{
-					LabelProposal: proposalName,
+					LabelProposal: truncateK8sName(proposalName),
 					LabelStep:     step,
 				},
 			},

controller/proposal/sandbox_test.go

@@ -109,6 +109,20 @@ func TestBuildClaim_Labels(t *testing.T) {
 	}
 }
 
+func TestBuildClaim_TruncatesLongProposalName(t *testing.T) {
+	longName := strings.Repeat("a", 80)
+	m := NewSandboxManager(nil, "ns", "")
+	claim := m.buildClaim("c", longName, "execution", "tpl")
+
+	labels := claim.GetLabels()
+	if len(labels[LabelProposal]) > 63 {
+		t.Fatalf("proposal label length %d exceeds 63", len(labels[LabelProposal]))
+	}
+	if labels[LabelProposal] != strings.Repeat("a", 63) {
+		t.Errorf("proposal label = %q, want %q", labels[LabelProposal], strings.Repeat("a", 63))
+	}
+}
+
 func TestBuildClaim_TemplateRef(t *testing.T) {
 	m := NewSandboxManager(nil, "ns", "")
 	claim := m.buildClaim("c", "p", "analysis", "my-template")