feat: enable ai-services to run in non-root mode#682
feat: enable ai-services to run in non-root mode#682Shubhamag12 wants to merge 5 commits intoIBM:mainfrom
Conversation
| type: Directory | ||
| containers: | ||
| - name: instruct | ||
| securityContext: |
There was a problem hiding this comment.
you don't need I believe, all the containers will have this label
There was a problem hiding this comment.
and same comment applies for all the other places where you have these labels.
There was a problem hiding this comment.
Okay, I will verify this and remove if not required
| } | ||
|
|
||
| // buildAndInstallSELinuxPolicy builds and installs the SELinux policy module. | ||
| func buildAndInstallSELinuxPolicy(tmpDir string) error { |
There was a problem hiding this comment.
may be we need to make this generic for other policy like podman socket, will do in a follow up PR
| } | ||
|
|
||
| // updateSELinuxFileContext updates the SELinux file context database. | ||
| func updateSELinuxFileContext() error { |
There was a problem hiding this comment.
This function explicitly labels the device file with but this needs to be run every time when new devices are created(hotplug etc.) to avoid this we can enhance the existing udev rule which will add the labels automatically when there is a new device onboard.
e.g:
# Example udev rule
SUBSYSTEM=="vfio", ACTION=="add", SECLABEL{selinux}="system_u:object_r:vfio_device_t:s0"
explore adding the content to the already existing udev rule
| } | ||
|
|
||
| // applySELinuxLabels applies SELinux labels to existing VFIO devices. | ||
| func applySELinuxLabels() error { |
There was a problem hiding this comment.
even this is not required if we handle them via udev rule which I mentioned above.
| @@ -25,10 +25,18 @@ func (r *RootRule) Description() string { | |||
| func (r *RootRule) Verify() error { | |||
There was a problem hiding this comment.
this becomes info only check instead of a blocking one?! correct if I'm wrong!
There was a problem hiding this comment.
I think you are right
That leaves us with one confusion, for bootstrap we require either sudo or root privileges. Also, this function is called while creating application as well (via validate), there this function will fail if user is non-root.
There was a problem hiding this comment.
I'm little worried about the below modification you are making that is it! IMO we should just remove this root validator itself considering we don't need root anymore..
Signed-off-by: sagarwal-ibm <sagarwal@ibm.com>
Signed-off-by: sagarwal-ibm <sagarwal@ibm.com>
Signed-off-by: sagarwal-ibm <sagarwal@ibm.com>
Signed-off-by: Shubham Agarwal <49315586+Shubhamag12@users.noreply.github.com>
Signed-off-by: sagarwal-ibm <sagarwal@ibm.com>
| if euid != 0 && os.Getenv("XDG_RUNTIME_DIR") == "" { | ||
| uid := os.Getuid() | ||
| logger.Infoln("running command as %s", uid, logger.VerbosityLevelDebug) | ||
| if err := os.Setenv("XDG_RUNTIME_DIR", fmt.Sprintf("/run/user/%d", uid)); err != nil { |
There was a problem hiding this comment.
If we set it here, it will only apply to that specific shell. I'm not sure why we're setting this environment variable here at all.
There was a problem hiding this comment.
Should I move it to create cmd in that case?
because while creating application, I was facing this error
failed to pull image registry.redhat.io/rhaiis/vllm-spyre-rhel9:3.2.5: reading JSON file "/run/containers/1001/auth.json": open /run/containers/1001/auth.json: permission denied
Even though the above error says permission denied, it was because of the fact that podman was not able to access the runtime files.
So thought of exporting this var here since we call validate function everytime we create application
There was a problem hiding this comment.
remove this validator and fix this permission somewhere else
and fix this somewhere else wher
|
|
||
| const ( | ||
| // dirPermissions is the default permission for creating directories. | ||
| dirPermissions = 0755 |
There was a problem hiding this comment.
These permissions are for tow different things, let us keep this variable here as well for the local consumption.
2 participants
Non-root user implementation based on proposal: #683