Skip to content

Pin virtee/beacon component versions via versions.env - #362

Open
LakshmiSaiHarika wants to merge 1 commit into
AMDEPYC:mainfrom
LakshmiSaiHarika:pin-virtee-tool-versions
Open

LakshmiSaiHarika wants to merge 1 commit into
AMDEPYC:mainfrom
LakshmiSaiHarika:pin-virtee-tool-versions

Conversation

@LakshmiSaiHarika

Copy link
Copy Markdown
Contributor

Introduce a top-level versions.env that pins the specific upstream release tags versions for snphost, snpguest, and beacon. Bumps are now deliberate, reviewed PR changes instead of silently tracking whatever the latest upstream release happens to be at build time.

  • CI: add a resolve-tags job that reads versions.env (falling back to the upstream latest when an entry is empty) and passes the resolved tags to the build job.
  • images/Makefile: include ../versions.env in the tags target; pinned values are used as-is, empty entries still fetch latest.

Introduce a top-level versions.env that pins the upstream release
tags for snphost, snpguest, and beacon. Bumps are now deliberate,
reviewed PR changes instead of silently tracking whatever the latest
upstream release happens to be at build time.

- CI: add a resolve-tags job that reads versions.env (falling back
  to the upstream latest when an entry is empty) and passes the
  resolved tags to the build job.
- images/Makefile: include ../versions.env in the tags target;
  pinned values are used as-is, empty entries still fetch latest.

Signed-off-by: Harika Nittala <[email protected]>

@markg-github markg-github left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

images/README.md should be updated to reflect not necessarily using latest release

Nothing serious. I decided to click Request changes below mostly because I believe requiring pins (vs no pin being allowed and meaning latest) would actually result in fewer checks.

Comment on lines +139 to +144

# Tags resolved once by the resolve-tags job (pinned in versions.env, else upstream latest)
SNPGUEST_TAG="${{ needs.resolve-tags.outputs.snpguest }}"
BEACON_TAG="${{ needs.resolve-tags.outputs.beacon }}"
SNPHOST_TAG="${{ needs.resolve-tags.outputs.snphost }}"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a lot would have to go wrong, but this approach allows shell command injection. Better to use env: block:

  • name: Apply resolved release tags
    env:
    SNPGUEST_TAG: ${{ needs.resolve-tags.outputs.snpguest }}
    BEACON_TAG: ${{ needs.resolve-tags.outputs.beacon }}
    SNPHOST_TAG: ${{ needs.resolve-tags.outputs.snphost }}
    run: |
    set -euo pipefail

    sed -i "/^[Build]/a\Environment=SNPGUEST_TAG=$SNPGUEST_TAG" modules/build/common/snpguest/mkosi.conf
    sed -i "/^[Build]/a\Environment=BEACON_TAG=$BEACON_TAG" modules/build/host/beacon/mkosi.conf
    ...


jobs:
# Resolve component versions once: pinned in versions.env, else upstream latest
resolve-tags:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

resolve-tags inherits write permission. It only needs read. Permissions can be narrowed with a resolve-tags job-level permissions: contents: read

Comment on lines +57 to +59
echo "snphost=$(resolve virtee/snphost "${SNPHOST_TAG:-}")"
echo "snpguest=$(resolve virtee/snpguest "${SNPGUEST_TAG:-}")"
echo "beacon=$(resolve AMDEPYC/beacon "${BEACON_TAG:-}")"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A lot would have to go wrong for resolve to fail, but it failing used to cause the step to fail, now it doesn't, such failures would be masked. One possible fix:

# with this assignment approach, resolve failing isn't masked
snphost=$(resolve virtee/snphost "${SNPHOST_TAG:-}")
snpguest=$(resolve virtee/snpguest "${SNPGUEST_TAG:-}")
beacon=$(resolve AMDEPYC/beacon "${BEACON_TAG:-}")

# some, but not all, resolve failures prevent following code from exiting
for t in "$snphost" "$snpguest" "$beacon"; do
[ -n "$t" ] && [ "$t" != null ] || { echo "::error::could not resolve a component tag"; exit 1; }
done

{
echo "snphost=$snphost"
echo "snpguest=$snpguest"
echo "beacon=$beacon"
} >> "$GITHUB_OUTPUT"

GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
run: |
set -euo pipefail
. versions.env

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typos in versions.env could cause failures whose root causes would be difficult to find - add checks

Comment thread images/Makefile
GH_LATEST = curl -sf $(GH_AUTH) https://api.github.com/repos/$(1)/releases/latest | jq -r '.tag_name'

# Pinned component versions (override on the command line with make VAR=x)
-include ../versions.env

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typos in versions.env could cause failures whose root causes would be difficult to find - add checks

Comment thread versions.env
# Bump deliberately via a PR so any breakage in a new release is caught at review time.
# Leave a value empty to fall back to the upstream latest release at build time.
SNPHOST_TAG=v0.9.0
SNPGUEST_TAG=v0.10.0

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add comment explaining why not the latest release:

snpguest held at v0.10.0: v0.11.0 (virtee/snpguest#145) changed generate id-block --family-id/--image-id from 16-char ASCII to 32-char hex. sev_verify/cvm_props.py passes ASCII defaults and compares report bytes as ASCII, so ID-block generation (id_block_test, key_derivation_test) currently fails on v0.11.0.


Then, verify for successful execution with the built new host and guest OS images under `devel` tag.

The `build-and-release` workflow resolves the `snphost`, `snpguest` and `beacon` release tags in its `resolve-tags` job before building. Versions are pinned in [`versions.env`](https://github.com/AMDEPYC/sev-certify/blob/main/versions.env) at the repository root (an empty entry falls back to the upstream latest release). The resolved repository/version pairs are shown as a table in the `resolve-tags` job summary.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the table?

Comment thread README.md

Host and Guest images are constructed in GitHub Workflows via [`mkosi`](https://github.com/systemd/mkosi). Host images are designed to be booted on a SEV-enabled EPYC server, and are configured with a series of tests in the form of custom systemd services that will run on an embedded guest image. The resulting host and guest images are available in GitHub releases.

The upstream release versions of the components baked into the images — [`snphost`](https://github.com/virtee/snphost), [`snpguest`](https://github.com/virtee/snpguest), and [`beacon`](https://github.com/AMDEPYC/beacon) — are pinned in [`versions.env`](./versions.env) at the repository root. Bumps are made deliberately via a pull request so that any breakage in a new upstream release is caught at review time; the `build-and-release` workflow resolves the pinned tags (falling back to the upstream latest for any empty entry) and reports the resolved `repository → version` pairs in the `resolve-tags` job summary. Leave a value empty in `versions.env` to track the upstream latest release at build time.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

resolve-tags job summary?

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.

2 participants