Merge pull request #2386 from tonistiigi/v0.9-picks-10012021

[v0.9] cherry-picks

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

control/control.go

@@ -46,15 +46,15 @@ type Opt struct {
 }
 
 type Controller struct { // TODO: ControlService
-	*tracev1.UnimplementedTraceServiceServer
-
+	// buildCount needs to be 64bit aligned
 	buildCount       int64
 	opt              Opt
 	solver           *llbsolver.Solver
 	cache            solver.CacheManager
 	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
 	}

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")
+}

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

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
 		}

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

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,28 +128,31 @@ 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
 			}
 			res, err := r.hosts(domain)
 			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