From 7580862528c35cb7ad70633dcdad864f8ed9d3cb Mon Sep 17 00:00:00 2001 From: Thuan Vo Date: Tue, 9 Jul 2024 18:57:27 -0700 Subject: [PATCH] fix: consider version when getting CRDs for validating descriptors An additional condition is included for matching `apiVersion` of example CRs with CRD `version` when searching for the CRD in the CSV. This ensures the correct CRD version is selected for validations. Closes #6781 Signed-off-by: Thuan Vo --- changelog/fragments/01-olm-scorecard-fix.yaml | 28 ++++ internal/scorecard/tests/bundle_test.go | 151 ++++++++++-------- internal/scorecard/tests/olm.go | 4 +- 3 files changed, 115 insertions(+), 68 deletions(-) create mode 100644 changelog/fragments/01-olm-scorecard-fix.yaml diff --git a/changelog/fragments/01-olm-scorecard-fix.yaml b/changelog/fragments/01-olm-scorecard-fix.yaml new file mode 100644 index 0000000000..685eb642a5 --- /dev/null +++ b/changelog/fragments/01-olm-scorecard-fix.yaml @@ -0,0 +1,28 @@ +# entries is a list of entries to include in +# release notes and/or the migration guide +entries: + - description: > + An additional condition is included for matching `apiVersion` of example CRs with CRD `version` when searching for the CRD in the CSV. + Previously, The `olm-spec-descriptors` scorecard test failed when multiple versions of CRD is included in the CSV. + The CR specified in `alm-examples` annotations are validated only against the first matched CRD (by name), which is incorrect. + This ensures the correct CRD version is selected for validations. + + # kind is one of: + # - addition + # - change + # - deprecation + # - removal + # - bugfix + kind: "bugfix" + + # Is this a breaking change? + breaking: false + + # NOTE: ONLY USE `pull_request_override` WHEN ADDING THIS + # FILE FOR A PREVIOUSLY MERGED PULL_REQUEST! + # + # The generator auto-detects the PR number from the commit + # message in which this file was originally added. + # + # What is the pull request number (without the "#")? + # pull_request_override: 0 diff --git a/internal/scorecard/tests/bundle_test.go b/internal/scorecard/tests/bundle_test.go index 75144b5432..69048a3d16 100644 --- a/internal/scorecard/tests/bundle_test.go +++ b/internal/scorecard/tests/bundle_test.go @@ -127,15 +127,25 @@ var _ = Describe("Basic and OLM tests", func() { Describe("OLM Tests", func() { Describe("Test Status and Spec Descriptors", func() { - var ( - cr unstructured.Unstructured - csv operatorsv1alpha1.ClusterServiceVersion - ) - - csv = operatorsv1alpha1.ClusterServiceVersion{ + csv := operatorsv1alpha1.ClusterServiceVersion{ Spec: operatorsv1alpha1.ClusterServiceVersionSpec{ CustomResourceDefinitions: operatorsv1alpha1.CustomResourceDefinitions{ Owned: []operatorsv1alpha1.CRDDescription{ + operatorsv1alpha1.CRDDescription{ + Name: "Test", + Version: "v2", + Kind: "TestKind", + StatusDescriptors: []operatorsv1alpha1.StatusDescriptor{ + operatorsv1alpha1.StatusDescriptor{ + Path: "newStatus", + }, + }, + SpecDescriptors: []operatorsv1alpha1.SpecDescriptor{ + operatorsv1alpha1.SpecDescriptor{ + Path: "newSpec", + }, + }, + }, operatorsv1alpha1.CRDDescription{ Name: "Test", Version: "v1", @@ -156,88 +166,107 @@ var _ = Describe("Basic and OLM tests", func() { }, } - It("should pass when csv with owned cr and required fields is present", func() { - cr = unstructured.Unstructured{ - Object: map[string]interface{}{ - "status": map[string]interface{}{ - "status": "val", - }, - "spec": map[string]interface{}{ - "spec": "val", - }, - }, + It("should pass when CR Object Descriptor is nil", func() { + cr := unstructured.Unstructured{ + Object: nil, } cr.SetGroupVersionKind(schema.GroupVersionKind{ - Kind: "TestKind", - Group: "test.example.com", + Kind: "TestKind", + Group: "test.example.com", + Version: "v1", }) result = checkOwnedCSVStatusDescriptor(cr, &csv, result) Expect(result.State).To(Equal(scapiv1alpha3.PassState)) }) - It("should return warning when no spec status are defined for CRD", func() { - cr = unstructured.Unstructured{ + It("should pass when status descriptor field is present in CR", func() { + cr := unstructured.Unstructured{ Object: map[string]interface{}{ + "status": map[string]interface{}{ + "status": "val", + }, "spec": map[string]interface{}{ "spec": "val", }, }, } cr.SetGroupVersionKind(schema.GroupVersionKind{ - Kind: "TestKind", - Group: "test.example.com", + Kind: "TestKind", + Group: "test.example.com", + Version: "v1", }) result = checkOwnedCSVStatusDescriptor(cr, &csv, result) - Expect(result.Suggestions).To(HaveLen(1)) Expect(result.State).To(Equal(scapiv1alpha3.PassState)) }) - It("should pass when CR Object Descriptor is nil", func() { + It("should pass when required spec descriptor field is present in CR", func() { cr := unstructured.Unstructured{ - Object: nil, + Object: map[string]interface{}{ + "status": map[string]interface{}{ + "status": "val", + }, + "spec": map[string]interface{}{ + "spec": "val", + }, + }, } cr.SetGroupVersionKind(schema.GroupVersionKind{ - Kind: "TestKind", - Group: "test.example.com", + Kind: "TestKind", + Group: "test.example.com", + Version: "v1", }) - result = checkOwnedCSVStatusDescriptor(cr, &csv, result) + result = checkOwnedCSVSpecDescriptors(cr, &csv, result) Expect(result.State).To(Equal(scapiv1alpha3.PassState)) }) - It("should fail when CR Object Descriptor is nil and CRD with given GVK cannot be found", func() { + It("should pass with warning when no status descriptor field is present in CR", func() { cr := unstructured.Unstructured{ - Object: nil, + Object: map[string]interface{}{ + "spec": map[string]interface{}{ + "spec": "val", + }, + }, } cr.SetGroupVersionKind(schema.GroupVersionKind{ - Kind: "TestKindNotPresent", - Group: "testnotpresent.example.com", + Kind: "TestKind", + Group: "test.example.com", + Version: "v1", }) result = checkOwnedCSVStatusDescriptor(cr, &csv, result) - Expect(result.State).To(Equal(scapiv1alpha3.FailState)) + Expect(result.Suggestions).To(HaveLen(1)) + Expect(result.State).To(Equal(scapiv1alpha3.PassState)) }) - It("should fail when owned CRD for CR does not have GVK set", func() { + It("should fail CRD with given GVK cannot be found", func() { cr := unstructured.Unstructured{ Object: map[string]interface{}{ "status": map[string]interface{}{ "status": "val", }, + "spec": map[string]interface{}{ + "spec": "val", + }, }, } + cr.SetGroupVersionKind(schema.GroupVersionKind{ + Kind: "TestKindNotPresent", + Group: "testnotpresent.example.com", + Version: "unknown", + }) result = checkOwnedCSVStatusDescriptor(cr, &csv, result) Expect(result.State).To(Equal(scapiv1alpha3.FailState)) }) - It("should fail when required descriptor field is not present in CR", func() { + It("should fail when CR does not have GVK set", func() { cr := unstructured.Unstructured{ Object: map[string]interface{}{ - "node": map[string]interface{}{ - "node": "val", + "status": map[string]interface{}{ + "status": "val", }, }, } @@ -245,58 +274,48 @@ var _ = Describe("Basic and OLM tests", func() { result = checkOwnedCSVStatusDescriptor(cr, &csv, result) Expect(result.State).To(Equal(scapiv1alpha3.FailState)) }) - It("should pass when required descriptor field is present in CR", func() { + + It("should fail when required spec descriptor field is not present in CR", func() { cr := unstructured.Unstructured{ Object: map[string]interface{}{ - "status": map[string]interface{}{ - "status": "val", - }, "spec": map[string]interface{}{ - "spec": "val", + "node": "val", }, }, } cr.SetGroupVersionKind(schema.GroupVersionKind{ - Kind: "TestKind", - Group: "test.example.com", + Kind: "TestKind", + Group: "test.example.com", + Version: "v1", }) - result = checkOwnedCSVSpecDescriptors(cr, &csv, result) - Expect(result.State).To(Equal(scapiv1alpha3.PassState)) - }) - It("should fail when required spec descriptor field is not present in CR", func() { - cr := unstructured.Unstructured{ - Object: map[string]interface{}{ - "status": map[string]interface{}{ - "status": "val", - }, - }, - } - result = checkOwnedCSVSpecDescriptors(cr, &csv, result) Expect(result.State).To(Equal(scapiv1alpha3.FailState)) }) - It("should fail when CRs do not have spec field specified", func() { + + It("should pass when CRs have spec field specified", func() { cr := []unstructured.Unstructured{ - unstructured.Unstructured{ - Object: map[string]interface{}{}, + { + Object: map[string]interface{}{ + "spec": map[string]interface{}{ + "spec": "val", + }, + }, }, } result = checkSpec(cr, result) Expect(result.State).To(Equal(scapiv1alpha3.PassState)) }) - It("should pass when CRs do have spec field specified", func() { + + It("should pass with warning when CRs do not have spec field specified", func() { cr := []unstructured.Unstructured{ - unstructured.Unstructured{ - Object: map[string]interface{}{ - "spec": map[string]interface{}{ - "spec": "val", - }, - }, + { + Object: map[string]interface{}{}, }, } result = checkSpec(cr, result) Expect(result.State).To(Equal(scapiv1alpha3.PassState)) + Expect(result.Suggestions).To(HaveLen(1)) }) }) diff --git a/internal/scorecard/tests/olm.go b/internal/scorecard/tests/olm.go index 16b164bdeb..02bf21b5d8 100644 --- a/internal/scorecard/tests/olm.go +++ b/internal/scorecard/tests/olm.go @@ -237,7 +237,7 @@ func checkOwnedCSVStatusDescriptor(cr unstructured.Unstructured, csv *operatorsv var crdDescription *operatorsv1alpha1.CRDDescription for _, owned := range csv.Spec.CustomResourceDefinitions.Owned { - if owned.Kind == cr.GetKind() { + if owned.Kind == cr.GetKind() && owned.Version == cr.GroupVersionKind().Version { crdDescription = &owned break } @@ -288,7 +288,7 @@ func checkOwnedCSVSpecDescriptors(cr unstructured.Unstructured, csv *operatorsv1 var crd *operatorsv1alpha1.CRDDescription for _, owned := range csv.Spec.CustomResourceDefinitions.Owned { - if owned.Kind == cr.GetKind() { + if owned.Kind == cr.GetKind() && owned.Version == cr.GroupVersionKind().Version { crd = &owned break }