cockpit: Prevent restart on freedombox startup

- Add a domain only if it is not already present.

- Remove a domain only if it is already present.

- Refactor utility methods in separate module for reuse.

Signed-off-by: Sunil Mohan Adapa <sunil@medhas.org>
Reviewed-by: James Valleroy <jvalleroy@mailbox.org>

actions/cockpit

@@ -21,11 +21,8 @@ Configuration helper for Cockpit.
 
 import argparse
 
-import augeas
-
 from plinth import action_utils
-
+from plinth.modules.cockpit import utils
-CONFIG_FILE = '/etc/cockpit/cockpit.conf'
 
 
 def parse_arguments():
@@ -53,49 +50,40 @@ def parse_arguments():
 
 def subcommand_setup(arguments):
     """Setup Cockpit configuration."""
-    aug = load_augeas()
+    aug = utils.load_augeas()
     origins = [
-        _get_origin_from_domain(domain) for domain in arguments.domain_names
+        utils.get_origin_from_domain(domain)
+        for domain in arguments.domain_names
     ]
     origins += ['https://localhost', 'https://localhost:4430']
     _set_origin_domains(aug, origins)
-    aug.set('/files' + CONFIG_FILE + '/WebService/UrlRoot', '/_cockpit/')
+    aug.set('/files' + utils.CONFIG_FILE + '/WebService/UrlRoot', '/_cockpit/')
     aug.save()
 
     action_utils.service_restart('cockpit.socket')
 
 
-def _get_origin_domains(aug):
-    """Return the list of allowed origin domains."""
-    origins = aug.get('/files' + CONFIG_FILE + '/WebService/Origins')
-    return set(origins.split()) if origins else set()
-
-
 def _set_origin_domains(aug, origins):
     """Set the list of allowed origin domains."""
-    aug.set('/files' + CONFIG_FILE + '/WebService/Origins', ' '.join(origins))
+    aug.set('/files' + utils.CONFIG_FILE + '/WebService/Origins',
-
+            ' '.join(origins))
-
-def _get_origin_from_domain(domain):
-    """Return the origin that should be allowed for a domain."""
-    return 'https://{domain}'.format(domain=domain)
 
 
 def subcommand_add_domain(arguments):
     """Allow a new domain to be origin for Cockpit's WebSocket."""
-    aug = load_augeas()
+    aug = utils.load_augeas()
-    origins = _get_origin_domains(aug)
+    origins = utils.get_origin_domains(aug)
-    origins.add(_get_origin_from_domain(arguments.domain_name))
+    origins.add(utils.get_origin_from_domain(arguments.domain_name))
     _set_origin_domains(aug, origins)
     aug.save()
 
 
 def subcommand_remove_domain(arguments):
     """Disallow a domain from being origin for Cockpit's WebSocket."""
-    aug = load_augeas()
+    aug = utils.load_augeas()
-    origins = _get_origin_domains(aug)
+    origins = utils.get_origin_domains(aug)
     try:
-        origins.remove(_get_origin_from_domain(arguments.domain_name))
+        origins.remove(utils.get_origin_from_domain(arguments.domain_name))
     except KeyError:
         pass
     else:
@@ -103,16 +91,6 @@ def subcommand_remove_domain(arguments):
         aug.save()
 
 
-def load_augeas():
-    """Initialize Augeas."""
-    aug = augeas.Augeas(
-        flags=augeas.Augeas.NO_LOAD + augeas.Augeas.NO_MODL_AUTOLOAD)
-    aug.set('/augeas/load/inifile/lens', 'Puppet.lns')
-    aug.set('/augeas/load/inifile/incl[last() + 1]', CONFIG_FILE)
-    aug.load()
-    return aug
-
-
 def main():
     """Parse arguments and perform all duties."""
     arguments = parse_arguments()

plinth/modules/cockpit/__init__.py

@@ -31,6 +31,7 @@ from plinth.modules.firewall.components import Firewall
 from plinth.signals import domain_added, domain_removed
 from plinth.utils import format_lazy
 
+from . import utils
 from .manifest import backup, clients  # noqa, pylint: disable=unused-import
 
 version = 1
@@ -133,13 +134,17 @@ def on_domain_added(sender, domain_type, name, description='', services=None,
     """Handle addition of a new domain."""
     setup_helper = globals()['setup_helper']
     if setup_helper.get_state() != 'needs-setup':
-        actions.superuser_run('cockpit', ['add-domain', name])
+        if name not in utils.get_domains():
-        actions.superuser_run('service', ['try-restart', managed_services[0]])
+            actions.superuser_run('cockpit', ['add-domain', name])
+            actions.superuser_run('service',
+                                  ['try-restart', managed_services[0]])
 
 
 def on_domain_removed(sender, domain_type, name, **kwargs):
     """Handle removal of a domain."""
     setup_helper = globals()['setup_helper']
     if setup_helper.get_state() != 'needs-setup':
-        actions.superuser_run('cockpit', ['remove-domain', name])
+        if name in utils.get_domains():
-        actions.superuser_run('service', ['try-restart', managed_services[0]])
+            actions.superuser_run('cockpit', ['remove-domain', name])
+            actions.superuser_run('service',
+                                  ['try-restart', managed_services[0]])

plinth/modules/cockpit/utils.py

@@ -0,0 +1,58 @@
+#
+# This file is part of FreedomBox.
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU Affero General Public License as
+# published by the Free Software Foundation, either version 3 of the
+# License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU Affero General Public License for more details.
+#
+# You should have received a copy of the GNU Affero General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+#
+"""
+Minor utility methods for Cockpit.
+"""
+
+import urllib.parse
+
+import augeas
+
+CONFIG_FILE = '/etc/cockpit/cockpit.conf'
+
+
+def load_augeas():
+    """Initialize Augeas."""
+    aug = augeas.Augeas(
+        flags=augeas.Augeas.NO_LOAD + augeas.Augeas.NO_MODL_AUTOLOAD)
+    aug.set('/augeas/load/inifile/lens', 'Puppet.lns')
+    aug.set('/augeas/load/inifile/incl[last() + 1]', CONFIG_FILE)
+    aug.load()
+    return aug
+
+
+def get_origin_domains(aug):
+    """Return the list of allowed origin domains."""
+    origins = aug.get('/files' + CONFIG_FILE + '/WebService/Origins')
+    return set(origins.split()) if origins else set()
+
+
+def get_origin_from_domain(domain):
+    """Return the origin that should be allowed for a domain."""
+    return 'https://{domain}'.format(domain=domain)
+
+
+def _get_domain_from_origin(origin):
+    """Return the domain from an origin URL."""
+    return urllib.parse.urlparse(origin).netloc
+
+
+def get_domains():
+    """Return the domain name in origin URL."""
+    aug = load_augeas()
+    origins = get_origin_domains(aug)
+    return [_get_domain_from_origin(origin) for origin in origins]