| Message ID | 20250113-jag-bringup_fixes-v1-2-fb28030b1f26@kernel.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | kdevops: Various fixes | expand |
On Mon, Jan 13, 2025 at 12:53:00PM +0100, Joel Granados wrote: > * New heuristic uses the following sudo command options: > -n : "non-interactive"; will prevent sudo from prompting the user for > any kind of input. > -v : Update the user's cache credentials; has distinct output for users > with and without sudo: > 1. Without sudo the output is "Sorry, user __USER__ may not run > sudo..." > 2. With sudo it has two messages (with and without password). But > we ignore the distinction as both of these mean that user can > sudo. > * Avoid setting SUDO_ASKPASS. This env variable is suppose to have the > executable that will be executed when asking for sudo instead of a > boolean. > > Signed-off-by: Joel Granados <joel.granados@kernel.org> > --- > scripts/libvirt_pool.sh | 25 +++++++++++++++++++++---- > 1 file changed, 21 insertions(+), 4 deletions(-) > > diff --git a/scripts/libvirt_pool.sh b/scripts/libvirt_pool.sh > index f8c309e..ffae928 100644 > --- a/scripts/libvirt_pool.sh > +++ b/scripts/libvirt_pool.sh > @@ -1,6 +1,26 @@ > #!/bin/bash > # Helpers to work with virsh pools > > +get_can_sudo() > +{ > + # Heuristic to see if the current user can sudo > + # -n : This is the non-interactive argument in sudo and will prevent > + # sudo from prompting the user for any kind of input. > + # -v : This tries to update the users cache credentials but also has > + # distinct output for users with and without sudo: > + # 1. Without sudo the output is > + # "Sorry, user __USER__ may not run sudo..." > + # 2. With sudo it has two messages: one for paswordless sudo and > + # one passwordfull sudo. But we ignore the distinction as both > + # of these mean that can_sudo is "y". (2.) Both can sudo but we are not solving yet the paswordfull sudo case in Ansible. Since we may add that support later, and it does not break passwordless systems: looks good to me. Reviewed-by: Daniel Gomez <da.gomez@samsung.com> > + if [[ $(sudo -nv 2>&1 | grep 'may not' > /dev/null) -eq 0 ]]; then > + echo "n" > + exit > + fi > + echo "y" > + exit > +} > + > get_pool_vars() > { > if [[ -f $OS_FILE ]]; then > @@ -10,10 +30,7 @@ get_pool_vars() > fi > fi > > - SUDO_ASKPASS=/bin/false sudo -A whoami > /dev/null 2>&1 > - if [[ $? -eq 0 ]]; then > - CAN_SUDO="y" > - fi > + CAN_SUDO=get_can_sudo > > if [[ "$USES_QEMU_USER_SESSION" != "y" ]]; then > REQ_SUDO="sudo" > > -- > 2.44.2 > >
On Thu, Jan 30, 2025 at 09:23:09AM +0100, Daniel Gomez wrote: > On Mon, Jan 13, 2025 at 12:53:00PM +0100, Joel Granados wrote: > > * New heuristic uses the following sudo command options: > > -n : "non-interactive"; will prevent sudo from prompting the user for > > any kind of input. > > -v : Update the user's cache credentials; has distinct output for users > > with and without sudo: > > 1. Without sudo the output is "Sorry, user __USER__ may not run > > sudo..." > > 2. With sudo it has two messages (with and without password). But > > we ignore the distinction as both of these mean that user can > > sudo. > > * Avoid setting SUDO_ASKPASS. This env variable is suppose to have the > > executable that will be executed when asking for sudo instead of a > > boolean. > > > > Signed-off-by: Joel Granados <joel.granados@kernel.org> > > --- > > scripts/libvirt_pool.sh | 25 +++++++++++++++++++++---- > > 1 file changed, 21 insertions(+), 4 deletions(-) > > > > diff --git a/scripts/libvirt_pool.sh b/scripts/libvirt_pool.sh > > index f8c309e..ffae928 100644 > > --- a/scripts/libvirt_pool.sh > > +++ b/scripts/libvirt_pool.sh > > @@ -1,6 +1,26 @@ > > #!/bin/bash > > # Helpers to work with virsh pools > > > > +get_can_sudo() > > +{ > > + # Heuristic to see if the current user can sudo > > + # -n : This is the non-interactive argument in sudo and will prevent > > + # sudo from prompting the user for any kind of input. > > + # -v : This tries to update the users cache credentials but also has > > + # distinct output for users with and without sudo: > > + # 1. Without sudo the output is > > + # "Sorry, user __USER__ may not run sudo..." > > + # 2. With sudo it has two messages: one for paswordless sudo and > > + # one passwordfull sudo. But we ignore the distinction as both > > + # of these mean that can_sudo is "y". > > (2.) Both can sudo but we are not solving yet the paswordfull sudo case > in Ansible. Since we may add that support later, and it does not break > passwordless systems: looks good to me. This patch is probably the only one in this series that stands on its own. How about we push this one by itself? > > Reviewed-by: Daniel Gomez <da.gomez@samsung.com> > > > + if [[ $(sudo -nv 2>&1 | grep 'may not' > /dev/null) -eq 0 ]]; then > > + echo "n" > > + exit > > + fi > > + echo "y" > > + exit > > +} > > + > > get_pool_vars() > > { > > if [[ -f $OS_FILE ]]; then > > @@ -10,10 +30,7 @@ get_pool_vars() > > fi > > fi > > > > - SUDO_ASKPASS=/bin/false sudo -A whoami > /dev/null 2>&1 > > - if [[ $? -eq 0 ]]; then > > - CAN_SUDO="y" > > - fi > > + CAN_SUDO=get_can_sudo > > > > if [[ "$USES_QEMU_USER_SESSION" != "y" ]]; then > > REQ_SUDO="sudo" > > > > -- > > 2.44.2 > > > >
On Thu, Jan 30, 2025 at 09:23:09AM +0100, Daniel Gomez wrote: > On Mon, Jan 13, 2025 at 12:53:00PM +0100, Joel Granados wrote: > > * New heuristic uses the following sudo command options: > > -n : "non-interactive"; will prevent sudo from prompting the user for > > any kind of input. > > -v : Update the user's cache credentials; has distinct output for users > > with and without sudo: > > 1. Without sudo the output is "Sorry, user __USER__ may not run > > sudo..." > > 2. With sudo it has two messages (with and without password). But > > we ignore the distinction as both of these mean that user can > > sudo. > > * Avoid setting SUDO_ASKPASS. This env variable is suppose to have the > > executable that will be executed when asking for sudo instead of a > > boolean. > > > > Signed-off-by: Joel Granados <joel.granados@kernel.org> > > --- > > scripts/libvirt_pool.sh | 25 +++++++++++++++++++++---- > > 1 file changed, 21 insertions(+), 4 deletions(-) > > > > diff --git a/scripts/libvirt_pool.sh b/scripts/libvirt_pool.sh > > index f8c309e..ffae928 100644 > > --- a/scripts/libvirt_pool.sh > > +++ b/scripts/libvirt_pool.sh > > @@ -1,6 +1,26 @@ > > #!/bin/bash > > # Helpers to work with virsh pools > > > > +get_can_sudo() > > +{ > > + # Heuristic to see if the current user can sudo > > + # -n : This is the non-interactive argument in sudo and will prevent > > + # sudo from prompting the user for any kind of input. > > + # -v : This tries to update the users cache credentials but also has > > + # distinct output for users with and without sudo: > > + # 1. Without sudo the output is > > + # "Sorry, user __USER__ may not run sudo..." > > + # 2. With sudo it has two messages: one for paswordless sudo and > > + # one passwordfull sudo. But we ignore the distinction as both > > + # of these mean that can_sudo is "y". > > (2.) Both can sudo but we are not solving yet the paswordfull sudo case > in Ansible. Since we may add that support later, and it does not break > passwordless systems: looks good to me. > > Reviewed-by: Daniel Gomez <da.gomez@samsung.com> I'll cherry-pick this one into a V2 of this patchset. I could probably just push it after your Reviewed-by, but I'll take it out for more clarity. > > > + if [[ $(sudo -nv 2>&1 | grep 'may not' > /dev/null) -eq 0 ]]; then > > + echo "n" > > + exit > > + fi > > + echo "y" > > + exit > > +} > > + > > get_pool_vars() > > { > > if [[ -f $OS_FILE ]]; then > > @@ -10,10 +30,7 @@ get_pool_vars() > > fi > > fi > > > > - SUDO_ASKPASS=/bin/false sudo -A whoami > /dev/null 2>&1 > > - if [[ $? -eq 0 ]]; then > > - CAN_SUDO="y" > > - fi > > + CAN_SUDO=get_can_sudo > > > > if [[ "$USES_QEMU_USER_SESSION" != "y" ]]; then > > REQ_SUDO="sudo" > > > > -- > > 2.44.2 > > > >
diff --git a/scripts/libvirt_pool.sh b/scripts/libvirt_pool.sh index f8c309e..ffae928 100644 --- a/scripts/libvirt_pool.sh +++ b/scripts/libvirt_pool.sh @@ -1,6 +1,26 @@ #!/bin/bash # Helpers to work with virsh pools +get_can_sudo() +{ + # Heuristic to see if the current user can sudo + # -n : This is the non-interactive argument in sudo and will prevent + # sudo from prompting the user for any kind of input. + # -v : This tries to update the users cache credentials but also has + # distinct output for users with and without sudo: + # 1. Without sudo the output is + # "Sorry, user __USER__ may not run sudo..." + # 2. With sudo it has two messages: one for paswordless sudo and + # one passwordfull sudo. But we ignore the distinction as both + # of these mean that can_sudo is "y". + if [[ $(sudo -nv 2>&1 | grep 'may not' > /dev/null) -eq 0 ]]; then + echo "n" + exit + fi + echo "y" + exit +} + get_pool_vars() { if [[ -f $OS_FILE ]]; then @@ -10,10 +30,7 @@ get_pool_vars() fi fi - SUDO_ASKPASS=/bin/false sudo -A whoami > /dev/null 2>&1 - if [[ $? -eq 0 ]]; then - CAN_SUDO="y" - fi + CAN_SUDO=get_can_sudo if [[ "$USES_QEMU_USER_SESSION" != "y" ]]; then REQ_SUDO="sudo"
* New heuristic uses the following sudo command options: -n : "non-interactive"; will prevent sudo from prompting the user for any kind of input. -v : Update the user's cache credentials; has distinct output for users with and without sudo: 1. Without sudo the output is "Sorry, user __USER__ may not run sudo..." 2. With sudo it has two messages (with and without password). But we ignore the distinction as both of these mean that user can sudo. * Avoid setting SUDO_ASKPASS. This env variable is suppose to have the executable that will be executed when asking for sudo instead of a boolean. Signed-off-by: Joel Granados <joel.granados@kernel.org> --- scripts/libvirt_pool.sh | 25 +++++++++++++++++++++---- 1 file changed, 21 insertions(+), 4 deletions(-)