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

install: Mount /boot readonly by default #341

Merged
merged 1 commit into from
Feb 14, 2024

Conversation

cgwalters
Copy link
Collaborator

As we want to support enabling root.transient in some images, this means that things like apt|dnf install foo literally just works out of the box.

However...we have a looming danger around things like kernels. Typically the package installation scripts for those aren't going to handle this correctly.

Let's mount /boot readonly by default, as we have been doing in Fedora CoreOS and derivatives for a while.

Now I'm not totally happy with this because ultimately I think this should be configurable by the OS, not hardcoded in bootc. We have some thought to put in to exactly how that's exposed.

But for now let's set the precedent here.

As we want to support enabling `root.transient` in some images,
this means that things like `apt|dnf install foo` literally
just works out of the box.

However...we have a looming danger around things like
kernels.  Typically the package installation scripts
for those aren't going to handle this correctly.

Let's mount `/boot` readonly by default, as we have been doing
in Fedora CoreOS and derivatives for a while.

Now I'm not totally happy with this because ultimately
I think this should be configurable by the OS, not hardcoded
in bootc.  We have some thought to put in to exactly how
that's exposed.

But for now let's set the precedent here.

Signed-off-by: Colin Walters <walters@verbum.org>
@github-actions github-actions bot added the area/install Issues related to `bootc install` label Feb 14, 2024
@cgwalters cgwalters merged commit e1ef710 into containers:main Feb 14, 2024
11 checks passed
@cgwalters
Copy link
Collaborator Author

Ah we got bit by #57 ...I enabled auto-merge letting it be blocked on a review, but that got reverted again.

Copy link
Contributor

@mvo5 mvo5 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can give this a retro 👍 - I actually read it before it got merged and it looks (and looked) good but as I'm not super fluent in rust for a real +1 I would have checked it out and played a bit first.

@cgwalters
Copy link
Collaborator Author

Thanks! Post-merge review is still good.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/install Issues related to `bootc install`
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants