From cfb453e6d7e6e562e2ded210f9fa81db891282b6 Mon Sep 17 00:00:00 2001
From: Ben Luddy <bluddy@redhat.com>
Date: Thu, 2 May 2024 12:00:12 -0400
Subject: [PATCH] Demonstrate that admission cannot ensure field uniqueness.

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.
---
 test/e2e/cluster_extension_admission_test.go | 68 ++++++++++++++++++++
 1 file changed, 68 insertions(+)

diff --git a/test/e2e/cluster_extension_admission_test.go b/test/e2e/cluster_extension_admission_test.go
index bfa58d4c5..04719a8a5 100644
--- a/test/e2e/cluster_extension_admission_test.go
+++ b/test/e2e/cluster_extension_admission_test.go
@@ -3,15 +3,19 @@ package e2e
 import (
 	"context"
 	"fmt"
+	"net/http"
+	"sync"
 	"testing"
 
 	"github.com/stretchr/testify/require"
 	"k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
+	"k8s.io/client-go/rest"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	ocv1alpha1 "github.com/operator-framework/operator-controller/api/v1alpha1"
+	"github.com/operator-framework/operator-controller/pkg/scheme"
 )
 
 func TestClusterExtensionPackageUniqueness(t *testing.T) {
@@ -96,3 +100,67 @@ func TestClusterExtensionPackageUniqueness(t *testing.T) {
 	}
 	require.NoError(t, c.Patch(ctx, intent, client.Apply, client.ForceOwnership, fieldOwner))
 }
+
+type synchronizedRoundTripper struct {
+	ready    <-chan struct{}
+	delegate http.RoundTripper
+}
+
+func (rt synchronizedRoundTripper) RoundTrip(r *http.Request) (*http.Response, error) {
+	// not necessary to reproduce but improves the odds
+	<-rt.ready
+	return rt.delegate.RoundTrip(r)
+}
+
+func TestClusterExtensionPackageUniquenessConsistency(t *testing.T) {
+	ctx := context.Background()
+
+	if err := c.DeleteAllOf(ctx, &ocv1alpha1.ClusterExtension{}); err != nil {
+		t.Fatal(err)
+	}
+
+	cfg := rest.CopyConfig(cfg)
+	ready := make(chan struct{})
+	cfg.Wrap(func(rt http.RoundTripper) http.RoundTripper {
+		return synchronizedRoundTripper{delegate: rt, ready: ready}
+	})
+
+	var wg sync.WaitGroup
+	for i := 0; i < 100; i++ {
+		wg.Add(1)
+		go func(i int) {
+			defer wg.Done()
+			c, err := client.New(cfg, client.Options{Scheme: scheme.Scheme})
+			if err != nil {
+				panic(err)
+			}
+
+			_ = c.Create(ctx, &ocv1alpha1.ClusterExtension{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: fmt.Sprintf("ext-%d", i),
+				},
+				Spec: ocv1alpha1.ClusterExtensionSpec{
+					PackageName: "pkg-x",
+				},
+			})
+		}(i)
+	}
+	close(ready)
+	wg.Wait()
+
+	var l ocv1alpha1.ClusterExtensionList
+	if err := c.List(ctx, &l); err != nil {
+		t.Fatal(err)
+	}
+
+	counts := make(map[string]int)
+	for _, ext := range l.Items {
+		counts[ext.Spec.PackageName]++
+	}
+
+	for pkg, count := range counts {
+		if count > 1 {
+			t.Errorf("duplicate package name: %s (%d duplicates)", pkg, count)
+		}
+	}
+}