#39 Require container-selinux when selinux-policy is installed (#1881218)
Merged 3 years ago by lsm5. Opened 3 years ago by jjelen.
rpms/ jjelen/podman container-selinux  into  master

file modified
+1 -1
@@ -97,7 +97,7 @@ 

  %endif

  %if 0%{?fedora} || 0%{?centos} >= 8 || 0%{?eln}

  Recommends: catatonit

- Recommends: container-selinux

+ Requires: (container-selinux if selinux-policy)

  Recommends: runc

  Recommends: slirp4netns >= 0.3.0-2

  %else

no initial comment

This has been a long and confusing battle. If you're on the container-teams mailing list, please see the thread of March 2 2020 with Subject: container-selinux package requirements

I read the thread, but (1.) there was no mention/suggestion of using boolean dependencies there and (2.) the thread has only two emails and doesn't give the impression of a "long and confusing battle". Can you elaborate?

Podman currently recommends container-selinux

rpm -q --recommends podman
catatonit
container-selinux
crun >= 0.14-2
fuse-overlayfs >= 0.3-8
podman-plugins = 2:2.1.0-2.fc33
runc
slirp4netns >= 0.3.0-2

It works perfectly if it is not installed. If you run on an SELinux enabled system then you need to install it.

The change needs to enclose the Boolean Expression in brackets or it will be just a list of three packages (container-selinux if selinux-policy)

1 new commit added

  • Enclose boolean dependency in parenthesis to unbreak podman installation
3 years ago

It works perfectly if it is not installed. If you run on an SELinux enabled system then you need to install it.

It is not my experience from the bug report[1] that it would work perfectly without container-selinux with enabled SELinux in the host. Quite the opposite and without looking to the audit log, the user is left without any pointers what could have been wrong and why the heck the container did not start.

It is at least terrible user experience for users of minimal installations.

https://bugzilla.redhat.com/show_bug.cgi?id=1881218

I agree the Requires: (container-selinux if selinux-policy) is the best solution here as depsolver will solve the problem for the end user without him requiring enabling soft deps. Enablement of soft deps is a wholesale solution which pulls in loads of deps for no reason also for all other packages.

LGTM, @dwalsh do you still object?

@jjelen could you squash commits please?

rebased onto 5620973

3 years ago

Done. Sorry for a delay.

Pull-Request has been merged by lsm5

3 years ago

@jjelen thanks, I guess we'll need this in f33 and f32 as well, yes? I can cherry-pick them there.

The issue we are trying to prevent is installing Buildah or Podman within a container.

We want to be able to eliminate the need for these huge packages being installed in a container image that is not going to use them.

If this change is made, could you verify that installing buildah into the fedora image does not pull in container-selinux and selinux-policy?

@dwalsh Actually, this is exactly what this change brings... Before this, if you tried to install podman or buildah inside a container, DNF would pull in both container-selinux and selinux-policy by default, unless you overrode DNF settings or added -x container-selinux. After this change, it won't pull in the SELinux packages unless selinux-policy is already installed.

So I think this PR is what you really want, not the Recommends. (Or perhaps Recommends: (container-selinux if selinux-policy), but that would break @jjelen's use case again :)

I think there is consensus that this approach is the correct and desired one -- in theory. What troubles me is that we used to have this conditional requires, but it was removed. Unfortunately there is no explanation in the commit message. @lsm5 do you have any memory of why you removed the conditional Requires?

@santiago hmm, don't remember tbh, but maybe I guess back then I thought Recommends would make everyone happy.

I am fine as long as the container use case works. I like the idea of not having to specify the
-x container-selinux

OK so broken-record me will now request: can we please, please get a nice, descriptive commit message that summarizes this discussion? Including: why this is being done (as in, the project/team that is blocked by it), why we believe it will work, why 'Recommends' isn't adequate? I really, really do not want to have this conversation again in six months.