fix(crdvalidator): update e2e tests to be less flakey

Makefile

@@ -182,9 +182,8 @@ install-crdvalidator: build-crdvalidator-image kind-load-crdvalidator ## Install
 
 deploy-crdvalidator: cert-mgr install-crdvalidator ## Install the crdvalidator manifests to the current cluster and ensures that cert-manager is installed prior.
 
-FOCUS := $(if $(TEST),-focus "$(TEST)")
 test-crdvalidator-e2e: ginkgo ## Run the crdvalidator e2e tests. Assumes you have already installed the crdvalidator webhook.
-	$(GINKGO) -v -trace -progress $(FOCUS) cmd/crdvalidator/test/e2e
+	$(GINKGO) -trace -progress $(FOCUS) cmd/crdvalidator/test/e2e
 
 ################
 # Hack / Tools #

cmd/crdvalidator/internal/crd/crd.go

@@ -19,13 +19,15 @@ import (
 	"reflect"
 
 	"k8s.io/apiextensions-apiserver/pkg/apis/apiextensions"
-	apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
 	"k8s.io/apiextensions-apiserver/pkg/apiserver/validation"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/apimachinery/pkg/util/validation/field"
 	"sigs.k8s.io/controller-runtime/pkg/client"
+
+	apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
 )
 
 // Validate is a wrapper for doing four things:
@@ -35,7 +37,14 @@ import (
 // 	4. Reporting any errors that it encounters along the way.
 func Validate(ctx context.Context, cl client.Client, newCrd *apiextensionsv1.CustomResourceDefinition) error {
 	oldCRD := &apiextensionsv1.CustomResourceDefinition{}
-	if err := client.IgnoreNotFound(cl.Get(ctx, client.ObjectKeyFromObject(newCrd), oldCRD)); err != nil {
+
+	err := client.IgnoreNotFound(cl.Get(ctx, client.ObjectKeyFromObject(newCrd), oldCRD))
+	if apierrors.IsNotFound(err) {
+		// Return early if the CRD has not been created yet
+		// as we know it is valid.
+		return nil
+	}
+	if err != nil {
 		return err
 	}
 

cmd/crdvalidator/manifests/01_cluster_role.yaml

@@ -6,3 +6,6 @@ rules:
 - apiGroups: ["apiextensions.k8s.io"]
   resources: ["customresourcedefinitions"]
   verbs: ["get", "watch", "list"]
+- apiGroups: ["*"]
+  resources: ["*"]
+  verbs: ["list"]

cmd/crdvalidator/test/e2e/crdvalidator_suite_test.go

@@ -19,7 +19,7 @@ var (
 
 func TestE2E(t *testing.T) {
 	RegisterFailHandler(Fail)
-	SetDefaultEventuallyTimeout(1 * time.Minute)
+	SetDefaultEventuallyTimeout(30 * time.Second)
 	SetDefaultEventuallyPollingInterval(1 * time.Second)
 	RunSpecs(t, "crdvalidator E2E Suite")
 }

cmd/crdvalidator/test/e2e/crdvalidator_test.go

@@ -15,25 +15,83 @@ import (
 
 var _ = Describe("crd validation webhook", func() {
 	When("a crd event is emitted", func() {
-		var (
-			ctx   context.Context
-			v1crd *apiextensionsv1.CustomResourceDefinition
-		)
-
-		BeforeEach(func() {
-			ctx = context.Background()
-
-			name := names.SimpleNameGenerator.GenerateName("samplecrd")
-			v1crd = &apiextensionsv1.CustomResourceDefinition{
-				ObjectMeta: metav1.ObjectMeta{
-					Name: name + ".cluster.com",
-				},
-				Spec: apiextensionsv1.CustomResourceDefinitionSpec{
-					Scope: apiextensionsv1.NamespaceScoped,
-					Group: "cluster.com",
-					Versions: []apiextensionsv1.CustomResourceDefinitionVersion{
+		var ctx context.Context
+
+		BeforeEach(func() { ctx = context.Background() })
+		AfterEach(func() { ctx.Done() })
+
+		When("incoming crd event is safe", func() {
+			var crd *apiextensionsv1.CustomResourceDefinition
+
+			BeforeEach(func() {
+				crd = newTestingCRD()
+
+				err := c.Create(ctx, crd)
+				Expect(err).To(BeNil())
+
+				Eventually(func() error {
+					return c.Get(ctx, client.ObjectKeyFromObject(crd), crd)
+				}).Should(Succeed(), "should be able to create a safe crd but was not")
+			})
+
+			AfterEach(func() {
+				By("deleting the testing Bundle resource")
+				Expect(c.Delete(ctx, crd)).To(BeNil())
+			})
+
+			It("should allow the crd update event to occur", func() {
+				Eventually(func() error {
+					if err := c.Get(ctx, client.ObjectKeyFromObject(crd), crd); err != nil {
+						return err
+					}
+
+					crd.Spec.Versions[0].Storage = false
+
+					crd.Spec.Versions = append(crd.Spec.Versions, apiextensionsv1.CustomResourceDefinitionVersion{
+						Name:    "v1alpha2",
+						Served:  true,
+						Storage: true,
+						Schema: &apiextensionsv1.CustomResourceValidation{
+							OpenAPIV3Schema: &apiextensionsv1.JSONSchemaProps{
+								Type:        "object",
+								Description: "my crd schema",
+							},
+						},
+					})
+
+					return c.Update(ctx, crd)
+				}).Should(Succeed())
+			})
+		})
+
+		When("incoming crd event removes a stored version", func() {
+			var crd *apiextensionsv1.CustomResourceDefinition
+
+			BeforeEach(func() {
+				crd = newTestingCRD()
+
+				err := c.Create(ctx, crd)
+				Expect(err).To(BeNil())
+
+				Eventually(func() error {
+					return c.Get(ctx, client.ObjectKeyFromObject(crd), crd)
+				}).Should(Succeed(), "should be able to create a safe crd but was not")
+			})
+
+			AfterEach(func() {
+				By("deleting the testing Bundle resource")
+				Expect(c.Delete(ctx, crd)).To(BeNil())
+			})
+
+			It("should deny admission", func() {
+				Eventually(func() bool {
+					if err := c.Get(ctx, client.ObjectKeyFromObject(crd), crd); err != nil {
+						return false
+					}
+
+					crd.Spec.Versions = []apiextensionsv1.CustomResourceDefinitionVersion{
 						{
-							Name:    "v1alpha1",
+							Name:    "v1alpha2",
 							Served:  true,
 							Storage: true,
 							Schema: &apiextensionsv1.CustomResourceValidation{
@@ -43,34 +101,32 @@ var _ = Describe("crd validation webhook", func() {
 								},
 							},
 						},
-					},
-					Names: apiextensionsv1.CustomResourceDefinitionNames{
-						Plural:   name,
-						Singular: name,
-						Kind:     name,
-						ListKind: "list" + name,
-					},
-				},
-			}
+					}
 
-			err := c.Create(ctx, v1crd)
-			Expect(err).To(BeNil())
+					err := c.Create(ctx, crd)
+					if err == nil {
+						return false
+					}
 
-			Eventually(func() error {
-				crdHolder := &apiextensionsv1.CustomResourceDefinition{}
-				return c.Get(ctx, client.ObjectKeyFromObject(v1crd), crdHolder)
-			}).Should(Succeed(), "should admit a new and safe crd event")
-		})
-
-		AfterEach(func() {
-			By("deleting the testing Bundle resource")
-			Expect(c.Delete(ctx, v1crd)).To(BeNil())
+					return errorsLooselyEqual(err, errors.New("cannot remove stored versions"))
+				}).Should(BeTrue())
+			})
 		})
+	})
+})
 
-		It("should deny admission for the incoming CRD if it removes a stored version", func() {
-			v1crd.Spec.Versions = []apiextensionsv1.CustomResourceDefinitionVersion{
+func newTestingCRD() *apiextensionsv1.CustomResourceDefinition {
+	name := names.SimpleNameGenerator.GenerateName("samplecrd")
+	return &apiextensionsv1.CustomResourceDefinition{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: name + ".e2e.io",
+		},
+		Spec: apiextensionsv1.CustomResourceDefinitionSpec{
+			Scope: apiextensionsv1.ClusterScoped,
+			Group: "e2e.io",
+			Versions: []apiextensionsv1.CustomResourceDefinitionVersion{
 				{
-					Name:    "v2alpha1",
+					Name:    "v1alpha1",
 					Served:  true,
 					Storage: true,
 					Schema: &apiextensionsv1.CustomResourceValidation{
@@ -80,14 +136,15 @@ var _ = Describe("crd validation webhook", func() {
 						},
 					},
 				},
-			}
-
-			By("emitting an error indicating that you cannot remove stored versions")
-			err := c.Create(ctx, v1crd)
-			Expect(err).ToNot(BeNil(), "expected create to fail but it succeeded")
-			Expect(errorsLooselyEqual(err, errors.New("cannot remove stored versions"))).To(BeTrue())
-		})
-	})
-})
+			},
+			Names: apiextensionsv1.CustomResourceDefinitionNames{
+				Plural:   name,
+				Singular: name,
+				Kind:     name,
+				ListKind: name + "List",
+			},
+		},
+	}
+}
 
 func errorsLooselyEqual(a, b error) bool { return strings.Contains(a.Error(), b.Error()) }