fix(sdk): return a single version tag when a commit has multiple tags - #53
Merged
Merged
Conversation
`get_git_version()` used the raw output of `git tag --points-at HEAD`, which prints one line per tag. Two ACL releases were cut from the same `aclmain` commit (3.0.20260706-3.0-1153684 and 3.0.20260809-3.0-1179296), so the OS version became a two-line string. That multi-line value flowed into `create_versionfile` and into the container name derived by `run_sdk_container`, producing: Error response from daemon: Invalid container name (flatcar-sdk-all-4459.0.0_os-3.0.20260706-3.0-1153684 3.0.20260809-3.0-1179296), only [a-zA-Z0-9][a-zA-Z0-9_.-] are allowed which failed "Build ACL Base Image" on every RPM image leg (amd64 and aarch64, Azure and QEMU). Pick the highest version tag deterministically with `sort -V | tail -n 1`, and clamp `git describe` to one line. Also clamp `vernum_from_version` and `build_id_from_version`, whose `sed -n ...p` emits one line per match, so no caller-supplied version can reintroduce a multi-line container name. Single-tag behaviour is unchanged. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR hardens SDK/OS version derivation in sdk_lib/sdk_container_common.sh to prevent multi-line version strings (e.g., when a commit is annotated with multiple release tags) from propagating into the versionfile and Docker container names, which can break container creation.
Changes:
- Update
get_git_version()to deterministically select a single tag whenHEADhas multiple tags. - Clamp version parsing helpers (
vernum_from_version(),build_id_from_version()) to a single output line to prevent multi-line fragments from propagating. - Clamp the
git describefallback output to one line.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Review feedback: `git describe --tags | head -n 1` made the fallback branch always exit 0, so a `git describe` failure (rc 128) would no longer abort callers such as `run_sdk_container`, which runs under `set -e` and does a plain `os_version=$(get_git_version)` assignment. Use `git for-each-ref --count=1 --sort=-v:refname` to pick the highest tag directly, and leave `git describe --tags` unpiped so its exit status still propagates. `git describe` only ever prints one line, so the `head -n 1` clamp was unnecessary. Verified `for-each-ref` returns the same tag as `sort -V | tail -n 1` for the real failing tag pair (3.0.20260706-3.0-1153684 and 3.0.20260809-3.0-1179296), that single-tag and untagged-HEAD behaviour is unchanged, and that a git failure now yields rc 128 again. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
SeanDougherty
approved these changes
Aug 26, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Build acl-pr.20260817.1184334 failed the Build ACL Base Image task on every RPM image leg (amd64 + aarch64, Azure + QEMU) with:
Root cause
acl/build_rpm_image.shinvokesrun_sdk_containerwithout-v, so the OS version falls back toget_git_version():git tag --points-atprints one line per tag. Two ACL releases were cut from the sameaclmaincommit:So
os_versionbecame a two-line string.strip_version_prefix()then amplified it —vernum_from_version()andbuild_id_from_version()usesed -n ...p, which also emits one line per match — yielding the exact three-line value seen in the log.run_sdk_containerinterpolates that into the container name, and Docker rejects it.Nothing in the PR under test caused this: the build broke the moment a second release tag landed on the already-tagged
aclmaincommit.Fix
get_git_version()picks the highest version tag deterministically viagit for-each-ref --count=1 --sort=-v:refname, with no pipeline, so agitfailure is never masked by a trailingtail/head; thegit describefallback is left unpiped so its exit status still propagates.vernum_from_version()andbuild_id_from_version()are clamped withhead -n 1, so no caller-supplied version (e.g. via-v) can reintroduce a multi-line container name.Single-tag behaviour is unchanged.
Validation
Exercised the version helpers against the real failing tag set:
bash -n sdk_lib/sdk_container_common.shpasses.