From 8cdd5e43d743fcba0cb2ec1b53a454787eb5ffb6 Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Fri, 1 Oct 2021 02:18:35 +0200 Subject: [PATCH 01/18] Simplify / fix inconsistencies in app files kept after install/upgrade --- src/yunohost/app.py | 77 ++++++++++++--------------------------------- 1 file changed, 20 insertions(+), 57 deletions(-) diff --git a/src/yunohost/app.py b/src/yunohost/app.py index 56e4c344b..c111de405 100644 --- a/src/yunohost/app.py +++ b/src/yunohost/app.py @@ -79,6 +79,17 @@ re_app_instance_name = re.compile( r"^(?P[\w-]+?)(__(?P[1-9][0-9]*))?$" ) +APP_FILES_TO_COPY = [ + "manifest.json", + "manifest.toml", + "actions.json", + "actions.toml", + "config_panel.toml", + "scripts", + "conf", + "hooks", + "doc", +] def app_catalog(full=False, with_categories=False): """ @@ -710,38 +721,11 @@ def app_upgrade(app=[], url=None, file=None, force=False, no_safety_backup=False hook_add(app_instance_name, extracted_app_folder + "/hooks/" + hook) # Replace scripts and manifest and conf (if exists) - os.system( - 'rm -rf "%s/scripts" "%s/manifest.toml %s/manifest.json %s/conf"' - % ( - app_setting_path, - app_setting_path, - app_setting_path, - app_setting_path, - ) - ) - - if os.path.exists(os.path.join(extracted_app_folder, "manifest.json")): - os.system( - 'mv "%s/manifest.json" "%s/scripts" %s' - % (extracted_app_folder, extracted_app_folder, app_setting_path) - ) - if os.path.exists(os.path.join(extracted_app_folder, "manifest.toml")): - os.system( - 'mv "%s/manifest.toml" "%s/scripts" %s' - % (extracted_app_folder, extracted_app_folder, app_setting_path) - ) - - for file_to_copy in [ - "actions.json", - "actions.toml", - "config_panel.toml", - "conf", - ]: + # Move scripts and manifest to the right place + for file_to_copy in APP_FILES_TO_COPY: + os.system(f"rm -rf '{app_setting_path}/{file_to_copy}'") if os.path.exists(os.path.join(extracted_app_folder, file_to_copy)): - os.system( - "cp -R %s/%s %s" - % (extracted_app_folder, file_to_copy, app_setting_path) - ) + os.system(f"cp -R '{extracted_app_folder}/{file_to_copy} {app_setting_path}'") # Clean and set permissions shutil.rmtree(extracted_app_folder) @@ -945,23 +929,9 @@ def app_install( _set_app_settings(app_instance_name, app_settings) # Move scripts and manifest to the right place - if os.path.exists(os.path.join(extracted_app_folder, "manifest.json")): - os.system("cp %s/manifest.json %s" % (extracted_app_folder, app_setting_path)) - if os.path.exists(os.path.join(extracted_app_folder, "manifest.toml")): - os.system("cp %s/manifest.toml %s" % (extracted_app_folder, app_setting_path)) - os.system("cp -R %s/scripts %s" % (extracted_app_folder, app_setting_path)) - - for file_to_copy in [ - "actions.json", - "actions.toml", - "config_panel.toml", - "conf", - ]: + for file_to_copy in APP_FILES_TO_COPY: if os.path.exists(os.path.join(extracted_app_folder, file_to_copy)): - os.system( - "cp -R %s/%s %s" - % (extracted_app_folder, file_to_copy, app_setting_path) - ) + os.system(f"cp -R '{extracted_app_folder}/{file_to_copy}' '{app_setting_path}'" # Initialize the main permission for the app # The permission is initialized with no url associated, and with tile disabled @@ -1038,11 +1008,7 @@ def app_install( logger.warning(m18n.n("app_remove_after_failed_install")) # Setup environment for remove script - env_dict_remove = {} - env_dict_remove["YNH_APP_ID"] = app_id - env_dict_remove["YNH_APP_INSTANCE_NAME"] = app_instance_name - env_dict_remove["YNH_APP_INSTANCE_NUMBER"] = str(instance_number) - env_dict_remove["YNH_APP_MANIFEST_VERSION"] = manifest.get("version", "?") + env_dict_remove = _make_environment_for_app_script(app_instance_name) env_dict_remove["YNH_APP_BASEDIR"] = extracted_app_folder # Execute remove script @@ -1156,12 +1122,9 @@ def app_remove(operation_logger, app, purge=False): env_dict = {} app_id, app_instance_nb = _parse_app_instance_name(app) - env_dict["YNH_APP_ID"] = app_id - env_dict["YNH_APP_INSTANCE_NAME"] = app - env_dict["YNH_APP_INSTANCE_NUMBER"] = str(app_instance_nb) - env_dict["YNH_APP_MANIFEST_VERSION"] = manifest.get("version", "?") - env_dict["YNH_APP_PURGE"] = str(1 if purge else 0) + env_dict = _make_environment_for_app_script(app) env_dict["YNH_APP_BASEDIR"] = tmp_workdir_for_app + env_dict["YNH_APP_PURGE"] = str(1 if purge else 0) operation_logger.extra.update({"env": env_dict}) operation_logger.flush() From a1ff2ced37a3ef01966e359dfeebe62114f28673 Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Fri, 1 Oct 2021 02:21:31 +0200 Subject: [PATCH 02/18] Clarify code that extract app from folder/gitrepo + misc other small refactors --- src/yunohost/app.py | 364 ++++++++++++++++++++------------------------ 1 file changed, 163 insertions(+), 201 deletions(-) diff --git a/src/yunohost/app.py b/src/yunohost/app.py index c111de405..88deb5b6b 100644 --- a/src/yunohost/app.py +++ b/src/yunohost/app.py @@ -566,22 +566,22 @@ def app_upgrade(app=[], url=None, file=None, force=False, no_safety_backup=False if file and isinstance(file, dict): # We use this dirty hack to test chained upgrades in unit/functional tests - manifest, extracted_app_folder = _extract_app_from_file( - file[app_instance_name] - ) + new_app_src = file[app_instance_name] elif file: - manifest, extracted_app_folder = _extract_app_from_file(file) + new_app_src = file elif url: - manifest, extracted_app_folder = _fetch_app_from_git(url) + new_app_src = url elif app_dict["upgradable"] == "url_required": logger.warning(m18n.n("custom_app_url_required", app=app_instance_name)) continue elif app_dict["upgradable"] == "yes" or force: - manifest, extracted_app_folder = _fetch_app_from_git(app_instance_name) + new_app_src = app_dict["id"] else: logger.success(m18n.n("app_already_up_to_date", app=app_instance_name)) continue + manifest, extracted_app_folder = _extract_app(new_app_src) + # Manage upgrade type and avoid any upgrade if there is nothing to do upgrade_type = "UNKNOWN" # Get current_version and new version @@ -748,12 +748,7 @@ def app_manifest(app): raw_app_list = _load_apps_catalog()["apps"] - if app in raw_app_list or ("@" in app) or ("http://" in app) or ("https://" in app): - manifest, extracted_app_folder = _fetch_app_from_git(app) - elif os.path.exists(app): - manifest, extracted_app_folder = _extract_app_from_file(app) - else: - raise YunohostValidationError("app_unknown") + manifest, extracted_app_folder = _extract_app(app) shutil.rmtree(extracted_app_folder) @@ -796,19 +791,28 @@ def app_install( ) from yunohost.regenconf import manually_modified_files - def confirm_install(confirm): + # Check if disk space available + if free_space_in_directory("/") <= 512 * 1000 * 1000: + raise YunohostValidationError("disk_space_not_sufficient_install") + + def confirm_install(app): + # Ignore if there's nothing for confirm (good quality app), if --force is used # or if request on the API (confirm already implemented on the API side) - if confirm is None or force or Moulinette.interface.type == "api": + if force or Moulinette.interface.type == "api": + return + + quality = app_quality(app) + if quality == "success": return # i18n: confirm_app_install_warning # i18n: confirm_app_install_danger # i18n: confirm_app_install_thirdparty - if confirm in ["danger", "thirdparty"]: + if quality in ["danger", "thirdparty"]: answer = Moulinette.prompt( - m18n.n("confirm_app_install_" + confirm, answers="Yes, I understand"), + m18n.n("confirm_app_install_" + quality, answers="Yes, I understand"), color="red", ) if answer != "Yes, I understand": @@ -816,51 +820,13 @@ def app_install( else: answer = Moulinette.prompt( - m18n.n("confirm_app_install_" + confirm, answers="Y/N"), color="yellow" + m18n.n("confirm_app_install_" + quality, answers="Y/N"), color="yellow" ) if answer.upper() != "Y": raise YunohostError("aborting") - raw_app_list = _load_apps_catalog()["apps"] - - if app in raw_app_list or ("@" in app) or ("http://" in app) or ("https://" in app): - - # If we got an app name directly (e.g. just "wordpress"), we gonna test this name - if app in raw_app_list: - app_name_to_test = app - # If we got an url like "https://github.com/foo/bar_ynh, we want to - # extract "bar" and test if we know this app - elif ("http://" in app) or ("https://" in app): - app_name_to_test = app.strip("/").split("/")[-1].replace("_ynh", "") - else: - # FIXME : watdo if '@' in app ? - app_name_to_test = None - - if app_name_to_test in raw_app_list: - - state = raw_app_list[app_name_to_test].get("state", "notworking") - level = raw_app_list[app_name_to_test].get("level", None) - confirm = "danger" - if state in ["working", "validated"]: - if isinstance(level, int) and level >= 5: - confirm = None - elif isinstance(level, int) and level > 0: - confirm = "warning" - else: - confirm = "thirdparty" - - confirm_install(confirm) - - manifest, extracted_app_folder = _fetch_app_from_git(app) - elif os.path.exists(app): - confirm_install("thirdparty") - manifest, extracted_app_folder = _extract_app_from_file(app) - else: - raise YunohostValidationError("app_unknown") - - # Check if disk space available - if free_space_in_directory("/") <= 512 * 1000 * 1000: - raise YunohostValidationError("disk_space_not_sufficient_install") + confirm_install(app) + manifest, extracted_app_folder = _extract_app(app) # Check ID if "id" not in manifest or "__" in manifest["id"] or "." in manifest["id"]: @@ -874,7 +840,7 @@ def app_install( _assert_system_is_sane_for_app(manifest, "pre") # Check if app can be forked - instance_number = _installed_instance_number(app_id, last=True) + 1 + instance_number = _next_instance_number_for_app(app_id) if instance_number > 1: if "multi_instance" not in manifest or not is_true(manifest["multi_instance"]): raise YunohostValidationError("app_already_installed", app=app_id) @@ -1914,55 +1880,6 @@ def _set_app_settings(app_id, settings): yaml.safe_dump(settings, f, default_flow_style=False) -def _extract_app_from_file(path): - """ - Unzip / untar / copy application tarball or directory to a tmp work directory - - Keyword arguments: - path -- Path of the tarball or directory - """ - logger.debug(m18n.n("extracting")) - - path = os.path.abspath(path) - - extracted_app_folder = _make_tmp_workdir_for_app() - - if ".zip" in path: - extract_result = os.system( - f"unzip '{path}' -d {extracted_app_folder} > /dev/null 2>&1" - ) - elif ".tar" in path: - extract_result = os.system( - f"tar -xf '{path}' -C {extracted_app_folder} > /dev/null 2>&1" - ) - elif os.path.isdir(path): - shutil.rmtree(extracted_app_folder) - if path[-1] != "/": - path = path + "/" - extract_result = os.system(f"cp -a '{path}' {extracted_app_folder}") - else: - extract_result = 1 - - if extract_result != 0: - raise YunohostError("app_extraction_failed") - - try: - if len(os.listdir(extracted_app_folder)) == 1: - for folder in os.listdir(extracted_app_folder): - extracted_app_folder = extracted_app_folder + "/" + folder - manifest = _get_manifest_of_app(extracted_app_folder) - manifest["lastUpdate"] = int(time.time()) - except IOError: - raise YunohostError("app_install_files_invalid") - except ValueError as e: - raise YunohostError("app_manifest_invalid", error=e) - - logger.debug(m18n.n("done")) - - manifest["remote"] = {"type": "file", "path": path} - return manifest, extracted_app_folder - - def _get_manifest_of_app(path): "Get app manifest stored in json or in toml" @@ -2158,143 +2075,169 @@ def _set_default_ask_questions(arguments): return arguments -def _get_git_last_commit_hash(repository, reference="HEAD"): +def _app_quality(app: str) -> str: """ - Attempt to retrieve the last commit hash of a git repository - - Keyword arguments: - repository -- The URL or path of the repository - + app may in fact be an app name, an url, or a path """ - try: - cmd = "git ls-remote --exit-code {0} {1} | awk '{{print $1}}'".format( - repository, reference - ) - commit = check_output(cmd) - except subprocess.CalledProcessError: - logger.error("unable to get last commit from %s", repository) - raise ValueError("Unable to get last commit with git") + + raw_app_catalog = _load_apps_catalog()["apps"] + if app in raw_app_catalog or ("@" in app) or ("http://" in app) or ("https://" in app): + + # If we got an app name directly (e.g. just "wordpress"), we gonna test this name + if app in raw_app_list: + app_name_to_test = app + # If we got an url like "https://github.com/foo/bar_ynh, we want to + # extract "bar" and test if we know this app + elif ("http://" in app) or ("https://" in app): + app_name_to_test = app.strip("/").split("/")[-1].replace("_ynh", "") + else: + # FIXME : watdo if '@' in app ? + app_name_to_test = None + + if app_name_to_test in raw_app_list: + + state = raw_app_list[app_name_to_test].get("state", "notworking") + level = raw_app_list[app_name_to_test].get("level", None) + if state in ["working", "validated"]: + if isinstance(level, int) and level >= 5: + return "success" + elif isinstance(level, int) and level > 0: + return "warning" + return "danger" + else: + return "thirdparty" + + elif os.path.exists(app): + return "thirdparty" else: - return commit.strip() + raise YunohostValidationError("app_unknown") -def _fetch_app_from_git(app): +def _extract_app(src: str) -> Tuple[Dict, str]: """ - Unzip or untar application tarball to a tmp directory - - Keyword arguments: - app -- App_id or git repo URL + src may be an app name, an url, or a path """ - # Extract URL, branch and revision to download - if ("@" in app) or ("http://" in app) or ("https://" in app): - url = app - branch = "master" - if "/tree/" in url: - url, branch = url.split("/tree/", 1) - revision = "HEAD" - else: - app_dict = _load_apps_catalog()["apps"] + raw_app_catalog = _load_apps_catalog()["apps"] - app_id, _ = _parse_app_instance_name(app) - - if app_id not in app_dict: - raise YunohostValidationError("app_unknown") - elif "git" not in app_dict[app_id]: + # App is an appname in the catalog + if src in raw_app_catalog: + if "git" not in raw_app_catalog[src]: raise YunohostValidationError("app_unsupported_remote_type") - app_info = app_dict[app_id] + app_info = raw_app_catalog[src] url = app_info["git"]["url"] branch = app_info["git"]["branch"] revision = str(app_info["git"]["revision"]) + return _extract_app_from_gitrepo(url, branch, revision, app_info) + # App is a git repo url + elif ("@" in src) or ("http://" in src) or ("https://" in src): + url = src + branch = "master" + revision = "HEAD" + if "/tree/" in url: + url, branch = url.split("/tree/", 1) + return _extract_app_from_gitrepo(url, branch, revision, {}) + # App is a local folder + elif os.path.exists(src): + return _extract_app_from_folder(src) + else: + raise YunohostValidationError("app_unknown") + + +def _extract_app_from_folder(path: str) -> Tuple[Dict, str]: + """ + Unzip / untar / copy application tarball or directory to a tmp work directory + + Keyword arguments: + path -- Path of the tarball or directory + """ + logger.debug(m18n.n("extracting")) + + path = os.path.abspath(path) extracted_app_folder = _make_tmp_workdir_for_app() + if ".zip" in path: + extract_result = os.system( + f"unzip '{path}' -d {extracted_app_folder} > /dev/null 2>&1" + ) + elif ".tar" in path: + extract_result = os.system( + f"tar -xf '{path}' -C {extracted_app_folder} > /dev/null 2>&1" + ) + elif os.path.isdir(path): + shutil.rmtree(extracted_app_folder) + if path[-1] != "/": + path = path + "/" + extract_result = os.system(f"cp -a '{path}' {extracted_app_folder}") + else: + extract_result = 1 + + if extract_result != 0: + raise YunohostError("app_extraction_failed") + + try: + if len(os.listdir(extracted_app_folder)) == 1: + for folder in os.listdir(extracted_app_folder): + extracted_app_folder = extracted_app_folder + "/" + folder + except IOError: + raise YunohostError("app_install_files_invalid") + + manifest = _get_manifest_of_app(extracted_app_folder) + manifest["lastUpdate"] = int(time.time()) + + logger.debug(m18n.n("done")) + + manifest["remote"] = {"type": "file", "path": path} + return manifest, extracted_app_folder + + +def _extract_app_from_gitrepo(url: str, branch: str, revision: str, app_info={}: Dict) -> Tuple[Dict, str]: + logger.debug(m18n.n("downloading")) + extracted_app_folder = _make_tmp_workdir_for_app() + # Download only this commit try: # We don't use git clone because, git clone can't download # a specific revision only + ref = branch if revision == "HEAD" else revision run_commands([["git", "init", extracted_app_folder]], shell=False) run_commands( [ ["git", "remote", "add", "origin", url], - [ - "git", - "fetch", - "--depth=1", - "origin", - branch if revision == "HEAD" else revision, - ], + ["git", "fetch", "--depth=1", "origin", ref], ["git", "reset", "--hard", "FETCH_HEAD"], ], cwd=extracted_app_folder, shell=False, ) - manifest = _get_manifest_of_app(extracted_app_folder) except subprocess.CalledProcessError: raise YunohostError("app_sources_fetch_failed") - except ValueError as e: - raise YunohostError("app_manifest_invalid", error=e) else: logger.debug(m18n.n("done")) + manifest = _get_manifest_of_app(extracted_app_folder) + # Store remote repository info into the returned manifest manifest["remote"] = {"type": "git", "url": url, "branch": branch} if revision == "HEAD": try: - manifest["remote"]["revision"] = _get_git_last_commit_hash(url, branch) + # Get git last commit hash + cmd = f"git ls-remote --exit-code {url} {branch} | awk '{{print $1}}'" + manifest["remote"]["revision"] = check_output(cmd) except Exception as e: - logger.debug("cannot get last commit hash because: %s ", e) + logger.warning("cannot get last commit hash because: %s ", e) else: manifest["remote"]["revision"] = revision - manifest["lastUpdate"] = app_info["lastUpdate"] + manifest["lastUpdate"] = app_info.get("lastUpdate") return manifest, extracted_app_folder -def _installed_instance_number(app, last=False): - """ - Check if application is installed and return instance number - - Keyword arguments: - app -- id of App to check - last -- Return only last instance number - - Returns: - Number of last installed instance | List or instances - - """ - if last: - number = 0 - try: - installed_apps = os.listdir(APPS_SETTING_PATH) - except OSError: - os.makedirs(APPS_SETTING_PATH) - return 0 - - for installed_app in installed_apps: - if number == 0 and app == installed_app: - number = 1 - elif "__" in installed_app: - if app == installed_app[: installed_app.index("__")]: - if int(installed_app[installed_app.index("__") + 2 :]) > number: - number = int(installed_app[installed_app.index("__") + 2 :]) - - return number - - else: - instance_number_list = [] - instances_dict = app_map(app=app, raw=True) - for key, domain in instances_dict.items(): - for key, path in domain.items(): - instance_number_list.append(path["instance"]) - - return sorted(instance_number_list) - - -def _is_installed(app): +def _is_installed(app: str) -> bool: """ Check if application is installed @@ -2308,18 +2251,18 @@ def _is_installed(app): return os.path.isdir(APPS_SETTING_PATH + app) -def _assert_is_installed(app): +def _assert_is_installed(app: str) -> None: if not _is_installed(app): raise YunohostValidationError( "app_not_installed", app=app, all_apps=_get_all_installed_apps_id() ) -def _installed_apps(): +def _installed_apps() -> List[str]: return os.listdir(APPS_SETTING_PATH) -def _check_manifest_requirements(manifest, app_instance_name): +def _check_manifest_requirements(manifest: Dict, app: str): """Check if required packages are met from the manifest""" packaging_format = int(manifest.get("packaging_format", 0)) @@ -2331,7 +2274,7 @@ def _check_manifest_requirements(manifest, app_instance_name): if not requirements: return - logger.debug(m18n.n("app_requirements_checking", app=app_instance_name)) + logger.debug(m18n.n("app_requirements_checking", app=app)) # Iterate over requirements for pkgname, spec in requirements.items(): @@ -2342,7 +2285,7 @@ def _check_manifest_requirements(manifest, app_instance_name): pkgname=pkgname, version=version, spec=spec, - app=app_instance_name, + app=app, ) @@ -2513,6 +2456,25 @@ def _parse_app_instance_name(app_instance_name): return (appid, app_instance_nb) +def _next_instance_number_for_app(app): + + # Get list of sibling apps, such as {app}, {app}__2, {app}__4 + apps = _installed_apps() + sibling_app_ids = [a for a in apps if a == app or a.startswith(f"{app}__")] + + # Find the list of ids, such as [1, 2, 4] + sibling_ids = [_parse_app_instance_name(a)[1] for a in sibling_app_ids] + + # Find the first 'i' that's not in the sibling_ids list already + i = 1 + while True: + if i not in sibling_ids: + return i + else: + i += 1 + + + # # ############################### # # Applications list management # From f84d8618bcc01bcd6f569723b7db8bc9e146a16b Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Fri, 1 Oct 2021 02:22:23 +0200 Subject: [PATCH 03/18] Offload legacy app patching stuff to utils/legacy.py to limit the size of app.py --- src/yunohost/app.py | 212 +---------------- src/yunohost/backup.py | 6 +- .../0016_php70_to_php73_pools.py | 3 +- src/yunohost/utils/legacy.py | 217 +++++++++++++++++- 4 files changed, 222 insertions(+), 216 deletions(-) diff --git a/src/yunohost/app.py b/src/yunohost/app.py index 88deb5b6b..011c953e0 100644 --- a/src/yunohost/app.py +++ b/src/yunohost/app.py @@ -534,6 +534,7 @@ def app_upgrade(app=[], url=None, file=None, force=False, no_safety_backup=False ) from yunohost.permission import permission_sync_to_user from yunohost.regenconf import manually_modified_files + from yunohost.utils.legacy import _patch_legacy_php_versions, _patch_legacy_helpers apps = app # Check if disk space available @@ -790,6 +791,7 @@ def app_install( permission_sync_to_user, ) from yunohost.regenconf import manually_modified_files + from yunohost.utils.legacy import _patch_legacy_php_versions, _patch_legacy_helpers # Check if disk space available if free_space_in_directory("/") <= 512 * 1000 * 1000: @@ -2769,213 +2771,3 @@ def _assert_system_is_sane_for_app(manifest, when): raise YunohostValidationError("dpkg_is_broken") elif when == "post": raise YunohostError("this_action_broke_dpkg") - - -LEGACY_PHP_VERSION_REPLACEMENTS = [ - ("/etc/php5", "/etc/php/7.3"), - ("/etc/php/7.0", "/etc/php/7.3"), - ("/var/run/php5-fpm", "/var/run/php/php7.3-fpm"), - ("/var/run/php/php7.0-fpm", "/var/run/php/php7.3-fpm"), - ("php5", "php7.3"), - ("php7.0", "php7.3"), - ( - 'phpversion="${phpversion:-7.0}"', - 'phpversion="${phpversion:-7.3}"', - ), # Many helpers like the composer ones use 7.0 by default ... - ( - '"$phpversion" == "7.0"', - '$(bc <<< "$phpversion >= 7.3") -eq 1', - ), # patch ynh_install_php to refuse installing/removing php <= 7.3 -] - - -def _patch_legacy_php_versions(app_folder): - - files_to_patch = [] - files_to_patch.extend(glob.glob("%s/conf/*" % app_folder)) - files_to_patch.extend(glob.glob("%s/scripts/*" % app_folder)) - files_to_patch.extend(glob.glob("%s/scripts/*/*" % app_folder)) - files_to_patch.extend(glob.glob("%s/scripts/.*" % app_folder)) - files_to_patch.append("%s/manifest.json" % app_folder) - files_to_patch.append("%s/manifest.toml" % app_folder) - - for filename in files_to_patch: - - # Ignore non-regular files - if not os.path.isfile(filename): - continue - - c = ( - "sed -i " - + "".join( - "-e 's@{pattern}@{replace}@g' ".format(pattern=p, replace=r) - for p, r in LEGACY_PHP_VERSION_REPLACEMENTS - ) - + "%s" % filename - ) - os.system(c) - - -def _patch_legacy_php_versions_in_settings(app_folder): - - settings = read_yaml(os.path.join(app_folder, "settings.yml")) - - if settings.get("fpm_config_dir") == "/etc/php/7.0/fpm": - settings["fpm_config_dir"] = "/etc/php/7.3/fpm" - if settings.get("fpm_service") == "php7.0-fpm": - settings["fpm_service"] = "php7.3-fpm" - if settings.get("phpversion") == "7.0": - settings["phpversion"] = "7.3" - - # We delete these checksums otherwise the file will appear as manually modified - list_to_remove = ["checksum__etc_php_7.0_fpm_pool", "checksum__etc_nginx_conf.d"] - settings = { - k: v - for k, v in settings.items() - if not any(k.startswith(to_remove) for to_remove in list_to_remove) - } - - write_to_yaml(app_folder + "/settings.yml", settings) - - -def _patch_legacy_helpers(app_folder): - - files_to_patch = [] - files_to_patch.extend(glob.glob("%s/scripts/*" % app_folder)) - files_to_patch.extend(glob.glob("%s/scripts/.*" % app_folder)) - - stuff_to_replace = { - # Replace - # sudo yunohost app initdb $db_user -p $db_pwd - # by - # ynh_mysql_setup_db --db_user=$db_user --db_name=$db_user --db_pwd=$db_pwd - "yunohost app initdb": { - "pattern": r"(sudo )?yunohost app initdb \"?(\$\{?\w+\}?)\"?\s+-p\s\"?(\$\{?\w+\}?)\"?", - "replace": r"ynh_mysql_setup_db --db_user=\2 --db_name=\2 --db_pwd=\3", - "important": True, - }, - # Replace - # sudo yunohost app checkport whaterver - # by - # ynh_port_available whatever - "yunohost app checkport": { - "pattern": r"(sudo )?yunohost app checkport", - "replace": r"ynh_port_available", - "important": True, - }, - # We can't migrate easily port-available - # .. but at the time of writing this code, only two non-working apps are using it. - "yunohost tools port-available": {"important": True}, - # Replace - # yunohost app checkurl "${domain}${path_url}" -a "${app}" - # by - # ynh_webpath_register --app=${app} --domain=${domain} --path_url=${path_url} - "yunohost app checkurl": { - "pattern": r"(sudo )?yunohost app checkurl \"?(\$\{?\w+\}?)\/?(\$\{?\w+\}?)\"?\s+-a\s\"?(\$\{?\w+\}?)\"?", - "replace": r"ynh_webpath_register --app=\4 --domain=\2 --path_url=\3", - "important": True, - }, - # Remove - # Automatic diagnosis data from YunoHost - # __PRE_TAG1__$(yunohost tools diagnosis | ...)__PRE_TAG2__" - # - "yunohost tools diagnosis": { - "pattern": r"(Automatic diagnosis data from YunoHost( *\n)*)? *(__\w+__)? *\$\(yunohost tools diagnosis.*\)(__\w+__)?", - "replace": r"", - "important": False, - }, - # Old $1, $2 in backup/restore scripts... - "app=$2": { - "only_for": ["scripts/backup", "scripts/restore"], - "pattern": r"app=\$2", - "replace": r"app=$YNH_APP_INSTANCE_NAME", - "important": True, - }, - # Old $1, $2 in backup/restore scripts... - "backup_dir=$1": { - "only_for": ["scripts/backup", "scripts/restore"], - "pattern": r"backup_dir=\$1", - "replace": r"backup_dir=.", - "important": True, - }, - # Old $1, $2 in backup/restore scripts... - "restore_dir=$1": { - "only_for": ["scripts/restore"], - "pattern": r"restore_dir=\$1", - "replace": r"restore_dir=.", - "important": True, - }, - # Old $1, $2 in install scripts... - # We ain't patching that shit because it ain't trivial to patch all args... - "domain=$1": {"only_for": ["scripts/install"], "important": True}, - } - - for helper, infos in stuff_to_replace.items(): - infos["pattern"] = ( - re.compile(infos["pattern"]) if infos.get("pattern") else None - ) - infos["replace"] = infos.get("replace") - - for filename in files_to_patch: - - # Ignore non-regular files - if not os.path.isfile(filename): - continue - - try: - content = read_file(filename) - except MoulinetteError: - continue - - replaced_stuff = False - show_warning = False - - for helper, infos in stuff_to_replace.items(): - - # Ignore if not relevant for this file - if infos.get("only_for") and not any( - filename.endswith(f) for f in infos["only_for"] - ): - continue - - # If helper is used, attempt to patch the file - if helper in content and infos["pattern"]: - content = infos["pattern"].sub(infos["replace"], content) - replaced_stuff = True - if infos["important"]: - show_warning = True - - # If the helper is *still* in the content, it means that we - # couldn't patch the deprecated helper in the previous lines. In - # that case, abort the install or whichever step is performed - if helper in content and infos["important"]: - raise YunohostValidationError( - "This app is likely pretty old and uses deprecated / outdated helpers that can't be migrated easily. It can't be installed anymore.", - raw_msg=True, - ) - - if replaced_stuff: - - # Check the app do load the helper - # If it doesn't, add the instruction ourselve (making sure it's after the #!/bin/bash if it's there... - if filename.split("/")[-1] in [ - "install", - "remove", - "upgrade", - "backup", - "restore", - ]: - source_helpers = "source /usr/share/yunohost/helpers" - if source_helpers not in content: - content.replace("#!/bin/bash", "#!/bin/bash\n" + source_helpers) - if source_helpers not in content: - content = source_helpers + "\n" + content - - # Actually write the new content in the file - write_to_file(filename, content) - - if show_warning: - # And complain about those damn deprecated helpers - logger.error( - r"/!\ Packagers ! This app uses a very old deprecated helpers ... Yunohost automatically patched the helpers to use the new recommended practice, but please do consider fixing the upstream code right now ..." - ) diff --git a/src/yunohost/backup.py b/src/yunohost/backup.py index a47fba5f7..b650cb3da 100644 --- a/src/yunohost/backup.py +++ b/src/yunohost/backup.py @@ -49,10 +49,6 @@ from yunohost.app import ( app_info, _is_installed, _make_environment_for_app_script, - _patch_legacy_helpers, - _patch_legacy_php_versions, - _patch_legacy_php_versions_in_settings, - LEGACY_PHP_VERSION_REPLACEMENTS, _make_tmp_workdir_for_app, ) from yunohost.hook import ( @@ -1190,6 +1186,7 @@ class RestoreManager: """ Apply dirty patch to redirect php5 and php7.0 files to php7.3 """ + from yunohost.utils.legacy import LEGACY_PHP_VERSION_REPLACEMENTS backup_csv = os.path.join(self.work_dir, "backup.csv") @@ -1351,6 +1348,7 @@ class RestoreManager: app_instance_name -- (string) The app name to restore (no app with this name should be already install) """ + from yunohost.utils.legacy import _patch_legacy_php_versions, _patch_legacy_php_versions_in_settings, _patch_legacy_helpers from yunohost.user import user_group_list from yunohost.permission import ( permission_create, diff --git a/src/yunohost/data_migrations/0016_php70_to_php73_pools.py b/src/yunohost/data_migrations/0016_php70_to_php73_pools.py index 6b424f211..fed96c9c8 100644 --- a/src/yunohost/data_migrations/0016_php70_to_php73_pools.py +++ b/src/yunohost/data_migrations/0016_php70_to_php73_pools.py @@ -4,7 +4,8 @@ from shutil import copy2 from moulinette.utils.log import getActionLogger -from yunohost.app import _is_installed, _patch_legacy_php_versions_in_settings +from yunohost.app import _is_installed +from yunohost.utils.legacy import _patch_legacy_php_versions_in_settings from yunohost.tools import Migration from yunohost.service import _run_service_command diff --git a/src/yunohost/utils/legacy.py b/src/yunohost/utils/legacy.py index eb92dd71f..7ae98bed8 100644 --- a/src/yunohost/utils/legacy.py +++ b/src/yunohost/utils/legacy.py @@ -1,7 +1,10 @@ import os +import re +import glob from moulinette import m18n +from moulinette.core import MoulinetteError from moulinette.utils.log import getActionLogger -from moulinette.utils.filesystem import write_to_json, read_yaml +from moulinette.utils.filesystem import read_file, write_to_file, write_to_json, write_to_yaml, read_yaml from yunohost.user import user_list from yunohost.app import ( @@ -14,6 +17,8 @@ from yunohost.permission import ( user_permission_update, permission_sync_to_user, ) +from yunohost.utils.error import YunohostValidationError + logger = getActionLogger("yunohost.legacy") @@ -237,3 +242,213 @@ def translate_legacy_rules_in_ssowant_conf_json_persistent(): logger.warning( "YunoHost automatically translated some legacy rules in /etc/ssowat/conf.json.persistent to match the new permission system" ) + + +LEGACY_PHP_VERSION_REPLACEMENTS = [ + ("/etc/php5", "/etc/php/7.3"), + ("/etc/php/7.0", "/etc/php/7.3"), + ("/var/run/php5-fpm", "/var/run/php/php7.3-fpm"), + ("/var/run/php/php7.0-fpm", "/var/run/php/php7.3-fpm"), + ("php5", "php7.3"), + ("php7.0", "php7.3"), + ( + 'phpversion="${phpversion:-7.0}"', + 'phpversion="${phpversion:-7.3}"', + ), # Many helpers like the composer ones use 7.0 by default ... + ( + '"$phpversion" == "7.0"', + '$(bc <<< "$phpversion >= 7.3") -eq 1', + ), # patch ynh_install_php to refuse installing/removing php <= 7.3 +] + + +def _patch_legacy_php_versions(app_folder): + + files_to_patch = [] + files_to_patch.extend(glob.glob("%s/conf/*" % app_folder)) + files_to_patch.extend(glob.glob("%s/scripts/*" % app_folder)) + files_to_patch.extend(glob.glob("%s/scripts/*/*" % app_folder)) + files_to_patch.extend(glob.glob("%s/scripts/.*" % app_folder)) + files_to_patch.append("%s/manifest.json" % app_folder) + files_to_patch.append("%s/manifest.toml" % app_folder) + + for filename in files_to_patch: + + # Ignore non-regular files + if not os.path.isfile(filename): + continue + + c = ( + "sed -i " + + "".join( + "-e 's@{pattern}@{replace}@g' ".format(pattern=p, replace=r) + for p, r in LEGACY_PHP_VERSION_REPLACEMENTS + ) + + "%s" % filename + ) + os.system(c) + + +def _patch_legacy_php_versions_in_settings(app_folder): + + settings = read_yaml(os.path.join(app_folder, "settings.yml")) + + if settings.get("fpm_config_dir") == "/etc/php/7.0/fpm": + settings["fpm_config_dir"] = "/etc/php/7.3/fpm" + if settings.get("fpm_service") == "php7.0-fpm": + settings["fpm_service"] = "php7.3-fpm" + if settings.get("phpversion") == "7.0": + settings["phpversion"] = "7.3" + + # We delete these checksums otherwise the file will appear as manually modified + list_to_remove = ["checksum__etc_php_7.0_fpm_pool", "checksum__etc_nginx_conf.d"] + settings = { + k: v + for k, v in settings.items() + if not any(k.startswith(to_remove) for to_remove in list_to_remove) + } + + write_to_yaml(app_folder + "/settings.yml", settings) + + +def _patch_legacy_helpers(app_folder): + + files_to_patch = [] + files_to_patch.extend(glob.glob("%s/scripts/*" % app_folder)) + files_to_patch.extend(glob.glob("%s/scripts/.*" % app_folder)) + + stuff_to_replace = { + # Replace + # sudo yunohost app initdb $db_user -p $db_pwd + # by + # ynh_mysql_setup_db --db_user=$db_user --db_name=$db_user --db_pwd=$db_pwd + "yunohost app initdb": { + "pattern": r"(sudo )?yunohost app initdb \"?(\$\{?\w+\}?)\"?\s+-p\s\"?(\$\{?\w+\}?)\"?", + "replace": r"ynh_mysql_setup_db --db_user=\2 --db_name=\2 --db_pwd=\3", + "important": True, + }, + # Replace + # sudo yunohost app checkport whaterver + # by + # ynh_port_available whatever + "yunohost app checkport": { + "pattern": r"(sudo )?yunohost app checkport", + "replace": r"ynh_port_available", + "important": True, + }, + # We can't migrate easily port-available + # .. but at the time of writing this code, only two non-working apps are using it. + "yunohost tools port-available": {"important": True}, + # Replace + # yunohost app checkurl "${domain}${path_url}" -a "${app}" + # by + # ynh_webpath_register --app=${app} --domain=${domain} --path_url=${path_url} + "yunohost app checkurl": { + "pattern": r"(sudo )?yunohost app checkurl \"?(\$\{?\w+\}?)\/?(\$\{?\w+\}?)\"?\s+-a\s\"?(\$\{?\w+\}?)\"?", + "replace": r"ynh_webpath_register --app=\4 --domain=\2 --path_url=\3", + "important": True, + }, + # Remove + # Automatic diagnosis data from YunoHost + # __PRE_TAG1__$(yunohost tools diagnosis | ...)__PRE_TAG2__" + # + "yunohost tools diagnosis": { + "pattern": r"(Automatic diagnosis data from YunoHost( *\n)*)? *(__\w+__)? *\$\(yunohost tools diagnosis.*\)(__\w+__)?", + "replace": r"", + "important": False, + }, + # Old $1, $2 in backup/restore scripts... + "app=$2": { + "only_for": ["scripts/backup", "scripts/restore"], + "pattern": r"app=\$2", + "replace": r"app=$YNH_APP_INSTANCE_NAME", + "important": True, + }, + # Old $1, $2 in backup/restore scripts... + "backup_dir=$1": { + "only_for": ["scripts/backup", "scripts/restore"], + "pattern": r"backup_dir=\$1", + "replace": r"backup_dir=.", + "important": True, + }, + # Old $1, $2 in backup/restore scripts... + "restore_dir=$1": { + "only_for": ["scripts/restore"], + "pattern": r"restore_dir=\$1", + "replace": r"restore_dir=.", + "important": True, + }, + # Old $1, $2 in install scripts... + # We ain't patching that shit because it ain't trivial to patch all args... + "domain=$1": {"only_for": ["scripts/install"], "important": True}, + } + + for helper, infos in stuff_to_replace.items(): + infos["pattern"] = ( + re.compile(infos["pattern"]) if infos.get("pattern") else None + ) + infos["replace"] = infos.get("replace") + + for filename in files_to_patch: + + # Ignore non-regular files + if not os.path.isfile(filename): + continue + + try: + content = read_file(filename) + except MoulinetteError: + continue + + replaced_stuff = False + show_warning = False + + for helper, infos in stuff_to_replace.items(): + + # Ignore if not relevant for this file + if infos.get("only_for") and not any( + filename.endswith(f) for f in infos["only_for"] + ): + continue + + # If helper is used, attempt to patch the file + if helper in content and infos["pattern"]: + content = infos["pattern"].sub(infos["replace"], content) + replaced_stuff = True + if infos["important"]: + show_warning = True + + # If the helper is *still* in the content, it means that we + # couldn't patch the deprecated helper in the previous lines. In + # that case, abort the install or whichever step is performed + if helper in content and infos["important"]: + raise YunohostValidationError( + "This app is likely pretty old and uses deprecated / outdated helpers that can't be migrated easily. It can't be installed anymore.", + raw_msg=True, + ) + + if replaced_stuff: + + # Check the app do load the helper + # If it doesn't, add the instruction ourselve (making sure it's after the #!/bin/bash if it's there... + if filename.split("/")[-1] in [ + "install", + "remove", + "upgrade", + "backup", + "restore", + ]: + source_helpers = "source /usr/share/yunohost/helpers" + if source_helpers not in content: + content.replace("#!/bin/bash", "#!/bin/bash\n" + source_helpers) + if source_helpers not in content: + content = source_helpers + "\n" + content + + # Actually write the new content in the file + write_to_file(filename, content) + + if show_warning: + # And complain about those damn deprecated helpers + logger.error( + r"/!\ Packagers ! This app uses a very old deprecated helpers ... Yunohost automatically patched the helpers to use the new recommended practice, but please do consider fixing the upstream code right now ..." + ) From 206b430a9f66c698821f5fc56d1cfaa967e1361c Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Fri, 1 Oct 2021 02:50:30 +0200 Subject: [PATCH 04/18] Offload appcatalog stuff to a separate file to limit the size of app.py --- .gitlab/ci/test.gitlab-ci.yml | 4 +- src/yunohost/app.py | 296 ++---------------- ...est_appscatalog.py => test_app_catalog.py} | 2 +- src/yunohost/tools.py | 4 +- 4 files changed, 31 insertions(+), 275 deletions(-) rename src/yunohost/tests/{test_appscatalog.py => test_app_catalog.py} (99%) diff --git a/.gitlab/ci/test.gitlab-ci.yml b/.gitlab/ci/test.gitlab-ci.yml index 6a422ea22..1aad46fbe 100644 --- a/.gitlab/ci/test.gitlab-ci.yml +++ b/.gitlab/ci/test.gitlab-ci.yml @@ -113,10 +113,10 @@ test-apps: test-appscatalog: extends: .test-stage script: - - python3 -m pytest src/yunohost/tests/test_appscatalog.py + - python3 -m pytest src/yunohost/tests/test_app_catalog.py only: changes: - - src/yunohost/app.py + - src/yunohost/app_calalog.py test-appurl: extends: .test-stage diff --git a/src/yunohost/app.py b/src/yunohost/app.py index 011c953e0..7cddca3e9 100644 --- a/src/yunohost/app.py +++ b/src/yunohost/app.py @@ -31,15 +31,12 @@ import yaml import time import re import subprocess -import glob import tempfile from collections import OrderedDict -from typing import List +from typing import List, Tuple, Dict from moulinette import Moulinette, m18n -from moulinette.core import MoulinetteError from moulinette.utils.log import getActionLogger -from moulinette.utils.network import download_json from moulinette.utils.process import run_commands, check_output from moulinette.utils.filesystem import ( read_file, @@ -48,8 +45,6 @@ from moulinette.utils.filesystem import ( read_yaml, write_to_file, write_to_json, - write_to_yaml, - mkdir, ) from yunohost.utils import packages @@ -64,17 +59,13 @@ from yunohost.utils.i18n import _value_for_locale from yunohost.utils.error import YunohostError, YunohostValidationError from yunohost.utils.filesystem import free_space_in_directory from yunohost.log import is_unit_operation, OperationLogger +from yunohost.app_catalog import app_catalog, app_search, _load_apps_catalog, app_fetchlist # noqa logger = getActionLogger("yunohost.app") APPS_SETTING_PATH = "/etc/yunohost/apps/" APP_TMP_WORKDIRS = "/var/cache/yunohost/app_tmp_work_dirs" -APPS_CATALOG_CACHE = "/var/cache/yunohost/repo" -APPS_CATALOG_CONF = "/etc/yunohost/apps_catalog.yml" -APPS_CATALOG_API_VERSION = 2 -APPS_CATALOG_DEFAULT_URL = "https://app.yunohost.org/default" - re_app_instance_name = re.compile( r"^(?P[\w-]+?)(__(?P[1-9][0-9]*))?$" ) @@ -91,80 +82,6 @@ APP_FILES_TO_COPY = [ "doc", ] -def app_catalog(full=False, with_categories=False): - """ - Return a dict of apps available to installation from Yunohost's app catalog - """ - - # Get app list from catalog cache - catalog = _load_apps_catalog() - installed_apps = set(_installed_apps()) - - # Trim info for apps if not using --full - for app, infos in catalog["apps"].items(): - infos["installed"] = app in installed_apps - - infos["manifest"]["description"] = _value_for_locale( - infos["manifest"]["description"] - ) - - if not full: - catalog["apps"][app] = { - "description": infos["manifest"]["description"], - "level": infos["level"], - } - else: - infos["manifest"]["arguments"] = _set_default_ask_questions( - infos["manifest"].get("arguments", {}) - ) - - # 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"]) - - if not full: - catalog["categories"] = [ - {"id": c["id"], "description": c["description"]} - for c in catalog["categories"] - ] - - if not with_categories: - return {"apps": catalog["apps"]} - else: - return {"apps": catalog["apps"], "categories": catalog["categories"]} - - -def app_search(string): - """ - Return a dict of apps whose description or name match the search string - """ - - # Retrieve a simple dict listing all apps - catalog_of_apps = app_catalog() - - # Selecting apps according to a match in app name or description - matching_apps = {"apps": {}} - for app in catalog_of_apps["apps"].items(): - if re.search(string, app[0], flags=re.IGNORECASE) or re.search( - string, app[1]["description"], flags=re.IGNORECASE - ): - matching_apps["apps"][app[0]] = app[1] - - return matching_apps - - -# Old legacy function... -def app_fetchlist(): - logger.warning( - "'yunohost app fetchlist' is deprecated. Please use 'yunohost tools update --apps' instead" - ) - from yunohost.tools import tools_update - - tools_update(target="apps") - def app_list(full=False, installed=False, filter=None): """ @@ -1728,22 +1645,6 @@ ynh_app_config_run $1 return values -def _get_all_installed_apps_id(): - """ - Return something like: - ' * app1 - * app2 - * ...' - """ - - all_apps_ids = sorted(_installed_apps()) - - all_apps_ids_formatted = "\n * ".join(all_apps_ids) - all_apps_ids_formatted = "\n * " + all_apps_ids_formatted - - return all_apps_ids_formatted - - def _get_app_actions(app_id): "Get app config panel stored in json or in toml" actions_toml_path = os.path.join(APPS_SETTING_PATH, app_id, "actions.toml") @@ -2239,6 +2140,13 @@ def _extract_app_from_gitrepo(url: str, branch: str, revision: str, app_info={}: return manifest, extracted_app_folder +# +# ############################### # +# Small utilities # +# ############################### # +# + + def _is_installed(app: str) -> bool: """ Check if application is installed @@ -2264,6 +2172,22 @@ def _installed_apps() -> List[str]: return os.listdir(APPS_SETTING_PATH) +def _get_all_installed_apps_id(): + """ + Return something like: + ' * app1 + * app2 + * ...' + """ + + all_apps_ids = sorted(_installed_apps()) + + all_apps_ids_formatted = "\n * ".join(all_apps_ids) + all_apps_ids_formatted = "\n * " + all_apps_ids_formatted + + return all_apps_ids_formatted + + def _check_manifest_requirements(manifest: Dict, app: str): """Check if required packages are met from the manifest""" @@ -2476,176 +2400,6 @@ def _next_instance_number_for_app(app): i += 1 - -# -# ############################### # -# Applications list management # -# ############################### # -# - - -def _initialize_apps_catalog_system(): - """ - This function is meant to intialize the apps_catalog system with YunoHost's default app catalog. - """ - - default_apps_catalog_list = [{"id": "default", "url": APPS_CATALOG_DEFAULT_URL}] - - try: - logger.debug( - "Initializing apps catalog system with YunoHost's default app list" - ) - write_to_yaml(APPS_CATALOG_CONF, default_apps_catalog_list) - except Exception as e: - raise YunohostError( - "Could not initialize the apps catalog system... : %s" % str(e) - ) - - logger.success(m18n.n("apps_catalog_init_success")) - - -def _read_apps_catalog_list(): - """ - Read the json corresponding to the list of apps catalogs - """ - - try: - list_ = read_yaml(APPS_CATALOG_CONF) - # Support the case where file exists but is empty - # by returning [] if list_ is None - return list_ if list_ else [] - except Exception as e: - raise YunohostError("Could not read the apps_catalog list ... : %s" % str(e)) - - -def _actual_apps_catalog_api_url(base_url): - - return "{base_url}/v{version}/apps.json".format( - base_url=base_url, version=APPS_CATALOG_API_VERSION - ) - - -def _update_apps_catalog(): - """ - Fetches the json for each apps_catalog and update the cache - - apps_catalog_list is for example : - [ {"id": "default", "url": "https://app.yunohost.org/default/"} ] - - Then for each apps_catalog, the actual json URL to be fetched is like : - https://app.yunohost.org/default/vX/apps.json - - And store it in : - /var/cache/yunohost/repo/default.json - """ - - apps_catalog_list = _read_apps_catalog_list() - - logger.info(m18n.n("apps_catalog_updating")) - - # Create cache folder if needed - if not os.path.exists(APPS_CATALOG_CACHE): - logger.debug("Initialize folder for apps catalog cache") - mkdir(APPS_CATALOG_CACHE, mode=0o750, parents=True, uid="root") - - for apps_catalog in apps_catalog_list: - apps_catalog_id = apps_catalog["id"] - actual_api_url = _actual_apps_catalog_api_url(apps_catalog["url"]) - - # Fetch the json - try: - apps_catalog_content = download_json(actual_api_url) - except Exception as e: - raise YunohostError( - "apps_catalog_failed_to_download", - apps_catalog=apps_catalog_id, - error=str(e), - ) - - # Remember the apps_catalog api version for later - apps_catalog_content["from_api_version"] = APPS_CATALOG_API_VERSION - - # Save the apps_catalog data in the cache - cache_file = "{cache_folder}/{list}.json".format( - cache_folder=APPS_CATALOG_CACHE, list=apps_catalog_id - ) - try: - write_to_json(cache_file, apps_catalog_content) - except Exception as e: - raise YunohostError( - "Unable to write cache data for %s apps_catalog : %s" - % (apps_catalog_id, str(e)) - ) - - logger.success(m18n.n("apps_catalog_update_success")) - - -def _load_apps_catalog(): - """ - Read all the apps catalog cache files and build a single dict (merged_catalog) - corresponding to all known apps and categories - """ - - merged_catalog = {"apps": {}, "categories": []} - - for apps_catalog_id in [L["id"] for L in _read_apps_catalog_list()]: - - # Let's load the json from cache for this catalog - cache_file = "{cache_folder}/{list}.json".format( - cache_folder=APPS_CATALOG_CACHE, list=apps_catalog_id - ) - - try: - apps_catalog_content = ( - read_json(cache_file) if os.path.exists(cache_file) else None - ) - except Exception as e: - raise YunohostError( - "Unable to read cache for apps_catalog %s : %s" % (cache_file, e), - raw_msg=True, - ) - - # Check that the version of the data matches version .... - # ... otherwise it means we updated yunohost in the meantime - # and need to update the cache for everything to be consistent - if ( - not apps_catalog_content - or apps_catalog_content.get("from_api_version") != APPS_CATALOG_API_VERSION - ): - logger.info(m18n.n("apps_catalog_obsolete_cache")) - _update_apps_catalog() - apps_catalog_content = read_json(cache_file) - - del apps_catalog_content["from_api_version"] - - # Add apps from this catalog to the output - for app, info in apps_catalog_content["apps"].items(): - - # (N.B. : there's a small edge case where multiple apps catalog could be listing the same apps ... - # in which case we keep only the first one found) - if app in merged_catalog["apps"]: - logger.warning( - "Duplicate app %s found between apps catalog %s and %s" - % (app, apps_catalog_id, merged_catalog["apps"][app]["repository"]) - ) - continue - - info["repository"] = apps_catalog_id - merged_catalog["apps"][app] = info - - # Annnnd categories - merged_catalog["categories"] += apps_catalog_content["categories"] - - return merged_catalog - - -# -# ############################### # -# Small utilities # -# ############################### # -# - - def _make_tmp_workdir_for_app(app=None): # Create parent dir if it doesn't exists yet diff --git a/src/yunohost/tests/test_appscatalog.py b/src/yunohost/tests/test_app_catalog.py similarity index 99% rename from src/yunohost/tests/test_appscatalog.py rename to src/yunohost/tests/test_app_catalog.py index a2619a660..8423b868e 100644 --- a/src/yunohost/tests/test_appscatalog.py +++ b/src/yunohost/tests/test_app_catalog.py @@ -9,7 +9,7 @@ from moulinette import m18n from moulinette.utils.filesystem import read_json, write_to_json, write_to_yaml from yunohost.utils.error import YunohostError -from yunohost.app import ( +from yunohost.app_catalog import ( _initialize_apps_catalog_system, _read_apps_catalog_list, _update_apps_catalog, diff --git a/src/yunohost/tools.py b/src/yunohost/tools.py index ed8c04153..4cdd62d8f 100644 --- a/src/yunohost/tools.py +++ b/src/yunohost/tools.py @@ -37,10 +37,12 @@ from moulinette.utils.process import check_output, call_async_output from moulinette.utils.filesystem import read_yaml, write_to_yaml from yunohost.app import ( - _update_apps_catalog, app_info, app_upgrade, +) +from yunohost.app_catalog import ( _initialize_apps_catalog_system, + _update_apps_catalog, ) from yunohost.domain import domain_add from yunohost.dyndns import _dyndns_available, _dyndns_provides From 1e046dcd37a2f6d0365be4003e86622e58dd285d Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Fri, 1 Oct 2021 02:50:38 +0200 Subject: [PATCH 05/18] Misc typoz --- src/yunohost/app.py | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/src/yunohost/app.py b/src/yunohost/app.py index 7cddca3e9..8dcb5aef3 100644 --- a/src/yunohost/app.py +++ b/src/yunohost/app.py @@ -609,7 +609,7 @@ def app_upgrade(app=[], url=None, file=None, force=False, no_safety_backup=False if upgrade_failed or broke_the_system: # display this if there are remaining apps - if apps[number + 1 :]: + if apps[number + 1:]: not_upgraded_apps = apps[number:] logger.error( m18n.n( @@ -664,8 +664,6 @@ def app_upgrade(app=[], url=None, file=None, force=False, no_safety_backup=False def app_manifest(app): - raw_app_list = _load_apps_catalog()["apps"] - manifest, extracted_app_folder = _extract_app(app) shutil.rmtree(extracted_app_folder) @@ -721,7 +719,7 @@ def app_install( if force or Moulinette.interface.type == "api": return - quality = app_quality(app) + quality = _app_quality(app) if quality == "success": return @@ -816,7 +814,7 @@ def app_install( # Move scripts and manifest to the right place for file_to_copy in APP_FILES_TO_COPY: if os.path.exists(os.path.join(extracted_app_folder, file_to_copy)): - os.system(f"cp -R '{extracted_app_folder}/{file_to_copy}' '{app_setting_path}'" + os.system(f"cp -R '{extracted_app_folder}/{file_to_copy}' '{app_setting_path}'") # Initialize the main permission for the app # The permission is initialized with no url associated, and with tile disabled @@ -976,6 +974,7 @@ def app_remove(operation_logger, app, purge=False): purge -- Remove with all app data """ + from yunohost.utils.legacy import _patch_legacy_php_versions, _patch_legacy_helpers from yunohost.hook import hook_exec, hook_remove, hook_callback from yunohost.permission import ( user_permission_list, @@ -1987,7 +1986,7 @@ def _app_quality(app: str) -> str: if app in raw_app_catalog or ("@" in app) or ("http://" in app) or ("https://" in app): # If we got an app name directly (e.g. just "wordpress"), we gonna test this name - if app in raw_app_list: + if app in raw_app_catalog: app_name_to_test = app # If we got an url like "https://github.com/foo/bar_ynh, we want to # extract "bar" and test if we know this app @@ -1997,10 +1996,10 @@ def _app_quality(app: str) -> str: # FIXME : watdo if '@' in app ? app_name_to_test = None - if app_name_to_test in raw_app_list: + if app_name_to_test in raw_app_catalog: - state = raw_app_list[app_name_to_test].get("state", "notworking") - level = raw_app_list[app_name_to_test].get("level", None) + state = raw_app_catalog[app_name_to_test].get("state", "notworking") + level = raw_app_catalog[app_name_to_test].get("level", None) if state in ["working", "validated"]: if isinstance(level, int) and level >= 5: return "success" @@ -2096,7 +2095,7 @@ def _extract_app_from_folder(path: str) -> Tuple[Dict, str]: return manifest, extracted_app_folder -def _extract_app_from_gitrepo(url: str, branch: str, revision: str, app_info={}: Dict) -> Tuple[Dict, str]: +def _extract_app_from_gitrepo(url: str, branch: str, revision: str, app_info: Dict = {}) -> Tuple[Dict, str]: logger.debug(m18n.n("downloading")) From 80f4b892e65b6838146bc520da8feb7991327a32 Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Fri, 1 Oct 2021 03:05:14 +0200 Subject: [PATCH 06/18] Prevent full-domain app to be moved to a subpath with change-url ? --- src/yunohost/app.py | 36 +++++++++++++++++------------------- 1 file changed, 17 insertions(+), 19 deletions(-) diff --git a/src/yunohost/app.py b/src/yunohost/app.py index 8dcb5aef3..f221e2406 100644 --- a/src/yunohost/app.py +++ b/src/yunohost/app.py @@ -33,7 +33,7 @@ import re import subprocess import tempfile from collections import OrderedDict -from typing import List, Tuple, Dict +from typing import List, Tuple, Dict, Any from moulinette import Moulinette, m18n from moulinette.utils.log import getActionLogger @@ -51,7 +51,6 @@ from yunohost.utils import packages from yunohost.utils.config import ( ConfigPanel, ask_questions_and_parse_answers, - Question, DomainQuestion, PathQuestion, ) @@ -384,8 +383,9 @@ def app_change_url(operation_logger, app, domain, path): "app_change_url_identical_domains", domain=domain, path=path ) - # Check the url is available - _assert_no_conflicting_apps(domain, path, ignore_app=app) + app_setting_path = os.path.join(APPS_SETTING_PATH, app) + path_requirement = _guess_webapp_path_requirement(app_setting_path) + _validate_webpath_requirement({"domain": domain, "path": path}, path_requirement, ignore_app=app) tmp_workdir_for_app = _make_tmp_workdir_for_app(app=app) @@ -777,8 +777,8 @@ def app_install( } # Validate domain / path availability for webapps - path_requirement = _guess_webapp_path_requirement(questions, extracted_app_folder) - _validate_webpath_requirement(questions, path_requirement) + path_requirement = _guess_webapp_path_requirement(extracted_app_folder) + _validate_webpath_requirement(args, path_requirement) # Attempt to patch legacy helpers ... _patch_legacy_helpers(extracted_app_folder) @@ -2214,13 +2214,16 @@ def _check_manifest_requirements(manifest: Dict, app: str): ) -def _guess_webapp_path_requirement(questions: List[Question], app_folder: str) -> str: +def _guess_webapp_path_requirement(app_folder: str) -> str: # If there's only one "domain" and "path", validate that domain/path # is an available url and normalize the path. - domain_questions = [question for question in questions if question.type == "domain"] - path_questions = [question for question in questions if question.type == "path"] + manifest = _get_manifest_of_app(app_folder) + raw_questions = manifest.get("arguments", {}).get("install", {}) + + domain_questions = [question for question in raw_questions if question.get("type") == "domain"] + path_questions = [question for question in raw_questions if question.get("type") == "path"] if len(domain_questions) == 0 and len(path_questions) == 0: return "" @@ -2248,22 +2251,17 @@ def _guess_webapp_path_requirement(questions: List[Question], app_folder: str) - def _validate_webpath_requirement( - questions: List[Question], path_requirement: str + args: Dict[str, Any], path_requirement: str, ignore_app=None ) -> None: - domain_questions = [question for question in questions if question.type == "domain"] - path_questions = [question for question in questions if question.type == "path"] + domain = args.get("domain") + path = args.get("path") if path_requirement == "domain_and_path": - - domain = domain_questions[0].value - path = path_questions[0].value - _assert_no_conflicting_apps(domain, path, full_domain=True) + _assert_no_conflicting_apps(domain, path, ignore_app=ignore_app) elif path_requirement == "full_domain": - - domain = domain_questions[0].value - _assert_no_conflicting_apps(domain, "/", full_domain=True) + _assert_no_conflicting_apps(domain, "/", full_domain=True, ignore_app=ignore_app) def _get_conflicting_apps(domain, path, ignore_app=None): From 5fc493058df2766097494b710f5f628fafaf81a2 Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Fri, 1 Oct 2021 03:10:26 +0200 Subject: [PATCH 07/18] Aaaaaand I forgot to commit app_catalog.py ... --- src/yunohost/app_catalog.py | 255 ++++++++++++++++++++++++++++++++++++ 1 file changed, 255 insertions(+) create mode 100644 src/yunohost/app_catalog.py diff --git a/src/yunohost/app_catalog.py b/src/yunohost/app_catalog.py new file mode 100644 index 000000000..e4ffa1db6 --- /dev/null +++ b/src/yunohost/app_catalog.py @@ -0,0 +1,255 @@ +import os +import re + +from moulinette import m18n +from moulinette.utils.log import getActionLogger +from moulinette.utils.network import download_json +from moulinette.utils.filesystem import ( + read_json, + read_yaml, + write_to_json, + write_to_yaml, + mkdir, +) + +from yunohost.utils.i18n import _value_for_locale +from yunohost.utils.error import YunohostError + +logger = getActionLogger("yunohost.app_catalog") + +APPS_CATALOG_CACHE = "/var/cache/yunohost/repo" +APPS_CATALOG_CONF = "/etc/yunohost/apps_catalog.yml" +APPS_CATALOG_API_VERSION = 2 +APPS_CATALOG_DEFAULT_URL = "https://app.yunohost.org/default" + + +# Old legacy function... +def app_fetchlist(): + logger.warning( + "'yunohost app fetchlist' is deprecated. Please use 'yunohost tools update --apps' instead" + ) + from yunohost.tools import tools_update + + tools_update(target="apps") + + +def app_catalog(full=False, with_categories=False): + """ + Return a dict of apps available to installation from Yunohost's app catalog + """ + + from yunohost.app import _installed_apps, _set_default_ask_questions + + # Get app list from catalog cache + catalog = _load_apps_catalog() + installed_apps = set(_installed_apps()) + + # Trim info for apps if not using --full + for app, infos in catalog["apps"].items(): + infos["installed"] = app in installed_apps + + infos["manifest"]["description"] = _value_for_locale( + infos["manifest"]["description"] + ) + + if not full: + catalog["apps"][app] = { + "description": infos["manifest"]["description"], + "level": infos["level"], + } + else: + infos["manifest"]["arguments"] = _set_default_ask_questions( + infos["manifest"].get("arguments", {}) + ) + + # 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"]) + + if not full: + catalog["categories"] = [ + {"id": c["id"], "description": c["description"]} + for c in catalog["categories"] + ] + + if not with_categories: + return {"apps": catalog["apps"]} + else: + return {"apps": catalog["apps"], "categories": catalog["categories"]} + + +def app_search(string): + """ + Return a dict of apps whose description or name match the search string + """ + + # Retrieve a simple dict listing all apps + catalog_of_apps = app_catalog() + + # Selecting apps according to a match in app name or description + matching_apps = {"apps": {}} + for app in catalog_of_apps["apps"].items(): + if re.search(string, app[0], flags=re.IGNORECASE) or re.search( + string, app[1]["description"], flags=re.IGNORECASE + ): + matching_apps["apps"][app[0]] = app[1] + + return matching_apps + + +def _initialize_apps_catalog_system(): + """ + This function is meant to intialize the apps_catalog system with YunoHost's default app catalog. + """ + + default_apps_catalog_list = [{"id": "default", "url": APPS_CATALOG_DEFAULT_URL}] + + try: + logger.debug( + "Initializing apps catalog system with YunoHost's default app list" + ) + write_to_yaml(APPS_CATALOG_CONF, default_apps_catalog_list) + except Exception as e: + raise YunohostError( + "Could not initialize the apps catalog system... : %s" % str(e) + ) + + logger.success(m18n.n("apps_catalog_init_success")) + + +def _read_apps_catalog_list(): + """ + Read the json corresponding to the list of apps catalogs + """ + + try: + list_ = read_yaml(APPS_CATALOG_CONF) + # Support the case where file exists but is empty + # by returning [] if list_ is None + return list_ if list_ else [] + except Exception as e: + raise YunohostError("Could not read the apps_catalog list ... : %s" % str(e)) + + +def _actual_apps_catalog_api_url(base_url): + + return "{base_url}/v{version}/apps.json".format( + base_url=base_url, version=APPS_CATALOG_API_VERSION + ) + + +def _update_apps_catalog(): + """ + Fetches the json for each apps_catalog and update the cache + + apps_catalog_list is for example : + [ {"id": "default", "url": "https://app.yunohost.org/default/"} ] + + Then for each apps_catalog, the actual json URL to be fetched is like : + https://app.yunohost.org/default/vX/apps.json + + And store it in : + /var/cache/yunohost/repo/default.json + """ + + apps_catalog_list = _read_apps_catalog_list() + + logger.info(m18n.n("apps_catalog_updating")) + + # Create cache folder if needed + if not os.path.exists(APPS_CATALOG_CACHE): + logger.debug("Initialize folder for apps catalog cache") + mkdir(APPS_CATALOG_CACHE, mode=0o750, parents=True, uid="root") + + for apps_catalog in apps_catalog_list: + apps_catalog_id = apps_catalog["id"] + actual_api_url = _actual_apps_catalog_api_url(apps_catalog["url"]) + + # Fetch the json + try: + apps_catalog_content = download_json(actual_api_url) + except Exception as e: + raise YunohostError( + "apps_catalog_failed_to_download", + apps_catalog=apps_catalog_id, + error=str(e), + ) + + # Remember the apps_catalog api version for later + apps_catalog_content["from_api_version"] = APPS_CATALOG_API_VERSION + + # Save the apps_catalog data in the cache + cache_file = "{cache_folder}/{list}.json".format( + cache_folder=APPS_CATALOG_CACHE, list=apps_catalog_id + ) + try: + write_to_json(cache_file, apps_catalog_content) + except Exception as e: + raise YunohostError( + "Unable to write cache data for %s apps_catalog : %s" + % (apps_catalog_id, str(e)) + ) + + logger.success(m18n.n("apps_catalog_update_success")) + + +def _load_apps_catalog(): + """ + Read all the apps catalog cache files and build a single dict (merged_catalog) + corresponding to all known apps and categories + """ + + merged_catalog = {"apps": {}, "categories": []} + + for apps_catalog_id in [L["id"] for L in _read_apps_catalog_list()]: + + # Let's load the json from cache for this catalog + cache_file = "{cache_folder}/{list}.json".format( + cache_folder=APPS_CATALOG_CACHE, list=apps_catalog_id + ) + + try: + apps_catalog_content = ( + read_json(cache_file) if os.path.exists(cache_file) else None + ) + except Exception as e: + raise YunohostError( + "Unable to read cache for apps_catalog %s : %s" % (cache_file, e), + raw_msg=True, + ) + + # Check that the version of the data matches version .... + # ... otherwise it means we updated yunohost in the meantime + # and need to update the cache for everything to be consistent + if ( + not apps_catalog_content + or apps_catalog_content.get("from_api_version") != APPS_CATALOG_API_VERSION + ): + logger.info(m18n.n("apps_catalog_obsolete_cache")) + _update_apps_catalog() + apps_catalog_content = read_json(cache_file) + + del apps_catalog_content["from_api_version"] + + # Add apps from this catalog to the output + for app, info in apps_catalog_content["apps"].items(): + + # (N.B. : there's a small edge case where multiple apps catalog could be listing the same apps ... + # in which case we keep only the first one found) + if app in merged_catalog["apps"]: + logger.warning( + "Duplicate app %s found between apps catalog %s and %s" + % (app, apps_catalog_id, merged_catalog["apps"][app]["repository"]) + ) + continue + + info["repository"] = apps_catalog_id + merged_catalog["apps"][app] = info + + # Annnnd categories + merged_catalog["categories"] += apps_catalog_content["categories"] + + return merged_catalog From 313897d18421e150a119e3de0d66d3fb1f50e340 Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Fri, 1 Oct 2021 03:33:12 +0200 Subject: [PATCH 08/18] Simplify _check_manifest_requirements --- src/yunohost/app.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/yunohost/app.py b/src/yunohost/app.py index f221e2406..87b02cd19 100644 --- a/src/yunohost/app.py +++ b/src/yunohost/app.py @@ -538,7 +538,7 @@ def app_upgrade(app=[], url=None, file=None, force=False, no_safety_backup=False upgrade_type = "UPGRADE_FULL" # Check requirements - _check_manifest_requirements(manifest, app_instance_name=app_instance_name) + _check_manifest_requirements(manifest) _assert_system_is_sane_for_app(manifest, "pre") app_setting_path = os.path.join(APPS_SETTING_PATH, app_instance_name) @@ -753,7 +753,7 @@ def app_install( label = label if label else manifest["name"] # Check requirements - _check_manifest_requirements(manifest, app_id) + _check_manifest_requirements(manifest) _assert_system_is_sane_for_app(manifest, "pre") # Check if app can be forked @@ -2187,7 +2187,7 @@ def _get_all_installed_apps_id(): return all_apps_ids_formatted -def _check_manifest_requirements(manifest: Dict, app: str): +def _check_manifest_requirements(manifest: Dict): """Check if required packages are met from the manifest""" packaging_format = int(manifest.get("packaging_format", 0)) @@ -2199,6 +2199,8 @@ def _check_manifest_requirements(manifest: Dict, app: str): if not requirements: return + app = manifest.get("id", "?") + logger.debug(m18n.n("app_requirements_checking", app=app)) # Iterate over requirements From a26145ece0c41c5b84e359e1b157cc42cb587a85 Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Fri, 1 Oct 2021 03:42:59 +0200 Subject: [PATCH 09/18] Simplify(?) YNH_APP_BASEDIR creation, to happen in _make_env_for_app_script --- src/yunohost/app.py | 23 ++++++++++------------- src/yunohost/backup.py | 13 ++++--------- 2 files changed, 14 insertions(+), 22 deletions(-) diff --git a/src/yunohost/app.py b/src/yunohost/app.py index 87b02cd19..df4dd089c 100644 --- a/src/yunohost/app.py +++ b/src/yunohost/app.py @@ -390,12 +390,11 @@ def app_change_url(operation_logger, app, domain, path): tmp_workdir_for_app = _make_tmp_workdir_for_app(app=app) # Prepare env. var. to pass to script - env_dict = _make_environment_for_app_script(app) + env_dict = _make_environment_for_app_script(app, workdir=tmp_workdir_for_app) env_dict["YNH_APP_OLD_DOMAIN"] = old_domain env_dict["YNH_APP_OLD_PATH"] = old_path env_dict["YNH_APP_NEW_DOMAIN"] = domain env_dict["YNH_APP_NEW_PATH"] = path - env_dict["YNH_APP_BASEDIR"] = tmp_workdir_for_app if domain != old_domain: operation_logger.related_to.append(("domain", old_domain)) @@ -544,12 +543,11 @@ def app_upgrade(app=[], url=None, file=None, force=False, no_safety_backup=False app_setting_path = os.path.join(APPS_SETTING_PATH, app_instance_name) # Prepare env. var. to pass to script - env_dict = _make_environment_for_app_script(app_instance_name) + env_dict = _make_environment_for_app_script(app_instance_name, workdir=extracted_app_folder) env_dict["YNH_APP_UPGRADE_TYPE"] = upgrade_type env_dict["YNH_APP_MANIFEST_VERSION"] = str(app_new_version) env_dict["YNH_APP_CURRENT_VERSION"] = str(app_current_version) env_dict["NO_BACKUP_UPGRADE"] = "1" if no_safety_backup else "0" - env_dict["YNH_APP_BASEDIR"] = extracted_app_folder # We'll check that the app didn't brutally edit some system configuration manually_modified_files_before_install = manually_modified_files() @@ -829,8 +827,7 @@ def app_install( ) # Prepare env. var. to pass to script - env_dict = _make_environment_for_app_script(app_instance_name, args=args) - env_dict["YNH_APP_BASEDIR"] = extracted_app_folder + env_dict = _make_environment_for_app_script(app_instance_name, args=args, workdir=extracted_app_folder) env_dict_for_logging = env_dict.copy() for question in questions: @@ -891,8 +888,7 @@ def app_install( logger.warning(m18n.n("app_remove_after_failed_install")) # Setup environment for remove script - env_dict_remove = _make_environment_for_app_script(app_instance_name) - env_dict_remove["YNH_APP_BASEDIR"] = extracted_app_folder + env_dict_remove = _make_environment_for_app_script(app_instance_name, workdir=extracted_app_folder) # Execute remove script operation_logger_remove = OperationLogger( @@ -1006,8 +1002,7 @@ def app_remove(operation_logger, app, purge=False): env_dict = {} app_id, app_instance_nb = _parse_app_instance_name(app) - env_dict = _make_environment_for_app_script(app) - env_dict["YNH_APP_BASEDIR"] = tmp_workdir_for_app + env_dict = _make_environment_for_app_script(app, workdir=tmp_workdir_for_app) env_dict["YNH_APP_PURGE"] = str(1 if purge else 0) operation_logger.extra.update({"env": env_dict}) @@ -1501,9 +1496,8 @@ def app_action_run(operation_logger, app, action, args=None): tmp_workdir_for_app = _make_tmp_workdir_for_app(app=app) - env_dict = _make_environment_for_app_script(app, args=args, args_prefix="ACTION_") + env_dict = _make_environment_for_app_script(app, args=args, args_prefix="ACTION_", workdir=tmp_workdir_for_app) env_dict["YNH_ACTION"] = action - env_dict["YNH_APP_BASEDIR"] = tmp_workdir_for_app _, action_script = tempfile.mkstemp(dir=tmp_workdir_for_app) @@ -2328,7 +2322,7 @@ def _assert_no_conflicting_apps(domain, path, ignore_app=None, full_domain=False ) -def _make_environment_for_app_script(app, args={}, args_prefix="APP_ARG_"): +def _make_environment_for_app_script(app, args={}, args_prefix="APP_ARG_", workdir=None): app_setting_path = os.path.join(APPS_SETTING_PATH, app) @@ -2342,6 +2336,9 @@ def _make_environment_for_app_script(app, args={}, args_prefix="APP_ARG_"): "YNH_APP_MANIFEST_VERSION": manifest.get("version", "?"), } + if workdir: + env_dict["YNH_APP_BASEDIR"] = workdir + for arg_name, arg_value in args.items(): env_dict["YNH_%s%s" % (args_prefix, arg_name.upper())] = str(arg_value) diff --git a/src/yunohost/backup.py b/src/yunohost/backup.py index b650cb3da..5664af3a0 100644 --- a/src/yunohost/backup.py +++ b/src/yunohost/backup.py @@ -1483,7 +1483,9 @@ class RestoreManager: logger.debug(m18n.n("restore_running_app_script", app=app_instance_name)) # Prepare env. var. to pass to script - env_dict = _make_environment_for_app_script(app_instance_name) + # FIXME : workdir should be a tmp workdir + app_workdir = os.path.join(self.work_dir, "apps", app_instance_name, "settings") + env_dict = _make_environment_for_app_script(app_instance_name, workdir=app_workdir) env_dict.update( { "YNH_BACKUP_DIR": self.work_dir, @@ -1491,9 +1493,6 @@ class RestoreManager: "YNH_APP_BACKUP_DIR": os.path.join( self.work_dir, "apps", app_instance_name, "backup" ), - "YNH_APP_BASEDIR": os.path.join( - self.work_dir, "apps", app_instance_name, "settings" - ), } ) @@ -1530,11 +1529,7 @@ class RestoreManager: remove_script = os.path.join(app_scripts_in_archive, "remove") # Setup environment for remove script - env_dict_remove = _make_environment_for_app_script(app_instance_name) - env_dict_remove["YNH_APP_BASEDIR"] = os.path.join( - self.work_dir, "apps", app_instance_name, "settings" - ) - + env_dict_remove = _make_environment_for_app_script(app_instance_name, workdir=app_workdir) remove_operation_logger = OperationLogger( "remove_on_failed_restore", [("app", app_instance_name)], From 680e13c0442a9adc590e2f6ae78d245b69da3e4d Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Fri, 1 Oct 2021 03:45:40 +0200 Subject: [PATCH 10/18] Make mypy happier --- src/yunohost/app.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/yunohost/app.py b/src/yunohost/app.py index df4dd089c..0f4c14b7a 100644 --- a/src/yunohost/app.py +++ b/src/yunohost/app.py @@ -1988,7 +1988,7 @@ def _app_quality(app: str) -> str: app_name_to_test = app.strip("/").split("/")[-1].replace("_ynh", "") else: # FIXME : watdo if '@' in app ? - app_name_to_test = None + return "thirdparty" if app_name_to_test in raw_app_catalog: From 2bda85c80042f0687415c9ed436db53695d04545 Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Fri, 1 Oct 2021 03:46:13 +0200 Subject: [PATCH 11/18] Stale i18n string --- locales/en.json | 1 - 1 file changed, 1 deletion(-) diff --git a/locales/en.json b/locales/en.json index 5d242b2e8..53b0a7e83 100644 --- a/locales/en.json +++ b/locales/en.json @@ -36,7 +36,6 @@ "app_manifest_install_ask_is_public": "Should this app be exposed to anonymous visitors?", "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_manifest_invalid": "Something is wrong with the app manifest: {error}", "app_not_correctly_installed": "{app} seems to be incorrectly installed", "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", From 171b3b25d9d4cb7aad8b2cbbc5aeb7eee6243a1f Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Fri, 1 Oct 2021 16:13:57 +0200 Subject: [PATCH 12/18] Improve check that arg is a yunohost repo url --- src/yunohost/app.py | 25 +++++++++++++++++++++++-- src/yunohost/tests/test_appurl.py | 30 +++++++++++++++++++++++++++++- 2 files changed, 52 insertions(+), 3 deletions(-) diff --git a/src/yunohost/app.py b/src/yunohost/app.py index 0f4c14b7a..5b6d1a7f8 100644 --- a/src/yunohost/app.py +++ b/src/yunohost/app.py @@ -69,6 +69,10 @@ re_app_instance_name = re.compile( r"^(?P[\w-]+?)(__(?P[1-9][0-9]*))?$" ) +APP_REPO_URL = re.compile( + r"^https://[a-zA-Z0-9-_.]+/[a-zA-Z0-9-_./]+/[a-zA-Z0-9-_.]+_ynh(/?(-/)?tree/[a-zA-Z0-9-_]+)?(\.git)?/?$" +) + APP_FILES_TO_COPY = [ "manifest.json", "manifest.toml", @@ -1971,13 +1975,24 @@ def _set_default_ask_questions(arguments): return arguments +def _is_app_repo_url(string: str) -> bool: + + string = string.strip() + + # Dummy test for ssh-based stuff ... should probably be improved somehow + if '@' in string: + return True + + return APP_REPO_URL.match(string) + + def _app_quality(app: str) -> str: """ app may in fact be an app name, an url, or a path """ raw_app_catalog = _load_apps_catalog()["apps"] - if app in raw_app_catalog or ("@" in app) or ("http://" in app) or ("https://" in app): + if app in raw_app_catalog or _is_app_repo_url(app): # If we got an app name directly (e.g. just "wordpress"), we gonna test this name if app in raw_app_catalog: @@ -2027,10 +2042,14 @@ def _extract_app(src: str) -> Tuple[Dict, str]: revision = str(app_info["git"]["revision"]) return _extract_app_from_gitrepo(url, branch, revision, app_info) # App is a git repo url - elif ("@" in src) or ("http://" in src) or ("https://" in src): + elif _is_app_repo_url(src): url = src branch = "master" revision = "HEAD" + # gitlab urls may look like 'https://domain/org/group/repo/-/tree/testing' + # compated to github urls looking like 'https://domain/org/repo/tree/testing' + if '/-/' in url: + url = url.replace('/-/', '/') if "/tree/" in url: url, branch = url.split("/tree/", 1) return _extract_app_from_gitrepo(url, branch, revision, {}) @@ -2038,6 +2057,8 @@ def _extract_app(src: str) -> Tuple[Dict, str]: elif os.path.exists(src): return _extract_app_from_folder(src) else: + if "http://" in src or "https://" in src: + logger.error(f"{src} is not a valid app url: app url are expected to look like https://domain.tld/path/to/repo_ynh") raise YunohostValidationError("app_unknown") diff --git a/src/yunohost/tests/test_appurl.py b/src/yunohost/tests/test_appurl.py index 5954d239a..d50877445 100644 --- a/src/yunohost/tests/test_appurl.py +++ b/src/yunohost/tests/test_appurl.py @@ -4,7 +4,7 @@ import os from .conftest import get_test_apps_dir from yunohost.utils.error import YunohostError -from yunohost.app import app_install, app_remove +from yunohost.app import app_install, app_remove, _is_app_repo_url from yunohost.domain import _get_maindomain, domain_url_available from yunohost.permission import _validate_and_sanitize_permission_url @@ -28,6 +28,34 @@ def teardown_function(function): pass +def test_repo_url_definition(): + assert _is_app_repo_url("https://github.com/YunoHost-Apps/foobar123_ynh") + assert _is_app_repo_url("https://github.com/YunoHost-Apps/foobar123_ynh/") + assert _is_app_repo_url("https://github.com/YunoHost-Apps/foobar123_ynh.git") + assert _is_app_repo_url("https://github.com/YunoHost-Apps/foobar123_ynh/tree/testing") + assert _is_app_repo_url("https://github.com/YunoHost-Apps/foobar123_ynh/tree/testing/") + assert _is_app_repo_url("https://github.com/YunoHost-Apps/foo-bar-123_ynh") + assert _is_app_repo_url("https://github.com/YunoHost-Apps/foo_bar_123_ynh") + assert _is_app_repo_url("https://github.com/YunoHost-Apps/FooBar123_ynh") + assert _is_app_repo_url("https://github.com/labriqueinternet/vpnclient_ynh") + assert _is_app_repo_url("https://framagit.org/YunoHost/apps/nodebb_ynh") + assert _is_app_repo_url("https://framagit.org/YunoHost/apps/nodebb_ynh/-/tree/testing") + assert _is_app_repo_url("https://gitlab.com/yunohost-apps/foobar_ynh") + assert _is_app_repo_url("https://code.antopie.org/miraty/qr_ynh") + assert _is_app_repo_url("https://gitlab.domainepublic.net/Neutrinet/neutrinet_ynh/-/tree/unstable") + assert _is_app_repo_url("git@github.com:YunoHost-Apps/foobar_ynh.git") + + assert not _is_app_repo_url("github.com/YunoHost-Apps/foobar_ynh") + assert not _is_app_repo_url("http://github.com/YunoHost-Apps/foobar_ynh") + assert not _is_app_repo_url("https://github.com/YunoHost-Apps/foobar_wat") + assert not _is_app_repo_url("https://github.com/YunoHost-Apps/foobar_ynh_wat") + assert not _is_app_repo_url("https://github.com/YunoHost-Apps/foobar/tree/testing") + assert not _is_app_repo_url("https://github.com/YunoHost-Apps/foobar_ynh_wat/tree/testing") + assert not _is_app_repo_url("https://framagit.org/YunoHost/apps/") + assert not _is_app_repo_url("https://framagit.org/YunoHost/apps/pwet") + assert not _is_app_repo_url("https://framagit.org/YunoHost/apps/pwet_foo") + + def test_urlavailable(): # Except the maindomain/macnuggets to be available From 34d9573121a4bd5caea2974f27c20be3cf20726b Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Fri, 1 Oct 2021 16:46:05 +0200 Subject: [PATCH 13/18] Misc fixes for app repo url check / parsing --- src/yunohost/app.py | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/src/yunohost/app.py b/src/yunohost/app.py index 5b6d1a7f8..3fede8dcd 100644 --- a/src/yunohost/app.py +++ b/src/yunohost/app.py @@ -1986,21 +1986,21 @@ def _is_app_repo_url(string: str) -> bool: return APP_REPO_URL.match(string) -def _app_quality(app: str) -> str: +def _app_quality(src: str) -> str: """ app may in fact be an app name, an url, or a path """ raw_app_catalog = _load_apps_catalog()["apps"] - if app in raw_app_catalog or _is_app_repo_url(app): + if src in raw_app_catalog or _is_app_repo_url(src): # If we got an app name directly (e.g. just "wordpress"), we gonna test this name - if app in raw_app_catalog: - app_name_to_test = app + if src in raw_app_catalog: + app_name_to_test = src # If we got an url like "https://github.com/foo/bar_ynh, we want to # extract "bar" and test if we know this app - elif ("http://" in app) or ("https://" in app): - app_name_to_test = app.strip("/").split("/")[-1].replace("_ynh", "") + elif ("http://" in src) or ("https://" in src): + app_name_to_test = src.strip("/").split("/")[-1].replace("_ynh", "") else: # FIXME : watdo if '@' in app ? return "thirdparty" @@ -2018,9 +2018,11 @@ def _app_quality(app: str) -> str: else: return "thirdparty" - elif os.path.exists(app): + elif os.path.exists(src): return "thirdparty" else: + if "http://" in src or "https://" in src: + logger.error(f"{src} is not a valid app url: app url are expected to look like https://domain.tld/path/to/repo_ynh") raise YunohostValidationError("app_unknown") @@ -2043,7 +2045,7 @@ def _extract_app(src: str) -> Tuple[Dict, str]: return _extract_app_from_gitrepo(url, branch, revision, app_info) # App is a git repo url elif _is_app_repo_url(src): - url = src + url = src.strip().strip("/") branch = "master" revision = "HEAD" # gitlab urls may look like 'https://domain/org/group/repo/-/tree/testing' From 310b664fab6c9ebef3b94575956f3654c366211f Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Fri, 1 Oct 2021 19:20:21 +0200 Subject: [PATCH 14/18] Clarify return type for mypy --- src/yunohost/app.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/yunohost/app.py b/src/yunohost/app.py index 3fede8dcd..36b65f873 100644 --- a/src/yunohost/app.py +++ b/src/yunohost/app.py @@ -1983,7 +1983,7 @@ def _is_app_repo_url(string: str) -> bool: if '@' in string: return True - return APP_REPO_URL.match(string) + return bool(APP_REPO_URL.match(string)) def _app_quality(src: str) -> str: From 0f0f26f5c48af161da16eaf4775e95e6bcab25fc Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Fri, 1 Oct 2021 19:25:32 +0200 Subject: [PATCH 15/18] For some reason these tests now wanted to interactively ask for is_public arg ... don't know why it was working before ... --- src/yunohost/tests/test_permission.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/yunohost/tests/test_permission.py b/src/yunohost/tests/test_permission.py index b33c2f213..00799d0fd 100644 --- a/src/yunohost/tests/test_permission.py +++ b/src/yunohost/tests/test_permission.py @@ -1049,7 +1049,7 @@ def test_permission_app_remove(): def test_permission_app_change_url(): app_install( os.path.join(get_test_apps_dir(), "permissions_app_ynh"), - args="domain=%s&domain_2=%s&path=%s&admin=%s" + args="domain=%s&domain_2=%s&path=%s&is_public=1&admin=%s" % (maindomain, other_domains[0], "/urlpermissionapp", "alice"), force=True, ) @@ -1072,7 +1072,7 @@ def test_permission_app_change_url(): def test_permission_protection_management_by_helper(): app_install( os.path.join(get_test_apps_dir(), "permissions_app_ynh"), - args="domain=%s&domain_2=%s&path=%s&admin=%s" + args="domain=%s&domain_2=%s&path=%s&is_public=1&admin=%s" % (maindomain, other_domains[0], "/urlpermissionapp", "alice"), force=True, ) @@ -1135,7 +1135,7 @@ def test_permission_legacy_app_propagation_on_ssowat(): app_install( os.path.join(get_test_apps_dir(), "legacy_app_ynh"), - args="domain=%s&domain_2=%s&path=%s" + args="domain=%s&domain_2=%s&path=%s&is_public=1" % (maindomain, other_domains[0], "/legacy"), force=True, ) From 3daac24443eb02f449d5e1c6ce6b2b91d38c5e7b Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Fri, 1 Oct 2021 19:25:36 +0200 Subject: [PATCH 16/18] Typos --- src/yunohost/app.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/yunohost/app.py b/src/yunohost/app.py index 36b65f873..b6150d152 100644 --- a/src/yunohost/app.py +++ b/src/yunohost/app.py @@ -645,7 +645,7 @@ def app_upgrade(app=[], url=None, file=None, force=False, no_safety_backup=False for file_to_copy in APP_FILES_TO_COPY: os.system(f"rm -rf '{app_setting_path}/{file_to_copy}'") if os.path.exists(os.path.join(extracted_app_folder, file_to_copy)): - os.system(f"cp -R '{extracted_app_folder}/{file_to_copy} {app_setting_path}'") + os.system(f"cp -R '{extracted_app_folder}/{file_to_copy}' '{app_setting_path}'") # Clean and set permissions shutil.rmtree(extracted_app_folder) @@ -2089,7 +2089,7 @@ def _extract_app_from_folder(path: str) -> Tuple[Dict, str]: shutil.rmtree(extracted_app_folder) if path[-1] != "/": path = path + "/" - extract_result = os.system(f"cp -a '{path}' {extracted_app_folder}") + extract_result = os.system(f"cp -a '{path}' '{extracted_app_folder}'") else: extract_result = 1 From 6c03cf2b118b1c0178aad905819e6fe623ff5ec2 Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Fri, 1 Oct 2021 20:22:59 +0200 Subject: [PATCH 17/18] Yoloattempt to try to cleanup some ugly os.system --- src/yunohost/app.py | 44 +++++++++++++++++++----------------------- src/yunohost/dns.py | 4 ++-- src/yunohost/domain.py | 4 ++-- src/yunohost/dyndns.py | 19 +++++++++--------- src/yunohost/hook.py | 5 ++--- src/yunohost/tools.py | 26 +++++++++++-------------- 6 files changed, 46 insertions(+), 56 deletions(-) diff --git a/src/yunohost/app.py b/src/yunohost/app.py index b6150d152..cf4b47681 100644 --- a/src/yunohost/app.py +++ b/src/yunohost/app.py @@ -45,6 +45,10 @@ from moulinette.utils.filesystem import ( read_yaml, write_to_file, write_to_json, + cp, + rm, + chown, + chmod, ) from yunohost.utils import packages @@ -643,15 +647,15 @@ def app_upgrade(app=[], url=None, file=None, force=False, no_safety_backup=False # Replace scripts and manifest and conf (if exists) # Move scripts and manifest to the right place for file_to_copy in APP_FILES_TO_COPY: - os.system(f"rm -rf '{app_setting_path}/{file_to_copy}'") + rm(f'{app_setting_path}/{file_to_copy}', recursive=True, force=True) if os.path.exists(os.path.join(extracted_app_folder, file_to_copy)): - os.system(f"cp -R '{extracted_app_folder}/{file_to_copy}' '{app_setting_path}'") + cp(f'{extracted_app_folder}/{file_to_copy}', f'{app_setting_path}/{file_to_copy}', recursive=True) # Clean and set permissions shutil.rmtree(extracted_app_folder) - os.system("chmod 600 %s" % app_setting_path) - os.system("chmod 400 %s/settings.yml" % app_setting_path) - os.system("chown -R root: %s" % app_setting_path) + chmod(app_setting_path, 0o600) + chmod(f"{app_setting_path}/settings.yml", 0o400) + chown(app_setting_path, "root", recursive=True) # So much win logger.success(m18n.n("app_upgraded", app=app_instance_name)) @@ -816,7 +820,7 @@ def app_install( # Move scripts and manifest to the right place for file_to_copy in APP_FILES_TO_COPY: if os.path.exists(os.path.join(extracted_app_folder, file_to_copy)): - os.system(f"cp -R '{extracted_app_folder}/{file_to_copy}' '{app_setting_path}'") + cp(f'{extracted_app_folder}/{file_to_copy}', f'{app_setting_path}/{file_to_copy}', recursive=True) # Initialize the main permission for the app # The permission is initialized with no url associated, and with tile disabled @@ -955,9 +959,9 @@ def app_install( # Clean and set permissions shutil.rmtree(extracted_app_folder) - os.system("chmod 600 %s" % app_setting_path) - os.system("chmod 400 %s/settings.yml" % app_setting_path) - os.system("chown -R root: %s" % app_setting_path) + chmod(app_setting_path, 0o600) + chmod(f"{app_setting_path}/settings.yml", 0o400) + chown(app_setting_path, "root", recursive=True) logger.success(m18n.n("installation_complete")) @@ -1153,7 +1157,7 @@ def app_makedefault(operation_logger, app, domain=None): write_to_json( "/etc/ssowat/conf.json.persistent", ssowat_conf, sort_keys=True, indent=4 ) - os.system("chmod 644 /etc/ssowat/conf.json.persistent") + chmod("/etc/ssowat/conf.json.persistent", 0o644) logger.success(m18n.n("ssowat_conf_updated")) @@ -2077,24 +2081,16 @@ def _extract_app_from_folder(path: str) -> Tuple[Dict, str]: extracted_app_folder = _make_tmp_workdir_for_app() - if ".zip" in path: - extract_result = os.system( - f"unzip '{path}' -d {extracted_app_folder} > /dev/null 2>&1" - ) - elif ".tar" in path: - extract_result = os.system( - f"tar -xf '{path}' -C {extracted_app_folder} > /dev/null 2>&1" - ) - elif os.path.isdir(path): + if os.path.isdir(path): shutil.rmtree(extracted_app_folder) if path[-1] != "/": path = path + "/" - extract_result = os.system(f"cp -a '{path}' '{extracted_app_folder}'") + cp(path, extracted_app_folder, recursive=True) else: - extract_result = 1 - - if extract_result != 0: - raise YunohostError("app_extraction_failed") + try: + shutil.unpack_archive(path, extracted_app_folder) + except Exception: + raise YunohostError("app_extraction_failed") try: if len(os.listdir(extracted_app_folder)) == 1: diff --git a/src/yunohost/dns.py b/src/yunohost/dns.py index 689950490..b0d40db9e 100644 --- a/src/yunohost/dns.py +++ b/src/yunohost/dns.py @@ -32,7 +32,7 @@ from collections import OrderedDict from moulinette import m18n, Moulinette from moulinette.utils.log import getActionLogger -from moulinette.utils.filesystem import read_file, write_to_file, read_toml +from moulinette.utils.filesystem import read_file, write_to_file, read_toml, mkdir from yunohost.domain import ( domain_list, @@ -471,7 +471,7 @@ def _get_dns_zone_for_domain(domain): # Check if there's a NS record for that domain answer = dig(parent, rdtype="NS", full_answers=True, resolvers="force_external") if answer[0] == "ok": - os.system(f"mkdir -p {cache_folder}") + mkdir(cache_folder, parents=True) write_to_file(cache_file, parent) return parent diff --git a/src/yunohost/domain.py b/src/yunohost/domain.py index f5b14748d..b40831d25 100644 --- a/src/yunohost/domain.py +++ b/src/yunohost/domain.py @@ -29,7 +29,7 @@ from typing import Dict, Any from moulinette import m18n, Moulinette from moulinette.core import MoulinetteError from moulinette.utils.log import getActionLogger -from moulinette.utils.filesystem import write_to_file, read_yaml, write_to_yaml +from moulinette.utils.filesystem import write_to_file, read_yaml, write_to_yaml, rm from yunohost.app import ( app_ssowatconf, @@ -328,7 +328,7 @@ def domain_remove(operation_logger, domain, remove_apps=False, force=False): ] for stuff in stuff_to_delete: - os.system("rm -rf {stuff}") + rm(stuff, force=True, recursive=True) # Sometime we have weird issues with the regenconf where some files # appears as manually modified even though they weren't touched ... diff --git a/src/yunohost/dyndns.py b/src/yunohost/dyndns.py index 519fbc8f0..e33cf4f22 100644 --- a/src/yunohost/dyndns.py +++ b/src/yunohost/dyndns.py @@ -33,7 +33,7 @@ import subprocess from moulinette import m18n from moulinette.core import MoulinetteError from moulinette.utils.log import getActionLogger -from moulinette.utils.filesystem import write_to_file, read_file +from moulinette.utils.filesystem import write_to_file, read_file, rm, chown, chmod from moulinette.utils.network import download_json from yunohost.utils.error import YunohostError, YunohostValidationError @@ -152,13 +152,12 @@ def dyndns_subscribe( os.system( "cd /etc/yunohost/dyndns && " - "dnssec-keygen -a hmac-sha512 -b 512 -r /dev/urandom -n USER %s" - % domain - ) - os.system( - "chmod 600 /etc/yunohost/dyndns/*.key /etc/yunohost/dyndns/*.private" + f"dnssec-keygen -a hmac-sha512 -b 512 -r /dev/urandom -n USER {domain}" ) + chmod("/etc/yunohost/dyndns", 0o600, recursive=True) + chown("/etc/yunohost/dyndns", "root", recursive=True) + private_file = glob.glob("/etc/yunohost/dyndns/*%s*.private" % domain)[0] key_file = glob.glob("/etc/yunohost/dyndns/*%s*.key" % domain)[0] with open(key_file) as f: @@ -175,12 +174,12 @@ def dyndns_subscribe( timeout=30, ) except Exception as e: - os.system("rm -f %s" % private_file) - os.system("rm -f %s" % key_file) + rm(private_file, force=True) + rm(key_file, force=True) raise YunohostError("dyndns_registration_failed", error=str(e)) if r.status_code != 201: - os.system("rm -f %s" % private_file) - os.system("rm -f %s" % key_file) + rm(private_file, force=True) + rm(key_file, force=True) try: error = json.loads(r.text)["error"] except Exception: diff --git a/src/yunohost/hook.py b/src/yunohost/hook.py index c55809fce..20757bf3c 100644 --- a/src/yunohost/hook.py +++ b/src/yunohost/hook.py @@ -34,7 +34,7 @@ from importlib import import_module from moulinette import m18n, Moulinette from yunohost.utils.error import YunohostError, YunohostValidationError from moulinette.utils import log -from moulinette.utils.filesystem import read_yaml +from moulinette.utils.filesystem import read_yaml, cp HOOK_FOLDER = "/usr/share/yunohost/hooks/" CUSTOM_HOOK_FOLDER = "/etc/yunohost/hooks.d/" @@ -60,8 +60,7 @@ def hook_add(app, file): os.makedirs(CUSTOM_HOOK_FOLDER + action) finalpath = CUSTOM_HOOK_FOLDER + action + "/" + priority + "-" + app - os.system("cp %s %s" % (file, finalpath)) - os.system("chown -hR admin: %s" % HOOK_FOLDER) + cp(file, finalpath) return {"hook": finalpath} diff --git a/src/yunohost/tools.py b/src/yunohost/tools.py index 4cdd62d8f..671f8768c 100644 --- a/src/yunohost/tools.py +++ b/src/yunohost/tools.py @@ -34,7 +34,7 @@ from typing import List from moulinette import Moulinette, m18n from moulinette.utils.log import getActionLogger from moulinette.utils.process import check_output, call_async_output -from moulinette.utils.filesystem import read_yaml, write_to_yaml +from moulinette.utils.filesystem import read_yaml, write_to_yaml, cp, mkdir, rm from yunohost.app import ( app_info, @@ -1147,13 +1147,11 @@ class Migration(object): backup_folder = "/home/yunohost.backup/premigration/" + time.strftime( "%Y%m%d-%H%M%S", time.gmtime() ) - os.makedirs(backup_folder, 0o750) + mkdir(backup_folder, 0o750, parents=True) os.system("systemctl stop slapd") - os.system(f"cp -r --preserve /etc/ldap {backup_folder}/ldap_config") - os.system(f"cp -r --preserve /var/lib/ldap {backup_folder}/ldap_db") - os.system( - f"cp -r --preserve /etc/yunohost/apps {backup_folder}/apps_settings" - ) + cp("/etc/ldap", f"{backup_folder}/ldap_config", recursive=True) + cp("/var/lib/ldap", f"{backup_folder}/ldap_db", recursive=True) + cp("/etc/yunohost/apps", f"{backup_folder}/apps_settings", recursive=True) except Exception as e: raise YunohostError( "migration_ldap_can_not_backup_before_migration", error=str(e) @@ -1169,17 +1167,15 @@ class Migration(object): ) os.system("systemctl stop slapd") # To be sure that we don't keep some part of the old config - os.system("rm -r /etc/ldap/slapd.d") - os.system(f"cp -r --preserve {backup_folder}/ldap_config/. /etc/ldap/") - os.system(f"cp -r --preserve {backup_folder}/ldap_db/. /var/lib/ldap/") - os.system( - f"cp -r --preserve {backup_folder}/apps_settings/. /etc/yunohost/apps/" - ) + rm("/etc/ldap/slapd.d", force=True, recursive=True) + cp(f"{backup_folder}/ldap_config", "/etc/ldap", recursive=True) + cp(f"{backup_folder}/ldap_db", "/var/lib/ldap", recursive=True) + cp(f"{backup_folder}/apps_settings", "/etc/yunohost/apps", recursive=True) os.system("systemctl start slapd") - os.system(f"rm -r {backup_folder}") + rm(backup_folder, force=True, recursive=True) logger.info(m18n.n("migration_ldap_rollback_success")) raise else: - os.system(f"rm -r {backup_folder}") + rm(backup_folder, force=True, recursive=True) return func From 4fda262bc0c69b68d2288d9773215b581bab945c Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Sat, 2 Oct 2021 01:59:16 +0200 Subject: [PATCH 18/18] fix tests: Missing mkdir on force --- src/yunohost/dns.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/yunohost/dns.py b/src/yunohost/dns.py index b0d40db9e..534ade918 100644 --- a/src/yunohost/dns.py +++ b/src/yunohost/dns.py @@ -471,7 +471,7 @@ def _get_dns_zone_for_domain(domain): # Check if there's a NS record for that domain answer = dig(parent, rdtype="NS", full_answers=True, resolvers="force_external") if answer[0] == "ok": - mkdir(cache_folder, parents=True) + mkdir(cache_folder, parents=True, force=True) write_to_file(cache_file, parent) return parent