Image digests for referencing bundles

[anik120]

Description of the change:

• Change schema of OperatorBundle table to include field 'bundlepath'.
• Extend API to allow querying of the new bundlepath field.
• Modify API to return empty struct instead of an error while querying empty bundle

Reviewer Checklist

• Implementation matches the proposed design, or proposal is updated to match implementation
• Sufficient unit test coverage
• Sufficient end-to-end test coverage
• Docs updated or added to /docs
• Commit messages sensible and descriptive

[openshift-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: anik120
To complete the pull request process, please assign ecordell
You can assign the PR to them by writing /assign @ecordell in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[ecordell]

@anik120 I like this approach and think it will give us a good transition path on the client side.

Couple of things to think about:

• Do we want to use this schema change to test our new migration process? If so, we should wait for Database migration initialization #90 to merge, and add a migration here
• Tests 😄

[anik120]

@ecordell thank you for your feedback.

• I've looked at Database migration initialization #90 and looks like we still don't need to migrate our database as we're at ground zero with storing image-digests. Once we have long lived clusters with image digests stored in the registry and there needs to be a change in the schema, that is when we'll need to migrate the db.
• I've included a API test to test querying a bundle path(currently it just checks for an empty path as all bundlePath values are set to null). Also, seems like at some point we should refactor the API tests to mock a database instead of using an actual database which seems more like integration tests?

[ecordell]

Also, seems like at some point we should refactor the API tests to mock a database instead of using an actual database which seems more like integration tests?

It would be good to have two layers, but sqlite is often used as a mock sql database anyway, so IMO it's low on the list of priorities.

[ecordell]

/hold

I missed some things on my initial review.

There are multiple API calls in OLM that will need to have Paths returned to them. See the querier: https://github.com/operator-framework/operator-lifecycle-manager/blob/master/pkg/controller/registry/resolver/querier.go

(the source in that package is the grpc client to the registry server)

Because there are multiple places that we need to return this, could we extend the Bundle struct itself? We could add a new field path, and then clients can prefer the path field if it exists?

[ecordell]

probably shouldn't defer this, it won't get run until the server exits (which could be never)

[anik120]

There are multiple API calls in OLM that will need to have Paths returned to them. See the querier: https://github.com/operator-framework/operator-lifecycle-manager/blob/master/pkg/controller/registry/resolver/querier.go

(the source in that package is the grpc client to the registry server)

Because there are multiple places that we need to return this, could we extend the Bundle struct itself? We could add a new field path, and then clients can prefer the path field if it exists?

@ecordell that is actually a very good point. I've opened another PR where I've extended the Bundle struct and GetBundle API instead of adding a new API. I'll go ahead and close this PR if that looks good.

[anik120]

Closing in favor of #103