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

2018-05-17 Thread Daniel P . Berrangé
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

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

2018-05-17 Thread Andrea Bolognani
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

2018-05-17 Thread Daniel P . Berrangé
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 
> > > ---
> > >  guests/lcitool | 10 +-
> > >  1 file changed, 5 insertions(+), 5 deletions(-)
> > 
> > Reviewed-by: Daniel P. Berrangé 
> 
> 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

2018-05-17 Thread Andrea Bolognani
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 
> > ---
> >  guests/lcitool | 10 +-
> >  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> Reviewed-by: Daniel P. Berrangé 

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

2018-05-17 Thread Daniel P . Berrangé
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 
> ---
>  guests/lcitool | 10 +-
>  1 file changed, 5 insertions(+), 5 deletions(-)

Reviewed-by: Daniel P. Berrangé 

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