scorecard - missing version field in CSV causes Have Resources test to fail

[jmccormick2001]

Bug Report

What did you do? when running the memcached example, and updating the generated CSV with the owned CRD block, I forgot to include the CRD version field.

What did you expect to see? I would expect the operator-sdk scorecard to detect and report the missing field as a suggestion or error instead of the scorecard fail message generated below.

What did you see instead? Under which circumstances?

if you by mistake leave out the spec.customresourcedefinitions.owned.version field in your CSV, you get a failure from operator-sdk scorecard as follows:

   "tests": [
    {
      "state": "fail",
      "name": "Owned CRDs have resources listed",
      "description": "All Owned CRDs contain a resources subsection",
      "earnedPoints": 0,
      "maximumPoints": 0,
      "suggestions": [],
      "errors": []
    },

Environment

• operator-sdk version:

operator-sdk version operator-sdk version: "v0.10.0-36-gb557289", commit: "b557289a7d5c44a4187c0e695ec9f4084824d6ca", go version: "go1.12.7 linux/amd64"

built off of master branch/HEAD.

• go version:

go version go1.12.7 linux/amd64

• Kubernetes version information:

Client Version: version.Info{Major:"1", Minor:"15", GitVersion:"v1.15.3", GitCommit:"2d3c76f9091b6bec110a5e63777c332469e0cba2", GitTreeState:"clean", BuildDate:"2019-08-19T11:13:54Z", GoVersion:"go1.12.9", Compiler:"gc", Platform:"linux/amd64"} Server Version: version.Info{Major:"1", Minor:"15", GitVersion:"v1.15.3", GitCommit:"2d3c76f9091b6bec110a5e63777c332469e0cba2", GitTreeState:"clean", BuildDate:"2019-08-19T11:05:50Z", GoVersion:"go1.12.9", Compiler:"gc", Platform:"linux/amd64"}

• Kubernetes cluster kind:

• Are you writing your operator in ansible, helm, or go?

go operator, memcached example

Possible Solution line 242 of olm_tests.go checks to see if the CRD and CR versions and Kinds match, if they do, then it processes as you would expect, if the version doesn't exist then the string comparison will fail, however nothing is reported back to the end user other than the failed HaveResourcesTest.

Perhaps an 'else' could be added here so that if the version and kind do not match, then some sort of error or suggestion be reported back to the end user so they know to add the missing version field in the CSV.

Additional context Add any other context about the problem here.

[jmccormick2001]

    {
      "state": "fail",
      "name": "Owned CRDs have resources listed",
      "description": "All Owned CRDs contain a resources subsection",
      "earnedPoints": 0,
      "maximumPoints": 0,
      "suggestions": [],
      "errors": [
        "verify that version fields match between CR and CSV spec.customresourcedefinitions.owned.version"
      ]
    }

after the suggested change, the error reported back to the user would appear as above.

[joelanford]

@jmccormick2001 Yep this definitely sounds like an issue.

In the near future, we're planning to use https://github.com/operator-framework/api to store the OLM APIs and their static validators/linters. And part of our upcoming scorecard work is to run those validators and linters against the operator bundle, which would have likely caught this issue.

In these tests, we're testing a single CR at a time, so we have to loop over the CSV's owned CRDs to find the one that goes with this CR. That's why we're making the GVK check. If I understand correctly, I think we'd produce erroneous suggestions for any CSVs that contain multiple owned CRDs if we just added an else to that conditional.

/cc @estroz @gallettilance Thoughts on what we should do in the meantime? Is the static validation library ready to be incorporated into scorecard? Maybe as a separate OLM test?

If that's not quite ready to be integrated into scorecard, maybe we error out of that test if any of the fields we're comparing are empty?

Looks like there could be similar issues at lines 202, 393, and 432.

[openshift-bot]

Issues go stale after 90d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle stale. Stale issues rot after an additional 30d of inactivity and eventually close. Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle stale

[openshift-bot]

Stale issues rot after 30d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle rotten. Rotten issues close after an additional 30d of inactivity. Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle rotten /remove-lifecycle stale

[estroz]

/lifecycle frozen

[jmccormick2001]

an update on this one, awhile back when OLM bundle validation test was written and included into scorecard, I also had the validation check the CSV upon scorecard execution...you now get this error when you remove for example the CSV's CRD version field....the user now will get the following error message if the CSV is missing the version field:

jeffmc@doppio114 ~/projects/memcached-operator> operator-sdk scorecard -o text INFO[0000] Using config file: /home/jeffmc/projects/memcached-operator/.osdk-scorecard.yaml ERRO[0000] Plugin OLM Integration failed with error (error validating ClusterServiceVersion: Error: Value cache.example.com/v1alpha1, Kind=Memcached: example must have a provided API ) OLM Integration
CR: Labels: Errors: plugin error Log: error validating ClusterServiceVersion: Error: Value cache.example.com/v1alpha1, Kind=Memcached: example must have a provided API : Logs:

I think this will suffice in addressing this issue.

[dvaldivia]

@jmccormick2001 it's not clear to me how the example must have a provided API error was solved, the validation is returning this error for the example on the CSV, yet the example does have API, in my case

[
  {
    "apiVersion": "v1",
    "kind": "Namespace",
    "metadata": {
      "name": "tenant-lite"
    }
  },...

So what is the missing API that error might be referring to?

[cniackz]

Hello @dvaldivia,

Even when your example have API as you shown, you have to reference an existing API in your CRD:

    alm-examples: |
      [
        {
          "apiVersion": "minio.min.io/v2",
          "kind": "Tenant",
          "spec": {
            "group": "minio.min.io"
          }
        }
      ]

Where v2 is located in the customresourcedefinition

  customresourcedefinitions:
    owned:
    - version: "v2"
      description: Tenant
      displayName: Tenant
      kind: Tenant
      name: tenants.minio.min.io

Then test will pass:

$ operator-sdk scorecard . -o text --selector=suite=olm -c=config.yaml
--------------------------------------------------------------------------------
Image:      quay.io/operator-framework/scorecard-test:latest
Entrypoint: [scorecard-test olm-bundle-validation]
Labels:
    "suite":"olm"
    "test":"olm-bundle-validation-test"
Results:
    Name: olm-bundle-validation
    State: pass

By the way, I was having the exact same problem exposed in this place before:

$ operator-sdk scorecard . -o text --selector=suite=olm -c=config.yaml
--------------------------------------------------------------------------------
Image:      quay.io/operator-framework/scorecard-test:latest
Entrypoint: [scorecard-test olm-bundle-validation]
Labels:
    "suite":"olm"
    "test":"olm-bundle-validation-test"
Results:
    Name: olm-bundle-validation
    State: fail

    Suggestions:
        Warning: Value minio.min.io/v1, Kind=Tenant: provided API should have an example annotation
    Errors:
        Error: Value minio.min.io/v1, Kind=Namespace: example must have a provided API

But got solved with my new yaml file. I will take care of this for redhat market place. But in the meantime, I am creating as much as personal notes in my repo: https://github.com/cniackz/minio/wiki/RedHat-MarketPlace-Operator

My plan is to get this working first, and then little by little make this automatically with our olm.sh script.

Anyway, hope it helps in the future.

Regards, -Cesar