Re: [libvirt] [jenkins-ci PATCH] lcitool: Don't encrypt password manually
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(a)redhat.com>
--
Andrea Bolognani / Red Hat / Virtualization