diff --git a/Makefile b/Makefile index f6e30d31..30b5bef9 100644 --- a/Makefile +++ b/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 # diff --git a/cmd/crdvalidator/internal/crd/crd.go b/cmd/crdvalidator/internal/crd/crd.go index b68059c5..6421b979 100644 --- a/cmd/crdvalidator/internal/crd/crd.go +++ b/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 } diff --git a/cmd/crdvalidator/manifests/01_cluster_role.yaml b/cmd/crdvalidator/manifests/01_cluster_role.yaml index 4d7373b6..0b248da2 100644 --- a/cmd/crdvalidator/manifests/01_cluster_role.yaml +++ b/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"] diff --git a/cmd/crdvalidator/test/e2e/crdvalidator_suite_test.go b/cmd/crdvalidator/test/e2e/crdvalidator_suite_test.go index 0a25e393..708d8242 100644 --- a/cmd/crdvalidator/test/e2e/crdvalidator_suite_test.go +++ b/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") } diff --git a/cmd/crdvalidator/test/e2e/crdvalidator_test.go b/cmd/crdvalidator/test/e2e/crdvalidator_test.go index c2038f6f..e6b19893 100644 --- a/cmd/crdvalidator/test/e2e/crdvalidator_test.go +++ b/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()) }