operator-registry icon indicating copy to clipboard operation
operator-registry copied to clipboard
verified publisher icon operator-framework

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

Open cdjohnson opened this issue 3 years ago • 3 comments

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)

cdjohnson avatar May 27 '22 11:05 cdjohnson

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.

joelanford avatar Jun 22 '22 02:06 joelanford

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

jdockter avatar Jul 24 '22 21:07 jdockter

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?

joelanford avatar Jul 25 '22 13:07 joelanford