Merge ~hloeung/ubuntu-syncproxy-charm:master into ubuntu-syncproxy-charm:master

[Haw Loeung]

Allow syncing from upstream without user/password

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

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

[Paul Collins (pjdc)]

review: Approve (lgtm)

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

Change successfully merged at revision 057cd698dd3e4fec33e4ef53c151bd24a19cc995

diff --git a/reactive/ubuntu_syncproxy.py b/reactive/ubuntu_syncproxy.py
index 8da73e8..c2b92f5 100644
--- a/reactive/ubuntu_syncproxy.py
+++ b/reactive/ubuntu_syncproxy.py
@@ -97,6 +97,8 @@ def configure_ubuntu_syncproxy(home_path=''):
         'password': config['upstream-rsync-password'],
     }
     for k in sorted(rsync_conf.keys()):
+        if k in ['user', 'password']:
+            continue
         if not rsync_conf[k]:
             status.blocked('Required sync rsync configs missing ({})'.format(k))
             reactive.set_flag('ubuntu-syncproxy.configured')
@@ -136,17 +138,21 @@ CONF_TMPL = """
 TO={target}
 RSYNC_HOST={host}
 RSYNC_USER={user}
-FROM=$RSYNC_USER@$RSYNC_HOST::{module}
+FROM={user_host}::{module}
 RSYNC_PASSWORD={password}
 """
 
 
 def _setup_archive_sync_conf(conf, user, home_path):
+    user_host = '$RSYNC_USER@$RSYNC_HOST'
+    if not conf['user']:
+        user_host = '$RSYNC_HOST'
     content = CONF_TMPL.format(
         header=JUJU_HEADER,
         host=conf['host'],
         module=conf['module'],
         target=conf['target'],
+        user_host=user_host,
         user=conf['user'],
         password=conf['password'],
     )
diff --git a/tests/unit/test_ubuntu_syncproxy.py b/tests/unit/test_ubuntu_syncproxy.py
index 033e457..be44443 100644
--- a/tests/unit/test_ubuntu_syncproxy.py
+++ b/tests/unit/test_ubuntu_syncproxy.py
@@ -237,23 +237,14 @@ class TestCharm(unittest.TestCase):
         status.blocked.reset_mock()
         self.mock_config.return_value['upstream-rsync-module'] = 'ftp'
         ubuntu_syncproxy.configure_ubuntu_syncproxy(self.tmpdir)
-        status.blocked.assert_called_with('Required sync rsync configs missing (password)')
-
-        status.blocked.reset_mock()
-        self.mock_config.return_value['upstream-rsync-password'] = 'my-secure-rsync-password'
-        ubuntu_syncproxy.configure_ubuntu_syncproxy(self.tmpdir)
         status.blocked.assert_called_with('Required sync rsync configs missing (target)')
 
-        status.blocked.reset_mock()
-        self.mock_config.return_value['upstream-rsync-target'] = '/srv/ftp.root/ubuntu'
-        ubuntu_syncproxy.configure_ubuntu_syncproxy(self.tmpdir)
-        status.blocked.assert_called_with('Required sync rsync configs missing (user)')
-
         setup.assert_not_called()
         want = [mock.call('ubuntu-syncproxy.configured')]
         set_flag.assert_has_calls(want, any_order=True)
 
         status.blocked.reset_mock()
+        self.mock_config.return_value['upstream-rsync-target'] = '/srv/ftp.root/ubuntu'
         self.mock_config.return_value['upstream-rsync-user'] = 'syncproxy'
         ubuntu_syncproxy.configure_ubuntu_syncproxy(self.tmpdir)
         status.blocked.assert_not_called()
@@ -262,6 +253,28 @@ class TestCharm(unittest.TestCase):
 
     @mock.patch('charms.reactive.clear_flag')
     @mock.patch('charms.reactive.set_flag')
+    @mock.patch('reactive.ubuntu_syncproxy._setup_archive_sync_conf')
+    @mock.patch('reactive.ubuntu_syncproxy._setup_archive_sync_target')
+    @mock.patch('reactive.ubuntu_syncproxy._setup_rsyncd')
+    def test_configure_ubuntu_rsync_missing_user_or_password(
+        self, setup_rsyncd, setup_target, setup, set_flag, clear_flag
+    ):
+        self.mock_config.return_value['upstream-rsync-password'] = 'my-special-pass'
+        self.mock_config.return_value['upstream-rsync-user'] = ''
+        ubuntu_syncproxy.configure_ubuntu_syncproxy(self.tmpdir)
+        setup.assert_called_once()
+        status.blocked.assert_not_called()
+
+        setup.reset_mock()
+        status.blocked.reset_mock()
+        self.mock_config.return_value['upstream-rsync-password'] = ''
+        self.mock_config.return_value['upstream-rsync-user'] = 'myuser'
+        ubuntu_syncproxy.configure_ubuntu_syncproxy(self.tmpdir)
+        setup.assert_called_once()
+        status.blocked.assert_not_called()
+
+    @mock.patch('charms.reactive.clear_flag')
+    @mock.patch('charms.reactive.set_flag')
     @mock.patch('reactive.ubuntu_syncproxy._setup_rsyncd')
     def test_configure_ubuntu_syncproxy_script(self, setup_rsyncd, set_flag, clear_flag):
         self.mock_config.return_value['sync-script'] = '#!/bin/sh\necho "here test_configure_ubuntu_syncproxy_script"\n'
@@ -396,6 +409,31 @@ RSYNC_PASSWORD=my-secure-rsync-password
             got = f.read()
         self.assertEqual(got, want)
 
+    def test__setup_archive_sync_conf_no_user(self):
+        rsync_conf = {
+            'host': 'ftpmaster.internal',
+            'module': 'ftp',
+            'user': '',
+            'password': 'my-secure-rsync-password',
+            'target': '/srv/ftp.root/ubuntu',
+        }
+        ubuntu_syncproxy._setup_archive_sync_conf(rsync_conf, user=self.user, home_path=self.tmpdir)
+        want = """
+# This file is Juju managed - do not edit by hand #
+
+
+
+TO=/srv/ftp.root/ubuntu
+RSYNC_HOST=ftpmaster.internal
+RSYNC_USER=
+FROM=$RSYNC_HOST::ftp
+RSYNC_PASSWORD=my-secure-rsync-password
+"""
+        conf_path = os.path.join(self.tmpdir, 'archive-sync.conf')
+        with open(conf_path, 'r') as f:
+            got = f.read()
+        self.assertEqual(got, want)
+
     @mock.patch('os.chown')
     def test__setup_archive_sync_target(self, chown):
         current_usr = pwd.getpwuid(os.getuid()).pw_name