Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix user and group ID #251

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

Fix user and group ID #251

wants to merge 1 commit into from

Conversation

blackliner
Copy link

I think the current values can't work, and they don't work when using nfs-csi:

/opt/bedrock-entry.sh: line 30: restify.err: Permission denied

@itzg
Copy link
Owner

itzg commented Mar 6, 2025

It's a security best practice. Refer to the contributing PR and issue

#155

Besides, those are just defaults. Set those values as you need during deploy time.

@blackliner
Copy link
Author

blackliner commented Mar 7, 2025

It's a security best practice.

By "it" you mean using securityContext:, then yes, I fully agree, no changes there. But the chosen numbers for UID (1000) and GID (3000) won't work with fsgroup (2000). The example in the official docs does not specify these numbers for any meaningful reason, other than showing these are integers, like the usage of foo, bar, and baz that you might know of in many docs or tutorials.

Regarding defaults, shouldn't it be safe AND working by default?

I will double check if anything else could be the reason for this not working with csi-nfs

@itzg
Copy link
Owner

itzg commented Mar 7, 2025

@uhthomas sorry to ping you after a 2 year old PR, but can you comment on the points brought up here?

@itzg
Copy link
Owner

itzg commented Mar 7, 2025

Regarding defaults, shouldn't it be safe AND working by default?

This is the first I'm hearing that the values there don't work by default.

I don't use any of these charts myself, so I personally can't speak to the usability of the defaults.

@blackliner blackliner marked this pull request as draft March 7, 2025 17:57
@blackliner
Copy link
Author

I have a suspicion that this is the fault of cni-nfs, I'll keep you posted

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.

2 participants