Use container/pod securityContext user/group as owner for /usr/share/opensearch/data directory #1109
Conversation
patelsmit32123
requested review from
idanl21,
jochenkressin,
pchmielnik,
prudhvigodithi,
salyh and
swoehrl-mw
as code owners
patelsmit32123
changed the title
…/opensearch/data directory Signed-off-by: patelsmit32123 <[email protected]>
patelsmit32123
force-pushed
the
configurable-data-owner
branch
from
b238dce to
a0fe23d
Compare
patelsmit32123
changed the title
| ImagePullPolicy: initHelperImage.GetImagePullPolicy(), | ||
| Resources: resources, | ||
| Command: []string{"sh", "-c"}, | ||
| Args: []string{"chown -R 1000:1000 /usr/share/opensearch/data"}, |
There was a problem hiding this comment.
The 1000:1000 is based on official OpenSearch docker image https://github.com/opensearch-project/opensearch-build/blob/main/docker/release/dockerfiles/opensearch.al2023.dockerfile#L19-L20.
|
This is a good change. LGTM |
| } | ||
|
|
||
| // ResolveUidGid resolves the UID and GID using security context hierarchy | ||
| // Priority: securityContext.runAsUser/Group > podSecurityContext.runAsUser/Group > defaults (1000:1000) |
There was a problem hiding this comment.
Can you please add this to the repo and helm charts readme https://github.com/opensearch-project/opensearch-k8s-operator/blob/main/docs/userguide/main.md ?
There was a problem hiding this comment.
Does the operator really need to know about UID & GID?
If the securityContext "fsGroup: 1000" would be set, the PVC is chowned on volume mount.
Docs:
fsGroup: A special supplemental group that applies to all containers in a pod. Some volume types allow the Kubelet to change the ownership of that volume to be owned by the pod: 1. The owning GID will be the FSGroup 2. The setgid bit is set (new files created in the volume will be owned by FSGroup) 3. The permission bits are OR'd with rw-rw---- If unset, the Kubelet will not modify the ownership and permissions of any volume. Note that this field cannot be set when spec.os.name is windows.
|
@patelsmit32123 you can create a separate PR for this comment #1109 (comment). |
github-project-automation
bot
moved this to 👀 In Review
in Engineering Effectiveness Board
github-project-automation
bot
moved this from 👀 In Review
to ✅ Done
in Engineering Effectiveness Board
3 participants
Issues Resolved
Fixes #1108
Check List
make lint)If CRDs are changed:
make manifests) and also copied into the helm chartPlease refer to the PR guidelines before submitting this pull request.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.