diff --git a/data/helpers.d/config b/data/helpers.d/config index e834db041..62ee228d9 100644 --- a/data/helpers.d/config +++ b/data/helpers.d/config @@ -40,14 +40,14 @@ EOL old[$short_setting]="$($getter)" formats[${short_setting}]="yaml" - elif [[ "$bind" == "null" ]]; + elif [[ "$bind" == "null" ]] then old[$short_setting]="YNH_NULL" # Get value from app settings or from another file - elif [[ "$type" == "file" ]]; + elif [[ "$type" == "file" ]] then - if [[ "$bind" == "settings" ]]; + if [[ "$bind" == "settings" ]] then ynh_die "File '${short_setting}' can't be stored in settings" fi @@ -55,12 +55,12 @@ EOL file_hash[$short_setting]="true" # Get multiline text from settings or from a full file - elif [[ "$type" == "text" ]]; + elif [[ "$type" == "text" ]] then - if [[ "$bind" == "settings" ]]; + if [[ "$bind" == "settings" ]] then old[$short_setting]="$(ynh_app_setting_get $app $short_setting)" - elif [[ "$bind" == *":"* ]]; + elif [[ "$bind" == *":"* ]] then ynh_die "For technical reasons, multiline text '${short_setting}' can't be stored automatically in a variable file, you have to create custom getter/setter" else @@ -69,7 +69,7 @@ EOL # Get value from a kind of key/value file else - if [[ "$bind" == "settings" ]]; + if [[ "$bind" == "settings" ]] then bind=":/etc/yunohost/apps/$app/settings.yml" fi @@ -90,26 +90,26 @@ _ynh_app_config_apply() { local setter="set__${short_setting}" local bind="${binds[$short_setting]}" local type="${types[$short_setting]}" - if [ "${changed[$short_setting]}" == "true" ]; + if [ "${changed[$short_setting]}" == "true" ] then # Apply setter if exists if type -t $setter 2>/dev/null | grep -q '^function$' 2>/dev/null; then $setter - elif [[ "$bind" == "null" ]]; + elif [[ "$bind" == "null" ]] then continue # Save in a file - elif [[ "$type" == "file" ]]; + elif [[ "$type" == "file" ]] then - if [[ "$bind" == "settings" ]]; + if [[ "$bind" == "settings" ]] then ynh_die "File '${short_setting}' can't be stored in settings" fi local bind_file="$(echo "$bind" | sed s@__FINALPATH__@$final_path@ | sed s/__APP__/$app/)" - if [[ "${!short_setting}" == "" ]]; + if [[ "${!short_setting}" == "" ]] then ynh_backup_if_checksum_is_different --file="$bind_file" rm -f "$bind_file" @@ -123,15 +123,15 @@ _ynh_app_config_apply() { fi # Save value in app settings - elif [[ "$bind" == "settings" ]]; + elif [[ "$bind" == "settings" ]] then ynh_app_setting_set $app $short_setting "${!short_setting}" ynh_print_info "Configuration key '$short_setting' edited in app settings" # Save multiline text in a file - elif [[ "$type" == "text" ]]; + elif [[ "$type" == "text" ]] then - if [[ "$bind" == *":"* ]]; + if [[ "$bind" == *":"* ]] then ynh_die "For technical reasons, multiline text '${short_setting}' can't be stored automatically in a variable file, you have to create custom getter/setter" fi @@ -169,9 +169,9 @@ _ynh_app_config_apply() { _ynh_app_config_show() { for short_setting in "${!old[@]}" do - if [[ "${old[$short_setting]}" != YNH_NULL ]]; + if [[ "${old[$short_setting]}" != YNH_NULL ]] then - if [[ "${formats[$short_setting]}" == "yaml" ]]; + if [[ "${formats[$short_setting]}" == "yaml" ]] then ynh_return "${short_setting}:" ynh_return "$(echo "${old[$short_setting]}" | sed 's/^/ /g')" @@ -192,27 +192,27 @@ _ynh_app_config_validate() { for short_setting in "${!old[@]}" do changed[$short_setting]=false - if [ -z ${!short_setting+x} ]; + if [ -z ${!short_setting+x} ] then # Assign the var with the old value in order to allows multiple # args validation declare "$short_setting"="${old[$short_setting]}" continue fi - if [ ! -z "${file_hash[${short_setting}]}" ]; + if [ ! -z "${file_hash[${short_setting}]}" ] then file_hash[old__$short_setting]="" file_hash[new__$short_setting]="" - if [ -f "${old[$short_setting]}" ]; + if [ -f "${old[$short_setting]}" ] then file_hash[old__$short_setting]=$(sha256sum "${old[$short_setting]}" | cut -d' ' -f1) - if [ -z "${!short_setting}" ]; + if [ -z "${!short_setting}" ] then changed[$short_setting]=true nothing_changed=false fi fi - if [ -f "${!short_setting}" ]; + if [ -f "${!short_setting}" ] then file_hash[new__$short_setting]=$(sha256sum "${!short_setting}" | cut -d' ' -f1) if [[ "${file_hash[old__$short_setting]}" != "${file_hash[new__$short_setting]}" ]] @@ -248,8 +248,20 @@ _ynh_app_config_validate() { fi if [ -n "$result" ] then - local key="YNH_ERROR_${short_setting}" - ynh_return "$key: \"$result\"" + # + # Return a yaml such as: + # + # validation_errors: + # some_key: "An error message" + # some_other_key: "Another error message" + # + # We use changes_validated to know if this is + # the first validation error + if [[ "$changes_validated" == true ]] + then + ynh_return "validation_errors:" + fi + ynh_return " ${short_setting}: \"$result\"" changes_validated=false fi done diff --git a/src/yunohost/app.py b/src/yunohost/app.py index e9712edb4..4047369e0 100644 --- a/src/yunohost/app.py +++ b/src/yunohost/app.py @@ -1695,9 +1695,7 @@ def app_config_set( Question.operation_logger = operation_logger - result = config_.set(key, value, args, args_file, operation_logger=operation_logger) - - return result + return config_.set(key, value, args, args_file, operation_logger=operation_logger) class AppConfigPanel(ConfigPanel): @@ -1715,7 +1713,18 @@ class AppConfigPanel(ConfigPanel): def _apply(self): env = {key: str(value) for key, value in self.new_values.items()} - self.errors = self._call_config_script("apply", env=env) + return_content = self._call_config_script("apply", env=env) + + # If the script returned validation error + # raise a ValidationError exception using + # the first key + if return_content: + for key, message in return_content.get("validation_errors").items(): + raise YunohostValidationError( + "app_argument_invalid", + name=key, + error=message, + ) def _call_config_script(self, action, env={}): from yunohost.hook import hook_exec diff --git a/src/yunohost/tests/test_app_config.py b/src/yunohost/tests/test_app_config.py index af792c431..4ace0aaf9 100644 --- a/src/yunohost/tests/test_app_config.py +++ b/src/yunohost/tests/test_app_config.py @@ -5,8 +5,11 @@ import pytest from .conftest import get_test_apps_dir +from moulinette.utils.filesystem import read_file + from yunohost.domain import _get_maindomain from yunohost.app import ( + app_setting, app_install, app_remove, _is_installed, @@ -15,7 +18,7 @@ from yunohost.app import ( app_ssowatconf, ) -from yunohost.utils.error import YunohostValidationError +from yunohost.utils.error import YunohostError, YunohostValidationError def setup_function(function): @@ -128,17 +131,68 @@ def test_app_config_get_nonexistentstuff(config_app): app_config_get(config_app, "main.components.nonexistent") -def test_app_config_set_boolean(config_app): +def test_app_config_regular_setting(config_app): assert app_config_get(config_app, "main.components.boolean") is None app_config_set(config_app, "main.components.boolean", "no") assert app_config_get(config_app, "main.components.boolean") == "0" + assert app_setting(config_app, "boolean") == "0" app_config_set(config_app, "main.components.boolean", "yes") assert app_config_get(config_app, "main.components.boolean") == "1" + assert app_setting(config_app, "boolean") == "1" with pytest.raises(YunohostValidationError): app_config_set(config_app, "main.components.boolean", "pwet") + + +def test_app_config_bind_on_file(config_app): + + # c.f. conf/test.php in the config app + assert '$arg5= "Arg5 value";' in read_file("/var/www/config_app/test.php") + assert app_config_get(config_app, "bind.variable.arg5") == "Arg5 value" + assert app_setting(config_app, "arg5") is None + + app_config_set(config_app, "bind.variable.arg5", "Foo Bar") + + assert '$arg5= "Foo Bar";' in read_file("/var/www/config_app/test.php") + assert app_config_get(config_app, "bind.variable.arg5") == "Foo Bar" + assert app_setting(config_app, "arg5") == "Foo Bar" + + +def test_app_config_custom_get(config_app): + + assert app_setting(config_app, "arg9") is None + assert "Files in /var/www" in app_config_get(config_app, "bind.function.arg9")["ask"]["en"] + assert app_setting(config_app, "arg9") is None + + +def test_app_config_custom_validator(config_app): + + # c.f. the config script + # arg8 is a password that must be at least 8 chars + assert not os.path.exists("/var/www/config_app/password") + assert app_setting(config_app, "arg8") is None + + with pytest.raises(YunohostValidationError): + app_config_set(config_app, "bind.function.arg8", "pZo6i7u91h") + + assert not os.path.exists("/var/www/config_app/password") + assert app_setting(config_app, "arg8") is None + + +def test_app_config_custom_set(config_app): + + assert not os.path.exists("/var/www/config_app/password") + assert app_setting(config_app, "arg8") is None + + app_config_set(config_app, "bind.function.arg8", "OneSuperStrongPassword") + + assert os.path.exists("/var/www/config_app/password") + content = read_file("/var/www/config_app/password") + assert "OneSuperStrongPassword" not in content + assert content.startswith("$6$saltsalt$") + assert app_setting(config_app, "arg8") is None diff --git a/src/yunohost/utils/config.py b/src/yunohost/utils/config.py index 744849199..6b491386f 100644 --- a/src/yunohost/utils/config.py +++ b/src/yunohost/utils/config.py @@ -135,6 +135,8 @@ class ConfigPanel: try: self._apply() + except YunohostError: + raise # Script got manually interrupted ... # N.B. : KeyboardInterrupt does not inherit from Exception except (KeyboardInterrupt, EOFError): @@ -152,16 +154,10 @@ class ConfigPanel: # Delete files uploaded from API FileQuestion.clean_upload_dirs() - if self.errors: - return { - "errors": self.errors, - } - self._reload_services() logger.success("Config updated as expected") operation_logger.success() - return {} def _get_toml(self): return read_toml(self.config_path)