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 
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 <thuan.votann@gmail.com>
  • Loading branch information
@tthvo
tthvo committed Jul 15, 2024
1 parent e95abdb commit e133c7e
Show file tree
Hide file tree
Showing 3 changed files with 125 additions and 68 deletions.
28 changes: 28 additions & 0 deletions changelog/fragments/01-olm-scorecard-fix.yaml
View file
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
150 changes: 84 additions & 66 deletions internal/scorecard/tests/bundle_test.go
View file
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,138 +166,136 @@ 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 return 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() {
cr := []unstructured.Unstructured{
unstructured.Unstructured{
Object: map[string]interface{}{},
},
}
result = checkSpec(cr, result)
Expect(result.State).To(Equal(scapiv1alpha3.PassState))
})

It("should pass when CRs do have spec field specified", func() {
cr := []unstructured.Unstructured{
unstructured.Unstructured{
{
Object: map[string]interface{}{
"spec": map[string]interface{}{
"spec": "val",
Expand All @@ -299,6 +307,16 @@ var _ = Describe("Basic and OLM tests", func() {
Expect(result.State).To(Equal(scapiv1alpha3.PassState))
})

It("should fail when CRs do not have spec field specified", func() {
cr := []unstructured.Unstructured{
{
Object: map[string]interface{}{},
},
}
result = checkSpec(cr, result)
Expect(result.State).To(Equal(scapiv1alpha3.PassState))
})

})

})
Expand Down
15 changes: 13 additions & 2 deletions internal/scorecard/tests/olm.go
View file
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 == getVersionFromGV(cr) {
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 == getVersionFromGV(cr) {
crd = &owned
break
}
Expand Down Expand Up @@ -388,6 +388,17 @@ func isCRFromCRDApi(cr unstructured.Unstructured, crds []*apiextv1.CustomResourc
return r
}

func getVersionFromGV(cr unstructured.Unstructured) string {
gv := strings.Split(cr.GetAPIVersion(), "/")
if len(gv) > 2 {
return "" // invalid apiVersion
}
if len(gv) == 2 {
return gv[1]
}
return cr.GetAPIVersion()
}

func wrapResult(r scapiv1alpha3.TestResult) scapiv1alpha3.TestStatus {
return scapiv1alpha3.TestStatus{
Results: []scapiv1alpha3.TestResult{r},
Expand Down

0 comments on commit e133c7e

Please sign in to comment.