pyinfra-dev / pyinfra

pyinfra turns Python code into shell commands and runs them on your servers. Execute ad-hoc commands and write declarative operations. Target SSH servers, local machine and Docker containers. Fast and scales from one server to thousands.
https://pyinfra.com
MIT License
3.85k stars 374 forks source link

Setting `_sudo` in the group data doesn't enable sudo #1151

Closed taliaferro closed 2 months ago

taliaferro commented 2 months ago

Describe the bug

The documentation about inventory data reads:

Data can also provide default values for Global Arguments, for example:

_sudo = True
_sudo_user = "pyinfra"

However, setting those variables in, for example, group_data/all.py, does not enable sudo by default for all operations.

To Reproduce

Here's an example using the @docker connector. The operation will succeed because the container already runs as root, but I've set it to maximum verbosity to see the actual commands getting run -- notice that those commands do not include sudo.

echo "_sudo=True" > /tmp/group_data.py
pyinfra -y -vvv \
    --group-data /tmp/group_data.py \
    @docker/debian:bookworm \
    apt.packages hello update=true 2>&1 \
    | grep "docker exec" # to show only the commands being run

output:

[@docker/debian:bookworm] >>> sh -c 'docker exec -i ccf7ebf5603de453c0d5b57d82ee6ffcffe5c20298841c9a72085596472af6f2 sh -c '"'"'sh -c '"'"'"'"'"'"'"'"'apt-get update'"'"'"'"'"'"'"'"''"'"''
[@docker/debian:bookworm] >>> sh -c 'docker exec -i ccf7ebf5603de453c0d5b57d82ee6ffcffe5c20298841c9a72085596472af6f2 sh -c '"'"'sh -c '"'"'"'"'"'"'"'"'! command -v dpkg >/dev/null || dpkg -l'"'"'"'"'"'"'"'"''"'"''
[@docker/debian:bookworm] >>> sh -c 'docker exec -i ccf7ebf5603de453c0d5b57d82ee6ffcffe5c20298841c9a72085596472af6f2 sh -c '"'"'sh -c '"'"'"'"'"'"'"'"'DEBIAN_FRONTEND=noninteractive apt-get -y -o Dpkg::Options::="--force-confdef" -o Dpkg::Options::="--force-confold" install hello'"'"'"'"'"'"'"'"''"'"''

Expected behavior

When I add the line _sudo=True to group_data/all.py, I expect operations to be run with sudo by default.

Meta

System: Linux
Platform: Linux-6.9.7-arch1-1-x86_64-with-glibc2.39
Release: 6.9.7-arch1-1
Machine: x86_64
pyinfra: v3.0
Executable: /home/taliaferro/.local/share/virtualenvs/infra-gRqFrJWx/bin/pyinfra
Python: 3.12.4 (CPython, GCC 14.1.1 20240522)
Fizzadar commented 2 months ago

Hi @taliaferro - you've stumbled across a long standing bug here!

The group_data flag is designed to specify a directory, but the actual check also joins group_data, so if you say --group-data tmp the file must be tmp/group_data/all.py - this is clearly very broken.

Additionally there's an issue loading global arguments from group data (filtering out keys starting _). I suspect this has gone unnoticed due to the expanded use of inventory functions as an alternative to group data.

So there's a few issues:

Fizzadar commented 2 months ago

Fixed in https://github.com/pyinfra-dev/pyinfra/commit/139d8b6c794554e5f7c4ba6d93852fbf253aa0d6 and https://github.com/pyinfra-dev/pyinfra/commit/a03c2cef281c6b0f3f7df95ee89a554f4a31dd6c. Will be releasing 3.0.1 with these today.