[libvirt] [jenkins-ci PATCH] lcitool: Don't encrypt password manually

Andrea Bolognani abologna at redhat.com
Wed Sep 5 13:38:57 UTC 2018


On Wed, 2018-09-05 at 11:39 +0200, Martin Kletzander wrote:
> On Tue, Sep 04, 2018 at 01:53:45PM +0200, Andrea Bolognani wrote:
> > On Tue, 2018-09-04 at 10:49 +0200, Martin Kletzander wrote:
> > 
> > s/manually/ourselves/ in the subject.

[...]
> > This is a really nice improvement overall, but we can't quite get
> > rid of the entire function: we still need to try and open the file,
> > or at least stat() it, like we do in get_vault_password_file(), so
> > that we can error out early instead of having Ansible bail out on
> > us really late in the game.
> 
> So what you had in mind is something like the following squashed in?

Pretty much exactly, yes :)

[...]
>      def get_root_password_file(self):
> -        return self._get_config_file("root-password")
> +        root_pass_file = None
> +

These two lines can be avoided.

> +        root_pass_file = self._get_config_file("root-password")
> +
> +        try:
> +            with open(root_pass_file, "r") as infile:
> +                if not infile.readline().strip():
> +                    raise ValueError

This introduces a *slight* semantic change, in that we won't
accept an empty root password anymore. I'd argue it's probably
for the best anyway, so let's just go ahead with it.

> Or we could have the check in ansible itself, but that would be a bigger change
> and the codebase is not prepared for that.

It shouldn't be *too* hard, and we could also take the
opportunity to fix the long-standing issue of 'update' not
working correctly if ~/.ssh/id_rsa.pub doesn't exist.

But we can worry about that another day; in the meantime, if
you fix the two nits pointed out above you can have my

  Reviewed-by: Andrea Bolognani <abologna at redhat.com>

-- 
Andrea Bolognani / Red Hat / Virtualization




More information about the libvir-list mailing list