Add API for SELinuxRelabelPolicy #91838
Conversation
k8s-ci-robot
added
release-note
|
Preview of implementation of this feature is available in #91839 (All kubelet changes are there + AWS EBS and CSI volume plugin updates). |
|
This PR may require API review. If so, when the changes are ready, complete the pre-review checklist and request an API review. Status of requested reviews is tracked in the API Review project. |
|
cc @msau42 @tallclair @gnufied |
k8s-ci-robot
added
the
api-review
|
/assign |
|
/assign @smarterclayton |
k8s-ci-robot
added
the
needs-rebase
| // "OnVolumeMount" tries to mount volumes used by the Pod with the right context and skip recursive ownership | ||
| // change. Kubernetes may fall back to policy "Always" if a storage backed does not support this policy. | ||
| // + optional | ||
| SELinuxRelabelPolicy *PodSELinuxRelabelPolicy |
There was a problem hiding this comment.
I can't remember if this came up on the KEP, but should this be per-volume rather than per-pod?
There was a problem hiding this comment.
See below, I'm following FSGroupChangePolicy.
| // "OnVolumeMount" tries to mount volumes used by the Pod with the right context and skip recursive ownership | ||
| // change. Kubernetes may fall back to policy "Always" if a storage backed does not support this policy. | ||
| // + optional | ||
| SELinuxRelabelPolicy *PodSELinuxRelabelPolicy |
There was a problem hiding this comment.
Can we group this & FSGroupChangePolicy together in a way that makes it clear both pertain to volumes? I'm thinking a sub-struct along the lines of VolumeSecurityContext? See above comment, it may make sense to move this to the volume struct.
There was a problem hiding this comment.
I just followed fsGroupChangePolicy, which, after the same discussion, ended up in SecurityContext. @gnufied, do you remember why we kept it there?
There was a problem hiding this comment.
I would like to know why this wouldn't be in the volume struct too.
There was a problem hiding this comment.
The discussion from fsGroupChangePolicy - kubernetes/enhancements#1478 (comment) . tldr; of that discussion is -
- Having this setting on per-volume basis is more complicated for the end user and its puts this burden of deciding which policy to choose for which volume type.
- Also, if there were an automated tool which injected such policies in pods, the tool also has to be aware of volumes it should target.
- On the other hand - having this setting on per-volume basis does not really buy much at the cost of added complexity.
| // SELinuxRelabelOnVolumeMount indicates that, if supported by the storage backend, SELinux | ||
| // label will be assigned during mount of pod's volumes as a mount option. This can skip | ||
| // potentially long recursive SELinux label change performed by container runtime when starting | ||
| // a pod. |
There was a problem hiding this comment.
Why would you not always set this? Add the drawbacks to the doc comment.
There was a problem hiding this comment.
It's... complicated. I added more description there, however, I was not able to avoid mentioning Linux kernel.
There was a problem hiding this comment.
What is the tradeoff when you skip this? I.e. you talk about a benefit (performance) but you don't present what you lose (tim's question). I would guess that you are not secure. If you're not secure, why. Is there a link somewhere to the presentation of why relabel is not desirable that we can copy?
There was a problem hiding this comment.
Sorry, I updated only storage/v1, forgetting core/v1.
Added "As tradeoff, all pods that simultaneously access the volumes must run with the same SELinux context to be able to access data on them, even if they use just a subPath of them."
The goal is to say it's safe, unless you use a single volume with two pods with different SELinux contexts, each using a different subpath of the volume. This was working before and it is working even now with policy "Always".
k8s-ci-robot
added
needs-rebase
Swap Sysctls and FSGroupChangePolicy in the internal API to have the same order as in v1 API.
k8s-ci-robot
removed
the
needs-rebase
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: jsafrane The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
@jsafrane: The following tests failed, say
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. 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. I understand the commands that are listed here. |
|
/close
|
|
@jsafrane: Closed this PR. In response to this:
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. |
SELinuxRelabelPolicy allows users to optionally skip recursive relabeling of all files on pod on volumes on systems with SELinux support.
KEP: https://github.com/kubernetes/enhancements/tree/master/keps/sig-storage/1710-selinux-relabeling
When the feature is enabled + supported by volume plugin / CSI driver + enabled in
Pod.SecurityContext+ kubelet knows SELinux context of pod volumes, kubelet mounts pod's volumes with-o context=<xyz>. No relabeling is then necessary.What type of PR is this?
/kind api-change
Changes from KEP:
type SELinuxRelabelPolicy->PodSELinuxRelabelPolicyEnum values
OnVolumeMount,AlwaysRelabel->SELinuxRelabel*Does this PR introduce a user-facing change?:
Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.: