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

Add manageiq user to disk group #435

Merged
merged 1 commit into from
Jan 9, 2024

Conversation

nasark
Copy link
Member

@nasark nasark commented Jan 2, 2024

Since we run as non-root, this will allow smartstate scans to be run on ISCSI store types

@miq-bot assign @agrare
@miq-bot add_label bug, quinteros/yes?

@miq-bot miq-bot added bug Something isn't working quinteros/yes? labels Jan 2, 2024
@@ -65,6 +65,7 @@ Requires: cockpit-ws
# Add sub uids/gids for running non-privileged containers
grep manageiq /etc/subuid >/dev/null 2>&1 || /usr/sbin/usermod --add-subuids 100000-165535 manageiq
grep manageiq /etc/subgid >/dev/null 2>&1 || /usr/sbin/usermod --add-subgids 100000-165535 manageiq
/usr/sbin/usermod -a -G disk manageiq
Copy link
Member

Choose a reason for hiding this comment

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

Also, very minor, but for future readers, I wonder if it's easier to use the full option names?

Suggested change
/usr/sbin/usermod -a -G disk manageiq
/usr/sbin/usermod --append --groups disk manageiq

Copy link
Member

Choose a reason for hiding this comment

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

We should check what the command does when the user is already part of the group, if it returns a non-zero exit code that will cause the rpm scriptlet to fail

Copy link
Member

Choose a reason for hiding this comment

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

[root@manageiq ~]# groups manageiq
manageiq : manageiq
[root@manageiq ~]# /usr/sbin/usermod --append --groups disk manageiq
[root@manageiq ~]# echo $?
0
[root@manageiq ~]# groups manageiq
manageiq : manageiq disk
[root@manageiq ~]# /usr/sbin/usermod --append --groups disk manageiq
[root@manageiq ~]# echo $?
0

Seems like it returns 0 in either case, might be a good idea to check if it is in the group first though groups manageiq | grep disk >/dev/null 2>&1 || /usr/sbin/usermod ... just so it is more obvious

@nasark nasark force-pushed the add_miq_user_to_disk_group branch from b69237b to a0da7a8 Compare January 2, 2024 19:04
@miq-bot
Copy link
Member

miq-bot commented Jan 2, 2024

Checked commit nasark@a0da7a8 with ruby 2.7.8, rubocop 1.56.3, haml-lint 0.51.0, and yamllint
0 files checked, 0 offenses detected
Everything looks fine. 🏆

@nasark
Copy link
Member Author

nasark commented Jan 2, 2024

Ready for another pass

@bdunne bdunne merged commit 658af63 into ManageIQ:master Jan 9, 2024
3 checks passed
@bdunne bdunne assigned bdunne and unassigned agrare Jan 9, 2024
@Fryguy
Copy link
Member

Fryguy commented Jan 11, 2024

Backported to quinteros in commit acab9a3.

commit acab9a30c34e093f55ad89bbebd1c0631a7d8eeb
Author: Brandon Dunne <brandondunne@hotmail.com>
Date:   Tue Jan 9 15:50:13 2024 -0500

    Merge pull request #435 from nasark/add_miq_user_to_disk_group
    
    Add manageiq user to disk group
    
    (cherry picked from commit 658af63fc1f5da6cda4e827d88572ad25a7bd7ca)

Fryguy pushed a commit that referenced this pull request Jan 11, 2024
Add manageiq user to disk group

(cherry picked from commit 658af63)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working quinteros/backported
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants