Skip to content

Commit

Permalink
Merge pull request #3271 from jedevc/duplicate-exporter
Browse files Browse the repository at this point in the history 
Don't allow duplicate cache exporters
  • Loading branch information
@tonistiigi
tonistiigi authored Nov 21, 2022
2 parents b20bc6a + 621ab1c commit ae9d0f5
Show file tree
Hide file tree
Showing 4 changed files with 107 additions and 116 deletions.
59 changes: 52 additions & 7 deletions control/control.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,13 @@ package control

import (
"context"
"fmt"
"sync"
"sync/atomic"
"time"

"github.com/docker/distribution/reference"
"github.com/mitchellh/hashstructure/v2"
controlapi "github.com/moby/buildkit/api/services/control"
apitypes "github.com/moby/buildkit/api/types"
"github.com/moby/buildkit/cache/remotecache"
Expand All @@ -21,7 +23,6 @@ import (
"github.com/moby/buildkit/solver"
"github.com/moby/buildkit/solver/llbsolver"
"github.com/moby/buildkit/solver/llbsolver/proc"
solverutil "github.com/moby/buildkit/solver/llbsolver/util"
"github.com/moby/buildkit/solver/pb"
"github.com/moby/buildkit/util/bklog"
"github.com/moby/buildkit/util/imageutil"
Expand Down Expand Up @@ -293,14 +294,17 @@ func (c *Controller) Solve(ctx context.Context, req *controlapi.SolveRequest) (*
}
}

var cacheImports []frontend.CacheOptionsEntry

var cacheExporters []llbsolver.RemoteCacheExporter
exportCacheOptEntries, err := solverutil.DedupCacheOptions(req.Cache.Exports)
if err != nil {
if c, err := findDuplicateCacheOptions(req.Cache.Exports); err != nil {
return nil, err
} else if c != nil {
types := []string{}
for _, c := range c {
types = append(types, c.Type)
}
return nil, errors.Errorf("duplicate cache exports %s", types)
}
for _, e := range exportCacheOptEntries {
var cacheExporters []llbsolver.RemoteCacheExporter
for _, e := range req.Cache.Exports {
cacheExporterFunc, ok := c.opt.ResolveCacheExporterFuncs[e.Type]
if !ok {
return nil, errors.Errorf("unknown cache exporter: %q", e.Type)
Expand All @@ -317,6 +321,8 @@ func (c *Controller) Solve(ctx context.Context, req *controlapi.SolveRequest) (*
}
cacheExporters = append(cacheExporters, exp)
}

var cacheImports []frontend.CacheOptionsEntry
for _, im := range req.Cache.Imports {
cacheImports = append(cacheImports, frontend.CacheOptionsEntry{
Type: im.Type,
Expand Down Expand Up @@ -571,3 +577,42 @@ func toPBBuildkitVersion(in client.BuildkitVersion) *apitypes.BuildkitVersion {
Revision: in.Revision,
}
}

func findDuplicateCacheOptions(cacheOpts []*controlapi.CacheOptionsEntry) ([]*controlapi.CacheOptionsEntry, error) {
seen := map[string]*controlapi.CacheOptionsEntry{}
duplicate := map[string]struct{}{}
for _, opt := range cacheOpts {
k, err := cacheOptKey(*opt)
if err != nil {
return nil, err
}
if _, ok := seen[k]; ok {
duplicate[k] = struct{}{}
}
seen[k] = opt
}

var duplicates []*controlapi.CacheOptionsEntry
for k := range duplicate {
duplicates = append(duplicates, seen[k])
}
return duplicates, nil
}

func cacheOptKey(opt controlapi.CacheOptionsEntry) (string, error) {
if opt.Type == "registry" && opt.Attrs["ref"] != "" {
return opt.Attrs["ref"], nil
}
var rawOpt = struct {
Type string
Attrs map[string]string
}{
Type: opt.Type,
Attrs: opt.Attrs,
}
hash, err := hashstructure.Hash(rawOpt, hashstructure.FormatV2, nil)
if err != nil {
return "", err
}
return fmt.Sprint(opt.Type, ":", hash), nil
}
55 changes: 19 additions & 36 deletions solver/llbsolver/util/util_test.go → control/control_test.go
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package util
package control

import (
"testing"
Expand All @@ -7,21 +7,15 @@ import (
"github.com/stretchr/testify/require"
)

func TestDedupCacheOptions(t *testing.T) {
func TestDuplicateCacheOptions(t *testing.T) {
var testCases = []struct {
name string
opts []*controlapi.CacheOptionsEntry
expected []*controlapi.CacheOptionsEntry
expected []uint
}{
{
name: "deduplicates opts",
name: "avoids unique opts",
opts: []*controlapi.CacheOptionsEntry{
{
Type: "registry",
Attrs: map[string]string{
"ref": "example.com/ref:v1.0.0",
},
},
{
Type: "registry",
Attrs: map[string]string{
Expand All @@ -34,30 +28,18 @@ func TestDedupCacheOptions(t *testing.T) {
"dest": "/path/for/export",
},
},
},
expected: nil,
},
{
name: "finds duplicate opts",
opts: []*controlapi.CacheOptionsEntry{
{
Type: "local",
Attrs: map[string]string{
"dest": "/path/for/export",
},
},
{
Type: "azblob",
Attrs: map[string]string{
"account-url": "url",
"blobs_prefix": "prefix",
"name": "name",
},
},
{
Type: "azblob",
Type: "registry",
Attrs: map[string]string{
"account-url": "url",
"blobs_prefix": "prefix",
"name": "name",
"ref": "example.com/ref:v1.0.0",
},
},
},
expected: []*controlapi.CacheOptionsEntry{
{
Type: "registry",
Attrs: map[string]string{
Expand All @@ -71,22 +53,23 @@ func TestDedupCacheOptions(t *testing.T) {
},
},
{
Type: "azblob",
Type: "local",
Attrs: map[string]string{
"account-url": "url",
"blobs_prefix": "prefix",
"name": "name",
"dest": "/path/for/export",
},
},
},
expected: []uint{0, 2},
},
}

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
opts, err := DedupCacheOptions(tc.opts)
p, err := findDuplicateCacheOptions(tc.opts)
require.NoError(t, err)
require.ElementsMatch(t, opts, tc.expected)
for i, j := range tc.expected {
require.Equal(t, p[i], tc.opts[j])
}
})
}
}
65 changes: 36 additions & 29 deletions solver/llbsolver/solver.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -215,13 +215,24 @@ func (s *Solver) Solve(ctx context.Context, id string, sessionID string, req fro
return nil, err
}

var exporterResponse map[string]string
cacheExporters, inlineCacheExporter := splitCacheExporters(exp.CacheExporters)

var exporterResponse map[string]string
if e := exp.Exporter; e != nil {
exporterResponse, err = exportWithInlineCacheExporter(ctx, e, inlineCacheExporter, j, cached, inp)
meta, err := runInlineCacheExporter(ctx, e, inlineCacheExporter, j, cached)
if err != nil {
return nil, err
}
for k, v := range meta {
inp.AddMeta(k, v)
}

if err := inBuilderContext(ctx, j, e.Name(), j.SessionID+"-export", func(ctx context.Context, _ session.Group) error {
exporterResponse, err = e.Export(ctx, inp, j.SessionID)
return err
}); err != nil {
return nil, err
}
}

cacheExporterResponse, err := runCacheExporters(ctx, cacheExporters, j, cached, inp)
Expand Down Expand Up @@ -304,39 +315,35 @@ func runCacheExporters(ctx context.Context, exporters []RemoteCacheExporter, j *
return cacheExporterResponse, nil
}

func exportWithInlineCacheExporter(ctx context.Context, e exporter.ExporterInstance, inlineExporter *RemoteCacheExporter, j *solver.Job, cached *result.Result[solver.CachedResult], inp *result.Result[cache.ImmutableRef]) (exporterResponse map[string]string, err error) {
if inlineExporter != nil {
if err := inBuilderContext(ctx, j, "preparing layers for inline cache", j.SessionID+"-cache-inline", func(ctx context.Context, _ session.Group) error {
if cached.Ref != nil {
dtic, err := inlineCache(ctx, inlineExporter.Exporter, cached.Ref, e.Config().Compression(), session.NewGroup(j.SessionID))
if err != nil {
return err
}
if dtic != nil {
inp.AddMeta(exptypes.ExporterInlineCache, dtic)
}
func runInlineCacheExporter(ctx context.Context, e exporter.ExporterInstance, inlineExporter *RemoteCacheExporter, j *solver.Job, cached *result.Result[solver.CachedResult]) (map[string][]byte, error) {
meta := map[string][]byte{}
if inlineExporter == nil {
return nil, nil
}
if err := inBuilderContext(ctx, j, "preparing layers for inline cache", j.SessionID+"-cache-inline", func(ctx context.Context, _ session.Group) error {
if res := cached.Ref; res != nil {
dtic, err := inlineCache(ctx, inlineExporter.Exporter, res, e.Config().Compression(), session.NewGroup(j.SessionID))
if err != nil {
return err
}
for k, res := range cached.Refs {
dtic, err := inlineCache(ctx, inlineExporter.Exporter, res, e.Config().Compression(), session.NewGroup(j.SessionID))
if err != nil {
return err
}
if dtic != nil {
inp.AddMeta(fmt.Sprintf("%s/%s", exptypes.ExporterInlineCache, k), dtic)
}
if dtic != nil {
meta[exptypes.ExporterInlineCache] = dtic
}
return nil
}); err != nil {
return nil, err
}
}
if err := inBuilderContext(ctx, j, e.Name(), j.SessionID+"-export", func(ctx context.Context, _ session.Group) error {
exporterResponse, err = e.Export(ctx, inp, j.SessionID)
return err
for k, res := range cached.Refs {
dtic, err := inlineCache(ctx, inlineExporter.Exporter, res, e.Config().Compression(), session.NewGroup(j.SessionID))
if err != nil {
return err
}
if dtic != nil {
meta[fmt.Sprintf("%s/%s", exptypes.ExporterInlineCache, k)] = dtic
}
}
return nil
}); err != nil {
return nil, err
}
return exporterResponse, nil
return meta, nil
}

func splitCacheExporters(exporters []RemoteCacheExporter) (rest []RemoteCacheExporter, inline *RemoteCacheExporter) {
Expand Down
44 changes: 0 additions & 44 deletions solver/llbsolver/util/util.go
View file

This file was deleted.

0 comments on commit ae9d0f5

Please sign in to comment.