-
Notifications
You must be signed in to change notification settings - Fork 40k
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
SELinux Overhaul #33663
SELinux Overhaul #33663
Conversation
a3f3bf6
to
18dfe60
Compare
cc @kubernetes/sig-node @kubernetes/sig-rktnetes |
591246b
to
698fca6
Compare
@yifan-gu ready for review, PTAL. |
@@ -510,7 +498,7 @@ function start_kubelet { | |||
--volume=/var/run:/var/run:rw \ | |||
--volume=/sys:/sys:ro \ | |||
--volume=/var/lib/docker/:/var/lib/docker:ro \ | |||
--volume=/var/lib/kubelet/:/var/lib/kubelet:rw,z \ | |||
--volume=/var/lib/kubelet/:/var/lib/kubelet:rw \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note, the relabel to a shared SELinux context should no longer be necessary, because the Kubelet will be running with spc_t
due to running in a privileged container, which is unconfined.
Jenkins GCE etcd3 e2e failed for commit aa855b9. Full PR test history. The magic incantation to run this job again is |
Jenkins GCE e2e failed for commit aa855b9. Full PR test history. The magic incantation to run this job again is |
Jenkins GCI GCE e2e failed for commit aa855b9. Full PR test history. The magic incantation to run this job again is |
@k8s-bot test this please, issue #IGNORE. |
lgtm |
|
||
// Have docker relabel the termination log path if SELinux is | ||
// enabled. | ||
if selinux.SELinuxEnabled() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any reason why this was not here before?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wasn't as smart before :) It was a bug.
ContainerPath: etcHostsPath, | ||
HostPath: hostsFilePath, | ||
ReadOnly: false, | ||
SELinuxRelabel: true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe comment it with Isolate pods on each host from one another by default
? Thought it is already captured in the docs (I liked reading it), the comment here could help to make the correct associations sooner. Just nit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ingvagabund I'm not sure I understand what is being asked for here. Elaborate?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
to point out the SELinuxRelabel
is set to true to isolate the pods from each other by default.
LGTM |
@k8s-bot test this [submit-queue is verifying that this PR is safe to merge] |
Automatic merge from submit-queue |
Automatic merge from submit-queue Remove GetRootContext method from VolumeHost interface Remove the `GetRootContext` call from the `VolumeHost` interface, since Kubernetes no longer needs to know the SELinux context of the Kubelet directory. Per #33951 and #35127. Depends on #33663; only the last commit is relevant to this PR.
The original change that added the unconfined label included a comment indicating it won't be needed in the future. See: kubernetes#33555 (comment) That time is now. kubernetes#33663 has landed and means we no longer have to go out of our way to make that work. Removing the label also increases security since there wasn't really a good reason for etcd to be run with such broad selinux privileges. This also will allow kubeadm to avoid errors on distros without an spc_t type, such as Gentoo and Container Linux (at the time of writing at least). Fixes kubernetes/kubeadm#269
Automatic merge from submit-queue (batch tested with PRs 49328, 49285, 49307, 49127, 49163) kubeadm: don't customize etcd selinux label The original change that added the unconfined label included a comment indicating it won't be needed in the future. See: #33555 (comment) That time is now. #33663 has landed and means we no longer have to go out of our way to make that work. Removing the label also increases security since there wasn't really a good reason for etcd to be run with such broad selinux privileges. This also will allow kubeadm to avoid errors on distros without an spc_t type, such as Gentoo and Container Linux (at the time of writing at least). Fixes kubernetes/kubeadm#269 **Release note**: ```release-note NONE ```
The original change that added the unconfined label included a comment indicating it won't be needed in the future. See: kubernetes#33555 (comment) That time is now. kubernetes#33663 has landed and means we no longer have to go out of our way to make that work. Removing the label also increases security since there wasn't really a good reason for etcd to be run with such broad selinux privileges. This also will allow kubeadm to avoid errors on distros without an spc_t type, such as Gentoo and Container Linux (at the time of writing at least). Fixes kubernetes/kubeadm#269
Automatic merge from submit-queue Remove GetRootContext method from VolumeHost interface Remove the `GetRootContext` call from the `VolumeHost` interface, since Kubernetes no longer needs to know the SELinux context of the Kubelet directory. Per kubernetes#33951 and kubernetes#35127. Depends on kubernetes#33663; only the last commit is relevant to this PR.
Overhauls handling of SELinux in Kubernetes. TLDR: Kubelet dir no longer has to be labeled
svirt_sandbox_file_t
.Fixes #33351 and #33510. Implements #33951.
This change is