Skip to content

Introduce new attestation exporter options#3403

Closed
jedevc wants to merge 5 commits intomoby:masterfrom
jedevc:exporter-attestations-options
Closed

Introduce new attestation exporter options#3403
jedevc wants to merge 5 commits intomoby:masterfrom
jedevc:exporter-attestations-options

Conversation

@jedevc
Copy link
Copy Markdown
Member

@jedevc jedevc commented Dec 14, 2022

These replace the inline-only attestation fields introduce in #3342.

Instead, we can now specify attestations=<true/false/filter-list> per exporter, to enable specific attestation per exporter - the default in buildkit is set to true. The possible values are:

  • true: output all provided attestations from the solver
  • false: output none of the provided attestations from the solver
  • reason,[reasons]: a csv list of reasons, outputs only the attestations that were generated for one of those specified reasons, using the metadata field. e.g. --output type=...,attestations=provenance will only include provenance attestations in the output for that exporter, even if other attestations (like SBOMs) were generated in the solver.

We also update the attestation-prefix option to be attestation[s]-prefix - to be more aligned with the new attestations flag.

Additionally, we have a few smaller minor refactoring commits around the exporter:

  • Propogate metadata through unbundling (wasn't causing any issues now, but could in the future, since logically we should inherit metadata through the process)
  • Moves fs opt parsing into shared logic between the local+tar exporters, so we don't need to duplicate all of that.
  • Removes non-dist options from the tar exporter (introduced in Do not re-tag non-distributable blob descriptors #2561) - these aren't ever used, so we can silently drop them. If clients were using this, no error will occur, it will just silently do nothing.

Copy link
Copy Markdown
Member

@tonistiigi tonistiigi left a comment

Choose a reason for hiding this comment

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

I'm not convinced we should remove inline-only. I think this is the correct way for client to ask for expected behavior without knowing too much about the internals. I don't mind a way to skip attestation on exporter level as well if there is a use case.

@jedevc
Copy link
Copy Markdown
Member Author

jedevc commented Dec 16, 2022

Removed inline-only in #3420 - we shouldn't block the rest of the changes on that.

}

for _, att := range attestations {
target, ok := att.Metadata[result.AttestationReasonKey]
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Not from this PR but I don't understand why this is called a "reason". It is attestation type. So why can't we just use the already existing predicate type field?

jedevc added 5 commits January 5, 2023 14:25
Signed-off-by: Justin Chadwell <me@jedevc.com>
This allows one implementation for all the opts parsing, similar to how
we do today for the ImageCommitOpts. Additionally, we rename
attestation-prefix to attestations-prefix (pluralized) to prepare for
the new attestations option.

Signed-off-by: Justin Chadwell <me@jedevc.com>
This option looks like a mistake added in 45fc3ed.
These options aren't ever used, so we don't need to parse them, we can
just silently discard them.

Signed-off-by: Justin Chadwell <me@jedevc.com>
Signed-off-by: Justin Chadwell <me@jedevc.com>
Instead of just the boolean true/false values, we allow the attestation
option for exporters to contain an arbitrary list of "attestation
reasons". Only attestations that have a reason matching the list will
actually be output.

This allows clients to completely detach the concepts of "what
attestations to generate" and "what attestations to output".

Signed-off-by: Justin Chadwell <me@jedevc.com>
@jedevc jedevc force-pushed the exporter-attestations-options branch from fe28241 to 2488167 Compare January 5, 2023 14:30
@jedevc jedevc closed this Jan 6, 2023
ycpss91255 added a commit to ycpss91255-docker/template that referenced this pull request Apr 24, 2026
`ARG TARGETARCH=amd64` (with a default) silently shadows BuildKit's
per-platform auto-inject (moby/buildkit#3403). Consequence: every
multi-arch build of this Dockerfile — including every `:vX.Y.Z` and
`:latest` pushed to GHCR by release-test-tools.yaml since v0.9.10 —
ran its `case` branch with TARGETARCH=amd64 *regardless of target
platform*. The arm64 variant therefore shipped x86_64 shellcheck
and hadolint binaries; downstream CI on arm64 runners fails at
`shellcheck: Exec format error` the first time it consumes
test-tools on arm64 (caught by ros1_bridge PR #27 enabling the arm64
matrix).

Fix: drop the default. `ARG TARGETARCH` (bare declaration) lets
BuildKit auto-inject take effect. Outside BuildKit, pass
`--build-arg TARGETARCH=<arch>` explicitly — the `case` default
branch already fails loud on empty TARGETARCH so nobody silently
ships the wrong binaries.

Regression test in template_spec.bats asserts the ARG has no
default, plus the existing bare-declaration test continues to
guarantee consumption is still wired. 674 → 675.

Re-release plan: tag v0.10.0-rc2 after merge. release-test-tools.yaml
re-runs on tag push and reissues ghcr.io/.../test-tools:v0.10.0-rc2
+ :latest with correct per-arch binaries. Downstream repos on the
broken v0.9.13 / v0.10.0-rc1 tags stay broken on arm64 until they
bump @tag.
ycpss91255 added a commit to ycpss91255-docker/ros1_bridge that referenced this pull request Apr 24, 2026
v0.9.13's release-test-tools.yaml shipped a multi-arch
ghcr.io/ycpss91255-docker/test-tools:v0.9.13 with x86_64 shellcheck /
hadolint binaries in BOTH manifest variants — the `ARG TARGETARCH=amd64`
default in template/dockerfile/Dockerfile.test-tools shadowed
BuildKit's per-platform auto-inject (moby/buildkit#3403), so every
multi-arch run fell back to amd64. This branch enabled the arm64
matrix but kept pointing at @v0.9.13, so the arm64 shard blew up
at `shellcheck: Exec format error` on first run.

Template v0.10.0-rc2 (#126 / #127) drops the ARG default. The rc2
GHCR image's arm64 variant contains real aarch64 binaries (verified
via `docker cp` + `file`). Bumping build-worker.yaml + release-
worker.yaml @tag so this PR's arm64 shard picks up the correct
image.
ycpss91255 added a commit to ycpss91255-docker/ros1_bridge that referenced this pull request Apr 24, 2026
* ci: enable arm64 in build matrix (linux/amd64,linux/arm64)

PR #23 rebuilt devel from a multi-arch base for Jetson support, but
main.yaml never opted in to the template's arm64 build matrix — the
build-worker.yaml input defaults to linux/amd64 only. As a result, every
PR since #23 has only verified amd64 in CI; arm64 has been validated
manually via rsync-to-Jetson.

Pass platforms: linux/amd64,linux/arm64 to build-worker.yaml so CI runs
both architectures in parallel (template v0.9.11 added the
ubuntu-24.04-arm runner support).

* ci: bump template @tag to v0.10.0-rc2 for arm64 test-tools fix

v0.9.13's release-test-tools.yaml shipped a multi-arch
ghcr.io/ycpss91255-docker/test-tools:v0.9.13 with x86_64 shellcheck /
hadolint binaries in BOTH manifest variants — the `ARG TARGETARCH=amd64`
default in template/dockerfile/Dockerfile.test-tools shadowed
BuildKit's per-platform auto-inject (moby/buildkit#3403), so every
multi-arch run fell back to amd64. This branch enabled the arm64
matrix but kept pointing at @v0.9.13, so the arm64 shard blew up
at `shellcheck: Exec format error` on first run.

Template v0.10.0-rc2 (#126 / #127) drops the ARG default. The rc2
GHCR image's arm64 variant contains real aarch64 binaries (verified
via `docker cp` + `file`). Bumping build-worker.yaml + release-
worker.yaml @tag so this PR's arm64 shard picks up the correct
image.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants