From 778221cbfd50c8b91eeb02fb3e52444bd938f81d Mon Sep 17 00:00:00 2001 From: Tonis Tiigi Date: Thu, 12 Aug 2021 09:46:47 -0700 Subject: [PATCH 1/7] dockerfile: fix parsing required key without value Signed-off-by: Tonis Tiigi (cherry picked from commit 67352249e77b0dd39721e01a40b1a41f5c75785c) --- .../dockerfile/dockerfile_secrets_test.go | 29 +++++++++++++++++++ .../instructions/commands_runmount.go | 3 ++ 2 files changed, 32 insertions(+) diff --git a/frontend/dockerfile/dockerfile_secrets_test.go b/frontend/dockerfile/dockerfile_secrets_test.go index e8c1634373dd..519d7bd91e8f 100644 --- a/frontend/dockerfile/dockerfile_secrets_test.go +++ b/frontend/dockerfile/dockerfile_secrets_test.go @@ -15,6 +15,7 @@ import ( var secretsTests = []integration.Test{ testSecretFileParams, + testSecretRequiredWithoutValue, } func init() { @@ -51,3 +52,31 @@ RUN [ ! -f /mysecret ] # check no stub left behind }, nil) require.NoError(t, err) } + +func testSecretRequiredWithoutValue(t *testing.T, sb integration.Sandbox) { + f := getFrontend(t, sb) + + dockerfile := []byte(` +FROM busybox +RUN --mount=type=secret,required,id=mysecret foo +`) + + dir, err := tmpdir( + fstest.CreateFile("Dockerfile", dockerfile, 0600), + ) + require.NoError(t, err) + defer os.RemoveAll(dir) + + c, err := client.New(sb.Context(), sb.Address()) + require.NoError(t, err) + defer c.Close() + + _, err = f.Solve(sb.Context(), c, client.SolveOpt{ + LocalDirs: map[string]string{ + builder.DefaultLocalNameDockerfile: dir, + builder.DefaultLocalNameContext: dir, + }, + }, nil) + require.Error(t, err) + require.Contains(t, err.Error(), "secret mysecret: not found") +} diff --git a/frontend/dockerfile/instructions/commands_runmount.go b/frontend/dockerfile/instructions/commands_runmount.go index 0431a3d8a231..f32b72489deb 100644 --- a/frontend/dockerfile/instructions/commands_runmount.go +++ b/frontend/dockerfile/instructions/commands_runmount.go @@ -147,6 +147,9 @@ func parseMount(value string, expander SingleWordExpander) (*Mount, error) { key := strings.ToLower(parts[0]) if len(parts) == 1 { + if expander == nil { + continue // evaluate later + } switch key { case "readonly", "ro": m.ReadOnly = true From a995ed6881ced5bc569a5d0d037e70ac44a36bab Mon Sep 17 00:00:00 2001 From: Tonis Tiigi Date: Fri, 20 Aug 2021 10:39:19 -0700 Subject: [PATCH 2/7] control: fix 64bit alignment for buildcount Signed-off-by: Tonis Tiigi (cherry picked from commit 5cfe388b8862100b77289f3049d023692522a019) --- control/control.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/control/control.go b/control/control.go index 3e54060e3b03..9f49fbd13893 100644 --- a/control/control.go +++ b/control/control.go @@ -46,8 +46,7 @@ type Opt struct { } type Controller struct { // TODO: ControlService - *tracev1.UnimplementedTraceServiceServer - + // buildCount needs to be 64bit aligned buildCount int64 opt Opt solver *llbsolver.Solver @@ -55,6 +54,7 @@ type Controller struct { // TODO: ControlService gatewayForwarder *controlgateway.GatewayForwarder throttledGC func() gcmu sync.Mutex + *tracev1.UnimplementedTraceServiceServer } func NewController(opt Opt) (*Controller, error) { From 076a4450f5984fe7e47d2cc0f5629505d34239fe Mon Sep 17 00:00:00 2001 From: Tonis Tiigi Date: Sat, 18 Sep 2021 21:04:16 -0700 Subject: [PATCH 3/7] resolver: use different mutext for handlers and hosts hosts mutex is called on initialization, meaning `GetResolver` might block if it is in the middle of auth exchange. This is currently bad in the case where Job initialization needs to register a name before timeout is reached. Signed-off-by: Tonis Tiigi (cherry picked from commit 6bd2eb50468fa2e96c7be8895f1e72434ff6328d) --- util/resolver/authorizer.go | 19 ++++++++++--------- util/resolver/pool.go | 12 ++++++------ 2 files changed, 16 insertions(+), 15 deletions(-) diff --git a/util/resolver/authorizer.go b/util/resolver/authorizer.go index 3b029b245d69..2766f2157e07 100644 --- a/util/resolver/authorizer.go +++ b/util/resolver/authorizer.go @@ -25,11 +25,12 @@ import ( type authHandlerNS struct { counter int64 // needs to be 64bit aligned for 32bit systems - mu sync.Mutex - handlers map[string]*authHandler - hosts map[string][]docker.RegistryHost - sm *session.Manager - g flightcontrol.Group + handlers map[string]*authHandler + muHandlers sync.Mutex + hosts map[string][]docker.RegistryHost + muHosts sync.Mutex + sm *session.Manager + g flightcontrol.Group } func newAuthHandlerNS(sm *session.Manager) *authHandlerNS { @@ -118,8 +119,8 @@ func newDockerAuthorizer(client *http.Client, handlers *authHandlerNS, sm *sessi // Authorize handles auth request. func (a *dockerAuthorizer) Authorize(ctx context.Context, req *http.Request) error { - a.handlers.mu.Lock() - defer a.handlers.mu.Unlock() + a.handlers.muHandlers.Lock() + defer a.handlers.muHandlers.Unlock() // skip if there is no auth handler ah := a.handlers.get(ctx, req.URL.Host, a.sm, a.session) @@ -141,8 +142,8 @@ func (a *dockerAuthorizer) getCredentials(host string) (sessionID, username, sec } func (a *dockerAuthorizer) AddResponses(ctx context.Context, responses []*http.Response) error { - a.handlers.mu.Lock() - defer a.handlers.mu.Unlock() + a.handlers.muHandlers.Lock() + defer a.handlers.muHandlers.Unlock() last := responses[len(responses)-1] host := last.Request.URL.Host diff --git a/util/resolver/pool.go b/util/resolver/pool.go index 810e9620498b..71347d7bf27b 100644 --- a/util/resolver/pool.go +++ b/util/resolver/pool.go @@ -40,7 +40,7 @@ func (p *Pool) gc() { defer p.mu.Unlock() for k, ns := range p.m { - ns.mu.Lock() + ns.muHandlers.Lock() for key, h := range ns.handlers { if time.Since(h.lastUsed) < 10*time.Minute { continue @@ -58,7 +58,7 @@ func (p *Pool) gc() { if len(ns.handlers) == 0 { delete(p.m, k) } - ns.mu.Unlock() + ns.muHandlers.Unlock() } time.AfterFunc(5*time.Minute, p.gc) @@ -128,9 +128,9 @@ func (r *Resolver) HostsFunc(host string) ([]docker.RegistryHost, error) { return func(domain string) ([]docker.RegistryHost, error) { v, err := r.handler.g.Do(context.TODO(), domain, func(ctx context.Context) (interface{}, error) { // long lock not needed because flightcontrol.Do - r.handler.mu.Lock() + r.handler.muHosts.Lock() v, ok := r.handler.hosts[domain] - r.handler.mu.Unlock() + r.handler.muHosts.Unlock() if ok { return v, nil } @@ -138,9 +138,9 @@ func (r *Resolver) HostsFunc(host string) ([]docker.RegistryHost, error) { if err != nil { return nil, err } - r.handler.mu.Lock() + r.handler.muHosts.Lock() r.handler.hosts[domain] = res - r.handler.mu.Unlock() + r.handler.muHosts.Unlock() return res, nil }) if err != nil || v == nil { From 5fcc3d19e81e7c0cea400b03f0d753c6d7889509 Mon Sep 17 00:00:00 2001 From: Tonis Tiigi Date: Sat, 18 Sep 2021 21:22:32 -0700 Subject: [PATCH 4/7] resolver: make sure authorizer is not overwritten on other resolvers Authorizer stores the current session.Group so if it is overwritten for another resolver it means that session might have been dropped and authentication will fail. Signed-off-by: Tonis Tiigi (cherry picked from commit f62bb55245bc15b6fb79a4cf0cbe6c5792909222) --- util/resolver/pool.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/util/resolver/pool.go b/util/resolver/pool.go index 71347d7bf27b..983a527132f8 100644 --- a/util/resolver/pool.go +++ b/util/resolver/pool.go @@ -146,10 +146,13 @@ func (r *Resolver) HostsFunc(host string) ([]docker.RegistryHost, error) { if err != nil || v == nil { return nil, err } - res := v.([]docker.RegistryHost) - if len(res) == 0 { + vv := v.([]docker.RegistryHost) + if len(vv) == 0 { return nil, nil } + // make a copy so authorizer is set on unique instance + res := make([]docker.RegistryHost, len(vv)) + copy(res, vv) auth := newDockerAuthorizer(res[0].Client, r.handler, r.sm, r.g) for i := range res { res[i].Authorizer = auth From c705e4604177b9190694d718d92ee0db450c1b37 Mon Sep 17 00:00:00 2001 From: Tonis Tiigi Date: Sat, 18 Sep 2021 21:28:45 -0700 Subject: [PATCH 5/7] solver: increase timeout for job registration Signed-off-by: Tonis Tiigi (cherry picked from commit 161b581e71c0ced8a8b8403e33614e6c00d1b696) --- control/control.go | 2 ++ solver/jobs.go | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/control/control.go b/control/control.go index 9f49fbd13893..568f3fed7067 100644 --- a/control/control.go +++ b/control/control.go @@ -239,6 +239,8 @@ func (c *Controller) Solve(ctx context.Context, req *controlapi.SolveRequest) (* atomic.AddInt64(&c.buildCount, 1) defer atomic.AddInt64(&c.buildCount, -1) + // This method registers job ID in solver.Solve. Make sure there are no blocking calls before that might delay this. + if err := translateLegacySolveRequest(req); err != nil { return nil, err } diff --git a/solver/jobs.go b/solver/jobs.go index b5cc3ecedaa5..a55c47114b45 100644 --- a/solver/jobs.go +++ b/solver/jobs.go @@ -455,7 +455,7 @@ func (jl *Solver) NewJob(id string) (*Job, error) { } func (jl *Solver) Get(id string) (*Job, error) { - ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second) + ctx, cancel := context.WithTimeout(context.Background(), 6*time.Second) defer cancel() go func() { From 6316fbc8ce75cc6fb2e5f24c5812d5ae02de00a2 Mon Sep 17 00:00:00 2001 From: Jonathan Giannuzzi Date: Wed, 15 Sep 2021 14:08:26 +0100 Subject: [PATCH 6/7] Fix issues #1980 and #2198 Signed-off-by: Jonathan Giannuzzi (cherry picked from commit 2c540bdc9d3c53c63afa2d3e875a2c2cb3e36bcd) --- cache/remotecache/v1/cachestorage.go | 54 +++++++++++++++------------- 1 file changed, 30 insertions(+), 24 deletions(-) diff --git a/cache/remotecache/v1/cachestorage.go b/cache/remotecache/v1/cachestorage.go index dee02f4b3699..7512b15e796f 100644 --- a/cache/remotecache/v1/cachestorage.go +++ b/cache/remotecache/v1/cachestorage.go @@ -214,35 +214,41 @@ func (cs *cacheResultStorage) Save(res solver.Result, createdAt time.Time) (solv } func (cs *cacheResultStorage) LoadWithParents(ctx context.Context, res solver.CacheResult) (map[string]solver.Result, error) { - v := cs.byResultID(res.ID) - if v == nil || v.result == nil { - return nil, errors.WithStack(solver.ErrNotFound) - } - m := map[string]solver.Result{} visited := make(map[*item]struct{}) - if err := v.walkAllResults(func(i *item) error { - if i.result == nil { - return nil - } - id, ok := cs.byItem[i] - if !ok { - return nil - } - if isSubRemote(*i.result, *v.result) { - ref, err := cs.w.FromRemote(ctx, i.result) - if err != nil { - return err + + ids, ok := cs.byResult[res.ID] + if !ok || len(ids) == 0 { + return nil, errors.WithStack(solver.ErrNotFound) + } + + for id := range ids { + v, ok := cs.byID[id] + if ok && v.result != nil { + if err := v.walkAllResults(func(i *item) error { + if i.result == nil { + return nil + } + id, ok := cs.byItem[i] + if !ok { + return nil + } + if isSubRemote(*i.result, *v.result) { + ref, err := cs.w.FromRemote(ctx, i.result) + if err != nil { + return err + } + m[id] = worker.NewWorkerRefResult(ref, cs.w) + } + return nil + }, visited); err != nil { + for _, v := range m { + v.Release(context.TODO()) + } + return nil, err } - m[id] = worker.NewWorkerRefResult(ref, cs.w) - } - return nil - }, visited); err != nil { - for _, v := range m { - v.Release(context.TODO()) } - return nil, err } return m, nil From e0ce30a13dfd4de18ee2022e65b7e150fc983bc7 Mon Sep 17 00:00:00 2001 From: Erik Sipsma Date: Wed, 21 Jul 2021 23:05:55 +0000 Subject: [PATCH 7/7] solver: include cachemap index in flightcontrol. Signed-off-by: Erik Sipsma (cherry picked from commit 808091a4e1918558c93447d6662fe43e3c84f5b9) --- solver/jobs.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/solver/jobs.go b/solver/jobs.go index a55c47114b45..82b29527ff00 100644 --- a/solver/jobs.go +++ b/solver/jobs.go @@ -715,7 +715,7 @@ func (s *sharedOp) CacheMap(ctx context.Context, index int) (resp *cacheMapResp, if err != nil { return nil, err } - res, err := s.g.Do(ctx, "cachemap", func(ctx context.Context) (ret interface{}, retErr error) { + res, err := s.g.Do(ctx, fmt.Sprintf("cachemap-%d", index), func(ctx context.Context) (ret interface{}, retErr error) { if s.cacheRes != nil && s.cacheDone || index < len(s.cacheRes) { return s.cacheRes, nil }