From eeec30d78c798bceefd69c7cc5773ba2c9acd0db Mon Sep 17 00:00:00 2001 From: axolotle Date: Thu, 3 Nov 2022 16:36:02 +0100 Subject: [PATCH 01/43] add antifeatures to app catalog --- share/actionsmap.yml | 4 ++++ src/app_catalog.py | 57 ++++++++++++++++++++++++++------------------ 2 files changed, 38 insertions(+), 23 deletions(-) diff --git a/share/actionsmap.yml b/share/actionsmap.yml index 98ae59a7b..29ea2ca12 100644 --- a/share/actionsmap.yml +++ b/share/actionsmap.yml @@ -756,6 +756,10 @@ app: full: --with-categories help: Also return a list of app categories action: store_true + -a: + full: --with-antifeatures + help: Also return a list of antifeatures categories + action: store_true ### app_search() search: diff --git a/src/app_catalog.py b/src/app_catalog.py index 22599a5a5..aa891f35c 100644 --- a/src/app_catalog.py +++ b/src/app_catalog.py @@ -41,12 +41,12 @@ APPS_CATALOG_API_VERSION = 3 APPS_CATALOG_DEFAULT_URL = "https://app.yunohost.org/default" -def app_catalog(full=False, with_categories=False): +def app_catalog(full=False, with_categories=False, with_antifeatures=False): """ Return a dict of apps available to installation from Yunohost's app catalog """ - from yunohost.app import _installed_apps, _set_default_ask_questions + from yunohost.app import _installed_apps # Get app list from catalog cache catalog = _load_apps_catalog() @@ -65,28 +65,38 @@ def app_catalog(full=False, with_categories=False): "description": infos["manifest"]["description"], "level": infos["level"], } - else: - infos["manifest"]["install"] = _set_default_ask_questions( - infos["manifest"].get("install", {}) - ) - # Trim info for categories if not using --full - for category in catalog["categories"]: - category["title"] = _value_for_locale(category["title"]) - category["description"] = _value_for_locale(category["description"]) - for subtags in category.get("subtags", []): - subtags["title"] = _value_for_locale(subtags["title"]) + _catalog = {"apps": catalog["apps"]} - if not full: - catalog["categories"] = [ - {"id": c["id"], "description": c["description"]} - for c in catalog["categories"] - ] + if with_categories: + for category in catalog["categories"]: + category["title"] = _value_for_locale(category["title"]) + category["description"] = _value_for_locale(category["description"]) + for subtags in category.get("subtags", []): + subtags["title"] = _value_for_locale(subtags["title"]) - if not with_categories: - return {"apps": catalog["apps"]} - else: - return {"apps": catalog["apps"], "categories": catalog["categories"]} + if not full: + catalog["categories"] = [ + {"id": c["id"], "description": c["description"]} + for c in catalog["categories"] + ] + + _catalog["categories"] = catalog["categories"] + + if with_antifeatures: + for antifeature in catalog["antifeatures"]: + antifeature["title"] = _value_for_locale(antifeature["title"]) + antifeature["description"] = _value_for_locale(antifeature["description"]) + + if not full: + catalog["antifeatures"] = [ + {"id": a["id"], "description": a["description"]} + for a in catalog["antifeatures"] + ] + + _catalog["antifeatures"] = catalog["antifeatures"] + + return _catalog def app_search(string): @@ -211,7 +221,7 @@ def _load_apps_catalog(): corresponding to all known apps and categories """ - merged_catalog = {"apps": {}, "categories": []} + merged_catalog = {"apps": {}, "categories": [], "antifeatures": []} for apps_catalog_id in [L["id"] for L in _read_apps_catalog_list()]: @@ -261,7 +271,8 @@ def _load_apps_catalog(): info["repository"] = apps_catalog_id merged_catalog["apps"][app] = info - # Annnnd categories + # Annnnd categories + antifeatures merged_catalog["categories"] += apps_catalog_content["categories"] + merged_catalog["antifeatures"] += apps_catalog_content["antifeatures"] return merged_catalog From 56de320a9afa1fd855a8891c7067aca44d0dea16 Mon Sep 17 00:00:00 2001 From: axolotle Date: Thu, 3 Nov 2022 16:47:18 +0100 Subject: [PATCH 02/43] add antifeatures and alternatives to catalog's apps manifest --- src/app.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/app.py b/src/app.py index f4d125a47..eee2d3c65 100644 --- a/src/app.py +++ b/src/app.py @@ -2301,6 +2301,9 @@ def _extract_app_from_gitrepo( manifest["remote"]["revision"] = revision manifest["lastUpdate"] = app_info.get("lastUpdate") + manifest["antifeatures"] = app_info["antifeatures"] + manifest["potential_alternative_to"] = app_info["potential_alternative_to"] + return manifest, extracted_app_folder From c45c0a98f21156ffc20302a880d972c55a05b459 Mon Sep 17 00:00:00 2001 From: axolotle Date: Mon, 14 Nov 2022 19:24:11 +0100 Subject: [PATCH 03/43] add app quality, antifeatures and alternative to base manifest --- src/app.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/app.py b/src/app.py index eee2d3c65..68c164760 100644 --- a/src/app.py +++ b/src/app.py @@ -2255,6 +2255,8 @@ def _extract_app_from_folder(path: str) -> Tuple[Dict, str]: logger.debug(m18n.n("done")) manifest["remote"] = {"type": "file", "path": path} + manifest["quality"] = {"level": -1, "state": "thirdparty"} + return manifest, extracted_app_folder @@ -2301,8 +2303,12 @@ def _extract_app_from_gitrepo( manifest["remote"]["revision"] = revision manifest["lastUpdate"] = app_info.get("lastUpdate") - manifest["antifeatures"] = app_info["antifeatures"] - manifest["potential_alternative_to"] = app_info["potential_alternative_to"] + manifest["quality"] = { + "level": app_info.get("level", -1), + "state": app_info.get("state", "thirdparty"), + } + manifest["antifeatures"] = app_info.get("antifeatures", []) + manifest["potential_alternative_to"] = app_info.get("potential_alternative_to") return manifest, extracted_app_folder From 727bef92e5a0c4e28bb9311b35e8ff720c2f3a8e Mon Sep 17 00:00:00 2001 From: axolotle Date: Mon, 14 Nov 2022 19:25:53 +0100 Subject: [PATCH 04/43] add ask_confirmation helper --- src/app.py | 39 +++++++++++++++++++++++++++------------ 1 file changed, 27 insertions(+), 12 deletions(-) diff --git a/src/app.py b/src/app.py index 68c164760..3419830f5 100644 --- a/src/app.py +++ b/src/app.py @@ -807,19 +807,9 @@ def _confirm_app_install(app, force=False): # i18n: confirm_app_install_thirdparty if quality in ["danger", "thirdparty"]: - answer = Moulinette.prompt( - m18n.n("confirm_app_install_" + quality, answers="Yes, I understand"), - color="red", - ) - if answer != "Yes, I understand": - raise YunohostError("aborting") - + _ask_confirmation("confirm_app_install_" + quality) else: - answer = Moulinette.prompt( - m18n.n("confirm_app_install_" + quality, answers="Y/N"), color="yellow" - ) - if answer.upper() != "Y": - raise YunohostError("aborting") + _ask_confirmation("confirm_app_install_" + quality, soft=True) @is_unit_operation() @@ -2750,3 +2740,28 @@ def _assert_system_is_sane_for_app(manifest, when): raise YunohostValidationError("dpkg_is_broken") elif when == "post": raise YunohostError("this_action_broke_dpkg") + + +# FIXME: move this to Moulinette +def _ask_confirmation( + question: str, + params: object = {}, + soft: bool = False, + force: bool = False, +): + if force or Moulinette.interface.type == "api": + return + + if soft: + answer = Moulinette.prompt( + m18n.n(question, answers="Y/N", **params), color="yellow" + ) + answer = answer.upper() == "Y" + else: + answer = Moulinette.prompt( + m18n.n(question, answers="Yes, I understand", **params), color="red" + ) + answer = answer == "Yes, I understand" + + if not answer: + raise YunohostError("aborting") From e202df479302173de1af784aaa7fbb17cefcfb27 Mon Sep 17 00:00:00 2001 From: axolotle Date: Mon, 14 Nov 2022 19:30:25 +0100 Subject: [PATCH 05/43] update checks for app requirements as generator --- src/app.py | 160 +++++++++++++++++++++++++++++++++++------------------ 1 file changed, 106 insertions(+), 54 deletions(-) diff --git a/src/app.py b/src/app.py index 3419830f5..0a62fd985 100644 --- a/src/app.py +++ b/src/app.py @@ -29,7 +29,7 @@ import subprocess import tempfile import copy from collections import OrderedDict -from typing import List, Tuple, Dict, Any +from typing import List, Tuple, Dict, Any, Iterator from packaging import version from moulinette import Moulinette, m18n @@ -587,7 +587,11 @@ def app_upgrade(app=[], url=None, file=None, force=False, no_safety_backup=False upgrade_type = "UPGRADE_FULL" # Check requirements - _check_manifest_requirements(manifest, action="upgrade") + for _, check, values, err in _check_manifest_requirements( + manifest, action="upgrade" + ): + if not check: + raise YunohostValidationError(err, **values) if manifest["packaging_format"] >= 2: if no_safety_backup: @@ -788,6 +792,18 @@ def app_manifest(app): raw_questions = manifest.get("install", {}).values() manifest["install"] = hydrate_questions_with_choices(raw_questions) + manifest["requirements"] = {} + for name, check, values, err in _check_manifest_requirements( + manifest, action="install" + ): + if Moulinette.interface.type == "api": + manifest["requirements"][name] = { + "pass": check, + "values": values, + } + else: + manifest["requirements"][name] = "ok" if check else m18n.n(err, **values) + return manifest @@ -873,7 +889,17 @@ def app_install( app_id = manifest["id"] # Check requirements - _check_manifest_requirements(manifest, action="install") + for name, check, values, err in _check_manifest_requirements( + manifest, action="install" + ): + if not check: + if name == "ram": + _ask_confirmation( + "confirm_app_install_insufficient_ram", params=values, force=force + ) + else: + raise YunohostValidationError(err, **values) + _assert_system_is_sane_for_app(manifest, "pre") # Check if app can be forked @@ -2351,72 +2377,98 @@ def _get_all_installed_apps_id(): return all_apps_ids_formatted -def _check_manifest_requirements(manifest: Dict, action: str): +def _check_manifest_requirements( + manifest: Dict, action: str = "" +) -> Iterator[Tuple[str, bool, object, str]]: """Check if required packages are met from the manifest""" - if manifest["packaging_format"] not in [1, 2]: - raise YunohostValidationError("app_packaging_format_not_supported") - app_id = manifest["id"] - logger.debug(m18n.n("app_requirements_checking", app=app_id)) - # Yunohost version requirement - - yunohost_requirement = version.parse(manifest["integration"]["yunohost"] or "4.3") - yunohost_installed_version = version.parse( - get_ynh_package_version("yunohost")["version"] + # Packaging format + yield ( + "packaging_format", + manifest["packaging_format"] in (1, 2), + {}, + "app_packaging_format_not_supported", # i18n: app_packaging_format_not_supported + ) + + # Yunohost version + required_yunohost_version = manifest["integration"].get("yunohost", "4.3") + current_yunohost_version = get_ynh_package_version("yunohost")["version"] + + yield ( + "version", + version.parse(required_yunohost_version) + <= version.parse(current_yunohost_version), + {"current": current_yunohost_version, "required": required_yunohost_version}, + "app_yunohost_version_not_supported", # i18n: app_yunohost_version_not_supported ) - if yunohost_requirement > yunohost_installed_version: - # FIXME : i18n - raise YunohostValidationError( - f"This app requires Yunohost >= {yunohost_requirement} but current installed version is {yunohost_installed_version}" - ) # Architectures arch_requirement = manifest["integration"]["architectures"] - if arch_requirement != "all": - arch = system_arch() - if arch not in arch_requirement: - # FIXME: i18n - raise YunohostValidationError( - f"This app can only be installed on architectures {', '.join(arch_requirement)} but your server architecture is {arch}" - ) + arch = system_arch() + + yield ( + "arch", + arch_requirement == "all" or arch in arch_requirement, + {"current": arch, "required": arch_requirement}, + "app_arch_not_supported", # i18n: app_arch_not_supported + ) # Multi-instance - if action == "install" and manifest["integration"]["multi_instance"] == False: - apps = _installed_apps() - sibling_apps = [a for a in apps if a == app_id or a.startswith(f"{app_id}__")] - if len(sibling_apps) > 0: - raise YunohostValidationError("app_already_installed", app=app_id) + if action == "install": + multi_instance = manifest["integration"]["multi_instance"] == True + if not multi_instance: + apps = _installed_apps() + sibling_apps = [ + a for a in apps if a == app_id or a.startswith(f"{app_id}__") + ] + multi_instance = len(sibling_apps) > 0 + + yield ( + "install", + multi_instance, + {}, + "app_already_installed", # i18n: app_already_installed + ) # Disk if action == "install": - disk_requirement = manifest["integration"]["disk"] - - if free_space_in_directory("/") <= human_to_binary( - disk_requirement - ) or free_space_in_directory("/var") <= human_to_binary(disk_requirement): - # FIXME : i18m - raise YunohostValidationError( - f"This app requires {disk_requirement} free space." - ) - - # Ram for build - ram_build_requirement = manifest["integration"]["ram"]["build"] - # Is "include_swap" really useful ? We should probably decide wether to always include it or not instead - ram_include_swap = manifest["integration"]["ram"].get("include_swap", False) - - ram, swap = ram_available() - if ram_include_swap: - ram += swap - - if ram < human_to_binary(ram_build_requirement): - # FIXME : i18n - ram_human = binary_to_human(ram) - raise YunohostValidationError( - f"This app requires {ram_build_requirement} RAM to install/upgrade but only {ram_human} is available right now." + disk_req_bin = human_to_binary(manifest["integration"]["disk"]) + root_free_space = free_space_in_directory("/") + var_free_space = free_space_in_directory("/var") + has_enough_disk = ( + root_free_space > disk_req_bin and var_free_space > disk_req_bin ) + free_space = binary_to_human( + root_free_space + if root_free_space == var_free_space + else root_free_space + var_free_space + ) + + yield ( + "disk", + has_enough_disk, + {"current": free_space, "required": manifest["integration"]["disk"]}, + "app_not_enough_disk", # i18n: app_not_enough_disk + ) + + # Ram + ram_requirement = manifest["integration"]["ram"] + ram, swap = ram_available() + # Is "include_swap" really useful ? We should probably decide wether to always include it or not instead + if ram_requirement.get("include_swap", False): + ram += swap + can_build = ram > human_to_binary(ram_requirement["build"]) + can_run = ram > human_to_binary(ram_requirement["runtime"]) + + yield ( + "ram", + can_build and can_run, + {"current": binary_to_human(ram), "required": ram_requirement["build"]}, + "app_not_enough_ram", # i18n: app_not_enough_ram + ) def _guess_webapp_path_requirement(app_folder: str) -> str: From d3d18c5ff266f42dd6531324b3f56b7fecf7ab00 Mon Sep 17 00:00:00 2001 From: axolotle Date: Tue, 15 Nov 2022 18:35:04 +0100 Subject: [PATCH 06/43] add locales & fix multi_instance check --- locales/en.json | 5 +++++ src/app.py | 6 +++--- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/locales/en.json b/locales/en.json index d18f8791e..0daf06d90 100644 --- a/locales/en.json +++ b/locales/en.json @@ -13,6 +13,7 @@ "app_already_installed": "{app} is already installed", "app_already_installed_cant_change_url": "This app is already installed. The URL cannot be changed just by this function. Check in `app changeurl` if it's available.", "app_already_up_to_date": "{app} is already up-to-date", + "app_arch_not_supported": "This app can only be installed on architectures {', '.join(required)} but your server architecture is {current}", "app_argument_choice_invalid": "Pick a valid value for argument '{name}': '{value}' is not among the available choices ({choices})", "app_argument_invalid": "Pick a valid value for the argument '{name}': {error}", "app_argument_password_no_default": "Error while parsing password argument '{name}': password argument can't have a default value for security reasons", @@ -39,6 +40,8 @@ "app_manifest_install_ask_password": "Choose an administration password for this app", "app_manifest_install_ask_path": "Choose the URL path (after the domain) where this app should be installed", "app_not_correctly_installed": "{app} seems to be incorrectly installed", + "app_not_enough_disk": "This app requires {required} free space.", + "app_not_enough_ram": "This app requires {required} RAM to install/upgrade but only {current} is available right now.", "app_not_installed": "Could not find {app} in the list of installed apps: {all_apps}", "app_not_properly_removed": "{app} has not been properly removed", "app_not_upgraded": "The app '{failed_app}' failed to upgrade, and as a consequence the following apps' upgrades have been cancelled: {apps}", @@ -61,6 +64,7 @@ "app_upgrade_several_apps": "The following apps will be upgraded: {apps}", "app_upgrade_some_app_failed": "Some apps could not be upgraded", "app_upgraded": "{app} upgraded", + "app_yunohost_version_not_supported": "This app requires YunoHost >= {required} but current installed version is {current}", "apps_already_up_to_date": "All apps are already up-to-date", "apps_catalog_failed_to_download": "Unable to download the {apps_catalog} app catalog: {error}", "apps_catalog_init_success": "App catalog system initialized!", @@ -159,6 +163,7 @@ "config_validate_url": "Should be a valid web URL", "config_version_not_supported": "Config panel versions '{version}' are not supported.", "confirm_app_install_danger": "DANGER! This app is known to be still experimental (if not explicitly not working)! You should probably NOT install it unless you know what you are doing. NO SUPPORT will be provided if this app doesn't work or breaks your system... If you are willing to take that risk anyway, type '{answers}'", + "confirm_app_install_insufficient_ram": "DANGER! This app requires {required} RAM to install/upgrade but only {current} is available right now. Even if this app could run, its installation process requires a large amount of RAM so your server may freeze and fail miserably. If you are willing to take that risk anyway, type '{answers}'", "confirm_app_install_thirdparty": "DANGER! This app is not part of YunoHost's app catalog. Installing third-party apps may compromise the integrity and security of your system. You should probably NOT install it unless you know what you are doing. NO SUPPORT will be provided if this app doesn't work or breaks your system... If you are willing to take that risk anyway, type '{answers}'", "confirm_app_install_warning": "Warning: This app may work, but is not well-integrated into YunoHost. Some features such as single sign-on and backup/restore might not be available. Install anyway? [{answers}] ", "custom_app_url_required": "You must provide a URL to upgrade your custom app {app}", diff --git a/src/app.py b/src/app.py index 0a62fd985..87aedeefc 100644 --- a/src/app.py +++ b/src/app.py @@ -2418,18 +2418,18 @@ def _check_manifest_requirements( # Multi-instance if action == "install": - multi_instance = manifest["integration"]["multi_instance"] == True + multi_instance = manifest["integration"]["multi_instance"] is True if not multi_instance: apps = _installed_apps() sibling_apps = [ a for a in apps if a == app_id or a.startswith(f"{app_id}__") ] - multi_instance = len(sibling_apps) > 0 + multi_instance = len(sibling_apps) == 0 yield ( "install", multi_instance, - {}, + {"app": app_id}, "app_already_installed", # i18n: app_already_installed ) From f405bfb6131ca27a42cda18f0f62dba43893bdc5 Mon Sep 17 00:00:00 2001 From: axolotle Date: Tue, 22 Nov 2022 14:44:05 +0100 Subject: [PATCH 07/43] [fix] catalog: default values for categories & antifeatures --- src/app_catalog.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/app_catalog.py b/src/app_catalog.py index aa891f35c..ea9b0f53e 100644 --- a/src/app_catalog.py +++ b/src/app_catalog.py @@ -272,7 +272,7 @@ def _load_apps_catalog(): merged_catalog["apps"][app] = info # Annnnd categories + antifeatures - merged_catalog["categories"] += apps_catalog_content["categories"] - merged_catalog["antifeatures"] += apps_catalog_content["antifeatures"] + merged_catalog["categories"] += apps_catalog_content.get("categories", []) + merged_catalog["antifeatures"] += apps_catalog_content.get("antifeatures", []) return merged_catalog From cbaa26f4729c9abb11ae150894f8d1032c1d8216 Mon Sep 17 00:00:00 2001 From: axolotle Date: Tue, 22 Nov 2022 15:23:42 +0100 Subject: [PATCH 08/43] AppUpgrade: ask confirmation when not enough ram --- locales/en.json | 2 +- src/app.py | 11 ++++++++--- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/locales/en.json b/locales/en.json index 0daf06d90..c39687fbe 100644 --- a/locales/en.json +++ b/locales/en.json @@ -163,9 +163,9 @@ "config_validate_url": "Should be a valid web URL", "config_version_not_supported": "Config panel versions '{version}' are not supported.", "confirm_app_install_danger": "DANGER! This app is known to be still experimental (if not explicitly not working)! You should probably NOT install it unless you know what you are doing. NO SUPPORT will be provided if this app doesn't work or breaks your system... If you are willing to take that risk anyway, type '{answers}'", - "confirm_app_install_insufficient_ram": "DANGER! This app requires {required} RAM to install/upgrade but only {current} is available right now. Even if this app could run, its installation process requires a large amount of RAM so your server may freeze and fail miserably. If you are willing to take that risk anyway, type '{answers}'", "confirm_app_install_thirdparty": "DANGER! This app is not part of YunoHost's app catalog. Installing third-party apps may compromise the integrity and security of your system. You should probably NOT install it unless you know what you are doing. NO SUPPORT will be provided if this app doesn't work or breaks your system... If you are willing to take that risk anyway, type '{answers}'", "confirm_app_install_warning": "Warning: This app may work, but is not well-integrated into YunoHost. Some features such as single sign-on and backup/restore might not be available. Install anyway? [{answers}] ", + "confirm_app_insufficient_ram": "DANGER! This app requires {required} RAM to install/upgrade but only {current} is available right now. Even if this app could run, its installation/upgrade process requires a large amount of RAM so your server may freeze and fail miserably. If you are willing to take that risk anyway, type '{answers}'", "custom_app_url_required": "You must provide a URL to upgrade your custom app {app}", "danger": "Danger:", "diagnosis_apps_allgood": "All installed apps respect basic packaging practices", diff --git a/src/app.py b/src/app.py index 87aedeefc..de4636461 100644 --- a/src/app.py +++ b/src/app.py @@ -587,11 +587,16 @@ def app_upgrade(app=[], url=None, file=None, force=False, no_safety_backup=False upgrade_type = "UPGRADE_FULL" # Check requirements - for _, check, values, err in _check_manifest_requirements( + for name, check, values, err in _check_manifest_requirements( manifest, action="upgrade" ): if not check: - raise YunohostValidationError(err, **values) + if name == "ram": + _ask_confirmation( + "confirm_app_insufficient_ram", params=values, force=force + ) + else: + raise YunohostValidationError(err, **values) if manifest["packaging_format"] >= 2: if no_safety_backup: @@ -895,7 +900,7 @@ def app_install( if not check: if name == "ram": _ask_confirmation( - "confirm_app_install_insufficient_ram", params=values, force=force + "confirm_app_insufficient_ram", params=values, force=force ) else: raise YunohostValidationError(err, **values) From ea3826fb8d1da772510122aafe5a779d70684801 Mon Sep 17 00:00:00 2001 From: axolotle Date: Wed, 23 Nov 2022 16:00:35 +0100 Subject: [PATCH 09/43] add new type 'simple' to cli _ask_confirmation --- src/app.py | 24 ++++++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/src/app.py b/src/app.py index de4636461..2974a69d8 100644 --- a/src/app.py +++ b/src/app.py @@ -830,7 +830,7 @@ def _confirm_app_install(app, force=False): if quality in ["danger", "thirdparty"]: _ask_confirmation("confirm_app_install_" + quality) else: - _ask_confirmation("confirm_app_install_" + quality, soft=True) + _ask_confirmation("confirm_app_install_" + quality, kind="soft") @is_unit_operation() @@ -2802,14 +2802,30 @@ def _assert_system_is_sane_for_app(manifest, when): # FIXME: move this to Moulinette def _ask_confirmation( question: str, - params: object = {}, - soft: bool = False, + params: dict = {}, + kind: str = "hard", force: bool = False, ): + """ + Ask confirmation + + Keyword argument: + question -- m18n key or string + params -- dict of values passed to the string formating + kind -- "hard": ask with "Yes, I understand", "soft": "Y/N", "simple": "press enter" + force -- Will not ask for confirmation + + """ if force or Moulinette.interface.type == "api": return - if soft: + if kind == "simple": + answer = Moulinette.prompt( + m18n.n(question, answers="Press enter to continue", **params), + color="yellow", + ) + answer = True + elif kind == "soft": answer = Moulinette.prompt( m18n.n(question, answers="Y/N", **params), color="yellow" ) From cdcd967eb1f3fdd7c1e5bdfb13171b85a59298ba Mon Sep 17 00:00:00 2001 From: axolotle Date: Wed, 23 Nov 2022 16:02:15 +0100 Subject: [PATCH 10/43] app: add notifications helpers --- locales/en.json | 1 + src/app.py | 23 +++++++++++++++++++++++ 2 files changed, 24 insertions(+) diff --git a/locales/en.json b/locales/en.json index c39687fbe..aac3bf69e 100644 --- a/locales/en.json +++ b/locales/en.json @@ -166,6 +166,7 @@ "confirm_app_install_thirdparty": "DANGER! This app is not part of YunoHost's app catalog. Installing third-party apps may compromise the integrity and security of your system. You should probably NOT install it unless you know what you are doing. NO SUPPORT will be provided if this app doesn't work or breaks your system... If you are willing to take that risk anyway, type '{answers}'", "confirm_app_install_warning": "Warning: This app may work, but is not well-integrated into YunoHost. Some features such as single sign-on and backup/restore might not be available. Install anyway? [{answers}] ", "confirm_app_insufficient_ram": "DANGER! This app requires {required} RAM to install/upgrade but only {current} is available right now. Even if this app could run, its installation/upgrade process requires a large amount of RAM so your server may freeze and fail miserably. If you are willing to take that risk anyway, type '{answers}'", + "confirm_notifications_read": "WARNING: You should check the app notifications above before continuing, there might be important stuff to know. [{answers}]", "custom_app_url_required": "You must provide a URL to upgrade your custom app {app}", "danger": "Danger:", "diagnosis_apps_allgood": "All installed apps respect basic packaging practices", diff --git a/src/app.py b/src/app.py index 2974a69d8..fac0bd484 100644 --- a/src/app.py +++ b/src/app.py @@ -2799,6 +2799,29 @@ def _assert_system_is_sane_for_app(manifest, when): raise YunohostError("this_action_broke_dpkg") +def _filter_and_hydrate_notifications(notifications, current_version=None, data={}): + return { + # Should we render the markdown maybe? idk + name: _hydrate_app_template(_value_for_locale(content_per_lang), data) + for name, content_per_lang in notifications.items() + if current_version is None + or name == "main" + or version.parse(name) > current_version + } + + +def _display_notifications(notifications, force=False): + if not notifications: + return + + for name, content in notifications.items(): + print(f"========== {name}") + print(content) + print("==========") + + _ask_confirmation("confirm_notifications_read", kind="simple", force=force) + + # FIXME: move this to Moulinette def _ask_confirmation( question: str, From c4432b7823ec7c420237e5303b3ead391ea84b39 Mon Sep 17 00:00:00 2001 From: axolotle Date: Wed, 23 Nov 2022 16:05:11 +0100 Subject: [PATCH 11/43] app_upgrade: display pre and post notifications --- src/app.py | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/src/app.py b/src/app.py index fac0bd484..ef8b54a01 100644 --- a/src/app.py +++ b/src/app.py @@ -598,6 +598,19 @@ def app_upgrade(app=[], url=None, file=None, force=False, no_safety_backup=False else: raise YunohostValidationError(err, **values) + # Display pre-upgrade notifications and ask for simple confirm + if ( + manifest["notifications"]["pre_upgrade"] + and Moulinette.interface.type == "cli" + ): + settings = _get_app_settings(app_instance_name) + notifications = _filter_and_hydrate_notifications( + manifest["notifications"]["pre_upgrade"], + current_version=app_current_version, + data=settings, + ) + _display_notifications(notifications, force=force) + if manifest["packaging_format"] >= 2: if no_safety_backup: # FIXME: i18n @@ -780,6 +793,21 @@ def app_upgrade(app=[], url=None, file=None, force=False, no_safety_backup=False # So much win logger.success(m18n.n("app_upgraded", app=app_instance_name)) + # Display post-upgrade notifications and ask for simple confirm + if ( + manifest["notifications"]["post_upgrade"] + and Moulinette.interface.type == "cli" + ): + # Get updated settings to hydrate notifications + settings = _get_app_settings(app_instance_name) + notifications = _filter_and_hydrate_notifications( + manifest["notifications"]["post_upgrade"], + current_version=app_current_version, + data=settings, + ) + _display_notifications(notifications, force=force) + # FIXME Should return a response with post upgrade notif formatted with the newest settings to the web-admin + hook_callback("post_app_upgrade", env=env_dict) operation_logger.success() From d0faf8a64a5b7845c27f75259883b6f3e6994ca9 Mon Sep 17 00:00:00 2001 From: axolotle Date: Wed, 23 Nov 2022 16:07:22 +0100 Subject: [PATCH 12/43] tools_update: add hydrated pre/post upgrade notifications --- src/app.py | 3 ++- src/tools.py | 24 +++++++++++++++++++++++- 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/src/app.py b/src/app.py index ef8b54a01..c938a91c4 100644 --- a/src/app.py +++ b/src/app.py @@ -164,6 +164,8 @@ def app_info(app, full=False, upgradable=False): ret["current_version"] = f" ({current_revision})" ret["new_version"] = f" ({new_revision})" + ret["settings"] = settings + if not full: return ret @@ -175,7 +177,6 @@ def app_info(app, full=False, upgradable=False): ret["manifest"]["install"] = _set_default_ask_questions( ret["manifest"].get("install", {}) ) - ret["settings"] = settings ret["from_catalog"] = from_catalog diff --git a/src/tools.py b/src/tools.py index b4eef34cc..58c8dee7f 100644 --- a/src/tools.py +++ b/src/tools.py @@ -20,6 +20,7 @@ import re import os import subprocess import time +import shutil from importlib import import_module from packaging import version from typing import List @@ -29,7 +30,13 @@ from moulinette.utils.log import getActionLogger from moulinette.utils.process import call_async_output from moulinette.utils.filesystem import read_yaml, write_to_yaml, cp, mkdir, rm, chown -from yunohost.app import app_upgrade, app_list +from yunohost.app import ( + app_upgrade, + app_list, + _filter_and_hydrate_notifications, + _extract_app, + _parse_app_instance_name, +) from yunohost.app_catalog import ( _initialize_apps_catalog_system, _update_apps_catalog, @@ -365,6 +372,21 @@ def tools_update(target=None): upgradable_apps = list(app_list(upgradable=True)["apps"]) + # Retrieve next manifest notifications + for app in upgradable_apps: + absolute_app_name, _ = _parse_app_instance_name(app["id"]) + manifest, extracted_app_folder = _extract_app(absolute_app_name) + current_version = version.parse(app["current_version"]) + app["notifications"] = { + type_: _filter_and_hydrate_notifications( + manifest["notifications"][type_], current_version, app["settings"] + ) + for type_ in ("pre_upgrade", "post_upgrade") + } + # FIXME Post-upgrade notifs should be hydrated with post-upgrade app settings + del app["settings"] + shutil.rmtree(extracted_app_folder) + if len(upgradable_apps) == 0 and len(upgradable_system_packages) == 0: logger.info(m18n.n("already_up_to_date")) From a54e976e21d2d7746963af7090bd917b7f3b206e Mon Sep 17 00:00:00 2001 From: axolotle Date: Wed, 23 Nov 2022 16:08:22 +0100 Subject: [PATCH 13/43] app_install: update notifications display with helpers --- src/app.py | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/src/app.py b/src/app.py index c938a91c4..b8e1cc771 100644 --- a/src/app.py +++ b/src/app.py @@ -908,11 +908,10 @@ def app_install( # Display pre_install notices in cli mode if manifest["notifications"]["pre_install"] and Moulinette.interface.type == "cli": - for notice in manifest["notifications"]["pre_install"].values(): - # Should we render the markdown maybe? idk - print("==========") - print(_value_for_locale(notice)) - print("==========") + notifications = _filter_and_hydrate_notifications( + manifest["notifications"]["pre_install"] + ) + _display_notifications(notifications, force=force) packaging_format = manifest["packaging_format"] @@ -1182,13 +1181,12 @@ def app_install( # Display post_install notices in cli mode if manifest["notifications"]["post_install"] and Moulinette.interface.type == "cli": - # (Call app_info to get the version hydrated with settings) - infos = app_info(app_instance_name, full=True) - for notice in infos["manifest"]["notifications"]["post_install"].values(): - # Should we render the markdown maybe? idk - print("==========") - print(_value_for_locale(notice)) - print("==========") + # Get the generated settings to hydrate notifications + settings = _get_app_settings(app_instance_name) + notifications = _filter_and_hydrate_notifications( + manifest["notifications"]["post_install"], data=settings + ) + _display_notifications(notifications, force=force) # Call postinstall hook hook_callback("post_app_install", env=env_dict) From 2d3546247a1df55e56a288108b4c00e2bb4ba002 Mon Sep 17 00:00:00 2001 From: axolotle Date: Sat, 26 Nov 2022 12:44:15 +0100 Subject: [PATCH 14/43] [kindafix] app_install: override manifest name by given label --- src/app.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/app.py b/src/app.py index b8e1cc771..f975178f5 100644 --- a/src/app.py +++ b/src/app.py @@ -1010,6 +1010,10 @@ def app_install( recursive=True, ) + # Override manifest name by given label + if label: + manifest["name"] = label + if packaging_format >= 2: from yunohost.utils.resources import AppResourceManager @@ -1029,7 +1033,7 @@ def app_install( permission_create( app_instance_name + ".main", allowed=["all_users"], - label=label if label else manifest["name"], + label=manifest["name"], show_tile=False, protected=False, ) From f096a14189b9da3052aabad87b5ba488e83b1cab Mon Sep 17 00:00:00 2001 From: axolotle Date: Sat, 26 Nov 2022 12:46:11 +0100 Subject: [PATCH 15/43] app_config_get: do not raise error if no config panel found on API --- src/app.py | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/app.py b/src/app.py index f975178f5..9740aa206 100644 --- a/src/app.py +++ b/src/app.py @@ -1666,8 +1666,15 @@ def app_config_get(app, key="", full=False, export=False): else: mode = "classic" - config_ = AppConfigPanel(app) - return config_.get(key, mode) + try: + config_ = AppConfigPanel(app) + return config_.get(key, mode) + except YunohostValidationError as e: + if Moulinette.interface.type == 'api' and e.key == "config_no_panel": + # Be more permissive when no config panel found + return {} + else: + raise @is_unit_operation() From d766e74a6a46aabcfa9a87bada7ef8ab9e4e9676 Mon Sep 17 00:00:00 2001 From: axolotle Date: Sat, 26 Nov 2022 13:52:19 +0100 Subject: [PATCH 16/43] app_install: return post_install notifs to API --- src/app.py | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/src/app.py b/src/app.py index 9740aa206..0f8a71f23 100644 --- a/src/app.py +++ b/src/app.py @@ -1183,18 +1183,23 @@ def app_install( logger.success(m18n.n("installation_complete")) + # Get the generated settings to hydrate notifications + settings = _get_app_settings(app_instance_name) + notifications = _filter_and_hydrate_notifications( + manifest["notifications"]["post_install"], data=settings + ) + # Display post_install notices in cli mode - if manifest["notifications"]["post_install"] and Moulinette.interface.type == "cli": - # Get the generated settings to hydrate notifications - settings = _get_app_settings(app_instance_name) - notifications = _filter_and_hydrate_notifications( - manifest["notifications"]["post_install"], data=settings - ) + if notifications and Moulinette.interface.type == "cli": _display_notifications(notifications, force=force) # Call postinstall hook hook_callback("post_app_install", env=env_dict) + # Return hydrated post install notif for API + if Moulinette.interface.type == "api": + return {"notifications": notifications} + @is_unit_operation() def app_remove(operation_logger, app, purge=False): From dcf4b85b21eb78f49147cf7818b22d9b4d8b8d1f Mon Sep 17 00:00:00 2001 From: axolotle Date: Sat, 26 Nov 2022 15:48:18 +0100 Subject: [PATCH 17/43] app_manifest: return base64 screenshot to API --- src/app.py | 23 ++++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/src/app.py b/src/app.py index 0f8a71f23..730603c52 100644 --- a/src/app.py +++ b/src/app.py @@ -821,11 +821,28 @@ def app_manifest(app): manifest, extracted_app_folder = _extract_app(app) - shutil.rmtree(extracted_app_folder) - raw_questions = manifest.get("install", {}).values() manifest["install"] = hydrate_questions_with_choices(raw_questions) + # Add a base64 image to be displayed in web-admin + if Moulinette.interface.type == "api": + import base64 + + manifest["image"] = None + screenshots_folder = os.path.join(extracted_app_folder, "doc", "screenshots") + + if os.path.exists(screenshots_folder): + with os.scandir(screenshots_folder) as it: + for entry in it: + ext = os.path.splitext(entry.name)[1].replace(".", "").lower() + if entry.is_file() and ext in ("png", "jpg", "jpeg", "webp"): + with open(entry.path, "rb") as img_file: + data = base64.b64encode(img_file.read()).decode("utf-8") + manifest["image"] = f"data:image/{ext};charset=utf-8;base64,{data}" + break + + shutil.rmtree(extracted_app_folder) + manifest["requirements"] = {} for name, check, values, err in _check_manifest_requirements( manifest, action="install" @@ -1675,7 +1692,7 @@ def app_config_get(app, key="", full=False, export=False): config_ = AppConfigPanel(app) return config_.get(key, mode) except YunohostValidationError as e: - if Moulinette.interface.type == 'api' and e.key == "config_no_panel": + if Moulinette.interface.type == "api" and e.key == "config_no_panel": # Be more permissive when no config panel found return {} else: From 34b191582a8b6ea3dd5a41f4d96fa811ec84426a Mon Sep 17 00:00:00 2001 From: Axolotle Date: Sun, 4 Dec 2022 13:12:42 +0100 Subject: [PATCH 18/43] rename 'check' boolean to 'passed' Co-authored-by: Alexandre Aubin --- src/app.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/app.py b/src/app.py index 74ffa0014..946b46705 100644 --- a/src/app.py +++ b/src/app.py @@ -588,10 +588,10 @@ def app_upgrade(app=[], url=None, file=None, force=False, no_safety_backup=False upgrade_type = "UPGRADE_FULL" # Check requirements - for name, check, values, err in _check_manifest_requirements( + for name, passed, values, err in _check_manifest_requirements( manifest, action="upgrade" ): - if not check: + if not passed: if name == "ram": _ask_confirmation( "confirm_app_insufficient_ram", params=values, force=force @@ -844,16 +844,16 @@ def app_manifest(app): shutil.rmtree(extracted_app_folder) manifest["requirements"] = {} - for name, check, values, err in _check_manifest_requirements( + for name, passed, values, err in _check_manifest_requirements( manifest, action="install" ): if Moulinette.interface.type == "api": manifest["requirements"][name] = { - "pass": check, + "pass": passed, "values": values, } else: - manifest["requirements"][name] = "ok" if check else m18n.n(err, **values) + manifest["requirements"][name] = "ok" if passed else m18n.n(err, **values) return manifest @@ -939,10 +939,10 @@ def app_install( app_id = manifest["id"] # Check requirements - for name, check, values, err in _check_manifest_requirements( + for name, passed, values, err in _check_manifest_requirements( manifest, action="install" ): - if not check: + if not passed: if name == "ram": _ask_confirmation( "confirm_app_insufficient_ram", params=values, force=force From 8d9605161cf7fdabd7fc4023c8da5b161f8cdda4 Mon Sep 17 00:00:00 2001 From: Axolotle Date: Sun, 4 Dec 2022 13:15:29 +0100 Subject: [PATCH 19/43] add some comments Co-authored-by: Alexandre Aubin --- src/app.py | 1 + src/app_catalog.py | 1 + 2 files changed, 2 insertions(+) diff --git a/src/app.py b/src/app.py index 946b46705..06310871e 100644 --- a/src/app.py +++ b/src/app.py @@ -1028,6 +1028,7 @@ def app_install( ) # Override manifest name by given label + # This info is also later picked-up by the 'permission' resource initialization if label: manifest["name"] = label diff --git a/src/app_catalog.py b/src/app_catalog.py index ea9b0f53e..22a878579 100644 --- a/src/app_catalog.py +++ b/src/app_catalog.py @@ -272,6 +272,7 @@ def _load_apps_catalog(): merged_catalog["apps"][app] = info # Annnnd categories + antifeatures + # (we use .get here, only because the dev catalog doesnt include the categories/antifeatures keys) merged_catalog["categories"] += apps_catalog_content.get("categories", []) merged_catalog["antifeatures"] += apps_catalog_content.get("antifeatures", []) From 92608c6ee31171e6b88c0a900dfb35aabc456246 Mon Sep 17 00:00:00 2001 From: Axolotle Date: Sun, 4 Dec 2022 13:19:11 +0100 Subject: [PATCH 20/43] normalize _extract_app_from_* manifest keys Co-authored-by: Alexandre Aubin --- src/app.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/app.py b/src/app.py index 06310871e..c24276cf3 100644 --- a/src/app.py +++ b/src/app.py @@ -874,7 +874,7 @@ def _confirm_app_install(app, force=False): # i18n: confirm_app_install_thirdparty if quality in ["danger", "thirdparty"]: - _ask_confirmation("confirm_app_install_" + quality) + _ask_confirmation("confirm_app_install_" + quality, kind="hard") else: _ask_confirmation("confirm_app_install_" + quality, kind="soft") @@ -2338,6 +2338,8 @@ def _extract_app_from_folder(path: str) -> Tuple[Dict, str]: manifest["remote"] = {"type": "file", "path": path} manifest["quality"] = {"level": -1, "state": "thirdparty"} + manifest["antifeatures"] = [] + manifest["potential_alternative_to"] = [] return manifest, extracted_app_folder @@ -2390,7 +2392,7 @@ def _extract_app_from_gitrepo( "state": app_info.get("state", "thirdparty"), } manifest["antifeatures"] = app_info.get("antifeatures", []) - manifest["potential_alternative_to"] = app_info.get("potential_alternative_to") + manifest["potential_alternative_to"] = app_info.get("potential_alternative_to", []) return manifest, extracted_app_folder From d4d739bbe2380cdc777552c010c394909928b646 Mon Sep 17 00:00:00 2001 From: Axolotle Date: Sun, 4 Dec 2022 13:23:32 +0100 Subject: [PATCH 21/43] improve version check & rename check key Co-authored-by: Alexandre Aubin --- src/app.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/app.py b/src/app.py index c24276cf3..b2cf45c80 100644 --- a/src/app.py +++ b/src/app.py @@ -2462,11 +2462,11 @@ def _check_manifest_requirements( ) # Yunohost version - required_yunohost_version = manifest["integration"].get("yunohost", "4.3") + required_yunohost_version = manifest["integration"].get("yunohost", "4.3").strip(">= ") current_yunohost_version = get_ynh_package_version("yunohost")["version"] yield ( - "version", + "required_yunohost_version", version.parse(required_yunohost_version) <= version.parse(current_yunohost_version), {"current": current_yunohost_version, "required": required_yunohost_version}, From b17e00c31ec1becd4fe1039424ffcd1bfaeec352 Mon Sep 17 00:00:00 2001 From: Axolotle Date: Sun, 4 Dec 2022 13:24:47 +0100 Subject: [PATCH 22/43] skip confirmation if ran from CLI in a non-interactive context Co-authored-by: Alexandre Aubin --- src/app.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/app.py b/src/app.py index b2cf45c80..1b4f03be6 100644 --- a/src/app.py +++ b/src/app.py @@ -2900,7 +2900,11 @@ def _ask_confirmation( """ if force or Moulinette.interface.type == "api": return - + + # If ran from the CLI in a non-interactive context, + # skip confirmation (except in hard mode) + if not os.isatty(1) and kind in ["simple", "soft"]: + return if kind == "simple": answer = Moulinette.prompt( m18n.n(question, answers="Press enter to continue", **params), From 6ae9108decfd5d98e83b64995f0f3fe4f89df2e6 Mon Sep 17 00:00:00 2001 From: axolotle Date: Sun, 4 Dec 2022 14:42:07 +0100 Subject: [PATCH 23/43] add --with-screenshot option for app_manifest + rename 'image' key to 'screenshot' --- share/actionsmap.yml | 4 ++++ src/app.py | 10 +++++----- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/share/actionsmap.yml b/share/actionsmap.yml index 59d9cd3a2..0a6f10856 100644 --- a/share/actionsmap.yml +++ b/share/actionsmap.yml @@ -807,6 +807,10 @@ app: arguments: app: help: Name, local path or git URL of the app to fetch the manifest of + -s: + full: --with-screenshot + help: Also return a base64 screenshot if any (API only) + action: store_true ### app_list() list: diff --git a/src/app.py b/src/app.py index 1b4f03be6..aa8afc72c 100644 --- a/src/app.py +++ b/src/app.py @@ -817,7 +817,7 @@ def app_upgrade(app=[], url=None, file=None, force=False, no_safety_backup=False logger.success(m18n.n("upgrade_complete")) -def app_manifest(app): +def app_manifest(app, with_screenshot=False): manifest, extracted_app_folder = _extract_app(app) @@ -825,20 +825,20 @@ def app_manifest(app): manifest["install"] = hydrate_questions_with_choices(raw_questions) # Add a base64 image to be displayed in web-admin - if Moulinette.interface.type == "api": + if with_screenshot and Moulinette.interface.type == "api": import base64 - manifest["image"] = None + manifest["screenshot"] = None screenshots_folder = os.path.join(extracted_app_folder, "doc", "screenshots") if os.path.exists(screenshots_folder): with os.scandir(screenshots_folder) as it: for entry in it: ext = os.path.splitext(entry.name)[1].replace(".", "").lower() - if entry.is_file() and ext in ("png", "jpg", "jpeg", "webp"): + if entry.is_file() and ext in ("png", "jpg", "jpeg", "webp", "gif"): with open(entry.path, "rb") as img_file: data = base64.b64encode(img_file.read()).decode("utf-8") - manifest["image"] = f"data:image/{ext};charset=utf-8;base64,{data}" + manifest["screenshot"] = f"data:image/{ext};charset=utf-8;base64,{data}" break shutil.rmtree(extracted_app_folder) From 968687b5128629a68669851551e9902b7f280ce3 Mon Sep 17 00:00:00 2001 From: axolotle Date: Sun, 4 Dec 2022 15:02:32 +0100 Subject: [PATCH 24/43] move tools_update check for notif in _list_upgradable_apps helper --- src/app.py | 21 +++++++++++++++++++++ src/tools.py | 22 ++-------------------- 2 files changed, 23 insertions(+), 20 deletions(-) diff --git a/src/app.py b/src/app.py index aa8afc72c..d6fa3979a 100644 --- a/src/app.py +++ b/src/app.py @@ -2397,6 +2397,27 @@ def _extract_app_from_gitrepo( return manifest, extracted_app_folder +def _list_upgradable_apps(): + upgradable_apps = list(app_list(upgradable=True)["apps"]) + + # Retrieve next manifest pre_upgrade notifications + for app in upgradable_apps: + absolute_app_name, _ = _parse_app_instance_name(app["id"]) + manifest, extracted_app_folder = _extract_app(absolute_app_name) + current_version = version.parse(app["current_version"]) + app["notifications"] = {} + if manifest["notifications"]["pre_upgrade"]: + app["notifications"]["pre_upgrade"] = _filter_and_hydrate_notifications( + manifest["notifications"]["pre_upgrade"], + current_version, + app["settings"], + ) + del app["settings"] + shutil.rmtree(extracted_app_folder) + + return upgradable_apps + + # # ############################### # # Small utilities # diff --git a/src/tools.py b/src/tools.py index 58c8dee7f..c52cad675 100644 --- a/src/tools.py +++ b/src/tools.py @@ -20,7 +20,6 @@ import re import os import subprocess import time -import shutil from importlib import import_module from packaging import version from typing import List @@ -33,9 +32,7 @@ from moulinette.utils.filesystem import read_yaml, write_to_yaml, cp, mkdir, rm, from yunohost.app import ( app_upgrade, app_list, - _filter_and_hydrate_notifications, - _extract_app, - _parse_app_instance_name, + _list_upgradable_apps, ) from yunohost.app_catalog import ( _initialize_apps_catalog_system, @@ -370,22 +367,7 @@ def tools_update(target=None): except YunohostError as e: logger.error(str(e)) - upgradable_apps = list(app_list(upgradable=True)["apps"]) - - # Retrieve next manifest notifications - for app in upgradable_apps: - absolute_app_name, _ = _parse_app_instance_name(app["id"]) - manifest, extracted_app_folder = _extract_app(absolute_app_name) - current_version = version.parse(app["current_version"]) - app["notifications"] = { - type_: _filter_and_hydrate_notifications( - manifest["notifications"][type_], current_version, app["settings"] - ) - for type_ in ("pre_upgrade", "post_upgrade") - } - # FIXME Post-upgrade notifs should be hydrated with post-upgrade app settings - del app["settings"] - shutil.rmtree(extracted_app_folder) + upgradable_apps = _list_upgradable_apps() if len(upgradable_apps) == 0 and len(upgradable_system_packages) == 0: logger.info(m18n.n("already_up_to_date")) From 700154ceb65fbd0c736bf095cc58ff5c45a2c677 Mon Sep 17 00:00:00 2001 From: axolotle Date: Sun, 4 Dec 2022 15:20:03 +0100 Subject: [PATCH 25/43] app_upgrade return post_install notif to API --- src/app.py | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/src/app.py b/src/app.py index d6fa3979a..702f23edb 100644 --- a/src/app.py +++ b/src/app.py @@ -527,6 +527,8 @@ def app_upgrade(app=[], url=None, file=None, force=False, no_safety_backup=False if len(apps) > 1: logger.info(m18n.n("app_upgrade_several_apps", apps=", ".join(apps))) + notifications = {} + for number, app_instance_name in enumerate(apps): logger.info(m18n.n("app_upgrade_app_name", app=app_instance_name)) @@ -794,11 +796,8 @@ def app_upgrade(app=[], url=None, file=None, force=False, no_safety_backup=False # So much win logger.success(m18n.n("app_upgraded", app=app_instance_name)) - # Display post-upgrade notifications and ask for simple confirm - if ( - manifest["notifications"]["post_upgrade"] - and Moulinette.interface.type == "cli" - ): + # Format post-upgrade notifications + if manifest["notifications"]["post_upgrade"]: # Get updated settings to hydrate notifications settings = _get_app_settings(app_instance_name) notifications = _filter_and_hydrate_notifications( @@ -806,8 +805,9 @@ def app_upgrade(app=[], url=None, file=None, force=False, no_safety_backup=False current_version=app_current_version, data=settings, ) - _display_notifications(notifications, force=force) - # FIXME Should return a response with post upgrade notif formatted with the newest settings to the web-admin + if Moulinette.interface.type == "cli": + # ask for simple confirm + _display_notifications(notifications, force=force) hook_callback("post_app_upgrade", env=env_dict) operation_logger.success() @@ -816,6 +816,9 @@ def app_upgrade(app=[], url=None, file=None, force=False, no_safety_backup=False logger.success(m18n.n("upgrade_complete")) + if Moulinette.interface.type == "api": + return {"notifications": {"post_upgrade": notifications}} + def app_manifest(app, with_screenshot=False): @@ -2921,7 +2924,7 @@ def _ask_confirmation( """ if force or Moulinette.interface.type == "api": return - + # If ran from the CLI in a non-interactive context, # skip confirmation (except in hard mode) if not os.isatty(1) and kind in ["simple", "soft"]: From 57c36a668de96aebb43f78ee5abbf2480df15af9 Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Wed, 7 Dec 2022 21:12:54 +0100 Subject: [PATCH 26/43] appv2: for app v1 backward compat, don't set arbitrary values for ram/disk usage, use '?' instead --- src/app.py | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/src/app.py b/src/app.py index 1a8849ed0..468daaecc 100644 --- a/src/app.py +++ b/src/app.py @@ -2092,12 +2092,12 @@ def _convert_v1_manifest_to_v2(manifest): .replace(">", "") .replace("=", "") .replace(" ", ""), - "architectures": "all", + "architectures": "?", "multi_instance": manifest.get("multi_instance", False), "ldap": "?", "sso": "?", - "disk": "50M", - "ram": {"build": "50M", "runtime": "10M"}, + "disk": "?", + "ram": {"build": "?", "runtime": "?"}, } maintainers = manifest.get("maintainer", {}) @@ -2505,7 +2505,7 @@ def _check_manifest_requirements( yield ( "arch", - arch_requirement == "all" or arch in arch_requirement, + arch_requirement in ["all", "?"] or arch in arch_requirement, {"current": arch, "required": arch_requirement}, "app_arch_not_supported", # i18n: app_arch_not_supported ) @@ -2529,12 +2529,15 @@ def _check_manifest_requirements( # Disk if action == "install": - disk_req_bin = human_to_binary(manifest["integration"]["disk"]) root_free_space = free_space_in_directory("/") var_free_space = free_space_in_directory("/var") - has_enough_disk = ( - root_free_space > disk_req_bin and var_free_space > disk_req_bin - ) + if manifest["integration"]["disk"] == "?": + has_enough_disk = True + else: + disk_req_bin = human_to_binary(manifest["integration"]["disk"]) + has_enough_disk = ( + root_free_space > disk_req_bin and var_free_space > disk_req_bin + ) free_space = binary_to_human( root_free_space if root_free_space == var_free_space @@ -2554,8 +2557,8 @@ def _check_manifest_requirements( # Is "include_swap" really useful ? We should probably decide wether to always include it or not instead if ram_requirement.get("include_swap", False): ram += swap - can_build = ram > human_to_binary(ram_requirement["build"]) - can_run = ram > human_to_binary(ram_requirement["runtime"]) + can_build = ram_requirement["build"] == "?" or ram > human_to_binary(ram_requirement["build"]) + can_run = ram_requirement["runtime"] == "?" or ram > human_to_binary(ram_requirement["runtime"]) yield ( "ram", From cf2e7e1295d9a85fe45ab6cb248b3c930631a862 Mon Sep 17 00:00:00 2001 From: axolotle Date: Wed, 21 Dec 2022 13:04:09 +0100 Subject: [PATCH 27/43] _check_manifest_requirements: raise error for "packaging_format" check --- src/app.py | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/app.py b/src/app.py index 468daaecc..1893febb8 100644 --- a/src/app.py +++ b/src/app.py @@ -2480,12 +2480,8 @@ def _check_manifest_requirements( logger.debug(m18n.n("app_requirements_checking", app=app_id)) # Packaging format - yield ( - "packaging_format", - manifest["packaging_format"] in (1, 2), - {}, - "app_packaging_format_not_supported", # i18n: app_packaging_format_not_supported - ) + if manifest["packaging_format"] not in [1, 2]: + raise YunohostValidationError("app_packaging_format_not_supported") # Yunohost version required_yunohost_version = manifest["integration"].get("yunohost", "4.3").strip(">= ") From 66d99e7fcbf90c8f64af887b93675def3bf55848 Mon Sep 17 00:00:00 2001 From: axolotle Date: Wed, 21 Dec 2022 15:53:05 +0100 Subject: [PATCH 28/43] _check_manifest_requirements: fix "disk" req message with least free space dir --- src/app.py | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/app.py b/src/app.py index 1893febb8..5463fe357 100644 --- a/src/app.py +++ b/src/app.py @@ -2534,11 +2534,7 @@ def _check_manifest_requirements( has_enough_disk = ( root_free_space > disk_req_bin and var_free_space > disk_req_bin ) - free_space = binary_to_human( - root_free_space - if root_free_space == var_free_space - else root_free_space + var_free_space - ) + free_space = binary_to_human(min(root_free_space, var_free_space)) yield ( "disk", From fa2ef3e7ec9d68f6e4ee1f1397962041c7cf229b Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Wed, 21 Dec 2022 20:39:10 +0100 Subject: [PATCH 29/43] appv2: better error handling for app resources provisioning/deprovisioning/update failures --- locales/en.json | 1 + src/app.py | 45 +++++++++++++++--------------------------- src/backup.py | 10 +++------- src/utils/resources.py | 36 +++++++++++++++++++++------------ 4 files changed, 43 insertions(+), 49 deletions(-) diff --git a/locales/en.json b/locales/en.json index 3b51403c8..1d4d37d92 100644 --- a/locales/en.json +++ b/locales/en.json @@ -27,6 +27,7 @@ "app_full_domain_unavailable": "Sorry, this app must be installed on a domain of its own, but other apps are already installed on the domain '{domain}'. You could use a subdomain dedicated to this app instead.", "app_id_invalid": "Invalid app ID", "app_install_failed": "Unable to install {app}: {error}", + "app_resource_failed": "Provisioning, deprovisioning, or updating resources for {app} failed: {error}", "app_install_files_invalid": "These files cannot be installed", "app_install_script_failed": "An error occurred inside the app installation script", "app_label_deprecated": "This command is deprecated! Please use the new command 'yunohost user permission update' to manage the app label.", diff --git a/src/app.py b/src/app.py index 5463fe357..8281f6d43 100644 --- a/src/app.py +++ b/src/app.py @@ -675,13 +675,9 @@ def app_upgrade(app=[], url=None, file=None, force=False, no_safety_backup=False if manifest["packaging_format"] >= 2: from yunohost.utils.resources import AppResourceManager - try: - AppResourceManager( - app_instance_name, wanted=manifest, current=app_dict["manifest"] - ).apply(rollback_if_failure=True) - except Exception: - # FIXME : improve error handling .... - raise + AppResourceManager( + app_instance_name, wanted=manifest, current=app_dict["manifest"] + ).apply(rollback_and_raise_exception_if_failure=True, operation_logger=operation_logger) # Execute the app upgrade script upgrade_failed = True @@ -1038,13 +1034,9 @@ def app_install( if packaging_format >= 2: from yunohost.utils.resources import AppResourceManager - try: - AppResourceManager(app_instance_name, wanted=manifest, current={}).apply( - rollback_if_failure=True - ) - except Exception: - # FIXME : improve error handling .... - raise + AppResourceManager(app_instance_name, wanted=manifest, current={}).apply( + rollback_and_raise_exception_if_failure=True, operation_logger=operation_logger + ) else: # Initialize the main permission for the app # The permission is initialized with no url associated, and with tile disabled @@ -1108,6 +1100,9 @@ def app_install( "Packagers /!\\ This app manually modified some system configuration files! This should not happen! If you need to do so, you should implement a proper conf_regen hook. Those configuration were affected:\n - " + "\n -".join(manually_modified_files_by_app) ) + # Actually forbid this for app packaging >= 2 + if packaging_format >= 2: + broke_the_system = True # If the install failed or broke the system, we remove it if install_failed or broke_the_system: @@ -1157,13 +1152,9 @@ def app_install( if packaging_format >= 2: from yunohost.utils.resources import AppResourceManager - try: - AppResourceManager( - app_instance_name, wanted={}, current=manifest - ).apply(rollback_if_failure=False) - except Exception: - # FIXME : improve error handling .... - raise + AppResourceManager( + app_instance_name, wanted={}, current=manifest + ).apply(rollback_and_raise_exception_if_failure=False) else: # Remove all permission in LDAP for permission_name in user_permission_list()["permissions"].keys(): @@ -1288,15 +1279,11 @@ def app_remove(operation_logger, app, purge=False): packaging_format = manifest["packaging_format"] if packaging_format >= 2: - try: - from yunohost.utils.resources import AppResourceManager + from yunohost.utils.resources import AppResourceManager - AppResourceManager(app, wanted={}, current=manifest).apply( - rollback_if_failure=False - ) - except Exception: - # FIXME : improve error handling .... - raise + AppResourceManager(app, wanted={}, current=manifest).apply( + rollback_and_raise_exception_if_failure=False + ) else: # Remove all permission in LDAP for permission_name in user_permission_list(apps=[app])["permissions"].keys(): diff --git a/src/backup.py b/src/backup.py index 78d52210b..21d499eaf 100644 --- a/src/backup.py +++ b/src/backup.py @@ -1518,13 +1518,9 @@ class RestoreManager: if manifest["packaging_format"] >= 2: from yunohost.utils.resources import AppResourceManager - try: - AppResourceManager( - app_instance_name, wanted=manifest, current={} - ).apply(rollback_if_failure=True) - except Exception: - # FIXME : improve error handling .... - raise + AppResourceManager( + app_instance_name, wanted=manifest, current={} + ).apply(rollback_and_raise_exception_if_failure=True, operation_logger=operation_logger) # Execute the app install script restore_failed = True diff --git a/src/utils/resources.py b/src/utils/resources.py index f48722236..4e8388e61 100644 --- a/src/utils/resources.py +++ b/src/utils/resources.py @@ -36,9 +36,6 @@ logger = getActionLogger("yunohost.app_resources") class AppResourceManager: - # FIXME : add some sort of documentation mechanism - # to create a have a detailed description of each resource behavior - def __init__(self, app: str, current: Dict, wanted: Dict): self.app = app @@ -50,7 +47,7 @@ class AppResourceManager: if "resources" not in self.wanted: self.wanted["resources"] = {} - def apply(self, rollback_if_failure, **context): + def apply(self, rollback_and_raise_exception_if_failure, operation_logger=None, **context): todos = list(self.compute_todos()) completed = [] @@ -69,12 +66,13 @@ class AppResourceManager: elif todo == "update": logger.info(f"Updating {name} ...") new.provision_or_update(context=context) - # FIXME FIXME FIXME : this exception doesnt catch Ctrl+C ?!?! - except Exception as e: + except (KeyboardInterrupt, Exception) as e: exception = e - # FIXME: better error handling ? display stacktrace ? - logger.warning(f"Failed to {todo} for {name} : {e}") - if rollback_if_failure: + if isinstance(e, KeyboardInterrupt): + logger.error(m18n.n("operation_interrupted")) + else: + logger.warning(f"Failed to {todo} {name} : {e}") + if rollback_and_raise_exception_if_failure: rollback = True completed.append((todo, name, old, new)) break @@ -97,12 +95,24 @@ class AppResourceManager: elif todo == "update": logger.info(f"Reverting {name} ...") old.provision_or_update(context=context) - except Exception as e: - # FIXME: better error handling ? display stacktrace ? - logger.error(f"Failed to rollback {name} : {e}") + except (KeyboardInterrupt, Exception) as e: + if isinstance(e, KeyboardInterrupt): + logger.error(m18n.n("operation_interrupted")) + else: + logger.error(f"Failed to rollback {name} : {e}") if exception: - raise exception + if rollback_and_raise_exception_if_failure: + logger.error(m18n.n("app_resource_failed", app=self.app, error=exception)) + if operation_logger: + failure_message_with_debug_instructions = operation_logger.error(str(exception)) + raise YunohostError( + failure_message_with_debug_instructions, raw_msg=True + ) + else: + raise YunohostError(str(exception), raw_msg=True0 + else: + logger.error(exception) def compute_todos(self): From a50e73dc0f41786938d110bbc6f9571c2f88c5fe Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Wed, 21 Dec 2022 22:26:45 +0100 Subject: [PATCH 30/43] app resources: implement permission update --- src/permission.py | 5 +++++ src/utils/resources.py | 33 ++++++++++++++++++--------------- 2 files changed, 23 insertions(+), 15 deletions(-) diff --git a/src/permission.py b/src/permission.py index 801576afd..e451bb74c 100644 --- a/src/permission.py +++ b/src/permission.py @@ -479,6 +479,7 @@ def permission_url( url=None, add_url=None, remove_url=None, + set_url=None, auth_header=None, clear_urls=False, sync_perm=True, @@ -491,6 +492,7 @@ def permission_url( url -- (optional) URL for which access will be allowed/forbidden. add_url -- (optional) List of additional url to add for which access will be allowed/forbidden remove_url -- (optional) List of additional url to remove for which access will be allowed/forbidden + set_url -- (optional) List of additional url to set/replace for which access will be allowed/forbidden auth_header -- (optional) Define for the URL of this permission, if SSOwat pass the authentication header to the application clear_urls -- (optional) Clean all urls (url and additional_urls) """ @@ -556,6 +558,9 @@ def permission_url( new_additional_urls = [u for u in new_additional_urls if u not in remove_url] + if set_url: + new_additional_urls = set_url + if auth_header is None: auth_header = existing_permission["auth_header"] diff --git a/src/utils/resources.py b/src/utils/resources.py index 4e8388e61..f993f4092 100644 --- a/src/utils/resources.py +++ b/src/utils/resources.py @@ -258,7 +258,7 @@ class PermissionsResource(AppResource): ##### Provision/Update: - Delete any permissions that may exist and be related to this app yet is not declared anymore - - Loop over the declared permissions and create them if needed or update them with the new values (FIXME : update ain't implemented yet >_>) + - Loop over the declared permissions and create them if needed or update them with the new values ##### Deprovision: - Delete all permission related to this app @@ -312,7 +312,7 @@ class PermissionsResource(AppResource): from yunohost.permission import ( permission_create, - # permission_url, + permission_url, permission_delete, user_permission_list, user_permission_update, @@ -330,7 +330,8 @@ class PermissionsResource(AppResource): permission_delete(perm, force=True, sync_perm=False) for perm, infos in self.permissions.items(): - if f"{self.app}.{perm}" not in existing_perms: + perm_id = f"{self.app}.{perm}" + if perm_id not in existing_perms: # Use the 'allowed' key from the manifest, # or use the 'init_{perm}_permission' from the install questions # which is temporarily saved as a setting as an ugly hack to pass the info to this piece of code... @@ -340,7 +341,7 @@ class PermissionsResource(AppResource): or [] ) permission_create( - f"{self.app}.{perm}", + perm_id, allowed=init_allowed, # This is why the ugly hack with self.manager exists >_> label=self.manager.wanted["name"] if perm == "main" else perm, @@ -351,17 +352,19 @@ class PermissionsResource(AppResource): ) self.delete_setting(f"init_{perm}_permission") - user_permission_update( - f"{self.app}.{perm}", - show_tile=infos["show_tile"], - protected=infos["protected"], - sync_perm=False, - ) - else: - pass - # FIXME : current implementation of permission_url is hell for - # easy declarativeness of additional_urls >_> ... - # permission_url(f"{self.app}.{perm}", url=infos["url"], auth_header=infos["auth_header"], sync_perm=False) + user_permission_update( + perm_id, + show_tile=infos["show_tile"], + protected=infos["protected"], + sync_perm=False, + ) + permission_url( + perm_id, + url=infos["url"], + set_url=infos["additional_urls"], + auth_header=infos["auth_header"], + sync_perm=False, + ) permission_sync_to_user() From 8ab28849a1ce456024a780eae54a9adadd2d61dc Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Wed, 21 Dec 2022 23:11:09 +0100 Subject: [PATCH 31/43] app resource: handle the --purge logic for data_dir removal --- src/app.py | 2 +- src/utils/resources.py | 34 +++++++++++++++++++++------------- 2 files changed, 22 insertions(+), 14 deletions(-) diff --git a/src/app.py b/src/app.py index 8281f6d43..23a10077f 100644 --- a/src/app.py +++ b/src/app.py @@ -1282,7 +1282,7 @@ def app_remove(operation_logger, app, purge=False): from yunohost.utils.resources import AppResourceManager AppResourceManager(app, wanted={}, current=manifest).apply( - rollback_and_raise_exception_if_failure=False + rollback_and_raise_exception_if_failure=False, purge_data_dir=purge ) else: # Remove all permission in LDAP diff --git a/src/utils/resources.py b/src/utils/resources.py index f993f4092..6f5462312 100644 --- a/src/utils/resources.py +++ b/src/utils/resources.py @@ -536,6 +536,8 @@ class InstalldirAppResource(AppResource): if not current_install_dir and os.path.isdir(self.dir): rm(self.dir, recursive=True) + # isdir will be True if the path is a symlink pointing to a dir + # This should cover cases where people moved the data dir to another place via a symlink (ie we dont enter the if) if not os.path.isdir(self.dir): # Handle case where install location changed, in which case we shall move the existing install dir # FIXME: confirm that's what we wanna do @@ -564,8 +566,10 @@ class InstalldirAppResource(AppResource): perm_octal = 0o100 * owner_perm_octal + 0o010 * group_perm_octal - chmod(self.dir, perm_octal) - chown(self.dir, owner, group) + # NB: we use realpath here to cover cases where self.dir could actually be a symlink + # in which case we want to apply the perm to the pointed dir, not to the symlink + chmod(os.path.realpath(self.dir), perm_octal) + chown(os.path.realpath(self.dir), owner, group) # FIXME: shall we apply permissions recursively ? self.set_setting("install_dir", self.dir) @@ -605,9 +609,8 @@ class DatadirAppResource(AppResource): - save the value of `dir` as `data_dir` in the app's settings, which can be then used by the app scripts (`$data_dir`) and conf templates (`__DATA_DIR__`) ##### Deprovision: - - recursively deletes the directory if it exists - - FIXME: this should only be done if the PURGE option is set - - FIXME: this should also delete the corresponding setting + - (only if the purge option is chosen by the user) recursively deletes the directory if it exists + - also delete the corresponding setting ##### Legacy management: - In the past, the setting may have been called `datadir`. The code will automatically rename it as `data_dir`. @@ -641,11 +644,15 @@ class DatadirAppResource(AppResource): current_data_dir = self.get_setting("data_dir") or self.get_setting("datadir") + # isdir will be True if the path is a symlink pointing to a dir + # This should cover cases where people moved the data dir to another place via a symlink (ie we dont enter the if) if not os.path.isdir(self.dir): # Handle case where install location changed, in which case we shall move the existing install dir # FIXME: same as install_dir, is this what we want ? - # FIXME: What if people manually mved the data dir and changed the setting value and dont want the folder to be moved ? x_x if current_data_dir and os.path.isdir(current_data_dir): + logger.warning( + f"Moving {current_data_dir} to {self.dir} ... (this may take a while)" + ) shutil.move(current_data_dir, self.dir) else: mkdir(self.dir) @@ -664,8 +671,10 @@ class DatadirAppResource(AppResource): ) perm_octal = 0o100 * owner_perm_octal + 0o010 * group_perm_octal - chmod(self.dir, perm_octal) - chown(self.dir, owner, group) + # NB: we use realpath here to cover cases where self.dir could actually be a symlink + # in which case we want to apply the perm to the pointed dir, not to the symlink + chmod(os.path.realpath(self.dir), perm_octal) + chown(os.path.realpath(self.dir), owner, group) self.set_setting("data_dir", self.dir) self.delete_setting("datadir") # Legacy @@ -676,11 +685,10 @@ class DatadirAppResource(AppResource): assert self.owner.strip() assert self.group.strip() - # FIXME: This should rm the datadir only if purge is enabled - pass - # if os.path.isdir(self.dir): - # rm(self.dir, recursive=True) - # FIXME : in fact we should delete settings to be consistent + if context.get("purge_data_dir", False) and os.path.isdir(self.dir): + rm(self.dir, recursive=True) + + self.delete_setting("data_dir") class AptDependenciesAppResource(AppResource): From 4775b40b95984a879d60e3cb4061da2be3c915dc Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Thu, 22 Dec 2022 01:15:54 +0100 Subject: [PATCH 32/43] Hmpf --- src/utils/resources.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/utils/resources.py b/src/utils/resources.py index 6f5462312..813bf9979 100644 --- a/src/utils/resources.py +++ b/src/utils/resources.py @@ -22,6 +22,7 @@ import shutil import random from typing import Dict, Any, List +from moulinette import m18n from moulinette.utils.process import check_output from moulinette.utils.log import getActionLogger from moulinette.utils.filesystem import mkdir, chown, chmod, write_to_file @@ -110,7 +111,7 @@ class AppResourceManager: failure_message_with_debug_instructions, raw_msg=True ) else: - raise YunohostError(str(exception), raw_msg=True0 + raise YunohostError(str(exception), raw_msg=True) else: logger.error(exception) From ed77bcc29a9b8a640eb9544d4e2e3fcf20e04689 Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Thu, 22 Dec 2022 05:05:43 +0100 Subject: [PATCH 33/43] Friskies --- src/tests/test_app_resources.py | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/src/tests/test_app_resources.py b/src/tests/test_app_resources.py index cbfed78cb..779a5f7a2 100644 --- a/src/tests/test_app_resources.py +++ b/src/tests/test_app_resources.py @@ -75,7 +75,7 @@ def test_provision_dummy(): assert not os.path.exists(dummyfile) AppResourceManager("testapp", current=current, wanted=wanted).apply( - rollback_if_failure=False + rollback_and_raise_exception_if_failure=False ) assert open(dummyfile).read().strip() == "foo" @@ -89,7 +89,7 @@ def test_deprovision_dummy(): assert open(dummyfile).read().strip() == "foo" AppResourceManager("testapp", current=current, wanted=wanted).apply( - rollback_if_failure=False + rollback_and_raise_exception_if_failure=False ) assert not os.path.exists(dummyfile) @@ -101,7 +101,7 @@ def test_provision_dummy_nondefaultvalue(): assert not os.path.exists(dummyfile) AppResourceManager("testapp", current=current, wanted=wanted).apply( - rollback_if_failure=False + rollback_and_raise_exception_if_failure=False ) assert open(dummyfile).read().strip() == "bar" @@ -115,7 +115,7 @@ def test_update_dummy(): assert open(dummyfile).read().strip() == "foo" AppResourceManager("testapp", current=current, wanted=wanted).apply( - rollback_if_failure=False + rollback_and_raise_exception_if_failure=False ) assert open(dummyfile).read().strip() == "bar" @@ -130,7 +130,7 @@ def test_update_dummy_fail(): assert open(dummyfile).read().strip() == "foo" with pytest.raises(Exception): AppResourceManager("testapp", current=current, wanted=wanted).apply( - rollback_if_failure=False + rollback_and_raise_exception_if_failure=False ) assert open(dummyfile).read().strip() == "forbiddenvalue" @@ -145,7 +145,7 @@ def test_update_dummy_failwithrollback(): assert open(dummyfile).read().strip() == "foo" with pytest.raises(Exception): AppResourceManager("testapp", current=current, wanted=wanted).apply( - rollback_if_failure=True + rollback_and_raise_exception_if_failure=True ) assert open(dummyfile).read().strip() == "foo" @@ -397,9 +397,7 @@ def test_resource_permissions(): res = user_permission_list(full=True)["permissions"] - # FIXME FIXME FIXME : this is the current behavior but - # it is NOT okay. c.f. comment in the code - assert res["testapp.admin"]["url"] == "/admin" # should be '/adminpanel' + assert res["testapp.admin"]["url"] == "/adminpanel" r(conf, "testapp").deprovision() From df8f14eec61f050d5c03e384ce52958098185bb1 Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Thu, 22 Dec 2022 20:04:37 +0100 Subject: [PATCH 34/43] app resources: implement logic for port 'exposed' and 'fixed' options --- src/firewall.py | 20 ++++++++++++------ src/tests/test_app_resources.py | 36 +++++++++++++++++++-------------- src/utils/resources.py | 33 ++++++++++++++++++++++-------- 3 files changed, 60 insertions(+), 29 deletions(-) diff --git a/src/firewall.py b/src/firewall.py index 8e0e70e99..eb3c9b009 100644 --- a/src/firewall.py +++ b/src/firewall.py @@ -32,7 +32,7 @@ logger = getActionLogger("yunohost.firewall") def firewall_allow( - protocol, port, ipv4_only=False, ipv6_only=False, no_upnp=False, no_reload=False + protocol, port, ipv4_only=False, ipv6_only=False, no_upnp=False, no_reload=False, reload_only_if_change=False ): """ Allow connections on a port @@ -70,14 +70,18 @@ def firewall_allow( "ipv6", ] + changed = False + for p in protocols: # Iterate over IP versions to add port for i in ipvs: if port not in firewall[i][p]: firewall[i][p].append(port) + changed = True else: ipv = "IPv%s" % i[3] - logger.warning(m18n.n("port_already_opened", port=port, ip_version=ipv)) + if not reload_only_if_change: + logger.warning(m18n.n("port_already_opened", port=port, ip_version=ipv)) # Add port forwarding with UPnP if not no_upnp and port not in firewall["uPnP"][p]: firewall["uPnP"][p].append(port) @@ -89,12 +93,12 @@ def firewall_allow( # Update and reload firewall _update_firewall_file(firewall) - if not no_reload: + if not no_reload or (reload_only_if_change and changed): return firewall_reload() def firewall_disallow( - protocol, port, ipv4_only=False, ipv6_only=False, upnp_only=False, no_reload=False + protocol, port, ipv4_only=False, ipv6_only=False, upnp_only=False, no_reload=False, reload_only_if_change=False ): """ Disallow connections on a port @@ -139,14 +143,18 @@ def firewall_disallow( elif upnp_only: ipvs = [] + changed = False + for p in protocols: # Iterate over IP versions to remove port for i in ipvs: if port in firewall[i][p]: firewall[i][p].remove(port) + changed = True else: ipv = "IPv%s" % i[3] - logger.warning(m18n.n("port_already_closed", port=port, ip_version=ipv)) + if not reload_only_if_change: + logger.warning(m18n.n("port_already_closed", port=port, ip_version=ipv)) # Remove port forwarding with UPnP if upnp and port in firewall["uPnP"][p]: firewall["uPnP"][p].remove(port) @@ -156,7 +164,7 @@ def firewall_disallow( # Update and reload firewall _update_firewall_file(firewall) - if not no_reload: + if not no_reload or (reload_only_if_change and changed): return firewall_reload() diff --git a/src/tests/test_app_resources.py b/src/tests/test_app_resources.py index 779a5f7a2..879f6e29a 100644 --- a/src/tests/test_app_resources.py +++ b/src/tests/test_app_resources.py @@ -11,6 +11,7 @@ from yunohost.utils.resources import ( AppResourceClassesByType, ) from yunohost.permission import user_permission_list, permission_delete +from yunohost.firewall import firewall_list dummyfile = "/tmp/dummyappresource-testapp" @@ -120,21 +121,6 @@ def test_update_dummy(): assert open(dummyfile).read().strip() == "bar" -def test_update_dummy_fail(): - - current = {"resources": {"dummy": {}}} - wanted = {"resources": {"dummy": {"content": "forbiddenvalue"}}} - - open(dummyfile, "w").write("foo") - - assert open(dummyfile).read().strip() == "foo" - with pytest.raises(Exception): - AppResourceManager("testapp", current=current, wanted=wanted).apply( - rollback_and_raise_exception_if_failure=False - ) - assert open(dummyfile).read().strip() == "forbiddenvalue" - - def test_update_dummy_failwithrollback(): current = {"resources": {"dummy": {}}} @@ -276,6 +262,26 @@ def test_resource_ports_several(): assert not app_setting("testapp", "port_foobar") +def test_resource_ports_firewall(): + + r = AppResourceClassesByType["ports"] + conf = {"main": {"default": 12345}} + + r(conf, "testapp").provision_or_update() + + assert 12345 not in firewall_list()["opened_ports"] + + conf = {"main": {"default": 12345, "exposed": "TCP"}} + + r(conf, "testapp").provision_or_update() + + assert 12345 in firewall_list()["opened_ports"] + + r(conf, "testapp").deprovision() + + assert 12345 not in firewall_list()["opened_ports"] + + def test_resource_database(): r = AppResourceClassesByType["database"] diff --git a/src/utils/resources.py b/src/utils/resources.py index 813bf9979..4efefc576 100644 --- a/src/utils/resources.py +++ b/src/utils/resources.py @@ -778,16 +778,16 @@ class PortsResource(AppResource): ##### Properties (for every port name): - `default`: The prefered value for the port. If this port is already being used by another process right now, or is booked in another app's setting, the code will increment the value until it finds a free port and store that value as the setting. If no value is specified, a random value between 10000 and 60000 is used. - - `exposed`: (default: `false`) Wether this port should be opened on the firewall and be publicly reachable. This should be kept to `false` for the majority of apps than only need a port for internal reverse-proxying! Possible values: `false`, `true`(=`Both`), `Both`, `TCP`, `UDP`. This will result in the port being opened on the firewall, and the diagnosis checking that a program answers on that port. (FIXME: this is not implemented yet) - - `fixed`: (default: `false`) Tells that the app absolutely needs the specific value provided in `default`, typically because it's needed for a specific protocol (FIXME: this is not implemented yet) + - `exposed`: (default: `false`) Wether this port should be opened on the firewall and be publicly reachable. This should be kept to `false` for the majority of apps than only need a port for internal reverse-proxying! Possible values: `false`, `true`(=`Both`), `Both`, `TCP`, `UDP`. This will result in the port being opened on the firewall, and the diagnosis checking that a program answers on that port. + - `fixed`: (default: `false`) Tells that the app absolutely needs the specific value provided in `default`, typically because it's needed for a specific protocol ##### Provision/Update (for every port name): - If not already booked, look for a free port, starting with the `default` value (or a random value between 10000 and 60000 if no `default` set) - - (FIXME) If `exposed` is not `false`, open the port in the firewall accordingly - otherwise make sure it's closed. + - If `exposed` is not `false`, open the port in the firewall accordingly - otherwise make sure it's closed. - The value of the port is stored in the `$port` setting for the `main` port, or `$port_NAME` for other `NAME`s ##### Deprovision: - - (FIXME) Close the ports on the firewall + - Close the ports on the firewall if relevant - Deletes all the port settings ##### Legacy management: @@ -806,8 +806,8 @@ class PortsResource(AppResource): default_port_properties = { "default": None, - "exposed": False, # or True(="Both"), "TCP", "UDP" # FIXME : implement logic for exposed port (allow/disallow in firewall ?) - "fixed": False, # FIXME: implement logic. Corresponding to wether or not the port is "fixed" or any random port is ok + "exposed": False, # or True(="Both"), "TCP", "UDP" + "fixed": False, } ports: Dict[str, Dict[str, Any]] @@ -839,6 +839,8 @@ class PortsResource(AppResource): def provision_or_update(self, context: Dict = {}): + from yunohost.firewall import firewall_allow, firewall_disallow + for name, infos in self.ports.items(): setting_name = f"port_{name}" if name != "main" else "port" @@ -854,16 +856,31 @@ class PortsResource(AppResource): if not port_value: port_value = infos["default"] - while self._port_is_used(port_value): - port_value += 1 + + if infos["fixed"]: + if self._port_is_used(port_value): + raise ValidationError(f"Port {port_value} is already used by another process or app.") + else: + while self._port_is_used(port_value): + port_value += 1 self.set_setting(setting_name, port_value) + if infos["exposed"]: + firewall_allow(infos["exposed"], port_value, reload_only_if_change=True) + else: + firewall_disallow(infos["exposed"], port_value, reload_only_if_change=True) + def deprovision(self, context: Dict = {}): + from yunohost.firewall import firewall_disallow + for name, infos in self.ports.items(): setting_name = f"port_{name}" if name != "main" else "port" + value = self.get_setting(setting_name) self.delete_setting(setting_name) + if value and str(value).strip(): + firewall_disallow(infos["exposed"], int(value), reload_only_if_change=True) class DatabaseAppResource(AppResource): From 6d4659a782f58f3303fb4f3d0af8246f42f69546 Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Thu, 22 Dec 2022 20:05:04 +0100 Subject: [PATCH 35/43] app resources: fix ambiguity for db resource 'type' property --- src/utils/resources.py | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/src/utils/resources.py b/src/utils/resources.py index 4efefc576..e1c6e75f4 100644 --- a/src/utils/resources.py +++ b/src/utils/resources.py @@ -922,7 +922,7 @@ class DatabaseAppResource(AppResource): priority = 90 default_properties: Dict[str, Any] = { - "type": None, # FIXME: eeeeeeeh is this really a good idea considering 'type' is supposed to be the resource type x_x + "dbtype": None, } def __init__(self, properties: Dict[str, Any], *args, **kwargs): @@ -935,13 +935,18 @@ class DatabaseAppResource(AppResource): "Specifying the type of db ('mysql' or 'postgresql') is mandatory for db resources" ) + # Hack so that people can write type = "mysql/postgresql" in toml but it's loaded as dbtype + # to avoid conflicting with the generic self.type of the resource object ... + # dunno if that's really a good idea :| + properties["dbtype"] = properties.pop("type") + super().__init__(properties, *args, **kwargs) def db_exists(self, db_name): - if self.type == "mysql": + if self.dbtype == "mysql": return os.system(f"mysqlshow '{db_name}' >/dev/null 2>/dev/null") == 0 - elif self.type == "postgresql": + elif self.dbtype == "postgresql": return ( os.system( f"sudo --login --user=postgres psql -c '' '{db_name}' >/dev/null 2>/dev/null" @@ -965,7 +970,7 @@ class DatabaseAppResource(AppResource): else: # Legacy setting migration legacypasswordsetting = ( - "psqlpwd" if self.type == "postgresql" else "mysqlpwd" + "psqlpwd" if self.dbtype == "postgresql" else "mysqlpwd" ) if self.get_setting(legacypasswordsetting): db_pwd = self.get_setting(legacypasswordsetting) @@ -980,12 +985,12 @@ class DatabaseAppResource(AppResource): if not self.db_exists(db_name): - if self.type == "mysql": + if self.dbtype == "mysql": self._run_script( "provision", f"ynh_mysql_create_db '{db_name}' '{db_user}' '{db_pwd}'", ) - elif self.type == "postgresql": + elif self.dbtype == "postgresql": self._run_script( "provision", f"ynh_psql_create_user '{db_user}' '{db_pwd}'; ynh_psql_create_db '{db_name}' '{db_user}'", @@ -996,11 +1001,11 @@ class DatabaseAppResource(AppResource): db_name = self.app.replace("-", "_").replace(".", "_") db_user = db_name - if self.type == "mysql": + if self.dbtype == "mysql": self._run_script( "deprovision", f"ynh_mysql_remove_db '{db_name}' '{db_user}'" ) - elif self.type == "postgresql": + elif self.dbtype == "postgresql": self._run_script( "deprovision", f"ynh_psql_remove_db '{db_name}' '{db_user}'" ) From 7d9984c109856b6e1999ce92b0ed038fb92ae863 Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Mon, 26 Dec 2022 15:37:28 +0100 Subject: [PATCH 36/43] Fix a bunch of inconsistency or variable not properly replaced between final_path/install_dir --- helpers/config | 12 ++--- helpers/php | 6 +-- helpers/utils | 7 ++- src/app.py | 1 + tests/test_helpers.d/ynhtest_setup_source.sh | 46 ++++++++++---------- 5 files changed, 39 insertions(+), 33 deletions(-) diff --git a/helpers/config b/helpers/config index fce215a30..77f118c5f 100644 --- a/helpers/config +++ b/helpers/config @@ -22,7 +22,7 @@ _ynh_app_config_get_one() { if [[ "$bind" == "settings" ]]; then ynh_die --message="File '${short_setting}' can't be stored in settings" fi - old[$short_setting]="$(ls "$(echo $bind | sed s@__FINALPATH__@$final_path@ | sed s/__APP__/$app/)" 2>/dev/null || echo YNH_NULL)" + old[$short_setting]="$(ls "$(echo $bind | sed s@__INSTALL_DIR__@$install_dir@ | sed s@__FINALPATH__@$final_path@ | sed s/__APP__/$app/)" 2>/dev/null || echo YNH_NULL)" file_hash[$short_setting]="true" # Get multiline text from settings or from a full file @@ -32,7 +32,7 @@ _ynh_app_config_get_one() { elif [[ "$bind" == *":"* ]]; then ynh_die --message="For technical reasons, multiline text '${short_setting}' can't be stored automatically in a variable file, you have to create custom getter/setter" else - old[$short_setting]="$(cat $(echo $bind | sed s@__FINALPATH__@$final_path@ | sed s/__APP__/$app/) 2>/dev/null || echo YNH_NULL)" + old[$short_setting]="$(cat $(echo $bind | sed s@__INSTALL_DIR__@$install_dir@ | sed s@__FINALPATH__@$final_path@ | sed s/__APP__/$app/) 2>/dev/null || echo YNH_NULL)" fi # Get value from a kind of key/value file @@ -47,7 +47,7 @@ _ynh_app_config_get_one() { bind_after="$(echo "${bind_key_}" | cut -d'>' -f1)" bind_key_="$(echo "${bind_key_}" | cut -d'>' -f2)" fi - local bind_file="$(echo "$bind" | cut -d: -f2 | sed s@__FINALPATH__@$final_path@ | sed s/__APP__/$app/)" + local bind_file="$(echo "$bind" | cut -d: -f2 | sed s@__INSTALL_DIR__@$install_dir@ | sed s@__FINALPATH__@$final_path@ | sed s/__APP__/$app/)" old[$short_setting]="$(ynh_read_var_in_file --file="${bind_file}" --key="${bind_key_}" --after="${bind_after}")" fi @@ -73,7 +73,7 @@ _ynh_app_config_apply_one() { if [[ "$bind" == "settings" ]]; then ynh_die --message="File '${short_setting}' can't be stored in settings" fi - local bind_file="$(echo "$bind" | sed s@__FINALPATH__@$final_path@ | sed s/__APP__/$app/)" + local bind_file="$(echo "$bind" | sed s@__INSTALL_DIR__@$install_dir@ | sed s@__FINALPATH__@$final_path@ | sed s/__APP__/$app/)" if [[ "${!short_setting}" == "" ]]; then ynh_backup_if_checksum_is_different --file="$bind_file" ynh_secure_remove --file="$bind_file" @@ -98,7 +98,7 @@ _ynh_app_config_apply_one() { if [[ "$bind" == *":"* ]]; then ynh_die --message="For technical reasons, multiline text '${short_setting}' can't be stored automatically in a variable file, you have to create custom getter/setter" fi - local bind_file="$(echo "$bind" | sed s@__FINALPATH__@$final_path@ | sed s/__APP__/$app/)" + local bind_file="$(echo "$bind" | sed s@__INSTALL_DIR__@$install_dir@ | sed s@__FINALPATH__@$final_path@ | sed s/__APP__/$app/)" ynh_backup_if_checksum_is_different --file="$bind_file" echo "${!short_setting}" >"$bind_file" ynh_store_file_checksum --file="$bind_file" --update_only @@ -113,7 +113,7 @@ _ynh_app_config_apply_one() { bind_after="$(echo "${bind_key_}" | cut -d'>' -f1)" bind_key_="$(echo "${bind_key_}" | cut -d'>' -f2)" fi - local bind_file="$(echo "$bind" | cut -d: -f2 | sed s@__FINALPATH__@$final_path@ | sed s/__APP__/$app/)" + local bind_file="$(echo "$bind" | cut -d: -f2 | sed s@__INSTALL_DIR__@$install_dir@ | sed s@__FINALPATH__@$final_path@ | sed s/__APP__/$app/)" ynh_backup_if_checksum_is_different --file="$bind_file" ynh_write_var_in_file --file="${bind_file}" --key="${bind_key_}" --value="${!short_setting}" --after="${bind_after}" diff --git a/helpers/php b/helpers/php index da833ae9e..6119c4870 100644 --- a/helpers/php +++ b/helpers/php @@ -474,9 +474,9 @@ YNH_COMPOSER_VERSION=${YNH_COMPOSER_VERSION:-$YNH_DEFAULT_COMPOSER_VERSION} # Execute a command with Composer # -# usage: ynh_composer_exec [--phpversion=phpversion] [--workdir=$final_path] --commands="commands" +# usage: ynh_composer_exec [--phpversion=phpversion] [--workdir=$install_dir] --commands="commands" # | arg: -v, --phpversion - PHP version to use with composer -# | arg: -w, --workdir - The directory from where the command will be executed. Default $final_path. +# | arg: -w, --workdir - The directory from where the command will be executed. Default $install_dir or $final_path # | arg: -c, --commands - Commands to execute. # # Requires YunoHost version 4.2 or higher. @@ -489,7 +489,7 @@ ynh_composer_exec() { local commands # Manage arguments with getopts ynh_handle_getopts_args "$@" - workdir="${workdir:-$final_path}" + workdir="${workdir:-${install_dir:-$final_path}}" phpversion="${phpversion:-$YNH_PHP_VERSION}" COMPOSER_HOME="$workdir/.composer" COMPOSER_MEMORY_LIMIT=-1 \ diff --git a/helpers/utils b/helpers/utils index 3b1e9c6bb..344493ff3 100644 --- a/helpers/utils +++ b/helpers/utils @@ -192,6 +192,9 @@ ynh_setup_source() { # Extract source into the app dir mkdir --parents "$dest_dir" + if [ -n "${install_dir:-}" ] && [ "$dest_dir" == "$install_dir" ]; then + _ynh_apply_default_permissions $dest_dir + fi if [ -n "${final_path:-}" ] && [ "$dest_dir" == "$final_path" ]; then _ynh_apply_default_permissions $dest_dir fi @@ -330,7 +333,7 @@ ynh_local_curl() { # | arg: -d, --destination= - Destination of the config file # # examples: -# ynh_add_config --template=".env" --destination="$final_path/.env" use the template file "../conf/.env" +# ynh_add_config --template=".env" --destination="$install_dir/.env" use the template file "../conf/.env" # ynh_add_config --template="/etc/nginx/sites-available/default" --destination="etc/nginx/sites-available/mydomain.conf" # # The template can be by default the name of a file in the conf directory @@ -444,8 +447,10 @@ ynh_replace_vars() { ynh_replace_string --match_string="__NAMETOCHANGE__" --replace_string="$app" --target_file="$file" ynh_replace_string --match_string="__USER__" --replace_string="$app" --target_file="$file" fi + # Legacy if test -n "${final_path:-}"; then ynh_replace_string --match_string="__FINALPATH__" --replace_string="$final_path" --target_file="$file" + ynh_replace_string --match_string="__INSTALL_DIR__" --replace_string="$final_path" --target_file="$file" fi if test -n "${YNH_PHP_VERSION:-}"; then ynh_replace_string --match_string="__PHPVERSION__" --replace_string="$YNH_PHP_VERSION" --target_file="$file" diff --git a/src/app.py b/src/app.py index 23a10077f..af7822308 100644 --- a/src/app.py +++ b/src/app.py @@ -1762,6 +1762,7 @@ ynh_app_config_run $1 "app": app, "app_instance_nb": str(app_instance_nb), "final_path": settings.get("final_path", ""), + "install_dir": settings.get("install_dir", ""), "YNH_APP_BASEDIR": os.path.join(APPS_SETTING_PATH, app), } ) diff --git a/tests/test_helpers.d/ynhtest_setup_source.sh b/tests/test_helpers.d/ynhtest_setup_source.sh index fe61e7401..6a74a587c 100644 --- a/tests/test_helpers.d/ynhtest_setup_source.sh +++ b/tests/test_helpers.d/ynhtest_setup_source.sh @@ -18,51 +18,51 @@ _make_dummy_src() { } ynhtest_setup_source_nominal() { - final_path="$(mktemp -d -p $VAR_WWW)" + install_dir="$(mktemp -d -p $VAR_WWW)" _make_dummy_src > ../conf/dummy.src - ynh_setup_source --dest_dir="$final_path" --source_id="dummy" + ynh_setup_source --dest_dir="$install_dir" --source_id="dummy" - test -e "$final_path" - test -e "$final_path/index.html" + test -e "$install_dir" + test -e "$install_dir/index.html" } ynhtest_setup_source_nominal_upgrade() { - final_path="$(mktemp -d -p $VAR_WWW)" + install_dir="$(mktemp -d -p $VAR_WWW)" _make_dummy_src > ../conf/dummy.src - ynh_setup_source --dest_dir="$final_path" --source_id="dummy" + ynh_setup_source --dest_dir="$install_dir" --source_id="dummy" - test "$(cat $final_path/index.html)" == "Lorem Ipsum" + test "$(cat $install_dir/index.html)" == "Lorem Ipsum" # Except index.html to get overwritten during next ynh_setup_source - echo "IEditedYou!" > $final_path/index.html - test "$(cat $final_path/index.html)" == "IEditedYou!" + echo "IEditedYou!" > $install_dir/index.html + test "$(cat $install_dir/index.html)" == "IEditedYou!" - ynh_setup_source --dest_dir="$final_path" --source_id="dummy" + ynh_setup_source --dest_dir="$install_dir" --source_id="dummy" - test "$(cat $final_path/index.html)" == "Lorem Ipsum" + test "$(cat $install_dir/index.html)" == "Lorem Ipsum" } ynhtest_setup_source_with_keep() { - final_path="$(mktemp -d -p $VAR_WWW)" + install_dir="$(mktemp -d -p $VAR_WWW)" _make_dummy_src > ../conf/dummy.src - echo "IEditedYou!" > $final_path/index.html - echo "IEditedYou!" > $final_path/test.txt + echo "IEditedYou!" > $install_dir/index.html + echo "IEditedYou!" > $install_dir/test.txt - ynh_setup_source --dest_dir="$final_path" --source_id="dummy" --keep="index.html test.txt" + ynh_setup_source --dest_dir="$install_dir" --source_id="dummy" --keep="index.html test.txt" - test -e "$final_path" - test -e "$final_path/index.html" - test -e "$final_path/test.txt" - test "$(cat $final_path/index.html)" == "IEditedYou!" - test "$(cat $final_path/test.txt)" == "IEditedYou!" + test -e "$install_dir" + test -e "$install_dir/index.html" + test -e "$install_dir/test.txt" + test "$(cat $install_dir/index.html)" == "IEditedYou!" + test "$(cat $install_dir/test.txt)" == "IEditedYou!" } ynhtest_setup_source_with_patch() { - final_path="$(mktemp -d -p $VAR_WWW)" + install_dir="$(mktemp -d -p $VAR_WWW)" _make_dummy_src > ../conf/dummy.src mkdir -p ../sources/patches @@ -74,7 +74,7 @@ ynhtest_setup_source_with_patch() { +Lorem Ipsum dolor sit amet EOF - ynh_setup_source --dest_dir="$final_path" --source_id="dummy" + ynh_setup_source --dest_dir="$install_dir" --source_id="dummy" - test "$(cat $final_path/index.html)" == "Lorem Ipsum dolor sit amet" + test "$(cat $install_dir/index.html)" == "Lorem Ipsum dolor sit amet" } From e9b5ec90a4f49482e752bf8bf5112991f04d27aa Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Tue, 3 Jan 2023 00:46:14 +0100 Subject: [PATCH 37/43] Yoloimplementation of app logo support (require change in app catalog build) --- conf/nginx/yunohost_admin.conf.inc | 4 ++++ src/app.py | 4 ++++ src/app_catalog.py | 36 ++++++++++++++++++++++++++++++ 3 files changed, 44 insertions(+) diff --git a/conf/nginx/yunohost_admin.conf.inc b/conf/nginx/yunohost_admin.conf.inc index b5eff7a5e..84c49d30b 100644 --- a/conf/nginx/yunohost_admin.conf.inc +++ b/conf/nginx/yunohost_admin.conf.inc @@ -19,6 +19,10 @@ location /yunohost/admin/ { more_set_headers "Cache-Control: no-store, no-cache, must-revalidate"; } + location /yunohost/admin/applogos/ { + alias /usr/share/yunohost/applogos/; + } + more_set_headers "Content-Security-Policy: upgrade-insecure-requests; default-src 'self'; connect-src 'self' https://paste.yunohost.org wss://$host; style-src 'self' 'unsafe-inline'; script-src 'self' 'unsafe-eval'; object-src 'none'; img-src 'self' data:;"; more_set_headers "Content-Security-Policy-Report-Only:"; } diff --git a/src/app.py b/src/app.py index af7822308..842c71b80 100644 --- a/src/app.py +++ b/src/app.py @@ -71,6 +71,7 @@ from yunohost.app_catalog import ( # noqa app_catalog, app_search, _load_apps_catalog, + APPS_CATALOG_LOGOS, ) logger = getActionLogger("yunohost.app") @@ -151,6 +152,9 @@ def app_info(app, full=False, upgradable=False): absolute_app_name, _ = _parse_app_instance_name(app) from_catalog = _load_apps_catalog()["apps"].get(absolute_app_name, {}) + # Check if $app.png exists in the app logo folder, this is a trick to be able to easily customize the logo + # of an app just by creating $app.png (instead of the hash.png) in the corresponding folder + ret["logo"] = app if os.path.exists(f"{APPS_CATALOG_LOGOS}/{app}.png") else from_catalog.get("logo_hash") ret["upgradable"] = _app_upgradable({**ret, "from_catalog": from_catalog}) if ret["upgradable"] == "yes": diff --git a/src/app_catalog.py b/src/app_catalog.py index 22a878579..79d02e8f1 100644 --- a/src/app_catalog.py +++ b/src/app_catalog.py @@ -18,6 +18,7 @@ # import os import re +import hashlib from moulinette import m18n from moulinette.utils.log import getActionLogger @@ -36,6 +37,7 @@ from yunohost.utils.error import YunohostError logger = getActionLogger("yunohost.app_catalog") APPS_CATALOG_CACHE = "/var/cache/yunohost/repo" +APPS_CATALOG_LOGOS = "/usr/share/yunohost/applogos" APPS_CATALOG_CONF = "/etc/yunohost/apps_catalog.yml" APPS_CATALOG_API_VERSION = 3 APPS_CATALOG_DEFAULT_URL = "https://app.yunohost.org/default" @@ -182,6 +184,9 @@ def _update_apps_catalog(): logger.debug("Initialize folder for apps catalog cache") mkdir(APPS_CATALOG_CACHE, mode=0o750, parents=True, uid="root") + if not os.path.exists(APPS_CATALOG_LOGOS): + mkdir(APPS_CATALOG_LOGOS, mode=0o755, parents=True, uid="root") + for apps_catalog in apps_catalog_list: if apps_catalog["url"] is None: continue @@ -212,6 +217,37 @@ def _update_apps_catalog(): raw_msg=True, ) + # Download missing app logos + logos_to_download = [] + for app, infos in apps_catalog_content["apps"].items(): + logo_hash = infos.get("logo_hash") + if not logo_hash or os.path.exists(f"{APPS_CATALOG_LOGOS}/{logo_hash}.png"): + continue + logos_to_download.append(logo_hash) + + if len(logos_to_download) > 20: + logger.info(f"(Will fetch {len(logos_to_download)} logos, this may take a couple minutes)") + + import requests + from multiprocessing.pool import ThreadPool + + def fetch_logo(logo_hash): + try: + r = requests.get(f"{apps_catalog['url']}/v{APPS_CATALOG_API_VERSION}/logos/{logo_hash}.png", timeout=10) + assert r.status_code == 200, f"Got status code {r.status_code}, expected 200" + if hashlib.sha256(r.content).hexdigest() != logo_hash: + raise Exception(f"Found inconsistent hash while downloading logo {logo_hash}") + open(f"{APPS_CATALOG_LOGOS}/{logo_hash}.png", "wb").write(r.content) + return True + except Exception as e: + logger.debug(f"Failed to download logo {logo_hash} : {e}") + return False + + results = ThreadPool(8).imap_unordered(fetch_logo, logos_to_download) + for result in results: + # Is this even needed to iterate on the results ? + pass + logger.success(m18n.n("apps_catalog_update_success")) From 2f1ddb1edf8df1680eaae8e9a3dfab3f4b07e918 Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Wed, 4 Jan 2023 01:16:47 +0100 Subject: [PATCH 38/43] appv2: simplify the expected notification file/folder structure in apps --- src/app.py | 34 ++++++++++++++++++++-------------- src/tests/test_apps.py | 20 ++++++++++---------- 2 files changed, 30 insertions(+), 24 deletions(-) diff --git a/src/app.py b/src/app.py index 842c71b80..e8079eb55 100644 --- a/src/app.py +++ b/src/app.py @@ -607,12 +607,12 @@ def app_upgrade(app=[], url=None, file=None, force=False, no_safety_backup=False # Display pre-upgrade notifications and ask for simple confirm if ( - manifest["notifications"]["pre_upgrade"] + manifest["notifications"]["PRE_UPGRADE"] and Moulinette.interface.type == "cli" ): settings = _get_app_settings(app_instance_name) notifications = _filter_and_hydrate_notifications( - manifest["notifications"]["pre_upgrade"], + manifest["notifications"]["PRE_UPGRADE"], current_version=app_current_version, data=settings, ) @@ -797,11 +797,11 @@ def app_upgrade(app=[], url=None, file=None, force=False, no_safety_backup=False logger.success(m18n.n("app_upgraded", app=app_instance_name)) # Format post-upgrade notifications - if manifest["notifications"]["post_upgrade"]: + if manifest["notifications"]["POST_UPGRADE"]: # Get updated settings to hydrate notifications settings = _get_app_settings(app_instance_name) notifications = _filter_and_hydrate_notifications( - manifest["notifications"]["post_upgrade"], + manifest["notifications"]["POST_UPGRADE"], current_version=app_current_version, data=settings, ) @@ -817,7 +817,7 @@ def app_upgrade(app=[], url=None, file=None, force=False, no_safety_backup=False logger.success(m18n.n("upgrade_complete")) if Moulinette.interface.type == "api": - return {"notifications": {"post_upgrade": notifications}} + return {"notifications": {"POST_UPGRADE": notifications}} def app_manifest(app, with_screenshot=False): @@ -927,9 +927,9 @@ def app_install( manifest, extracted_app_folder = _extract_app(app) # Display pre_install notices in cli mode - if manifest["notifications"]["pre_install"] and Moulinette.interface.type == "cli": + if manifest["notifications"]["PRE_INSTALL"] and Moulinette.interface.type == "cli": notifications = _filter_and_hydrate_notifications( - manifest["notifications"]["pre_install"] + manifest["notifications"]["PRE_INSTALL"] ) _display_notifications(notifications, force=force) @@ -1202,7 +1202,7 @@ def app_install( # Get the generated settings to hydrate notifications settings = _get_app_settings(app_instance_name) notifications = _filter_and_hydrate_notifications( - manifest["notifications"]["post_install"], data=settings + manifest["notifications"]["POST_INSTALL"], data=settings ) # Display post_install notices in cli mode @@ -2001,6 +2001,7 @@ def _get_manifest_of_app(path): def _parse_app_doc_and_notifications(path): doc = {} + notification_names = ["PRE_INSTALL", "POST_INSTALL", "PRE_UPGRADE", "POST_UPGRADE"] for filepath in glob.glob(os.path.join(path, "doc") + "/*.md"): @@ -2011,7 +2012,12 @@ def _parse_app_doc_and_notifications(path): if not m: # FIXME: shall we display a warning ? idk continue + pagename, lang = m.groups() + + if pagename in notification_names: + continue + lang = lang.strip("_") if lang else "en" if pagename not in doc: @@ -2020,10 +2026,10 @@ def _parse_app_doc_and_notifications(path): notifications = {} - for step in ["pre_install", "post_install", "pre_upgrade", "post_upgrade"]: + for step in notification_names: notifications[step] = {} for filepath in glob.glob( - os.path.join(path, "doc", "notifications", f"{step}*.md") + os.path.join(path, "doc", f"{step}*.md") ): m = re.match(step + "(_[a-z]{2,3})?.md", filepath.split("/")[-1]) if not m: @@ -2035,7 +2041,7 @@ def _parse_app_doc_and_notifications(path): notifications[step][pagename][lang] = read_file(filepath).strip() for filepath in glob.glob( - os.path.join(path, "doc", "notifications", f"{step}.d") + "/*.md" + os.path.join(path, "doc", f"{step}.d") + "/*.md" ): m = re.match( r"([A-Za-z0-9\.\~]*)(_[a-z]{2,3})?.md", filepath.split("/")[-1] @@ -2403,9 +2409,9 @@ def _list_upgradable_apps(): manifest, extracted_app_folder = _extract_app(absolute_app_name) current_version = version.parse(app["current_version"]) app["notifications"] = {} - if manifest["notifications"]["pre_upgrade"]: - app["notifications"]["pre_upgrade"] = _filter_and_hydrate_notifications( - manifest["notifications"]["pre_upgrade"], + if manifest["notifications"]["PRE_UPGRADE"]: + app["notifications"]["PRE_UPGRADE"] = _filter_and_hydrate_notifications( + manifest["notifications"]["PRE_UPGRADE"], current_version, app["settings"], ) diff --git a/src/tests/test_apps.py b/src/tests/test_apps.py index 6cd52659d..6efdaa0b0 100644 --- a/src/tests/test_apps.py +++ b/src/tests/test_apps.py @@ -223,10 +223,10 @@ def test_legacy_app_manifest_preinstall(): assert "install" in m assert m["doc"] == {} assert m["notifications"] == { - "pre_install": {}, - "pre_upgrade": {}, - "post_install": {}, - "post_upgrade": {}, + "PRE_INSTALL": {}, + "PRE_UPGRADE": {}, + "POST_INSTALL": {}, + "POST_UPGRADE": {}, } @@ -249,11 +249,11 @@ def test_manifestv2_app_manifest_preinstall(): assert "notifications" in m assert ( "This is a dummy disclaimer to display prior to the install" - in m["notifications"]["pre_install"]["main"]["en"] + in m["notifications"]["PRE_INSTALL"]["main"]["en"] ) assert ( "Ceci est un faux disclaimer à présenter avant l'installation" - in m["notifications"]["pre_install"]["main"]["fr"] + in m["notifications"]["PRE_INSTALL"]["main"]["fr"] ) @@ -295,15 +295,15 @@ def test_manifestv2_app_info_postinstall(): assert "notifications" in m assert ( "The app install dir is /var/www/manifestv2_app" - in m["notifications"]["post_install"]["main"]["en"] + in m["notifications"]["POST_INSTALL"]["main"]["en"] ) assert ( "The app id is manifestv2_app" - in m["notifications"]["post_install"]["main"]["en"] + in m["notifications"]["POST_INSTALL"]["main"]["en"] ) assert ( f"The app url is {main_domain}/manifestv2" - in m["notifications"]["post_install"]["main"]["en"] + in m["notifications"]["POST_INSTALL"]["main"]["en"] ) @@ -341,7 +341,7 @@ def test_manifestv2_app_info_preupgrade(monkeypatch): # should parse the files in the original app repo, possibly with proper i18n etc assert ( "This is a dummy disclaimer to display prior to any upgrade" - in i["from_catalog"]["manifest"]["notifications"]["pre_upgrade"]["main"]["en"] + in i["from_catalog"]["manifest"]["notifications"]["PRE_UPGRADE"]["main"]["en"] ) From d0d0d3e0da9180fba6d0065a4e851fa61bebb147 Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Wed, 4 Jan 2023 01:18:06 +0100 Subject: [PATCH 39/43] appv2: cosmetic, having the notification name is weird, it's gonna be pretty much always just 'main' --- src/app.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/app.py b/src/app.py index e8079eb55..6343275e8 100644 --- a/src/app.py +++ b/src/app.py @@ -2893,7 +2893,7 @@ def _display_notifications(notifications, force=False): return for name, content in notifications.items(): - print(f"========== {name}") + print("==========") print(content) print("==========") From 7a35a3a671dd02ec944bb8efbb63bf9ab1582039 Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Wed, 4 Jan 2023 20:22:49 +0100 Subject: [PATCH 40/43] appv2: implement dismiss logic for app notifications --- share/actionsmap.yml | 11 +++++++++++ src/app.py | 37 +++++++++++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+) diff --git a/share/actionsmap.yml b/share/actionsmap.yml index 0a6f10856..d95c25f8b 100644 --- a/share/actionsmap.yml +++ b/share/actionsmap.yml @@ -981,6 +981,17 @@ app: help: Undo redirection action: store_true + ### app_dismiss_notification + dismiss-notification: + hide_in_help: True + action_help: Dismiss post_install or post_upgrade notification + api: PUT /apps//dismiss_notification/ + arguments: + app: + help: App ID to dismiss notification for + name: + help: Notification name, either post_install or post_upgrade + ### app_ssowatconf() ssowatconf: action_help: Regenerate SSOwat configuration file diff --git a/src/app.py b/src/app.py index 6343275e8..d92f81647 100644 --- a/src/app.py +++ b/src/app.py @@ -190,6 +190,13 @@ def app_info(app, full=False, upgradable=False): ret["manifest"]["doc"][pagename][lang] = _hydrate_app_template( content, settings ) + + # Filter dismissed notification + ret["manifest"]["notifications"] = {k: v + for k, v in ret["manifest"]["notifications"].items() + if not _notification_is_dismissed(k, settings) } + + # Hydrate notifications (also filter uneeded post_upgrade notification based on version) for step, notifications in ret["manifest"]["notifications"].items(): for name, content_per_lang in notifications.items(): for lang, content in content_per_lang.items(): @@ -809,6 +816,9 @@ def app_upgrade(app=[], url=None, file=None, force=False, no_safety_backup=False # ask for simple confirm _display_notifications(notifications, force=force) + # Reset the dismiss flag for post upgrade notification + app_setting(app, "_dismiss_notification_post_upgrade", delete=True) + hook_callback("post_app_upgrade", env=env_dict) operation_logger.success() @@ -2877,6 +2887,33 @@ def _assert_system_is_sane_for_app(manifest, when): raise YunohostError("this_action_broke_dpkg") +def app_dismiss_notification(app, name): + + assert isinstance(name, str) + name = name.lower() + assert name in ["post_install", "post_upgrade"] + _assert_is_installed(app) + + app_setting(app, f"_dismiss_notification_{name}", value="1") + + +def _notification_is_dismissed(name, settings): + # Check for _dismiss_notiication_$name setting and also auto-dismiss + # notifications after one week (otherwise people using mostly CLI would + # never really dismiss the notification and it would be displayed forever) + + if name == "POST_INSTALL": + return settings.get("_dismiss_notification_post_install") \ + or (int(time.time()) - settings.get("install_time", 0)) / (24 * 3600) > 7 + elif name == "POST_UPGRADE": + # Check on update_time also implicitly prevent the post_upgrade notification + # from being displayed after install, because update_time is only set during upgrade + return settings.get("_dismiss_notification_post_upgrade") \ + or (int(time.time()) - settings.get("update_time", 0)) / (24 * 3600) > 7 + else: + return False + + def _filter_and_hydrate_notifications(notifications, current_version=None, data={}): return { # Should we render the markdown maybe? idk From 946c0bcf7d845560d3e50f6039f83df8200f478c Mon Sep 17 00:00:00 2001 From: axolotle Date: Thu, 5 Jan 2023 20:35:47 +0100 Subject: [PATCH 41/43] fix app_instance_name var + formatting --- src/app.py | 58 +++++++++++++++++++++++++++++++++++------------------- 1 file changed, 38 insertions(+), 20 deletions(-) diff --git a/src/app.py b/src/app.py index d92f81647..15ce0e453 100644 --- a/src/app.py +++ b/src/app.py @@ -154,7 +154,11 @@ def app_info(app, full=False, upgradable=False): # Check if $app.png exists in the app logo folder, this is a trick to be able to easily customize the logo # of an app just by creating $app.png (instead of the hash.png) in the corresponding folder - ret["logo"] = app if os.path.exists(f"{APPS_CATALOG_LOGOS}/{app}.png") else from_catalog.get("logo_hash") + ret["logo"] = ( + app + if os.path.exists(f"{APPS_CATALOG_LOGOS}/{app}.png") + else from_catalog.get("logo_hash") + ) ret["upgradable"] = _app_upgradable({**ret, "from_catalog": from_catalog}) if ret["upgradable"] == "yes": @@ -192,9 +196,11 @@ def app_info(app, full=False, upgradable=False): ) # Filter dismissed notification - ret["manifest"]["notifications"] = {k: v - for k, v in ret["manifest"]["notifications"].items() - if not _notification_is_dismissed(k, settings) } + ret["manifest"]["notifications"] = { + k: v + for k, v in ret["manifest"]["notifications"].items() + if not _notification_is_dismissed(k, settings) + } # Hydrate notifications (also filter uneeded post_upgrade notification based on version) for step, notifications in ret["manifest"]["notifications"].items(): @@ -688,7 +694,10 @@ def app_upgrade(app=[], url=None, file=None, force=False, no_safety_backup=False AppResourceManager( app_instance_name, wanted=manifest, current=app_dict["manifest"] - ).apply(rollback_and_raise_exception_if_failure=True, operation_logger=operation_logger) + ).apply( + rollback_and_raise_exception_if_failure=True, + operation_logger=operation_logger, + ) # Execute the app upgrade script upgrade_failed = True @@ -817,7 +826,9 @@ def app_upgrade(app=[], url=None, file=None, force=False, no_safety_backup=False _display_notifications(notifications, force=force) # Reset the dismiss flag for post upgrade notification - app_setting(app, "_dismiss_notification_post_upgrade", delete=True) + app_setting( + app_instance_name, "_dismiss_notification_post_upgrade", delete=True + ) hook_callback("post_app_upgrade", env=env_dict) operation_logger.success() @@ -1049,7 +1060,8 @@ def app_install( from yunohost.utils.resources import AppResourceManager AppResourceManager(app_instance_name, wanted=manifest, current={}).apply( - rollback_and_raise_exception_if_failure=True, operation_logger=operation_logger + rollback_and_raise_exception_if_failure=True, + operation_logger=operation_logger, ) else: # Initialize the main permission for the app @@ -2038,9 +2050,7 @@ def _parse_app_doc_and_notifications(path): for step in notification_names: notifications[step] = {} - for filepath in glob.glob( - os.path.join(path, "doc", f"{step}*.md") - ): + for filepath in glob.glob(os.path.join(path, "doc", f"{step}*.md")): m = re.match(step + "(_[a-z]{2,3})?.md", filepath.split("/")[-1]) if not m: continue @@ -2050,9 +2060,7 @@ def _parse_app_doc_and_notifications(path): notifications[step][pagename] = {} notifications[step][pagename][lang] = read_file(filepath).strip() - for filepath in glob.glob( - os.path.join(path, "doc", f"{step}.d") + "/*.md" - ): + for filepath in glob.glob(os.path.join(path, "doc", f"{step}.d") + "/*.md"): m = re.match( r"([A-Za-z0-9\.\~]*)(_[a-z]{2,3})?.md", filepath.split("/")[-1] ) @@ -2492,7 +2500,9 @@ def _check_manifest_requirements( raise YunohostValidationError("app_packaging_format_not_supported") # Yunohost version - required_yunohost_version = manifest["integration"].get("yunohost", "4.3").strip(">= ") + required_yunohost_version = ( + manifest["integration"].get("yunohost", "4.3").strip(">= ") + ) current_yunohost_version = get_ynh_package_version("yunohost")["version"] yield ( @@ -2557,8 +2567,12 @@ def _check_manifest_requirements( # Is "include_swap" really useful ? We should probably decide wether to always include it or not instead if ram_requirement.get("include_swap", False): ram += swap - can_build = ram_requirement["build"] == "?" or ram > human_to_binary(ram_requirement["build"]) - can_run = ram_requirement["runtime"] == "?" or ram > human_to_binary(ram_requirement["runtime"]) + can_build = ram_requirement["build"] == "?" or ram > human_to_binary( + ram_requirement["build"] + ) + can_run = ram_requirement["runtime"] == "?" or ram > human_to_binary( + ram_requirement["runtime"] + ) yield ( "ram", @@ -2903,13 +2917,17 @@ def _notification_is_dismissed(name, settings): # never really dismiss the notification and it would be displayed forever) if name == "POST_INSTALL": - return settings.get("_dismiss_notification_post_install") \ - or (int(time.time()) - settings.get("install_time", 0)) / (24 * 3600) > 7 + return ( + settings.get("_dismiss_notification_post_install") + or (int(time.time()) - settings.get("install_time", 0)) / (24 * 3600) > 7 + ) elif name == "POST_UPGRADE": # Check on update_time also implicitly prevent the post_upgrade notification # from being displayed after install, because update_time is only set during upgrade - return settings.get("_dismiss_notification_post_upgrade") \ - or (int(time.time()) - settings.get("update_time", 0)) / (24 * 3600) > 7 + return ( + settings.get("_dismiss_notification_post_upgrade") + or (int(time.time()) - settings.get("update_time", 0)) / (24 * 3600) > 7 + ) else: return False From e54bf2ed670507173a572018df744d1bebdb405b Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Thu, 5 Jan 2023 20:50:43 +0100 Subject: [PATCH 42/43] appv2: fix pre-upgrade version check --- src/app.py | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/src/app.py b/src/app.py index 15ce0e453..30e3a7be8 100644 --- a/src/app.py +++ b/src/app.py @@ -2425,12 +2425,11 @@ def _list_upgradable_apps(): for app in upgradable_apps: absolute_app_name, _ = _parse_app_instance_name(app["id"]) manifest, extracted_app_folder = _extract_app(absolute_app_name) - current_version = version.parse(app["current_version"]) app["notifications"] = {} if manifest["notifications"]["PRE_UPGRADE"]: app["notifications"]["PRE_UPGRADE"] = _filter_and_hydrate_notifications( manifest["notifications"]["PRE_UPGRADE"], - current_version, + app["current_version"], app["settings"], ) del app["settings"] @@ -2933,13 +2932,22 @@ def _notification_is_dismissed(name, settings): def _filter_and_hydrate_notifications(notifications, current_version=None, data={}): + + def is_version_more_recent_than_current_version(name): + # Boring code to handle the fact that "0.1 < 9999~ynh1" is False + + if "~" in name: + return version.parse(name) > version.parse(current_version) + else: + return version.parse(name) > version.parse(current_version.split("~")[0]) + return { # Should we render the markdown maybe? idk name: _hydrate_app_template(_value_for_locale(content_per_lang), data) for name, content_per_lang in notifications.items() if current_version is None or name == "main" - or version.parse(name) > current_version + or is_version_more_recent_than_current_version(name) } From 02abcd41f9717464a8fd2c8ed558f3dcb13a71eb Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Thu, 5 Jan 2023 23:55:08 +0100 Subject: [PATCH 43/43] app resources: Fix tests --- src/utils/resources.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/utils/resources.py b/src/utils/resources.py index e1c6e75f4..cb8688046 100644 --- a/src/utils/resources.py +++ b/src/utils/resources.py @@ -30,7 +30,7 @@ from moulinette.utils.filesystem import ( rm, ) -from yunohost.utils.error import YunohostError +from yunohost.utils.error import YunohostError, YunohostValidationError logger = getActionLogger("yunohost.app_resources") @@ -859,7 +859,7 @@ class PortsResource(AppResource): if infos["fixed"]: if self._port_is_used(port_value): - raise ValidationError(f"Port {port_value} is already used by another process or app.") + raise YunohostValidationError(f"Port {port_value} is already used by another process or app.") else: while self._port_is_used(port_value): port_value += 1 @@ -920,6 +920,7 @@ class DatabaseAppResource(AppResource): type = "database" priority = 90 + dbtype: str = "" default_properties: Dict[str, Any] = { "dbtype": None, @@ -932,7 +933,8 @@ class DatabaseAppResource(AppResource): "postgresql", ]: raise YunohostError( - "Specifying the type of db ('mysql' or 'postgresql') is mandatory for db resources" + "Specifying the type of db ('mysql' or 'postgresql') is mandatory for db resources", + raw_msg=True ) # Hack so that people can write type = "mysql/postgresql" in toml but it's loaded as dbtype