lint: Add --fix

[cgwalters]

I really wanted to avoid bootc being involved in being a "build system" - we should clearly support multiple build systems.

Right now we have bootc container lint which is an optional thing to invoke as part of a build, and it has grown more and more checks. It's readonly today.

In this issue I propose adding --fix (much like cargo clippy and cargo clippy --fix etc.).

Some things like /var/run we could easily apply the fix for (delete the link and let it be created by systemd-tmpfiles) e.g.

Fixing users and tmpfiles

In coreos/rpm-ostree#5230 I was thinking about trying to split out rpm-ostree's code for dealing with /var -> tmpfiles.d into something that can be invoked in a bit more standalone fashion.

However if we have --fix here...I think we could consider moving the tmpfiles.d and sysusers.d handling into this project instead. It's clearly within our dependency set already.

Does anyone have opinions on this?

If we did things this way then what would happen is all the tmpfiles.d code would move into this project, and rpm-ostree would just call it.

[cgwalters]

Tangential a while ago of course we added ostree container commit but then didn't do that much with it...but now that we've merged the ostree-container stuff here we should also make ostree container commit be the same thing as bootc container lint --fix.

[mrguitar]

I'm not apposed to this. The main con I see with relying on an extra post processing step like this is that users have to get into the weeds and know to run this. There may not be an easy way to avoid that. In general, I like the linting check and I think we should promote it more. A more controversial question is, do we even need to expose a separate "--fix" option? Why not fix by default? I see pros & cons either way.

[imbev]

It would be useful to ignore some fixes.

e.g.

• Base image
  • Creates /var/run for Flatpak
  • container lint --fix --ignore-var_run
• Derivative image
  • Uses /var/run for Flatpak
  • container lint --fix

[cgwalters]

Creates /var/run for Flatpak

What relevance does /var/run have for flatpak specifically? Nothing should be using /var/run anymore, it should all be /run.

Only for compatibility do we have a symlink /var/run -> /run, which we relatively recently added to the reference fedora base images in https://gitlab.com/fedora/bootc/base-images/-/merge_requests/71

The only fatal problem (that the lint checks for) is if /var/run exists and is not a symlink; that's always going to cause serious problems.

container lint --fix for /var/run...well if it's a directory, we'd actually need in theory to do a static analysis and make sure that everything installed in there has corresponding systemd-tmpfiles - which in practice it should.

[imbev]

Sorry, I meant /var/roothome, not /var/run.

[cgwalters]

Sorry, I meant /var/roothome, not /var/run.

It's tempting to add /var/roothome to the base image as well, though each time we step into this world of /var we increase the potential to confuse people into putting stuff there into their images.

But still the same question though; what relevance does /var/roothome have for flatpak? Is it that the CLI tool just expects it to exist? I guess that wouldn't be surprising, but ultimately flatpak when invoked as root in a container build I don't think should be trying to touch the home directory.

[cgwalters]

A more controversial question is, do we even need to expose a separate "--fix" option? Why not fix by default? I see pros & cons either way.

The rationale for not doing it by default is that anything we fix is really something better fixed elsewhere to start for most of these things...and the original naming was intended for that.

This said, there's some things we know we can really safely and sanely fix (like when you have /var/run as an empty directory) but others especially around what I'm thinking around tmpfiles.d and sysusers.d have the potential for false positives and would be a lot more invasive I feel to do in a command that was formerly just informative.

[imbev]

But still the same question though; what relevance does /var/roothome have for flatpak? Is it that the CLI tool just expects it to exist? I guess that wouldn't be surprising, but ultimately flatpak when invoked as root in a container build I don't think should be trying to touch the home directory.

This was the case previously. After a quick test on Stream 10, it appears to now be unnecessary.

Having a fix flag with fine-grained control of ignored fixes would allow more leeway for aggressive fixes.