🐛 Demonstrate that admission cannot ensure field uniqueness.

[benluddy]

Description

The only field you can practically guarantee uniqueness on is .metadata.name because it is used to construct the etcd key, and etcd does offer consistency. API servers can't because they can handle more than one concurrent request for the same resource and both can be past admission but before storage. There's also latency between creation and an update to the binding's param informer. The problems gets worse in HA setups when you have multiple kube-apiserver instances.

This test nearly always fails on my machine against a kind cluster set up with make run:

    cluster_extension_admission_test.go:164: duplicate package name: pkg-x (47 duplicates)
--- FAIL: TestClusterExtensionPackageUniquenessConsistency (0.13s)

Reviewer Checklist

• API Go Documentation
• Tests: Unit Tests (and E2E Tests, if appropriate)
• Comprehensive Commit Messages
• Links to related GitHub Issue(s)

[netlify]

✅ Deploy Preview for olmv1 ready!

Name	Link
🔨 Latest commit	cfb453e
🔍 Latest deploy log	https://app.netlify.com/sites/olmv1/deploys/6633bc67ff8912000800d6a1
😎 Deploy Preview	https://deploy-preview-819--olmv1.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site configuration.

[dinhxuanvu]

@joelanford @m1kola FYI

[benluddy]

https://github.com/operator-framework/operator-controller/actions/runs/8927081787/job/24519632591?pr=819#step:4:393

 === RUN   TestClusterExtensionPackageUniquenessConsistency
    cluster_extension_admission_test.go:163: duplicate package name: pkg-x (18 duplicates)
--- FAIL: TestClusterExtensionPackageUniquenessConsistency (0.13s)

[joelanford]

Thanks! And also, 😭.

As our resident apiserver experts, do you have any suggestions about how to handle package uniqueness. Should we keep these best effort admission checks and then fallback to a reconciler-based check? Even then, it seems like we'd need to have some synchorization going on to avoid similar problems detecting dups in a reconciler.

[benluddy]

If you must guarantee that at most one ClusterExtension resource can have a certain package name, the only practical option is to validate that ClusterExtension's metadata.name is the same as spec.packageName. If you do that, duplicate name conflicts come from the etcd level and will be consistent.

The prior art on this that comes to mind is name validation for CustomResourceDefinitions and the singleton pattern (validating that metadata.name matches some constant string).

[joelanford]

I think that's what OCP platform-operators does. I need to see if all package names validate as metadata.name though.

[m1kola]

Thanks for the demo and explanation. At some point I was wondering about issues of this sort. My bad for not digging deeper.

[openshift-merge-robot]

PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[varshaprasad96]

Please hold this from merging till #846 gets in. Thanks!