Rename every declared service when cloning an instance, or refuse
The compose rewrite assumed each app had exactly <type>-service and <type>_db. That holds for Bookstack and almost nothing else: cloning Nextcloud left -db, -redis and -web pointing at the ORIGINAL app's containers, and Matrix, Ollama, Mastodon, Owncloud, Gitea, Jitsi, Invidious, Rocketchat and Mattermost all had the same hole. Docker refuses a duplicate container name and two Traefik routers sharing a name fight over the host, so those clones could not have worked. Service identities are now discovered from the compose itself — its SERVICE_TAG_<n> markers plus its container_name values — and each is renamed. Verified across all 38 shipped apps: 15 are fixed, 21 produce byte-identical output to the old rule (Bookstack among them, so the running instances are unaffected), and 2 are refused. Details worth knowing: - Separators compare as equivalent, so the app dir libreportal_catalog matches its libreportal-catalog-* services instead of being wrongly refused. - Tokens are substituted longest-first through placeholders. \b has to end a token because per-port routers are named <service>-<portname> (traefik.http.routers.adguard-service-webui), which also means a short name could otherwise match inside a longer one — ordering is what prevents that. - Commented-out lines are not harvested. Several templates park an optional sidecar behind # (adguard-exporter, pihole-exporter, wireguard-exporter); renaming those also mangled the image name in the same block, leaving a trap for anyone uncommenting it. Commented image: lines are skipped too. - image: lines are genuinely excluded now. The old comment claimed service tokens "never appear in an image path", but libreportal builds a local image named after its own service and the old rule rewrote that reference. An app with a service carrying no <type> prefix (stoat's api/database/minio, prometheus's node-exporter/cadvisor) cannot be made unique mechanically, and rewriting a bare word like minio would corrupt image: minio/minio. Those are refused with an explanation and the partial clone is removed, rather than handed back as an instance that silently fights the base app. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
librelad
parent
74ee73da01
commit
9a7df822dc
@ -17,13 +17,24 @@ findConfigFileForOption()
|
||||
|
||||
# Fall back to per-app configs (CFG_<APP>_*). Without this, tools that
|
||||
# mutate per-app CFGs (password reset, shortcut manager) silently no-op.
|
||||
#
|
||||
# Enumerated via runFileOp, NOT a glob. In a rootless install the containers
|
||||
# tree is owned by dockerinstall and mode 751: the manager that runs the CLI
|
||||
# can TRAVERSE it but not LIST it. A shell glob needs the list, so
|
||||
# "$containers_dir"*/*.config expanded to nothing and every per-app lookup
|
||||
# reported "not found in any config file" — while the very same file was
|
||||
# readable by direct path. That silently no-op'd `libreportal config update`
|
||||
# for every per-app option. runFileOp runs the enumeration as the user that
|
||||
# owns the tree, exactly as the updater's scan already lists app dirs.
|
||||
if [[ -n "$containers_dir" && -d "$containers_dir" ]]; then
|
||||
for config_file in "$containers_dir"*/*.config; do
|
||||
if [ -f "$config_file" ] && grep -q "^$config_option=" "$config_file"; then
|
||||
echo "$config_file"
|
||||
local app_config
|
||||
while IFS= read -r app_config; do
|
||||
[ -n "$app_config" ] || continue
|
||||
if [ -f "$app_config" ] && grep -q "^$config_option=" "$app_config"; then
|
||||
echo "$app_config"
|
||||
return 0
|
||||
fi
|
||||
done
|
||||
done < <(runFileOp find "${containers_dir%/}" -mindepth 2 -maxdepth 2 -type f -name '*.config' 2>/dev/null)
|
||||
fi
|
||||
return 1
|
||||
}
|
||||
|
||||
@ -12,8 +12,14 @@ updateConfigOption()
|
||||
# If no config file provided, auto-discover it
|
||||
if [[ -z "$config_file" ]]; then
|
||||
config_file=$(findConfigFileForOption "$config_option")
|
||||
# Return non-zero. This used to fall through with an empty filename, hit
|
||||
# the `else` below and end — returning 0, so configUpdateBatch counted
|
||||
# the change as applied and printed "Applied 1 config change(s); 0
|
||||
# skipped/failed" having written nothing at all. A setting that reports
|
||||
# success and does not take effect is worse than one that errors.
|
||||
if [[ $? -ne 0 || -z "$config_file" ]]; then
|
||||
isNotice "Config option '$config_option' not found in any config file"
|
||||
return 1
|
||||
fi
|
||||
fi
|
||||
|
||||
@ -42,8 +48,11 @@ updateConfigOption()
|
||||
fi
|
||||
|
||||
# Check if config option exists in the file. grep + read can use the
|
||||
# current user — both dirs are world-readable; only the write needs
|
||||
# escalation.
|
||||
# current user: the FILES are world-readable and the directories are
|
||||
# traversable, so a direct path works. (The containers dir is not world-
|
||||
# LISTABLE — see findConfigFileForOption, which is why discovering the path
|
||||
# needs runFileOp even though reading it afterwards does not.) Only the
|
||||
# write needs escalation.
|
||||
if grep -q "^$config_option=" "$config_file"; then
|
||||
# Extract the comment part first (everything after the first #)
|
||||
local original_line=$(grep "^$config_option=" "$config_file")
|
||||
@ -68,7 +77,9 @@ updateConfigOption()
|
||||
checkSuccess "Updated $config_option to $config_value" ;;
|
||||
esac
|
||||
source "$config_file"
|
||||
return 0
|
||||
else
|
||||
isNotice "Unable to find $config_option with value in $config_file"
|
||||
return 1
|
||||
fi
|
||||
}
|
||||
|
||||
@ -144,9 +144,87 @@ _instanceRewriteCompose() {
|
||||
local type="$1" slug="$2" dir="$3"
|
||||
local f="$dir/docker-compose.yml"
|
||||
[[ -f "$f" ]] || return 0
|
||||
# 1. Traefik router/service names (<type>-service) and the db container/host
|
||||
# (<type>_db) — these tokens are unambiguous, rewrite everywhere.
|
||||
sed -i -E "s/\b${type}-service\b/${slug}-service/g; s/\b${type}_db\b/${slug}_db/g" "$f"
|
||||
|
||||
# 1. Every compound identity the compose declares — its service keys (from the
|
||||
# SERVICE_TAG_<n> markers) and its container_name values. These are the
|
||||
# names Docker and Traefik require to be unique, so a clone that keeps any
|
||||
# of them collides head-on with the base app: docker refuses the duplicate
|
||||
# container, and two routers with one name fight over the same host.
|
||||
#
|
||||
# Discovered rather than hardcoded. This used to assume every app had
|
||||
# exactly <type>-service and <type>_db, which is true of Bookstack and
|
||||
# false of most others — cloning Nextcloud would have left its -db, -redis
|
||||
# and -web services pointing at the ORIGINAL app's containers.
|
||||
#
|
||||
# Commented-out lines are excluded. Several templates ship an optional
|
||||
# sidecar parked behind `#` (adguard-exporter, pihole-exporter, …);
|
||||
# harvesting those renamed a service that isn't running AND mangled the
|
||||
# image name inside the same block, leaving a trap for whoever uncomments
|
||||
# it later. A real service line carries its SERVICE_TAG mid-line, so
|
||||
# dropping lines that *start* with # keeps every live one.
|
||||
local -a tokens=()
|
||||
mapfile -t tokens < <(
|
||||
grep -v '^[[:space:]]*#' "$f" 2>/dev/null | {
|
||||
grep -oE '#LIBREPORTAL\|SERVICE_TAG_[0-9]+\|[^[:space:]]+|container_name:[[:space:]]*[^[:space:]]+' \
|
||||
| sed -E 's/.*SERVICE_TAG_[0-9]+\|//; s/container_name:[[:space:]]*//'
|
||||
} | sed '/^$/d' | sort -u
|
||||
)
|
||||
|
||||
# Split into compound (<type> + separator + suffix) and bare (<type> exactly).
|
||||
# Only compound names are safe to rewrite everywhere: the bare app name also
|
||||
# appears inside image paths and values like MYSQL_USER=<type>, so it stays
|
||||
# confined to the two anchored positions in rules 2/3 below — same split the
|
||||
# original hardcoded rules relied on, now applied to whatever the app declares.
|
||||
#
|
||||
# Longest first: the substitution below lets `-` end a token, so a short
|
||||
# name would otherwise match inside a longer one (`redis` inside
|
||||
# `redis-cache`). Placeholdering the long name first makes that impossible.
|
||||
local -a sorted=()
|
||||
mapfile -t sorted < <(printf '%s\n' "${tokens[@]}" | awk '{print length"\t"$0}' | sort -rn | cut -f2-)
|
||||
|
||||
# Separators are compared as equivalent: an app dir named libreportal_catalog
|
||||
# ships services named libreportal-catalog-*, and treating that as "unprefixed"
|
||||
# would refuse an app that is perfectly instance-safe.
|
||||
local type_norm="${type//-/_}"
|
||||
local -a olds=() news=()
|
||||
local t t_norm
|
||||
for t in "${sorted[@]}"; do
|
||||
t_norm="${t//-/_}"
|
||||
if [[ "$t_norm" == "$type_norm" ]]; then
|
||||
continue # bare name — handled by rules 2/3
|
||||
elif [[ "$t_norm" == "${type_norm}_"* ]]; then
|
||||
# Offset by ${#type}: swapping separators never changes length, so the
|
||||
# suffix (and its original separator char) survives intact.
|
||||
olds+=("$t"); news+=("${slug}${t:${#type}}")
|
||||
else
|
||||
# No <type> prefix at all (stoat's `database`/`minio`/`caddy`,
|
||||
# prometheus's `node-exporter`). There is no mechanical way to make
|
||||
# these unique — and rewriting a bare word like `minio` would also
|
||||
# corrupt `image: minio/minio`. Refuse rather than hand back an
|
||||
# instance that silently fights the base app for container names.
|
||||
isError "Instance create: '$type' declares a service or container named '$t', which carries no '$type' prefix."
|
||||
isNotice "Such names cannot be made unique per instance, so '$type' is not instance-safe. Prefix its services with '$type-' in the compose, or leave CFG_${type^^}_MULTI_INSTANCE unset."
|
||||
return 1
|
||||
fi
|
||||
done
|
||||
|
||||
# Two-phase substitution via a placeholder, so a rewritten name can never be
|
||||
# re-matched by a later pass.
|
||||
#
|
||||
# \b is deliberate: per-port Traefik routers are named <service>-<portname>
|
||||
# (traefik.http.routers.adguard-service-webui), so the service token has to
|
||||
# match when a `-` follows it. That is also what makes the longest-first
|
||||
# ordering above load-bearing rather than cosmetic. image: lines are skipped
|
||||
# outright — an app's name legitimately appears in its own image path.
|
||||
local i ph
|
||||
for i in "${!olds[@]}"; do
|
||||
ph=$'\001'"LPSVC${i}"$'\001'
|
||||
sed -i -E "/^[[:space:]]*#?[[:space:]]*image:/! s/\b${olds[$i]}\b/${ph}/g" "$f"
|
||||
done
|
||||
for i in "${!olds[@]}"; do
|
||||
ph=$'\001'"LPSVC${i}"$'\001'
|
||||
sed -i "s|${ph}|${news[$i]}|g" "$f"
|
||||
done
|
||||
# 2. The standalone app container (container_name: <type>) — anchored so the
|
||||
# image: line ending in <type> is never touched.
|
||||
sed -i -E "s/(container_name:[[:space:]]*)${type}\b/\1${slug}/g" "$f"
|
||||
@ -291,7 +369,14 @@ instanceCreate() {
|
||||
# 4. Make the cloned compose + config ports + tools target the instance's own
|
||||
# identity. The config ports must follow the compose or the WebUI can't
|
||||
# join the instance's ports to its services.
|
||||
_instanceRewriteCompose "$type" "$slug" "$inst_dir"
|
||||
# A refusal here means the app isn't instance-safe. Drop the clone rather
|
||||
# than leave a half-rewritten template on disk that a later scan would pick
|
||||
# up as a real app and try to install.
|
||||
if ! _instanceRewriteCompose "$type" "$slug" "$inst_dir"; then
|
||||
rm -rf "$inst_dir"
|
||||
isError "Instance create aborted; no changes were made."
|
||||
return 1
|
||||
fi
|
||||
_instanceRewriteConfigPorts "$type" "$slug" "$cfg"
|
||||
_instanceRewriteTools "$type" "$slug" "$inst_dir"
|
||||
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user