Merge ~barryprice/ubuntu-mirror-charm/+git/ubuntu-mirror-charm:master into ubuntu-mirror-charm:master

[Barry Price]

Handle LP:1891566 by bind-mounting any paths outside /srv/ftp.root back into it, so that our chrooted vsftp can serve all the same files as apache2/rsync.

[🤖 Canonical IS Merge Bot (canonical-is-mergebot)]

This merge proposal is being monitored by mergebot. Change the status to Approved to merge.

[Barry Price (barryprice)]

review: Needs Resubmitting

[Paul Collins (pjdc)]

review: Approve (lgtm)

[Haw Loeung (hloeung)]

LGTM

review: Approve

[🤖 Canonical IS Merge Bot (canonical-is-mergebot)]

Change successfully merged at revision 70d7abcd0bae4f2c2d8603576e0d6cfcb045b5e0

diff --git a/hooks/hooks.py b/hooks/hooks.py
index 0b80434..2757b35 100755
--- a/hooks/hooks.py
+++ b/hooks/hooks.py
@@ -4,6 +4,7 @@
 # Author: Chris Stratford <[e-mail]>
 
 import base64
+import datetime
 import json
 import os
 import os.path
@@ -85,6 +86,8 @@ scripts_to_copy = [
     'check-updates.sh',
 ]
 
+bind_mount_comment = "# Bind mount managed by the ubuntu-mirror charm"
+
 
 def juju_header():
     header = ("#-------------------------------------------------#\n"
@@ -144,6 +147,110 @@ def check_output(cmd):
         return None
 
 
+def get_managed_mounts():
+    managed_mounts = []
+    with open("/etc/fstab", "r") as f:
+        for line in f:
+            if line.strip().endswith(bind_mount_comment):
+                managed_mounts.append(line.replace(bind_mount_comment, '').strip())
+    return managed_mounts
+
+
+def add_bind_mount(fstab_line):
+    target = fstab_line.split()[1]
+    line_to_write = "{} {}\n".format(fstab_line, bind_mount_comment)
+    fstab_path = "/etc/fstab"
+
+    suffix = "_{:%Y-%m-%d_%H:%M:%S}".format(datetime.datetime.now())
+    tempfile_suffix = "_temp" + suffix
+    backup_suffix = "_backup" + suffix
+
+    tempfile_path = fstab_path + tempfile_suffix
+    backup_path = fstab_path + backup_suffix
+
+    shutil.copyfile(fstab_path, tempfile_path)
+    shutil.copyfile(fstab_path, backup_path)
+
+    with open(tempfile_path, "a") as f:
+        f.write(line_to_write)
+    os.rename(tempfile_path, fstab_path)
+    if not os.path.isdir(target):
+        mkdir(target)
+    check_call("mount {}".format(target).split())
+
+
+def remove_bind_mount(fstab_line):
+    target = fstab_line.split()[1]
+    removed = False
+    fstab_path = "/etc/fstab"
+
+    suffix = "_{:%Y-%m-%d_%H:%M:%S}".format(datetime.datetime.now())
+    tempfile_suffix = "_temp" + suffix
+    backup_suffix = "_backup" + suffix
+
+    tempfile_path = fstab_path + tempfile_suffix
+    backup_path = fstab_path + backup_suffix
+
+    shutil.copyfile(fstab_path, backup_path)
+
+    with open(fstab_path, "r") as f:
+        lines = f.readlines()
+    with open(tempfile_path, "w") as f:
+        for line in lines:
+            if line.strip().endswith(bind_mount_comment):
+                if line.replace(bind_mount_comment, '').strip() == fstab_line:
+                    removed = True
+                    continue
+            f.write(line)
+    os.rename(tempfile_path, fstab_path)
+    if removed:
+        check_call("umount {}".format(target).split())
+        check_call("rmdir {}".format(target).split())
+
+
+def update_bind_mounts(required_mounts):
+    managed_mounts = get_managed_mounts()
+
+    for required_mount in required_mounts:
+        if required_mount not in managed_mounts:
+            add_bind_mount(required_mount)
+
+    for managed_mount in managed_mounts:
+        if managed_mount not in required_mounts:
+            remove_bind_mount(managed_mount)
+
+
+def configure_bind_mounts(conf, hostname):
+    roles = conf.roles()
+    paths = []
+    required_mounts = []
+    ftp_root = "/srv/ftp.root"  # configure_vsftp() uses this hard-coded path, so we will too
+    if roles.get(hostname):
+        for role in roles.get(hostname):
+            try:
+                mirror = conf.mirror_map(role)
+            except ValueError:
+                log("CHARM: Missing details - skipping {}".format(role))
+                continue
+
+            paths.append(mirror['path'])
+    else:
+        print("DEBUG: configure_bind_mounts found no defined roles for host {}".format(hostname))
+
+    if not paths:
+        print("DEBUG: configure_bind_mounts found no configured paths for host {}".format(hostname))
+
+    for path in paths:
+        if not path.startswith(ftp_root):
+            dir_name = os.path.basename(os.path.normpath(path))
+            target = os.path.join("/srv/ftp.root", dir_name)
+            print("DEBUG: configure_bind_mounts needs to bind-mount {} to {}".format(path, target))
+            fstab_line = "{} {} none defaults,bind 0 0".format(path, target)
+            required_mounts.append(fstab_line)
+
+    update_bind_mounts(required_mounts)
+
+
 def configure_scripts(conf, hostname):
     mkdir(conf.script_dir())
     for script in scripts_to_copy:
@@ -978,6 +1085,7 @@ def config_changed():
     configure_rsync_client(conf, hostname)
     configure_rsync_server(conf, hostname)
     configure_apache(conf, hostname)
+    configure_bind_mounts(conf, hostname)
     configure_vsftp(conf, hostname)
     configure_triggers(conf, hostname)
     configure_nrpe(conf, hostname)
diff --git a/tests/unit/requirements.txt b/tests/unit/requirements.txt
index 449073d..b83c91e 100644
--- a/tests/unit/requirements.txt
+++ b/tests/unit/requirements.txt
@@ -4,4 +4,5 @@ pytest
 pytest-cov
 # charm requirements
 Cheetah
+freezegun
 pyyaml
diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py
index a8a2fb8..9be689a 100644
--- a/tests/unit/test_charm.py
+++ b/tests/unit/test_charm.py
@@ -1,3 +1,4 @@
+import freezegun
 import json
 import os
 import pytest
@@ -5,20 +6,23 @@ import unittest
 
 from base64 import b64encode
 from copy import deepcopy
-from mock import patch
+from mock import mock_open, patch
 
 import Config  # for mockery
 import hooks   # noqa: F401; for mockery
 
 from hooks import (
     SSH_TRIGGER_TEMPLATE,
+    add_bind_mount,
     collect_mountpoints,
     configure_triggers,
     extract_ssh_public_key,
     generate_disk_check,
+    get_managed_mounts,
     make_triggers,
     merge_shared_key_triggers,
     mountpoint_of,
+    remove_bind_mount,
 )
 
 from utils import (
@@ -141,6 +145,68 @@ class TestUbuntuMirrorCharm(unittest.TestCase):
         """Given the tests for mountpoint_of, we can test collect_mountpoints like this."""
         self.assertEqual(collect_mountpoints(['/', '/bin', '/proc/self/fd']), {'/', '/proc'})
 
+    def test_get_managed_mounts_empty(self):
+        """On a fresh install, there will be none."""
+        with patch(
+            "__builtin__.open",
+            mock_open(
+                read_data="LABEL=cloudimg-rootfs  /       ext4    defaults        0 0\n"
+            )
+        ) as m:
+            self.assertEqual(get_managed_mounts(), [])
+        m.assert_called_once_with("/etc/fstab", "r")
+
+    def test_get_managed_mounts_not_empty(self):
+        """On a unit with a managed mount, we should identify it."""
+        with patch(
+            "__builtin__.open",
+            mock_open(
+                read_data="LABEL=cloudimg-rootfs  /       ext4    defaults        0 0\n"
+                          "/srv2/ftp.root/ubuntu-ports /srv/ftp.root/ubuntu-ports none defaults,bind 0 0"
+                          " # Bind mount managed by the ubuntu-mirror charm\n"
+            )
+        ) as m:
+            self.assertEqual(
+                get_managed_mounts(),
+                ["/srv2/ftp.root/ubuntu-ports /srv/ftp.root/ubuntu-ports none defaults,bind 0 0", ]
+            )
+        m.assert_called_once_with("/etc/fstab", "r")
+
+    @patch("hooks.check_call")
+    @patch("hooks.mkdir")
+    @patch("os.rename")
+    @patch("shutil.copyfile")
+    @freezegun.freeze_time("2020-09-09 12:00:01")
+    def test_add_bind_mount(self, _copyfile, _rename, _mkdir, _check_call):
+        with patch("__builtin__.open", mock_open()) as m:
+            add_bind_mount("/srv2/ftp.root/ubuntu-ports /srv/ftp.root/ubuntu-ports none defaults,bind 0 0")
+        m.assert_called_with("/etc/fstab_temp_2020-09-09_12:00:01", "a")
+        _check_call.assert_called_once_with(["mount", "/srv/ftp.root/ubuntu-ports"])
+        _mkdir.assert_called_once_with("/srv/ftp.root/ubuntu-ports")
+        _rename.assert_called_once_with("/etc/fstab_temp_2020-09-09_12:00:01", "/etc/fstab")
+        _copyfile.assert_called_with("/etc/fstab", "/etc/fstab_backup_2020-09-09_12:00:01")
+
+    @patch("hooks.check_call")
+    @patch("os.rename")
+    @patch("shutil.copyfile")
+    @freezegun.freeze_time("2020-09-09 12:00:01")
+    def test_remove_bind_mount(self, _copyfile, _rename, _check_call):
+        with patch(
+            "__builtin__.open",
+            mock_open(
+                read_data="LABEL=cloudimg-rootfs  /       ext4    defaults        0 0\n"
+                          "/srv2/ftp.root/ubuntu-ports /srv/ftp.root/ubuntu-ports none defaults,bind 0 0"
+                          " # Bind mount managed by the ubuntu-mirror charm\n"
+            )
+        ) as m:
+            remove_bind_mount("/srv2/ftp.root/ubuntu-ports /srv/ftp.root/ubuntu-ports none defaults,bind 0 0")
+        m.assert_any_call("/etc/fstab", "r")
+        m.assert_called_with("/etc/fstab_temp_2020-09-09_12:00:01", "w")
+        m().write.assert_called_once_with("LABEL=cloudimg-rootfs  /       ext4    defaults        0 0\n")
+        _copyfile.assert_called_with("/etc/fstab", "/etc/fstab_backup_2020-09-09_12:00:01")
+        _check_call.assert_any_call(["umount", "/srv/ftp.root/ubuntu-ports"])
+        _check_call.assert_any_call(["rmdir", "/srv/ftp.root/ubuntu-ports"])
+
     # Cheetah warns about using the Python version of NameMapper, so
     # we ignore it here.  Matching more closely by the message doesn't
     # seem to work, probably because it begins with a newline and