kubeslice / kubeslice/kubeslice-controller

Bug: validateClusterInAnySlice allows cluster deletion on API errors

Open
#371 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

`validateClusterInAnySlice` in `service/cluster_webhook_validation.go` at lines 81–106 validates whether a cluster can be safely deleted by checking if it participates in any slice. The function calls `util.ListResources` at line 84, but the error-handling logic at lines 98–105 only executes when `err == nil`. If the API call fails, control falls through both conditionals and returns `nil` — signaling that deletion is allowed.

```go
// service/cluster_webhook_validation.go, lines 81-106
func validateClusterInAnySlice(ctx context.Context, c *controllerv1alpha1.Cluster) *field.Error {
workerSlice := &workerv1alpha1.WorkerSliceConfigList{}
label := map[string]string{"worker-cluster": c.Name}
err := util.ListResources(ctx, workerSlice, client.MatchingLabels(label), client.InNamespace(c.Namespace))

workerSliceCount := len(workerSlice.Items)
defaultWorkerSliceCount := 0
for _, slice := range workerSlice.Items {
// ... count default slices ...
}
// line 98: only enters when err == nil
if err == nil && workerSliceCount == defaultWorkerSliceCount {
return nil
}
// line 102: only enters when err == nil
if err == nil && len(workerSlice.Items) > 0 {
return field.Forbidden(field.NewPath("Cluster"), "The cluster cannot be deleted which is participating in slice config")
}
return nil // ← line 105: reached when err != nil — deletion allowed!
}
```

When `ListResources` returns an error (API server unreachable, RBAC issue, etcd timeout), `err != nil` causes both `err == nil` checks to be `false`. The function falls through to `return nil`, which tells the webhook "no validation error — proceed with deletion."

This is a safety-critical validation gate: it exists specifically to prevent deletion of clusters that are actively participating in slices. Allowing deletion on API errors defeats the purpose of the validation.

### 👟 Reproduction steps

1. Create a project with a cluster participating in a slice:

apiVersion: controller.kubeslice.io/v1alpha1
kind: SliceConfig
metadata:
name: production-slice
namespace: kubeslice-my-project
spec:
sliceSubnet: 10.1.0.0/16
clusters:
- worker-1
- worker-2

2. Induce a transient API server failure during the delete-validation webhook call. This can occur naturally during:
- etcd leader failover
- API server rolling restart
- Network partition between webhook and API server
- Resource quota or rate limiting

3. Attempt to delete `worker-1`:

kubectl delete cluster worker-1 -n kubeslice-my-project

4. The validating webhook calls `validateClusterInAnySlice`. `util.ListResources` fails with a transient error. Both `err == nil` conditions are false. The function returns `nil`.

5. The webhook admits the deletion. The cluster is removed while still participating in `production-slice`.

### 👍 Expected behavior

When `ListResources` returns an error, the webhook should reject the deletion with an internal error, following the fail-closed principle for safety-critical validations. The user can retry the deletion after the transient issue resolves.

### 👎 Actual Behavior

The webhook returns `nil` (no validation error), allowing the deletion to proceed. The cluster is deleted while it is still participating in a slice.

Downstream effects of deleting a cluster that's in an active slice:
- Worker slice gateways lose their cluster endpoint — VPN tunnels fail
- The slice's overlay network is disrupted for all remaining clusters
- WorkerSliceConfig resources for the deleted cluster become orphaned
- ServiceExportConfig and WorkerServiceImport resources referencing the cluster become stale
- Manual cleanup and re-registration is required to restore slice connectivity

### 🐚 Relevant log output

```shell
No error is logged by the webhook. The API server logs show the deletion was admitted:

I0515 12:00:00.000000 webhook/cluster admission allowed for DELETE cluster worker-1

The transient ListResources error is silently discarded — no trace of the failure
appears in controller logs, webhook logs, or audit logs.
```

### Version

master branch (latest HEAD as of 2026-05-15). The bug exists since the initial implementation of cluster delete validation.

### 🖥️ What operating system are you seeing the problem on?

_No response_

### ✅ Proposed Solution

Add an error check immediately after the `ListResources` call. Apply at `service/cluster_webhook_validation.go`, between lines 84 and 86:

Current code:

```go
err := util.ListResources(ctx, workerSlice, client.MatchingLabels(label), client.InNamespace(c.Namespace))

workerSliceCount := len(workerSlice.Items)
```

Fixed code:

```go
err := util.ListResources(ctx, workerSlice, client.MatchingLabels(label), client.InNamespace(c.Namespace))
if err != nil {
return field.InternalError(field.NewPath("Cluster"), fmt.Errorf("failed to verify cluster slice participation: %w", err))
}

workerSliceCount := len(workerSlice.Items)
```

This follows the fail-closed principle: if the webhook cannot verify the cluster's slice membership, it rejects the deletion. The user receives a clear error message and can retry after the transient issue resolves.

The redundant `err == nil` guards on lines 98 and 102 can then be simplified since `err` is guaranteed to be `nil` at that point.

### 👀 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.