diff --git a/changelog/fragments/scorecard-descriptor-non-object-panic.yaml b/changelog/fragments/scorecard-descriptor-non-object-panic.yaml new file mode 100644 index 00000000000..5bb7038b5c5 --- /dev/null +++ b/changelog/fragments/scorecard-descriptor-non-object-panic.yaml @@ -0,0 +1,9 @@ +entries: + - description: > + Fixed a panic in the scorecard `olm-spec-descriptors`, `olm-status-descriptors`, + and `olm-crds-have-validation` tests when a custom resource in a bundle's + `alm-examples` has a `spec` or `status` field that is not a JSON object (for + example a string or an array). These fields are now treated as having no + descriptors to check instead of crashing the scorecard test binary. + kind: bugfix + breaking: false diff --git a/internal/scorecard/tests/bundle_test.go b/internal/scorecard/tests/bundle_test.go index 450fcf6e8f7..5df773694ee 100644 --- a/internal/scorecard/tests/bundle_test.go +++ b/internal/scorecard/tests/bundle_test.go @@ -293,6 +293,44 @@ var _ = Describe("Basic and OLM tests", func() { Expect(result.State).To(Equal(scapiv1alpha3.FailState)) }) + It("should fail cleanly (not panic) when spec is not an object", func() { + cr := unstructured.Unstructured{ + Object: map[string]any{ + "spec": "i-am-a-string-not-a-map", + }, + } + cr.SetGroupVersionKind(schema.GroupVersionKind{ + Kind: "TestKind", + Group: "test.example.com", + Version: "v1", + }) + + Expect(func() { + result = checkOwnedCSVSpecDescriptors(cr, &csv, result) + }).NotTo(Panic()) + Expect(result.State).To(Equal(scapiv1alpha3.FailState)) + }) + + It("should not panic when status is not an object", func() { + cr := unstructured.Unstructured{ + Object: map[string]any{ + "status": []any{"not", "a", "map"}, + "spec": map[string]any{ + "spec": "val", + }, + }, + } + cr.SetGroupVersionKind(schema.GroupVersionKind{ + Kind: "TestKind", + Group: "test.example.com", + Version: "v1", + }) + + Expect(func() { + result = checkOwnedCSVStatusDescriptor(cr, &csv, result) + }).NotTo(Panic()) + }) + It("should pass when CRs have spec field specified", func() { cr := []unstructured.Unstructured{ { @@ -416,6 +454,24 @@ var _ = Describe("Basic and OLM tests", func() { }) + It("should not panic when spec or status is not an object", func() { + cr = unstructured.Unstructured{ + Object: map[string]any{ + "spec": "not-a-map", + "status": []any{"also", "not", "a", "map"}, + }, + } + cr.SetGroupVersionKind(schema.GroupVersionKind{ + Kind: "TestKind", + Group: "test.example.com", + Version: "v1", + }) + + Expect(func() { + result = isCRFromCRDApi(cr, crd, result) + }).NotTo(Panic()) + }) + }) Describe("Check CRDs for resources", func() { diff --git a/internal/scorecard/tests/olm.go b/internal/scorecard/tests/olm.go index a3c9c0904fa..c21cc5a95d8 100644 --- a/internal/scorecard/tests/olm.go +++ b/internal/scorecard/tests/olm.go @@ -251,9 +251,9 @@ func checkOwnedCSVStatusDescriptor(cr unstructured.Unstructured, csv *operatorsv } hasStatusDefinition := false - if cr.Object["status"] != nil { + if status, ok := cr.Object["status"].(map[string]any); ok { // Ensure that has no empty keys - hasStatusDefinition = len(cr.Object["status"].(map[string]any)) > 0 + hasStatusDefinition = len(status) > 0 } if !hasStatusDefinition { @@ -279,13 +279,15 @@ func checkOwnedCSVStatusDescriptor(cr unstructured.Unstructured, csv *operatorsv // I don't think it will be validated. func checkOwnedCSVSpecDescriptors(cr unstructured.Unstructured, csv *operatorsv1alpha1.ClusterServiceVersion, r scapiv1alpha3.TestResult) scapiv1alpha3.TestResult { - if cr.Object[specDescriptor] == nil { + block, ok := cr.Object[specDescriptor].(map[string]any) + if !ok { + // A nil spec, or a spec that isn't a JSON object (for example a scalar + // or array coming from a malformed alm-examples CR), has no descriptor + // fields to check, so treat it the same as a missing spec. r.State = scapiv1alpha3.FailState return r } - block := cr.Object[specDescriptor].(map[string]any) - var crd *operatorsv1alpha1.CRDDescription for _, owned := range csv.Spec.CustomResourceDefinitions.Owned { if owned.Kind == cr.GetKind() && owned.Version == cr.GroupVersionKind().Version { @@ -358,8 +360,7 @@ func isCRFromCRDApi(cr unstructured.Unstructured, crds []*apiextv1.CustomResourc continue } failed := false - if cr.Object["spec"] != nil { - spec := cr.Object["spec"].(map[string]any) + if spec, ok := cr.Object["spec"].(map[string]any); ok { for key := range spec { if _, ok := version.Schema.OpenAPIV3Schema.Properties["spec"].Properties[key]; !ok { failed = true @@ -369,8 +370,7 @@ func isCRFromCRDApi(cr unstructured.Unstructured, crds []*apiextv1.CustomResourc } } } - if cr.Object["status"] != nil { - status := cr.Object["status"].(map[string]any) + if status, ok := cr.Object["status"].(map[string]any); ok { for key := range status { if _, ok := version.Schema.OpenAPIV3Schema.Properties["status"].Properties[key]; !ok { failed = true