On Wed, May 13, 2020 at 07:00:49PM +0200, Andrea Bolognani wrote:
> On Tue, 2020-05-12 at 16:42 +0200, Erik Skultety wrote:
> > This series is trying to consolidate the number of config files we currently
> > recognize under ~/.config/lcitool to a single global YAML config file. Thanks
> > to this effort we can expose more seetings than we previously could which will
> > come handy in terms of generating cloudinit images for OpenStack.
> >
> > Patches 1-4 (ACKed)
> >
> > Since RFC:
> > - replaced TOML with YAML which simplified some aspects of the code, thanks
> > Andrea
> > - instead of hardcoding the default values, the config within the repo is used
> > as a template and overriden with user-selected options
> >
> > Since v1:
> > - uncommented the mandatory options in the default template YAML config so that
> > we know about all the supported keys which is useful for validating the user
> > config
> > - removed guests/group_vars/all/install.yaml in patch 11 (which I forgot in v1)
> > - added checks for value types we get from the config
> > - use yaml.safe_load instead of yaml.load
> > - added code snippet to delete keys we don't recognize so as not to introduce a
> > security issue, because we essentially just take the config and pass it to
> > ansible, we don't want users to use to re-define some of Ansible's variables
> > - added the last patch just to demonstrate a number of test cases I used as a
> > proof for correctness of this revision (feel free to add more cases), but
> > this is not the right series to add pytest support into lcitool
>
> You should have all the ACKs you need now.
>
> Thanks for being patient during review, and for taking care of this
> in the first place :)
Thanks for the review, changes are now merged.
--
Erik Skultety