Merge lp:~kissiel/checkbox/fix-1556337-conf-files-ignored into lp:checkbox

[Maciej Kisielewski]

This MR fixes a problem where config files where ignored.

Now the cascade is back. ~/.config contents override /etc/xdg/ contents and launcher contents override both.

Note that config files are expected to use the same syntax as launchers.

bb124e0 checkbox-ng:launchers: fix handling of alternative strategies
44e4432 checkbox-ng:launchers: support config files in /etc/xdg and ~/.config/
c607367 checkbox-ng:launchers: handle invalid secure_ids gracefully
1de2867 checkbox-ng:launchers: make secure_id overridable by launchers/config
88af98b checkbox-ng:docs: add info about supplying secure_id for stock reports
7f99d7f checkbox-ng:launchers: handle config_filename var

[lp:~kissiel/checkbox/fix-1556337-conf-files-ignored]

4277. By Maciej Kisielewski on 2016-03-15

checkbox-ng:launchers: make secure_id overridable by launchers/config

This lets users use secure_id for stock reports, so only

[transport:c3]
secure_id = my_magic_sid

is needed.

Signed-off-by: Maciej Kisielewski <email address hidden>

4278. By Maciej Kisielewski on 2016-03-15

checkbox-ng:docs: add info about supplying secure_id for stock reports

Signed-off-by: Maciej Kisielewski <email address hidden>

4279. By Maciej Kisielewski on 2016-03-15

checkbox-ng:launchers: handle config_filename var

Signed-off-by: Maciej Kisielewski <email address hidden>

[Sylvain Pineau (sylvain-pineau)]

Thanks a lot for the fix, tested ok.

review: Approve

=== modified file 'checkbox-ng/checkbox_ng/launcher.py'
--- checkbox-ng/checkbox_ng/launcher.py	2016-03-08 19:51:40 +0000
+++ checkbox-ng/checkbox_ng/launcher.py	2016-03-15 13:15:32 +0000
@@ -51,6 +51,7 @@
 
     config_filename = config.Variable(
         section="config",
+        default="canonical-certification.conf",
         help_text=_("Name of custom configuration file"))
 
     def get_concrete_launcher(self):

=== modified file 'checkbox-ng/docs/launcher-tutorial.rst'
--- checkbox-ng/docs/launcher-tutorial.rst	2016-03-08 19:19:16 +0000
+++ checkbox-ng/docs/launcher-tutorial.rst	2016-03-15 13:15:32 +0000
@@ -69,6 +69,10 @@
 This field is a list; use commas or spaces to seperate stock reports. The
 default value: ``text, certification, submission_files``.
 
+When using ``certification`` stock report, ``secure_id`` might be overriden by
+the launcher. To do this define ``secure_id`` in a ``transport:c3`` section
+(this is the transport that's used by the ``certification`` stock reports).
+
 Launcher section example:
 
 ::
@@ -78,6 +82,13 @@
     launcher_version = 1
     stock_reports = text
 
+Launcher using all defaults with overriden secure_id:
+
+::
+
+    [transport:c3]
+    secure_id = 001122334455667788
+
 Providers section
 =================
 

=== modified file 'checkbox-ng/launchers/checkbox-cli'
--- checkbox-ng/launchers/checkbox-cli	2016-03-14 22:39:29 +0000
+++ checkbox-ng/launchers/checkbox-cli	2016-03-15 13:15:32 +0000
@@ -39,6 +39,7 @@
 from guacamole.ingredients import cmdtree
 from guacamole.recipes.cmd import CommandRecipe
 
+from checkbox_ng.certification import InvalidSecureIDError
 from checkbox_ng.launcher import DefaultLauncherDefinition
 from checkbox_ng.launcher import LauncherDefinition
 from plainbox.abc import IJobResult
@@ -98,23 +99,29 @@
     def late_init(self, context):
         if not context.args.launcher:
             # launcher not supplied from cli - using the default one
-            context.cmd_toplevel.launcher = DefaultLauncherDefinition()
-            return
-        try:
-            with open(context.args.launcher,
-                      'rt', encoding='UTF-8') as stream:
-                first_line = stream.readline()
-                if not first_line.startswith("#!"):
-                    stream.seek(0)
-                text = stream.read()
-        except IOError as exc:
-            _logger.error(_("Unable to load launcher definition: %s"), exc)
-            raise SystemExit(1)
-
-        generic_launcher = LauncherDefinition()
-        generic_launcher.read_string(text)
-        launcher = generic_launcher.get_concrete_launcher()
-        launcher.read_string(text)
+            launcher = DefaultLauncherDefinition()
+            config_filename = launcher.config_filename
+        else:
+            try:
+                with open(context.args.launcher,
+                          'rt', encoding='UTF-8') as stream:
+                    first_line = stream.readline()
+                    if not first_line.startswith("#!"):
+                        stream.seek(0)
+                    text = stream.read()
+            except IOError as exc:
+                _logger.error(_("Unable to load launcher definition: %s"), exc)
+                raise SystemExit(1)
+            generic_launcher = LauncherDefinition()
+            generic_launcher.read_string(text)
+            config_filename = generic_launcher.config_filename
+            launcher = generic_launcher.get_concrete_launcher()
+        configs = [
+            '/etc/xdg/{}'.format(config_filename),
+            os.path.expanduser('~/.config/{}'.format(config_filename))]
+        if context.args.launcher:
+            configs.append(context.args.launcher)
+        launcher.read(configs)
         if launcher.problem_list:
             _logger.error(_("Unable to start launcher because of errors:"))
             for problem in launcher.problem_list:
@@ -227,8 +234,8 @@
                     self.launcher.restart_strategy)
                 kwargs = copy.deepcopy(self.launcher.restart)
                 # [restart] section has the kwargs for the strategy initializer
-                # and the 'alternative_strategy' which is not one, let's pop it
-                kwargs.pop('alternative_strategy')
+                # and the 'strategy' which is not one, let's pop it
+                kwargs.pop('strategy')
                 strategy = cls(**kwargs)
                 ctx.sa.use_alternate_restart_strategy(strategy)
 
@@ -644,7 +651,9 @@
             self.launcher.exporters['hexr'] = {
                 'unit': '2013.com.canonical.plainbox::hexr'}
             self.launcher.transports['c3'] = {
-                'type': 'certification'}
+                'type': 'certification',
+                'secure_id': self.launcher.transports.get('c3', {}).get(
+                    'secure_id', None)}
             self.launcher.reports['upload to certification'] = {
                 'transport': 'c3', 'exporter': 'hexr'}
         elif report == 'certification-staging':
@@ -706,11 +715,10 @@
             else:
                 url = ('https://certification.canonical.com/'
                        'submissions/submit/')
-            if 'secure_id' in self.launcher.transports[transport]:
-                secure_id = self.launcher.transports[transport]['secure_id']
-            else:
+            secure_id = self.launcher.transports[transport].get(
+                'secure_id', None)
+            if not secure_id:
                 secure_id = input(self.C.BLUE(_('Enter secure-id:')))
-
             options = "secure_id={}".format(secure_id)
             self.transports[transport] = cls(url, options)
 
@@ -734,9 +742,9 @@
             exporter_id = self.launcher.exporters[params['exporter']]['unit']
             done_sending = False
             while not done_sending:
-                self._create_transport(params['transport'])
-                transport = self.transports[params['transport']]
                 try:
+                    self._create_transport(params['transport'])
+                    transport = self.transports[params['transport']]
                     result = self.ctx.sa.export_to_transport(
                         exporter_id, transport)
                     if result and 'url' in result:
@@ -745,21 +753,31 @@
                     _logger.warning(
                         _("Problem occured when submitting %s report: %s"),
                         name, exc)
-                    if self.is_interactive:
-                        message = _("Do you want to retry?")
-                        cmd = self._pick_action_cmd([
-                            Action('y', _("yes"), 'y'),
-                            Action('n', _("no"), 'n')
-                        ], message)
-                        if cmd == 'y':
-                            # let's remove current transport, so in next
-                            # iteration it will be "rebuilt", so if some parts
-                            # were user-provided, checkbox will ask for them
-                            # again
-                            self.transports.pop(params['transport'])
-                            continue
+                    if self._retry_dialog():
+                        # let's remove current transport, so in next
+                        # iteration it will be "rebuilt", so if some parts
+                        # were user-provided, checkbox will ask for them
+                        # again
+                        self.transports.pop(params['transport'])
+                        continue
+                except InvalidSecureIDError:
+                    _logger.warning(_("Invalid secure_id"))
+                    if self._retry_dialog():
+                        self.launcher.transports['c3'].pop('secure_id')
+                        continue
                 done_sending = True
 
+    def _retry_dialog(self):
+        if self.is_interactive:
+            message = _("Do you want to retry?")
+            cmd = self._pick_action_cmd([
+                Action('y', _("yes"), 'y'),
+                Action('n', _("no"), 'n')
+            ], message)
+            if cmd == 'y':
+                return True
+        return False
+
     def register_arguments(self, parser):
         parser.add_argument(
             'launcher', metavar=_('LAUNCHER'), nargs='?',