From 2642b64af5bccb6f93a8612a42365edd68e9b118 Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Mon, 16 Sep 2019 23:17:46 +0200 Subject: [PATCH 1/4] Detect and warn early about unavailable full domain requirement... --- locales/en.json | 1 + src/yunohost/app.py | 37 +++++++++++++++++++++++++++++++------ 2 files changed, 32 insertions(+), 6 deletions(-) diff --git a/locales/en.json b/locales/en.json index 61fdcfa9b..4e9675cac 100644 --- a/locales/en.json +++ b/locales/en.json @@ -19,6 +19,7 @@ "app_change_url_no_script": "This application '{app_name:s}' doesn't support URL modification yet. Maybe you should upgrade it.", "app_change_url_success": "{app:s} URL is now {domain:s}{path:s}", "app_extraction_failed": "Could not extract the installation files", + "app_full_domain_unavailable": "Sorry, this application requires a full domain to be installed on, but some other apps are already installed on {domain}. One possible solution is to add a subdomain dedicated to this application.", "app_id_invalid": "Invalid app ID", "app_incompatible": "The app {app} is incompatible with your YunoHost version", "app_install_files_invalid": "These files cannot be installed", diff --git a/src/yunohost/app.py b/src/yunohost/app.py index 5a51e57bb..6f5c1dabe 100644 --- a/src/yunohost/app.py +++ b/src/yunohost/app.py @@ -847,6 +847,9 @@ def app_install(operation_logger, app, label=None, args=None, no_remove_on_failu args_list = [ value[0] for value in args_odict.values() ] args_list.append(app_instance_name) + # Validate domain / path availability for webapps + _validate_and_normalize_webpath(manifest, args_odict, extracted_app_folder) + # Prepare env. var. to pass to script env_dict = _make_environment_dict(args_odict) env_dict["YNH_APP_ID"] = app_id @@ -2547,8 +2550,7 @@ def _parse_args_for_action(action, args={}): def _parse_args_in_yunohost_format(args, action_args): """Parse arguments store in either manifest.json or actions.json """ - from yunohost.domain import (domain_list, _get_maindomain, - _get_conflicting_apps, _normalize_domain_path) + from yunohost.domain import domain_list, _get_maindomain from yunohost.user import user_info, user_list args_dict = OrderedDict() @@ -2666,13 +2668,18 @@ def _parse_args_in_yunohost_format(args, action_args): assert_password_is_strong_enough('user', arg_value) args_dict[arg_name] = (arg_value, arg_type) - # END loop over action_args... + return args_dict + + +def _validate_and_normalize_webpath(manifest, args_dict, app_folder): + + from yunohost.domain import _get_conflicting_apps, _normalize_domain_path # If there's only one "domain" and "path", validate that domain/path # is an available url and normalize the path. - domain_args = [ (name, value[0]) for name, value in args_dict.items() if value[1] == "domain" ] - path_args = [ (name, value[0]) for name, value in args_dict.items() if value[1] == "path" ] + domain_args = [(name, value[0]) for name, value in args_dict.items() if value[1] == "domain"] + path_args = [(name, value[0]) for name, value in args_dict.items() if value[1] == "path"] if len(domain_args) == 1 and len(path_args) == 1: @@ -2698,7 +2705,25 @@ def _parse_args_in_yunohost_format(args, action_args): # standard path format to deal with no matter what the user inputted) args_dict[path_args[0][0]] = (path, "path") - return args_dict + # This is likely to be a full-domain app... + elif len(domain_args) == 1 and len(path_args) == 0: + + # Confirm that this is a full-domain app This should cover most cases + # ... though anyway the proper solution is to implement some mechanism + # in the manifest for app to declare that they require a full domain + # (among other thing) so that we can dynamically check/display this + # requirement on the webadmin form and not miserably fail at submit time + + # Full-domain apps typically declare something like path_url="/" or path=/ + # and use ynh_webpath_register or yunohost_app_checkurl inside the install script + install_script_content = open(os.path.join(app_folder, 'scripts/install')).read() + if re.search(r"\npath(_url)?=[\"']?/[\"']?\n", install_script_content) \ + and re.search(r"(ynh_webpath_register|yunohost app checkurl)"): + + domain = domain_args[0][1] + conflicts = _get_conflicting_apps(domain, "/") + + raise YunohostError('app_full_domain_unavailable', domain) def _make_environment_dict(args_dict, prefix="APP_ARG_"): From 0d90133bb7a0181621824477a855f01256885ae6 Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Tue, 8 Oct 2019 18:18:27 +0200 Subject: [PATCH 2/4] Improve message --- locales/en.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/locales/en.json b/locales/en.json index 4e9675cac..1be7d1151 100644 --- a/locales/en.json +++ b/locales/en.json @@ -19,7 +19,7 @@ "app_change_url_no_script": "This application '{app_name:s}' doesn't support URL modification yet. Maybe you should upgrade it.", "app_change_url_success": "{app:s} URL is now {domain:s}{path:s}", "app_extraction_failed": "Could not extract the installation files", - "app_full_domain_unavailable": "Sorry, this application requires a full domain to be installed on, but some other apps are already installed on {domain}. One possible solution is to add a subdomain dedicated to this application.", + "app_full_domain_unavailable": "Sorry, this application requires a full domain to be installed on, but some other apps are already installed on domain '{domain}'. One possible solution is to add and use a subdomain dedicated to this application instead.", "app_id_invalid": "Invalid app ID", "app_incompatible": "The app {app} is incompatible with your YunoHost version", "app_install_files_invalid": "These files cannot be installed", From 342fe2d4be0a1300dddf0e747906cb4e16b9b091 Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Tue, 8 Oct 2019 18:19:50 +0200 Subject: [PATCH 3/4] Add unit test for full-domain apps --- src/yunohost/tests/test_apps.py | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/src/yunohost/tests/test_apps.py b/src/yunohost/tests/test_apps.py index 9c85df1e9..fc44ef105 100644 --- a/src/yunohost/tests/test_apps.py +++ b/src/yunohost/tests/test_apps.py @@ -36,16 +36,22 @@ def clean(): if _is_installed("legacy_app"): app_remove("legacy_app") + if _is_installed("full_domain_app"): + app_remove("full_domain_app") + to_remove = [] to_remove += glob.glob("/etc/nginx/conf.d/*.d/*legacy*") + to_remove += glob.glob("/etc/nginx/conf.d/*.d/*full_domain*") to_remove += glob.glob("/etc/nginx/conf.d/*.d/*break_yo_system*") for filepath in to_remove: os.remove(filepath) to_remove = [] to_remove += glob.glob("/etc/yunohost/apps/*legacy_app*") + to_remove += glob.glob("/etc/yunohost/apps/*full_domain_app*") to_remove += glob.glob("/etc/yunohost/apps/*break_yo_system*") to_remove += glob.glob("/var/www/*legacy*") + to_remove += glob.glob("/var/www/*full_domain*") for folderpath in to_remove: shutil.rmtree(folderpath, ignore_errors=True) @@ -120,6 +126,13 @@ def install_legacy_app(domain, path): force=True) +def install_full_domain_app(domain): + + app_install("./tests/apps/full_domain_app_ynh", + args="domain=%s" % domain, + force=True) + + def install_break_yo_system(domain, breakwhat): app_install("./tests/apps/break_yo_system_ynh", @@ -272,6 +285,22 @@ def test_legacy_app_failed_remove(secondary_domain): assert app_is_not_installed(secondary_domain, "legacy") +def test_full_domain_app(secondary_domain): + + install_full_domain_app(secondary_domain) + + assert app_is_exposed_on_http(secondary_domain, "/", "This is a dummy app") + + +def test_full_domain_app_with_conflicts(secondary_domain): + + install_legacy_app(secondary_domain, "/legacy") + + # TODO : once #808 is merged, add test that the message raised is 'app_full_domain_unavailable' + with pytest.raises(YunohostError): + install_full_domain_app(secondary_domain) + + def test_systemfuckedup_during_app_install(secondary_domain): with pytest.raises(YunohostError): From c70418c4b25952f9308b69d88e47b8a2490781c1 Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Tue, 8 Oct 2019 18:21:04 +0200 Subject: [PATCH 4/4] Fixes following tests --- src/yunohost/app.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/yunohost/app.py b/src/yunohost/app.py index 6f5c1dabe..8596f7297 100644 --- a/src/yunohost/app.py +++ b/src/yunohost/app.py @@ -2717,13 +2717,14 @@ def _validate_and_normalize_webpath(manifest, args_dict, app_folder): # Full-domain apps typically declare something like path_url="/" or path=/ # and use ynh_webpath_register or yunohost_app_checkurl inside the install script install_script_content = open(os.path.join(app_folder, 'scripts/install')).read() + if re.search(r"\npath(_url)?=[\"']?/[\"']?\n", install_script_content) \ - and re.search(r"(ynh_webpath_register|yunohost app checkurl)"): + and re.search(r"(ynh_webpath_register|yunohost app checkurl)", install_script_content): domain = domain_args[0][1] conflicts = _get_conflicting_apps(domain, "/") - raise YunohostError('app_full_domain_unavailable', domain) + raise YunohostError('app_full_domain_unavailable', domain=domain) def _make_environment_dict(args_dict, prefix="APP_ARG_"):