Fetch container image information from branch-metadata file#16840
Fetch container image information from branch-metadata file#16840richard-cox merged 4 commits intorancher:masterfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR migrates E2E/local Docker image and UI index selection from hardcoded values (and an auto-update workflow) to a single source of truth in branches-metadata.json, enabling branch-aware configuration and support for alternate registries.
Changes:
- Removed the release-branch workflow/script that auto-updated hardcoded E2E version values.
- Added a
get-branch-metadata.shhelper and updated scripts to derive Rancher image + UI index URLs frombranches-metadata.json. - Switched
docker:local:startfrom an inlinedocker runto a script wrapper.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| scripts/update-e2e-rancher-version.sh | Removed obsolete hardcoded-updater script. |
| .github/workflows/update-e2e-rancher-version.yml | Removed obsolete workflow that opened PRs to update hardcoded values. |
| scripts/get-branch-metadata.sh | New helper to read per-branch metadata from branches-metadata.json. |
| branches-metadata.json | Restructured/centralized per-branch milestone + image + URL metadata. |
| scripts/e2e-docker-start | Now selects Rancher image from branch metadata (registry/namespace/name/tag). |
| scripts/build-e2e | Now selects ember ui-index URL from branch metadata. |
| scripts/local-docker-start | New wrapper intended to select image from branch metadata for local starts. |
| package.json | docker:local:start now runs the new wrapper script. |
Comments suppressed due to low confidence (6)
scripts/e2e-docker-start:24
- This script sets
RANCHER_IMG_TAGfrom$1, but later branch-metadata parsing overwritesRANCHER_IMG_TAG, so passing an explicit tag becomes ineffective whenever branch metadata exists. Consider giving the CLI argument precedence (e.g., only read the tag from metadata when no argument is provided) or add an explicit override flag.
if [ $# -eq 1 ]; then
RANCHER_IMG_TAG=$1
# When an image is specified, we use that image, including its front-end, so we don't want the volume args to mount the locally-built UI
VOLUME_ARGS=""
fi
scripts/e2e-docker-start:28
BRANCH_DATAis derived via./scripts/get-branch-metadata.sh, which depends on the current working directory being the repo root. Since this script already computesDIR, use a path based onDIR/$0so it works regardless of where it’s invoked from.
# Target branch of PR
BRANCH_DATA=$(./scripts/get-branch-metadata.sh ${GITHUB_BASE_REF:-${GITHUB_REF_NAME}})
scripts/build-e2e:59
jq -rwill output the literal stringnullwhen the key is missing. As written,[[ -n "$UI_INDEX" ]]will treatnullas non-empty and setEMBER_URLtonull, causingcurlto fail. Use// emptyin the jq expression (or explicitly guard againstnull) before overriding the default URL.
UI_INDEX=$(echo "$BRANCH_DATA" | jq -r '.["rancher-rancher-repo"]."ui-index"')
if [[ -n "$UI_INDEX" ]]; then
EMBER_URL=$UI_INDEX
fi
scripts/build-e2e:62
curl $EMBER_URL ...is unquoted; ifEMBER_URLever contains characters that trigger word-splitting or globbing, the command will break. Quote the variable in the curl invocation.
curl $EMBER_URL -k -o $OUTPUT_EMBER_DIR/index.html
scripts/get-branch-metadata.sh:6
DIR=$(cd $(dirname $0)/..; pwd)is unquoted, so it will break if the repo path contains spaces (and can mis-handle unusual$0values). Quote the expansions and prefer using${BASH_SOURCE[0]}for robustness.
# Get the directory of the script to find the project root
DIR=$(cd $(dirname $0)/..; pwd)
scripts/e2e-docker-start:14
RANCHER_IMGis constructed once (using the initialRANCHER_IMG_TAG=head) and is never recomputed afterRANCHER_IMG_TAGis changed via$1unless branch metadata is found. IfBRANCH_DATAis empty (e.g., running on a non-listed local branch), passing an image tag argument won’t affect the image actually used. RecomputeRANCHER_IMGafter processing$1(and/or after metadata application).
RANCHER_IMG_TAG=head
RANCHER_IMG_REGISTRY=
RANCHER_IMG_NAMESPACE=rancher
RANCHER_IMG_NAME=rancher
RANCHER_IMG=${RANCHER_IMG_NAMESPACE}/${RANCHER_IMG_NAME}:${RANCHER_IMG_TAG}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- switch from.. - hardcoded images - releases branches updated automatically (via PR) on push - to... - images fetched from branches-metadata - we also handle different image registries
90f5f87 to
24d3cb9
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (5)
scripts/get-branch-metadata.sh:22
- In the metadata download,
$METADATA_JSON_URLis unquoted in thecurlinvocation. If the URL ever contains shell metacharacters (e.g. query params with&) this can break the command; quoting is safer and consistent with other robust shell scripts.
METADATA_JSON_URL=https://raw.gh.umua.top/rancher/dashboard/master/branches-metadata.json
METADATA_JSON=$(curl -s -f -L $METADATA_JSON_URL) || {
echo "Error: Failed to download $METADATA_JSON_URL" >&2
exit 1
scripts/e2e-docker-start:28
BRANCH_DATAis fetched via./scripts/get-branch-metadata.sh(relative to the current working directory). If this script is run from withinscripts/(e.g.cd scripts && ./e2e-docker-start), the path resolves toscripts/scripts/get-branch-metadata.shand fails. Use a path relative to the script location (e.g.$(dirname "$0")/get-branch-metadata.shor${DIR}/scripts/get-branch-metadata.sh).
# Target branch of PR
BRANCH_DATA=$(./scripts/get-branch-metadata.sh ${GITHUB_BASE_REF:-${GITHUB_REF_NAME}})
scripts/local-docker-start:13
BRANCH_DATAis fetched via./scripts/get-branch-metadata.sh, which is relative to the current working directory. Iflocal-docker-startis run from a different directory (or via a tool that changes CWD), this will fail. Prefer resolving the repo root from the script location (e.g.DIR=$(cd "$(dirname "$0")/.."; pwd)and then call${DIR}/scripts/get-branch-metadata.sh).
# Target branch of PR
BRANCH_DATA=$(./scripts/get-branch-metadata.sh)
scripts/build-e2e:62
curl $EMBER_URL ...should quote the URL variable. Unquoted URLs that ever include shell metacharacters (notably&in query strings) will be tokenized by the shell and can break the command (or background part of it). Quoting also avoids any accidental word-splitting/globbing if the metadata changes.
curl $EMBER_URL -k -o $OUTPUT_EMBER_DIR/index.html
scripts/build-e2e:62
- Using
curlwith the-kflag here disables TLS certificate validation, allowing an on‑path attacker to impersonateEMBER_URLand inject arbitrary HTML/JS into the e2e UI build. When that HTML is later loaded in a browser with Rancher credentials, this could be used to steal tokens or tamper with test behavior. Remove-kand rely on standard certificate validation (or configure a proper trust store for internal endpoints) instead of bypassing verification.
curl $EMBER_URL -k -o $OUTPUT_EMBER_DIR/index.html
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…ing index info (short term before branch merges)
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (4)
scripts/local-docker-start:13
get-branch-metadata.shis invoked via a relative path (./scripts/...). Because this script doesn’tcdto the repo root (unlike some other scripts), running it from a different working directory will fail to find./scripts/get-branch-metadata.sh. Consider resolving the script directory (e.g., viaDIR=$(cd "$(dirname "$0")"/..; pwd)and calling${DIR}/scripts/get-branch-metadata.sh).
# Target branch of PR
BRANCH_DATA=$(./scripts/get-branch-metadata.sh)
scripts/e2e-docker-start:26
BRANCH_DATAis populated by calling./scripts/get-branch-metadata.sh, but this script does not ensure the current working directory is the repo root. Ifscripts/e2e-docker-startis executed from outside the repo root, the relative path will break even thoughDIRis already computed. Consider invokingget-branch-metadata.shvia an absolute path derived fromDIR, and quote the branch argument to avoid word-splitting.
else
# Get container image from branch-metadata
BRANCH_DATA=$(./scripts/get-branch-metadata.sh ${GITHUB_BASE_REF:-${GITHUB_REF_NAME}})
scripts/build-e2e:62
curl $EMBER_URL ...uses an unquoted URL that can come from branch metadata. If the value contains whitespace or starts with-, it can lead to argument splitting or option injection. Quote the variable and use--before the URL so curl treats it strictly as a URL argument.
EMBER_URL=https://releases.rancher.com/ui/latest2/index.html
# Target branch of PR
BRANCH_DATA=$(./scripts/get-branch-metadata.sh ${GITHUB_BASE_REF:-${GITHUB_REF_NAME}})
if [ -n "$BRANCH_DATA" ]; then
UI_INDEX=$(echo "$BRANCH_DATA" | jq -r '.["rancher-rancher-repo"]."ui-index" // ""')
if [[ -n "$UI_INDEX" ]]; then
EMBER_URL=$UI_INDEX
fi
fi
curl $EMBER_URL -k -o $OUTPUT_EMBER_DIR/index.html
scripts/get-branch-metadata.sh:23
get-branch-metadata.shexits hard if it can’t downloadbranches-metadata.json. Callers likelocal-docker-startande2e-docker-startalready have sensible defaults, so failing the whole script on a transient network/GitHub outage can break local dev and CI unnecessarily. Consider falling back to the in-repobranches-metadata.jsonwhen present, or returning an empty result (with a warning) so callers can continue using defaults.
# 2. find the relevant json object in `master` branches-metadata.json
METADATA_JSON_URL=https://raw.gh.umua.top/rancher/dashboard/master/branches-metadata.json
METADATA_JSON=$(curl -s -f -L $METADATA_JSON_URL) || {
echo "Error: Failed to download $METADATA_JSON_URL" >&2
exit 1
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
rak-phillip
left a comment
There was a problem hiding this comment.
I think this looks good. I have one question about consistency in how the scripts are setup.
This is going to be a great improvement over what we had before 🚀
There was a problem hiding this comment.
It looks like set -e is missing from scripts/e2e-docker-start. Do we want to add that here to be consistent with scripts/get-branch-metadata.sh & scripts/local-docker-start?
There was a problem hiding this comment.
in theory it should be however the file wants to handle errors. given they're all intertwined i've made them consistent, including shebang as well.
Summary
rancher/rancher:head,stgregistry.suse.com/rancher/rancher:v2.12-headhttps://releases.rancher.com/ui/latest2/index.html,https://releases.rancher.com/ui/release-2.12/index.htmlOccurred changes and/or fixed issues
Areas or cases that should be tested
Checklist
Admin,Standard UserandUser Base