fix: truncate proposal name in label values to 63 chars#196
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (8)
🔗 Linked repositories identifiedCodeRabbit considers these linked repositories for cross-repo context during reviews:
📝 WalkthroughSummary by CodeRabbitRelease Notes
Walkthrough
ChangesLabelProposal truncation
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
The operator uses proposal.Name directly as a Kubernetes label value (agentic.openshift.io/proposal) when creating result CRs, RBAC resources, sandbox pods, and SandboxClaim CRs. Kubernetes label values are limited to 63 characters, so names exceeding this limit cause resource creation failures. Apply truncateK8sName() to proposalName in all four label-setting locations as defense in depth. Signed-off-by: Tomáš Remeš <tremes@redhat.com> Assisted-by: Claude Code:claude-opus-4-6
tremes
force-pushed
the
truncate-proposal-name
branch
from
be08812 to
f3388b1
Compare
| func rbacLabels(proposalName, component string) map[string]string { | ||
| return map[string]string{ | ||
| LabelProposal: proposalName, | ||
| LabelProposal: truncateK8sName(proposalName), |
There was a problem hiding this comment.
This is really about the truncateK8sName helper a few lines up (around line 216) rather than this line exactly, but it's the same concern this PR is fixing so I'll flag it here.
The helper does strings.TrimRight(name[:63], "-"), which only strips a trailing -. Since a Proposal is namespaced, its metadata.name is a DNS-1123 subdomain, so it can also contain ., and a few of the derived names mix in _. A label value has to start and end with an alphanumeric, so if the 63 byte cut happens to land right after a . or _, we'd still produce an invalid label value and hit the exact failure this PR is trying to prevent.
Could we widen the cutset to cover all of them?
name = strings.TrimRight(name[:63], "-._")Because TrimRight takes a cutset, this cleans up any trailing run of those characters, which is the full set that's legal in the middle of a name but not at the boundary.
It might also be worth adding a truncation test case that ends on one of these, something like strings.Repeat("a", 62) + ".bb", since the current Repeat("a", 80) inputs never exercise a trailing non alphanumeric, so the new behavior would otherwise ship without coverage.
Kubernetes label values must start and end with alphanumeric characters. The previous implementation only trimmed trailing hyphens after truncation, but dots and underscores are also invalid at label value boundaries. Expand the TrimRight cutset to "-._" and add test cases. Signed-off-by: Tomáš Remeš <tremes@redhat.com> Assisted-by: Claude Code:claude-opus-4-6
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
|
@tremes: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
2 participants
The operator uses proposal.Name directly as a Kubernetes label value (agentic.openshift.io/proposal) when creating result CRs, RBAC resources, sandbox pods, and SandboxClaim CRs. Kubernetes label values are limited to 63 characters, so names exceeding this limit cause resource creation failures.
Apply truncateK8sName() to proposalName in all four label-setting locations as defense in depth.
Assisted-by: Claude Code:claude-opus-4-6