Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: consider version when getting CRDs for validating descriptors #6784

Merged
merged 1 commit into from
Oct 16, 2024
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
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
Loading