kubeslice / kubeslice/kubeslice-controller

Bug: CreateMinimalWorkerSliceConfig mutates shared label map, breaking gateway cleanup

Open
#370 0 comments 0 reactions 1 assignee Claimed by @richiesebastian View on GitHub
bug
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

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.