This is possible with llb.Scratch in a multi-platform build. We were
accidentally discarding the nil entries in Refs.
Instead, we just skip over nil refs. This isn't ideal - we *should* be
able generate provenance for a nil ref. However, 1. this isn't handled
for llb.Scratch as a single-platform result, and 2. this is tricky,
since the ResultProxy is nil at this point (so there's no ID to match
on). This should be ok for a quick fix, but we can come back and fix
this later.
Signed-off-by: Justin Chadwell <me@jedevc.com>
Since go 1.20, interfaces (that can be compared) now implement the
comparable interface. The use of reflection in solver/result was a
workaround for this limitation, however, we can remove this with the
upstream fix.
Signed-off-by: Justin Chadwell <me@jedevc.com>
Previously, we would attempt to add file data for every single
SBOM - however, if these SBOMs were taken of layers that were not
exported, then these could be wrong.
To workaround this, for the file layer details to be added to the
resulting SBOM, we require that the scanner add a metadata property to
indicate the default value. This is configurable, since in the future we
may want behavior that allows the frontend to specify no file layers, or
wants an SBOM with layers other than the default.
Signed-off-by: Justin Chadwell <me@jedevc.com>
This reverts commit 6f21d6b403.
We don't need the exporter opts for multiplatform, they were a hack to
attempt to preserve the semantics of converting between Ref->Refs
freely (both buildkit-side and frontend-side).
Signed-off-by: Justin Chadwell <me@jedevc.com>
This reverts commit f2c770e45e.
This patch reworks the attestations code back out of the Refs map. The
logic for this change is that the loss-of-granularity in the data
structure for the protobuf requires convoluted logic in the exporter to
be able to determine the correct multi-platform behaviour.
Prior to attestations, Refs directly mapped each platform to a
reference. Attestations changed this structure, which means that there
is no longer a concrete way to determine if a reference should be
exported with multi-platform semantics or not (e.g. should a local
export with a single platform be flat/nested, should an image export use
an oci manifest or an oci index).
This patch restores the previous semantics, moving the attestion refs
back, pushing the attestations back down into a separate struct. This
has the disadvantage of requiring the result.Attestation struct to be
generic, however, this does simplify some of the helper methods around
it.
By changing this logic, we're able to simplify a few key pieces of
logic:
- Fewer version interop concerns. Because we're not changing the
structure of Ref/Refs between buildkit versions, mixing frontends and
buildkit versions will *always* produce the expected result. If a
frontend attaches attestations they won't be transmitted at all
automatically. Additionally, because attestations can also be attached
to a single Ref, the frontend doesn't need to force an index in the
future.
- Less complex exporter logic, we don't need to any magic around
detecting the user intention with regards to multi-platform exports,
we simply follow the previous semantics of len(res.Refs) > 0. We only
need the small snippet that ensures that the inline-only attestations
don't get attached to a single manifest, to keep the docker load case
working.
- Some simplification of the SBOM scanner code, we can return an
Attestation[llb.State], which we can then easily Convert into an
actual Attestation[Ref].
The patch looks large, but most of the changes are mostly just function
signature changes - the meat of the changes is in the rework of the
platform detection code. This is so we can support the use case of Ref +
Attestations, Refs + Attestations (multi-platform=false) and Refs +
Attestations (multi-platform=true).
Signed-off-by: Justin Chadwell <me@jedevc.com>
We can now attach a reason for why each attestation was generated, which
will in future allow us to include/exclude specific attestation types
from exporters if desired.
Signed-off-by: Justin Chadwell <me@jedevc.com>
The Result struct in this package is intended to replace the various
Result structs across the codebase, including:
- frontend.Result
- client.Result/gateway.Result
- exporter.Source
These Results are all fundamentally the same, and share the same logic,
with the only difference being the type used in the Ref/Refs properties.
Since these were all separate, we easily encounter the following
problems:
- Repetition of struct and method declarations.
Each separate Result type required it's own properties and methods to
be redeclared. Sometimes these were consistent with each other, and
other times they were not. Unifying all Results together ensures a
lack of duplication, and means that changing the structure of the Result
will not require extensive changes throughout every single declaration.
- Complex conversion between Result types.
At gateway and api boundaries, we are required to convert each Result
into a Result of a different type. Previously, this has been a lengthly
and error prone conversion, requiring knowledge of the exact structure
of the Result, so as to allow traversal. By unifying Results, we can
implement a ConvertResult function to convert from types of Result[A]
to types of Result[B] given a function from A to B, which cleanly
encapsulates the structure of the Result.
- Complex conversion to the protobuf gateway api types.
At the gateway api, we have to convert to pb.Results. Previously, we
had to include logic to convert to and from both frontend.Result and
client.Result. With a generic Result, we can simple convert to a
Result[pb.Ref] and then have one single set of methods to convert
Result[pb.Ref] to a pb.Result.
These problems have been an issue for some time, however, recently with
the addition of attestations, the problem has grown, since the amount of
logic required at conversion boundaries has massively increased.
This patch intends to simplify all of the above, and reduce line-count
and code complexity by using the new-ish go generics features. Care has
been taken to ensure that this patch is as small as possible, and
affects as few Go APIs as possible. This is ensured by using type aliases
for the Result type to re-create the various previous Results, e.g.
frontend.Result/client.Result/exporter.Source, so that little code in
packages that utilize these Results need to be changed. The only major
changes take place at conversion boundaries, where, for the most part,
the code is simplified.
One major caveat is that the result package is made significantly more
complex, due to the generic code, and the need for reflection to perform
comparisons of any types to nil, as interfaces such solver.ResultProxy
(used as a Ref type) do not implement the comparable interface, which is
a limitation of the current go language spec (hopefully to be resolved
in the future).
Signed-off-by: Justin Chadwell <me@jedevc.com>
