Skip to content

Commit

Permalink
fix: consider version when getting CRDs for validating descriptors (#…
Browse files Browse the repository at this point in the history
…6784)

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 <[email protected]>
  • Loading branch information
tthvo authored Oct 16, 2024
1 parent 6beabcd commit f542f5d
Show file tree
Hide file tree
Showing 3 changed files with 115 additions and 68 deletions.
28 changes: 28 additions & 0 deletions changelog/fragments/01-olm-scorecard-fix.yaml
Original file line number Diff line number Diff line change
@@ -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
151 changes: 85 additions & 66 deletions internal/scorecard/tests/bundle_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -156,147 +166,156 @@ 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",
},
},
}

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))
})

})
Expand Down
4 changes: 2 additions & 2 deletions internal/scorecard/tests/olm.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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
}
Expand Down

0 comments on commit f542f5d

Please sign in to comment.