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: manually label {/etc/fstab,tmpfile.d/bootc-root-ssh.conf} #389

Merged
merged 1 commit into from
Mar 17, 2024

Conversation

mvo5
Copy link
Contributor

@mvo5 mvo5 commented Mar 12, 2024

Right now bootc supports an experimental install from a non-selinux host when using the BOOTC_SKIP_SELINUX_HOST_CHECK=1 option.

This is nice and works relatively well. However files written during the install like /etc/fstab or the tmpfiles.dfile in /etc/tmpfile.d/bootc-root-ssh.conf must be labeled too.

This commit adds a (rather crude) manual way to do this.

Closes #362

Copy link

openshift-ci bot commented Mar 12, 2024

Hi @mvo5. Thanks for your PR.

I'm waiting for a containers member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@github-actions github-actions bot added the area/install Issues related to `bootc install` label Mar 12, 2024
@jeckersb
Copy link
Contributor

/ok-to-test

lib/src/install.rs Outdated Show resolved Hide resolved
Copy link
Collaborator

@cgwalters cgwalters left a comment

Choose a reason for hiding this comment

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

Thanks, clearly a next step here is to add a CI check that walks the filesystem tree for unlabeled files, should be an easy add. I can look at that.

This said, I find myself thinking we may just need to pull out the big hammer of doing a force relabel of /etc and /var in this case.

lib/src/install.rs Outdated Show resolved Hide resolved
@mvo5 mvo5 force-pushed the manual-labor branch 4 times, most recently from 3210c5e to 23bbc5a Compare March 14, 2024 15:47
@ondrejbudai
Copy link
Contributor

/ok-to-test

lib/src/install/osconfig.rs Outdated Show resolved Hide resolved
lib/src/install.rs Outdated Show resolved Hide resolved
lib/src/install/osconfig.rs Outdated Show resolved Hide resolved
Right now bootc supports an experimental install from a non-selinux
host when using the `BOOTC_SKIP_SELINUX_HOST_CHECK=1` option.

This is nice and works relatively well. However files written
during the install like /etc/fstab or the tmpfiles.dfile
in /etc/tmpfile.d/bootc-root-ssh.conf must be labeled too.

This commit adds a (rather crude) manual way to do this.

Closes containers#362

Signed-off-by: Michael Vogt <[email protected]>
@mvo5
Copy link
Contributor Author

mvo5 commented Mar 15, 2024

/ok-to-test

@cgwalters
Copy link
Collaborator

The prow/kola job failure is unrelated, should be fixed by coreos/coreos-assembler@836f8ac

@mvo5
Copy link
Contributor Author

mvo5 commented Mar 15, 2024

The prow/kola job failure is unrelated, should be fixed by coreos/coreos-assembler@836f8ac

Thank you! This should be okay to review now, I am not super familiar with rust so not sure if I overcomplicated things (or otherwise did non-idiomatic stuff) but it seems okay (I am really wondering if there is a simpler way to pass "function pointers", happy to look more into this if it's not a good/idiomatic solution). Or if there is a simple(r) way to mock State, in go I would probably just use an interface Labeler with just lsm_label and use that, may I should create a trait "labeler"? Would that be an idiomatic way to test this?

[edit: thoughts about how to avoid the passing in of the function]

Copy link
Collaborator

@cgwalters cgwalters left a comment

Choose a reason for hiding this comment

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

Thanks, let's go ahead with this as it is an improvement. There is a more complex approach I have in mind as a followup though.

@cgwalters
Copy link
Collaborator

/retest

@cgwalters cgwalters enabled auto-merge March 17, 2024 17:21
@cgwalters cgwalters merged commit 92940b1 into containers:main Mar 17, 2024
14 checks passed
@mvo5 mvo5 deleted the manual-labor branch March 18, 2024 07:52
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` ok-to-test
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Missing SELinux labeling on some files when built on SELinux-disabled hosts
4 participants