From 3bf816164fd245d024db54a4b4f11d91a69b8ab1 Mon Sep 17 00:00:00 2001 From: Jonny Stoten Date: Tue, 26 Jul 2022 11:40:04 +0100 Subject: [PATCH 1/3] Stop using legacy cache import/export in client This also fixes the bug where registry cache options with a missing ref would be silently discarded by the server-side component that turns the legacy settings into the new settings. Signed-off-by: Jonny Stoten --- client/solve.go | 44 ++++++++------------------------------------ 1 file changed, 8 insertions(+), 36 deletions(-) diff --git a/client/solve.go b/client/solve.go index c0b1b2905..62f518a28 100644 --- a/client/solve.go +++ b/client/solve.go @@ -414,14 +414,10 @@ func parseCacheOptions(ctx context.Context, isGateway bool, opt SolveOpt) (*cach var ( cacheExports []*controlapi.CacheOptionsEntry cacheImports []*controlapi.CacheOptionsEntry - // legacy API is used for registry caches, because the daemon might not support the new API - legacyExportRef string - legacyImportRefs []string ) contentStores := make(map[string]content.Store) indicesToUpdate := make(map[string]string) // key: index.JSON file name, value: tag frontendAttrs := make(map[string]string) - legacyExportAttrs := make(map[string]string) for _, ex := range opt.CacheExports { if ex.Type == "local" { csDir := ex.Attrs["dest"] @@ -440,19 +436,10 @@ func parseCacheOptions(ctx context.Context, isGateway bool, opt SolveOpt) (*cach indexJSONPath := filepath.Join(csDir, "index.json") indicesToUpdate[indexJSONPath] = "latest" } - if ex.Type == "registry" && legacyExportRef == "" { - legacyExportRef = ex.Attrs["ref"] - for k, v := range ex.Attrs { - if k != "ref" { - legacyExportAttrs[k] = v - } - } - } else { - cacheExports = append(cacheExports, &controlapi.CacheOptionsEntry{ - Type: ex.Type, - Attrs: ex.Attrs, - }) - } + cacheExports = append(cacheExports, &controlapi.CacheOptionsEntry{ + Type: ex.Type, + Attrs: ex.Attrs, + }) } for _, im := range opt.CacheImports { attrs := im.Attrs @@ -485,22 +472,12 @@ func parseCacheOptions(ctx context.Context, isGateway bool, opt SolveOpt) (*cach } contentStores["local:"+csDir] = cs } - if im.Type == "registry" { - legacyImportRef := attrs["ref"] - legacyImportRefs = append(legacyImportRefs, legacyImportRef) - } else { - cacheImports = append(cacheImports, &controlapi.CacheOptionsEntry{ - Type: im.Type, - Attrs: attrs, - }) - } + cacheImports = append(cacheImports, &controlapi.CacheOptionsEntry{ + Type: im.Type, + Attrs: attrs, + }) } if opt.Frontend != "" || isGateway { - // use legacy API for registry importers, because the frontend might not support the new API - if len(legacyImportRefs) > 0 { - frontendAttrs["cache-from"] = strings.Join(legacyImportRefs, ",") - } - // use new API for other importers if len(cacheImports) > 0 { s, err := json.Marshal(cacheImports) if err != nil { @@ -511,11 +488,6 @@ func parseCacheOptions(ctx context.Context, isGateway bool, opt SolveOpt) (*cach } res := cacheOptions{ options: controlapi.CacheOptions{ - // old API (for registry caches, planned to be removed in early 2019) - ExportRefDeprecated: legacyExportRef, - ExportAttrsDeprecated: legacyExportAttrs, - ImportRefsDeprecated: legacyImportRefs, - // new API Exports: cacheExports, Imports: cacheImports, }, From c5242ba8fba75baccac5140236c057357163d903 Mon Sep 17 00:00:00 2001 From: Jonny Stoten Date: Tue, 26 Jul 2022 11:45:09 +0100 Subject: [PATCH 2/3] Add simple validation for registry cache ref Signed-off-by: Jonny Stoten --- client/solve.go | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/client/solve.go b/client/solve.go index 62f518a28..c12748776 100644 --- a/client/solve.go +++ b/client/solve.go @@ -436,6 +436,12 @@ func parseCacheOptions(ctx context.Context, isGateway bool, opt SolveOpt) (*cach indexJSONPath := filepath.Join(csDir, "index.json") indicesToUpdate[indexJSONPath] = "latest" } + if ex.Type == "registry" { + regRef := ex.Attrs["ref"] + if regRef == "" { + return nil, errors.New("registry cache exporter requires ref") + } + } cacheExports = append(cacheExports, &controlapi.CacheOptionsEntry{ Type: ex.Type, Attrs: ex.Attrs, @@ -472,6 +478,12 @@ func parseCacheOptions(ctx context.Context, isGateway bool, opt SolveOpt) (*cach } contentStores["local:"+csDir] = cs } + if im.Type == "registry" { + regRef := im.Attrs["ref"] + if regRef == "" { + return nil, errors.New("registry cache importer requires ref") + } + } cacheImports = append(cacheImports, &controlapi.CacheOptionsEntry{ Type: im.Type, Attrs: attrs, From 3b8298b67a4328fdf39d44cfe78119d613a4a7ae Mon Sep 17 00:00:00 2001 From: Jonny Stoten Date: Tue, 26 Jul 2022 11:45:26 +0100 Subject: [PATCH 3/3] Add context to errors setting up caches Signed-off-by: Jonny Stoten --- control/control.go | 2 +- solver/llbsolver/bridge.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/control/control.go b/control/control.go index f0dba1939..5c3bed5a6 100644 --- a/control/control.go +++ b/control/control.go @@ -292,7 +292,7 @@ func (c *Controller) Solve(ctx context.Context, req *controlapi.SolveRequest) (* } cacheExporter, err = cacheExporterFunc(ctx, session.NewGroup(req.Session), e.Attrs) if err != nil { - return nil, err + return nil, errors.Wrapf(err, "failed to configure %v cache exporter", e.Type) } if exportMode, supported := parseCacheExportMode(e.Attrs["mode"]); !supported { bklog.G(ctx).Debugf("skipping invalid cache export mode: %s", e.Attrs["mode"]) diff --git a/solver/llbsolver/bridge.go b/solver/llbsolver/bridge.go index 0f1892feb..2dcbdc7eb 100644 --- a/solver/llbsolver/bridge.go +++ b/solver/llbsolver/bridge.go @@ -91,7 +91,7 @@ func (b *llbBridge) loadResult(ctx context.Context, def *pb.Definition, cacheImp } ci, desc, err := resolveCI(ctx, g, im.Attrs) if err != nil { - return err + return errors.Wrapf(err, "failed to configure %v cache importer", im.Type) } cmNew, err = ci.Resolve(ctx, desc, cmID, w) return err