crossplane / crossplane/crossplane
Namespaced Usage silently fails to protect cluster-scoped resources
- Dominant language
- Go
- Stars
- 12.1k
- Forks
- 1.3k
- Avg merge
- 1d 6h
- Merged PRs (30d)
- 46
Description
### What happened?
A namespaced `Usage` that references a cluster-scoped resource in `spec.of` appears to work. The reconciler applies the `crossplane.io/in-use` label to the target resource, but the webhook doesn't actually block deletion. The protection is silently ineffective.
The root cause is an index key mismatch between the Finder and the webhook.
The Finder indexes namespaced Usages at [`finder.go:77`](https://github.com/crossplane/crossplane/blob/v2.2.0/internal/protection/usage/finder.go#L77) using the Usage's namespace as a fallback when the ref has no explicit namespace:
```go
indexVal(u.Spec.Of.APIVersion, u.Spec.Of.Kind, u.Spec.Of.ResourceRef.Name,
ptr.Deref(u.Spec.Of.ResourceRef.Namespace, u.GetNamespace()))
```
For a Usage in namespace `foo` protecting a cluster-scoped resource named `bar`, this produces the index value `.SomeClusterKind.bar.foo`.
When the webhook intercepts a DELETE of the cluster-scoped resource, `FindUsageOf` queries at [`finder.go:118`](https://github.com/crossplane/crossplane/blob/v2.2.0/internal/protection/usage/finder.go#L118) using the resource's own namespace:
```go
indexVal(o.GetAPIVersion(), o.GetKind(), o.GetName(), o.GetNamespace())
```
For a cluster-scoped resource, `GetNamespace()` returns `""`, producing the lookup key `.SomeClusterKind.bar.`. This doesn't match the indexed value. The webhook finds zero usages and allows the deletion.
This is confusing because the in-use label _is_ present on the target resource. The label gets applied because `client.Get` with a spurious namespace on a cluster-scoped resource is harmless (the API server ignores it). So the resource looks protected but isn't.
Per https://github.com/crossplane/crossplane/pull/6345 I didn't intend a namespaced `Usage` to be allowed to block deletion of cluster scoped resources. That'd allow anyone with RBAC access to create a `Usage` in any namespace to block deletion of arbitrary cluster scoped resources, which is a form of privilege escalation.
#7123 asks to support this. I've laid out some options there. Assuming we _don't_ choose to allow a namespaced `Usage` to block deletion of a cluster scoped resource, I think we should:
1. **Detect and reject the unsupported configuration.** If a namespaced Usage references a cluster-scoped resource in `spec.of`, the reconciler should surface an error condition rather than silently applying the label. We could detect this by checking whether the resolved resource's `GetNamespace()` is empty while the Usage's `GetNamespace()` is not. A validating admission webhook could also reject this at creation time by resolving the target GVK's scope via the REST mapper, which would give faster feedback than waiting for the reconciler.
2. **Don't apply the in-use label if protection won't work.** The label is misleading when the Finder can't actually find the Usage. At minimum the reconciler should not apply the label (or should remove it) when it detects the mismatch.
### How can we reproduce it?
1. Create a cluster-scoped resource (e.g. a ClusterProviderConfig)
2. Create a namespaced Usage protecting it:
```yaml
apiVersion: protection.crossplane.io/v1beta1
kind: Usage
metadata:
name: protect-cpc
namespace: default
spec:
of:
apiVersion: helm.m.crossplane.io/v1beta1
kind: ClusterProviderConfig
resourceRef:
name: my-provider-config
by:
apiVersion: helm.m.crossplane.io/v1beta1
kind: Release
resourceRef:
name: my-release
replayDeletion: true
```
3. Observe the `crossplane.io/in-use` label is applied to the ClusterProviderConfig
4. Delete the ClusterProviderConfig — deletion succeeds despite the label and the Usage
### What environment did it happen in?
Crossplane version: v2.2.0
Contributor guide
Assessment
This issue has not been assessed yet.