kubeslice / kubeslice/kubeslice-controller
Bug: CreateMinimalWorkerSliceConfig mutates shared label map, breaking gateway cleanup
- Dominant language
- Go
- Stars
- 73
- Forks
- 48
- Avg merge
- 2d 21h
- Merged PRs (30d)
- 8
Description
### 📜 Description
`CreateMinimalWorkerSliceConfig` in `service/worker_slice_config_service.go` at lines 339–342 mutates the `label` map parameter in-place inside a loop over clusters. Since Go maps are reference types, this mutation pollutes the caller's map. The caller `ReconcileSliceConfig` (`service/slice_config_service.go`) passes the same `ownershipLabel` map to both `CreateMinimalWorkerSliceConfig` (line 201) and `CreateMinimumWorkerSliceGateways` (line 207), causing the gateway cleanup to operate on a corrupted label selector.
```go
// service/worker_slice_config_service.go, lines 338-342
// Inside for _, cluster := range clusters loop:
if !found {
label["project-namespace"] = namespace
label["original-slice-name"] = name
label["worker-cluster"] = cluster // <- mutates shared map
label["kubeslice-manager"] = "controller"
```
The caller in `service/slice_config_service.go`:
```go
// Lines 186, 201, 207
ownershipLabel := util.GetOwnerLabel(completeResourceName)
// ...
clusterMap, err := s.ms.CreateMinimalWorkerSliceConfig(ctx, ..., ownershipLabel, ...)
// After this call, ownershipLabel now contains "worker-cluster" =
// ...
_, err = s.sgs.CreateMinimumWorkerSliceGateways(ctx, ..., ownershipLabel, ...)
// Gateway operations now receive a polluted label map
```
Inside `CreateMinimumWorkerSliceGateways`, the polluted `ownershipLabel` is passed to `cleanupObsoleteGateways` (line 353 of `worker_slice_gateway_service.go`), which calls:
```go
// worker_slice_gateway_service.go, line 383
gateways, err := s.ListWorkerSliceGateways(ctx, ownerLabel, namespace)
// Uses client.MatchingLabels(ownerLabel) — now filters by "worker-cluster" = last cluster
```
The list query returns only gateways matching the **last cluster** in the iteration, not all gateways for the slice. Obsolete gateways belonging to other clusters are silently skipped during cleanup.
The same mutation pattern also exists in `CreateMinimalWorkerSliceConfigForNoNetworkSlice` at lines 462–465, though the NONET code path returns immediately after (line 190 of `slice_config_service.go`) so the polluted label is not reused.
Note: This is the same class of bug as #324 (shared label map in `buildMinimumGateway`, fixed by PR #326) and the same class as the bug fixed by PR #357 (shared label map in `CreateMinimalWorkerServiceImport`), but in a different function that neither PR covers.
### 👟 Reproduction steps
1. Create a SliceConfig with 3 clusters (e.g., `cluster-a`, `cluster-b`, `cluster-c`) and a network overlay:
apiVersion: controller.kubeslice.io/v1alpha1
kind: SliceConfig
metadata:
name: test-slice
namespace: kubeslice-my-project
spec:
sliceSubnet: 10.1.0.0/16
maxClusters: 8
clusters:
- cluster-a
- cluster-b
- cluster-c
2. Wait for the slice to fully reconcile (WorkerSliceConfigs and WorkerSliceGateways created for all cluster pairs).
3. Remove `cluster-a` from the SliceConfig:
spec:
clusters:
- cluster-b
- cluster-c
4. The `ReconcileSliceConfig` runs:
- `CreateMinimalWorkerSliceConfig` iterates `[cluster-b, cluster-c]`. After the loop, `ownershipLabel["worker-cluster"]` is set to `"cluster-c"` (the last cluster).
- `CreateMinimumWorkerSliceGateways` receives this polluted label. `cleanupObsoleteGateways` lists gateways filtered by `worker-cluster=cluster-c`, missing `cluster-a`'s gateways entirely.
5. Gateways for `cluster-a` (e.g., `test-slice-cluster-a-cluster-b`, `test-slice-cluster-a-cluster-c`) are **not deleted**. They remain as orphans with stale VPN tunnels.
### 👍 Expected behavior
When a cluster is removed from a SliceConfig, `cleanupObsoleteGateways` should list **all** gateways owned by the slice (using only the ownership label, without a `worker-cluster` filter) and delete those whose source or destination cluster is no longer in the cluster list.
### 👎 Actual Behavior
`cleanupObsoleteGateways` receives a label map polluted with `"worker-cluster" = `. It only lists gateways for that specific cluster, missing gateways for all other clusters. Obsolete gateways for removed clusters are never cleaned up.
Downstream effects:
- Orphaned `WorkerSliceGateway` resources remain in the project namespace
- VPN certificate secrets for the orphaned gateways are not deleted
- The worker-operator on other clusters may continue attempting to establish VPN tunnels to the removed cluster
- Repeated cluster add/remove cycles accumulate orphaned gateway resources
### 🐚 Relevant log output
```shell
No error is logged. The gateway cleanup silently skips non-matching gateways.
The only indication is that WorkerSliceGateway resources for the removed cluster
remain in the namespace after reconciliation completes:
$ kubectl get workerslicegateways -n kubeslice-my-project -l original-slice-name=test-slice
NAME AGE
test-slice-cluster-a-cluster-b 10m <- should have been deleted
test-slice-cluster-a-cluster-c 10m <- should have been deleted
test-slice-cluster-b-cluster-c 10m <- correctly retained
test-slice-cluster-c-cluster-b 10m <- correctly retained
```
### Version
master branch (latest HEAD as of 2026-05-15). The bug exists since the initial implementation of `CreateMinimalWorkerSliceConfig`.
### 🖥️ What operating system are you seeing the problem on?
_No response_
### ✅ Proposed Solution
Clone the label map at the top of `CreateMinimalWorkerSliceConfig` before any mutation. Apply at `service/worker_slice_config_service.go`, at the start of the function (after line 305):
```go
// Clone label map to avoid mutating the caller's map
label = util.CloneStringMap(label)
```
If `util.CloneStringMap` does not exist, a simple inline clone works:
```go
clonedLabel := make(map[string]string, len(label))
for k, v := range label {
clonedLabel[k] = v
}
label = clonedLabel
```
Apply the same fix to `CreateMinimalWorkerSliceConfigForNoNetworkSlice` (after line 445) for consistency, even though the NONET path does not currently reuse the label after the call.
This matches the approach used in PR #326 for `buildMinimumGateway`.
### 👀 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
Assessment
This issue has not been assessed yet.