Skip to content

util.GetOwnerLabel has no tests despite complex splitting logic and a known edge caseΒ #386

Description

@mdryaaan

πŸ“œ Description

util.GetOwnerLabel splits resource names longer than 63 characters acrossmultiple label keys using manual index arithmetic. When the resource name length is an exact multiple of 63 (126, 189, 252, …), the post-loop assignment at
line 245 runs unconditionally after the inner break, assigningcompleteResourceName[j:] to a new label key even when
j == len(completeResourceName). This produces a label key with an empty string value. Kubernetes accepts empty label values without error, so the resource is created silently, but any downstream label selector that expects the full split name will never match it.
There are also zero test files in the entire util/ package, so this edge case was undetectable by make unit-test.

πŸ‘Ÿ Reproduction steps

cd /mnt/c/Users/alikh/Desktop/Projects/kubeslice-controller
go test ./util/... -v -run TestGetOwnerLabel

The test name_exactly_126_chars_produces_exactly_two_name_labels_with_no_empty_value
fails with:

actual: map[string]string{
  "kubeslice-controller-resource-name":   "aaa...63 chars",
  "kubeslice-controller-resource-name-1": "bbb...63 chars",
  "kubeslice-controller-resource-name-2": "",
  "kubeslice-resource-owner":             "kubeslice-controller"
}

πŸ‘ Expected behavior

For a 126-character resource name, GetOwnerLabel produces exactly two name
label keys (kubeslice-controller-resource-name and
kubeslice-controller-resource-name-1), both with non-empty values. No
spurious third key is created.

πŸ‘Ž Actual Behavior

A third label key kubeslice-controller-resource-name-2 is created with an
empty string value. Label selectors using this key will silently fail to match
the resource.

🐚 Relevant log output

Image

βœ… Proposed Solution

Guard the post-loop assignment with if j < len(completeResourceName) so it
only runs when there are remaining characters to assign. Add
util/reconciliation_utility_test.go with table-driven tests covering names
shorter than 63 chars, exactly 63 chars, 64 chars, 126 chars (the edge case),
and 127 chars.

πŸ‘€ Have you spent some time to check if this issue has been raised before?

  • I checked and didn't find any similar issue

Code of Conduct

  • I agree to follow this project's Code of Conduct

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions