[fluent-bit] Add shareProcessNamespace option

[drehelis]

@stevehipwell apologies for accidentally closing #298.

@drehelis before this PR moves any further please could you split it up so a PR is only for a single chart. Could you also explain why you need the change and what alternatives you've tried/considered?

I need to be able to add configuration to fluent-bit and signal the service to reload itself. As currently fluent-bit does not support dynamically re-reading the configuration.

As I'm required to carry the operation from inside the pod, using extra container is an ideal solution for me, therefore shareProcessNamespace becomes handy. I'm able to signal fluent-bit gracefully to shutdown and restart to pickup the new config.

I've tried the fluent-bit operator but that's just overkill in my case.
Executing rollout restart is not an option due to permissions.

[naseemkullah]

Hi @drehelis,

Worth noting that

helm-charts/charts/fluent-bit/templates/daemonset.yaml

Line 29 in 32f0fbf

checksum/config: {{ include (print $.Template.BasePath "/configmap.yaml") . | sha256sum }}

should cause the pods to restart upon changes to the configmap, as described in helm documentation.

Could you confirm if that occurs upon config changes?

[drehelis]

Hi @naseemkullah,

This only valid if Helm is being re-applied and picks up the new sha256 for the configmap.yaml file.

Proposed option is also used in other helm chart I'm using, such as Hashicorp's Vault.
https://github.com/hashicorp/vault-helm/blob/main/values.yaml#L451

Using a volumeMount both for fluent-bit and in extraContainers allows me to monitor config changes and simply send SIGHUP to $(pidof fluent-bit), resulting in a graceful shutdown and config re-read.

[naseemkullah]

lgtm

[stevehipwell]

@drehelis have you requested this functionality as a native Fluent Bit capability? Or requested adding the ability to restart Fluent Bit from an API endpoint (or trigger a config reload)?

[drehelis]

@stevehipwell There's an open issue from 2017 on this matter here: fluent/fluent-bit#365.
There are bunch of suggestions but no suitable solution for my goal. The most native approach is to use the fluent-operator but as mentioned above, it's a whole different approach for me.

[drehelis]

@stevehipwell Is there anything else holding this back from being merged?

[stevehipwell]

@drehelis I can't see how it is preferential to loosen the security of Fluent Bit which has host level filesystem access instead of either using the operator which is designed for this or a sidecar with permissions to rollout the DaemonSet when your configuration changes?

[drehelis]

@stevehipwell I believe that by the end of the day, security aspect here is end user decision, after all we're shipping this default to false.
See securityContext for example, which is currently exposed by this chart and can easily be set to CAP_SYS_ADMIN to gain a full control.

Using the operator is totally different approach to this issue and not flexible enough and it doesn't suit everyone need.

Many popular charts are adding this setting, just few examples -

• Support shareProcessNamespace in pod spec pingidentity/helm-charts#229
• Add support for shareProcessNamespace to the helm chart kubernetes/ingress-nginx#8279
• Add shareprocessns parameter DataDog/helm-charts#558

Please re-consider

[stevehipwell]

@drehelis I don't think the pattern you're proposing is something which makes sense to "officially" support given the impact of weakened security, extra logic on all nodes & non-standard audit trail. There are a number of existing solutions to your problem that don't require extra chart complexity and the potential for someone to hold this wrong.

• Manage the config via the Helm chart which restarts Fluent Bit for you (RECOMMENDED)
• Use an external monitor pod to rollout the DaemonSet when a resource changes (only one pod/container is needed and the action is auditable in the K8s logs)
• Use the operator
• Post-process the Helm template output to make your modifications
• Use raw YAML

[drehelis]

@stevehipwell Once again, request adding this is AFTER we've examined all the proposed solutions.

In my usecase, config is being generated dynamically. The only valid solution for this is using the operator which just doesn't suit our needs because it mandates to use manifests as input/filter/output configs while my config is native fluent bit structure.

Having the possibility to signal the process from a sidecar container is the goto solution.

Is adding a disclaimer comment stating the impacted security concern around using this toggle is fair enough solution while shipping this default to false would be enough?

[stevehipwell]

@drehelis I've also given you a number of solutions which would provide the exact functionality that you're looking for without needing to modify this Helm chart. I'm pushing for the Fluent Helm charts to be made more secure not less secure; if you have an edge case like this and don't want to follow advice on how to achieve your requirements you're more than welcome to either patch the Helm output or create and maintain your own chart.

[drehelis]

Waving the security flag here is just ridicules, however, that's your decision and I have no other means to convince you. Thank you for your time and effort.

[drehelis]

Hallelujah! Hot reload is here.
https://docs.fluentbit.io/manual/administration/hot-reload

[stevehipwell]

@drehelis this is due to go into the chart using the webserver method; I'll link the PR here.

[stevehipwell]

@drehelis #384 will add hot-reload support.