Skip to content

Conversation

@theeternalrat
Copy link
Contributor

Resolves #449 . This ads the ability run the parent pod as a select uid/gid, and run an init container that will set file permissions appropriately on the mounted directories.

@theeternalrat theeternalrat requested review from a team and ilyam8 as code owners April 9, 2025 15:57
@CLAassistant
Copy link

CLAassistant commented Apr 9, 2025

CLA assistant check
All committers have signed the CLA.

@witalisoft
Copy link
Contributor

Thanks for the PR

Could you tell us what kind of use case it solves ?

btw there are some files inside the containers that are bound to that builtin user netdata (uid: 201)
how will this work after overriding the runAsUser or/and runAsGroup ?

@theeternalrat
Copy link
Contributor Author

@witalisoft I can't speak for the original author of #449, but my use-case is that I am running Netdata on an AWS EFS backed volume. If the U/ID of the container (201) does not match the ID of the filesystem, the Netdata process quits when it tries to chown the data directories. EFS squashes the file permissions for all operations, but it doesn't allow any chown operation on the files. Should any process attempt to chown, it is not allowed. Using this feature, I was able to set the container ID to match the EFS volume, allowing the Netdata startup checks to pass.

I cannot definitively say what effect might be for those specified files, but I have been running these changes in my cluster for the last day with seemingly normal operation.

@witalisoft
Copy link
Contributor

witalisoft commented Apr 11, 2025

@theeternalrat
Sounds reasonable, but having another use case introduces another piece of code to maintain, especially if it's a divergence from what we expect to have.

So what I would prefer is to put the placeholder for the initContainers in templates/parent/deployment.yaml like this:

...
{{ toYaml .Values.parent.extraInitContainers | indent 8 }}
      containers:
...

Now you get the flexibility, and we're not stuck with another use case for testing the helm chart.

The rest of what you presented like securityContext is ok.

@theeternalrat
Copy link
Contributor Author

@witalisoft I've made the requested change. It'll now be up to the user to defined their own volume-permissions container if they so choose.

Copy link
Contributor

@M4itee M4itee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good. Thanks @theeternalrat

@M4itee M4itee merged commit 7fa2177 into netdata:master Apr 14, 2025
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Feature Request | Enforce Volume Permissions

4 participants