manifestv2: fix many things, have resource system somewhat working for install/remove

helpers/apt

@@ -227,9 +227,8 @@ ynh_install_app_dependencies() {
     # Add a comma for each space between packages. But not add a comma if the space separate a version specification. (See  below)
     dependencies="$(echo "$dependencies" | sed 's/\([^\<=\>]\)\ \([^(]\)/\1, \2/g')"
     local dependencies=${dependencies//|/ | }
-    local manifest_path="$YNH_APP_BASEDIR/manifest.json"
 
-    local version=$(jq -r '.version' "$manifest_path")
+    local version=$(ynh_read_manifest --manifest_key="version")
     if [ -z "${version}" ] || [ "$version" == "null" ]; then
         version="1.0"
     fi

helpers/utils

@@ -765,12 +765,25 @@ ynh_read_manifest() {
     # Manage arguments with getopts
     ynh_handle_getopts_args "$@"
 
-    if [ ! -e "$manifest" ]; then
+    if [ ! -e "${manifest:-}" ]; then
         # If the manifest isn't found, try the common place for backup and restore script.
-        manifest="$YNH_APP_BASEDIR/manifest.json"
+        if [ -e "$YNH_APP_BASEDIR/manifest.json" ]
+        then
+            manifest="$YNH_APP_BASEDIR/manifest.json"
+        elif [ -e "$YNH_APP_BASEDIR/manifest.toml" ]
+        then
+            manifest="$YNH_APP_BASEDIR/manifest.toml"
+        else
+            ynh_die --message "No manifest found !?"
+        fi
     fi
 
-    jq ".$manifest_key" "$manifest" --raw-output
+    if echo "$manifest" | grep -q '\.json$'
+    then
+        jq ".$manifest_key" "$manifest" --raw-output
+    else
+        cat "$manifest" | python3 -c 'import json, toml, sys; print(json.dumps(toml.load(sys.stdin)))' | jq ".$manifest_key" --raw-output
+    fi
 }
 
 # Read the upstream version from the manifest or `$YNH_APP_MANIFEST_VERSION`
@@ -914,9 +927,7 @@ ynh_compare_current_package_version() {
 _ynh_apply_default_permissions() {
     local target=$1
 
-    local ynh_requirement=$(jq -r '.requirements.yunohost' $YNH_APP_BASEDIR/manifest.json | tr -d '>= ')
+    if dpkg --compare-versions ${YNH_APP_PACKAGING_FORMAT:-0} ge 2 || [ -z "$ynh_requirement" ] || [ "$ynh_requirement" == "null" ] || dpkg --compare-versions $ynh_requirement ge 4.2; then
-
-    if [ -z "$ynh_requirement" ] || [ "$ynh_requirement" == "null" ] || dpkg --compare-versions $ynh_requirement ge 4.2; then
         chmod o-rwx $target
         chmod g-w $target
         chown -R root:root $target

locales/en.json

@@ -43,7 +43,7 @@
     "app_packaging_format_not_supported": "This app cannot be installed because its packaging format is not supported by your YunoHost version. You should probably consider upgrading your system.",
     "app_remove_after_failed_install": "Removing the app following the installation failure...",
     "app_removed": "{app} uninstalled",
-    "app_requirements_checking": "Checking required packages for {app}...",
+    "app_requirements_checking": "Checking requirements for {app}...",
     "app_requirements_unmeet": "Requirements are not met for {app}, the package {pkgname} ({version}) must be {spec}",
     "app_restore_failed": "Could not restore {app}: {error}",
     "app_restore_script_failed": "An error occured inside the app restore script",

src/app.py

@@ -59,7 +59,6 @@ from yunohost.utils.config import (
     DomainQuestion,
     PathQuestion,
 )
-from yunohost.utils.resources import AppResourceSet
 from yunohost.utils.i18n import _value_for_locale
 from yunohost.utils.error import YunohostError, YunohostValidationError
 from yunohost.utils.system import (
@@ -790,7 +789,7 @@ def app_install(
     if free_space_in_directory("/") <= 512 * 1000 * 1000:
         raise YunohostValidationError("disk_space_not_sufficient_install")
 
-    _confirm_app_install(app)
+    _confirm_app_install(app, force)
     manifest, extracted_app_folder = _extract_app(app)
     packaging_format = manifest["packaging_format"]
 
@@ -824,10 +823,11 @@ def app_install(
     }
 
     # Validate domain / path availability for webapps
-    if packaging_format < 2:
+    # (ideally this should be handled by the resource system for manifest v >= 2
-        path_requirement = _guess_webapp_path_requirement(extracted_app_folder)
+    path_requirement = _guess_webapp_path_requirement(extracted_app_folder)
-        _validate_webpath_requirement(args, path_requirement)
+    _validate_webpath_requirement(args, path_requirement)
 
+    if packaging_format < 2:
         # Attempt to patch legacy helpers ...
         _patch_legacy_helpers(extracted_app_folder)
 
@@ -881,9 +881,14 @@ def app_install(
     # Initialize the main permission for the app
     # The permission is initialized with no url associated, and with tile disabled
     # For web app, the root path of the app will be added as url and the tile
-    # will be enabled during the app install. C.f. 'app_register_url()' below.
+    # will be enabled during the app install. C.f. 'app_register_url()' below
+    # or the webpath resource
     if packaging_format >= 2:
-        init_main_perm_allowed = ["visitors"] if not args.get("is_public") else ["all_users"]
+        if args.get("init_permission_main"):
+            init_main_perm_allowed = args.get("init_permission_main")
+        else:
+            init_main_perm_allowed = ["visitors"] if not args.get("is_public") else ["all_users"]
+
     else:
         init_main_perm_allowed = ["all_users"]
 
@@ -896,13 +901,13 @@ def app_install(
     )
 
     if packaging_format >= 2:
+        from yunohost.utils.resources import AppResourceManager
         try:
-            from yunohost.utils.resources import AppResourceManager
+            AppResourceManager(app_instance_name, wanted=manifest["resources"], current={}).apply()
-            resources = AppResourceManager(app_instance_name, current=app_setting_path, wanted=extracted_app_folder)
+        except Exception:
-            resources.apply()
+            # FIXME : improve error handling ....
-        except:
+            AppResourceManager(app_instance_name, wanted={}, current=manifest["resources"]).apply()
             raise
-            # FIXME : error handling
 
     # Prepare env. var. to pass to script
     env_dict = _make_environment_for_app_script(
@@ -1000,6 +1005,14 @@ def app_install(
                     m18n.n("unexpected_error", error="\n" + traceback.format_exc())
                 )
 
+            if packaging_format >= 2:
+                from yunohost.utils.resources import AppResourceManager
+                try:
+                    AppResourceManager(app_instance_name, wanted={}, current=manifest["resources"]).apply()
+                except Exception:
+                    # FIXME : improve error handling ....
+                    raise
+
             # Remove all permission in LDAP
             for permission_name in user_permission_list()["permissions"].keys():
                 if permission_name.startswith(app_instance_name + "."):
@@ -1103,21 +1116,30 @@ def app_remove(operation_logger, app, purge=False):
     finally:
         shutil.rmtree(tmp_workdir_for_app)
 
-    if ret == 0:
-        logger.success(m18n.n("app_removed", app=app))
-        hook_callback("post_app_remove", env=env_dict)
-    else:
-        logger.warning(m18n.n("app_not_properly_removed", app=app))
-
     # Remove all permission in LDAP
     for permission_name in user_permission_list(apps=[app])["permissions"].keys():
         permission_delete(permission_name, force=True, sync_perm=False)
 
+    packaging_format = manifest["packaging_format"]
+    if packaging_format >= 2:
+        try:
+            from yunohost.utils.resources import AppResourceManager
+            AppResourceManager(app, wanted={}, current=manifest["resources"]).apply()
+        except Exception:
+            # FIXME : improve error handling ....
+            raise
+
     if os.path.exists(app_setting_path):
         shutil.rmtree(app_setting_path)
 
     hook_remove(app)
 
+    if ret == 0:
+        logger.success(m18n.n("app_removed", app=app))
+        hook_callback("post_app_remove", env=env_dict)
+    else:
+        logger.warning(m18n.n("app_not_properly_removed", app=app))
+
     permission_sync_to_user()
     _assert_system_is_sane_for_app(manifest, "post")
 

src/utils/resources.py

@@ -20,10 +20,15 @@
 """
 import os
 import copy
+import shutil
 from typing import Dict, Any
 
+from moulinette.utils.process import check_output
 from moulinette.utils.log import getActionLogger
 from moulinette.utils.filesystem import mkdir, chown, chmod, write_to_file
+from moulinette.utils.filesystem import (
+    rm,
+)
 
 from yunohost.utils.error import YunohostError, YunohostValidationError
 from yunohost.hook import hook_exec
@@ -31,43 +36,57 @@ from yunohost.hook import hook_exec
 logger = getActionLogger("yunohost.app_resources")
 
 
-class AppResourceManager(object):
+class AppResourceManager:
 
-    def __init__(self, app: str, manifest: str):
+    def __init__(self, app: str, current: Dict, wanted: Dict):
 
         self.app = app
-        self.resources = {name: AppResourceClassesByType[name](infos, app)
+        self.current = current
-                          for name, infos in resources_dict.items()}
+        self.wanted = wanted
 
-    def apply(self):
+    def apply(self, **context):
+
+        for name, infos in self.wanted.items():
+            resource = AppResourceClassesByType[name](infos, self.app)
+            # FIXME: not a great place to check this because here
+            # we already started an operation
+            # We should find a way to validate this before actually starting
+            # the install procedure / theoperation log
+            if name not in self.current.keys():
+                resource.validate_availability(context=context)
+
+        for name, infos in reversed(self.current.items()):
+            if name not in self.wanted.keys():
+                resource = AppResourceClassesByType[name](infos, self.app)
+                # FIXME : i18n, better info strings
+                logger.info(f"Deprovisionning {name} ...")
+                resource.deprovision(context=context)
+
+        for name, infos in self.wanted.items():
+            resource = AppResourceClassesByType[name](infos, self.app)
+            if name not in self.current.keys():
+                # FIXME : i18n, better info strings
+                logger.info(f"Provisionning {name} ...")
+            else:
+                # FIXME : i18n, better info strings
+                logger.info(f"Updating {name} ...")
+            resource.provision_or_update(context=context)
 
 
-
+class AppResource:
-
-
-
-    def validate_resource_availability(self):
-
-        for name, resource in self.resources.items():
-            resource.validate_availability(context={})
-
-    def provision_or_update_resources(self):
-
-        for name, resource in self.resources.items():
-            logger.info("Running provision_or_upgrade for {self.type}")
-            resource.provision_or_update(context={})
-
-
-class AppResource(object):
 
     def __init__(self, properties: Dict[str, Any], app: str):
 
         self.app = app
 
         for key, value in self.default_properties.items():
+            if isinstance(value, str):
+                value = value.replace("__APP__", self.app)
             setattr(self, key, value)
 
         for key, value in properties.items():
+            if isinstance(value, str):
+                value = value.replace("__APP__", self.app)
             setattr(self, key, value)
 
     def get_setting(self, key):
@@ -78,7 +97,7 @@ class AppResource(object):
         from yunohost.app import app_setting
         app_setting(self.app, key, value=value)
 
-    def delete_setting(self, key, value):
+    def delete_setting(self, key):
         from yunohost.app import app_setting
         app_setting(self.app, key, delete=True)
 
@@ -104,11 +123,12 @@ ynh_abort_if_errors
 
         write_to_file(script_path, script)
 
-        print(env_)
+        #print(env_)
 
         # FIXME : use the hook_exec_with_debug_instructions_stuff
         ret, _ = hook_exec(script_path, env=env_)
-        print(ret)
+
+        #print(ret)
 
 
 class WebpathResource(AppResource):
@@ -137,20 +157,28 @@ class WebpathResource(AppResource):
     def validate_availability(self, context):
 
         from yunohost.app import _assert_no_conflicting_apps
-
         domain = self.get_setting("domain")
         path = self.get_setting("path") if not self.full_domain else "/"
         _assert_no_conflicting_apps(domain, path, ignore_app=self.app)
 
     def provision_or_update(self, context: Dict):
 
-        # Nothing to do ? Just setting the domain/path during install
+        from yunohost.permission import (
-        # already provisions it ...
+            permission_url,
-        return  # FIXME
+            user_permission_update,
+            permission_sync_to_user,
+        )
+
+        if context.get("action") == "install":
+            permission_url(f"{self.app}.main", url="/", sync_perm=False)
+            user_permission_update(f"{self.app}.main", show_tile=True, sync_perm=False)
+            permission_sync_to_user()
 
     def deprovision(self, context: Dict):
         self.delete_setting("domain")
         self.delete_setting("path")
+        # FIXME : theoretically here, should also remove the url in the main permission ?
+        # but is that worth the trouble ?
 
 
 class SystemuserAppResource(AppResource):
@@ -173,7 +201,7 @@ class SystemuserAppResource(AppResource):
     priority = 20
 
     default_properties = {
-        "allow_ssh": []
+        "allow_ssh": [],
         "allow_sftp": []
     }
 
@@ -190,37 +218,37 @@ class SystemuserAppResource(AppResource):
 
     def provision_or_update(self, context: Dict):
 
-        if os.system(f"getent passwd {self.app} &>/dev/null") != 0:
+        if not check_output(f"getent passwd {self.app} &>/dev/null || true").strip():
+            # FIXME: improve error handling ?
             cmd = f"useradd --system --user-group {self.app}"
             os.system(cmd)
 
-        if os.system(f"getent passwd {self.app} &>/dev/null") == 0:
+        if not check_output(f"getent passwd {self.app} &>/dev/null || true").strip():
-            raise YunohostError(f"Failed to create system user for {self.app}")
+            raise YunohostError(f"Failed to create system user for {self.app}", raw_msg=True)
+
+        groups = set(check_output(f"groups {self.app}").strip().split()[2:])
 
-        groups = []
         if self.allow_ssh:
-            groups.append("ssh.app")
+            groups.add("ssh.app")
         if self.allow_sftp:
-            groups.append("sftp.app")
+            groups.add("sftp.app")
-        groups =
-
-        cmd = f"usermod -a -G {groups} {self.app}"
-        # FIXME : handle case where group gets removed
-        os.system(cmd)
-
-#        useradd $user_home_dir --system --user-group $username $shell || ynh_die --message="Unable to create $username system account"
-#    for group in $groups; do
-#        usermod -a -G "$group" "$username"
-#    done
-
-
-# | arg: -g, --groups       - Add the user to system groups. Typically meant to add the user to the ssh.app / sftp.app group (e.g. for borgserver, my_webapp)
 
+        os.system(f"usermod -G {','.join(groups)} {self.app}")
 
     def deprovision(self, context: Dict):
 
-        self._run_script("deprovision",
+        if check_output(f"getent passwd {self.app} &>/dev/null || true").strip():
-                         f'ynh_system_user_delete "{self.username}"')
+            os.system(f"deluser {self.app} >/dev/null")
+        if check_output(f"getent passwd {self.app} &>/dev/null || true").strip():
+            raise YunohostError(f"Failed to delete system user for {self.app}")
+
+        if check_output(f"getent group {self.app} &>/dev/null || true").strip():
+            os.system(f"delgroup {self.app} >/dev/null")
+        if check_output(f"getent group {self.app} &>/dev/null || true").strip():
+            raise YunohostError(f"Failed to delete system user for {self.app}")
+
+        # FIXME : better logging and error handling, add stdout/stderr from the deluser/delgroup commands...
+
 
 #    # Check if the user exists on the system
 #if os.system(f"getent passwd {self.username} &>/dev/null") != 0:
@@ -288,7 +316,7 @@ class InstalldirAppResource(AppResource):
         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, oct(int(perm_octal)))
+        chmod(self.dir, int(perm_octal))
         chown(self.dir, owner, group)
 
         self.set_setting("install_dir", self.dir)
@@ -348,7 +376,7 @@ class DatadirAppResource(AppResource):
         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, oct(int(perm_octal)))
+        chmod(self.dir, int(perm_octal))
         chown(self.dir, owner, group)
 
         self.set_setting("data_dir", self.dir)