From 22681a4f241c074feaa628b5c8d28716d65812e9 Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Tue, 28 Feb 2023 18:50:31 +0100 Subject: [PATCH 1/3] Kill the old 'unprotected/protected/skipped' setting hell --- helpers/setting | 18 ++------ src/app.py | 115 ------------------------------------------------ 2 files changed, 3 insertions(+), 130 deletions(-) diff --git a/helpers/setting b/helpers/setting index a2cf3a93d..a89f72091 100644 --- a/helpers/setting +++ b/helpers/setting @@ -18,11 +18,7 @@ ynh_app_setting_get() { ynh_handle_getopts_args "$@" app="${app:-$_globalapp}" - if [[ $key =~ (unprotected|protected|skipped)_ ]]; then - yunohost app setting $app $key - else - ynh_app_setting "get" "$app" "$key" - fi + ynh_app_setting "get" "$app" "$key" } # Set an application setting @@ -45,11 +41,7 @@ ynh_app_setting_set() { ynh_handle_getopts_args "$@" app="${app:-$_globalapp}" - if [[ $key =~ (unprotected|protected|skipped)_ ]]; then - yunohost app setting $app $key -v $value - else - ynh_app_setting "set" "$app" "$key" "$value" - fi + ynh_app_setting "set" "$app" "$key" "$value" } # Delete an application setting @@ -70,11 +62,7 @@ ynh_app_setting_delete() { ynh_handle_getopts_args "$@" app="${app:-$_globalapp}" - if [[ "$key" =~ (unprotected|skipped|protected)_ ]]; then - yunohost app setting $app $key -d - else - ynh_app_setting "delete" "$app" "$key" - fi + ynh_app_setting "delete" "$app" "$key" } # Small "hard-coded" interface to avoid calling "yunohost app" directly each diff --git a/src/app.py b/src/app.py index f17c46929..65402a024 100644 --- a/src/app.py +++ b/src/app.py @@ -1492,119 +1492,6 @@ def app_setting(app, key, value=None, delete=False): """ app_settings = _get_app_settings(app) or {} - # - # Legacy permission setting management - # (unprotected, protected, skipped_uri/regex) - # - - is_legacy_permission_setting = any( - key.startswith(word + "_") for word in ["unprotected", "protected", "skipped"] - ) - - if is_legacy_permission_setting: - from yunohost.permission import ( - user_permission_list, - user_permission_update, - permission_create, - permission_delete, - permission_url, - ) - - permissions = user_permission_list(full=True, apps=[app])["permissions"] - key_ = key.split("_")[0] - permission_name = f"{app}.legacy_{key_}_uris" - permission = permissions.get(permission_name) - - # GET - if value is None and not delete: - return ( - ",".join(permission.get("uris", []) + permission["additional_urls"]) - if permission - else None - ) - - # DELETE - if delete: - # If 'is_public' setting still exists, we interpret this as - # coming from a legacy app (because new apps shouldn't manage the - # is_public state themselves anymore...) - # - # In that case, we interpret the request for "deleting - # unprotected/skipped" setting as willing to make the app - # private - if ( - "is_public" in app_settings - and "visitors" in permissions[app + ".main"]["allowed"] - ): - if key.startswith("unprotected_") or key.startswith("skipped_"): - user_permission_update(app + ".main", remove="visitors") - - if permission: - permission_delete(permission_name) - - # SET - else: - urls = value - # If the request is about the root of the app (/), ( = the vast majority of cases) - # we interpret this as a change for the main permission - # (i.e. allowing/disallowing visitors) - if urls == "/": - if key.startswith("unprotected_") or key.startswith("skipped_"): - permission_url(app + ".main", url="/", sync_perm=False) - user_permission_update(app + ".main", add="visitors") - else: - user_permission_update(app + ".main", remove="visitors") - else: - urls = urls.split(",") - if key.endswith("_regex"): - urls = ["re:" + url for url in urls] - - if permission: - # In case of new regex, save the urls, to add a new time in the additional_urls - # In case of new urls, we do the same thing but inversed - if key.endswith("_regex"): - # List of urls to save - current_urls_or_regex = [ - url - for url in permission["additional_urls"] - if not url.startswith("re:") - ] - else: - # List of regex to save - current_urls_or_regex = [ - url - for url in permission["additional_urls"] - if url.startswith("re:") - ] - - new_urls = urls + current_urls_or_regex - # We need to clear urls because in the old setting the new setting override the old one and dont just add some urls - permission_url(permission_name, clear_urls=True, sync_perm=False) - permission_url(permission_name, add_url=new_urls) - else: - from yunohost.utils.legacy import legacy_permission_label - - # Let's create a "special" permission for the legacy settings - permission_create( - permission=permission_name, - # FIXME find a way to limit to only the user allowed to the main permission - allowed=["all_users"] - if key.startswith("protected_") - else ["all_users", "visitors"], - url=None, - additional_urls=urls, - auth_header=not key.startswith("skipped_"), - label=legacy_permission_label(app, key.split("_")[0]), - show_tile=False, - protected=True, - ) - - return - - # - # Regular setting management - # - # GET if value is None and not delete: return app_settings.get(key, None) @@ -1616,8 +1503,6 @@ def app_setting(app, key, value=None, delete=False): # SET else: - if key in ["redirected_urls", "redirected_regex"]: - value = yaml.safe_load(value) app_settings[key] = value _set_app_settings(app, app_settings) From 04eadd715c8723e43113461b92c7493a76760e02 Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Tue, 28 Feb 2023 18:55:47 +0100 Subject: [PATCH 2/3] Kill old --package option and .ini for PHP FPM config --- helpers/php | 71 ++--------------------------------------------------- 1 file changed, 2 insertions(+), 69 deletions(-) diff --git a/helpers/php b/helpers/php index 417dbbc61..1b28b32f7 100644 --- a/helpers/php +++ b/helpers/php @@ -15,7 +15,7 @@ YNH_PHP_VERSION=${YNH_PHP_VERSION:-$YNH_DEFAULT_PHP_VERSION} # # ----------------------------------------------------------------------------- # -# usage 2: ynh_add_fpm_config [--phpversion=7.X] --usage=usage --footprint=footprint [--package=packages] [--dedicated_service] +# usage 2: ynh_add_fpm_config [--phpversion=7.X] --usage=usage --footprint=footprint [--dedicated_service] # | arg: -v, --phpversion= - Version of PHP to use. # | arg: -f, --footprint= - Memory footprint of the service (low/medium/high). # low - Less than 20 MB of RAM by pool. @@ -30,7 +30,6 @@ YNH_PHP_VERSION=${YNH_PHP_VERSION:-$YNH_DEFAULT_PHP_VERSION} # medium - Low usage, few people or/and publicly accessible. # high - High usage, frequently visited website. # -# | arg: -p, --package= - Additionnal PHP packages to install for a specific version of PHP # | arg: -d, --dedicated_service - Use a dedicated PHP-FPM service instead of the common one. # # @@ -60,16 +59,14 @@ ynh_add_fpm_config() { local _globalphpversion=${phpversion-:} # Declare an array to define the options of this helper. local legacy_args=vtufpd - local -A args_array=([v]=phpversion= [t]=use_template [u]=usage= [f]=footprint= [p]=package= [d]=dedicated_service) + local -A args_array=([v]=phpversion= [t]=use_template [u]=usage= [f]=footprint= [d]=dedicated_service) local phpversion local use_template local usage local footprint - local package local dedicated_service # Manage arguments with getopts ynh_handle_getopts_args "$@" - package=${package:-} # The default behaviour is to use the template. use_template="${use_template:-1}" @@ -103,13 +100,6 @@ ynh_add_fpm_config() { fi fi - # Legacy args (packager should just list their php dependency as regular apt dependencies... - if [ -n "$package" ]; then - # Install the additionnal packages from the default repository - ynh_print_warn --message "Argument --package of ynh_add_fpm_config is deprecated and to be removed in the future" - ynh_install_app_dependencies "$package" - fi - if [ $dedicated_service -eq 1 ]; then local fpm_service="${app}-phpfpm" local fpm_config_dir="/etc/php/$phpversion/dedicated-fpm" @@ -197,11 +187,6 @@ pm.process_idle_timeout = 10s local finalphpconf="$fpm_config_dir/pool.d/$app.conf" ynh_add_config --template="$phpfpm_path" --destination="$finalphpconf" - if [ -e "$YNH_APP_BASEDIR/conf/php-fpm.ini" ]; then - ynh_print_warn --message="Packagers ! Please do not use a separate php ini file, merge your directives in the pool file instead." - ynh_add_config --template="php-fpm.ini" --destination="$fpm_config_dir/conf.d/20-$app.ini" - fi - if [ $dedicated_service -eq 1 ]; then # Create a dedicated php-fpm.conf for the service local globalphpconf=$fpm_config_dir/php-fpm-$app.conf @@ -272,9 +257,6 @@ ynh_remove_fpm_config() { fi ynh_secure_remove --file="$fpm_config_dir/pool.d/$app.conf" - if [ -e $fpm_config_dir/conf.d/20-$app.ini ]; then - ynh_secure_remove --file="$fpm_config_dir/conf.d/20-$app.ini" - fi if [ $dedicated_service -eq 1 ]; then # Remove the dedicated service PHP-FPM service for the app @@ -286,55 +268,6 @@ ynh_remove_fpm_config() { elif ynh_package_is_installed --package="php${phpversion}-fpm"; then ynh_systemd_action --service_name=$fpm_service --action=reload fi - - # If the PHP version used is not the default version for YunoHost - # The second part with YNH_APP_PURGE is an ugly hack to guess that we're inside the remove script - # (we don't actually care about its value, we just check its not empty hence it exists) - if [ "$phpversion" != "$YNH_DEFAULT_PHP_VERSION" ] && [ -n "${YNH_APP_PURGE:-}" ] && dpkg --compare-versions ${YNH_APP_PACKAGING_FORMAT:-0} lt 2; then - # Remove app dependencies ... but ideally should happen via an explicit call from packager - ynh_remove_app_dependencies - fi -} - -# Install another version of PHP. -# -# [internal] -# -# Legacy, to be remove on bullseye -# -# usage: ynh_install_php --phpversion=phpversion [--package=packages] -# | arg: -v, --phpversion= - Version of PHP to install. -# | arg: -p, --package= - Additionnal PHP packages to install -# -# Requires YunoHost version 3.8.1 or higher. -ynh_install_php() { - # Declare an array to define the options of this helper. - local legacy_args=vp - local -A args_array=([v]=phpversion= [p]=package=) - local phpversion - local package - # Manage arguments with getopts - ynh_handle_getopts_args "$@" - package=${package:-} - - if [ "$phpversion" == "$YNH_DEFAULT_PHP_VERSION" ]; then - ynh_die --message="Do not use ynh_install_php to install php$YNH_DEFAULT_PHP_VERSION" - fi - - ynh_install_app_dependencies "$package" -} - -# Remove the specific version of PHP used by the app. -# -# [internal] -# -# Legacy, to be remove on bullseye -# -# usage: ynh_remove_php -# -# Requires YunoHost version 3.8.1 or higher. -ynh_remove_php () { - ynh_remove_app_dependencies } # Define the values to configure PHP-FPM From 81b96ad6d45c16eeae0bbcd4774bec139b63ca08 Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Tue, 13 Jun 2023 21:30:20 +0200 Subject: [PATCH 3/3] tests: tmp tweaks to adapt for removed deprecated features --- src/tests/test_backuprestore.py | 5 ++++- src/tests/test_permission.py | 6 +++--- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/src/tests/test_backuprestore.py b/src/tests/test_backuprestore.py index 873deec7d..eb59d4fea 100644 --- a/src/tests/test_backuprestore.py +++ b/src/tests/test_backuprestore.py @@ -439,7 +439,8 @@ def test_backup_using_copy_method(): # App restore # # - +# FIXME : switch to a backup from 11.x +@pytest.mark.skip @pytest.mark.with_wordpress_archive_from_4p2 @pytest.mark.with_custom_domain("yolo.test") def test_restore_app_wordpress_from_Ynh4p2(): @@ -504,6 +505,8 @@ def test_restore_app_not_in_backup(mocker): assert not _is_installed("yoloswag") +# FIXME : switch to a backup from 11.x +@pytest.mark.skip @pytest.mark.with_wordpress_archive_from_4p2 @pytest.mark.with_custom_domain("yolo.test") def test_restore_app_already_installed(mocker): diff --git a/src/tests/test_permission.py b/src/tests/test_permission.py index 8620e9611..dc9121745 100644 --- a/src/tests/test_permission.py +++ b/src/tests/test_permission.py @@ -1131,7 +1131,7 @@ def test_permission_app_propagation_on_ssowat(): 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&is_public=1" + args="domain=%s&domain_2=%s&path=%s&is_public=0" % (maindomain, other_domains[0], "/legacy"), force=True, ) @@ -1139,12 +1139,12 @@ def test_permission_legacy_app_propagation_on_ssowat(): # App is configured as public by default using the legacy unprotected_uri mechanics # It should automatically be migrated during the install res = user_permission_list(full=True)["permissions"] - assert "visitors" in res["legacy_app.main"]["allowed"] + assert "visitors" not in res["legacy_app.main"]["allowed"] assert "all_users" in res["legacy_app.main"]["allowed"] app_webroot = "https://%s/legacy" % maindomain - assert can_access_webpage(app_webroot, logged_as=None) + assert not can_access_webpage(app_webroot, logged_as=None) assert can_access_webpage(app_webroot, logged_as="alice") # Try to update the permission and check that permissions are still consistent