Fixes #36365 - Drop usage of sudo for any pulpcore-manager commands

[sayan3296]

If root is not allowed to use sudo , users will bound to run into either of these issues during upgrade or restore

E, [2023-03-10 03:48:22+0000 #49723] ERROR -- : Failed executing sudo PULP_SETTINGS=/etc/pulp/settings.py DJANGO_SETTINGS_MODULE=pulpcore.app.settings pulpcore-managern migrate --noinput, exit status 1:
sudo: sudoers specifies that root is not allowed to sudo (ForemanMaintain::Error::ExecutionError)

or

Failed executing sudo PULP_SETTINGS=/etc/pulp/settings.py DJANGO_SETTINGS_MODULE=pulpcore.app.settings pulpcore-manager rpm-trim-changelogs, exit status 1:
Sorry, user root is not allowed to execute '/usr/bin/pulpcore-manager rpm-trim-changelogs' as root on satellite.example.com

Since we anyway run foreman-maintain\satellite-maintain as the root user, Let's remove the usage of sudo to run those commands normally.

I did a test for restore and upgrade with these changes and they worked just fine.

It would also probably need to go in 1.2.X ( if any future releases are planned )

[theforeman-bot]

Issues: #36365

[ekohl]

Technically we should run all pulpcore-manager commands as the pulp user. Especially when it writes files that's crucial. When running as root, then you can end up with file permission problems.

IMHO we need some wrapper for pulpcore-manager with the correct env vars etc.

[sayan3296]

Technically we should run all pulpcore-manager commands as the pulp user. Especially when it writes files that's crucial. When running as root, then you can end up with file permission problems.

IMHO we need some wrapper for pulpcore-manager with the correct env vars etc.

I do agree and that is what my initial thought was. The same is mentioned in the BZ as well. But so far, these commands are being executed as the root user only ( with sudo ) and things are working just fine.

I also discussed with the Pulp team about it and they confirmed that If I am passing

PULP_SETTINGS='/etc/pulp/settings.py' DJANGO_SETTINGS_MODULE=pulpcore.app.settings

along with the pulpcore-manager command and the whole command is executed as root user, Then things should work just fine as pulpcore-manager would know who to identify itself as, to the db as well as django and it gets that from the settings

If your recommendation remains the same, Then I can try to think of something about that wrapper or else I would have to call both the commands in this way i.e. su - pulp -s /bin/bash -c "command here" ( since pulp has nologin shell ).

[ekohl]

While it's true that when it's DB-only it's fine, it is a problem when root starts to write files to /var/lib/pulp.

The correct tool for switching user in scripts is runuser.

[sayan3296]

Technically we should run all pulpcore-manager commands as the pulp user. Especially when it writes files that's crucial. When running as root, then you can end up with file permission problems.
IMHO we need some wrapper for pulpcore-manager with the correct env vars etc.

I do agree and that is what my initial thought was. The same is mentioned in the BZ as well. But so far, these commands are being executed as the root user only ( with sudo ) and things are working just fine.

I also discussed with the Pulp team about it and they confirmed that If I am passing

PULP_SETTINGS='/etc/pulp/settings.py' DJANGO_SETTINGS_MODULE=pulpcore.app.settings

along with the pulpcore-manager command and the whole command is executed as root user, Then things should work just fine as pulpcore-manager would know who to identify itself as, to the db as well as django and it gets that from the settings

If your recommendation remains the same, Then I can try to think of something about that wrapper or else I would have to call both the commands in this way i.e. su - pulp -s /bin/bash -c "command here" ( since pulp has nologin shell ).

And to add on, Both of these commands ( from where sudo is being removed ) are performing database-level actions ( on pulpcore ) as database user pulp and the user is being picked up from /etc/pulp/settings.py. So even if the whole command is executed as root, the main action is being taken as DB user pulp only.

[sayan3296]

While it's true that when it's DB-only it's fine, it is a problem when root starts to write files to /var/lib/pulp.

The correct tool for switching user in scripts is runuser.

I just noticed this one and yes, Agreed.

Writing on disk as root is super bad but fortunately, neither of these actions is doing so.

And also, whether it's runuser or su, as pulp is associated with nologin shell, we need to pass -s /bin/bash ( whether we use the command directly or create a wrapper ).

I will wait for your final recommendation here and take the necessary actions as per the same.

[sayan3296]

Pushed changes in the commit as per previous suggestions. Now foreman_maintain should run the pulpcore db migration and trim-changelogs steps as pulp user using runuser

@ekohl Please, Let me know if that looks good to you or if any other recommendations would be there

[ekohl]

You can avoid the additional shell by using runuser -u pulp.

# getent passwd pulp
pulp:x:974:964::/var/lib/pulp:/sbin/nologin
# runuser - pulp id
This account is currently not available.
# runuser -u pulp id
uid=974(pulp) gid=964(pulp) groups=964(pulp) context=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023

And I'm pretty sure DJANGO_SETTINGS_MODULE isn't needed. pulpcore-manager is precisely that. See https://github.com/pulp/pulpcore/blob/7c6d59f5f302efab669e335a23f1912eae739f2c/pulpcore/app/manage.py#L7 for details. Sadly it doesn't set PULP_SETTINGS either but you can't have it all.

So I think PULP_SETTINGS=/etc/pulp/settings.py runuser -u pulp pulpcore-manager $COMMAND would be the best invocation.

[ehelms]

Given y'all have aligned on a common structure for calling commands, adding that as a function in https://github.com/theforeman/foreman_maintain/blob/master/lib/foreman_maintain/concerns/pulp_common.rb to which you can just pass the unique parts from each would be helpful so that any future commands call out properly.

[sayan3296]

You can avoid the additional shell by using runuser -u pulp.

# getent passwd pulp
pulp:x:974:964::/var/lib/pulp:/sbin/nologin
# runuser - pulp id
This account is currently not available.
# runuser -u pulp id
uid=974(pulp) gid=964(pulp) groups=964(pulp) context=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023

And I'm pretty sure DJANGO_SETTINGS_MODULE isn't needed. pulpcore-manager is precisely that. See https://github.com/pulp/pulpcore/blob/7c6d59f5f302efab669e335a23f1912eae739f2c/pulpcore/app/manage.py#L7 for details. Sadly it doesn't set PULP_SETTINGS either but you can't have it all.

So I think PULP_SETTINGS=/etc/pulp/settings.py runuser -u pulp pulpcore-manager $COMMAND would be the best invocation.

I actually had tried it but it failed and my mistake was that I passed PULP_SETTINGS=/etc/pulp/settings.py after runuser -u pulp that was stupid of me.

And I never removed DJANGO_SETTINGS_MODULE=pulpcore.app.settings as it was always part of the old code.

Now, even if I have this working i.e.

PULP_SETTINGS=/etc/pulp/settings.py runuser -u pulp pulpcore-manager rpm-trim-changelogs

as soon as i try the same with migrate, It will not work . Examples:

# PULP_SETTINGS=/etc/pulp/settings.py runuser -u pulp pulpcore-manager migrate --noinput
runuser: unrecognized option '--noinput'
Try 'runuser --help' for more information.

# PULP_SETTINGS=/etc/pulp/settings.py runuser -u pulp 'pulpcore-manager migrate --noinput'
runuser: failed to execute pulpcore-manager migrate --noinput: Permission denied

# runuser -u pulp 'PULP_SETTINGS=/etc/pulp/settings.py pulpcore-manager migrate --noinput' 
runuser: failed to execute PULP_SETTINGS=/etc/pulp/settings.py pulpcore-manager migrate --noinput: Permission denied

That is why I resorted to the generic approach i.e. runuser - pulp -s /bin/bash -c . But do let me know if there is a better way to run the "pulpcore-manager migrate --noinput" command here.

[ekohl]

I'ts probably interpreting --noinput as an argument to runuser. I suspect PULP_SETTINGS=/etc/pulp/settings.py runuser -u pulp -- pulpcore-manager migrate --noinput will work.

[sayan3296]

I'ts probably interpreting --noinput as an argument to runuser. I suspect PULP_SETTINGS=/etc/pulp/settings.py runuser -u pulp -- pulpcore-manager migrate --noinput will work.

It does the trick.. I have force pushed the changes now.