From fd0af43d775e66305e86dac2d3c4a502a1bf9f34 Mon Sep 17 00:00:00 2001 From: CrazyMax Date: Wed, 16 Feb 2022 20:32:50 +0100 Subject: [PATCH 1/2] buildinfo: export metadata without exporter Signed-off-by: CrazyMax --- client/client_test.go | 39 +++++++++++++++++++++++++++++++++ solver/llbsolver/solver.go | 44 +++++++++++++++++++++++--------------- 2 files changed, 66 insertions(+), 17 deletions(-) diff --git a/client/client_test.go b/client/client_test.go index 13454cd8098d..ad7f92d851ae 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -152,6 +152,7 @@ func TestIntegration(t *testing.T) { testBuildExportWithForeignLayer, testBuildInfoExporter, testBuildInfoInline, + testBuildInfoNoExport, ) tests = append(tests, diffOpTestCases()...) integration.Run(t, tests, mirrors) @@ -5188,6 +5189,44 @@ func testBuildInfoInline(t *testing.T, sb integration.Sandbox) { } } +func testBuildInfoNoExport(t *testing.T, sb integration.Sandbox) { + requiresLinux(t) + c, err := New(sb.Context(), sb.Address()) + require.NoError(t, err) + defer c.Close() + + frontend := func(ctx context.Context, c gateway.Client) (*gateway.Result, error) { + st := llb.Image("busybox:latest").Run( + llb.Args([]string{"/bin/sh", "-c", `echo hello`}), + ) + def, err := st.Marshal(sb.Context()) + if err != nil { + return nil, err + } + return c.Solve(ctx, gateway.SolveRequest{ + Definition: def.ToPB(), + FrontendOpt: map[string]string{"build-arg:foo": "bar"}, + }) + } + + res, err := c.Build(sb.Context(), SolveOpt{}, "", frontend, nil) + require.NoError(t, err) + + require.Contains(t, res.ExporterResponse, exptypes.ExporterBuildInfo) + decbi, err := base64.StdEncoding.DecodeString(res.ExporterResponse[exptypes.ExporterBuildInfo]) + require.NoError(t, err) + + var exbi binfotypes.BuildInfo + err = json.Unmarshal(decbi, &exbi) + require.NoError(t, err) + + attrval := "bar" + require.Equal(t, exbi.Attrs, map[string]*string{"build-arg:foo": &attrval}) + require.Equal(t, len(exbi.Sources), 1) + require.Equal(t, exbi.Sources[0].Type, binfotypes.SourceTypeDockerImage) + require.Equal(t, exbi.Sources[0].Ref, "docker.io/library/busybox:latest") +} + func tmpdir(appliers ...fstest.Applier) (string, error) { tmpdir, err := ioutil.TempDir("", "buildkit-client") if err != nil { diff --git a/solver/llbsolver/solver.go b/solver/llbsolver/solver.go index de9ab8f671cd..1ffcbcbd5c9d 100644 --- a/solver/llbsolver/solver.go +++ b/solver/llbsolver/solver.go @@ -157,6 +157,33 @@ func (s *Solver) Solve(ctx context.Context, id string, sessionID string, req fro return nil, err } + if res.Metadata == nil { + res.Metadata = make(map[string][]byte) + } + if r := res.Ref; r != nil { + dtbi, err := buildinfo.Encode(ctx, res.Metadata[exptypes.ExporterBuildInfo], r.BuildSources()) + if err != nil { + return nil, err + } + if dtbi != nil && len(dtbi) > 0 { + res.Metadata[exptypes.ExporterBuildInfo] = dtbi + } + } + if res.Refs != nil { + for k, r := range res.Refs { + if r == nil { + continue + } + dtbi, err := buildinfo.Encode(ctx, res.Metadata[fmt.Sprintf("%s/%s", exptypes.ExporterBuildInfo, k)], r.BuildSources()) + if err != nil { + return nil, err + } + if dtbi != nil && len(dtbi) > 0 { + res.Metadata[fmt.Sprintf("%s/%s", exptypes.ExporterBuildInfo, k)] = dtbi + } + } + } + var exporterResponse map[string]string if e := exp.Exporter; e != nil { inp := exporter.Source{ @@ -175,14 +202,6 @@ func (s *Solver) Solve(ctx context.Context, id string, sessionID string, req fro return nil, errors.Errorf("invalid reference: %T", r.Sys()) } inp.Ref = workerRef.ImmutableRef - dtbi, err := buildinfo.Encode(ctx, inp.Metadata[exptypes.ExporterBuildInfo], res.BuildSources()) - if err != nil { - return nil, err - } - if dtbi != nil && len(dtbi) > 0 { - inp.Metadata[exptypes.ExporterBuildInfo] = dtbi - } - dtic, err := inlineCache(ctx, exp.CacheExporter, r, e.Config().Compression, session.NewGroup(sessionID)) if err != nil { return nil, err @@ -206,14 +225,6 @@ func (s *Solver) Solve(ctx context.Context, id string, sessionID string, req fro return nil, errors.Errorf("invalid reference: %T", r.Sys()) } m[k] = workerRef.ImmutableRef - dtbi, err := buildinfo.Encode(ctx, inp.Metadata[fmt.Sprintf("%s/%s", exptypes.ExporterBuildInfo, k)], res.BuildSources()) - if err != nil { - return nil, err - } - if dtbi != nil && len(dtbi) > 0 { - inp.Metadata[fmt.Sprintf("%s/%s", exptypes.ExporterBuildInfo, k)] = dtbi - } - dtic, err := inlineCache(ctx, exp.CacheExporter, r, e.Config().Compression, session.NewGroup(sessionID)) if err != nil { return nil, err @@ -225,7 +236,6 @@ func (s *Solver) Solve(ctx context.Context, id string, sessionID string, req fro } inp.Refs = m } - if err := inBuilderContext(ctx, j, e.Name(), "", func(ctx context.Context, _ session.Group) error { exporterResponse, err = e.Export(ctx, inp, j.SessionID) return err From bb746566db733b6283680002cccb1143230def12 Mon Sep 17 00:00:00 2001 From: CrazyMax Date: Wed, 16 Feb 2022 21:22:54 +0100 Subject: [PATCH 2/2] buildinfo: change buildinfo attr export key to bool Signed-off-by: CrazyMax --- README.md | 2 +- docs/build-repro.md | 5 ++-- exporter/containerimage/export.go | 35 +++++++++++---------------- exporter/containerimage/writer.go | 18 +++++++------- exporter/oci/export.go | 33 ++++++++++---------------- util/buildinfo/export.go | 39 ------------------------------- 6 files changed, 37 insertions(+), 95 deletions(-) delete mode 100644 util/buildinfo/export.go diff --git a/README.md b/README.md index f66771606a2f..7cc2315463b9 100644 --- a/README.md +++ b/README.md @@ -237,7 +237,7 @@ Keys supported by image output: * `compression=[uncompressed,gzip,estargz,zstd]`: choose compression type for layers newly created and cached, gzip is default value. estargz should be used with `oci-mediatypes=true`. * `compression-level=[value]`: compression level for gzip, estargz (0-9) and zstd (0-22) * `force-compression=true`: forcefully apply `compression` option to all layers (including already existing layers). -* `buildinfo=[all,imageconfig,metadata,none]`: choose [build dependency](docs/build-repro.md#build-dependencies) version to export (default `all`). +* `buildinfo=true`: inline build info in [image config](docs/build-repro.md#image-config) (default `true`). * `buildinfo-attrs=true`: inline build info attributes in [image config](docs/build-repro.md#image-config) (default `false`). If credentials are required, `buildctl` will attempt to read Docker configuration file `$DOCKER_CONFIG/config.json`. diff --git a/docs/build-repro.md b/docs/build-repro.md index cbbcda4abd96..f8356d75eaa5 100644 --- a/docs/build-repro.md +++ b/docs/build-repro.md @@ -6,9 +6,8 @@ Build dependencies are generated when your image has been built. These dependencies include versions of used images, git repositories and HTTP URLs used by LLB `Source` operation as well as build request attributes. -By default, the build dependencies are embedded in the image configuration and -also available in the solver response. The export mode can be refined with -the [`buildinfo` attribute](../README.md#imageregistry). +By default, the build dependencies are inlined in the image configuration. You +can disable this behavior with the [`buildinfo` attribute](../README.md#imageregistry). ### Image config diff --git a/exporter/containerimage/export.go b/exporter/containerimage/export.go index 7cab3ed9aa82..429a3ce6df3b 100644 --- a/exporter/containerimage/export.go +++ b/exporter/containerimage/export.go @@ -22,7 +22,6 @@ import ( "github.com/moby/buildkit/exporter/containerimage/exptypes" "github.com/moby/buildkit/session" "github.com/moby/buildkit/snapshot" - "github.com/moby/buildkit/util/buildinfo" "github.com/moby/buildkit/util/compression" "github.com/moby/buildkit/util/contentutil" "github.com/moby/buildkit/util/leaseutil" @@ -79,7 +78,7 @@ func (e *imageExporter) Resolve(ctx context.Context, opt map[string]string) (exp i := &imageExporterInstance{ imageExporter: e, layerCompression: compression.Default, - buildInfoMode: buildinfo.ExportDefault, + buildInfo: true, } var esgz bool @@ -182,19 +181,14 @@ func (e *imageExporter) Resolve(ctx context.Context, opt map[string]string) (exp i.compressionLevel = &v case keyBuildInfo: if v == "" { + i.buildInfo = true continue } - bimode, err := buildinfo.ParseExportMode(v) - if err != nil { - return nil, err - } - i.buildInfoMode = bimode - case preferNondistLayersKey: b, err := strconv.ParseBool(v) if err != nil { - return nil, errors.Wrapf(err, "non-bool value %s specified for %s", v, k) + return nil, errors.Wrapf(err, "non-bool value specified for %s", k) } - i.preferNondistLayers = b + i.buildInfo = b case keyBuildInfoAttrs: if v == "" { i.buildInfoAttrs = false @@ -205,6 +199,12 @@ func (e *imageExporter) Resolve(ctx context.Context, opt map[string]string) (exp return nil, errors.Wrapf(err, "non-bool value specified for %s", k) } i.buildInfoAttrs = b + case preferNondistLayersKey: + b, err := strconv.ParseBool(v) + if err != nil { + return nil, errors.Wrapf(err, "non-bool value %s specified for %s", v, k) + } + i.preferNondistLayers = b default: if i.meta == nil { i.meta = make(map[string][]byte) @@ -232,10 +232,10 @@ type imageExporterInstance struct { layerCompression compression.Type forceCompression bool compressionLevel *int - buildInfoMode buildinfo.ExportMode + buildInfo bool + buildInfoAttrs bool meta map[string][]byte preferNondistLayers bool - buildInfoAttrs bool } func (e *imageExporterInstance) Name() string { @@ -271,7 +271,7 @@ func (e *imageExporterInstance) Export(ctx context.Context, src exporter.Source, defer done(context.TODO()) refCfg := e.refCfg() - desc, err := e.opt.ImageWriter.Commit(ctx, src, e.ociTypes, refCfg, e.buildInfoMode, e.buildInfoAttrs, sessionID) + desc, err := e.opt.ImageWriter.Commit(ctx, src, e.ociTypes, refCfg, e.buildInfo, e.buildInfoAttrs, sessionID) if err != nil { return nil, err } @@ -280,15 +280,6 @@ func (e *imageExporterInstance) Export(ctx context.Context, src exporter.Source, e.opt.ImageWriter.ContentStore().Delete(context.TODO(), desc.Digest) }() - if e.buildInfoMode&buildinfo.ExportMetadata == 0 { - for k := range src.Metadata { - if !strings.HasPrefix(k, exptypes.ExporterBuildInfo) { - continue - } - delete(src.Metadata, k) - } - } - resp := make(map[string]string) if n, ok := src.Metadata["image.name"]; e.targetName == "*" && ok { diff --git a/exporter/containerimage/writer.go b/exporter/containerimage/writer.go index 88b9cd5768a0..a68d3abccd1c 100644 --- a/exporter/containerimage/writer.go +++ b/exporter/containerimage/writer.go @@ -50,7 +50,7 @@ type ImageWriter struct { opt WriterOpt } -func (ic *ImageWriter) Commit(ctx context.Context, inp exporter.Source, oci bool, refCfg cacheconfig.RefConfig, buildInfoMode buildinfo.ExportMode, buildInfoAttrs bool, sessionID string) (*ocispecs.Descriptor, error) { +func (ic *ImageWriter) Commit(ctx context.Context, inp exporter.Source, oci bool, refCfg cacheconfig.RefConfig, buildInfo bool, buildInfoAttrs bool, sessionID string) (*ocispecs.Descriptor, error) { platformsBytes, ok := inp.Metadata[exptypes.ExporterPlatformsKey] if len(inp.Refs) > 0 && !ok { @@ -63,16 +63,16 @@ func (ic *ImageWriter) Commit(ctx context.Context, inp exporter.Source, oci bool return nil, err } - var buildInfo []byte - if buildInfoMode&buildinfo.ExportImageConfig > 0 { - if buildInfo, err = buildinfo.Format(inp.Metadata[exptypes.ExporterBuildInfo], buildinfo.FormatOpts{ + var dtbi []byte + if buildInfo { + if dtbi, err = buildinfo.Format(inp.Metadata[exptypes.ExporterBuildInfo], buildinfo.FormatOpts{ RemoveAttrs: !buildInfoAttrs, }); err != nil { return nil, err } } - mfstDesc, configDesc, err := ic.commitDistributionManifest(ctx, inp.Ref, inp.Metadata[exptypes.ExporterImageConfigKey], &remotes[0], oci, inp.Metadata[exptypes.ExporterInlineCache], buildInfo) + mfstDesc, configDesc, err := ic.commitDistributionManifest(ctx, inp.Ref, inp.Metadata[exptypes.ExporterImageConfigKey], &remotes[0], oci, inp.Metadata[exptypes.ExporterInlineCache], dtbi) if err != nil { return nil, err } @@ -134,16 +134,16 @@ func (ic *ImageWriter) Commit(ctx context.Context, inp exporter.Source, oci bool config := inp.Metadata[fmt.Sprintf("%s/%s", exptypes.ExporterImageConfigKey, p.ID)] inlineCache := inp.Metadata[fmt.Sprintf("%s/%s", exptypes.ExporterInlineCache, p.ID)] - var buildInfo []byte - if buildInfoMode&buildinfo.ExportImageConfig > 0 { - if buildInfo, err = buildinfo.Format(inp.Metadata[fmt.Sprintf("%s/%s", exptypes.ExporterBuildInfo, p.ID)], buildinfo.FormatOpts{ + var dtbi []byte + if buildInfo { + if dtbi, err = buildinfo.Format(inp.Metadata[fmt.Sprintf("%s/%s", exptypes.ExporterBuildInfo, p.ID)], buildinfo.FormatOpts{ RemoveAttrs: !buildInfoAttrs, }); err != nil { return nil, err } } - desc, _, err := ic.commitDistributionManifest(ctx, r, config, &remotes[remotesMap[p.ID]], oci, inlineCache, buildInfo) + desc, _, err := ic.commitDistributionManifest(ctx, r, config, &remotes[remotesMap[p.ID]], oci, inlineCache, dtbi) if err != nil { return nil, err } diff --git a/exporter/oci/export.go b/exporter/oci/export.go index cc373a0f08a2..153211c9b709 100644 --- a/exporter/oci/export.go +++ b/exporter/oci/export.go @@ -18,7 +18,6 @@ import ( "github.com/moby/buildkit/exporter/containerimage/exptypes" "github.com/moby/buildkit/session" "github.com/moby/buildkit/session/filesync" - "github.com/moby/buildkit/util/buildinfo" "github.com/moby/buildkit/util/compression" "github.com/moby/buildkit/util/contentutil" "github.com/moby/buildkit/util/grpcerrors" @@ -69,7 +68,7 @@ func (e *imageExporter) Resolve(ctx context.Context, opt map[string]string) (exp i := &imageExporterInstance{ imageExporter: e, layerCompression: compression.Default, - buildInfoMode: buildinfo.ExportDefault, + buildInfo: true, } var esgz bool for k, v := range opt { @@ -120,19 +119,14 @@ func (e *imageExporter) Resolve(ctx context.Context, opt map[string]string) (exp *ot = b case keyBuildInfo: if v == "" { + i.buildInfo = true continue } - bimode, err := buildinfo.ParseExportMode(v) - if err != nil { - return nil, err - } - i.buildInfoMode = bimode - case preferNondistLayersKey: b, err := strconv.ParseBool(v) if err != nil { return nil, errors.Wrapf(err, "non-bool value specified for %s", k) } - i.preferNonDist = b + i.buildInfo = b case keyBuildInfoAttrs: if v == "" { i.buildInfoAttrs = false @@ -143,6 +137,12 @@ func (e *imageExporter) Resolve(ctx context.Context, opt map[string]string) (exp return nil, errors.Wrapf(err, "non-bool value specified for %s", k) } i.buildInfoAttrs = b + case preferNondistLayersKey: + b, err := strconv.ParseBool(v) + if err != nil { + return nil, errors.Wrapf(err, "non-bool value specified for %s", k) + } + i.preferNonDist = b default: if i.meta == nil { i.meta = make(map[string][]byte) @@ -170,9 +170,9 @@ type imageExporterInstance struct { layerCompression compression.Type forceCompression bool compressionLevel *int - buildInfoMode buildinfo.ExportMode - preferNonDist bool + buildInfo bool buildInfoAttrs bool + preferNonDist bool } func (e *imageExporterInstance) Name() string { @@ -218,7 +218,7 @@ func (e *imageExporterInstance) Export(ctx context.Context, src exporter.Source, } defer done(context.TODO()) - desc, err := e.opt.ImageWriter.Commit(ctx, src, e.ociTypes, e.refCfg(), e.buildInfoMode, e.buildInfoAttrs, sessionID) + desc, err := e.opt.ImageWriter.Commit(ctx, src, e.ociTypes, e.refCfg(), e.buildInfo, e.buildInfoAttrs, sessionID) if err != nil { return nil, err } @@ -226,15 +226,6 @@ func (e *imageExporterInstance) Export(ctx context.Context, src exporter.Source, e.opt.ImageWriter.ContentStore().Delete(context.TODO(), desc.Digest) }() - if e.buildInfoMode&buildinfo.ExportMetadata == 0 { - for k := range src.Metadata { - if !strings.HasPrefix(k, exptypes.ExporterBuildInfo) { - continue - } - delete(src.Metadata, k) - } - } - if desc.Annotations == nil { desc.Annotations = map[string]string{} } diff --git a/util/buildinfo/export.go b/util/buildinfo/export.go deleted file mode 100644 index 485ccf7d82f0..000000000000 --- a/util/buildinfo/export.go +++ /dev/null @@ -1,39 +0,0 @@ -package buildinfo - -import "github.com/pkg/errors" - -// ExportMode represents the export mode for buildinfo opt. -type ExportMode int - -const ( - // ExportNone doesn't export build dependencies. - ExportNone ExportMode = 0 - // ExportImageConfig exports build dependencies to - // the image config. - ExportImageConfig = 1 << iota - // ExportMetadata exports build dependencies as metadata to - // the exporter response. - ExportMetadata - // ExportAll exports build dependencies as metadata and - // image config. - ExportAll = -1 -) - -// ExportDefault is the default export mode for buildinfo opt. -const ExportDefault = ExportAll - -// ParseExportMode returns the export mode matching a string. -func ParseExportMode(s string) (ExportMode, error) { - switch s { - case "none": - return ExportNone, nil - case "imageconfig": - return ExportImageConfig, nil - case "metadata": - return ExportMetadata, nil - case "all": - return ExportAll, nil - default: - return 0, errors.Errorf("unknown buildinfo export mode: %s", s) - } -}