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

prepare-root: Introduce ostree/prepare-root.conf && sysroot.readonly improvements #2930

Merged
merged 3 commits into from
Jul 26, 2023

Conversation

cgwalters
Copy link
Member

@cgwalters cgwalters commented Jul 13, 2023

prepare-root: Introduce ostree/prepare-root.conf

Using the repository configuration for configuration of this
program was always a bit hacky.

But actually with composefs, we really must validate
the target root before we parse anything in it.

Let's add a config file for ostree-prepare-root that can live
in the initramfs, which will already have been verified.

In the future we'll also add configuration for composefs here.

We expect OS builders to drop this in /usr/lib/ostree/prepare-root.conf,
but system local configuration can live in /etc.


prepare-root: Default sysroot.readonly=true if composefs

Not because it's logically required or anything, but because
it's just a good idea.


prepare-root: Don't parse target root when composefs enabled

We shouldn't load anything from the target root filesystem before
verifying its integrity if composefs is enabled.

In effect, we want to force composefs users to migrate to
/usr/lib/ostree/prepare-root.conf which lives in the initramfs.
(But because we enable sysroot.readonly=true if composefs is enabled
too, they don't actually need to)


@openshift-ci
Copy link

openshift-ci bot commented Jul 13, 2023

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@cgwalters cgwalters force-pushed the prepare-root-config3 branch 2 times, most recently from ad3d7cb to 5da297b Compare July 14, 2023 11:28
@cgwalters
Copy link
Member Author

/test all

@cgwalters
Copy link
Member Author

Discussion on this is saying that we also need the readonly sysroot stuff in the initramfs, which is an argument for a config file.

@cgwalters
Copy link
Member Author

Leaning towards an /etc/ostree/prepare-root.conf keyfile that gets installed by our dracut module.

@cgwalters
Copy link
Member Author

Split out prep work into #2935 - this one will morph into more configuration work.

@cgwalters cgwalters changed the title prepare-root: Also support configuration via OSTREE_COMPOSEFS prepare-root: Introduce ostree/prepare-root.conf && sysroot.readonly improvements Jul 23, 2023
@cgwalters cgwalters marked this pull request as ready for review July 23, 2023 18:41
@cgwalters
Copy link
Member Author

OK I've reworked this PR a bit to not (yet) be about configuring composefs via keyfile, but instead just the preparatory work around correctly handling sysroot.readonly w/composefs.

Using the repository configuration for configuration of this
program was always a bit hacky.

But actually with composefs, we really must validate
the target root *before* we parse anything in it.

Let's add a config file for `ostree-prepare-root` that can live
in the initramfs, which will already have been verified.

In the future we'll also add configuration for composefs here.

We expect OS builders to drop this in `/usr/lib/ostree/prepare-root.conf`,
but system local configuration can live in `/etc`.
Not because it's logically required or anything, but because
it's just a good idea.
We shouldn't load anything from the target root filesystem *before*
verifying its integrity if composefs is enabled.

In effect, we want to force composefs users to migrate to
`/usr/lib/ostree/prepare-root.conf` which lives in the initramfs.
(But because we enable sysroot.readonly=true if composefs is enabled
 too, they don't actually need to)
@cgwalters
Copy link
Member Author

Rebased 🏄 for textual (not semantic) merge conflict.

Copy link
Collaborator

@ericcurtin ericcurtin left a comment

Choose a reason for hiding this comment

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

LGTM

@ericcurtin ericcurtin merged commit 66e4255 into ostreedev:main Jul 26, 2023
20 checks passed
// Encourage porting to the new config file
if (sysroot_readonly)
g_print ("Found legacy sysroot.readonly flag, not configured in %s\n",
PREPARE_ROOT_CONFIG_PATH);
Copy link
Member

Choose a reason for hiding this comment

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

Note: The fedora readonly migration service will enable this and trigger this warning.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, we will need to have a long period of migrating things to the new API. It doesn't actually contain the string "warning".

markmc added a commit to markmc/ostree that referenced this pull request Nov 24, 2023
The explanation of sysroot.readonly is a little confusing - we say
that "everything else is mounted read-only" but it's perhaps clearer
to say /sysroot is mounted read-only.

Also note that read-only is the default with composefs.

Finally, document the option in ostree.repo-config even though it is
now considered legacy - as of commit 22b8e4f (ostreedev#2930) - it is still
commonly seen in repo configs, so users will look to understand
what it means.
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

Successfully merging this pull request may close these issues.

4 participants