Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

>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 this issue Aug 28, 2023 · 9 comments

Comments

@stevekuznetsov
Copy link
Member

          >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 #144 (comment)

@stevekuznetsov
Copy link
Member Author

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
Copy link
Member

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
Copy link
Member Author

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
Copy link
Member

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.

  • opm generate dockerfile generates a dockerfile that copies over the entire FBC root directly into the image, .indexignore files and all
  • opm serve understands how to ignore files called out by .indexignore, so existing FBC catalog images have this feature and users can rely on it
  • catalogd uses various declcfg.Load* functions to read FBC filesystems, and all of those functions account for the .indexignore file.

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
Copy link
Member Author

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
Copy link
Contributor

grokspawn commented Aug 29, 2023

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
Copy link
Contributor

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
Copy link
Member

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
Copy link
Member

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants