-
Notifications
You must be signed in to change notification settings - Fork 20
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
Unify the EL7 and EL8 content and fix the SELinux policy packaging #20
Conversation
548e4cf
to
ebe92e8
Compare
437585f
to
5a0b9b4
Compare
The actual SELinux policy content was identical between EL7 and EL8, with minor differences in the RPM spec file. Furthermore, the scripts for actually doing these builds were not significantly different. This unifies them so that we don't suffer maintaining two versions for no reason. Additionally, rewrite the SELinux policy package RPM spec file to align with SELinux module packaging to make it easier to ship in distributions. Signed-off-by: Neal Gompa <ngompa13@gmail.com>
5a0b9b4
to
8e7fb44
Compare
@Conan-Kudo there is a specific reason why we split the RPMs. When installing the policy built on a We explicitly "cloned" the policies between el7 and el8 so that we could easily support distro-specific policy changes if they were to ever show up. |
@Oats87 That is still preserved. The build is still run through EL7 and EL8, just the inputs are unified. It produces EL7 and EL8 RPMs. Distro-specific policy settings are already possible even with unified policy files and spec file. |
I think what @Oats87 was speaking to here is a painful lesson that we learned with https://github.com/rancher/rke2-selinux (the interface and type-enforcement files differ between el7 and el8 for legitimate reasons out of our control) and to keep things tidy we did replicate that lesson here with a split of policy-and-spec-per-distro. We considered branch-per-distro instead of repo-subpath-per-distro but determined that would be more confusing given our existing processes and therefore less easy to maintain correctly. @Conan-Kudo we do appreciate the effort put in to rectify a perceived problem and we should use this opportunity to review whether the split, such as it is, still makes sense. |
I do not understand why mild pushback on a PR for unifying policy and RPM spec specific to el7/el8 is preventing solutions for #8 and #19. Part of the reason for this split right now, admittedly more relevant for rke2 at this time, is because our initial support requirement was for both el7 and el8 which have different, somewhat incompatible core selinux policy and also differ significantly in the version of the upstream (to this project) container-selinux policy that can be installed. One assumes that such a split will be necessary to maintain to support k3s on both sles15 and tumbleweed/microos but as mentioned here, #20 (comment), we should review that assumption. But, it looks as if my work to upstream containerd-specific policy into the container-selinux project (that was developed in this project to enable containerd for k3s on selinux systems), combined with containers/container-selinux#140, goes most of the way to resolving #8 and #19. If not, I am open to suggestions as to what is left to be done. That said, I do not believe that selinux policy specific to k3s belongs in the container-selinux project and I strongly object to containers/container-selinux#140 on the principle that it invites breakage of our existing installation process for k3s on our currently supported selinux-enabled operating systems (e.g., if we implement a type label that gets ported up to container-selinux then k3s-selinux will fail to install because k3s-selinux depends on container-selinux and a duplicated type definition will fail a policy installation). This ties our hands unnecessarily and I wish that you, @sysrich and @Conan-Kudo, had consulted with the k3s team before pushing such a PR for consideration and that @rhatdan had consulted with the k3s team before merging/tagging. |
I originally made this PR at the behest of @sysrich as I have experience in writing SELinux policy modules and packaging SELinux policies. I started with something that was, in my view (based on some light testing), relatively simple. Unifying the policy and packaging made the policy much clearer. If it had been merged, then I would have been able to trivially layer on openSUSE support, since the packaging guidelines from Fedora for SELinux policy modules are used for openSUSE too (modulo some minor build dependency quirks because of OBS-isms). A simple build script on top for dapper targeting openSUSE would have been easy with no further changes to the spec required, and only a couple of minor adjustments for OBS would have been needed to ship in openSUSE for MicroOS and Kubic. When it became clear that this was going to drag on, @sysrich started asking me about forking the module so that we could ship it in openSUSE. I didn't want to do that, and the more I looked into this module, the more I thought this module should not exist separately, since it defines no special types and does very little on top of container-selinux. Additionally, I started second-guessing what this module actually did, and I wanted a second pair of eyes on it. Submitting it upstream to I was honestly surprised that k3s needed a separate SELinux module in the first place when it was explained to me that k3s is supposedly "just stripped down kubernetes". In Fedora, we're able to run kubernetes with just |
At a bare minimum, this project exists because the very first platform we committed to supporting was el7. And as we are still committed to supporting el7, this project is necessary (because container-selinux hasn't been updated there in quite some time and doesn't seem likely to ever be). |
This is perfectly reasonable, and as I said, a welcome opportunity to review whether or not to continue to maintain the current duplicative split, but ...
A fork of this project would have been preferable and is the expected, if sometimes temporary, solution to the problem of coercing a software project to support a new target runtime. Instead the choice was made to go completely around the maintainers of this project, who maintain it for legitimate reasons, and potentially add to their maintenance burden. But, because @rhatdan pared it back to only the file-contexts, we seem to have gotten lucky. |
Now, as for containers/container-selinux#140 (comment):
This reads very much like a pretext (albeit, flimsy) for doing what you wanted to do, regardless of the opinions of the k3s-selinux maintainers (which, seemingly, were not solicited). The "playing games" comment is unnecessarily disrespectful. I can understand frustration with a slow moving process. Please consider that we have priorities and responsibilities that are not limited to only this small, k3s-specific policy extension to container-selinux, nor even to k3s itself.
@rhatdan's comments on your PR were pitch perfect for the context they were delivered in: enhancing the selinux policy maintained at the (upstream to this project) container-selinux project. Have either of you, @Conan-Kudo or @sysrich, ported his concerns to this project via issue or pull-request? I seem to have missed such. Additionally, have you considered that the transitions in question are specific the how k3s is implemented and therefore not appropriate for inclusion in a general-purpose, container-specific policy library? I understand @rhatdan's comments perfectly well in the context of container-selinux (and as such, they are correct) but the transitions that he pushed back on were necessary to get k3s working correctly in all modes of operation that we support. This is why we maintain this small policy extension project. If you have suggestions on how we can do better at this, issues and pull-request are always welcome.
Cool, that is good news, truly. Please refer to the transitions as written in our TE files when your users report some things not working quite as expected on your selinux-enabled platform. And if you end up having some fixes to suggest, pull requests are always welcome. |
Here's the thing: virtually all the work I'm doing in nearly every project I work on is on my own increasingly limited free time. It's very rare that someone is paying me to do anything on this, and certainly for k3s, nobody is paying me at all. I'm volunteering my time to try to get k3s working in more places. I realize that this is quite rare in the Kubernetes community, so this is probably a mismatch of understanding how community project management works. This is particularly why I don't do much in the Kubernetes space, because it's fairly unpleasant for volunteers to work in these projects. I was content to wait until it started turning into a problem for openSUSE. And after talking to the openSUSE MicroOS folks, I increasingly got the picture that I was going to be forced to either fork the module or find an alternate solution to fix this. I made this pull request in March, made a small comment in April that led to a complaint that I unified it, at which point I responded that the build is still fine for EL7 and EL8. Nobody responded since. Then @sysrich comments in July, and he starts talking to me about forking it. At which point I start looking at the module more closely to understand what it does to understand why it exists. Then I decided to make the PR to
I would literally not do that unless it was absolutely the last resort. I do not have the bandwidth to maintain a fork of anything. As I said earlier, nobody pays me to work on k3s, and I know from past experience that a fork of that nature often becomes irreconcilable. Maintaining SELinux policy modules is a lot of work, and I would have to constantly merge the two policies back into one myself, and that was not sustainable. As I said earlier, I'm not paid to work on this, so if I can't get it fixed somehow in an upstream project, then it doesn't get fixed. In my world, there is no time for temporary mid-term forks. If you think that having a fork is a good solution to force fixing things, then to me, that says you don't actually care about a community beyond your specific corporate goals. And that's fine, but I don't align with that, so I'll come up with a solution that works for me.
I did actually consider that. I also considered that it's not actually because of how k3s is implemented, but how you deliver k3s with your And I think you're missing the point here about |
I can imagine. I did not work in k8s (nor selinux) at all until I started at Rancher Labs before it was acquired by SUSE. I did, however, contribute to projects such as LinuxKit and other Docker-adjacent projects as a member of the community. The contextual burden placed on contributors to those projects was daunting, k8s even more so. Then there are the concerns of the companies involved to navigate ... it is a lot and I wish it wasn't like that.
[emphasis added by me] And so I now better understand your path to solving this problem, for your particular constraints, and your frustration with the whole ordeal. You suggested at #20 (comment) that this PR could help resolve #8 and #19 but this didn't follow for @Oats87 and nor does it for me because as k3s maintainers we have a different set of constraints than you do as a contributor from the community. This is to be expected! Maybe you felt disrespected by the response(s) that you got concerning your proposed change (thank you for working on it!) but in review of the comments here I do not see any hostility nor disrespect, overt or otherwise directed at you, this PR, or @sysrich (whom you linked to and also did threaten a fork). However, I do view the action described in the above emphasized sentence as disrespectful because I see no follow-up effort with this PR to make the case for your change and/or why solving it the way you did is important to your use case. It is perhaps unfair to put such a burden on members of the community when they submit pull requests but we have to consider all incoming changes in the context of how they will effect our existing support matrix (which is true regardless of the project, "corporate" or not).
Indeed, something not to be undertaken lightly.
Which comes first, the RPM policy installation or k3s executing and unpacking it's "data" directory? The transitions as developed for k3s exist for a reason (to enable k3s to Just Work ™️ with selinux enabled under a fairly broad variety of usage patterns). I have to assume from this comment that you are proposing packaging k3s in what is essentially an unsupported manner. With selinux in the offing I would advise against that (easier to get away with otherwise, see the alpine packaging). You want k3s to work on openSUSE/MicroOS with no fuss, I want that too. I shouldn't need to make you aware that if you want to enable a software package/solution on your platform then you most likely want to also leverage the existing community for such and that an alternate, non-standard method of delivery is a great way to make that exceedingly fraught for everyone involved.
I think you are missing the point that we have to maintain selinux policy for k3s and we do not feel that it is appropriate to include k3s-specific policy in container-selinux and it would seem as if the maintainer of container-selinux, @rhatdan, agrees (if indirectly). I want k3s to run everywhere as securely as it can be made to run and so I ask, as a maintainer of this project, for you (community, corporate, come one come all) to work with us to make that happen for the platforms that you care about. I promise that we will do our best to work with you and incorporate your hypothetical change (but possibly some other hypothetical change) that enables your platform so long as it does not compromise our existing support matrix. We, of course, also welcome fixes that purport to help with the maintenance burden for this project but the trick there will be, as it is with any software project, to convince the maintainers that such fixes are in their best interest. |
The RPM policy installation would be first. But with the RPM-based install of k3s, it would pre-create those directories, which would wind up having the policies applied up front before k3s runs. There's basically no reason that we couldn't have an upstream package-based installation of k3s that works for RH/Fedora, SUSE, and Ubuntu distributions. I'd be happy to help with that if it's wanted. But my understanding is that Rancher doesn't particularly like packaged software, so I've never brought it up before when I started looking at Kubic stuff. |
Alright. I wish you the best of luck with this effort, truly. I have to reiterate (one last time) that I do worry about the follow-on maintenance burden for the k3s team. Also, I mean it when I say that any policy contributions that you may find necessary to enable you with this are welcome (in this repo, please) and we will be reviewing whether or not to maintain the distro split that this PR attempts to resolve. |
For what it's worth, it does look like the split makes some sense still in RKE2 (though introducing conditionals in the TE files that could be passed in at policy module build time would eliminate the need for it there too). I just don't think it makes any sense for k3s, hence the PR to this project. |
@Conan-Kudo once again, thanks for this PR. You clearly put a lot of effort into understanding what we tried to do here and improve upon it. I know that while the exchange got slightly heated, @dweomer, @Oats87, and I are appreciate of your effort and regret that we let the PR sit as long as we did. Lesson learned for us is to bring prs to more concrete conclusions, not to leave them hanging. We've been discussing this a good deal and here's how we'd like to proceed. First (and I realize this is painful) - we'd like to see the PR to container-selinux reverted. We want to buy ourselves sometime to reassess our approach holistically. Our big concern is that once new versions of container-selinux start shipping for the various OSes, we'll be behind the 8 ball. While that PR does not immediately break us, it does cause us several problems from a support and maintenance burden:
@dweomer will likely put in the PR to make the revert later today or tomorrow. We'll tag you in it. Next, as @dweomer mentioned, we'll review if maintaining the split makes sense (our experience with a need for a split with rke2 notwithstanding). This could mean we adopt your PR outright, choose to keep our current course, or do something completely different. The goal, in the end, will be to deliver policies that work for the SUSE family of OSes. |
You're too late to this. It's already shipping in Fedora 34, openSUSE Tumbleweed/MicroOS, and CentOS Stream (both 8 and 9). It will be part of RHEL 8.5 releasing shortly. |
I'm not convinced you need to do that. I'm also concerned that you consider yourselves at the "mercy" of their release cycle rather than cooperating and working with them to ship things as needed.
The other problem with your SELinux module is that it was not packageable in openSUSE or Fedora without this PR. Adding the scaffolding to build it properly was another aspect of this that I did. The underlying policy works on openSUSE Tumbleweed and MicroOS with this PR, since @sysrich and I are tracking fedora-selinux and container-selinux quite closely for openSUSE. This PR was a pre-requisite for being able to ship it as a package in Fedora and openSUSE, though with the container-selinux change, I don't really need it. If you succeed in getting it reverted, then I would need this PR merged. |
@Conan-Kudo is this part of an effort to ship k3s as an RPM for openSUSE? Can you point me at anything around that? |
Yes. We have it in openSUSE Factory now: https://build.opensuse.org/package/show/openSUSE:Factory/k3s Since the |
Got it. Thanks @Conan-Kudo. To your point, yeah its already released upstream. Ship has sailed. I'll close this PR then, since you got what you needed via container-selinux. We'll still take the time to re-evaluate the split and whatnot, but I don't want to continue to leave you hanging on it. |
The actual SELinux policy content was identical between EL7 and EL8,
with minor differences in the RPM spec file. Furthermore, the scripts
for actually doing these builds were not significantly different.
This unifies them so that we don't suffer maintaining two versions
for no reason.
Additionally, rewrite the SELinux policy package RPM spec file to align
with SELinux module packaging to make it easier to ship in distributions.