Add the option to enable default Pod Security Configuration #9017
Conversation
k8s-ci-robot
added
the
kind/feature
k8s-ci-robot
added
the
cncf-cla: no
|
Welcome @Foxlik! |
k8s-ci-robot
added
the
needs-ok-to-test
|
Hi @Foxlik. Thanks for your PR. I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. 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. |
k8s-ci-robot
added
the
size/M
|
(Trying to figure out who can sign CLA at my company since this was done for one of our clusters.) |
| @@ -1,3 +1,3 @@ | |||
| --- | |||
| # list of admission plugins that needs to be configured | |||
| kube_apiserver_admission_plugins_needs_configuration: [EventRateLimit] | |||
| kube_apiserver_admission_plugins_needs_configuration: [EventRateLimit, PodSecurity] | |||
There was a problem hiding this comment.
Shouldn't this addition be conditioned by kube_pod_security_use_default=true ?
There was a problem hiding this comment.
This variable has been created to ensure that admission plugins that need to be configured, could be provided with this.
In this case, I think it is not necessary to add PodSecurity here because the plugin doesn't need configuration to work.
Here are some examples on how to use it:
https://github.com/kubernetes-sigs/kubespray/search?q=kube_apiserver_admission_plugins_needs_configuration
There was a problem hiding this comment.
@alegrey91 Getting this PodSecurityConfiguration file created on the node is the sole reason I got into creating this PR. It's an admission plugin so I thought this was a good place to get it done. If there is a better place or if you think it's more clear to do it in other way, I'm ready to fix that. But the plugin needs the config to work.
I think this also answers your other comment about the PodSecurityConfiguration not being applied to the cluster.
The patch definitely does what's advertised - enable default PodSecurity for whole cluster and when configured, prevent insecure Pods from being spawned. Trust me, it forced me to fix quite a few apps ;).
There was a problem hiding this comment.
@cristicalin i've added a condition around the PodSecurity in here. I did it the most naive way. Not sure if it's up to standards. Please check.
The original idea was to get the file created always, just empty when kube_pod_security_use_default is false.
There was a problem hiding this comment.
@Foxlik I got your point.
So, you can leave the code in this way:
kube_apiserver_admission_plugins_needs_configuration: [EventRateLimit,PodSecurity]
You should probably rename the file roles/kubernetes/control-plane/templates/podsecurity.v1beta2.yaml.j2 to roles/kubernetes/control-plane/templates/podsecurity.yaml.j2.
This because the admission-controls.yaml.j2 will grab the name from the variable kube_apiserver_admission_plugins_needs_configuration and use it as name of the AdmissionConfiguration plugin list.
E.g.
apiVersion: apiserver.config.k8s.io/v1
kind: AdmissionConfiguration
plugins:
- name: EventRateLimit
path: /path/to/eventratelimit.yaml
- name: PodSecurity # extracted from plugin variable
path: /path/to/podsecurity.yaml # plugin | lower
Ping me if you have some doubt about the explanation.
Anyway, you did a great work!
|
Hi @Foxlik , thanks for your contribution, please ensure to sign the CLA so it can be accepted. |
|
@alegrey91 Could you take a look at this pull request? I guess you might be interested. |
k8s-ci-robot
added
cncf-cla: yes
|
I finally got the CLA sorted out. Reverted the latest changes, but added an explanation for the empty file. If this looks good to you, please set it to squash before merge. |
Thank you, could you please rebase master though ? Otherwise CI won't be happy :) |
Enable Pod Security in all namespaces by default with the option to exempt some namespaces. Without the change only namespaces explicitly configured will receive the admission plugin treatment.
- leave the empty file when kube_pod_security_use_default, but add comment explaining the empty file - don't attempt magic at conditionally adding PodSecurity to kube_apiserver_admission_plugins_needs_configuration
guess I should've said I've done so. anyway, rebased again. hoping to make CI smile again. |
|
Thanks for this work @Foxlik ! /approve |
k8s-ci-robot
added
ok-to-test
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: cristicalin, floryut, Foxlik 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 |
* upstream/master: (832 commits) add pre-commit hook to facilitate local testing (kubernetes-sigs#9158) cri-dockerd: add restart of docker.service (kubernetes-sigs#9205) do not run etcd role in scale.yml playbook when etcd installed by kubeadm (kubernetes-sigs#9210) optimize the format of evictionHard in kubelet-config.yaml template (kubernetes-sigs#9204) Update vars.md (kubernetes-sigs#9172) fix one bug in docs/nodes (kubernetes-sigs#9203) Fix containerd (<1.7) configuration for insecure registries (kubernetes-sigs#9207) 🌱 Enable cri-dockerd service (kubernetes-sigs#9201) Update vsphere-csi.md (kubernetes-sigs#9170) 9035: Make Cilium rolling-restart delay/timeout configurable (kubernetes-sigs#9176) [kubernetes] Add hashes for 1.24.4, 1.22.13, 1.23.10 and make v1.24.4 default (kubernetes-sigs#9191) Add 'avoid-buggy-ips' support of MetalLB (kubernetes-sigs#9166) Add the option to enable default Pod Security Configuration (kubernetes-sigs#9017) Add 'flush ip6tables' task in reset role (kubernetes-sigs#9168) add list nodes rules to cilium-operator clusterrole (kubernetes-sigs#9178) docs(kube-vip): fix broken links (kubernetes-sigs#9165) Disable DNSStubListener for Flatcar Linux (kubernetes-sigs#9160) Subnet setup order fix & Number of master nodes syntax fix (kubernetes-sigs#9159) Fix CSI drivers issues on Azure (kubernetes-sigs#9153) [calico] calico rr supports multiple groups (kubernetes-sigs#9134) ...
…es-sigs#9017) * Add the option to enable default Pod Security Configuration Enable Pod Security in all namespaces by default with the option to exempt some namespaces. Without the change only namespaces explicitly configured will receive the admission plugin treatment. * Fix the PR according to code review comments * Revert the latest changes - leave the empty file when kube_pod_security_use_default, but add comment explaining the empty file - don't attempt magic at conditionally adding PodSecurity to kube_apiserver_admission_plugins_needs_configuration
…es-sigs#9017) * Add the option to enable default Pod Security Configuration Enable Pod Security in all namespaces by default with the option to exempt some namespaces. Without the change only namespaces explicitly configured will receive the admission plugin treatment. * Fix the PR according to code review comments * Revert the latest changes - leave the empty file when kube_pod_security_use_default, but add comment explaining the empty file - don't attempt magic at conditionally adding PodSecurity to kube_apiserver_admission_plugins_needs_configuration
…es-sigs#9017) * Add the option to enable default Pod Security Configuration Enable Pod Security in all namespaces by default with the option to exempt some namespaces. Without the change only namespaces explicitly configured will receive the admission plugin treatment. * Fix the PR according to code review comments * Revert the latest changes - leave the empty file when kube_pod_security_use_default, but add comment explaining the empty file - don't attempt magic at conditionally adding PodSecurity to kube_apiserver_admission_plugins_needs_configuration
What type of PR is this?
/kind feature
What this PR does / why we need it:
By default only Pods in annotated namespaces are checked for following Pod Security Standards. This change adds the option to reverse the behavior, where Pods in all namespaces not exmpted are checked.
Special notes for your reviewer:
This is my first PR to kubespray and this looks like a quick and easy win to me, but I might be mistaken. Thanks for any feedback.
I'm not sure if any other namespaces should be on the default exempt list.
Does this PR introduce a user-facing change?: