From bedd1e3cea858b2c3cc11daeb2e0f9adba87e3dd Mon Sep 17 00:00:00 2001 From: Vu Dinh Date: Mon, 22 Jun 2020 01:49:53 -0400 Subject: [PATCH] fix(crd): Fix CRD validation falsely error on unused CRD versions CRD can have multiple versions and CSV can only reference one version. CRD validation falsely errors out when there are unused versions in provided CRDs. Signed-off-by: Vu Dinh --- pkg/validation/internal/bundle.go | 12 ++++++ pkg/validation/internal/bundle_test.go | 5 +++ .../test-operator.clusterserviceversion.yaml | 41 +++++++++++++++++++ .../test.example.com_tests_v1_crd.yaml | 19 +++++++++ ...test.example.com_testtwos_v1beta1_crd.yaml | 20 +++++++++ 5 files changed, 97 insertions(+) create mode 100644 pkg/validation/internal/testdata/valid_bundle_2/test-operator.clusterserviceversion.yaml create mode 100644 pkg/validation/internal/testdata/valid_bundle_2/test.example.com_tests_v1_crd.yaml create mode 100644 pkg/validation/internal/testdata/valid_bundle_2/test.example.com_testtwos_v1beta1_crd.yaml diff --git a/pkg/validation/internal/bundle.go b/pkg/validation/internal/bundle.go index 04258606a..d1e7217de 100644 --- a/pkg/validation/internal/bundle.go +++ b/pkg/validation/internal/bundle.go @@ -43,13 +43,25 @@ func validateOwnedCRDs(bundle *manifests.Bundle, csv *operatorsv1alpha1.ClusterS } // All owned keys must match a CRD in bundle. + ownedGVSet := make(map[schema.GroupKind]struct{}) for _, ownedKey := range ownedKeys { if _, ok := keySet[ownedKey]; !ok { result.Add(errors.ErrInvalidBundle(fmt.Sprintf("owned CRD %q not found in bundle %q", ownedKey, bundle.Name), ownedKey)) } else { delete(keySet, ownedKey) + gvKey := schema.GroupKind{Group: ownedKey.Group, Kind: ownedKey.Kind} + ownedGVSet[gvKey] = struct{}{} } } + + // Filter out unused versions of the same CRD + for key := range keySet { + gvKey := schema.GroupKind{Group: key.Group, Kind: key.Kind} + if _, ok := ownedGVSet[gvKey]; ok { + delete(keySet, key) + } + } + // All CRDs present in a CSV must be present in the bundle. for key := range keySet { result.Add(errors.WarnInvalidBundle(fmt.Sprintf("CRD %q is present in bundle %q but not defined in CSV", key, bundle.Name), key)) diff --git a/pkg/validation/internal/bundle_test.go b/pkg/validation/internal/bundle_test.go index 58b3b503b..762343faa 100644 --- a/pkg/validation/internal/bundle_test.go +++ b/pkg/validation/internal/bundle_test.go @@ -20,6 +20,11 @@ func TestValidateBundle(t *testing.T) { directory: "./testdata/valid_bundle", hasError: false, }, + { + description: "registryv1 valid bundle with multiple versions in CRD", + directory: "./testdata/valid_bundle_2", + hasError: false, + }, { description: "registryv1 invalid bundle without CRD etcdclusters v1beta2 in bundle", directory: "./testdata/invalid_bundle", diff --git a/pkg/validation/internal/testdata/valid_bundle_2/test-operator.clusterserviceversion.yaml b/pkg/validation/internal/testdata/valid_bundle_2/test-operator.clusterserviceversion.yaml new file mode 100644 index 000000000..a12847e37 --- /dev/null +++ b/pkg/validation/internal/testdata/valid_bundle_2/test-operator.clusterserviceversion.yaml @@ -0,0 +1,41 @@ +apiVersion: operators.coreos.com/v1alpha1 +kind: ClusterServiceVersion +metadata: + annotations: + capabilities: Basic Install + name: test-operator.v0.0.1 + namespace: placeholder +spec: + apiservicedefinitions: {} + customresourcedefinitions: + owned: + - description: Test is the Schema for the tests API + kind: Test + name: tests.test.example.com + version: v1alpha1 + - description: TestTwo is the Schema for the tests API + kind: TestTwo + name: TestTwos.test.example.com + version: v1beta1 + installModes: + - supported: true + type: OwnNamespace + - supported: true + type: SingleNamespace + - supported: false + type: MultiNamespace + - supported: true + type: AllNamespaces + keywords: + - test-operator + links: + - name: Test Operator + url: https://test-operator.domain + maintainers: + - email: your@email.com + name: Maintainer Name + maturity: alpha + provider: + name: Provider Name + url: https://your.domain + version: 0.0.1 diff --git a/pkg/validation/internal/testdata/valid_bundle_2/test.example.com_tests_v1_crd.yaml b/pkg/validation/internal/testdata/valid_bundle_2/test.example.com_tests_v1_crd.yaml new file mode 100644 index 000000000..cef0d2d22 --- /dev/null +++ b/pkg/validation/internal/testdata/valid_bundle_2/test.example.com_tests_v1_crd.yaml @@ -0,0 +1,19 @@ +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + name: tests.test.example.com +spec: + group: test.example.com + names: + kind: Test + listKind: TestList + plural: tests + singular: test + scope: Namespaced + versions: + - name: v1alpha1 + served: true + storage: true + - name: v1beta1 + served: true + storage: false diff --git a/pkg/validation/internal/testdata/valid_bundle_2/test.example.com_testtwos_v1beta1_crd.yaml b/pkg/validation/internal/testdata/valid_bundle_2/test.example.com_testtwos_v1beta1_crd.yaml new file mode 100644 index 000000000..e9e7ccc98 --- /dev/null +++ b/pkg/validation/internal/testdata/valid_bundle_2/test.example.com_testtwos_v1beta1_crd.yaml @@ -0,0 +1,20 @@ +apiVersion: apiextensions.k8s.io/v1beta1 +kind: CustomResourceDefinition +metadata: + name: testtwos.test.example.com +spec: + group: test.example.com + names: + kind: TestTwo + listKind: TestTwoList + plural: testtwos + singular: testtwo + scope: Namespaced + version: v1beta1 + versions: + - name: v1beta1 + served: true + storage: true + - name: v1alpha1 + served: true + storage: false