fix(util): prevent empty label value in GetOwnerLabel for 126-char resource names#387
Open
mdryaan wants to merge 1 commit into
Open
fix(util): prevent empty label value in GetOwnerLabel for 126-char resource names#387mdryaan wants to merge 1 commit into
mdryaan wants to merge 1 commit into
Conversation
…label value on exact multiples of 63 Signed-off-by: mdryaan <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
util.GetOwnerLabelsplits resource names longer than 63 characters across multiple label keys using manual index arithmetic. A post-loop assignment at line 245 ran unconditionally after the innerbreak, assigningcompleteResourceName[j:]to a new label key even whenj == len(completeResourceName)— which happens whenever the input length is an exact multiple of 63 (126, 189, 252, …). This produced a label key with an empty string value. Kubernetes accepts empty label values, so the resource was created without error, but any downstream label selector that expected the full split name would silently fail to match. The fix is a single guard:if j < len(completeResourceName). This PR also adds the first test file in theutil/package, with five table-driven cases covering all splitting branches including the edge case.Fixes #386
How Has This Been Tested?
Created
util/reconciliation_utility_test.gowithTestGetOwnerLabelcovering: name ≤ 63 chars, name = 63 chars, name = 64 chars (one overflow), name = 126chars (the edge case — previously produced a third label key with empty value), and name = 127 chars (three labels, all non-empty). All five subtests pass with
go test ./util/... -v -run TestGetOwnerLabel.go build ./...passes.Before fix
After fix
Checklist:
Does this PR introduce a breaking change for other components like worker-operator?
No — the fix only affects resource names whose byte length is an exact multiple of 63. Previously those names produced a spurious empty-value label that would never match any selector; after the fix they do not. Any selector already working correctly is unaffected.