WIP: 1710: Add SELinuxChangePolicy to PodSpec #4843
Conversation
k8s-ci-robot
added
do-not-merge/work-in-progress
k8s-ci-robot
added
the
kind/kep
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jsafrane The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
approved
| | AWS EBS CSI | true | `-o context=XYZ` | | 2) | | ||
| | AWS EBS CSI | unset or false | | `:Z` | 3) | | ||
| | NFS1 CSI | true | `-o context=XYZ,noshareacache` | | 4) | | ||
| | NFS2 CSI | unset or false | | | 5) | |
There was a problem hiding this comment.
IMO, this table can be removed and merged with "Volume Mounting" section.
There was a problem hiding this comment.
I;m not sure, "Volume Mounting" describes general mount capabilities. This example shows how we're going to use these capabilities in kubelet.
| // MountReadWriteOncePod mounts PersistentVolumes with access mode ReadWriteOncePod with `-o context` mount option. | ||
| // All other volumes are relabeled recursively by the container runtime. | ||
| // Mounting with `-o context` is instant and does not require the container runtime to recursively walk through the volume. | ||
| SELinuxChangePolicyMountReadWriteOncePod PodSELinuxChangePolicy = "MountReadWriteOncePod" |
There was a problem hiding this comment.
Also - for this option to work, Pod must have an explicit SELinux context specified, otherwise it will again default to recursive.
IMO - this default though makes sense, there isn't one single word that is going to capture all the necessary information.
I was thinking - how about we keep existing default and introduce a boolean field in securityContext. So something like:
securityContext.useSELInuxMountOption: true|false
So we keep existing behaviour for ReadWriteOncePod and other situations (if field is nil) but use this field if explicitly set to true. If this is explicitly set to false, then I guess we will have to use recursive chcon even for RWOP volumes. But that doesn't seem so bad.
There was a problem hiding this comment.
We should have all allowed options as explicit values in the API. nil should mean either true or false, not something in between.
I'll add the wording about SELinux label in Pod.
There was a problem hiding this comment.
Added a very short sentence about SELinuxLabel in PodSecurityContext / SecurityContext.
jsafrane
force-pushed
the
selinux-optional
branch
from
72b4bc0
to
520181b
Compare
k8s-ci-robot
added
size/XL
We found out that we *need* an explicit opt-in for mounting all volumes with `-o context` - all pods sharing the same volume must have the same label + must be all unprivileged (or all privileged). There are many changes in the KEP because of that. It feels like a new KEP.
jsafrane
force-pushed
the
selinux-optional
branch
from
4b6cb75
to
d995b96
Compare
|
I split it into some small commits, but there is still a huge one with the API change. And I tried to capture what was already done and how we want to move forward. |
| Files on the volume already have the right SELinux context, no recursive relabeling happens there. | ||
| * When a `PodSecurityContext` specifies incomplete SELinux label (i.e. omits `SELinuxOptions.User`, `.Role` or `.Type`), kubelet fills the blanks from the system defaults provided by [ContainerLabels() from go-selinux bindings](https://github.com/opencontainers/selinux/blob/621ca21a5218df44259bf5d7a6ee70d720a661d5/go-selinux/selinux_linux.go#L770). | ||
| Introduce a new PodSpec field `Spec.SecurityContext.SELinuxChangePolicy` with the following values: | ||
| * `UseMountOptionForReadWriteOncePod`: mount RWOP volumes with `-o context` (see other conditions below), all other volumes are relabeled recursively by the container runtime. |
There was a problem hiding this comment.
Another option I was thinking is - UseMountOptionsSafely, it is perhaps as much big as current choice, but again this name is going to be just picking "okay" option among worst. :(
jsafrane
force-pushed
the
selinux-optional
branch
from
ccb3c9a
to
754f206
Compare
jsafrane
force-pushed
the
selinux-optional
branch
from
754f206
to
7aff8dc
Compare
Make `MountOption` the default value. This way, we don't need `UseMountOptionForReadWriteOncePod` value.
We need SELinuxChangePolicy field to be available befor `SELinuxMount` feature gate is enabled.
jsafrane
force-pushed
the
selinux-optional
branch
from
f0c9575
to
50d4344
Compare
We found out that we need an explicit opt-out for mounting all volumes with
-o context- all pods sharing the same volume must have the same label + must be all unprivileged (or all privileged).There are many changes in the KEP because of that. It feels like a new KEP.
PodSecurityContext.SELinuxChangePolicywith valuesRecursiveandMountOption.MountOptionis the default one!SELinuxChangePolicygets enabled in Kubernetes 1.N.SELinuxChangePolicy.SELinuxMountfeature gate is still disabled.SELinuxMountgets enabled in the next Kubernetes release and fix them (either don't mix privileged and unprivileged pods or set theirSELinuxChangePolicy: Recursive).SELinuxMountenabled until the cluster admin fixes all problematic Pods.SELinuxMountgets enabled by default, but it does not break anything, because all problematic Pods were fixed before the upgrade.WIP: