kubeslice / kubeslice/kubeslice-controller

Bug: HasPrefix in util/common.go checks the last 2 characters, not the prefix

Open Beginner friendly
#332 0 comments 0 reactions 0 assignees View on GitHub
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.