[libvirt-ci PATCH] guests: don't force user to create a config.yaml file

Daniel P. Berrangé posted 1 patch 2 weeks ago
Failed in applying to current master (apply log)
guests/lcitool | 51 +++++++++++++++++++++++++-------------------------
1 file changed, 25 insertions(+), 26 deletions(-)

[libvirt-ci PATCH] guests: don't force user to create a config.yaml file

Posted by Daniel P. Berrangé 2 weeks ago
Only the "install" command requires a per-user config.yaml with a
password set. We should not require this for the other commands
to be run.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 guests/lcitool | 51 +++++++++++++++++++++++++-------------------------
 1 file changed, 25 insertions(+), 26 deletions(-)

diff --git a/guests/lcitool b/guests/lcitool
index 27aee7b..1f093d0 100755
--- a/guests/lcitool
+++ b/guests/lcitool
@@ -138,11 +138,15 @@ class Config:
         with open(os.path.join(base, "config.yaml"), "r") as fp:
             self.values = yaml.safe_load(fp)
 
+        user_config_path = self._get_config_file("config.yaml")
+        if not os.path.exists(user_config_path):
+            return
+
         try:
-            with open(self._get_config_file("config.yaml"), "r") as fp:
+            with open(user_config_path, "r") as fp:
                 user_config = yaml.safe_load(fp)
         except Exception as e:
-            raise Exception("Missing or invalid config.yaml file: {}".format(e))
+            raise Exception("Invalid config.yaml file: {}".format(e))
 
         if user_config is None:
             raise Exception("Missing or invalid config.yaml file")
@@ -181,20 +185,13 @@ class Config:
             if k not in known_keys:
                 del _dict[k]
 
-    def _validate_section(self, config, section, mandatory_keys):
+    def _validate_section(self, config, section):
         # remove keys we don't recognize
         self._remove_unknown_keys(config[section], self.values[section].keys())
 
-        # check that the mandatory keys are present and non-empty
-        for key in mandatory_keys:
-            if config.get(section).get(key) is None:
-                raise Exception(("Missing or empty value for mandatory key"
-                                 "'{}.{}'").format(section, key))
-
         # check that all keys have values assigned and of the right type
         for key in config[section].keys():
 
-            # mandatory keys were already checked, so this covers optional keys
             if config[section][key] is None:
                 raise Exception(
                     "Missing value for '{}.{}'".format(section, key)
@@ -209,22 +206,6 @@ class Config:
         # delete sections we don't recognize
         self._remove_unknown_keys(config, self.values.keys())
 
-        if "install" not in config:
-            raise Exception("Missing mandatory section 'install'")
-
-        self._validate_section(config, "install", ["root_password"])
-
-        # we only need this for the gitlab check below, if 'flavor' is missing
-        # that's okay, we'll provide a default later
-        flavor = config["install"].get("flavor")
-        if flavor is not None and flavor not in ["test", "jenkins", "gitlab"]:
-            raise Exception(
-                "Invalid value '{}' for 'install.flavor'".format(flavor)
-            )
-
-        if flavor == "gitlab":
-            self._validate_section(config, "gitlab", ["runner_secret"])
-
     def _update(self, values):
         self.values["install"].update(values["install"])
 
@@ -561,10 +542,28 @@ class Application:
         for project in self._projects.expand_pattern("all"):
             print(project)
 
+    def _validate_install_config(self, config):
+        if config.values["install"].get("root_password") is None:
+            raise Exception("Missing mandatory install.root_password config.yaml parameter")
+
+        # we only need this for the gitlab check below, if 'flavor' is missing
+        # that's okay, we'll provide a default later
+        flavor = config.values["install"].get("flavor")
+        if flavor is not None and flavor not in ["test", "jenkins", "gitlab"]:
+            raise Exception(
+                "Invalid value '{}' for 'install.flavor'".format(flavor)
+            )
+
+        if flavor == "gitlab":
+            if config.values["gitlab"].get("runner_secret") is None:
+                raise Exception("Missing mandatory gitlab.runner_secret config.yaml parameter")
+
     def _action_install(self, args):
         base = Util.get_base()
         config = self._config
 
+        self._validate_install_config(config)
+
         for host in self._inventory.expand_pattern(args.hosts):
             facts = self._inventory.get_facts(host)
 
-- 
2.26.2

Re: [libvirt-ci PATCH] guests: don't force user to create a config.yaml file

Posted by Andrea Bolognani 2 weeks ago
On Fri, 2020-05-15 at 11:06 +0100, Daniel P. Berrangé wrote:
> Only the "install" command requires a per-user config.yaml with a
> password set. We should not require this for the other commands
> to be run.

This is incorrect: the update and build actions, and in general any
action that requires using an Ansible playbook, needs it as well.

I agree, however, that the hosts, projects and dockerfile actions
should work even if the configuration file is not present.

-- 
Andrea Bolognani / Red Hat / Virtualization