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

HMS-3141: image: add new ContainerBuildable flag to OSTreeDiskImage #287

Merged
merged 2 commits into from
Dec 7, 2023

Conversation

mvo5
Copy link
Contributor

@mvo5 mvo5 commented Nov 29, 2023

One objective for bifrost images is that it should be possible to run osbuild inside a container. This can interfere with the selinux policies of the buildroot.

Inside the container everything is labeled
system_u:object_r:container_files_t. Labeling /usr/bin/osbuild as osbuild_exec_t is not possible in the general case because the host may not have osbuild-selinux installed that contains this type. The workaround is that the container labels osbuild itself as install_exec_t. This works fine however there is a selinux denial warning when the {,u}mount binary is called because the transition from install_t->mount_t is not allowed. The warning is "harmless" because install_t has enough privs to allow the {,u}mount binaries to work.

To silence this warning we can label {,u}mount in the buildroot as install_exec_t directly.

This commit allows to control this now via the ContainerBuildable flag that can be set on manifest.Build to enable this behavior.


Once this has landed https://github.com/osbuild/osbuild-deploy-container/pull/11/files#diff-895717f8731ac528a1e229abd5c4b2d22eb2c471412ea30c1df61b88dd90e4f5R49 will need an update to do img.ContainerBuildlable = true

mvo5 added a commit to mvo5/images that referenced this pull request Nov 29, 2023
When trying to create enough data for a full pipeline run for
osbuild#287 I struggled a bit
because it was not clear which items where faulty.

This commit adds more context to the errors. Sadly I had to
implement `assertPanicsWithErrorRegexp` because of
stretchr/testify#1304

I can contribute it upstream but for that it needs some tests
around it first :)
pkg/manifest/build.go Outdated Show resolved Hide resolved
mvo5 added a commit to mvo5/images that referenced this pull request Nov 29, 2023
When trying to create enough data for a full pipeline run for
osbuild#287 I struggled a bit
because it was not clear which items where faulty.

This commit adds more context to the errors. Sadly I had to
implement `assertPanicsWithErrorRegexp` because of
stretchr/testify#1304

I can contribute it upstream but for that it needs some tests
around it first :)
mvo5 added a commit to mvo5/images that referenced this pull request Nov 29, 2023
When trying to create enough data for a full pipeline run for
osbuild#287 I struggled a bit
because it was not clear which items where faulty.

This commit adds more context to the errors. Sadly I had to
implement `assertPanicsWithErrorRegexp` because of
stretchr/testify#1304

I can contribute it upstream but for that it needs some tests
around it first :)
One objective for bifrost images is that it should be possible
to run osbuild inside a container. This can interfere with the
selinux policies of the buildroot.

Inside the container everything is labeled
`system_u:object_r:container_files_t`. Labeling /usr/bin/osbuild as
`osbuild_exec_t` is not possible in the general case because
the host may not have `osbuild-selinux` installed that contains
this type. The workaround is that the container labels osbuild
itself as `install_exec_t`. This works fine however there is
a selinux denial warning when the `{,u}mount` binary is called
because the transition from `install_t`->`mount_t` is not allowed.
The warning is "harmless" because `install_t` has enough privs
to allow the `{,u}mount` binaries to work.

To silence this warning we can label `{,u}mount` in the buildroot
as `install_exec_t` directly.

This commit allows to control this now via the `ContainerBuildable`
flag that can be set on `manifest.Build` to enable this behavior.
mvo5 added a commit to mvo5/images that referenced this pull request Dec 4, 2023
When trying to create enough data for a full pipeline run for
osbuild#287 I struggled a bit
because it was not clear which items where faulty.

This commit adds more context to the errors. It uses the new
`assertx.PanicsWithErrorRegexp` that is needed because of
stretchr/testify#1304
(until fixed or contributed back).
github-merge-queue bot pushed a commit that referenced this pull request Dec 4, 2023
When trying to create enough data for a full pipeline run for
#287 I struggled a bit
because it was not clear which items where faulty.

This commit adds more context to the errors. It uses the new
`assertx.PanicsWithErrorRegexp` that is needed because of
stretchr/testify#1304
(until fixed or contributed back).
@mvo5 mvo5 changed the title image: add new ContainerBuildable flag to OSTreeDiskImage image: add new ContainerBuildable flag to OSTreeDiskImage (HMS-3141) Dec 5, 2023
Copy link
Member

@achilleas-k achilleas-k left a comment

Choose a reason for hiding this comment

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

This looks good to me. Should we mark it ready or does it need more work?

@mvo5 mvo5 marked this pull request as ready for review December 7, 2023 19:02
@mvo5
Copy link
Contributor Author

mvo5 commented Dec 7, 2023

This looks good to me. Should we mark it ready or does it need more work?

Thank you! I marked it ready, I only left it in draft as I wanted to make sure the API is okay :)

@achilleas-k achilleas-k added this pull request to the merge queue Dec 7, 2023
Merged via the queue into osbuild:main with commit 5d951a7 Dec 7, 2023
9 checks passed
mvo5 added a commit to mvo5/bootc-image-builder that referenced this pull request Dec 7, 2023
With the new
`image.NewOSTreeDiskImageFromContainer().ContainerBuildable` option it
is now possible to set the selinux policy so that there are no
selinux denials even when build in a container without osbuild-selinux
on the host. See osbuild/images#287 for
details.
@ochosi ochosi changed the title image: add new ContainerBuildable flag to OSTreeDiskImage (HMS-3141) HMS-3141: image: add new ContainerBuildable flag to OSTreeDiskImage Dec 7, 2023
github-merge-queue bot pushed a commit to osbuild/bootc-image-builder that referenced this pull request Dec 7, 2023
With the new
`image.NewOSTreeDiskImageFromContainer().ContainerBuildable` option it
is now possible to set the selinux policy so that there are no
selinux denials even when build in a container without osbuild-selinux
on the host. See osbuild/images#287 for
details.
mvo5 added a commit to mvo5/images that referenced this pull request Jan 25, 2024
Similar as osbuild#287 we need to
label `/usr/bin/{mount,umount}` as `install_exec_t` to prevent
an selinux denial warning when osbuild runs mount/unmount.

See dea1af4 for more details.
mvo5 added a commit to mvo5/images that referenced this pull request Jan 25, 2024
Similar as osbuild#287 we need to
label `/usr/bin/{mount,umount}` as `install_exec_t` to prevent
an selinux denial warning when osbuild runs mount/unmount.

See dea1af4 for more details.
github-merge-queue bot pushed a commit that referenced this pull request Jan 25, 2024
Similar as #287 we need to
label `/usr/bin/{mount,umount}` as `install_exec_t` to prevent
an selinux denial warning when osbuild runs mount/unmount.

See dea1af4 for more details.
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