kubeslice / kubeslice/kubeslice-controller
Bug: HasPrefix in util/common.go checks the last 2 characters, not the prefix
- Dominant language
- Go
- Stars
- 73
- Forks
- 48
- Avg merge
- 2d 21h
- Merged PRs (30d)
- 8
Description
### 📜 Description
`util.HasPrefix(subnet, prefix)` is named to check if a string starts with a given prefix, but its implementation slices the last 2 characters of `subnet` and compares them to `prefix`:
```go
func HasPrefix(subnet string, prefix string) bool {
return subnet[len(subnet)-2:] == prefix
}
```
The function has one caller in `service/slice_config_webhook_validation.go:267`:
```go
if !util.HasPrefix(sliceConfig.Spec.SliceSubnet, "16") {
return field.Invalid(..., "prefix must be 16")
}
```
This call passes `"16"` intending to check that the CIDR mask is `/16`. It
works today only by coincidence — the last 2 characters of `"10.0.0.0/16"`
happen to be `"16"`. The intent is a suffix check (`/16`), not a prefix check,
and the function name communicates the opposite of what it does.
### 👟 Reproduction steps
```go
util.HasPrefix("10.0.0.0/16", "/16") // Returns false (wrong — slices "16", not "/16")
util.HasPrefix("10.0.0.0/16", "10.") // Returns false (wrong — this IS a prefix)
util.HasPrefix("10.0.0.0/16", "16") // Returns true (coincidence — last 2 chars match)
util.HasPrefix("10.0.0.0/8", "16") // Returns false (mask is /8, but last 2 chars are "/8")
```
### 👍 Expected behavior
The CIDR mask validation should correctly check that the subnet ends with `/16` using `strings.HasSuffix`. `util.HasPrefix` should either be corrected to use `strings.HasPrefix` or removed since its only caller doesn't need it.
### 👎 Actual Behavior
The function checks the last 2 bytes of `subnet` regardless of `prefix` length, ignoring what `prefix` actually contains. The caller passes `"16"` instead of `"/16"`, which works only because `/16` subnets happen to end in `"16"`.
### 🐚 Relevant log output
```shell
```
### Version
_No response_
### 🖥️ What operating system are you seeing the problem on?
_No response_
### ✅ Proposed Solution
Fix `util.HasPrefix` to use `strings.HasPrefix(subnet, prefix)` and update the caller in `slice_config_webhook_validation.go` to use `strings.HasSuffix(subnet, "/16")` directly, which expresses the actual intent.
## 👀 Have you spent some time to check if this issue has been raised before?
- [x] I checked and didn't find any similar issue
## Code of Conduct
- [x] I agree to follow this project's Code of Conduct
### 👀 Have you spent some time to check if this issue has been raised before?
- [x] I checked and didn't find any similar issue
### Code of Conduct
- [x] I agree to follow this project's Code of Conduct
Contributor guide
Research direction
Start in util/common.go with HasPrefix, then inspect its sole caller at service/slice_config_webhook_validation.go:267. Reproduce the listed prefix and CIDR cases and make the completed behavior distinguish a true prefix check from the caller's intended /16 suffix validation.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- networking
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 72/100