Re: [libvirt-ci PATCH v2 00/13] Introduce a new global YAML config file for lcitool

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 :)