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 diff --git a/control/control.go b/control/control.go index 3e54060e3b03..568f3fed7067 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) { @@ -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/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 diff --git a/solver/jobs.go b/solver/jobs.go index b5cc3ecedaa5..82b29527ff00 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() { @@ -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 } 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..983a527132f8 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,18 +138,21 @@ 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 { 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