opm registry bundle add accepts invalid properties causing all FBC renderings to fail

[cdjohnson]

Problem: Two of our product teams inadvertently added a properties.yaml to their bundle that is malformed:

properties:
  - type: olm.package

This results in all opm commands to fail to query the bundle, as it cannot successfully convert it to FBC:
opm alpha list packages [icr.io/cpopen/ibm-operator-catalog:latest](http://icr.io/cpopen/ibm-operator-catalog@@sha256:17b9079371d52fcfdd5951e54b330e373a3bf791501eaa2ef339e9f573900482)

FATA[0022] render reference "icr.io/cpopen/ibm-operator-catalog:latest": invalid index:
├── invalid package "ibm-cpd-rstudio":
│   └── invalid channel "v1.0":
│       └── invalid bundle "ibm-cpd-rstudio.v1.0.9":
│           └── must be exactly one property with type "olm.package"
└── invalid package "ibm-cpd-ws-runtimes":
    └── invalid channel "v1.0":
        └── invalid bundle "ibm-cpd-ws-runtimes.v1.0.9":
            └── must be exactly one property with type "olm.package" 

When this bundle is added to the SQLITE catalog via opm registry add, it is blindly added to the catalog with an empty value:

sqlite3 index.db "SELECT type, value, operatorbundle_name, operatorbundle_version, operatorbundle_path FROM properties WHERE operatorbundle_name='ibm-cpd-rstudio.v1.0.9'"

olm.package|null|ibm-cpd-rstudio.v1.0.9|1.0.9|icr.io/cpopen/ibm-cpd-rstudio-operator-bundle@sha256:a3da5095fcd46b0b1e6f3f0af016da97b16215ed898a3059ff9047039626c13d
olm.package|{"packageName":"ibm-cpd-rstudio","version":"1.0.9"}|ibm-cpd-rstudio.v1.0.9|1.0.9|icr.io/cpopen/ibm-cpd-rstudio-operator-bundle@sha256:a3da5095fcd46b0b1e6f3f0af016da97b16215ed898a3059ff9047039626c13d
olm.gvk|{"group":"rstudio.cpd.ibm.com","kind":"RStudioAddon","version":"v1beta1"}|ibm-cpd-rstudio.v1.0.9|1.0.9|icr.io/cpopen/ibm-cpd-rstudio-operator-bundle@sha256:a3da5095fcd46b0b1e6f3f0af016da97b16215ed898a3059ff9047039626c13d
olm.package|{"packageName":"ibm-cpd-rstudio","version":"1.0.9"}|ibm-cpd-rstudio.v1.0.9|1.0.9|icr.io/cpopen/ibm-cpd-rstudio-operator-bundle@sha256:a3da5095fcd46b0b1e6f3f0af016da97b16215ed898a3059ff9047039626c13d

The GRPC API tolerates this:

$ CATALOG_PACKAGE=ibm-cpd-rstudio

$ grpcurl -plaintext  -d '{"name":"'$CATALOG_PACKAGE'"}' localhost:50051 api.Registry/GetPackage
{
  "name": "ibm-cpd-rstudio",
  "channels": [
    {
      "name": "v1.0",
      "csvName": "ibm-cpd-rstudio.v1.0.9"
    }
  ],
  "defaultChannelName": "v1.0"
}

$ grpcurl -plaintext localhost:50051 api.Registry/ListBundles | jq '. | select (.packageName | contains("'$CATALOG_PACKAGE'")) |{channelName: .channelName, name: .csvName, version: .version, replaces: .replaces, skipRange: .skipRange, skips: .skips, bundlePath: .bundlePath }' | jq -s -c 'sort_by(.channelName,.name)[]'

{"channelName":"v1.0","name":"ibm-cpd-rstudio.v1.0.0","version":"1.0.0","replaces":null,"skipRange":null,"skips":null,"bundlePath":"icr.io/cpopen/ibm-cpd-rstudio-operator-bundle@sha256:9bc69fe363b4684434742302b8060266da58cfa6de8eb981767e4ccbb2467887"}
{"channelName":"v1.0","name":"ibm-cpd-rstudio.v1.0.1","version":"1.0.1","replaces":"ibm-cpd-rstudio.v1.0.0","skipRange":">=1.0.0 <1.0.1","skips":null,"bundlePath":"icr.io/cpopen/ibm-cpd-rstudio-operator-bundle@sha256:f171da79d37fd805a507261388df52099c76e018f989200e36e1da3b1bd7f202"}
{"channelName":"v1.0","name":"ibm-cpd-rstudio.v1.0.2","version":"1.0.2","replaces":"ibm-cpd-rstudio.v1.0.1","skipRange":">=1.0.0 <1.0.2","skips":null,"bundlePath":"icr.io/cpopen/ibm-cpd-rstudio-operator-bundle@sha256:59eaf3dbd5788b93b8a61ad04e9acceb1772c0fd7f53da74537c31b767ae093f"}
{"channelName":"v1.0","name":"ibm-cpd-rstudio.v1.0.3","version":"1.0.3","replaces":"ibm-cpd-rstudio.v1.0.2","skipRange":">=1.0.0 <1.0.3","skips":null,"bundlePath":"icr.io/cpopen/ibm-cpd-rstudio-operator-bundle@sha256:735b85dbefe353034699866b202794b03845faf9dc297ff5c07036640d88dfcb"}
{"channelName":"v1.0","name":"ibm-cpd-rstudio.v1.0.4","version":"1.0.4","replaces":"ibm-cpd-rstudio.v1.0.3","skipRange":">=1.0.0 <1.0.4","skips":null,"bundlePath":"icr.io/cpopen/ibm-cpd-rstudio-operator-bundle@sha256:8283d30928a5b2193d8e4dd523436482000ec1457fb621d7ee426c4cd801a860"}
{"channelName":"v1.0","name":"ibm-cpd-rstudio.v1.0.6","version":"1.0.6","replaces":"ibm-cpd-rstudio.v1.0.4","skipRange":">=1.0.0 <1.0.6","skips":null,"bundlePath":"icr.io/cpopen/ibm-cpd-rstudio-operator-bundle@sha256:c4066835c883ac46d8cbf24fa9d932df6ea46ab3308380e7eae21c81a94cf7a8"}
{"channelName":"v1.0","name":"ibm-cpd-rstudio.v1.0.7","version":"1.0.7","replaces":"ibm-cpd-rstudio.v1.0.6","skipRange":">=1.0.0 <1.0.7","skips":null,"bundlePath":"icr.io/cpopen/ibm-cpd-rstudio-operator-bundle@sha256:0aed350c2690c3921cf685a29e89d3f7a0bdb1543bf7c8ea80c2bc73fee73767"}
{"channelName":"v1.0","name":"ibm-cpd-rstudio.v1.0.8","version":"1.0.8","replaces":"ibm-cpd-rstudio.v1.0.7","skipRange":">=1.0.0 <1.0.8","skips":null,"bundlePath":"icr.io/cpopen/ibm-cpd-rstudio-operator-bundle@sha256:0cd8b512fb84c1612856e5fe3268223e740ed26eddc988caae91d34d34cda618"}
{"channelName":"v1.0","name":"ibm-cpd-rstudio.v1.0.9","version":"1.0.9","replaces":"ibm-cpd-rstudio.v1.0.8","skipRange":">=1.0.0 <1.0.9","skips":null,"bundlePath":"icr.io/cpopen/ibm-cpd-rstudio-operator-bundle@sha256:a3da5095fcd46b0b1e6f3f0af016da97b16215ed898a3059ff9047039626c13d"}

A failing bundle example is:
icr.io/cpopen/ibm-cpd-rstudio-operator-bundle@sha256:a3da5095fcd46b0b1e6f3f0af016da97b16215ed898a3059ff9047039626c13d

Our public catalog instance with this invalid bundle:
icr.io/cpopen/ibm-operator-catalog@sha256:17b9079371d52fcfdd5951e54b330e373a3bf791501eaa2ef339e9f573900482

I think two things should happen here:

1. opm registry|index add needs to do some validation on the properties object to make sure that the known properties are valid.
2. The FBC render code could (this is debatable) tolerate this invalid state much like the GRPC server does.

The effect of this problem is that the entire catalog becomes poisoned to all opm queries that rely on the FBC conversion (which is most)

[joelanford]

The FBC render code could (this is debatable) tolerate this invalid state much like the GRPC server does.

I think we'd at least want to keep opm validate failing in this case. It seems like the transition from sqlite to FBC is a good opportunity to fix some of the warts that exist in the sqlite implementation.

To be honest, if anything I'd like to ratchet the validation up even higher (e.g. actually validate that icon data and mediatype are valid and match or that the relatedImage fields are valid)

I could maybe see an argument for skipping validation during opm render|migrate when dealing with sqlite images/dbs, with the idea that you could successfully render something to FBC, validate it to see what's wrong and then apply manual fixes to get opm validate to pass.

[jdockter]

@joelanford wasn't clear to me on your comments above on what changes, if any, you were recommending and in what code.

[joelanford]

I'm not sure I had jumped to making recommendations yet.

What happens on the OLM side when it receives a bundle like this from a ListBundles call that has a malformed olm.package property? Does OLM tolerate it as well? Is that bundle installable via OLM?