Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -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
56 changes: 56 additions & 0 deletions internal/scorecard/tests/bundle_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
{
Expand Down Expand Up @@ -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() {
Expand Down
18 changes: 9 additions & 9 deletions internal/scorecard/tests/olm.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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 {
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
Loading