✨ Mise en place des PodSecurityAdmission sur les namespaces clients #34
Conversation
There was a problem hiding this comment.
In the commit message, the following should not appear : "URL://pages/viewpage.action?pageId=228069521" -> This is an internal link and has no interest for the external reader. Instead you should mention the relevant content of the page on the commit message.
Your commit message also doesn't mention why you implemented the feature under a config map (and its addition of privileged_namespaces).
You also don't mention why you are not using a secure by default approach here.
Finally, you don't mention why you intend to warn in logs for privileged namespace, while it's been technically validated by the admin (by defining the config map) -- is that because it might be seen elsewhere/by different ppl?
Last but not least: Keep in mind I haven't tested it. Someone else must approve, as I don't know how this is intertwined with existing systems.
At first sight, it looks okay, and the update could work out of the box.
|
Tests :
Le label restricted est appliqué par défaut en raison de l'erreur de configuration du paramètre.
Le fonctionnement est donc validé sur cette partie d'application de labels. |
| "environment": project.Spec.Environment, | ||
| "pod-security.kubernetes.io/enforce": GetPodSecurityStandardName(project.Name), | ||
| "pod-security.kubernetes.io/warn": string(utils.Config.PodSecurityAdmissionWarning), | ||
| "pod-security.kubernetes.io/audit": string(utils.Config.PodSecurityAdmissionAudit), | ||
| } | ||
|
|
||
| return utils.Union(defaultLabels, utils.Config.CustomLabels) |
There was a problem hiding this comment.
I am not very familiar with the code base, so I might need to check what this does. That's why I said someone else should also review. AT FIRST SIGHT, it looks lik ethis would work.
| @@ -99,6 +100,24 @@ func MakeConfig() (*types.Config, error) { | |||
| ldapUserFilter := getEnv("LDAP_USERFILTER", "(cn=%s)") | |||
| tenant := strings.ToLower(getEnv("TENANT", KubiTenantUndeterminable)) | |||
|
|
|||
| podSecurityAdmissionEnforcement, errPodSecurityAdmissionEnforcement := podSecurity.ParseLevel(strings.ToLower(getEnv("PODSECURITYADMISSION_ENFORCEMENT", string(podSecurity.LevelRestricted)))) | |||
|
|
|||
There was a problem hiding this comment.
I would add a comment, for the lazy ppl like me, to say:
# No need to state a default or crash, because kubernetes defaults to restricted.
internal/services/provisionner.go
Outdated
| func GetPodSecurityStandardName(namespace string) string { | ||
| if utils.IsInPrivilegedNsList(namespace) { | ||
| utils.Log.Warn().Msgf("Namespace %v is labeled as privileged", namespace) | ||
| return utils.PodSecurityPrivileged |
There was a problem hiding this comment.
Maybe we can use that constant instead, and remove the string utils.PodSecurityPrivileged from defaults.go (as this is a constant, not a default!) .
Yet, I don't think it's a deal breaker, so let's move on.
| "creator": "kubi", | ||
| "environment": project.Spec.Environment, | ||
| "pod-security.kubernetes.io/enforce": GetPodSecurityStandardName(project.Name), | ||
| "pod-security.kubernetes.io/warn": string(utils.Config.PodSecurityAdmissionWarning), |
There was a problem hiding this comment.
nit: I will propose a rename in a followup commit, for clarity.
| utils.Log.Warn().Msgf("Namespace %v is labeled as privileged", namespace) | ||
| return utils.PodSecurityPrivileged | ||
| } | ||
| return string(utils.Config.PodSecurityAdmissionEnforcement) |
There was a problem hiding this comment.
nit: same for the returned constant here.
internal/utils/defaults.go
Outdated
| PodSecurityAdmissionWarning = "restricted" | ||
| PodSecurityAdmissionAudit = "restricted" | ||
|
|
||
| PodSecurityPrivileged = "privileged" |
There was a problem hiding this comment.
nit: this is a constant, not a default, unlike the first 3.
(The first 3 define what we want as default, picked for the ns)
Dans l'objectif de remplacer les podSecurityPolicy en prévision du passage en Kubernetes 1.26, il est nécessaire de mettre en place les nouvelles règles de sécurité. Nous mettons en place par défaut un enforce sur la policy "restricted", avec un warning emis dès lors que l'applicatif ne respecte pas la policy "restricted" et réalisons par défaut l'émission d'un event d'audit lorsque la policy ne respecte pas le niveau "restricted". Le choix restricted a été fait dans l'objectif d'avoir une politique sécurisée par défaut. En raison de projets clients devant être en privileged, nous rendons possible l'ajout d'une liste de namespaces devant être valorisés à privileged. Nous émettons un log de type Warn pour souligner à un administrateur la particularité de configuration d'un namespace en particulier dans les logs de l'opérateur. Sans cette modification tous les namespaces clients sont considérés comme héritant de la politique par défaut. La configuration est appliquée à travers le configmap de Kubi car c'est là qu'est toute la configuration du composant. Par conséquent, une modification de la configuration nécessite forcément un redémarrage de l'opérateur. (Peut être réalisé via Stakater).
je valide le fonctionnement. |
Dans l'objectif de remplacer les podSecurityPolicy en prévision du passage en Kubernetes 1.26, il est nécessaire de mettre en place les nouvelles règles de sécurité.
Nous mettons en place par défaut un enforce sur la policy "restricted", avec un warning emis dès lors que l'applicatif ne respecte pas la policy "restricted" et réalisons par défaut l'émission d'un event d'audit lorsque la policy ne respecte pas le niveau "restricted".
Le choix restricted a été fait dans l'objectif d'avoir une politique sécurisée par défaut.
En raison de projets clients devant être en privileged, nous rendons possible l'ajout d'une liste de namespaces devant être valorisés à privileged. Nous émettons un log de type Warn pour souligner à un administrateur la particularité de configuration d'un namespace en particulier dans les logs de l'opérateur.
Sans cette modification tous les namespaces clients sont considérés comme héritant de la politique par défaut.