diff --git a/scripts/app/install/app_install.sh b/scripts/app/install/app_install.sh index dee38ef..3d1330f 100644 --- a/scripts/app/install/app_install.sh +++ b/scripts/app/install/app_install.sh @@ -96,9 +96,30 @@ installApp() # install if nothing set — installApp called directly without flag = install. local actions="${!app_slug:-i}" + # Which drive this app goes on. It arrives either as `app install + # --storage=` or as the storage dropdown in the App Center's install + # form (which travels inside config_variables); both resolve to one answer, + # and the ordering that makes it stick is spelled out in + # scripts/storage/storage_place.sh. + local _storage_want + _storage_want=$(storageChoiceFor "$app_slug" "$config_variables") + if [[ -n "$_storage_want" ]]; then + if ! storagePlaceAppPre "$app_slug" "$_storage_want"; then + isError "Not installing $app_slug." + return 1 + fi + # So the override pass further down writes the choice rather than + # resetting it to the template's "default". + config_variables=$(storageChoiceMerge "$app_slug" "$_storage_want" "$config_variables") + fi + # Setup phase shared by every action (folder + variables). if [[ "$actions" == *[cCtTuUsSrRiI]* ]]; then dockerConfigSetupToContainer silent "$app_slug" + # Between the copy and the source: the fresh template says "default", + # and sourcing that would send every later appDir in this process to the + # primary root while the directory sits elsewhere. + storagePlaceAppPost "$app_slug" "$_storage_want" initializeAppVariables "$app_name" fi diff --git a/scripts/cli/commands/app/cli_app_commands.sh b/scripts/cli/commands/app/cli_app_commands.sh index be2e101..a8a7e63 100755 --- a/scripts/cli/commands/app/cli_app_commands.sh +++ b/scripts/cli/commands/app/cli_app_commands.sh @@ -21,6 +21,16 @@ cliHandleAppCommands() elif [[ "$initial_command5" == "--reset-network" ]]; then reset_network="true" fi + + # --storage=: which drive this app is installed onto. Scanned + # across the slots rather than fixed to one, because the config argument + # before it is optional and callers write both orders. + local app_storage="" + local _s + for _s in "$config" "$initial_command5" "$initial_command6" "$initial_command7"; do + [[ "$_s" == --storage=* ]] && app_storage="${_s#--storage=}" + done + [[ "$config" == --storage=* ]] && config="" case "$action" in "list") @@ -38,6 +48,12 @@ cliHandleAppCommands() "install") if [[ "$LIBREPORTAL_TASK_EXEC" == "1" ]]; then + # Read by storagePlaceAppPre, deep inside installApp. Passed as + # an environment variable rather than another positional because + # the install driver is reached through several call sites and + # threading an argument through all of them to be ignored by + # most is worse than one clearly-named variable. + [[ -n "$app_storage" ]] && export LP_INSTALL_STORAGE="$app_storage" dockerInstallApp "$app_name" "$config" "$reset_network" else local _mode="" @@ -48,9 +64,12 @@ cliHandleAppCommands() # flags, keep what dockerInstallApp expects. local _passthrough_config="$config" [[ "$_passthrough_config" == "--detach" || "$_passthrough_config" == "--reset-network" ]] && _passthrough_config="" + [[ "$_passthrough_config" == --storage=* ]] && _passthrough_config="" local _cmd="libreportal app install $app_name" [[ -n "$_passthrough_config" ]] && _cmd+=" '$_passthrough_config'" [[ "$reset_network" == "true" ]] && _cmd+=" --reset-network" + # Re-emitted for the task run, which is a separate process. + [[ -n "$app_storage" ]] && _cmd+=" --storage='$app_storage'" cliTaskRun "$_cmd" "install" "$app_name" "$_mode" fi ;; diff --git a/scripts/cli/commands/app/cli_app_header.sh b/scripts/cli/commands/app/cli_app_header.sh index eca7af5..bd7d642 100755 --- a/scripts/cli/commands/app/cli_app_header.sh +++ b/scripts/cli/commands/app/cli_app_header.sh @@ -12,7 +12,8 @@ cliShowAppHelp() echo " available - Show available apps to install" echo " installed - Show installed apps" echo "" - echo " libreportal app install [name*] [config] [--reset-network]" + echo " libreportal app install [name*] [config] [--reset-network] [--storage=]" + echo " --storage picks which drive the app is installed onto (see: storage list)" echo " - Install / reinstall the specified app." echo " On reinstall, IPs and ports are preserved by default." echo " Pass --reset-network to re-randomize them." diff --git a/scripts/config/core/config_update_option.sh b/scripts/config/core/config_update_option.sh index 77876f8..284ae21 100755 --- a/scripts/config/core/config_update_option.sh +++ b/scripts/config/core/config_update_option.sh @@ -42,8 +42,15 @@ updateConfigOption() # (sed -i writes its temp next to the target, so it inherits the dir's # write perms — and the manager can't write inside dockerinstall dirs). # runFileOp routes the write through the right user. + # + # Asked of the path resolver rather than compared against $containers_dir: + # that is only the PRIMARY root, and app data can live on any registered + # storage location. An app installed on a second disk therefore took the + # manager branch and hit the exact sed failure described above — silently + # for callers that tolerate a failed write, which is how `app move` could + # report success while leaving the config pointing at the old location. local _write_op="runInstallOp" - if [[ -n "${containers_dir:-}" && "$config_file" == "${containers_dir%/}/"* ]]; then + if _runCfgIsContainerPath "$config_file"; then _write_op="runFileOp" fi diff --git a/scripts/dev/lp-config-write-test b/scripts/dev/lp-config-write-test new file mode 100755 index 0000000..81f4e69 --- /dev/null +++ b/scripts/dev/lp-config-write-test @@ -0,0 +1,66 @@ +#!/bin/bash +# Which user does a config write drop to, for an app on a second disk? +# +# scripts/dev/lp-config-write-test +# +# `sed -i` writes its temporary file next to the target, so it needs write +# permission on the DIRECTORY. App directories are owned by the container user, +# so the manager cannot write in them and the write has to drop privilege. +# +# updateConfigOption chose that by comparing the path against $containers_dir — +# the PRIMARY root only. An app installed on any other registered storage +# location therefore took the manager branch and failed with +# +# sed: couldn't open temporary file /mnt/disk2/apps//sedXXXXXX: Permission denied +# +# which is the exact failure the comment above that code describes. Callers that +# tolerate a failed write hid it: `app move` persists the new location with +# `updateConfigOption ... || true`, so it reported a successful move while +# leaving the config naming the old disk. + +REPO="$(cd "$(dirname "$0")/../.." && pwd)" +BASE="$(mktemp -d "${TMPDIR:-/tmp}/lp-cfgwrite-test-XXXXXX")" +trap 'rm -rf "$BASE"' EXIT + +fail=0 +chk(){ if [[ "$2" == "$3" ]]; then echo " ok $1"; else echo " FAIL $1: got '$2' want '$3'"; fail=1; fi; } + +containers_dir="$BASE/primary/" +mkdir -p "$BASE/primary/appA" "$BASE/disk2/appB" "$BASE/sys/configs" +for f in "$BASE/primary/appA/appA.config" "$BASE/disk2/appB/appB.config" "$BASE/sys/configs/general"; do + mkdir -p "$(dirname "$f")"; printf 'CFG_X=old # a comment\n' > "$f" +done + +# The resolver knows every registered root; the primary-only comparison did not. +pathIsContainerData(){ + [[ "$1" == "$BASE/primary/"* || "$1" == "$BASE/disk2/"* ]] +} +source "$REPO/scripts/docker/command/run_privileged.sh" 2>/dev/null || true + +# Record which helper the write is routed through instead of actually escalating. +CHOSE="" +runFileOp(){ CHOSE="runFileOp"; shift 0; "$@"; } +runInstallOp(){ CHOSE="runInstallOp"; shift 0; "$@"; } +runAsManager(){ "$@"; } +isError(){ :; }; isSuccessful(){ :; }; isNotice(){ :; }; isQuestion(){ :; } +checkSuccess(){ :; } + +source "$REPO/scripts/config/core/config_update_option.sh" + +echo "--- an app on the PRIMARY root ---" +CHOSE=""; updateConfigOption CFG_X newA "$BASE/primary/appA/appA.config" >/dev/null 2>&1 +chk "drops to the container user" "$CHOSE" "runFileOp" +chk "and the value landed" "$(grep -c 'newA' "$BASE/primary/appA/appA.config")" "1" + +echo "--- an app on a SECOND registered location (the case that failed) ---" +CHOSE=""; updateConfigOption CFG_X newB "$BASE/disk2/appB/appB.config" >/dev/null 2>&1 +chk "drops to the container user" "$CHOSE" "runFileOp" +chk "and the value landed" "$(grep -c 'newB' "$BASE/disk2/appB/appB.config")" "1" + +echo "--- a manager-owned system config stays with the manager ---" +CHOSE=""; updateConfigOption CFG_X newS "$BASE/sys/configs/general" >/dev/null 2>&1 +chk "stays with the manager" "$CHOSE" "runInstallOp" + +echo "" +if (( fail )); then echo "FAILED"; exit 1; fi +echo "All config-write routing checks passed." diff --git a/scripts/dev/lp-storage-place-test b/scripts/dev/lp-storage-place-test new file mode 100755 index 0000000..8e00c7a --- /dev/null +++ b/scripts/dev/lp-storage-place-test @@ -0,0 +1,101 @@ +#!/bin/bash +# Per-app install location: does the choice arrive, validate, and stick? +# +# scripts/dev/lp-storage-place-test +# +# Two routes carry the same choice — `app install --storage=` and the +# storage dropdown in the App Center's install form, which travels inside +# config_variables — so the first thing to pin is that they resolve to one +# answer, with the explicit flag winning. +# +# The rest is ordering. installApp copies the app template, sources it, and +# later applies the form overrides. The template ships CFG__STORAGE=default, +# so a choice that is not folded into config_variables gets written back to +# "default" by that last pass — leaving an app whose directory is on one disk +# and whose config claims another. That resolves correctly only for as long as +# nothing sources the config, which is why it is easy to miss. + +REPO="$(cd "$(dirname "$0")/../.." && pwd)" +BASE="$(mktemp -d "${TMPDIR:-/tmp}/lp-place-test-XXXXXX")" +trap 'rm -rf "$BASE"' EXIT + +fail=0 +chk(){ if [[ "$2" == "$3" ]]; then echo " ok $1"; else echo " FAIL $1: got '$2' want '$3'"; fail=1; fi; } + +mkdir -p "$BASE/primary" "$BASE/disk1" +isNotice(){ :; }; isError(){ LAST_ERR="$*"; }; isSuccessful(){ :; } +primaryRoot(){ printf '%s' "$BASE/primary"; } +storageLocationPath(){ [[ "$1" == "disk1" ]] && { printf '%s' "$BASE/disk1"; return 0; }; return 1; } +storageLocationName(){ basename "$1"; } +storageRoots(){ printf '%s\n%s\n' "$BASE/primary" "$BASE/disk1"; } +storageRootAvailable(){ [[ "$1" == "$BASE/unmounted" ]] && return 1; return 0; } +storageIndexSet(){ INDEXED="$1 -> $2"; } +storageCacheReset(){ :; } +updateConfigOption(){ CFG_WRITTEN="$1=$2 -> $3"; } + +source "$REPO/scripts/storage/storage_place.sh" + +echo "--- the two routes resolve to one answer ---" +chk "flag only" "$(LP_INSTALL_STORAGE=disk1 storageChoiceFor myapp '')" "disk1" +chk "form only" "$(storageChoiceFor myapp 'CFG_MYAPP_STORAGE=disk1|CFG_MYAPP_PORT=80')" "disk1" +chk "flag wins" "$(LP_INSTALL_STORAGE=disk1 storageChoiceFor myapp 'CFG_MYAPP_STORAGE=primary')" "disk1" +chk "neither" "$(storageChoiceFor myapp 'CFG_MYAPP_PORT=80')" "" + +echo "--- the choice is folded into config_variables ---" +# Without this the later override pass writes the template's "default" back. +out=$(storageChoiceMerge myapp disk1 'CFG_MYAPP_PORT=80|CFG_MYAPP_STORAGE=default') +chk "storage set" "$(tr '|' '\n' <<< "$out" | grep -c '^CFG_MYAPP_STORAGE=disk1$')" "1" +chk "no stale value" "$(tr '|' '\n' <<< "$out" | grep -c '^CFG_MYAPP_STORAGE=default$')" "0" +chk "others kept" "$(tr '|' '\n' <<< "$out" | grep -c '^CFG_MYAPP_PORT=80$')" "1" +chk "from empty" "$(storageChoiceMerge myapp disk1 '')" "CFG_MYAPP_STORAGE=disk1" + +echo "--- placement validates before anything is created ---" +LAST_ERR=""; INDEXED="" +storagePlaceAppPre myapp nosuch && { echo " FAIL unknown location accepted"; fail=1; } || echo " ok unknown location refused" +chk "and says so" "${LAST_ERR:0:26}" "No such storage location: " +chk "nothing indexed" "$INDEXED" "" + +storageRootAvailable(){ return 1; } +LAST_ERR="" +storagePlaceAppPre myapp disk1 && { echo " FAIL unmounted location accepted"; fail=1; } || echo " ok unmounted location refused" +storageRootAvailable(){ return 0; } + +mkdir -p "$BASE/disk1/taken" +LAST_ERR="" +storagePlaceAppPre taken disk1 && { echo " FAIL installed over an existing directory"; fail=1; } || echo " ok refuses to install over an existing directory" + +echo "--- a good placement records both answers ---" +INDEXED="" +storagePlaceAppPre myapp disk1 || { echo " FAIL valid placement refused"; fail=1; } +chk "index" "$INDEXED" "myapp -> $BASE/disk1" +chk "exported" "$CFG_MYAPP_STORAGE" "disk1" + +echo "--- an app pinned to the primary root cannot be placed elsewhere ---" +# Some apps are referenced by other apps at a literal path, so their template +# marks the field **READONLY**. storageMoveApp refuses to move those; installing +# one onto another disk is the same violation from the other end. +install_containers_dir="$BASE/templates/" +mkdir -p "$BASE/templates/pinned" +printf 'CFG_PINNED_STORAGE=default # Storage Location - Fixed **READONLY**\n' \ + > "$BASE/templates/pinned/pinned.config" +LAST_ERR=""; INDEXED="" +storagePlaceAppPre pinned disk1 && { echo " FAIL a pinned app was placed elsewhere"; fail=1; } \ + || echo " ok refused" +chk "nothing indexed" "$INDEXED" "" +mkdir -p "$BASE/templates/free" +printf 'CFG_FREE_STORAGE=default # Storage Location - Which disk [default:Primary]\n' \ + > "$BASE/templates/free/free.config" +INDEXED="" +storagePlaceAppPre free disk1 || { echo " FAIL an unpinned app was refused"; fail=1; } +chk "unpinned still placed" "$INDEXED" "free -> $BASE/disk1" + +echo "--- \"default\" is not a placement ---" +# Templates ship it; treating it as a choice would pin every app to the primary +# root and break "follow CFG_STORAGE_DEFAULT". +INDEXED="" +storagePlaceAppPre other default || { echo " FAIL default should be a no-op"; fail=1; } +chk "nothing indexed" "$INDEXED" "" + +echo "" +if (( fail )); then echo "FAILED"; exit 1; fi +echo "All install-placement checks passed." diff --git a/scripts/source/files/arrays/function_manifest.sh b/scripts/source/files/arrays/function_manifest.sh index 1198868..8010f70 100644 --- a/scripts/source/files/arrays/function_manifest.sh +++ b/scripts/source/files/arrays/function_manifest.sh @@ -1031,6 +1031,8 @@ declare -gA LP_FN_MAP=( [_storageCheckRemovable]="storage/storage_checks.sh" [_storageCheckSharedWithBackup]="storage/storage_checks.sh" [_storageCheckSpace]="storage/storage_checks.sh" + [storageChoiceFor]="storage/storage_place.sh" + [storageChoiceMerge]="storage/storage_place.sh" [storageDisks]="storage/storage_disks.sh" [storageDisksData]="storage/storage_disks.sh" [_storageEmit]="storage/storage_checks.sh" @@ -1039,9 +1041,12 @@ declare -gA LP_FN_MAP=( [storageList]="storage/storage_locations.sh" [storageLocationConfig]="storage/storage_locations.sh" [storageLocationDir]="storage/storage_locations.sh" + [storageLocationNames]="storage/storage_place.sh" [storageLocationsDir]="storage/storage_locations.sh" [storageMoveApp]="storage/storage_move.sh" [_storageOptionList]="storage/storage_app_config.sh" + [storagePlaceAppPost]="storage/storage_place.sh" + [storagePlaceAppPre]="storage/storage_place.sh" [_storageProbeDir]="storage/storage_checks.sh" [_storageRefreshWebui]="storage/storage_locations.sh" [storageRemove]="storage/storage_locations.sh" @@ -1051,7 +1056,9 @@ declare -gA LP_FN_MAP=( [_storageSkipTarget]="storage/storage_scan.sh" [storageSnapshotSourcePath]="storage/storage_restore_path.sh" [storageSyncAllAppComments]="storage/storage_app_config.sh" + [storageSyncAllTemplateComments]="storage/storage_app_config.sh" [storageSyncAppComment]="storage/storage_app_config.sh" + [storageSyncTemplateComment]="storage/storage_app_config.sh" [_storageWriteLocationConfig]="storage/storage_locations.sh" [switchMigrateBackupApps]="docker/type_switcher/swap_docker_type.sh" [switchMigrateRestoreApps]="docker/type_switcher/swap_docker_type.sh" @@ -2270,6 +2277,8 @@ declare -gA LP_FN_ROOT=( [_storageCheckRemovable]="scripts" [_storageCheckSharedWithBackup]="scripts" [_storageCheckSpace]="scripts" + [storageChoiceFor]="scripts" + [storageChoiceMerge]="scripts" [storageDisks]="scripts" [storageDisksData]="scripts" [_storageEmit]="scripts" @@ -2278,9 +2287,12 @@ declare -gA LP_FN_ROOT=( [storageList]="scripts" [storageLocationConfig]="scripts" [storageLocationDir]="scripts" + [storageLocationNames]="scripts" [storageLocationsDir]="scripts" [storageMoveApp]="scripts" [_storageOptionList]="scripts" + [storagePlaceAppPost]="scripts" + [storagePlaceAppPre]="scripts" [_storageProbeDir]="scripts" [_storageRefreshWebui]="scripts" [storageRemove]="scripts" @@ -2290,7 +2302,9 @@ declare -gA LP_FN_ROOT=( [_storageSkipTarget]="scripts" [storageSnapshotSourcePath]="scripts" [storageSyncAllAppComments]="scripts" + [storageSyncAllTemplateComments]="scripts" [storageSyncAppComment]="scripts" + [storageSyncTemplateComment]="scripts" [_storageWriteLocationConfig]="scripts" [switchMigrateBackupApps]="scripts" [switchMigrateRestoreApps]="scripts" @@ -3547,6 +3561,8 @@ _storageCheckPersistence() { unset -f _storageCheckPersistence; __lpAutoload "${ _storageCheckRemovable() { unset -f _storageCheckRemovable; __lpAutoload "${install_scripts_dir}storage/storage_checks.sh"; _storageCheckRemovable "$@"; } _storageCheckSharedWithBackup() { unset -f _storageCheckSharedWithBackup; __lpAutoload "${install_scripts_dir}storage/storage_checks.sh"; _storageCheckSharedWithBackup "$@"; } _storageCheckSpace() { unset -f _storageCheckSpace; __lpAutoload "${install_scripts_dir}storage/storage_checks.sh"; _storageCheckSpace "$@"; } +storageChoiceFor() { unset -f storageChoiceFor; __lpAutoload "${install_scripts_dir}storage/storage_place.sh"; storageChoiceFor "$@"; } +storageChoiceMerge() { unset -f storageChoiceMerge; __lpAutoload "${install_scripts_dir}storage/storage_place.sh"; storageChoiceMerge "$@"; } storageDisks() { unset -f storageDisks; __lpAutoload "${install_scripts_dir}storage/storage_disks.sh"; storageDisks "$@"; } storageDisksData() { unset -f storageDisksData; __lpAutoload "${install_scripts_dir}storage/storage_disks.sh"; storageDisksData "$@"; } _storageEmit() { unset -f _storageEmit; __lpAutoload "${install_scripts_dir}storage/storage_checks.sh"; _storageEmit "$@"; } @@ -3555,9 +3571,12 @@ storageFstabLine() { unset -f storageFstabLine; __lpAutoload "${install_scripts_ storageList() { unset -f storageList; __lpAutoload "${install_scripts_dir}storage/storage_locations.sh"; storageList "$@"; } storageLocationConfig() { unset -f storageLocationConfig; __lpAutoload "${install_scripts_dir}storage/storage_locations.sh"; storageLocationConfig "$@"; } storageLocationDir() { unset -f storageLocationDir; __lpAutoload "${install_scripts_dir}storage/storage_locations.sh"; storageLocationDir "$@"; } +storageLocationNames() { unset -f storageLocationNames; __lpAutoload "${install_scripts_dir}storage/storage_place.sh"; storageLocationNames "$@"; } storageLocationsDir() { unset -f storageLocationsDir; __lpAutoload "${install_scripts_dir}storage/storage_locations.sh"; storageLocationsDir "$@"; } storageMoveApp() { unset -f storageMoveApp; __lpAutoload "${install_scripts_dir}storage/storage_move.sh"; storageMoveApp "$@"; } _storageOptionList() { unset -f _storageOptionList; __lpAutoload "${install_scripts_dir}storage/storage_app_config.sh"; _storageOptionList "$@"; } +storagePlaceAppPost() { unset -f storagePlaceAppPost; __lpAutoload "${install_scripts_dir}storage/storage_place.sh"; storagePlaceAppPost "$@"; } +storagePlaceAppPre() { unset -f storagePlaceAppPre; __lpAutoload "${install_scripts_dir}storage/storage_place.sh"; storagePlaceAppPre "$@"; } _storageProbeDir() { unset -f _storageProbeDir; __lpAutoload "${install_scripts_dir}storage/storage_checks.sh"; _storageProbeDir "$@"; } _storageRefreshWebui() { unset -f _storageRefreshWebui; __lpAutoload "${install_scripts_dir}storage/storage_locations.sh"; _storageRefreshWebui "$@"; } storageRemove() { unset -f storageRemove; __lpAutoload "${install_scripts_dir}storage/storage_locations.sh"; storageRemove "$@"; } @@ -3567,7 +3586,9 @@ storageScanCandidates() { unset -f storageScanCandidates; __lpAutoload "${instal _storageSkipTarget() { unset -f _storageSkipTarget; __lpAutoload "${install_scripts_dir}storage/storage_scan.sh"; _storageSkipTarget "$@"; } storageSnapshotSourcePath() { unset -f storageSnapshotSourcePath; __lpAutoload "${install_scripts_dir}storage/storage_restore_path.sh"; storageSnapshotSourcePath "$@"; } storageSyncAllAppComments() { unset -f storageSyncAllAppComments; __lpAutoload "${install_scripts_dir}storage/storage_app_config.sh"; storageSyncAllAppComments "$@"; } +storageSyncAllTemplateComments() { unset -f storageSyncAllTemplateComments; __lpAutoload "${install_scripts_dir}storage/storage_app_config.sh"; storageSyncAllTemplateComments "$@"; } storageSyncAppComment() { unset -f storageSyncAppComment; __lpAutoload "${install_scripts_dir}storage/storage_app_config.sh"; storageSyncAppComment "$@"; } +storageSyncTemplateComment() { unset -f storageSyncTemplateComment; __lpAutoload "${install_scripts_dir}storage/storage_app_config.sh"; storageSyncTemplateComment "$@"; } _storageWriteLocationConfig() { unset -f _storageWriteLocationConfig; __lpAutoload "${install_scripts_dir}storage/storage_locations.sh"; _storageWriteLocationConfig "$@"; } switchMigrateBackupApps() { unset -f switchMigrateBackupApps; __lpAutoload "${install_scripts_dir}docker/type_switcher/swap_docker_type.sh"; switchMigrateBackupApps "$@"; } switchMigrateRestoreApps() { unset -f switchMigrateRestoreApps; __lpAutoload "${install_scripts_dir}docker/type_switcher/swap_docker_type.sh"; switchMigrateRestoreApps "$@"; } diff --git a/scripts/source/paths.sh b/scripts/source/paths.sh index 635f437..cf8b606 100644 --- a/scripts/source/paths.sh +++ b/scripts/source/paths.sh @@ -264,7 +264,20 @@ storageLocationName() [[ -z "$_path" || "$_id" == \#* ]] && continue if [[ "${_path%/}" == "$want" ]]; then name_var="CFG_STORAGE_LOC_${_id}_NAME" - printf '%s' "${!name_var:-$_id}" + if [[ -n "${!name_var:-}" ]]; then + printf '%s' "${!name_var}" + return 0 + fi + # Not in scope. The name is the value CFG__STORAGE is set + # to, so falling back to the bare id would hand callers a label + # that does not resolve — the config-comment dropdown offered + # "location-1" as both the shown text and the value it writes. + # Read it from the location's own config instead. + local _f="${LP_SYSTEM_DIR%/}/configs/storage/locations/${_id}/location.config" + local _n="" + [[ -r "$_f" ]] && _n=$(sed -n "s/^CFG_STORAGE_LOC_${_id}_NAME=\"\{0,1\}\([^\"#]*\).*/\1/p" "$_f" 2>/dev/null | head -1) + _n="${_n%"${_n##*[![:space:]]}"}" + printf '%s' "${_n:-$_id}" return 0 fi done < "$lp_storage_registry" diff --git a/scripts/storage/storage_app_config.sh b/scripts/storage/storage_app_config.sh index 9506383..a0632d2 100644 --- a/scripts/storage/storage_app_config.sh +++ b/scripts/storage/storage_app_config.sh @@ -25,8 +25,9 @@ _storageOptionList() local id state path name_var name while IFS=$'\t' read -r id state path; do [[ -z "$id" ]] && continue - name_var="CFG_STORAGE_LOC_${id}_NAME" - name="${!name_var:-location-$id}" + # One resolver, so the label and the value it writes cannot disagree. + name=$(storageLocationName "$path" 2>/dev/null) + [[ -n "$name" ]] || name="$id" # An unavailable location stays selectable and says so, rather than # vanishing from the list and looking like it was deleted. if [[ "$state" == "ok" ]]; then @@ -87,6 +88,61 @@ storageSyncAppComment() return 0 } +# The same option list, but written into an app TEMPLATE. +# +# The App Center's install form reads an uninstalled app's fields from its +# template under install_containers_dir, and templates ship +# "[default:Primary]" — so the storage dropdown offered exactly one choice and +# the drives you could actually install onto were invisible until after the app +# was installed, which is the wrong way round. The value is preserved; only the +# option list is regenerated. +storageSyncTemplateComment() +{ + local app="$1" + [[ -n "$app" ]] || return 0 + + local cfg="${install_containers_dir%/}/$app/$app.config" + [[ -f "$cfg" ]] || return 0 + + local key="CFG_${app^^}_STORAGE" line + line=$(runInstallOp grep -m1 -E "^${key}=" "$cfg" 2>/dev/null) || return 0 + [[ -n "$line" ]] || return 0 + # Pinned to the primary root on purpose — nothing to offer. + [[ "$line" == *'**READONLY**'* ]] && return 0 + + local value="${line#*=}" + value="${value%%#*}" + value="${value//$'\r'/}" + value="${value#"${value%%[![:space:]]*}"}" + value="${value%"${value##*[![:space:]]}"}" + value="${value%\"}"; value="${value#\"}" + + local want="${key}=${value} # Storage Location - Which disk holds this app's data [$(_storageOptionList)]" + [[ "$line" == "$want" ]] && return 0 + + local tmp; tmp=$(mktemp) || return 1 + runInstallOp cat "$cfg" 2>/dev/null > "$tmp" || { rm -f "$tmp"; return 1; } + awk -v k="^${key}=" -v repl="$want" ' + $0 ~ k && !done { print repl; done=1; next } + { print } + ' "$tmp" | runInstallWrite "$cfg" >/dev/null + rm -f "$tmp" + return 0 +} + +# Every app template, so the install form offers the real drives. +storageSyncAllTemplateComments() +{ + local d app + [[ -d "${install_containers_dir%/}" ]] || return 0 + for d in "${install_containers_dir%/}"/*/; do + app=$(basename "$d") + [[ "$app" == "libreportal" || "$app" == "libreportal_catalog" ]] && continue + storageSyncTemplateComment "$app" + done + return 0 +} + # Sync every installed app. Called from the regen path; cheap because each app # short-circuits unless its resolved path or the option list actually changed. storageSyncAllAppComments() @@ -96,5 +152,8 @@ storageSyncAllAppComments() [[ -z "$app" || "$app" == "libreportal" ]] && continue storageSyncAppComment "$app" done < <(storageApps) + # Templates too: an app not installed yet is exactly the one whose install + # form needs to show which drives exist. + storageSyncAllTemplateComments return 0 } diff --git a/scripts/storage/storage_place.sh b/scripts/storage/storage_place.sh new file mode 100644 index 0000000..b91fad5 --- /dev/null +++ b/scripts/storage/storage_place.sh @@ -0,0 +1,166 @@ +#!/bin/bash + +# Choosing which drive an app is installed onto. +# +# The resolver already supported this — CFG__STORAGE names a location and +# appDir sends data, compose and config there — and every app template ships the +# field with a generated dropdown of the registered locations. What was missing +# was making the choice effective AT INSTALL TIME. Until now the only routes +# were editing the config by hand before installing, or installing onto the +# default disk and then `app move`ing it, which copies the data twice. +# +# The choice arrives two ways, and they are the same choice: +# +# libreportal app install --storage= -> LP_INSTALL_STORAGE +# the storage dropdown in the App Center install form -> config_variables +# +# Both are resolved to one answer here so there is a single code path. +# +# ORDERING is the whole difficulty. installApp does: +# +# 1. dockerConfigSetupToContainer silent creates the app dir at +# appDir() and copies the +# template config into it +# 2. initializeAppVariables sources that config +# ... +# 3. dockerConfigSetupToContainer loud install +# applies the form overrides +# +# The choice has to be live before (1) or the directory is created on the wrong +# disk; written into the config between (1) and (2) or sourcing the fresh +# template resets it to "default" and every later appDir in the same process +# returns the primary root; and present in config_variables for (3) or that +# pass writes "default" back over it. Miss any one and you get an app whose +# directory and config disagree — which resolves correctly only for as long as +# nothing sources the config. + +# The requested location, or empty for "no opinion" (the normal path). +# The CLI flag wins over the form field when both are present. +storageChoiceFor() +{ + local app="$1" config_vars="${2:-}" + [[ -n "$app" ]] || return 0 + + if [[ -n "${LP_INSTALL_STORAGE:-}" ]]; then + printf '%s' "$LP_INSTALL_STORAGE" + return 0 + fi + + local key="CFG_${app^^}_STORAGE" pair + local -a pairs=() + IFS='|' read -ra pairs <<< "$config_vars" + for pair in "${pairs[@]}"; do + if [[ "$pair" == "$key="* ]]; then + printf '%s' "${pair#$key=}" + return 0 + fi + done + return 0 +} + +# Make appDir resolve to the chosen location, before anything is created. +storagePlaceAppPre() +{ + local app="$1" want="$2" + [[ -n "$app" && -n "$want" ]] || return 0 + # "default" is the templates' own value and means "follow CFG_STORAGE_DEFAULT", + # so it is not a placement and must not be treated as one. + [[ "$want" == "default" ]] && return 0 + + # Some apps are deliberately fixed to the primary root because other apps + # reach them by literal path; their template marks the field **READONLY**. + # storageMoveApp already refuses to move those, and installing one somewhere + # else is the same violation arrived at from the other end — it just breaks + # the referring app immediately instead of later. + local tmpl="${install_containers_dir%/}/$app/$app.config" + if [[ -f "$tmpl" ]] && grep -qE "^CFG_${app^^}_STORAGE=.*\*\*READONLY\*\*" "$tmpl" 2>/dev/null; then + isError "$app is fixed to the default location — other apps reference it by path." + isNotice "Install it without a location, or move the apps that depend on it instead." + return 1 + fi + + local root + if [[ "$want" == "primary" ]]; then + root=$(primaryRoot) + elif ! root=$(storageLocationPath "$want"); then + isError "No such storage location: $want" + isNotice "Known locations: $(storageLocationNames 2>/dev/null | paste -sd, -)" + return 1 + fi + + if ! storageRootAvailable "$root"; then + isError "Storage location '$want' ($root) is not mounted — refusing to install onto it." + isNotice "Mount it first, or leave the location unset to use the default." + return 1 + fi + + if [[ -e "${root%/}/$app" ]]; then + isError "'${root%/}/$app' already exists — refusing to install over it." + return 1 + fi + + # Two records answering at different times: the exported variable is what + # _appDirIntended reads in THIS process before any config exists for the + # app; the index is what answers for a later process whose config lives on + # a drive that is not mounted. + export "CFG_${app^^}_STORAGE=$want" + storageIndexSet "$app" "$root" + storageCacheReset + + isNotice "Installing $app onto '$want' ($root)." + return 0 +} + +# Keep the choice, now that there is a deployed config to keep it in. +storagePlaceAppPost() +{ + local app="$1" want="$2" + [[ -n "$app" && -n "$want" && "$want" != "default" ]] || return 0 + + local dir cfg + dir=$(appDir "$app") || return 0 + cfg="$dir/$app.config" + [[ -f "$cfg" ]] || return 0 + + updateConfigOption "CFG_${app^^}_STORAGE" "$want" "$cfg" >/dev/null 2>&1 || true + export "CFG_${app^^}_STORAGE=$want" + declare -f storageSyncAppComment >/dev/null 2>&1 && storageSyncAppComment "$app" + return 0 +} + +# config_variables with the choice set, so the later override pass writes it +# rather than resetting it to the template's "default". +storageChoiceMerge() +{ + local app="$1" want="$2" config_vars="${3:-}" + local key="CFG_${app^^}_STORAGE" + [[ -n "$want" ]] || { printf '%s' "$config_vars"; return 0; } + + local -a keep=() pairs=() + local pair + IFS='|' read -ra pairs <<< "$config_vars" + for pair in "${pairs[@]}"; do + [[ -z "$pair" || "$pair" == "$key="* ]] && continue + keep+=("$pair") + done + keep+=("$key=$want") + + local out="" p + for p in "${keep[@]}"; do + [[ -n "$out" ]] && out+="|" + out+="$p" + done + printf '%s' "$out" +} + +# Every location an app can be installed onto, one per line. +storageLocationNames() +{ + printf '%s\n' primary + local root + while IFS= read -r root; do + [[ -z "$root" ]] && continue + [[ "${root%/}" == "$(primaryRoot)" ]] && continue + storageLocationName "$root" + done < <(storageRoots 2>/dev/null) +} diff --git a/scripts/webui/data/generators/system/webui_storage_candidates.sh b/scripts/webui/data/generators/system/webui_storage_candidates.sh index 65160a1..ec926b6 100644 --- a/scripts/webui/data/generators/system/webui_storage_candidates.sh +++ b/scripts/webui/data/generators/system/webui_storage_candidates.sh @@ -13,6 +13,17 @@ webuiGenerateStorageCandidates() { + # Refresh the per-app storage dropdowns while we are here. + # + # storageSyncAllAppComments was written for "the regen path" and then never + # wired into one, so the option list in every CFG__STORAGE comment was + # whatever it had been when the app was installed. Adding a drive did not + # make it selectable anywhere, which made per-app placement look unfinished + # when only its refresh was missing. This generator already runs whenever + # the WebUI's storage view is rebuilt, which is exactly the moment the lists + # go stale. + declare -f storageSyncAllAppComments >/dev/null 2>&1 && storageSyncAllAppComments + local out_dir="$(webuiDir)/frontend/data/system" local out_file="$out_dir/storage.json" createFolders "quiet" "$sudo_user_name" "$out_dir"