operator-framework / catalogd

On-cluster FBC catalog content server
Apache License 2.0
16 stars 32 forks source link

>We can assume that catalogs being added here were built in a pipeline that ran opm validate <directory> --flags as the final step. #146

Open stevekuznetsov opened 1 year ago

stevekuznetsov commented 1 year ago
          >We can assume that catalogs being added here were built in a pipeline that ran opm validate <directory>  --flags as the final step.

I feel this is not a safe assumption to make. IIUC opm validate ... does not mean non-FBC content does not exist in the structure, but rather it follows the FBC directory structure and any non-FBC content files are properly listed in the .indexignore file. I agree the declcfg.DeclarativeConfig object makes this a bit more simplistic, but I would again vote for this being done in a follow up. I want to try and avoid us expanding the scope of this PR so we can make progress on this implementation. We should create follow up issues for optimizations like this.

_Originally posted by @everettraven in https://github.com/operator-framework/catalogd/pull/144#discussion_r1307671317_

stevekuznetsov commented 1 year ago

The crux of the question here is - we're unpacking an index and get a filesystem out of it. We'd like to serve the content from that FBC filesystem as-is. Any work we can do at index creation time to make this easier is well worth it.

If we need to support an .indexignore, can we do that by not comitting those files to the index image? To what end would we want that content in the image layers in the first place?

joelanford commented 1 year ago

If we need to support an .indexignore, can we do that by not comitting those files to the index image? To what end would we want that content in the image layers in the first place?

That's a possibility to explore, but also a breaking change in the FBC spec. FBC in catalog images is currently allowed to ship with .indexignore (and ignored) files.

The most obvious first step, IMO, is to export this function in operator registry and use it here: https://github.com/operator-framework/operator-registry/blob/56771f76adbefa30190c4199d1cc970299f3ef04/alpha/declcfg/load.go#L88

stevekuznetsov commented 1 year ago

Is it possible for a user to rely on that side-effect of the implementation of today's FBC images? Who consumes other than OLM servers? What's the implication of "breaking" that?

joelanford commented 1 year ago

Is it possible for a user to rely on that side-effect of the implementation of today's FBC images?

I'm not exactly sure what you're asking, but I think the answer is yes no matter what.

I'm absolutely not opposed to saying something like "Catalogd expects FBC filesystems to have been pre-stripped of ignored files" if we think that's the right choice to make.

I think we should teach opm how to push an FBC directory directly to an image registry. You obviously don't want to push files that you're going to ignore anyway, so it totally makes sense to strip at push time.

stevekuznetsov commented 1 year ago

You wrote:

a breaking change in the FBC spec

I am asking - it's "breaking" in the sense that it changed, but would it be possible for a user to notice and care? In what situations would it be possible for a user to have been depending on the fact that, today, the ignored files are in the image, such that the user is "broken" with this change?

Or are you just noting that it is a change, but not one that would block us from being able to do it?

grokspawn commented 1 year ago

I think we should teach opm how to push an FBC directory directly to an image registry. You obviously don't want to push files that you're going to ignore anyway, so it totally makes sense to strip at push time.

I'm hesitant to add pushing to opm because I would like to reduce the number of roles where we require opm to manipulate FBC. We also have a backlog of registry credential issues in that repo that make me more hesitant to add one more.

As I mentioned on the call, I'd prefer a flag for opm validate which would relate to these more strict requirements. Like where we change the default behavior for the most restrictive set (push-ready), and for the relaxed set we use something similar to

opm validate --relaxed   # validates the contribution with relaxed restrictions, including .indexignore files and their mentions

And then obviously we continue to require that catalog images pass opm validate before being made generally available.

grokspawn commented 1 year ago

And then obviously we continue to require that catalog images pass opm validate before being made generally available.

Revise: and then we can document that opm serve expects catalog images to comply with the more-restrictive set of validations.

joelanford commented 1 year ago

I think the validation route is bound to be problematic if we start adding flags to it willy-nilly (not saying that's what you're suggesting, just calling out that we need to be cognizant of the kind of validations that we might want to relax or make more strict in the future)

One person's relax is another person's ... non-relaxed treasure? Basically, there are multiple, disparate, and orthogonal ways to relax validation (or make it more strict).

joelanford commented 1 year ago

The opm push idea is more of a long-term vision. If we want strict control over what gets pushed to an image registry, we need to be in the loop. opm generate dockerfile followed by "pretty please, use this dockerfile, will you?" is not a spec. And it isn't anything we can depend on or enforce.

If we want a "minimal FBC" image that contains only FBC files, that is just a different thing than standard FBC, according to the FBC spec, which allows .indexignore files. I would envision an image label being added to the image that says, "hey, i promise its just FBC here. concatenate away".

And then clients can look at image labels (or mediatype, or artifact type, etc.) to understand how to consume the image.