From 011c2059a55b10a56ce78e2d5cb17bd3d128b47a Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Sun, 16 Jan 2022 03:34:53 +0100 Subject: [PATCH] appresources: fixes /o\ --- src/tests/test_app_resources.py | 88 +++++++++++++++++++-------------- src/utils/resources.py | 43 ++++++++-------- 2 files changed, 73 insertions(+), 58 deletions(-) diff --git a/src/tests/test_app_resources.py b/src/tests/test_app_resources.py index 69e93a5ed..638f6b119 100644 --- a/src/tests/test_app_resources.py +++ b/src/tests/test_app_resources.py @@ -37,6 +37,11 @@ def setup_function(function): clean() + os.system("mkdir /etc/yunohost/apps/testapp") + os.system("echo 'id: testapp' > /etc/yunohost/apps/testapp/settings.yml") + os.system("echo 'packaging_format = 2' > /etc/yunohost/apps/testapp/manifest.toml") + os.system("echo 'id = \"testapp\"' >> /etc/yunohost/apps/testapp/manifest.toml") + def teardown_function(function): @@ -46,7 +51,11 @@ def teardown_function(function): def clean(): os.system(f"rm -f {dummyfile}") - os.system("apt remove lolcat sl nyancat yarn") + os.system("rm -rf /etc/yunohost/apps/testapp") + os.system("rm -rf /var/www/testapp") + os.system("rm -rf /home/yunohost.app/testapp") + os.system("apt remove lolcat sl nyancat yarn >/dev/null 2>/dev/null") + os.system("userdel testapp 2>/dev/null") def test_provision_dummy(): @@ -125,28 +134,28 @@ def test_resource_system_user(): conf = {} - assert os.system("getent passwd testapp &>/dev/null") != 0 + assert os.system("getent passwd testapp 2>/dev/null") != 0 r(conf, "testapp").provision_or_update() - assert os.system("getent passwd testapp &>/dev/null") == 0 - assert os.system("getent group testapp | grep -q sftp.app") != 0 + assert os.system("getent passwd testapp >/dev/null") == 0 + assert os.system("groups testapp | grep -q 'sftp.app'") != 0 conf["allow_sftp"] = True r(conf, "testapp").provision_or_update() - assert os.system("getent passwd testapp &>/dev/null") == 0 - assert os.system("getent group testapp | grep -q sftp.app") == 0 + assert os.system("getent passwd testapp >/dev/null") == 0 + assert os.system("groups testapp | grep -q 'sftp.app'") == 0 r(conf, "testapp").deprovision() - assert os.system("getent passwd testapp &>/dev/null") != 0 + assert os.system("getent passwd testapp 2>/dev/null") != 0 def test_resource_install_dir(): r = AppResourceClassesByType["install_dir"] - conf = {} + conf = {"owner": "nobody:rx", "group": "nogroup:rx"} # FIXME: should also check settings ? # FIXME: should also check automigrate from final_path @@ -159,10 +168,10 @@ def test_resource_install_dir(): assert os.path.exists("/var/www/testapp") unixperms = check_output("ls -ld /var/www/testapp").split() assert unixperms[0] == "dr-xr-x---" - assert unixperms[2] == "testapp" - assert unixperms[3] == "testapp" + assert unixperms[2] == "nobody" + assert unixperms[3] == "nogroup" - conf["group"] = "__APP__:rwx" + conf["owner"] = "nobody:rwx" conf["group"] = "www-data:x" r(conf, "testapp").provision_or_update() @@ -170,7 +179,7 @@ def test_resource_install_dir(): assert os.path.exists("/var/www/testapp") unixperms = check_output("ls -ld /var/www/testapp").split() assert unixperms[0] == "drwx--x---" - assert unixperms[2] == "testapp" + assert unixperms[2] == "nobody" assert unixperms[3] == "www-data" r(conf, "testapp").deprovision() @@ -181,7 +190,7 @@ def test_resource_install_dir(): def test_resource_data_dir(): r = AppResourceClassesByType["data_dir"] - conf = {} + conf = {"owner": "nobody:rx", "group": "nogroup:rx"} assert not os.path.exists("/home/yunohost.app/testapp") @@ -190,10 +199,10 @@ def test_resource_data_dir(): assert os.path.exists("/home/yunohost.app/testapp") unixperms = check_output("ls -ld /home/yunohost.app/testapp").split() assert unixperms[0] == "dr-xr-x---" - assert unixperms[2] == "testapp" - assert unixperms[3] == "testapp" + assert unixperms[2] == "nobody" + assert unixperms[3] == "nogroup" - conf["group"] = "__APP__:rwx" + conf["owner"] = "nobody:rwx" conf["group"] = "www-data:x" r(conf, "testapp").provision_or_update() @@ -201,12 +210,13 @@ def test_resource_data_dir(): assert os.path.exists("/home/yunohost.app/testapp") unixperms = check_output("ls -ld /home/yunohost.app/testapp").split() assert unixperms[0] == "drwx--x---" - assert unixperms[2] == "testapp" + assert unixperms[2] == "nobody" assert unixperms[3] == "www-data" r(conf, "testapp").deprovision() - assert not os.path.exists("/home/yunohost.app/testapp") + # FIXME : implement and check purge option + #assert not os.path.exists("/home/yunohost.app/testapp") def test_resource_port(): @@ -264,40 +274,42 @@ def test_resource_apt(): } } - assert os.system("dpkg --list | grep -q 'ii *nyancat'") != 0 - assert os.system("dpkg --list | grep -q 'ii *sl'") != 0 - assert os.system("dpkg --list | grep -q 'ii *yarn'") != 0 - assert os.system("dpkg --list | grep -q 'ii *lolcat'") != 0 - assert os.system("dpkg --list | grep -q 'ii *testapp-ynh-deps'") != 0 + assert os.system("dpkg --list | grep -q 'ii *nyancat '") != 0 + assert os.system("dpkg --list | grep -q 'ii *sl '") != 0 + assert os.system("dpkg --list | grep -q 'ii *yarn '") != 0 + assert os.system("dpkg --list | grep -q 'ii *lolcat '") != 0 + assert os.system("dpkg --list | grep -q 'ii *testapp-ynh-deps '") != 0 r(conf, "testapp").provision_or_update() - assert os.system("dpkg --list | grep -q 'ii *nyancat'") == 0 - assert os.system("dpkg --list | grep -q 'ii *sl'") == 0 - assert os.system("dpkg --list | grep -q 'ii *yarn'") == 0 - assert os.system("dpkg --list | grep -q 'ii *lolcat'") != 0 # Lolcat shouldnt be installed yet - assert os.system("dpkg --list | grep -q 'ii *testapp-ynh-deps'") == 0 + assert os.system("dpkg --list | grep -q 'ii *nyancat '") == 0 + assert os.system("dpkg --list | grep -q 'ii *sl '") == 0 + assert os.system("dpkg --list | grep -q 'ii *yarn '") == 0 + assert os.system("dpkg --list | grep -q 'ii *lolcat '") != 0 # Lolcat shouldnt be installed yet + assert os.system("dpkg --list | grep -q 'ii *testapp-ynh-deps '") == 0 conf["packages"] += ", lolcat" r(conf, "testapp").provision_or_update() - assert os.system("dpkg --list | grep -q 'ii *nyancat'") != 0 - assert os.system("dpkg --list | grep -q 'ii *sl'") != 0 - assert os.system("dpkg --list | grep -q 'ii *yarn'") != 0 - assert os.system("dpkg --list | grep -q 'ii *lolcat'") == 0 - assert os.system("dpkg --list | grep -q 'ii *testapp-ynh-deps'") != 0 + assert os.system("dpkg --list | grep -q 'ii *nyancat '") == 0 + assert os.system("dpkg --list | grep -q 'ii *sl '") == 0 + assert os.system("dpkg --list | grep -q 'ii *yarn '") == 0 + assert os.system("dpkg --list | grep -q 'ii *lolcat '") == 0 + assert os.system("dpkg --list | grep -q 'ii *testapp-ynh-deps '") == 0 r(conf, "testapp").deprovision() - assert os.system("dpkg --list | grep -q 'ii *nyancat'") != 0 - assert os.system("dpkg --list | grep -q 'ii *sl'") != 0 - assert os.system("dpkg --list | grep -q 'ii *yarn'") != 0 - assert os.system("dpkg --list | grep -q 'ii *lolcat'") != 0 - assert os.system("dpkg --list | grep -q 'ii *testapp-ynh-deps'") != 0 + assert os.system("dpkg --list | grep -q 'ii *nyancat '") != 0 + assert os.system("dpkg --list | grep -q 'ii *sl '") != 0 + assert os.system("dpkg --list | grep -q 'ii *yarn '") != 0 + assert os.system("dpkg --list | grep -q 'ii *lolcat '") != 0 + assert os.system("dpkg --list | grep -q 'ii *testapp-ynh-deps '") != 0 def test_resource_permissions(): + raise NotImplementedError() + r = AppResourceClassesByType["permissions"] conf = { "main": { diff --git a/src/utils/resources.py b/src/utils/resources.py index b4ec95bf8..1391e9c4a 100644 --- a/src/utils/resources.py +++ b/src/utils/resources.py @@ -216,7 +216,7 @@ class PermissionsResource(AppResource): super().__init__({"permissions": properties}, *args, **kwargs) - def provision_or_update(self, context: Dict): + def provision_or_update(self, context: Dict={}): from yunohost.permission import ( permission_create, @@ -267,7 +267,7 @@ class PermissionsResource(AppResource): permission_sync_to_user() - def deprovision(self, context: Dict): + def deprovision(self, context: Dict={}): from yunohost.permission import ( permission_delete, @@ -306,7 +306,7 @@ class SystemuserAppResource(AppResource): "allow_sftp": False } - def provision_or_update(self, context: Dict): + def provision_or_update(self, context: Dict={}): # FIXME : validate that no yunohost user exists with that name? # and/or that no system user exists during install ? @@ -328,7 +328,7 @@ class SystemuserAppResource(AppResource): os.system(f"usermod -G {','.join(groups)} {self.app}") - def deprovision(self, context: Dict): + def deprovision(self, context: Dict={}): if check_output(f"getent passwd {self.app} &>/dev/null || true").strip(): os.system(f"deluser {self.app} >/dev/null") @@ -382,7 +382,7 @@ class InstalldirAppResource(AppResource): # FIXME: change default dir to /opt/stuff if app ain't a webapp ... - def provision_or_update(self, context: Dict): + def provision_or_update(self, context: Dict={}): current_install_dir = self.get_setting("install_dir") or self.get_setting("final_path") @@ -403,16 +403,17 @@ class InstalldirAppResource(AppResource): group, group_perm = self.group.split(":") owner_perm_octal = (4 if "r" in owner_perm else 0) + (2 if "w" in owner_perm else 0) + (1 if "x" in owner_perm else 0) group_perm_octal = (4 if "r" in group_perm else 0) + (2 if "w" in group_perm else 0) + (1 if "x" in group_perm else 0) - perm_octal = str(owner_perm_octal) + str(group_perm_octal) + "0" - chmod(self.dir, int(perm_octal)) + perm_octal = 0o100 * owner_perm_octal + 0o010 * group_perm_octal + + chmod(self.dir, perm_octal) chown(self.dir, owner, group) # FIXME: shall we apply permissions recursively ? self.set_setting("install_dir", self.dir) self.delete_setting("final_path") # Legacy - def deprovision(self, context: Dict): + def deprovision(self, context: Dict={}): # FIXME : check that self.dir has a sensible value to prevent catastrophes if os.path.isdir(self.dir): rm(self.dir, recursive=True) @@ -444,7 +445,7 @@ class DatadirAppResource(AppResource): "group": "__APP__:rx", } - def provision_or_update(self, context: Dict): + def provision_or_update(self, context: Dict={}): current_data_dir = self.get_setting("data_dir") @@ -459,14 +460,14 @@ class DatadirAppResource(AppResource): group, group_perm = self.group.split(":") owner_perm_octal = (4 if "r" in owner_perm else 0) + (2 if "w" in owner_perm else 0) + (1 if "x" in owner_perm else 0) group_perm_octal = (4 if "r" in group_perm else 0) + (2 if "w" in group_perm else 0) + (1 if "x" in group_perm else 0) - perm_octal = str(owner_perm_octal) + str(group_perm_octal) + "0" + perm_octal = 0o100 * owner_perm_octal + 0o010 * group_perm_octal - chmod(self.dir, int(perm_octal)) + chmod(self.dir, perm_octal) chown(self.dir, owner, group) self.set_setting("data_dir", self.dir) - def deprovision(self, context: Dict): + def deprovision(self, context: Dict={}): # FIXME: This should rm the datadir only if purge is enabled pass #if os.path.isdir(self.dir): @@ -497,7 +498,7 @@ class DatadirAppResource(AppResource): # "main": {"url": "?", "sha256sum": "?", "predownload": True} # } # -# def provision_or_update(self, context: Dict): +# def provision_or_update(self, context: Dict={}): # # FIXME # return # @@ -534,7 +535,7 @@ class AptDependenciesAppResource(AppResource): super().__init__(properties, *args, **kwargs) - def provision_or_update(self, context: Dict): + def provision_or_update(self, context: Dict={}): script = [f"ynh_install_app_dependencies {self.packages}"] for repo, values in self.extras.items(): @@ -542,7 +543,7 @@ class AptDependenciesAppResource(AppResource): self._run_script("provision_or_update", '\n'.join(script)) - def deprovision(self, context: Dict): + def deprovision(self, context: Dict={}): self._run_script("deprovision", "ynh_remove_app_dependencies") @@ -580,7 +581,7 @@ class PortResource(AppResource): cmd2 = f"grep --quiet \"port: '{port}'\" /etc/yunohost/apps/*/settings.yml" return os.system(cmd1) == 0 and os.system(cmd2) == 0 - def provision_or_update(self, context: str): + def provision_or_update(self, context: Dict={}): # Don't do anything if port already defined ? if self.get_setting("port"): @@ -592,7 +593,7 @@ class PortResource(AppResource): self.set_setting("port", port) - def deprovision(self, context: Dict): + def deprovision(self, context: Dict={}): self.delete_setting("port") @@ -635,7 +636,7 @@ class DatabaseAppResource(AppResource): else: return False - def provision_or_update(self, context: str): + def provision_or_update(self, context: Dict={}): # This is equivalent to ynh_sanitize_dbid db_name = self.app.replace('-', '_').replace('.', '_') @@ -666,7 +667,7 @@ class DatabaseAppResource(AppResource): elif self.type == "postgresql": self._run_script("provision", f"ynh_psql_create_user '{db_user}' '{db_pwd}'; ynh_psql_create_db '{db_name}' '{db_user}'") - def deprovision(self, context: Dict): + def deprovision(self, context: Dict={}): db_name = self.app.replace('-', '_').replace('.', '_') db_user = db_name @@ -676,7 +677,9 @@ class DatabaseAppResource(AppResource): elif self.type == "postgresql": self._run_script("deprovision", f"ynh_psql_remove_db '{db_name}' '{db_user}'") - # FIXME : in fact we should delete settings to be consistent + self.delete_setting("db_name") + self.delete_setting("db_user") + self.delete_setting("db_pwd") AppResourceClassesByType = {c.type: c for c in AppResource.__subclasses__()}