Skip to content

Commit

Permalink
fix: modify olm-status-descriptor-test in scorecard
Browse files Browse the repository at this point in the history
Modify `olm-status-descriptor-test` to not check the presence of
`status` field in CR, instead validate only if status-descriptors
are present in owned CRDs.
  • Loading branch information
varshaprasad96 committed Oct 10, 2020
1 parent 0c46569 commit 93eecd0
Show file tree
Hide file tree
Showing 3 changed files with 92 additions and 38 deletions.
16 changes: 16 additions & 0 deletions changelog/fragments/scorecard-status-descriptor-test.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
# entries is a list of entries to include in
# release notes and/or the migration guide
entries:
- description: >
Modify `olm-status-descriptors-test` to only validate if the status-descriptors are present in CRD.
# kind is one of:
# - addition
# - change
# - deprecation
# - removal
# - bugfix
kind: "bugfix"
# Is this a breaking change?
breaking: false
38 changes: 26 additions & 12 deletions internal/scorecard/tests/bundle_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -128,9 +128,8 @@ var _ = Describe("Basic and OLM tests", func() {

Describe("Test Status and Spec Descriptors", func() {
var (
cr unstructured.Unstructured
descriptor string
csv operatorsv1alpha1.ClusterServiceVersion
cr unstructured.Unstructured
csv operatorsv1alpha1.ClusterServiceVersion
)

csv = operatorsv1alpha1.ClusterServiceVersion{
Expand All @@ -157,8 +156,6 @@ var _ = Describe("Basic and OLM tests", func() {
},
}

descriptor = "status"

It("should pass when csv with owned cr and required fields is present", func() {
cr = unstructured.Unstructured{
Object: map[string]interface{}{
Expand All @@ -175,16 +172,33 @@ var _ = Describe("Basic and OLM tests", func() {
Group: "test.example.com",
})

result = checkOwnedCSVDescriptors(cr, &csv, descriptor, result)
result = checkOwnedCSVStatusDescriptor(cr, &csv, result)
Expect(result.State).To(Equal(scapiv1alpha3.PassState))
})

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

result = checkOwnedCSVStatusDescriptor(cr, &csv, result)
Expect(result.State).To(Equal(scapiv1alpha3.PassState))
})

It("should fail when CR Object Descriptor is nil", func() {
It("should fail when CR Object Descriptor is nil and CRD with given GVK cannot be found", func() {
cr := unstructured.Unstructured{
Object: nil,
}
cr.SetGroupVersionKind(schema.GroupVersionKind{
Kind: "TestKindNotPresent",
Group: "testnotpresent.example.com",
})

result = checkOwnedCSVDescriptors(cr, &csv, descriptor, result)
result = checkOwnedCSVStatusDescriptor(cr, &csv, result)
Expect(result.State).To(Equal(scapiv1alpha3.FailState))
})

Expand All @@ -197,7 +211,7 @@ var _ = Describe("Basic and OLM tests", func() {
},
}

result = checkOwnedCSVDescriptors(cr, &csv, descriptor, result)
result = checkOwnedCSVStatusDescriptor(cr, &csv, result)
Expect(result.State).To(Equal(scapiv1alpha3.FailState))
})

Expand All @@ -210,7 +224,7 @@ var _ = Describe("Basic and OLM tests", func() {
},
}

result = checkOwnedCSVDescriptors(cr, &csv, descriptor, result)
result = checkOwnedCSVStatusDescriptor(cr, &csv, result)
Expect(result.State).To(Equal(scapiv1alpha3.FailState))
})
It("should pass when required descriptor field is present in CR", func() {
Expand All @@ -229,7 +243,7 @@ var _ = Describe("Basic and OLM tests", func() {
Group: "test.example.com",
})

result = checkOwnedCSVDescriptors(cr, &csv, descriptor, result)
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() {
Expand All @@ -241,7 +255,7 @@ var _ = Describe("Basic and OLM tests", func() {
},
}

result = checkOwnedCSVDescriptors(cr, &csv, descriptor, result)
result = checkOwnedCSVSpecDescriptors(cr, &csv, result)
Expect(result.State).To(Equal(scapiv1alpha3.FailState))
})
It("should fail when CRs do not have spec field specified", func() {
Expand Down
76 changes: 50 additions & 26 deletions internal/scorecard/tests/olm.go
Original file line number Diff line number Diff line change
Expand Up @@ -219,8 +219,45 @@ func checkCSVDescriptors(bundle *apimanifests.Bundle, r scapiv1alpha3.TestResult
}
r.Log += fmt.Sprintf("Loaded %d Custom Resources from alm-examples\n", len(crs))

for _, cr := range crs {
r = checkOwnedCSVDescriptors(cr, bundle.CSV, descriptor, r)
if descriptor == statusDescriptor {
for _, cr := range crs {
r = checkOwnedCSVStatusDescriptor(cr, bundle.CSV, r)
}
} else {
for _, cr := range crs {
r = checkOwnedCSVSpecDescriptors(cr, bundle.CSV, r)
}
}

return r
}

func checkOwnedCSVStatusDescriptor(cr unstructured.Unstructured, csv *operatorsv1alpha1.ClusterServiceVersion,
r scapiv1alpha3.TestResult) scapiv1alpha3.TestResult {

var crd *operatorsv1alpha1.CRDDescription

if cr.Object[statusDescriptor] == nil {
r.Suggestions = append(r.Suggestions, fmt.Sprintf("Status field can be added to the CR %s", cr.GetName()))
}

for _, owned := range csv.Spec.CustomResourceDefinitions.Owned {
if owned.Kind == cr.GetKind() {
crd = &owned
break
}
}

if crd == nil {
msg := fmt.Sprintf("Failed to find an owned CRD for CR %s with GVK %s", cr.GetName(), cr.GroupVersionKind().String())
r.Errors = append(r.Errors, msg)
r.State = scapiv1alpha3.FailState
return r
}

if len(crd.StatusDescriptors) == 0 {
r.Errors = append(r.Errors, fmt.Sprintf("%s does not have a status descriptor", crd.Name))
r.State = scapiv1alpha3.FailState
}

return r
Expand All @@ -229,15 +266,14 @@ func checkCSVDescriptors(bundle *apimanifests.Bundle, r scapiv1alpha3.TestResult
// TODO This is the validation we did in v1, but it looks like it only validates fields that
// are in the example CRs, if you have a field in your CRD that isn't present in one of your examples,
// I don't think it will be validated.
func checkOwnedCSVDescriptors(cr unstructured.Unstructured, csv *operatorsv1alpha1.ClusterServiceVersion,
descriptor string, r scapiv1alpha3.TestResult) scapiv1alpha3.TestResult {

if cr.Object[descriptor] == nil {
func checkOwnedCSVSpecDescriptors(cr unstructured.Unstructured, csv *operatorsv1alpha1.ClusterServiceVersion,
r scapiv1alpha3.TestResult) scapiv1alpha3.TestResult {
if cr.Object[specDescriptor] == nil {
r.State = scapiv1alpha3.FailState
return r
}

block := cr.Object[descriptor].(map[string]interface{})
block := cr.Object[specDescriptor].(map[string]interface{})

var crd *operatorsv1alpha1.CRDDescription
for _, owned := range csv.Spec.CustomResourceDefinitions.Owned {
Expand All @@ -254,30 +290,18 @@ func checkOwnedCSVDescriptors(cr unstructured.Unstructured, csv *operatorsv1alph
return r
}

if descriptor == statusDescriptor {
for key := range block {
for _, statDesc := range crd.StatusDescriptors {
if statDesc.Path == key {
delete(block, key)
break
}
}
}
}
if descriptor == specDescriptor {
for key := range block {
for _, specDesc := range crd.SpecDescriptors {
if specDesc.Path == key {
delete(block, key)
break
}
for key := range block {
for _, specDesc := range crd.SpecDescriptors {
if specDesc.Path == key {
delete(block, key)
break
}
}
}

for key := range block {
r.Errors = append(r.Errors, fmt.Sprintf("%s does not have a %s descriptor", key, descriptor))
r.Suggestions = append(r.Suggestions, fmt.Sprintf("Add a %s descriptor for %s", descriptor, key))
r.Errors = append(r.Errors, fmt.Sprintf("%s does not have a %s descriptor", key, specDescriptor))
r.Suggestions = append(r.Suggestions, fmt.Sprintf("Add a %s descriptor for %s", specDescriptor, key))
r.State = scapiv1alpha3.FailState
}
return r
Expand Down

0 comments on commit 93eecd0

Please sign in to comment.