-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Collection of fixes for attestations and platform shenanigans #3379
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
ddf1420
e0b7283
11a6960
c385601
eb38f6b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -101,6 +101,9 @@ func (e *localExporterInstance) Export(ctx context.Context, inp *exporter.Source | |
| } | ||
|
|
||
| isMap := len(inp.Refs) > 0 | ||
| if len(inp.Attestations) > 0 { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I do not understand this. Needs a comment. I still don't understand it even after reading the commit message (it says that there is a "flaw" and now things work because of a "quirk" but not what this line really does). |
||
| isMap = false | ||
| } | ||
|
|
||
| platformsBytes, ok := inp.Metadata[exptypes.ExporterPlatformsKey] | ||
| if len(inp.Refs) > 0 && !ok { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,6 +16,7 @@ import ( | |
| gogotypes "github.com/gogo/protobuf/types" | ||
| "github.com/golang/protobuf/ptypes/any" | ||
| "github.com/moby/buildkit/client/llb" | ||
| "github.com/moby/buildkit/frontend/attestations" | ||
| "github.com/moby/buildkit/frontend/gateway/client" | ||
| pb "github.com/moby/buildkit/frontend/gateway/pb" | ||
| "github.com/moby/buildkit/identity" | ||
|
|
@@ -161,7 +162,7 @@ func (c *grpcClient) Run(ctx context.Context, f client.BuildFunc) (retError erro | |
| } | ||
| } | ||
|
|
||
| if res.Attestations != nil && c.caps.Supports(pb.CapAttestations) == nil { | ||
| if res.Attestations != nil { | ||
| attestations := map[string]*pb.Attestations{} | ||
| for k, as := range res.Attestations { | ||
| for _, a := range as { | ||
|
|
@@ -483,8 +484,20 @@ func (c *grpcClient) ResolveImageConfig(ctx context.Context, ref string, opt llb | |
| } | ||
|
|
||
| func (c *grpcClient) BuildOpts() client.BuildOpts { | ||
| opts := c.opts | ||
| if c.caps.Supports(pb.CapAttestations) == nil { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I think the better behavior in this cases would be to make sure |
||
| opts = map[string]string{} | ||
| attestOpts := attestations.Filter(c.opts) | ||
| for k, v := range c.opts { | ||
| if _, ok := attestOpts[k]; ok { | ||
| continue | ||
| } | ||
| opts[k] = v | ||
| } | ||
| } | ||
|
|
||
| return client.BuildOpts{ | ||
| Opts: c.opts, | ||
| Opts: opts, | ||
| SessionID: c.sessionID, | ||
| Workers: c.workers, | ||
| Product: c.product, | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if the
len(inp.Refs) > 0clause above is wrong then it should be removed. If it is not wrong then this patch does not maintain that behavior.