New flatten buildpacks/builder implementation - Part 2 - Implementing RFC-0123

builder/buildpack_identifier.go

@@ -0,0 +1,5 @@
+package builder
+
+type BpIdentifier interface {
+	Id() string
+}

internal/builder/builder.go

@@ -68,24 +68,23 @@
 
 // Builder represents a pack builder, used to build images
 type Builder struct {
-	baseImageName            string
-	buildConfigEnv           map[string]string
-	image                    imgutil.Image
-	layerWriterFactory       archive.TarWriterFactory
-	lifecycle                Lifecycle
-	lifecycleDescriptor      LifecycleDescriptor
-	additionalBuildpacks     buildpack.ManagedCollection
-	additionalExtensions     buildpack.ManagedCollection
-	flattenExcludeBuildpacks []string
-	metadata                 Metadata
-	mixins                   []string
-	env                      map[string]string
-	uid, gid                 int
-	StackID                  string
-	replaceOrder             bool
-	order                    dist.Order
-	orderExtensions          dist.Order
-	validateMixins           bool
+	baseImageName        string
+	buildConfigEnv       map[string]string
+	image                imgutil.Image
+	layerWriterFactory   archive.TarWriterFactory
+	lifecycle            Lifecycle
+	lifecycleDescriptor  LifecycleDescriptor
+	additionalBuildpacks buildpack.ManagedCollection
+	additionalExtensions buildpack.ManagedCollection
+	metadata             Metadata
+	mixins               []string
+	env                  map[string]string
+	uid, gid             int
+	StackID              string
+	replaceOrder         bool
+	order                dist.Order
+	orderExtensions      dist.Order
+	validateMixins       bool
 }
 
 type orderTOML struct {
@@ -103,8 +102,7 @@
 type BuilderOption func(*options) error
 
 type options struct {
-	flatten bool
-	exclude []string
+	toFlatten buildpack.FlattenModuleInfos
 }
 
 // FromImage constructs a builder from a builder image
@@ -142,17 +140,16 @@
 	}
 
 	bldr := &Builder{
-		baseImageName:            img.Name(),
-		image:                    img,
-		layerWriterFactory:       layerWriterFactory,
-		metadata:                 metadata,
-		lifecycleDescriptor:      constructLifecycleDescriptor(metadata),
-		env:                      map[string]string{},
-		buildConfigEnv:           map[string]string{},
-		validateMixins:           true,
-		additionalBuildpacks:     *buildpack.NewModuleManager(opts.flatten),
-		additionalExtensions:     *buildpack.NewModuleManager(opts.flatten),
-		flattenExcludeBuildpacks: opts.exclude,
+		baseImageName:        img.Name(),
+		image:                img,
+		layerWriterFactory:   layerWriterFactory,
+		metadata:             metadata,
+		lifecycleDescriptor:  constructLifecycleDescriptor(metadata),
+		env:                  map[string]string{},
+		buildConfigEnv:       map[string]string{},
+		validateMixins:       true,
+		additionalBuildpacks: buildpack.NewManagedCollectionV2(opts.toFlatten),
+		additionalExtensions: buildpack.NewManagedCollectionV2(opts.toFlatten),
 	}
 
 	if err := addImgLabelsToBuildr(bldr); err != nil {
@@ -166,17 +163,9 @@
 	return bldr, nil
 }
 
-func FlattenAll() BuilderOption {
+func WithFlattened(modules buildpack.FlattenModuleInfos) BuilderOption {
 	return func(o *options) error {
-		o.flatten = true
-		return nil
-	}
-}
-
-func DoNotFlatten(exclude []string) BuilderOption {
-	return func(o *options) error {
-		o.flatten = true
-		o.exclude = exclude
+		o.toFlatten = modules
 		return nil
 	}
 }
@@ -306,14 +295,14 @@
 	return b.moduleManager(kind).AllModules()
 }
 
-func (b *Builder) moduleManager(kind string) *buildpack.ManagedCollection {
+func (b *Builder) moduleManager(kind string) buildpack.ManagedCollection {
 	switch kind {
 	case buildpack.KindBuildpack:
-		return &b.additionalBuildpacks
+		return b.additionalBuildpacks
 	case buildpack.KindExtension:
-		return &b.additionalExtensions
+		return b.additionalExtensions
 	}
-	return &buildpack.ManagedCollection{}
+	return nil
 }
 
 func (b *Builder) FlattenedModules(kind string) [][]buildpack.BuildModule {
@@ -662,15 +651,14 @@
 	)
 
 	buildModuleWriter := buildpack.NewBuildModuleWriter(logger, b.layerWriterFactory)
-	excludedModules := buildpack.Set(b.flattenExcludeBuildpacks)
 
 	for i, additionalModules := range flattenModules {
 		modFlattenTmpDir := filepath.Join(tmpDir, fmt.Sprintf("%s-%s-flatten", kind, strconv.Itoa(i)))
 		if err := os.MkdirAll(modFlattenTmpDir, os.ModePerm); err != nil {
 			return nil, errors.Wrap(err, "creating flatten temp dir")
 		}
 
-		finalTarPath, buildModuleExcluded, err = buildModuleWriter.NToLayerTar(modFlattenTmpDir, fmt.Sprintf("%s-flatten-%s", kind, strconv.Itoa(i)), additionalModules, excludedModules)
+		finalTarPath, buildModuleExcluded, err = buildModuleWriter.NToLayerTar(modFlattenTmpDir, fmt.Sprintf("%s-flatten-%s", kind, strconv.Itoa(i)), additionalModules, nil)
 		if err != nil {
 			return nil, errors.Wrapf(err, "writing layer %s", finalTarPath)
 		}

internal/builder/builder_test.go

@@ -1852,6 +1852,7 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) {
 		var (
 			bldr         *builder.Builder
 			builderImage imgutil.Image
+			deps         []buildpack.BuildModule
 		)
 
 		it.Before(func() {
@@ -1869,16 +1870,50 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) {
 			))
 
 			builderImage = baseImage
+			deps = []buildpack.BuildModule{bp2v1, bp1v2}
 		})
 
-		when("all", func() {
+		when("buildpacks to be flattened are NOT defined", func() {
 			it.Before(func() {
 				var err error
-				bldr, err = builder.New(builderImage, "some-builder", builder.FlattenAll())
+				bldr, err = builder.New(builderImage, "some-builder")
 				h.AssertNil(t, err)
 
-				// Let's add some buildpacks
-				deps := []buildpack.BuildModule{bp2v1, bp1v2}
+				// Let's add the buildpacks
+				bldr.AddBuildpacks(bp1v1, deps)
+			})
+
+			when("#FlattenedModules", func() {
+				it("it return an empty array", func() {
+					h.AssertEq(t, len(bldr.FlattenedModules(buildpack.KindBuildpack)), 0)
+				})
+			})
+
+			when("#AllModules", func() {
+				it("it returns each buildpack individually", func() {
+					h.AssertEq(t, len(bldr.AllModules(buildpack.KindBuildpack)), 3)
+				})
+			})
+
+			when("#ShouldFlatten", func() {
+				it("it returns false for each buildpack", func() {
+					h.AssertFalse(t, bldr.ShouldFlatten(bp1v1))
+					h.AssertFalse(t, bldr.ShouldFlatten(bp2v1))
+					h.AssertFalse(t, bldr.ShouldFlatten(bp1v2))
+				})
+			})
+		})
+
+		when("buildpacks to be flattened are defined", func() {
+			it.Before(func() {
+				var err error
+				flattenModules, err := buildpack.ParseFlattenBuildModules([]string{"buildpack-1-id@buildpack-1-version-1,buildpack-1-id@buildpack-1-version-2,buildpack-2-id@buildpack-2-version-1"})
+				h.AssertNil(t, err)
+
+				bldr, err = builder.New(builderImage, "some-builder", builder.WithFlattened(flattenModules))
+				h.AssertNil(t, err)
+
+				// Let's add the buildpacks
 				bldr.AddBuildpacks(bp1v1, deps)
 			})
 

internal/commands/builder_create.go

@@ -3,27 +3,26 @@ package commands
 import (
 	"fmt"
 	"path/filepath"
-	"strings"
 
 	"github.com/pkg/errors"
 	"github.com/spf13/cobra"
 
 	"github.com/buildpacks/pack/builder"
 	"github.com/buildpacks/pack/internal/config"
 	"github.com/buildpacks/pack/internal/style"
+	"github.com/buildpacks/pack/pkg/buildpack"
 	"github.com/buildpacks/pack/pkg/client"
 	"github.com/buildpacks/pack/pkg/image"
 	"github.com/buildpacks/pack/pkg/logging"
 )
 
 // BuilderCreateFlags define flags provided to the CreateBuilder command
 type BuilderCreateFlags struct {
-	Flatten         bool
 	Publish         bool
 	BuilderTomlPath string
 	Registry        string
 	Policy          string
-	FlattenExclude  []string
+	Flatten         []string
 }
 
 // CreateBuilder creates a builder image, based on a builder config
@@ -82,6 +81,11 @@ Creating a custom builder allows you to control what buildpacks are used and wha
 				return err
 			}
 
+			toFlatten, err := buildpack.ParseFlattenBuildModules(flags.Flatten)
+			if err != nil {
+				return err
+			}
+
 			imageName := args[0]
 			if err := pack.CreateBuilder(cmd.Context(), client.CreateBuilderOptions{
 				RelativeBaseDir: relativeBaseDir,
@@ -91,8 +95,7 @@ Creating a custom builder allows you to control what buildpacks are used and wha
 				Publish:         flags.Publish,
 				Registry:        flags.Registry,
 				PullPolicy:      pullPolicy,
-				Flatten:         flags.Flatten,
-				FlattenExclude:  flags.FlattenExclude,
+				Flatten:         toFlatten,
 			}); err != nil {
 				return err
 			}
@@ -109,8 +112,7 @@ Creating a custom builder allows you to control what buildpacks are used and wha
 	cmd.Flags().StringVarP(&flags.BuilderTomlPath, "config", "c", "", "Path to builder TOML file (required)")
 	cmd.Flags().BoolVar(&flags.Publish, "publish", false, "Publish the builder directly to the container registry specified in <image-name>, instead of the daemon.")
 	cmd.Flags().StringVar(&flags.Policy, "pull-policy", "", "Pull policy to use. Accepted values are always, never, and if-not-present. The default is always")
-	cmd.Flags().BoolVar(&flags.Flatten, "flatten", false, "Flatten each composite buildpack into a single layer")
-	cmd.Flags().StringSliceVarP(&flags.FlattenExclude, "flatten-exclude", "e", nil, "Buildpacks to exclude from flattening, in the form of '<buildpack-id>@<buildpack-version>'")
+	cmd.Flags().StringSliceVar(&flags.Flatten, "flatten", nil, "List of buildpacks to flatten together into a single layer (format: '<buildpack-id>@<buildpack-version>,<buildpack-id>@<buildpack-version>'")
 
 	AddHelpFlag(cmd, "create")
 	return cmd
@@ -133,13 +135,5 @@ func validateCreateFlags(flags *BuilderCreateFlags, cfg config.Config) error {
 		return errors.Errorf("Please provide a builder config path, using --config.")
 	}
 
-	if flags.Flatten && len(flags.FlattenExclude) > 0 {
-		for _, exclude := range flags.FlattenExclude {
-			if strings.Count(exclude, "@") != 1 {
-				return errors.Errorf("invalid format %s; please use '<buildpack-id>@<buildpack-version>' to exclude buildpack from flattening", exclude)
-			}
-		}
-	}
-
 	return nil
 }

internal/commands/builder_create_test.go

@@ -408,20 +408,19 @@ func testCreateCommand(t *testing.T, when spec.G, it spec.S) {
 			})
 		})
 
-		when("flatten is set to true", func() {
+		when("--flatten", func() {
 			it.Before(func() {
 				h.AssertNil(t, os.WriteFile(builderConfigPath, []byte(validConfig), 0666))
 			})
 
-			when("flatten exclude doesn't have format <buildpack>@<version>", func() {
+			when("requested buildpack doesn't have format <buildpack>@<version>", func() {
 				it("errors with a descriptive message", func() {
 					command.SetArgs([]string{
 						"some/builder",
 						"--config", builderConfigPath,
-						"--flatten",
-						"--flatten-exclude", "some-buildpack",
+						"--flatten", "some-buildpack",
 					})
-					h.AssertError(t, command.Execute(), fmt.Sprintf("invalid format %s; please use '<buildpack-id>@<buildpack-version>' to exclude buildpack from flattening", "some-buildpack"))
+					h.AssertError(t, command.Execute(), fmt.Sprintf("invalid format %s; please use '<buildpack-id>@<buildpack-version>' to add buildpacks to be flattened", "some-buildpack"))
 				})
 			})
 		})

pkg/buildpack/build_module_info.go

@@ -0,0 +1,68 @@
+package buildpack
+
+import (
+	"strings"
+
+	"github.com/pkg/errors"
+
+	"github.com/buildpacks/pack/pkg/dist"
+)
+
+type ModuleInfos interface {
+	BuildModule() []dist.ModuleInfo
+}
+
+type FlattenModuleInfos interface {
+	FlattenModules() []ModuleInfos
+}
+
+type flattenModules struct {
+	modules []ModuleInfos
+}
+
+func (fl *flattenModules) FlattenModules() []ModuleInfos {
+	return fl.modules
+}
+
+type buildModuleInfosImpl struct {
+	modules []dist.ModuleInfo
+}
+
+func (b *buildModuleInfosImpl) BuildModule() []dist.ModuleInfo {
+	return b.modules
+}
+
+func ParseFlattenBuildModules(buildpacksID []string) (FlattenModuleInfos, error) {
+	var buildModuleInfos []ModuleInfos
+	for _, ids := range buildpacksID {
+		modules, err := parseBuildpackName(ids)
+		if err != nil {
+			return nil, err
+		}
+		buildModuleInfos = append(buildModuleInfos, modules)
+	}
+	return &flattenModules{modules: buildModuleInfos}, nil
+}
+
+func parseBuildpackName(names string) (ModuleInfos, error) {
+	var buildModuleInfos []dist.ModuleInfo
+	ids := strings.Split(names, ",")
+	for _, id := range ids {
+		if strings.Count(id, "@") != 1 {
+			return nil, errors.Errorf("invalid format %s; please use '<buildpack-id>@<buildpack-version>' to add buildpacks to be flattened", id)
+		}
+		bpFullName := strings.Split(id, "@")
+		idFromName := strings.TrimSpace(bpFullName[0])
+		versionFromName := strings.TrimSpace(bpFullName[1])
+		if idFromName == "" || versionFromName == "" {
+			return nil, errors.Errorf("invalid format %s; '<buildpack-id>' and '<buildpack-version>' must be specified", id)
+		}
+
+		bpID := dist.ModuleInfo{
+			ID:      idFromName,
+			Version: versionFromName,
+		}
+		buildModuleInfos = append(buildModuleInfos, bpID)
+	}
+	return &buildModuleInfosImpl{modules: buildModuleInfos}, nil
+}