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

INI file with server auth setting sudo=true does not use sudo #1520

Open
zmughal opened this issue Dec 19, 2021 · 1 comment
Open

INI file with server auth setting sudo=true does not use sudo #1520

zmughal opened this issue Dec 19, 2021 · 1 comment
Labels
bug Confirmed bugs

Comments

@zmughal
Copy link

zmughal commented Dec 19, 2021

Describe the bug

Using sudo=true in an INI file as in the example for Rex::Group::Lookup::INI does not use sudo because it checks if the value is == 1, but this is a numeric comparison so "true" == 1 is false.

How to reproduce it

Steps to reproduce the behavior:

Shortest code example that demonstrates the bug:

# [= Rexfile =]
use Rex -feature => ['1.4','use_server_auth'];

use Rex::Group::Lookup::INI;

groups_file 'server-min.ini';

desc 'Install cpanminus';
task 'cpanminus', group => 'instr', sub {
	pkg "cpanminus", ensure => "present";
};
; [= server-min.ini =]
[instr]
;;; The below works without any Perl warnings

instrument-remote sudo=1


;;; Using the below instead says
;;;
;;; Argument "true" isn't numeric in numeric eq (==)
;;; at Rex/Interface/Connection/OpenSSH.pm
;;; on the line
;;;
;;;   if ( $self->{is_sudo} && $self->{is_sudo} == 1 ) {
;;;
;;; and
;;;
;;; Argument "true" isn't numeric in numeric eq (==)
;;; at Rex.pm
;;; on the line
;;;
;;;  if ( exists $CONNECTION_STACK[-1]->{server}->{auth}->{sudo}
;;;    && $CONNECTION_STACK[-1]->{server}->{auth}->{sudo} == 1 )
;;;

; instrument-remote sudo=true

Expected behavior

Should use sudo when sudo=true is in the INI file.

Per discussion on IRC with @ferki:

[ FErki[m]] at first glance, one possible fix might be the following:
[ FErki[m]] - change the test at https://github.com/RexOps/Rex/blob/master/t/ini.t#L100 to check for
            the value being `1`
[ FErki[m]] - change https://github.com/RexOps/Rex/blob/master/lib/Rex/Group/Entry/Server.pm#L78-L81
            to recognize the `"true"` string, and assign the value of `1` to the internal
            `$self->{auth}->{sudo}` (and perhaps add logic to recognize `"false"` as 0 as well)
[ FErki[m]] alternatively, we could clarify the docs (and the tests) that the value should be 0 or 1,
            but `true/false` seems to be more human-friendly to me in the context of auth info inside
            INI files
[ FErki[m]] a good next step could be to open a formal bug report as a GitHub issue to make it more
            visible for others (including the workaround) and to further discuss/design actual fixing
            steps

Circumstances

  • Rex version: (R)?ex 1.13.5
  • Perl version: perl 5, version 26, subversion 1 (v5.26.1) built for x86_64-linux
  • OS running rex: Debian GNU/Linux 5.15.0 amd64
  • OS managed by rex: Debian GNU/Linux 5.10.17 armv7l
  • How rex was installed: CPAN client

Debug log

@zmughal zmughal added the triage needed A potential bug that needs to be reproduced and understood label Dec 19, 2021
@ferki ferki added bug Confirmed bugs and removed triage needed A potential bug that needs to be reproduced and understood labels Jan 22, 2022
@ferki
Copy link
Member

ferki commented Jan 22, 2022

Thanks for your report, @zmughal!

I'll take a look at the tests and related code + docs to see how it's best to address this.

If I find it possible without breaking any backwards compatibility, I'll probably go for supporting true and yes as 1, and false and no as 0 values, while passing anything else as-is.

It's unlikely, but if something blocks this, I'll update the docs instead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Confirmed bugs
Projects
None yet
Development

No branches or pull requests

2 participants