Skip to content

Commit d9ec1cf

Browse files
committed
Verify a freshly built image before it is published
The invariants step ran after ecr-build-push-pull returned, but the action pushes the commit tag and the deps tag inside itself, so an image that fails the check is already in the shared cache. That happened four seconds apart on newton-world-prefixes (run 33735349483): the deps tag deps-6aebe721c56a91f7 was pushed at 09:05:07 and the invariant failed at 09:05:11, leaving an image whose Isaac Sim extensions cannot load. Any later run whose install inputs hash the same then gets a cache hit and inherits it, so gating the check on whether this run built the image would let the second push of the change that broke it go green. Move the assertion into the action, before every push, behind a verify-command input the base image job passes. A failure now publishes nothing, so the next run rebuilds and fails again. The built output added for the old gate is gone with it.
1 parent d2fbce5 commit d9ec1cf

4 files changed

Lines changed: 59 additions & 31 deletions

File tree

.github/actions/ecr-build-push-pull/README.md

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,11 +16,17 @@ ECR is also used as the BuildKit layer cache.
1616
ecr-url: (optional, complete url for ECR storage)
1717
```
1818
19-
## Outputs
19+
## Verifying a freshly built image
2020
21-
- `built`: `'true'` when this run built the image, so it exists locally under `image-tag`.
22-
An exact-tag hit pulls it as well; a deps-cache hit only creates the ECR tag unless
23-
`pull-on-deps-hit` is set. Gate steps that need the image locally on this output.
21+
Pass `verify-command` to assert against the image before it is published:
22+
23+
```yaml
24+
verify-command: uv run --no-project --with pytest python -m pytest -q docker/test/test_image_invariants.py
25+
```
26+
27+
It runs only on a full build, with `IMAGE_TAG` and `IMAGE_DIGEST` exported. A failure fails the
28+
action with nothing pushed, so the next run rebuilds rather than serving the bad image from the
29+
deps cache. Exact-tag and deps-cache hits skip it: that image passed the command when it was built.
2430

2531
## ECR URL resolution order
2632

.github/actions/ecr-build-push-pull/action.yml

Lines changed: 28 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,16 @@ inputs:
3737
description: Tag used for the ECR layer cache image (e.g. "cache-base", "cache-curobo").
3838
required: false
3939
default: 'cache'
40+
verify-command:
41+
description: >
42+
Command run against a freshly built image, before it is tagged or pushed, with IMAGE_TAG
43+
and IMAGE_DIGEST exported. A failing command fails the action with nothing published, so
44+
the next run rebuilds instead of inheriting the bad image from the cache.
45+
46+
Not run on an exact-tag or deps-cache hit: those serve an image that already passed this
47+
command when it was built.
48+
required: false
49+
default: ''
4050
pull-on-deps-hit:
4151
description: >
4252
Pull the image locally after a deps-cache hit. Needed by jobs that run
@@ -50,15 +60,6 @@ inputs:
5060
that only need the tag pushed.
5161
required: false
5262
default: 'false'
53-
outputs:
54-
built:
55-
description: >
56-
'true' when this run built the image, so it exists locally under image-tag.
57-
An exact-tag hit pulls it as well; a deps-cache hit only creates the ECR tag
58-
unless pull-on-deps-hit is set, so steps that need the image locally should
59-
check this output.
60-
value: ${{ steps.pull-exact.outputs.hit != 'true' && steps.deps-cache.outputs.deps-cache-hit != 'true' }}
61-
6263
runs:
6364
using: composite
6465
steps:
@@ -251,6 +252,24 @@ runs:
251252
cache-to: ${{ steps.resolve-ecr.outputs.available == 'true' && format('type=registry,ref={0},mode=max', env.CACHE_IMAGE) || '' }}
252253
deps-hash: ${{ steps.deps-hash.outputs.hash }}
253254

255+
# Assert against the image while it is only local: the push steps below publish under both
256+
# the commit tag and the deps tag, and a deps-cache hit later serves that image without
257+
# rebuilding it, so anything published unverified stays unverified.
258+
- name: Verify freshly built image
259+
if: >
260+
inputs.verify-command != '' &&
261+
steps.pull-exact.outputs.hit != 'true' &&
262+
steps.deps-cache.outputs.deps-cache-hit != 'true'
263+
shell: bash
264+
env:
265+
IMAGE_TAG: ${{ inputs.image-tag }}
266+
VERIFY_COMMAND: ${{ inputs.verify-command }}
267+
run: |
268+
set -euo pipefail
269+
IMAGE_DIGEST="$(docker image inspect --format '{{.Id}}' "${IMAGE_TAG}")"
270+
export IMAGE_DIGEST
271+
bash -c "${VERIFY_COMMAND}"
272+
254273
- name: Tag built image with ECR-prefixed name
255274
if: >
256275
steps.resolve-ecr.outputs.available == 'true' &&

.github/workflows/build.yaml

Lines changed: 4 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -207,32 +207,18 @@ jobs:
207207
with:
208208
enable-cache: true
209209

210+
# #6329 aborts the pip install when it strands a prebundled package's __init__.py
211+
# (nvbugs 6343978: 14 Isaac Sim extensions fail to load). The images install with
212+
# ``uv sync``, which never runs that guard, so assert the same invariant on the image.
210213
- name: Build and push to ECR
211-
id: image
212214
uses: ./.github/actions/ecr-build-push-pull
213215
with:
214216
image-tag: ${{ needs.config.outputs.ci_image_tag }}
215217
isaacsim-base-image: ${{ needs.config.outputs.isaacsim_image_name }}
216218
isaacsim-version: ${{ needs.config.outputs.isaacsim_image_tag }}
217219
dockerfile-path: docker/Dockerfile.base
218220
cache-tag: cache-base
219-
220-
# #6329 aborts the pip install when it strands a prebundled package's __init__.py
221-
# (nvbugs 6343978: 14 Isaac Sim extensions fail to load). The images install with
222-
# ``uv sync``, which never runs that guard, so assert the same invariant on the image.
223-
# Only a freshly built image is on this runner; a deps-cache hit reuses one that
224-
# passed this check when it was built.
225-
- name: Verify image invariants
226-
if: steps.image.outputs.built == 'true'
227-
shell: bash
228-
env:
229-
IMAGE_TAG: ${{ needs.config.outputs.ci_image_tag }}
230-
run: |
231-
set -euo pipefail
232-
IMAGE_DIGEST="$(docker image inspect --format '{{.Id}}' "${IMAGE_TAG}")"
233-
export IMAGE_DIGEST
234-
uv run --no-project --with pytest \
235-
python -m pytest -q docker/test/test_image_invariants.py
221+
verify-command: uv run --no-project --with pytest python -m pytest -q docker/test/test_image_invariants.py
236222

237223
build-curobo:
238224
name: Build cuRobo Docker Image

docker/test/test_container_profiles.py

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -289,6 +289,23 @@ def test_kitless_compose_service_has_no_isaac_sim_mounts():
289289
assert all("/kit/" not in mount["target"].lower() for mount in mounts)
290290

291291

292+
def test_image_is_verified_before_it_is_published():
293+
"""A published image must be a verified one.
294+
295+
The push steps publish under both the commit tag and the deps tag, and a later deps-cache hit
296+
serves that image without rebuilding it, so anything published unverified stays unverified.
297+
"""
298+
action = yaml.safe_load(
299+
(REPO_ROOT / ".github" / "actions" / "ecr-build-push-pull" / "action.yml").read_text(encoding="utf-8")
300+
)
301+
names = [step["name"] for step in action["runs"]["steps"] if "name" in step]
302+
303+
assert names.index("Verify freshly built image") < names.index("Push to ECR") < names.index("Push deps tag")
304+
305+
build = (REPO_ROOT / ".github" / "workflows" / "build.yaml").read_text(encoding="utf-8")
306+
assert "verify-command: uv run" in build, "the base image job must hand its invariants to the action"
307+
308+
292309
def test_run_tests_links_isaac_sim_only_where_kit_is_installed():
293310
"""The kit-less image has no Kit under ``/isaac-sim``, which the runtime mounts create anyway.
294311

0 commit comments

Comments
 (0)