config, names: Move setting hostname from config to names

Tests:

- Config app description is as expected.
- Config form does not show hostname anymore.
  - Submitting the form with changes works.
- Names app has correct link for configuring Local Domain Name. Clicking it
  takes to page for setting hostname.
- Avahi shows the current .local domain correctly in Names app.
- Change hostname form shows correct value for current hostname.
- Change hostname form sets the value for hostname properly.
  - Page title is correct.
  - Validations works.
  - Pre/post hostname change signals are sent properly
  - Success message as shown expected
  - hostnamectl shows the set domain
- If domain name is not set, downloaded OpenVPN profile shows hostname.
- Unit tests work.
- Functional tests on names/config/avahi apps work.

Signed-off-by: Sunil Mohan Adapa <sunil@medhas.org>
Reviewed-by: Veiko Aasa <veiko17@disroot.org>

plinth/modules/avahi/__init__.py

@@ -7,8 +7,8 @@ from plinth import app as app_module
 from plinth import cfg, menu
 from plinth.daemon import Daemon
 from plinth.modules.backups.components import BackupRestore
-from plinth.modules.config import get_hostname
 from plinth.modules.firewall.components import Firewall
+from plinth.modules.names import get_hostname
 from plinth.modules.names.components import DomainType
 from plinth.package import Packages
 from plinth.privileged import service as service_privileged
@@ -58,7 +58,7 @@ class AvahiApp(app_module.App):
         self.add(packages)
 
         domain_type = DomainType('domain-type-local',
-                                 _('Local Network Domain'), 'config:index',
+                                 _('Local Network Domain'), 'names:hostname',
                                  can_have_certificate=False)
         self.add(domain_type)
 

plinth/modules/config/__init__.py

@@ -20,7 +20,7 @@ from . import privileged
 
 _description = [
     _('Here you can set some general configuration options '
-      'like hostname, domain name, webserver home page etc.')
+      'like domain name, webserver home page etc.')
 ]
 
 ADVANCED_MODE_KEY = 'advanced_mode'
@@ -101,11 +101,6 @@ def get_domainname():
     return '.'.join(fqdn.split('.')[1:])
 
 
-def get_hostname():
-    """Return the hostname."""
-    return socket.gethostname()
-
-
 def home_page_url2scid(url):
     """Return the shortcut ID of the given home page url."""
     # url is None when the freedombox-apache-homepage configuration file does

plinth/modules/config/forms.py

@@ -46,23 +46,6 @@ def get_homepage_choices():
 
 class ConfigurationForm(forms.Form):
     """Main system configuration form"""
-    # See:
-    # https://tools.ietf.org/html/rfc952
-    # https://tools.ietf.org/html/rfc1035#section-2.3.1
-    # https://tools.ietf.org/html/rfc1123#section-2
-    # https://tools.ietf.org/html/rfc2181#section-11
-    hostname = forms.CharField(
-        label=gettext_lazy('Hostname'), help_text=format_lazy(
-            gettext_lazy(
-                'Hostname is the local name by which other devices on the '
-                'local network can reach your {box_name}.  It must start and '
-                'end with an alphabet or a digit and have as interior '
-                'characters only alphabets, digits and hyphens.  Total '
-                'length must be 63 characters or less.'),
-            box_name=gettext_lazy(cfg.box_name)), validators=[
-                validators.RegexValidator(HOSTNAME_REGEX,
-                                          gettext_lazy('Invalid hostname'))
-            ], strip=True)
 
     domainname = forms.CharField(
         label=gettext_lazy('Domain Name'), help_text=format_lazy(

plinth/modules/config/privileged.py

@@ -18,15 +18,6 @@ APACHE_HOMEPAGE_CONFIG = os.path.join(APACHE_CONF_ENABLED_DIR,
 JOURNALD_FILE = pathlib.Path('/etc/systemd/journald.conf.d/50-freedombox.conf')
 
 
-@privileged
-def set_hostname(hostname: str):
-    """Set system hostname using hostnamectl."""
-    subprocess.run(
-        ['hostnamectl', 'set-hostname', '--transient', '--static', hostname],
-        check=True)
-    action_utils.service_restart('avahi-daemon')
-
-
 @privileged
 def set_domainname(domainname: str | None = None):
     """Set system domainname in /etc/hosts."""

plinth/modules/config/tests/test_config.py

@@ -15,34 +15,6 @@ from plinth.modules.config import (_home_page_scid2url, change_home_page,
 from plinth.modules.config.forms import ConfigurationForm
 
 
-def test_hostname_field():
-    """Test that hostname field accepts only valid hostnames."""
-    valid_hostnames = [
-        'a', '0a', 'a0', 'AAA', '00', '0-0', 'example-hostname', 'example',
-        '012345678901234567890123456789012345678901234567890123456789012'
-    ]
-    invalid_hostnames = [
-        '', '-', '-a', 'a-', '.a', 'a.', 'a.a', '?', 'a?a',
-        '0123456789012345678901234567890123456789012345678901234567890123'
-    ]
-
-    for hostname in valid_hostnames:
-        form = ConfigurationForm({
-            'hostname': hostname,
-            'domainname': 'example.com',
-            'logging_mode': 'volatile'
-        })
-        assert form.is_valid()
-
-    for hostname in invalid_hostnames:
-        form = ConfigurationForm({
-            'hostname': hostname,
-            'domainname': 'example.com',
-            'logging_mode': 'volatile'
-        })
-        assert not form.is_valid()
-
-
 def test_domainname_field():
     """Test that domainname field accepts only valid domainnames."""
     valid_domainnames = [

plinth/modules/config/tests/test_functional.py

@@ -19,12 +19,6 @@ def fixture_background(session_browser):
     functional.login(session_browser)
 
 
-def test_change_hostname(session_browser):
-    """Test changing the hostname."""
-    functional.set_hostname(session_browser, 'mybox')
-    assert _get_hostname(session_browser) == 'mybox'
-
-
 def test_change_domain_name(session_browser):
     """Test changing the domain name."""
     functional.set_domain_name(session_browser, 'mydomain.example')
@@ -45,11 +39,6 @@ def test_change_home_page(session_browser):
     assert _check_home_page_redirect(session_browser, 'plinth')
 
 
-def _get_hostname(browser):
-    functional.nav_to_module(browser, 'config')
-    return browser.find_by_id('id_hostname').value
-
-
 def _get_domain_name(browser):
     functional.nav_to_module(browser, 'config')
     return browser.find_by_id('id_domainname').value

plinth/modules/config/views.py

@@ -8,8 +8,7 @@ from django.utils.translation import gettext as _
 
 from plinth import views
 from plinth.modules import config
-from plinth.signals import (domain_added, domain_removed, post_hostname_change,
+from plinth.signals import domain_added, domain_removed
-                            pre_hostname_change)
 
 from . import privileged
 from .forms import ConfigurationForm
@@ -26,7 +25,6 @@ class ConfigAppView(views.AppView):
     def get_initial(self):
         """Return the current status."""
         return {
-            'hostname': config.get_hostname(),
             'domainname': config.get_domainname(),
             'homepage': config.get_home_page(),
             'advanced_mode': config.get_advanced_mode(),
@@ -40,17 +38,6 @@ class ConfigAppView(views.AppView):
 
         is_changed = False
 
-        if old_status['hostname'] != new_status['hostname']:
-            try:
-                set_hostname(new_status['hostname'])
-            except Exception as exception:
-                messages.error(
-                    self.request,
-                    _('Error setting hostname: {exception}').format(
-                        exception=exception))
-            else:
-                messages.success(self.request, _('Hostname set'))
-
         if old_status['domainname'] != new_status['domainname']:
             try:
                 set_domainname(new_status['domainname'],
@@ -100,29 +87,6 @@ class ConfigAppView(views.AppView):
         return super().form_valid(form)
 
 
-def set_hostname(hostname):
-    """Set machine hostname and send signals before and after."""
-    old_hostname = config.get_hostname()
-    domainname = config.get_domainname()
-
-    # Hostname should be ASCII. If it's unicode but passed our
-    # valid_hostname check, convert
-    hostname = str(hostname)
-
-    pre_hostname_change.send_robust(sender='config', old_hostname=old_hostname,
-                                    new_hostname=hostname)
-
-    LOGGER.info('Changing hostname to - %s', hostname)
-    privileged.set_hostname(hostname)
-
-    LOGGER.info('Setting domain name after hostname change - %s', domainname)
-    privileged.set_domainname(domainname)
-
-    post_hostname_change.send_robust(sender='config',
-                                     old_hostname=old_hostname,
-                                     new_hostname=hostname)
-
-
 def set_domainname(domainname, old_domainname):
     """Set machine domain name to domainname."""
     old_domainname = config.get_domainname()

plinth/modules/names/__init__.py

@@ -4,6 +4,7 @@ FreedomBox app to configure name services.
 """
 
 import logging
+import socket
 import subprocess
 
 from django.utils.translation import gettext_lazy as _
@@ -17,7 +18,8 @@ from plinth.diagnostic_check import (DiagnosticCheck,
 from plinth.modules.backups.components import BackupRestore
 from plinth.package import Packages
 from plinth.privileged import service as service_privileged
-from plinth.signals import domain_added, domain_removed
+from plinth.signals import (domain_added, domain_removed, post_hostname_change,
+                            pre_hostname_change)
 from plinth.utils import format_lazy
 
 from . import components, manifest, privileged
@@ -158,6 +160,36 @@ def on_domain_removed(sender, domain_type, name='', **kwargs):
 ######################################################
 
 
+def get_hostname():
+    """Return the hostname."""
+    return socket.gethostname()
+
+
+def set_hostname(hostname):
+    """Set machine hostname and send signals before and after."""
+    from plinth.modules import config
+    from plinth.modules.config import privileged as config_privileged
+
+    old_hostname = get_hostname()
+    domainname = config.get_domainname()
+
+    # Hostname should be ASCII. If it's unicode but passed our
+    # valid_hostname check, convert
+    hostname = str(hostname)
+
+    pre_hostname_change.send_robust(sender='names', old_hostname=old_hostname,
+                                    new_hostname=hostname)
+
+    logger.info('Changing hostname to - %s', hostname)
+    privileged.set_hostname(hostname)
+
+    logger.info('Setting domain name after hostname change - %s', domainname)
+    config_privileged.set_domainname(domainname)
+
+    post_hostname_change.send_robust(sender='names', old_hostname=old_hostname,
+                                     new_hostname=hostname)
+
+
 def get_available_tls_domains():
     """Return an iterator with all domains able to have a certificate."""
     return (domain.name for domain in components.DomainName.list()

plinth/modules/names/forms.py

@@ -2,10 +2,14 @@
 """Forms for the names app."""
 
 from django import forms
+from django.core import validators
 from django.utils.translation import gettext_lazy as _
 
+from plinth import cfg
 from plinth.utils import format_lazy
 
+HOSTNAME_REGEX = r'^[a-zA-Z0-9]([-a-zA-Z0-9]{,61}[a-zA-Z0-9])?$'
+
 
 class NamesConfigurationForm(forms.Form):
     """Form to configure names app."""
@@ -64,3 +68,22 @@ class NamesConfigurationForm(forms.Form):
                  'No. <p class="help-block">Do not verify domain name '
                  'resolutions.</p>', allow_markup=True)),
         ], initial='no')
+
+
+class HostnameForm(forms.Form):
+    """Form to update system's hostname."""
+    # See:
+    # https://tools.ietf.org/html/rfc952
+    # https://tools.ietf.org/html/rfc1035#section-2.3.1
+    # https://tools.ietf.org/html/rfc1123#section-2
+    # https://tools.ietf.org/html/rfc2181#section-11
+    hostname = forms.CharField(
+        label=_('Hostname'), help_text=format_lazy(
+            _('Hostname is the local name by which other devices on the local '
+              'network can reach your {box_name}.  It must start and end with '
+              'an alphabet or a digit and have as interior characters only '
+              'alphabets, digits and hyphens.  Total length must be 63 '
+              'characters or less.'), box_name=_(cfg.box_name)), validators=[
+                  validators.RegexValidator(HOSTNAME_REGEX,
+                                            _('Invalid hostname'))
+              ], strip=True)

plinth/modules/names/privileged.py

@@ -2,6 +2,7 @@
 """Configure Names App."""
 
 import pathlib
+import subprocess
 
 import augeas
 
@@ -16,6 +17,15 @@ source_fallback_conf = pathlib.Path(
     '/etc/systemd/resolved.conf.d/freedombox-fallback.conf')
 
 
+@privileged
+def set_hostname(hostname: str):
+    """Set system hostname using hostnamectl."""
+    subprocess.run(
+        ['hostnamectl', 'set-hostname', '--transient', '--static', hostname],
+        check=True)
+    action_utils.service_restart('avahi-daemon')
+
+
 @privileged
 def set_resolved_configuration(dns_fallback: bool | None = None,
                                dns_over_tls: str | None = None,

plinth/modules/names/tests/test_forms.py

@@ -0,0 +1,24 @@
+# SPDX-License-Identifier: AGPL-3.0-or-later
+"""Tests for forms in names app."""
+
+from ..forms import HostnameForm
+
+
+def test_hostname_field():
+    """Test that hostname field accepts only valid hostnames."""
+    valid_hostnames = [
+        'a', '0a', 'a0', 'AAA', '00', '0-0', 'example-hostname', 'example',
+        '012345678901234567890123456789012345678901234567890123456789012'
+    ]
+    invalid_hostnames = [
+        '', '-', '-a', 'a-', '.a', 'a.', 'a.a', '?', 'a?a',
+        '0123456789012345678901234567890123456789012345678901234567890123'
+    ]
+
+    for hostname in valid_hostnames:
+        form = HostnameForm({'hostname': hostname})
+        assert form.is_valid()
+
+    for hostname in invalid_hostnames:
+        form = HostnameForm({'hostname': hostname})
+        assert not form.is_valid()

plinth/modules/names/tests/test_functional.py

@@ -0,0 +1,28 @@
+# SPDX-License-Identifier: AGPL-3.0-or-later
+"""Functional, browser based tests for names app."""
+
+import pytest
+
+from plinth.tests import functional
+
+pytestmark = [
+    pytest.mark.system, pytest.mark.essential, pytest.mark.domain,
+    pytest.mark.names
+]
+
+
+@pytest.fixture(scope='module', autouse=True)
+def fixture_background(session_browser):
+    """Login."""
+    functional.login(session_browser)
+
+
+def test_change_hostname(session_browser):
+    """Test changing the hostname."""
+    functional.set_hostname(session_browser, 'mybox')
+    assert _get_hostname(session_browser) == 'mybox'
+
+
+def _get_hostname(browser):
+    functional.visit(browser, '/plinth/sys/names/hostname/')
+    return browser.find_by_id('id_hostname-hostname').value

plinth/modules/names/urls.py

@@ -9,4 +9,6 @@ from . import views
 
 urlpatterns = [
     re_path(r'^sys/names/$', views.NamesAppView.as_view(), name='index'),
+    re_path(r'^sys/names/hostname/$', views.HostnameView.as_view(),
+            name='hostname'),
 ]

plinth/modules/names/views.py

@@ -4,12 +4,15 @@ FreedomBox app for name services.
 """
 
 from django.contrib import messages
+from django.urls import reverse_lazy
 from django.utils.translation import gettext_lazy as _
+from django.views.generic.edit import FormView
 
+from plinth.modules import names
 from plinth.views import AppView
 
 from . import components, privileged, resolved
-from .forms import NamesConfigurationForm
+from .forms import HostnameForm, NamesConfigurationForm
 
 
 class NamesAppView(AppView):
@@ -56,6 +59,40 @@ class NamesAppView(AppView):
         return super().form_valid(form)
 
 
+class HostnameView(FormView):
+    """View to update system's hostname."""
+    template_name = 'form.html'
+    form_class = HostnameForm
+    prefix = 'hostname'
+    success_url = reverse_lazy('names:index')
+
+    def get_context_data(self, **kwargs):
+        """Return additional context for rendering the template."""
+        context = super().get_context_data(**kwargs)
+        context['title'] = _('Set Hostname')
+        return context
+
+    def get_initial(self):
+        """Return the values to fill in the form."""
+        initial = super().get_initial()
+        initial['hostname'] = names.get_hostname()
+        return initial
+
+    def form_valid(self, form):
+        """Apply the form changes."""
+        if form.initial['hostname'] != form.cleaned_data['hostname']:
+            try:
+                names.set_hostname(form.cleaned_data['hostname'])
+                messages.success(self.request, _('Configuration updated'))
+            except Exception as exception:
+                messages.error(
+                    self.request,
+                    _('Error setting hostname: {exception}').format(
+                        exception=exception))
+
+        return super().form_valid(form)
+
+
 def get_status():
     """Get configured services per name."""
     domains = components.DomainName.list()

plinth/modules/openvpn/views.py

@@ -5,7 +5,7 @@ import logging
 
 from django.http import HttpResponse
 
-from plinth.modules import config
+from plinth.modules import config, names
 from plinth.views import AppView
 
 from . import privileged
@@ -26,7 +26,7 @@ def profile(request):
     domainname = config.get_domainname()
 
     if not config.get_domainname():
-        domainname = config.get_hostname()
+        domainname = names.get_hostname()
 
     profile_string = privileged.get_profile(username, domainname)
     response = HttpResponse(profile_string,

plinth/tests/functional/__init__.py

@@ -512,15 +512,18 @@ def running_inside_container():
     return result.stdout.decode('utf-8').strip().lower() != 'none'
 
 
+#############################
+# System -> Names utilities #
+#############################
+def set_hostname(browser, hostname):
+    visit(browser, '/plinth/sys/names/hostname/')
+    browser.find_by_id('id_hostname-hostname').fill(hostname)
+    submit(browser, form_class='form-hostname')
+
+
 ##############################
 # System -> Config utilities #
 ##############################
-def set_hostname(browser, hostname):
-    nav_to_module(browser, 'config')
-    browser.find_by_id('id_hostname').fill(hostname)
-    submit(browser, form_class='form-configuration')
-
-
 def set_advanced_mode(browser, mode):
     nav_to_module(browser, 'config')
     advanced_mode = browser.find_by_id('id_advanced_mode')