[libvirt] [jenkins-ci PATCH] lcitool: Use Perl to generate password hashes

Andrea Bolognani posted 1 patch 5 years, 11 months ago
Failed in applying to current master (apply log)
guests/lcitool | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
[libvirt] [jenkins-ci PATCH] lcitool: Use Perl to generate password hashes
Posted by Andrea Bolognani 5 years, 11 months ago
We claim to be using Python 2 at the moment: however, we rely
on crypt.mksalt(), which was introduced in Python 3 and has
only been backported to Python 2 in RHEL and Fedora, so the
script will only work on those operating systems.

We could move to Python 3, but the CI nodes are running on a
CentOS 7 machine, where it can't be installed without pulling
in third-party repositories and dealing with awkward naming;
moreover, Ansible itself is still tied to Python 2, so we
would be requiring both to be available.

Perl to the rescue! The script ends up being only marginally
more verbose and obscure as a result, the indentation is
significantly better, and it should finally run on pretty
much any platform.

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
---
 guests/lcitool | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/guests/lcitool b/guests/lcitool
index 5b2efb9..568e52c 100755
--- a/guests/lcitool
+++ b/guests/lcitool
@@ -18,11 +18,11 @@ die() {
 hash_file() {
     PASS_FILE="$1"
 
-    python2 -c "
-import crypt
-password = open('$PASS_FILE', 'r').read().strip()
-print(crypt.crypt(password,
-      crypt.mksalt(crypt.METHOD_SHA512)))"
+    perl -le '
+        my @chars = ("A".."Z", "a".."z", "0".."9");
+        my $salt; $salt .= $chars[rand @chars] for 1..16;
+        open(my $handle, "'"$PASS_FILE"'"); my $pass = <$handle>; chomp $pass;
+        print crypt("$pass", "\$6\$$salt\$");'
 }
 
 # yaml_var FILE VAR
-- 
2.17.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [jenkins-ci PATCH] lcitool: Use Perl to generate password hashes
Posted by Daniel P. Berrangé 5 years, 11 months ago
On Thu, May 17, 2018 at 03:09:11PM +0200, Andrea Bolognani wrote:
> We claim to be using Python 2 at the moment: however, we rely
> on crypt.mksalt(), which was introduced in Python 3 and has
> only been backported to Python 2 in RHEL and Fedora, so the
> script will only work on those operating systems.
> 
> We could move to Python 3, but the CI nodes are running on a
> CentOS 7 machine, where it can't be installed without pulling
> in third-party repositories and dealing with awkward naming;
> moreover, Ansible itself is still tied to Python 2, so we
> would be requiring both to be available.
> 
> Perl to the rescue! The script ends up being only marginally
> more verbose and obscure as a result, the indentation is
> significantly better, and it should finally run on pretty
> much any platform.
> 
> Signed-off-by: Andrea Bolognani <abologna@redhat.com>
> ---
>  guests/lcitool | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [jenkins-ci PATCH] lcitool: Use Perl to generate password hashes
Posted by Andrea Bolognani 5 years, 11 months ago
On Thu, 2018-05-17 at 14:18 +0100, Daniel P. Berrangé wrote:
> On Thu, May 17, 2018 at 03:09:11PM +0200, Andrea Bolognani wrote:
> > Perl to the rescue! The script ends up being only marginally
> > more verbose and obscure as a result, the indentation is
> > significantly better, and it should finally run on pretty
> > much any platform.
> > 
> > Signed-off-by: Andrea Bolognani <abologna@redhat.com>
> > ---
> >  guests/lcitool | 10 +++++-----
> >  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>

A friend suggested

diff --git a/guests/lcitool b/guests/lcitool
index 568e52c..5855368 100755
--- a/guests/lcitool
+++ b/guests/lcitool
@@ -21,7 +21,7 @@ hash_file() {
     perl -le '
         my @chars = ("A".."Z", "a".."z", "0".."9");
         my $salt; $salt .= $chars[rand @chars] for 1..16;
-        open(my $handle, "'"$PASS_FILE"'"); my $pass = <$handle>; chomp $pass;
+        open(my $handle, "'"$PASS_FILE"'"); chomp(my $pass = <$handle>);
         print crypt("$pass", "\$6\$$salt\$");'
 }

would be more idiomatic: mind if I squash that in before pushing?

-- 
Andrea Bolognani / Red Hat / Virtualization

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [jenkins-ci PATCH] lcitool: Use Perl to generate password hashes
Posted by Daniel P. Berrangé 5 years, 11 months ago
On Thu, May 17, 2018 at 04:10:37PM +0200, Andrea Bolognani wrote:
> On Thu, 2018-05-17 at 14:18 +0100, Daniel P. Berrangé wrote:
> > On Thu, May 17, 2018 at 03:09:11PM +0200, Andrea Bolognani wrote:
> > > Perl to the rescue! The script ends up being only marginally
> > > more verbose and obscure as a result, the indentation is
> > > significantly better, and it should finally run on pretty
> > > much any platform.
> > > 
> > > Signed-off-by: Andrea Bolognani <abologna@redhat.com>
> > > ---
> > >  guests/lcitool | 10 +++++-----
> > >  1 file changed, 5 insertions(+), 5 deletions(-)
> > 
> > Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> 
> A friend suggested
> 
> diff --git a/guests/lcitool b/guests/lcitool
> index 568e52c..5855368 100755
> --- a/guests/lcitool
> +++ b/guests/lcitool
> @@ -21,7 +21,7 @@ hash_file() {
>      perl -le '
>          my @chars = ("A".."Z", "a".."z", "0".."9");
>          my $salt; $salt .= $chars[rand @chars] for 1..16;
> -        open(my $handle, "'"$PASS_FILE"'"); my $pass = <$handle>; chomp $pass;
> +        open(my $handle, "'"$PASS_FILE"'"); chomp(my $pass = <$handle>);
>          print crypt("$pass", "\$6\$$salt\$");'
>  }
> 
> would be more idiomatic: mind if I squash that in before pushing?

Not sure I'd really agree - in fact at first glance I didn't expect this
to even work, because its declaring a variable inside a functin parameter.
I tested and it does work, but I find your original clearer.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [jenkins-ci PATCH] lcitool: Use Perl to generate password hashes
Posted by Andrea Bolognani 5 years, 11 months ago
On Thu, 2018-05-17 at 15:22 +0100, Daniel P. Berrangé wrote:
> On Thu, May 17, 2018 at 04:10:37PM +0200, Andrea Bolognani wrote:
> > A friend suggested
> > 
> > diff --git a/guests/lcitool b/guests/lcitool
> > index 568e52c..5855368 100755
> > --- a/guests/lcitool
> > +++ b/guests/lcitool
> > @@ -21,7 +21,7 @@ hash_file() {
> >      perl -le '
> >          my @chars = ("A".."Z", "a".."z", "0".."9");
> >          my $salt; $salt .= $chars[rand @chars] for 1..16;
> > -        open(my $handle, "'"$PASS_FILE"'"); my $pass = <$handle>; chomp $pass;
> > +        open(my $handle, "'"$PASS_FILE"'"); chomp(my $pass = <$handle>);
> >          print crypt("$pass", "\$6\$$salt\$");'
> >  }
> > 
> > would be more idiomatic: mind if I squash that in before pushing?
> 
> Not sure I'd really agree - in fact at first glance I didn't expect this
> to even work, because its declaring a variable inside a functin parameter.
> I tested and it does work, but I find your original clearer.

I was surprised as well, but then I noticed we're doing the same
thing with open() too :)

Can't say I disagree with you, though. We could actually go a bit
further and apply

diff --git a/guests/lcitool b/guests/lcitool
index 568e52c..0c1520e 100755
--- a/guests/lcitool
+++ b/guests/lcitool
@@ -21,7 +21,8 @@ hash_file() {
     perl -le '
         my @chars = ("A".."Z", "a".."z", "0".."9");
         my $salt; $salt .= $chars[rand @chars] for 1..16;
-        open(my $handle, "'"$PASS_FILE"'"); my $pass = <$handle>; chomp $pass;
+        my $handle; open($handle, "'"$PASS_FILE"'");
+        my $pass = <$handle>; chomp($pass);
         print crypt("$pass", "\$6\$$salt\$");'
 }

instead. I don't particularly care either way, but being consistent
seems preferable than using both approaches in the same line.

-- 
Andrea Bolognani / Red Hat / Virtualization

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [jenkins-ci PATCH] lcitool: Use Perl to generate password hashes
Posted by Daniel P. Berrangé 5 years, 11 months ago
On Thu, May 17, 2018 at 04:35:12PM +0200, Andrea Bolognani wrote:
> On Thu, 2018-05-17 at 15:22 +0100, Daniel P. Berrangé wrote:
> > On Thu, May 17, 2018 at 04:10:37PM +0200, Andrea Bolognani wrote:
> > > A friend suggested
> > > 
> > > diff --git a/guests/lcitool b/guests/lcitool
> > > index 568e52c..5855368 100755
> > > --- a/guests/lcitool
> > > +++ b/guests/lcitool
> > > @@ -21,7 +21,7 @@ hash_file() {
> > >      perl -le '
> > >          my @chars = ("A".."Z", "a".."z", "0".."9");
> > >          my $salt; $salt .= $chars[rand @chars] for 1..16;
> > > -        open(my $handle, "'"$PASS_FILE"'"); my $pass = <$handle>; chomp $pass;
> > > +        open(my $handle, "'"$PASS_FILE"'"); chomp(my $pass = <$handle>);
> > >          print crypt("$pass", "\$6\$$salt\$");'
> > >  }
> > > 
> > > would be more idiomatic: mind if I squash that in before pushing?
> > 
> > Not sure I'd really agree - in fact at first glance I didn't expect this
> > to even work, because its declaring a variable inside a functin parameter.
> > I tested and it does work, but I find your original clearer.
> 
> I was surprised as well, but then I noticed we're doing the same
> thing with open() too :)
> 
> Can't say I disagree with you, though. We could actually go a bit
> further and apply
> 
> diff --git a/guests/lcitool b/guests/lcitool
> index 568e52c..0c1520e 100755
> --- a/guests/lcitool
> +++ b/guests/lcitool
> @@ -21,7 +21,8 @@ hash_file() {
>      perl -le '
>          my @chars = ("A".."Z", "a".."z", "0".."9");
>          my $salt; $salt .= $chars[rand @chars] for 1..16;
> -        open(my $handle, "'"$PASS_FILE"'"); my $pass = <$handle>; chomp $pass;
> +        my $handle; open($handle, "'"$PASS_FILE"'");
> +        my $pass = <$handle>; chomp($pass);
>          print crypt("$pass", "\$6\$$salt\$");'
>  }
> 
> instead. I don't particularly care either way, but being consistent
> seems preferable than using both approaches in the same line.

Sure, good with me.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list