Skip to content

Fall back to the unversioned banner slug - #48

Merged
MickLesk merged 1 commit into
mainfrom
fix/vm-banner-versioned-app
Sep 19, 2026
Merged

MickLesk merged 1 commit into
mainfrom
fix/vm-banner-versioned-app

Conversation

@MickLesk

@MickLesk MickLesk commented Sep 19, 2026

Copy link
Copy Markdown
Member

✍️ Description

get_header() builds the banner slug from APP at call time. Three VM scripts set APP a second time with the release before drawing, so they ask for a banner that was never generated — the generator keys on the first APP= line:

Script second APP= slug requested upstream
vm/almalinux-vm.sh AlmaLinux ${var_version} VM almalinux10vm 404
vm/debian-vm.sh Debian ${var_version} debian13 404
vm/ubuntu-vm.sh Ubuntu ${var_version} VM ubuntu26.04vm 404

🔗 Related Issue

Fixes community-scripts/ProxmoxVE#17383

✅ Prerequisites (X in brackets)

  • Self-review completed
  • Tested thoroughly – see below
  • No security risks
  • API.txt regenerated_cs_header_slugs is not exported through a loader snapshot; get_header keeps its signature

🧪 Testing

_cs_header_slugs extracted from the file and run against 11 cases — all pass:

"AlmaLinux 10 VM"       -> almalinux10vm almalinux10 almalinux
"Debian 13"             -> debian13 debian
"Ubuntu 26.04 VM"       -> ubuntu26.04vm ubuntu26.04 ubuntu
"Home Assistant OS"     -> homeassistantos
"TrueNAS Scale VM"      -> truenasscalevm truenasscale
"Mikrotik RouterOS CHR" -> mikrotikrouteroschr
"Debian v13"            -> debianv13 debian
"Foo LXC"               -> foolxc foo
""                      -> (nothing)

get_header end to end against local fixtures:

"Debian 13"         -> DEBIAN-BANNER  rc=0   (fell back to debian)
"Alpine"            -> ALPINE-BANNER  rc=0   (direct hit)
"Home Assistant OS" -> <nothing>      rc=1   (no fallback, as intended)
cache written under the resolved slug: headers/vm/debian
second call serves from cache with the fixture removed: DEBIAN-BANNER rc=0

Not run on a Proxmox host. bash -n passes.

🛠️ Type of Change (X in brackets)

  • 🐞 Bug fix

get_header keys on APP, but almalinux-vm, debian-vm and ubuntu-vm set
APP again with the release before drawing, so they ask for
almalinux10vm, debian13 and ubuntu26.04vm. None of those was ever
generated -- the generator keys on the first APP= line -- so the request
404s and no banner is drawn.

Only a trailing version or VM/LXC is dropped and retried, so the walk
stops at the first word that is neither: "Home Assistant OS" still
resolves to homeassistantos alone and cannot land on a homeassistant
banner. The cache is keyed on the slug that resolved, not the one asked
for.

Fixes community-scripts/ProxmoxVE#17383
@github-actions

Copy link
Copy Markdown
Contributor

Try this branch

The engine and the scripts resolve independently, so a production script can
be run against the engine from this PR by setting one variable:

COMMUNITY_SCRIPTS_CORE_URL=https://raw.githubusercontent.com/community-scripts/core/fix/vm-banner-versioned-app \
bash -c "$(curl -fsSL https://raw.githubusercontent.com/community-scripts/ProxmoxVED/main/ct/debian.sh)"

Swap ct/debian.sh for whatever exercises the change.

This PR touches pve/, so it needs a Proxmox VE host.

Run a script from a fork as well
curl -fsSL https://raw.githubusercontent.com/community-scripts/core/fix/vm-banner-versioned-app/tools/run.sh |
  bash -s -- https://raw.githubusercontent.com/YOU/ProxmoxVED/your-branch ct/debian.sh \
             https://raw.githubusercontent.com/community-scripts/core/fix/vm-banner-versioned-app

Note that run.sh is reached through a pipe, so the script it starts inherits
an exhausted stdin. Whiptail is fine — it opens /dev/tty — but a plain read
would see EOF. The single-variable form above does not have that problem.

Useful flags while testing

dev_mode=net logs every engine fetch with status and duration, which is the
quickest way to confirm the branch is really being used. dev_mode=keep stops a
failed build from deleting the container along with the evidence.

@MickLesk
MickLesk merged commit e33d261 into main Sep 19, 2026
4 checks passed
MickLesk added a commit that referenced this pull request Sep 19, 2026
header_info handed get_header "${APP,,}" with spaces turned into
hyphens. That was harmless while get_header ignored its argument, but
#48 made the argument live, so the lookup started asking for
home-assistant-os instead of homeassistantos: ten VM scripts with a
multi-word APP lost the banner they had, and the three with a versioned
APP never got theirs, because a hyphenated string is one word and the
new fallback had nothing to drop.

Pass the name through unchanged.
@github-actions
github-actions Bot deleted the fix/vm-banner-versioned-app branch September 20, 2026 07:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

VM banners missing after version added to APP in almalinux, ubuntu, debian

1 participant