From 3ce37a6aa4206f4c6d7623bf08eeac0c70a16d9e Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Mon, 22 Feb 2021 10:41:18 +0100 Subject: [PATCH 1/2] vendor: github.com/moby/buildkit v0.8.2 full diff: https://github.com/moby/buildkit/compare/68bb095353c65bc3993fd534c26cf77fe05e61b1...9065b18ba4633c75862befca8188de4338d9f94a - fix seccomp compatibility in 32bit arm - fixes Unable to build alpine:edge containers for armv7 - fixes Buildx failing to build for arm/v7 platform on arm64 machine - resolver: avoid error caching on token fetch - fixes "Error: i/o timeout should not be cached" - fileop: fix checksum to contain indexes of inputs - frontend/dockerfile: add RunCommand.FlagsUsed field - relates to [20.10] Classic builder silently ignores unsupported Dockerfile command flags - update qemu emulators - relates to "Impossible to run git clone inside buildx with non x86 architecture" - Fix reference count issues on typed errors with mount references - fixes errors on releasing mounts with typed execerror refs - fixes / addresses invalid mutable ref when using shared cache mounts - dockerfile/docs: fix frontend image tags - git: set token only for main remote access - fixes "Loading repositories with submodules is repeated. Failed to clone submodule from googlesource" - allow skipping empty layer detection on cache export Signed-off-by: Sebastiaan van Stijn Signed-off-by: Tibor Vass (cherry picked from commit 9962a3f74e73d048394dcaac96468a8864e46dd4) Signed-off-by: Tibor Vass --- vendor.conf | 2 +- .../buildkit/cache/remotecache/v1/utils.go | 6 +- .../frontend/dockerfile/instructions/bflag.go | 9 ++ .../dockerfile/instructions/commands.go | 1 + .../frontend/dockerfile/instructions/parse.go | 2 +- .../buildkit/frontend/gateway/container.go | 2 + .../buildkit/solver/llbsolver/errdefs/exec.go | 9 ++ .../buildkit/solver/llbsolver/ops/exec.go | 16 ++-- .../solver/llbsolver/ops/exec_binfmt.go | 1 + .../buildkit/solver/llbsolver/ops/file.go | 46 +++++++++- .../github.com/moby/buildkit/solver/result.go | 8 +- .../moby/buildkit/solver/scheduler.go | 2 +- .../github.com/moby/buildkit/solver/types.go | 1 + .../moby/buildkit/source/git/gitsource.go | 13 ++- .../moby/buildkit/util/resolver/authorizer.go | 84 +++++++++---------- .../github.com/moby/buildkit/worker/result.go | 8 ++ 16 files changed, 154 insertions(+), 56 deletions(-) diff --git a/vendor.conf b/vendor.conf index b56df8947f..cdf48aefae 100644 --- a/vendor.conf +++ b/vendor.conf @@ -33,7 +33,7 @@ github.com/imdario/mergo 1afb36080aec31e0d1528973ebe6 golang.org/x/sync cd5d95a43a6e21273425c7ae415d3df9ea832eeb # buildkit -github.com/moby/buildkit 68bb095353c65bc3993fd534c26cf77fe05e61b1 # v0.8 branch +github.com/moby/buildkit 9065b18ba4633c75862befca8188de4338d9f94a # v0.8.2 github.com/tonistiigi/fsutil 0834f99b7b85462efb69b4f571a4fa3ca7da5ac9 github.com/tonistiigi/units 6950e57a87eaf136bbe44ef2ec8e75b9e3569de2 github.com/grpc-ecosystem/grpc-opentracing 8e809c8a86450a29b90dcc9efbf062d0fe6d9746 diff --git a/vendor/github.com/moby/buildkit/cache/remotecache/v1/utils.go b/vendor/github.com/moby/buildkit/cache/remotecache/v1/utils.go index fc494aa5a0..9531675ed3 100644 --- a/vendor/github.com/moby/buildkit/cache/remotecache/v1/utils.go +++ b/vendor/github.com/moby/buildkit/cache/remotecache/v1/utils.go @@ -10,6 +10,10 @@ import ( "github.com/pkg/errors" ) +// EmptyLayerRemovalSupported defines if implementation supports removal of empty layers. Buildkit image exporter +// removes empty layers, but moby layerstore based implementation does not. +var EmptyLayerRemovalSupported = true + // sortConfig sorts the config structure to make sure it is deterministic func sortConfig(cc *CacheConfig) { type indexedLayer struct { @@ -239,7 +243,7 @@ func marshalRemote(r *solver.Remote, state *marshalState) string { } desc := r.Descriptors[len(r.Descriptors)-1] - if desc.Digest == exptypes.EmptyGZLayer { + if desc.Digest == exptypes.EmptyGZLayer && EmptyLayerRemovalSupported { return parentID } diff --git a/vendor/github.com/moby/buildkit/frontend/dockerfile/instructions/bflag.go b/vendor/github.com/moby/buildkit/frontend/dockerfile/instructions/bflag.go index d8bf747394..66c53add05 100644 --- a/vendor/github.com/moby/buildkit/frontend/dockerfile/instructions/bflag.go +++ b/vendor/github.com/moby/buildkit/frontend/dockerfile/instructions/bflag.go @@ -108,6 +108,15 @@ func (fl *Flag) IsUsed() bool { return false } +// Used returns a slice of flag names that are set +func (bf *BFlags) Used() []string { + used := make([]string, 0, len(bf.used)) + for f := range bf.used { + used = append(used, f) + } + return used +} + // IsTrue checks if a bool flag is true func (fl *Flag) IsTrue() bool { if fl.flagType != boolType { diff --git a/vendor/github.com/moby/buildkit/frontend/dockerfile/instructions/commands.go b/vendor/github.com/moby/buildkit/frontend/dockerfile/instructions/commands.go index e6027445ac..b900ad7711 100644 --- a/vendor/github.com/moby/buildkit/frontend/dockerfile/instructions/commands.go +++ b/vendor/github.com/moby/buildkit/frontend/dockerfile/instructions/commands.go @@ -269,6 +269,7 @@ type RunCommand struct { withNameAndCode withExternalData ShellDependantCmdLine + FlagsUsed []string } // CmdCommand : CMD foo diff --git a/vendor/github.com/moby/buildkit/frontend/dockerfile/instructions/parse.go b/vendor/github.com/moby/buildkit/frontend/dockerfile/instructions/parse.go index 83cab66944..18e727229c 100644 --- a/vendor/github.com/moby/buildkit/frontend/dockerfile/instructions/parse.go +++ b/vendor/github.com/moby/buildkit/frontend/dockerfile/instructions/parse.go @@ -375,7 +375,7 @@ func parseRun(req parseRequest) (*RunCommand, error) { if err := req.flags.Parse(); err != nil { return nil, err } - + cmd.FlagsUsed = req.flags.Used() cmd.ShellDependantCmdLine = parseShellDependentCommand(req, false) cmd.withNameAndCode = newWithNameAndCode(req) diff --git a/vendor/github.com/moby/buildkit/frontend/gateway/container.go b/vendor/github.com/moby/buildkit/frontend/gateway/container.go index 1f79bb68f0..e124566da9 100644 --- a/vendor/github.com/moby/buildkit/frontend/gateway/container.go +++ b/vendor/github.com/moby/buildkit/frontend/gateway/container.go @@ -127,6 +127,7 @@ type MountRef struct { type MountMutableRef struct { Ref cache.MutableRef MountIndex int + NoCommit bool } type MakeMutable func(m *opspb.Mount, ref cache.ImmutableRef) (cache.MutableRef, error) @@ -196,6 +197,7 @@ func PrepareMounts(ctx context.Context, mm *mounts.MountManager, cm cache.Manage p.Actives = append(p.Actives, MountMutableRef{ MountIndex: i, Ref: active, + NoCommit: true, }) if m.Output != opspb.SkipOutput && ref != nil { p.OutputRefs = append(p.OutputRefs, MountRef{ diff --git a/vendor/github.com/moby/buildkit/solver/llbsolver/errdefs/exec.go b/vendor/github.com/moby/buildkit/solver/llbsolver/errdefs/exec.go index f60757104f..ed3c0d4b0f 100644 --- a/vendor/github.com/moby/buildkit/solver/llbsolver/errdefs/exec.go +++ b/vendor/github.com/moby/buildkit/solver/llbsolver/errdefs/exec.go @@ -21,10 +21,15 @@ func (e *ExecError) Unwrap() error { } func (e *ExecError) EachRef(fn func(solver.Result) error) (err error) { + m := map[solver.Result]struct{}{} for _, res := range e.Inputs { if res == nil { continue } + if _, ok := m[res]; ok { + continue + } + m[res] = struct{}{} if err1 := fn(res); err1 != nil && err == nil { err = err1 } @@ -33,6 +38,10 @@ func (e *ExecError) EachRef(fn func(solver.Result) error) (err error) { if res == nil { continue } + if _, ok := m[res]; ok { + continue + } + m[res] = struct{}{} if err1 := fn(res); err1 != nil && err == nil { err = err1 } diff --git a/vendor/github.com/moby/buildkit/solver/llbsolver/ops/exec.go b/vendor/github.com/moby/buildkit/solver/llbsolver/ops/exec.go index 5ec45fafd5..24aa46aa9f 100644 --- a/vendor/github.com/moby/buildkit/solver/llbsolver/ops/exec.go +++ b/vendor/github.com/moby/buildkit/solver/llbsolver/ops/exec.go @@ -235,7 +235,7 @@ func (e *execOp) Exec(ctx context.Context, g session.Group, inputs []solver.Resu if m.Input == -1 { continue } - execInputs[i] = inputs[m.Input] + execInputs[i] = inputs[m.Input].Clone() } execMounts := make([]solver.Result, len(e.op.Mounts)) copy(execMounts, execInputs) @@ -243,12 +243,16 @@ func (e *execOp) Exec(ctx context.Context, g session.Group, inputs []solver.Resu execMounts[p.OutputRefs[i].MountIndex] = res } for _, active := range p.Actives { - ref, cerr := active.Ref.Commit(ctx) - if cerr != nil { - err = errors.Wrapf(err, "error committing %s: %s", active.Ref.ID(), cerr) - continue + if active.NoCommit { + active.Ref.Release(context.TODO()) + } else { + ref, cerr := active.Ref.Commit(ctx) + if cerr != nil { + err = errors.Wrapf(err, "error committing %s: %s", active.Ref.ID(), cerr) + continue + } + execMounts[active.MountIndex] = worker.NewWorkerRefResult(ref, e.w) } - execMounts[active.MountIndex] = worker.NewWorkerRefResult(ref, e.w) } err = errdefs.WithExecError(err, execInputs, execMounts) } else { diff --git a/vendor/github.com/moby/buildkit/solver/llbsolver/ops/exec_binfmt.go b/vendor/github.com/moby/buildkit/solver/llbsolver/ops/exec_binfmt.go index 0603cf57fc..707c5f6722 100644 --- a/vendor/github.com/moby/buildkit/solver/llbsolver/ops/exec_binfmt.go +++ b/vendor/github.com/moby/buildkit/solver/llbsolver/ops/exec_binfmt.go @@ -27,6 +27,7 @@ var qemuArchMap = map[string]string{ "arm": "arm", "s390x": "s390x", "ppc64le": "ppc64le", + "386": "i386", } type emulator struct { diff --git a/vendor/github.com/moby/buildkit/solver/llbsolver/ops/file.go b/vendor/github.com/moby/buildkit/solver/llbsolver/ops/file.go index 1111cdb72e..554532c184 100644 --- a/vendor/github.com/moby/buildkit/solver/llbsolver/ops/file.go +++ b/vendor/github.com/moby/buildkit/solver/llbsolver/ops/file.go @@ -61,6 +61,8 @@ func (f *fileOp) CacheMap(ctx context.Context, g session.Group, index int) (*sol } } + indexes := make([][]int, 0, len(f.op.Actions)) + for _, action := range f.op.Actions { var dt []byte var err error @@ -103,14 +105,21 @@ func (f *fileOp) CacheMap(ctx context.Context, g session.Group, index int) (*sol } actions = append(actions, dt) + indexes = append(indexes, []int{int(action.Input), int(action.SecondaryInput), int(action.Output)}) + } + + if isDefaultIndexes(indexes) { + indexes = nil } dt, err := json.Marshal(struct { Type string Actions [][]byte + Indexes [][]int `json:"indexes,omitempty"` }{ Type: fileCacheType, Actions: actions, + Indexes: indexes, }) if err != nil { return nil, false, err @@ -421,7 +430,6 @@ func (s *FileOpSolver) getInput(ctx context.Context, idx int, inputs []fileoptyp if cerr == nil { outputRes[idx-len(inputs)] = worker.NewWorkerRefResult(ref.(cache.ImmutableRef), s.w) } - inpMount.Release(context.TODO()) } // If the action has a secondary input, commit it and set the ref on @@ -611,3 +619,39 @@ func (s *FileOpSolver) getInput(ctx context.Context, idx int, inputs []fileoptyp } return inp.(input), err } + +func isDefaultIndexes(idxs [][]int) bool { + // Older version of checksum did not contain indexes for actions resulting in possibility for a wrong cache match. + // We detect the most common pattern for indexes and maintain old checksum for that case to minimize cache misses on upgrade. + // If a future change causes braking changes in instruction cache consider removing this exception. + if len(idxs) == 0 { + return false + } + + for i, idx := range idxs { + if len(idx) != 3 { + return false + } + // input for first action is first input + if i == 0 && idx[0] != 0 { + return false + } + // input for other actions is previous action + if i != 0 && idx[0] != len(idxs)+(i-1) { + return false + } + // secondary input is second input or -1 + if idx[1] != -1 && idx[1] != 1 { + return false + } + // last action creates output + if i == len(idxs)-1 && idx[2] != 0 { + return false + } + // other actions do not create an output + if i != len(idxs)-1 && idx[2] != -1 { + return false + } + } + return true +} diff --git a/vendor/github.com/moby/buildkit/solver/result.go b/vendor/github.com/moby/buildkit/solver/result.go index 75b378c4fd..b35c74b49e 100644 --- a/vendor/github.com/moby/buildkit/solver/result.go +++ b/vendor/github.com/moby/buildkit/solver/result.go @@ -47,7 +47,7 @@ type splitResult struct { func (r *splitResult) Release(ctx context.Context) error { if atomic.AddInt64(&r.released, 1) > 1 { - err := errors.Errorf("releasing already released reference") + err := errors.Errorf("releasing already released reference %+v", r.Result.ID()) logrus.Error(err) return err } @@ -78,10 +78,14 @@ func NewSharedCachedResult(res CachedResult) *SharedCachedResult { } } -func (r *SharedCachedResult) Clone() CachedResult { +func (r *SharedCachedResult) CloneCachedResult() CachedResult { return &clonedCachedResult{Result: r.SharedResult.Clone(), cr: r.CachedResult} } +func (r *SharedCachedResult) Clone() Result { + return r.CloneCachedResult() +} + func (r *SharedCachedResult) Release(ctx context.Context) error { return r.SharedResult.Release(ctx) } diff --git a/vendor/github.com/moby/buildkit/solver/scheduler.go b/vendor/github.com/moby/buildkit/solver/scheduler.go index 5b598bd6b9..bcad4dcdcc 100644 --- a/vendor/github.com/moby/buildkit/solver/scheduler.go +++ b/vendor/github.com/moby/buildkit/solver/scheduler.go @@ -244,7 +244,7 @@ func (s *scheduler) build(ctx context.Context, edge Edge) (CachedResult, error) if err := p.Receiver.Status().Err; err != nil { return nil, err } - return p.Receiver.Status().Value.(*edgeState).result.Clone(), nil + return p.Receiver.Status().Value.(*edgeState).result.CloneCachedResult(), nil } // newPipe creates a new request pipe between two edges diff --git a/vendor/github.com/moby/buildkit/solver/types.go b/vendor/github.com/moby/buildkit/solver/types.go index 76e4c91163..f9de6a93b2 100644 --- a/vendor/github.com/moby/buildkit/solver/types.go +++ b/vendor/github.com/moby/buildkit/solver/types.go @@ -61,6 +61,7 @@ type Result interface { ID() string Release(context.Context) error Sys() interface{} + Clone() Result } // CachedResult is a result connected with its cache key diff --git a/vendor/github.com/moby/buildkit/source/git/gitsource.go b/vendor/github.com/moby/buildkit/source/git/gitsource.go index 7cef764642..3d1bfe21f4 100644 --- a/vendor/github.com/moby/buildkit/source/git/gitsource.go +++ b/vendor/github.com/moby/buildkit/source/git/gitsource.go @@ -231,7 +231,7 @@ func (gs *gitSourceHandler) getAuthToken(ctx context.Context, g session.Group) e if s.token { dt = []byte("basic " + base64.StdEncoding.EncodeToString([]byte(fmt.Sprintf("x-access-token:%s", dt)))) } - gs.auth = []string{"-c", "http.extraheader=Authorization: " + string(dt)} + gs.auth = []string{"-c", "http." + tokenScope(gs.src.Remote) + ".extraheader=Authorization: " + string(dt)} break } return nil @@ -631,3 +631,14 @@ func argsNoDepth(args []string) []string { } return out } + +func tokenScope(remote string) string { + // generally we can only use the token for fetching main remote but in case of github.com we do best effort + // to try reuse same token for all github.com remotes. This is the same behavior actions/checkout uses + for _, pfx := range []string{"https://github.com/", "https://www.github.com/"} { + if strings.HasPrefix(remote, pfx) { + return pfx + } + } + return remote +} diff --git a/vendor/github.com/moby/buildkit/util/resolver/authorizer.go b/vendor/github.com/moby/buildkit/util/resolver/authorizer.go index 80e2a35191..e58038c420 100644 --- a/vendor/github.com/moby/buildkit/util/resolver/authorizer.go +++ b/vendor/github.com/moby/buildkit/util/resolver/authorizer.go @@ -220,15 +220,13 @@ func (a *dockerAuthorizer) AddResponses(ctx context.Context, responses []*http.R // authResult is used to control limit rate. type authResult struct { - sync.WaitGroup token string - err error expires time.Time } // authHandler is used to handle auth request per registry server. type authHandler struct { - sync.Mutex + g flightcontrol.Group client *http.Client @@ -240,7 +238,8 @@ type authHandler struct { // scopedTokens caches token indexed by scopes, which used in // bearer auth case - scopedTokens map[string]*authResult + scopedTokens map[string]*authResult + scopedTokensMu sync.Mutex lastUsed time.Time @@ -292,46 +291,44 @@ func (ah *authHandler) doBearerAuth(ctx context.Context, sm *session.Manager, g // Docs: https://docs.docker.com/registry/spec/auth/scope scoped := strings.Join(to.Scopes, " ") - ah.Lock() - for { + res, err := ah.g.Do(ctx, scoped, func(ctx context.Context) (interface{}, error) { + ah.scopedTokensMu.Lock() r, exist := ah.scopedTokens[scoped] - if !exist { - // no entry cached - break - } - ah.Unlock() - r.Wait() - if r.err != nil { - select { - case <-ctx.Done(): - return "", r.err - default: + ah.scopedTokensMu.Unlock() + if exist { + if r.expires.IsZero() || r.expires.After(time.Now()) { + return r, nil } } - if !errors.Is(r.err, context.Canceled) && - (r.expires.IsZero() || r.expires.After(time.Now())) { - return r.token, r.err - } - // r.err is canceled or token expired. Get rid of it and try again - ah.Lock() - r2, exist := ah.scopedTokens[scoped] - if exist && r == r2 { - delete(ah.scopedTokens, scoped) + r, err := ah.fetchToken(ctx, sm, g, to) + if err != nil { + return nil, err } + ah.scopedTokensMu.Lock() + ah.scopedTokens[scoped] = r + ah.scopedTokensMu.Unlock() + return r, nil + }) + + if err != nil || res == nil { + return "", err } + r := res.(*authResult) + if r == nil { + return "", nil + } + return r.token, nil +} - // only one fetch token job - r := new(authResult) - r.Add(1) - ah.scopedTokens[scoped] = r - ah.Unlock() - +func (ah *authHandler) fetchToken(ctx context.Context, sm *session.Manager, g session.Group, to auth.TokenOptions) (r *authResult, err error) { var issuedAt time.Time var expires int + var token string defer func() { token = fmt.Sprintf("Bearer %s", token) - r.token, r.err = token, err + if err == nil { + r = &authResult{token: token} if issuedAt.IsZero() { issuedAt = time.Now() } @@ -339,7 +336,6 @@ func (ah *authHandler) doBearerAuth(ctx context.Context, sm *session.Manager, g r.expires = exp } } - r.Done() }() if ah.authority != nil { @@ -351,10 +347,11 @@ func (ah *authHandler) doBearerAuth(ctx context.Context, sm *session.Manager, g Scopes: to.Scopes, }, sm, g) if err != nil { - return "", err + return nil, err } issuedAt, expires = time.Unix(resp.IssuedAt, 0), int(resp.ExpiresIn) - return resp.Token, nil + token = resp.Token + return nil, nil } // fetch token for the resource scope @@ -374,29 +371,32 @@ func (ah *authHandler) doBearerAuth(ctx context.Context, sm *session.Manager, g if (errStatus.StatusCode == 405 && to.Username != "") || errStatus.StatusCode == 404 || errStatus.StatusCode == 401 { resp, err := auth.FetchTokenWithOAuth(ctx, ah.client, nil, "buildkit-client", to) if err != nil { - return "", err + return nil, err } issuedAt, expires = resp.IssuedAt, resp.ExpiresIn - return resp.AccessToken, nil + token = resp.AccessToken + return nil, nil } log.G(ctx).WithFields(logrus.Fields{ "status": errStatus.Status, "body": string(errStatus.Body), }).Debugf("token request failed") } - return "", err + return nil, err } issuedAt, expires = resp.IssuedAt, resp.ExpiresIn - return resp.Token, nil + token = resp.Token + return nil, nil } // do request anonymously resp, err := auth.FetchToken(ctx, ah.client, nil, to) if err != nil { - return "", errors.Wrap(err, "failed to fetch anonymous token") + return nil, errors.Wrap(err, "failed to fetch anonymous token") } issuedAt, expires = resp.IssuedAt, resp.ExpiresIn - return resp.Token, nil + token = resp.Token + return nil, nil } func invalidAuthorization(c auth.Challenge, responses []*http.Response) error { diff --git a/vendor/github.com/moby/buildkit/worker/result.go b/vendor/github.com/moby/buildkit/worker/result.go index e178ef3fff..9f4cdc5bfa 100644 --- a/vendor/github.com/moby/buildkit/worker/result.go +++ b/vendor/github.com/moby/buildkit/worker/result.go @@ -52,3 +52,11 @@ func (r *workerRefResult) Release(ctx context.Context) error { func (r *workerRefResult) Sys() interface{} { return r.WorkerRef } + +func (r *workerRefResult) Clone() solver.Result { + r2 := *r + if r.ImmutableRef != nil { + r.ImmutableRef = r.ImmutableRef.Clone() + } + return &r2 +} From 35f5f9e624e44c5e5586dfddacfc87a7b2f02953 Mon Sep 17 00:00:00 2001 From: Tibor Vass Date: Thu, 25 Feb 2021 01:14:37 +0000 Subject: [PATCH 2/2] builder: fix incorrect cache match for inline cache with empty layers See https://github.com/moby/buildkit/pull/1993 Signed-off-by: Tibor Vass (cherry picked from commit 9bf93e90fa9ce74b6f7309a22838da8189838b3b) Signed-off-by: Tibor Vass --- .../builder-next/adapters/localinlinecache/inlinecache.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/builder/builder-next/adapters/localinlinecache/inlinecache.go b/builder/builder-next/adapters/localinlinecache/inlinecache.go index 0c51e207e6..c5e19ce2d5 100644 --- a/builder/builder-next/adapters/localinlinecache/inlinecache.go +++ b/builder/builder-next/adapters/localinlinecache/inlinecache.go @@ -22,6 +22,11 @@ import ( "github.com/pkg/errors" ) +func init() { + // See https://github.com/moby/buildkit/pull/1993. + v1.EmptyLayerRemovalSupported = false +} + // ResolveCacheImporterFunc returns a resolver function for local inline cache func ResolveCacheImporterFunc(sm *session.Manager, resolverFunc docker.RegistryHosts, cs content.Store, rs reference.Store, is imagestore.Store) remotecache.ResolveCacheImporterFunc {