helm-charts
helm-charts copied to clipboard
fluent
Change Openshift scc seLinuxContext to RunAsAny
When creating a securityContextContraint for Openshift, the helm chart currently set the selinux strategy to MustRunAs, but this might lead to permission errors if you are using selinux in the node and you want to mount an host folder with an HostPath.
I think there is no reason for disallowing selinux context to be changed for the fluentbit deployment. Also, this is the same behavior used by red hat for their cluster logging operator that deploys fluentd. https://github.com/openshift/cluster-logging-operator/blob/master/internal/auth/securitycontextconstraint.go#L46
Sorry for all the pushes but I had to fix the verification of the commit :sweat_smile:
@AlbertoPimpo I'm not familiar enough with OpenShift to make a call on this change.
FYI you need to rebase and bump your chart version.
Maybe we need to ask to take a look this change @patrick-stephens ? He sometimes do some stuffs of his technology interests of Openshift.
FYI you need to rebase and bump your chart version
@stevehipwell done :+1:
If it can help, in the PodSecurityPolicy you indeed allow to change the selinux context https://github.com/fluent/helm-charts/blob/main/charts/fluent-bit/templates/psp.yaml#L28
I would not be a fan of changing it from one hardcoded value to another, let's make it configurable with a default.
I would not be a fan of changing it from one hardcoded value to another, let's make it configurable with a default.
@patrick-stephens I like the idea, maybe we should do the same for psp? I can take care of that.
@patrick-stephens sorry for taking so long, it's done now. Feel free to merge it.
@AlbertoPimpo PSPs have not been supported in Kubernetes since
1.26so I don't see any value in making changes there?
@stevehipwell It's a non-breaking change and a nice to have for the few users who are still stuck with old versions of kubernetes. Anyway, if this has to became a blocker for the PR, then I'll remove this part.
@AlbertoPimpo could you please rebase, look at the comments I just made and also update the chart changelog annotations? As the PSP resource is still in the chart I can't see an issue with making this change.
@AlbertoPimpo could you please rebase, look at the comments I just made and also update the chart changelog annotations? As the PSP resource is still in the chart I can't see an issue with making this change.
Thanks for the review @stevehipwell . I accepted your suggested changes, rebased and squashed everything in a single commit. We should be ready to merge.