Skip to content

Conversation

nikita-dubrovskii
Copy link
Contributor

@nikita-dubrovskii nikita-dubrovskii commented Oct 23, 2024

@dustymabe
Copy link
Member

nice. I'll check it out!

@nikita-dubrovskii nikita-dubrovskii force-pushed the issues_1877 branch 5 times, most recently from 30e4814 to 1c188e0 Compare October 29, 2024 12:46
@dustymabe
Copy link
Member

I was playing around with this trying to use file_contexts and target instead of labels and was never able to get it to work. I'm not opposed to leaving it like it is in this PR, but I also want to confirm that what I'm hitting isn't some sort of bug in the OSBuild PR upstream. I'll dig in more tomorrow.

@nikita-dubrovskii
Copy link
Contributor Author

I was playing around with this trying to use file_contexts and target instead of labels and was never able to get it to work. I'm not opposed to leaving it like it is in this PR, but I also want to confirm that what I'm hitting isn't some sort of bug in the OSBuild PR upstream. I'll dig in more tomorrow.

Do you have patch somewhere to check? file_contexts is somewhere under ostree/deploy/fcos/hash/, will check now using it

@nikita-dubrovskii
Copy link
Contributor Author

@dustymabe indeed there was a small issue, now that fixed. I've added patch with file_contexts usage on top

@nikita-dubrovskii nikita-dubrovskii force-pushed the issues_1877 branch 2 times, most recently from 322cfd5 to 11db312 Compare October 30, 2024 15:01
@dustymabe
Copy link
Member

added a few commits on top (the first one can be squashed into one of yours if you agree it adds value).

Let me know what you think!

@nikita-dubrovskii
Copy link
Contributor Author

LGTM, thx for your commits!

@nikita-dubrovskii nikita-dubrovskii force-pushed the issues_1877 branch 2 times, most recently from 60e36e9 to 3790fde Compare November 8, 2024 07:25
@nikita-dubrovskii nikita-dubrovskii force-pushed the issues_1877 branch 2 times, most recently from 2ea5590 to cc127c8 Compare November 15, 2024 13:36
nikita-dubrovskii and others added 3 commits November 15, 2024 12:07
This allows us to use the policy rather than hardcoding labels to set
on the mountpoints. The unfortunate thing here is that in order to
pick up a policy easily we have to use the `build` pipeline where
the files are written out plainly and we don't have to find where
the OSTree deployment is. I say unfortunate because right now for
FCOS the `build` pipeline was getting skipped because we weren't using
it for anything else, but now we'll be forced to build it.

That's OK I think, because we really want to start using a non-host
(i.e. non-COSA) buildroot for FCOS too if we can ever convince the
team/community to get python into it.

This commit also adds a comment to explain the "why" for the mkdir
and two selinux stages.
options:
uuid: random
label:
mpp-format-string: '{sd_fs_label}'
Copy link
Member

Choose a reason for hiding this comment

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

missing the mkdir here that the other platforms have to create /boot directory

Copy link
Member

Choose a reason for hiding this comment

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

ok I fixed this in a new push

Comment on lines 540 to 545
# We've created the filesystems. Now let's create the mountpoints (directories)
# on the filesystems and label them with appropriate SELinux labels. The labeling
# will happen once with just the root filesystem mounted and once with the boot
# filesystem mounted too (to make sure we get all potentially hidden mountpoints).
# https://github.com/coreos/fedora-coreos-tracker/issues/1771
Copy link
Member

Choose a reason for hiding this comment

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

This comment doesn't match the others exactly. That was my mistake. I'll fix it.

Copy link
Member

Choose a reason for hiding this comment

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

I fixed this in a new push.

Copy link
Member

@mike-nguyen mike-nguyen left a comment

Choose a reason for hiding this comment

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

The python patches to osbuild LGTM. I'm not too familiar with the osbuild bits but it seems sane to me.

@dustymabe dustymabe merged commit 088bc14 into coreos:main Nov 15, 2024
5 checks passed
@nikita-dubrovskii nikita-dubrovskii deleted the issues_1877 branch November 16, 2024 06:34
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.

3 participants