From f46c54147cec203d8c20fc1708fcab015b85845a Mon Sep 17 00:00:00 2001 From: Parthiba-Hazra Date: Fri, 12 Jan 2024 00:06:01 +0530 Subject: [PATCH 1/9] add an option to retag rather than replacing the target image while rebasing Signed-off-by: Parthiba-Hazra add an option to "retag" rather than replacing the target image while rebasing Signed-off-by: Parthiba-Hazra --- internal/commands/rebase.go | 1 + internal/commands/rebase_test.go | 35 ++++++++++++++++++++++++++++++++ pkg/client/rebase.go | 19 ++++++++++++++++- 3 files changed, 54 insertions(+), 1 deletion(-) diff --git a/internal/commands/rebase.go b/internal/commands/rebase.go index 0eed5f7281..79afada61a 100644 --- a/internal/commands/rebase.go +++ b/internal/commands/rebase.go @@ -49,6 +49,7 @@ func Rebase(logger logging.Logger, cfg config.Config, pack PackClient) *cobra.Co cmd.Flags().BoolVar(&opts.Publish, "publish", false, "Publish the rebased application image directly to the container registry specified in , instead of the daemon. The previous application image must also reside in the registry.") cmd.Flags().StringVar(&opts.RunImage, "run-image", "", "Run image to use for rebasing") cmd.Flags().StringVar(&policy, "pull-policy", "", "Pull policy to use. Accepted values are always, never, and if-not-present. The default is always") + cmd.Flags().StringSliceVarP(&opts.Tags, "tag", "t", nil, "Comma-separated list of tag to apply to the rebased image") cmd.Flags().StringVar(&opts.ReportDestinationDir, "report-output-dir", "", "Path to export build report.toml.\nOmitting the flag yield no report file.") cmd.Flags().BoolVar(&opts.Force, "force", false, "Perform rebase operation without target validation (only available for API >= 0.12)") diff --git a/internal/commands/rebase_test.go b/internal/commands/rebase_test.go index 8f235c409b..c4833d87b4 100644 --- a/internal/commands/rebase_test.go +++ b/internal/commands/rebase_test.go @@ -158,6 +158,41 @@ func testRebaseCommand(t *testing.T, when spec.G, it spec.S) { }) }) }) + when("image name and tags are provided", func() { + it.Before(func() { + runImage := "test/image" + testMirror1 := "example.com/some/run1" + testMirror2 := "example.com/some/run2" + + cfg.RunImages = []config.RunImage{{ + Image: runImage, + Mirrors: []string{testMirror1, testMirror2}, + }} + command = commands.Rebase(logger, cfg, mockClient) + + repoName = "test/repo-image" + tags := []string{"tag1"} + opts = client.RebaseOptions{ + RepoName: repoName, + Publish: false, + PullPolicy: image.PullAlways, + RunImage: "", + AdditionalMirrors: map[string][]string{ + runImage: {testMirror1, testMirror2}, + }, + Tags: tags, + } + }) + + it("works", func() { + mockClient.EXPECT(). + Rebase(gomock.Any(), gomock.Eq(opts)). + Return(nil) + + command.SetArgs([]string{repoName, "--tag", "tag1"}) + h.AssertNil(t, command.Execute()) + }) + }) }) }) } diff --git a/pkg/client/rebase.go b/pkg/client/rebase.go index 8f3f8cf5bd..36072f9a01 100644 --- a/pkg/client/rebase.go +++ b/pkg/client/rebase.go @@ -5,6 +5,7 @@ import ( "fmt" "os" "path/filepath" + "strings" "github.com/BurntSushi/toml" "github.com/buildpacks/lifecycle/phase" @@ -46,8 +47,13 @@ type RebaseOptions struct { // Pass-through force flag to lifecycle rebase command to skip target data // validated (will not have any effect if API < 0.12). Force bool + + // Tags to be applied to the rebased image. + Tags []string } +const tagDelim = ":" + // Rebase updates the run image layers in an app image. // This operation mutates the image specified in opts. func (c *Client) Rebase(ctx context.Context, opts RebaseOptions) error { @@ -56,7 +62,18 @@ func (c *Client) Rebase(ctx context.Context, opts RebaseOptions) error { return errors.Wrapf(err, "invalid image name '%s'", opts.RepoName) } - appImage, err := c.imageFetcher.Fetch(ctx, opts.RepoName, image.FetchOptions{Daemon: !opts.Publish, PullPolicy: opts.PullPolicy}) + appImageName := opts.RepoName + + if len(opts.Tags) > 0 { + parts := strings.SplitN(appImageName, tagDelim, 2) + if len(parts) == 2 { + appImageName = fmt.Sprintf("%s:%s", parts[0], opts.Tags[0]) + } else { + appImageName = fmt.Sprintf("%s:%s", appImageName, opts.Tags[0]) + } + } + + appImage, err := c.imageFetcher.Fetch(ctx, appImageName, image.FetchOptions{Daemon: !opts.Publish, PullPolicy: opts.PullPolicy}) if err != nil { return err } From da19388a0b01936b246e19ce524e70de2ffddc0e Mon Sep 17 00:00:00 2001 From: Rizul Gupta Date: Sat, 13 Jan 2024 23:38:11 +0530 Subject: [PATCH 2/9] change help text of --publish Signed-off-by: Rizul Gupta --- internal/commands/build.go | 2 +- internal/commands/builder_create.go | 2 +- internal/commands/buildpack_package.go | 2 +- internal/commands/create_builder.go | 2 +- internal/commands/extension_package.go | 2 +- internal/commands/package_buildpack.go | 2 +- internal/commands/rebase.go | 2 +- 7 files changed, 7 insertions(+), 7 deletions(-) diff --git a/internal/commands/build.go b/internal/commands/build.go index 9b8845d65d..26f48f9476 100644 --- a/internal/commands/build.go +++ b/internal/commands/build.go @@ -249,7 +249,7 @@ func buildCommandFlags(cmd *cobra.Command, buildFlags *BuildFlags, cfg config.Co cmd.Flags().StringVar(&buildFlags.Network, "network", "", "Connect detect and build containers to network") cmd.Flags().StringArrayVar(&buildFlags.PreBuildpacks, "pre-buildpack", []string{}, "Buildpacks to prepend to the groups in the builder's order") cmd.Flags().StringArrayVar(&buildFlags.PostBuildpacks, "post-buildpack", []string{}, "Buildpacks to append to the groups in the builder's order") - cmd.Flags().BoolVar(&buildFlags.Publish, "publish", false, "Publish the application image directly to the container registry specified in , instead of the daemon. The run image must also reside in the registry.") + cmd.Flags().BoolVar(&buildFlags.Publish, "publish", false, "This flag triggers the buildpack to publish the built image to a container registry after the build process is complete. This means that the resulting image is pushed to a specified registry, making it available for deployment and use by others.") cmd.Flags().StringVar(&buildFlags.DockerHost, "docker-host", "", `Address to docker daemon that will be exposed to the build container. If not set (or set to empty string) the standard socket location will be used. diff --git a/internal/commands/builder_create.go b/internal/commands/builder_create.go index 0a591bfed1..f5e82df884 100644 --- a/internal/commands/builder_create.go +++ b/internal/commands/builder_create.go @@ -112,7 +112,7 @@ Creating a custom builder allows you to control what buildpacks are used and wha cmd.Flags().MarkHidden("buildpack-registry") } 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 , instead of the daemon.") + cmd.Flags().BoolVar(&flags.Publish, "publish", false, "This flag triggers the buildpack to publish the built image to a container registry after the build process is complete. This means that the resulting image is pushed to a specified registry, making it available for deployment and use by others.") 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().StringSliceVar(&flags.Flatten, "flatten", nil, "List of buildpacks to flatten together into a single layer (format: '@,@'") cmd.Flags().StringToStringVarP(&flags.Label, "label", "l", nil, "Labels to add to the builder image, in the form of '='") diff --git a/internal/commands/buildpack_package.go b/internal/commands/buildpack_package.go index adb95c3e03..d69a9d6e40 100644 --- a/internal/commands/buildpack_package.go +++ b/internal/commands/buildpack_package.go @@ -131,7 +131,7 @@ func BuildpackPackage(logger logging.Logger, cfg config.Config, packager Buildpa cmd.Flags().StringVarP(&flags.PackageTomlPath, "config", "c", "", "Path to package TOML config") cmd.Flags().StringVarP(&flags.Format, "format", "f", "", `Format to save package as ("image" or "file")`) - cmd.Flags().BoolVar(&flags.Publish, "publish", false, `Publish the buildpack directly to the container registry specified in , instead of the daemon (applies to "--format=image" only).`) + cmd.Flags().BoolVar(&flags.Publish, "publish", false, `This flag triggers the buildpack to publish the built image to a container registry after the build process is complete. This means that the resulting image is pushed to a specified registry, making it available for deployment and use by others. (applies to "--format=image" only)`) 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().StringVarP(&flags.Path, "path", "p", "", "Path to the Buildpack that needs to be packaged") cmd.Flags().StringVarP(&flags.BuildpackRegistry, "buildpack-registry", "r", "", "Buildpack Registry name") diff --git a/internal/commands/create_builder.go b/internal/commands/create_builder.go index 9999392ef6..ff7128e3e2 100644 --- a/internal/commands/create_builder.go +++ b/internal/commands/create_builder.go @@ -89,7 +89,7 @@ Creating a custom builder allows you to control what buildpacks are used and wha cmd.Flags().MarkHidden("buildpack-registry") } 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 , instead of the daemon.") + cmd.Flags().BoolVar(&flags.Publish, "publish", false, "This flag triggers the buildpack to publish the built image to a container registry after the build process is complete. This means that the resulting image is pushed to a specified registry, making it available for deployment and use by others.") cmd.Flags().StringVar(&flags.Policy, "pull-policy", "", "Pull policy to use. Accepted values are always, never, and if-not-present. The default is always") return cmd } diff --git a/internal/commands/extension_package.go b/internal/commands/extension_package.go index 0415823e57..85ad7569df 100644 --- a/internal/commands/extension_package.go +++ b/internal/commands/extension_package.go @@ -102,7 +102,7 @@ func ExtensionPackage(logger logging.Logger, cfg config.Config, packager Extensi // flags will be added here cmd.Flags().StringVarP(&flags.PackageTomlPath, "config", "c", "", "Path to package TOML config") cmd.Flags().StringVarP(&flags.Format, "format", "f", "", `Format to save package as ("image" or "file")`) - cmd.Flags().BoolVar(&flags.Publish, "publish", false, `Publish the extension directly to the container registry specified in , instead of the daemon (applies to "--format=image" only).`) + cmd.Flags().BoolVar(&flags.Publish, "publish", false, `This flag triggers the buildpack to publish the built image to a container registry after the build process is complete. This means that the resulting image is pushed to a specified registry, making it available for deployment and use by others. (applies to "--format=image" only)`) cmd.Flags().StringVar(&flags.Policy, "pull-policy", "", "Pull policy to use. Accepted values are always, never, and if-not-present. The default is always") AddHelpFlag(cmd, "package") return cmd diff --git a/internal/commands/package_buildpack.go b/internal/commands/package_buildpack.go index 1bd17c4561..6b1e339e9c 100644 --- a/internal/commands/package_buildpack.go +++ b/internal/commands/package_buildpack.go @@ -85,7 +85,7 @@ func PackageBuildpack(logger logging.Logger, cfg config.Config, packager Buildpa cmd.Flags().StringVarP(&flags.PackageTomlPath, "config", "c", "", "Path to package TOML config (required)") cmd.Flags().StringVarP(&flags.Format, "format", "f", "", `Format to save package as ("image" or "file")`) - cmd.Flags().BoolVar(&flags.Publish, "publish", false, `Publish the buildpack directly to the container registry specified in , instead of the daemon (applies to "--format=image" only).`) + cmd.Flags().BoolVar(&flags.Publish, "publish", false, `This flag triggers the buildpack to publish the built image to a container registry after the build process is complete. This means that the resulting image is pushed to a specified registry, making it available for deployment and use by others. (applies to "--format=image" only)`) 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().StringVarP(&flags.BuildpackRegistry, "buildpack-registry", "r", "", "Buildpack Registry name") diff --git a/internal/commands/rebase.go b/internal/commands/rebase.go index 79afada61a..bf0e918825 100644 --- a/internal/commands/rebase.go +++ b/internal/commands/rebase.go @@ -46,7 +46,7 @@ func Rebase(logger logging.Logger, cfg config.Config, pack PackClient) *cobra.Co }), } - cmd.Flags().BoolVar(&opts.Publish, "publish", false, "Publish the rebased application image directly to the container registry specified in , instead of the daemon. The previous application image must also reside in the registry.") + cmd.Flags().BoolVar(&opts.Publish, "publish", false, "This flag triggers the buildpack to publish the built image to a container registry after the build process is complete. This means that the resulting image is pushed to a specified registry, making it available for deployment and use by others.") cmd.Flags().StringVar(&opts.RunImage, "run-image", "", "Run image to use for rebasing") cmd.Flags().StringVar(&policy, "pull-policy", "", "Pull policy to use. Accepted values are always, never, and if-not-present. The default is always") cmd.Flags().StringSliceVarP(&opts.Tags, "tag", "t", nil, "Comma-separated list of tag to apply to the rebased image") From ad4874858b3b4777f063dc57adbf95ddb870e327 Mon Sep 17 00:00:00 2001 From: Rizul Gupta <112455393+rizul2108@users.noreply.github.com> Date: Wed, 17 Jan 2024 23:01:42 +0530 Subject: [PATCH 3/9] Update internal/commands/rebase.go Co-authored-by: Natalie Arellano Signed-off-by: Rizul Gupta <112455393+rizul2108@users.noreply.github.com> --- internal/commands/rebase.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/commands/rebase.go b/internal/commands/rebase.go index bf0e918825..79afada61a 100644 --- a/internal/commands/rebase.go +++ b/internal/commands/rebase.go @@ -46,7 +46,7 @@ func Rebase(logger logging.Logger, cfg config.Config, pack PackClient) *cobra.Co }), } - cmd.Flags().BoolVar(&opts.Publish, "publish", false, "This flag triggers the buildpack to publish the built image to a container registry after the build process is complete. This means that the resulting image is pushed to a specified registry, making it available for deployment and use by others.") + cmd.Flags().BoolVar(&opts.Publish, "publish", false, "Publish the rebased application image directly to the container registry specified in , instead of the daemon. The previous application image must also reside in the registry.") cmd.Flags().StringVar(&opts.RunImage, "run-image", "", "Run image to use for rebasing") cmd.Flags().StringVar(&policy, "pull-policy", "", "Pull policy to use. Accepted values are always, never, and if-not-present. The default is always") cmd.Flags().StringSliceVarP(&opts.Tags, "tag", "t", nil, "Comma-separated list of tag to apply to the rebased image") From c571998ee969c84cea0c925dafcf107e9ee20354 Mon Sep 17 00:00:00 2001 From: Juan Bustamante Date: Tue, 3 Oct 2023 10:59:48 -0500 Subject: [PATCH 4/9] Implementing RFC-0123 During builder creation, end-users can provide the flag `--flatten` with the buildpacks they want to put in one layer. Signed-off-by: Juan Bustamante --- builder/buildpack_identifier.go | 2 +- internal/builder/builder.go | 20 ++-- internal/builder/builder_test.go | 5 +- internal/commands/builder_create.go | 9 +- pkg/buildpack/build_module_info.go | 11 +-- pkg/buildpack/managed_collection.go | 112 ++++++++++++++++++++--- pkg/buildpack/managed_collection_test.go | 12 +++ pkg/buildpack/package.go | 2 +- pkg/client/create_builder.go | 4 +- pkg/client/create_builder_test.go | 5 +- 10 files changed, 133 insertions(+), 49 deletions(-) diff --git a/builder/buildpack_identifier.go b/builder/buildpack_identifier.go index b32e111dc7..ec69a6506e 100644 --- a/builder/buildpack_identifier.go +++ b/builder/buildpack_identifier.go @@ -1,5 +1,5 @@ package builder -type BpIdentifier interface { +type Identifier interface { Id() string } diff --git a/internal/builder/builder.go b/internal/builder/builder.go index 88330845f2..6b347147cd 100644 --- a/internal/builder/builder.go +++ b/internal/builder/builder.go @@ -102,8 +102,7 @@ type moduleWithDiffID struct { type BuilderOption func(*options) error type options struct { - toFlatten buildpack.FlattenModuleInfos - labels map[string]string + modules buildpack.FlattenModuleInfos } // FromImage constructs a builder from a builder image @@ -156,8 +155,8 @@ func constructBuilder(img imgutil.Image, newName string, errOnMissingLabel bool, env: map[string]string{}, buildConfigEnv: map[string]string{}, validateMixins: true, - additionalBuildpacks: buildpack.NewManagedCollectionV2(opts.toFlatten), - additionalExtensions: buildpack.NewManagedCollectionV2(opts.toFlatten), + additionalBuildpacks: buildpack.NewModuleManagerV2(opts.modules), + additionalExtensions: buildpack.NewModuleManagerV2(opts.modules), } if err := addImgLabelsToBuildr(bldr); err != nil { @@ -171,16 +170,9 @@ func constructBuilder(img imgutil.Image, newName string, errOnMissingLabel bool, return bldr, nil } -func WithFlattened(modules buildpack.FlattenModuleInfos) BuilderOption { +func WithFlatten(modules buildpack.FlattenModuleInfos) BuilderOption { return func(o *options) error { - o.toFlatten = modules - return nil - } -} - -func WithLabels(labels map[string]string) BuilderOption { - return func(o *options) error { - o.labels = labels + o.modules = modules return nil } } @@ -317,7 +309,7 @@ func (b *Builder) moduleManager(kind string) buildpack.ManagedCollection { case buildpack.KindExtension: return b.additionalExtensions } - return nil + return buildpack.NewModuleManager(false) } func (b *Builder) FlattenedModules(kind string) [][]buildpack.BuildModule { diff --git a/internal/builder/builder_test.go b/internal/builder/builder_test.go index 80d39d03ff..cc61f60f31 100644 --- a/internal/builder/builder_test.go +++ b/internal/builder/builder_test.go @@ -1876,7 +1876,10 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) { when("buildpacks to be flattened are NOT defined", func() { it.Before(func() { var err error - bldr, err = builder.New(builderImage, "some-builder") + 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.WithFlatten(flattenModules)) h.AssertNil(t, err) // Let's add the buildpacks diff --git a/internal/commands/builder_create.go b/internal/commands/builder_create.go index f5e82df884..d67bf8df5d 100644 --- a/internal/commands/builder_create.go +++ b/internal/commands/builder_create.go @@ -23,7 +23,6 @@ type BuilderCreateFlags struct { Registry string Policy string Flatten []string - Label map[string]string } // CreateBuilder creates a builder image, based on a builder config @@ -82,7 +81,7 @@ Creating a custom builder allows you to control what buildpacks are used and wha return err } - toFlatten, err := buildpack.ParseFlattenBuildModules(flags.Flatten) + flattenBuildpacks, err := buildpack.ParseFlattenBuildModules(flags.Flatten) if err != nil { return err } @@ -96,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: toFlatten, - Labels: flags.Label, + Flatten: flattenBuildpacks, }); err != nil { return err } @@ -114,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, "This flag triggers the buildpack to publish the built image to a container registry after the build process is complete. This means that the resulting image is pushed to a specified registry, making it available for deployment and use by others.") 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().StringSliceVar(&flags.Flatten, "flatten", nil, "List of buildpacks to flatten together into a single layer (format: '@,@'") - cmd.Flags().StringToStringVarP(&flags.Label, "label", "l", nil, "Labels to add to the builder image, in the form of '='") + cmd.Flags().StringSliceVar(&flags.Flatten, "flatten", nil, "List of Buildpacks to flatten together in one layer (format: '@,@'") AddHelpFlag(cmd, "create") return cmd diff --git a/pkg/buildpack/build_module_info.go b/pkg/buildpack/build_module_info.go index 1549579cd2..f846cd489c 100644 --- a/pkg/buildpack/build_module_info.go +++ b/pkg/buildpack/build_module_info.go @@ -49,18 +49,15 @@ func parseBuildpackName(names string) (ModuleInfos, error) { ids := strings.Split(names, ",") for _, id := range ids { if strings.Count(id, "@") != 1 { - return nil, errors.Errorf("invalid format %s; please use '@' to add buildpacks to be flattened", id) + return nil, errors.Errorf("invalid format %s; please use '@' to add buildpacks to be flatten", id) } bpFullName := strings.Split(id, "@") - idFromName := strings.TrimSpace(bpFullName[0]) - versionFromName := strings.TrimSpace(bpFullName[1]) - if idFromName == "" || versionFromName == "" { + if len(bpFullName) != 2 { return nil, errors.Errorf("invalid format %s; '' and '' must be specified", id) } - bpID := dist.ModuleInfo{ - ID: idFromName, - Version: versionFromName, + ID: bpFullName[0], + Version: bpFullName[1], } buildModuleInfos = append(buildModuleInfos, bpID) } diff --git a/pkg/buildpack/managed_collection.go b/pkg/buildpack/managed_collection.go index 5896caeb63..70e5a46371 100644 --- a/pkg/buildpack/managed_collection.go +++ b/pkg/buildpack/managed_collection.go @@ -1,22 +1,11 @@ package buildpack -// ManagedCollection keeps track of build modules and the manner in which they should be added to an OCI image (as flattened or exploded). +// ManagedCollection defines the required behavior to deal with BuildModule when adding then to an OCI image. type ManagedCollection interface { - // AllModules returns all build modules handled by the manager. AllModules() []BuildModule - - // ExplodedModules returns all build modules that will be added to the output artifact as a single layer - // containing a single module. ExplodedModules() []BuildModule - - // AddModules adds module information to the collection as flattened or not, depending on how the collection is configured. AddModules(main BuildModule, deps ...BuildModule) - - // FlattenedModules returns all build modules that will be added to the output artifact as a single layer - // containing multiple modules. FlattenedModules() [][]BuildModule - - // ShouldFlatten returns true if the given module should be flattened. ShouldFlatten(module BuildModule) bool } @@ -25,22 +14,24 @@ type managedCollection struct { flattenedModules [][]BuildModule } +// ExplodedModules returns all flattenModuleInfos that will be added to the output artifact as a single layer containing a single module. func (f *managedCollection) ExplodedModules() []BuildModule { return f.explodedModules } +// FlattenedModules returns all flattenModuleInfos that will be added to the output artifact as a single layer containing multiple flattenModuleInfos. func (f *managedCollection) FlattenedModules() [][]BuildModule { return f.flattenedModules } +// AllModules returns all explodedModules handle by the manager func (f *managedCollection) AllModules() []BuildModule { all := f.explodedModules - for _, modules := range f.flattenedModules { - all = append(all, modules...) - } + all = append(all, f.flattenedModules...) return all } +// ShouldFlatten returns true if the given module is flattened. func (f *managedCollection) ShouldFlatten(module BuildModule) bool { for _, modules := range f.flattenedModules { for _, v := range modules { @@ -52,6 +43,97 @@ func (f *managedCollection) ShouldFlatten(module BuildModule) bool { return false } +// managedCollectionV1 can be used to flatten all the flattenModuleInfos or none of them. +type managedCollectionV1 struct { + managedCollection + flatten bool +} + +func NewModuleManager(flatten bool) ManagedCollection { + return &managedCollectionV1{ + flatten: flatten, + managedCollection: managedCollection{ + explodedModules: []BuildModule{}, + flattenedModules: [][]BuildModule{}, + }, + } +} + +// AddModules determines whether the explodedModules must be added as flattened or not. +func (f *managedCollectionV1) AddModules(main BuildModule, deps ...BuildModule) { + if !f.flatten { + // default behavior + f.explodedModules = append(f.explodedModules, append([]BuildModule{main}, deps...)...) + } else { + // flatten all + f.flattenedModules = append(f.flattenedModules, append([]BuildModule{main}, deps...)...) + } + return all +} + +func NewModuleManagerV2(modules FlattenModuleInfos) ManagedCollection { + flattenGroups := 0 + if modules != nil { + flattenGroups = len(modules.FlattenModules()) + } + + return &managedCollectionV2{ + flattenModuleInfos: modules, + managedCollection: managedCollection{ + explodedModules: []BuildModule{}, + flattenedModules: make([][]BuildModule, flattenGroups), + }, + } +} + +// managedCollectionV2 can be used when flattenModuleInfos to be flattened are known beforehand. These flattenModuleInfos are provided during +// initialization and the collection will take care of keeping them in the correct group once they are added. +type managedCollectionV2 struct { + managedCollection + flattenModuleInfos FlattenModuleInfos +} + +func (ff *managedCollectionV2) flattenGroups() []ModuleInfos { + return ff.flattenModuleInfos.FlattenModules() +} + +func (ff *managedCollectionV2) AddModules(main BuildModule, deps ...BuildModule) { + var allModules []BuildModule + allModules = append(allModules, append([]BuildModule{main}, deps...)...) + for _, module := range allModules { + if ff.flattenModuleInfos != nil && len(ff.flattenGroups()) > 0 { + pos := ff.flattenGroup(module) + if pos >= 0 { + ff.flattenedModules[pos] = append(ff.flattenedModules[pos], module) + } else { + // this module must not be flattened + ff.explodedModules = append(ff.explodedModules, module) + } + } else { + // we don't want to flatten anything + ff.explodedModules = append(ff.explodedModules, module) + } + } +} + +// flattenGroup given a module it will try to determine to which row (group) this module must be added to in order to +// be flattened. If it is not found, it means, the module must no me flattened at all +func (ff *managedCollectionV2) flattenGroup(module BuildModule) int { + pos := -1 + // flattenModuleInfos to be flattened are representing a two-dimension array. where each row represents a group of + // flattenModuleInfos that must be flattened together in the same layer. +init: + for i, flattenGroup := range ff.flattenGroups() { + for _, buildModuleInfo := range flattenGroup.BuildModule() { + if buildModuleInfo.FullName() == module.Descriptor().Info().FullName() { + pos = i + break init + } + } + } + return pos +} + // managedCollectionV1 can be used to flatten all the flattenModuleInfos or none of them. type managedCollectionV1 struct { managedCollection diff --git a/pkg/buildpack/managed_collection_test.go b/pkg/buildpack/managed_collection_test.go index 96357f61c5..297fb1792e 100644 --- a/pkg/buildpack/managed_collection_test.go +++ b/pkg/buildpack/managed_collection_test.go @@ -30,6 +30,7 @@ func testModuleManager(t *testing.T, when spec.G, it spec.S) { * bp31 */ var ( +<<<<<<< HEAD moduleManager buildpack.ManagedCollection compositeBP1 buildpack.BuildModule bp1 buildpack.BuildModule @@ -40,6 +41,17 @@ func testModuleManager(t *testing.T, when spec.G, it spec.S) { bp31 buildpack.BuildModule flattenBuildModules buildpack.FlattenModuleInfos err error +======= + moduleManager buildpack.ManagedCollection + compositeBP1 buildpack.BuildModule + bp1 buildpack.BuildModule + compositeBP2 buildpack.BuildModule + bp21 buildpack.BuildModule + bp22 buildpack.BuildModule + compositeBP3 buildpack.BuildModule + bp31 buildpack.BuildModule + err error +>>>>>>> 70a41dc4 (Implementing RFC-0123) ) it.Before(func() { diff --git a/pkg/buildpack/package.go b/pkg/buildpack/package.go index 012864c927..a3211b7393 100644 --- a/pkg/buildpack/package.go +++ b/pkg/buildpack/package.go @@ -34,7 +34,7 @@ func (s *syncPkg) GetLayer(diffID string) (io.ReadCloser, error) { } // extractBuildpacks when provided a flattened buildpack package containing N buildpacks, -// will return N modules: 1 module with a single tar containing ALL N buildpacks, and N-1 modules with empty tar files. +// will return N flattenModuleInfos: 1 module with a single tar containing ALL N buildpacks, and N-1 flattenModuleInfos with empty tar files. func extractBuildpacks(pkg Package) (mainBP BuildModule, depBPs []BuildModule, err error) { pkg = &syncPkg{pkg: pkg} md := &Metadata{} diff --git a/pkg/client/create_builder.go b/pkg/client/create_builder.go index ee1062bc4f..de74567b0b 100644 --- a/pkg/client/create_builder.go +++ b/pkg/client/create_builder.go @@ -48,7 +48,7 @@ type CreateBuilderOptions struct { // Strategy for updating images before a build. PullPolicy image.PullPolicy - // List of modules to be flattened + // List of buildpack to be flatten Flatten buildpack.FlattenModuleInfos } @@ -155,7 +155,7 @@ func (c *Client) createBaseBuilder(ctx context.Context, opts CreateBuilderOption var builderOpts []builder.BuilderOption if opts.Flatten != nil && len(opts.Flatten.FlattenModules()) > 0 { - builderOpts = append(builderOpts, builder.WithFlattened(opts.Flatten)) + builderOpts = append(builderOpts, builder.WithFlatten(opts.Flatten)) } if opts.Labels != nil && len(opts.Labels) > 0 { builderOpts = append(builderOpts, builder.WithLabels(opts.Labels)) diff --git a/pkg/client/create_builder_test.go b/pkg/client/create_builder_test.go index 34f5bc7aa8..46c1546a89 100644 --- a/pkg/client/create_builder_test.go +++ b/pkg/client/create_builder_test.go @@ -1083,6 +1083,7 @@ func testCreateBuilder(t *testing.T, when spec.G, it spec.S) { }) when("flatten all", func() { + var err error it("creates 1 layer for all buildpacks", func() { prepareFetcherWithRunImages() opts.Flatten, err = buildpack.ParseFlattenBuildModules([]string{"flatten/bp-1@1,flatten/bp-2@2,flatten/bp-4@4,flatten/bp-6@6,flatten/bp-7@7,flatten/bp-3@3,flatten/bp-5@5"}) @@ -1096,8 +1097,8 @@ func testCreateBuilder(t *testing.T, when spec.G, it spec.S) { }) }) - when("only some modules are flattened", func() { - it("creates 1 layer for buildpacks [1,2,3,4,5,6] and 1 layer for buildpack [7]", func() { + when("with exclude", func() { + it("creates 1 layer for buildpacks and 1 layer for buildpack excluded", func() { prepareFetcherWithRunImages() opts.Flatten, err = buildpack.ParseFlattenBuildModules([]string{"flatten/bp-1@1,flatten/bp-2@2,flatten/bp-4@4,flatten/bp-6@6,flatten/bp-3@3,flatten/bp-5@5"}) h.AssertNil(t, err) From ef68692d566a165f109653695fe75d062dba41b1 Mon Sep 17 00:00:00 2001 From: Juan Bustamante Date: Wed, 20 Dec 2023 17:41:20 -0400 Subject: [PATCH 5/9] adding feedback from review Signed-off-by: Juan Bustamante --- builder/buildpack_identifier.go | 2 +- internal/builder/builder.go | 12 +++---- internal/builder/builder_test.go | 2 +- internal/commands/builder_create.go | 6 ++-- pkg/buildpack/managed_collection.go | 46 +++++++++++++----------- pkg/buildpack/managed_collection_test.go | 19 ++++++++++ pkg/buildpack/package.go | 2 +- pkg/client/create_builder.go | 4 +-- pkg/client/create_builder_test.go | 8 +++++ 9 files changed, 66 insertions(+), 35 deletions(-) diff --git a/builder/buildpack_identifier.go b/builder/buildpack_identifier.go index ec69a6506e..b32e111dc7 100644 --- a/builder/buildpack_identifier.go +++ b/builder/buildpack_identifier.go @@ -1,5 +1,5 @@ package builder -type Identifier interface { +type BpIdentifier interface { Id() string } diff --git a/internal/builder/builder.go b/internal/builder/builder.go index 6b347147cd..ba405fb40c 100644 --- a/internal/builder/builder.go +++ b/internal/builder/builder.go @@ -102,7 +102,7 @@ type moduleWithDiffID struct { type BuilderOption func(*options) error type options struct { - modules buildpack.FlattenModuleInfos + toFlatten buildpack.FlattenModuleInfos } // FromImage constructs a builder from a builder image @@ -155,8 +155,8 @@ func constructBuilder(img imgutil.Image, newName string, errOnMissingLabel bool, env: map[string]string{}, buildConfigEnv: map[string]string{}, validateMixins: true, - additionalBuildpacks: buildpack.NewModuleManagerV2(opts.modules), - additionalExtensions: buildpack.NewModuleManagerV2(opts.modules), + additionalBuildpacks: buildpack.NewManagedCollectionV2(opts.toFlatten), + additionalExtensions: buildpack.NewManagedCollectionV2(opts.toFlatten), } if err := addImgLabelsToBuildr(bldr); err != nil { @@ -170,9 +170,9 @@ func constructBuilder(img imgutil.Image, newName string, errOnMissingLabel bool, return bldr, nil } -func WithFlatten(modules buildpack.FlattenModuleInfos) BuilderOption { +func Flatten(modules buildpack.FlattenModuleInfos) BuilderOption { return func(o *options) error { - o.modules = modules + o.toFlatten = modules return nil } } @@ -309,7 +309,7 @@ func (b *Builder) moduleManager(kind string) buildpack.ManagedCollection { case buildpack.KindExtension: return b.additionalExtensions } - return buildpack.NewModuleManager(false) + return nil } func (b *Builder) FlattenedModules(kind string) [][]buildpack.BuildModule { diff --git a/internal/builder/builder_test.go b/internal/builder/builder_test.go index cc61f60f31..56b4e926d8 100644 --- a/internal/builder/builder_test.go +++ b/internal/builder/builder_test.go @@ -1879,7 +1879,7 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) { 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.WithFlatten(flattenModules)) + bldr, err = builder.New(builderImage, "some-builder", builder.Flatten(flattenModules)) h.AssertNil(t, err) // Let's add the buildpacks diff --git a/internal/commands/builder_create.go b/internal/commands/builder_create.go index d67bf8df5d..879cb0ab03 100644 --- a/internal/commands/builder_create.go +++ b/internal/commands/builder_create.go @@ -81,7 +81,7 @@ Creating a custom builder allows you to control what buildpacks are used and wha return err } - flattenBuildpacks, err := buildpack.ParseFlattenBuildModules(flags.Flatten) + toFlatten, err := buildpack.ParseFlattenBuildModules(flags.Flatten) if err != nil { return err } @@ -95,7 +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: flattenBuildpacks, + Flatten: toFlatten, }); err != nil { return err } @@ -112,7 +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, "This flag triggers the buildpack to publish the built image to a container registry after the build process is complete. This means that the resulting image is pushed to a specified registry, making it available for deployment and use by others.") 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().StringSliceVar(&flags.Flatten, "flatten", nil, "List of Buildpacks to flatten together in one layer (format: '@,@'") + cmd.Flags().StringSliceVar(&flags.Flatten, "flatten", nil, "List of buildpacks to flatten together into a single layer (format: '@,@'") AddHelpFlag(cmd, "create") return cmd diff --git a/pkg/buildpack/managed_collection.go b/pkg/buildpack/managed_collection.go index 70e5a46371..a4113a669b 100644 --- a/pkg/buildpack/managed_collection.go +++ b/pkg/buildpack/managed_collection.go @@ -1,11 +1,22 @@ package buildpack -// ManagedCollection defines the required behavior to deal with BuildModule when adding then to an OCI image. +// ManagedCollection defines the required behavior to deal with build modules when adding then to an OCI image. type ManagedCollection interface { + // AllModules returns all build modules handle by the manager AllModules() []BuildModule + + // ExplodedModules returns all build modules that will be added to the output artifact as a single layer + // containing a single module. ExplodedModules() []BuildModule + + // AddModules determines whether the explodedModules must be added as flattened or not. AddModules(main BuildModule, deps ...BuildModule) + + // FlattenedModules returns all build modules that will be added to the output artifact as a single layer + // containing multiple modules. FlattenedModules() [][]BuildModule + + // ShouldFlatten returns true if the given module should be flattened. ShouldFlatten(module BuildModule) bool } @@ -14,24 +25,20 @@ type managedCollection struct { flattenedModules [][]BuildModule } -// ExplodedModules returns all flattenModuleInfos that will be added to the output artifact as a single layer containing a single module. func (f *managedCollection) ExplodedModules() []BuildModule { return f.explodedModules } -// FlattenedModules returns all flattenModuleInfos that will be added to the output artifact as a single layer containing multiple flattenModuleInfos. func (f *managedCollection) FlattenedModules() [][]BuildModule { return f.flattenedModules } -// AllModules returns all explodedModules handle by the manager func (f *managedCollection) AllModules() []BuildModule { all := f.explodedModules all = append(all, f.flattenedModules...) return all } -// ShouldFlatten returns true if the given module is flattened. func (f *managedCollection) ShouldFlatten(module BuildModule) bool { for _, modules := range f.flattenedModules { for _, v := range modules { @@ -46,12 +53,12 @@ func (f *managedCollection) ShouldFlatten(module BuildModule) bool { // managedCollectionV1 can be used to flatten all the flattenModuleInfos or none of them. type managedCollectionV1 struct { managedCollection - flatten bool + flattenAll bool } -func NewModuleManager(flatten bool) ManagedCollection { +func NewManagedCollectionV1(flattenAll bool) ManagedCollection { return &managedCollectionV1{ - flatten: flatten, + flattenAll: flattenAll, managedCollection: managedCollection{ explodedModules: []BuildModule{}, flattenedModules: [][]BuildModule{}, @@ -59,9 +66,8 @@ func NewModuleManager(flatten bool) ManagedCollection { } } -// AddModules determines whether the explodedModules must be added as flattened or not. func (f *managedCollectionV1) AddModules(main BuildModule, deps ...BuildModule) { - if !f.flatten { + if !f.flattenAll { // default behavior f.explodedModules = append(f.explodedModules, append([]BuildModule{main}, deps...)...) } else { @@ -71,7 +77,7 @@ func (f *managedCollectionV1) AddModules(main BuildModule, deps ...BuildModule) return all } -func NewModuleManagerV2(modules FlattenModuleInfos) ManagedCollection { +func NewManagedCollectionV2(modules FlattenModuleInfos) ManagedCollection { flattenGroups := 0 if modules != nil { flattenGroups = len(modules.FlattenModules()) @@ -86,8 +92,9 @@ func NewModuleManagerV2(modules FlattenModuleInfos) ManagedCollection { } } -// managedCollectionV2 can be used when flattenModuleInfos to be flattened are known beforehand. These flattenModuleInfos are provided during -// initialization and the collection will take care of keeping them in the correct group once they are added. +// managedCollectionV2 can be used when the build modules to be flattened are known at the point of initialization. +// The flattened build modules are provided when the collection is initialized and the collection will take care of +// keeping them in the correct group (flattened or exploded) once they are added. type managedCollectionV2 struct { managedCollection flattenModuleInfos FlattenModuleInfos @@ -102,7 +109,7 @@ func (ff *managedCollectionV2) AddModules(main BuildModule, deps ...BuildModule) allModules = append(allModules, append([]BuildModule{main}, deps...)...) for _, module := range allModules { if ff.flattenModuleInfos != nil && len(ff.flattenGroups()) > 0 { - pos := ff.flattenGroup(module) + pos := ff.flattenedLayerFor(module) if pos >= 0 { ff.flattenedModules[pos] = append(ff.flattenedModules[pos], module) } else { @@ -116,22 +123,19 @@ func (ff *managedCollectionV2) AddModules(main BuildModule, deps ...BuildModule) } } -// flattenGroup given a module it will try to determine to which row (group) this module must be added to in order to +// flattenedLayerFor given a module it will try to determine to which row (group) this module must be added to in order to // be flattened. If it is not found, it means, the module must no me flattened at all -func (ff *managedCollectionV2) flattenGroup(module BuildModule) int { - pos := -1 +func (ff *managedCollectionV2) flattenedLayerFor(module BuildModule) int { // flattenModuleInfos to be flattened are representing a two-dimension array. where each row represents a group of // flattenModuleInfos that must be flattened together in the same layer. -init: for i, flattenGroup := range ff.flattenGroups() { for _, buildModuleInfo := range flattenGroup.BuildModule() { if buildModuleInfo.FullName() == module.Descriptor().Info().FullName() { - pos = i - break init + return i } } } - return pos + return -1 } // managedCollectionV1 can be used to flatten all the flattenModuleInfos or none of them. diff --git a/pkg/buildpack/managed_collection_test.go b/pkg/buildpack/managed_collection_test.go index 297fb1792e..95e94ae8ab 100644 --- a/pkg/buildpack/managed_collection_test.go +++ b/pkg/buildpack/managed_collection_test.go @@ -150,12 +150,20 @@ func testModuleManager(t *testing.T, when spec.G, it spec.S) { }) when("manager is configured in flatten mode", func() { +<<<<<<< HEAD when("V1 is used", func() { when("flatten all", func() { it.Before(func() { moduleManager = buildpack.NewManagedCollectionV1(true) moduleManager.AddModules(compositeBP1, []buildpack.BuildModule{bp1, compositeBP2, bp21, bp22, compositeBP3, bp31}...) }) +======= + when("flatten all", func() { + it.Before(func() { + moduleManager = buildpack.NewManagedCollectionV1(true) + moduleManager.AddModules(compositeBP1, []buildpack.BuildModule{bp1, compositeBP2, bp21, bp22, compositeBP3, bp31}...) + }) +>>>>>>> 0edc1a25 (adding feedback from review) when("#FlattenedModules", func() { it("returns one flatten module (1 layer)", func() { @@ -249,9 +257,20 @@ func testModuleManager(t *testing.T, when spec.G, it spec.S) { }) when("manager is not configured in flatten mode", func() { +<<<<<<< HEAD when("V1 is used", func() { it.Before(func() { moduleManager = buildpack.NewManagedCollectionV1(false) +======= + it.Before(func() { + moduleManager = buildpack.NewManagedCollectionV1(false) + }) + + when("#ExplodedModules", func() { + it("returns nil when no explodedModules are added", func() { + modules := moduleManager.ExplodedModules() + h.AssertEq(t, len(modules), 0) +>>>>>>> 0edc1a25 (adding feedback from review) }) when("#ExplodedModules", func() { diff --git a/pkg/buildpack/package.go b/pkg/buildpack/package.go index a3211b7393..012864c927 100644 --- a/pkg/buildpack/package.go +++ b/pkg/buildpack/package.go @@ -34,7 +34,7 @@ func (s *syncPkg) GetLayer(diffID string) (io.ReadCloser, error) { } // extractBuildpacks when provided a flattened buildpack package containing N buildpacks, -// will return N flattenModuleInfos: 1 module with a single tar containing ALL N buildpacks, and N-1 flattenModuleInfos with empty tar files. +// will return N modules: 1 module with a single tar containing ALL N buildpacks, and N-1 modules with empty tar files. func extractBuildpacks(pkg Package) (mainBP BuildModule, depBPs []BuildModule, err error) { pkg = &syncPkg{pkg: pkg} md := &Metadata{} diff --git a/pkg/client/create_builder.go b/pkg/client/create_builder.go index de74567b0b..2b452c22e3 100644 --- a/pkg/client/create_builder.go +++ b/pkg/client/create_builder.go @@ -48,7 +48,7 @@ type CreateBuilderOptions struct { // Strategy for updating images before a build. PullPolicy image.PullPolicy - // List of buildpack to be flatten + // List of modules to be flattened Flatten buildpack.FlattenModuleInfos } @@ -155,7 +155,7 @@ func (c *Client) createBaseBuilder(ctx context.Context, opts CreateBuilderOption var builderOpts []builder.BuilderOption if opts.Flatten != nil && len(opts.Flatten.FlattenModules()) > 0 { - builderOpts = append(builderOpts, builder.WithFlatten(opts.Flatten)) + builderOpts = append(builderOpts, builder.Flatten(opts.Flatten)) } if opts.Labels != nil && len(opts.Labels) > 0 { builderOpts = append(builderOpts, builder.WithLabels(opts.Labels)) diff --git a/pkg/client/create_builder_test.go b/pkg/client/create_builder_test.go index 46c1546a89..37880b67e7 100644 --- a/pkg/client/create_builder_test.go +++ b/pkg/client/create_builder_test.go @@ -1097,11 +1097,19 @@ func testCreateBuilder(t *testing.T, when spec.G, it spec.S) { }) }) +<<<<<<< HEAD when("with exclude", func() { it("creates 1 layer for buildpacks and 1 layer for buildpack excluded", func() { prepareFetcherWithRunImages() opts.Flatten, err = buildpack.ParseFlattenBuildModules([]string{"flatten/bp-1@1,flatten/bp-2@2,flatten/bp-4@4,flatten/bp-6@6,flatten/bp-3@3,flatten/bp-5@5"}) h.AssertNil(t, err) +======= + when("only some modules are flattened", func() { + it("creates 1 layer for buildpacks and 1 layer for buildpack excluded", func() { + prepareFetcherWithRunImages() + opts.Flatten, err = buildpack.ParseFlattenBuildModules([]string{"flatten/bp-1@1,flatten/bp-2@2,flatten/bp-4@4,flatten/bp-6@6,flatten/bp-3@3,flatten/bp-5@5"}) + h.AssertNil(t, err) +>>>>>>> 0edc1a25 (adding feedback from review) successfullyCreateFlattenBuilder() From e1a4d0096b10325f49692f12a3133f2a51d1e5be Mon Sep 17 00:00:00 2001 From: Juan Bustamante Date: Tue, 23 Jan 2024 09:58:52 -0500 Subject: [PATCH 6/9] Moves lifecycle package to phase Signed-off-by: Juan Bustamante --- go.mod | 1 - go.sum | 4 ---- 2 files changed, 5 deletions(-) diff --git a/go.mod b/go.mod index 0897aa9079..0630bd8ed4 100644 --- a/go.mod +++ b/go.mod @@ -75,7 +75,6 @@ require ( github.com/containerd/containerd v1.7.7 // indirect github.com/containerd/log v0.1.0 // indirect github.com/containerd/stargz-snapshotter/estargz v0.14.3 // indirect - github.com/containerd/typeurl v1.0.2 // indirect github.com/containerd/typeurl/v2 v2.1.1 // indirect github.com/cyphar/filepath-securejoin v0.2.4 // indirect github.com/dimchansky/utfbom v1.1.1 // indirect diff --git a/go.sum b/go.sum index 5bd9465469..4e1b008b36 100644 --- a/go.sum +++ b/go.sum @@ -106,8 +106,6 @@ github.com/containerd/log v0.1.0 h1:TCJt7ioM2cr/tfR8GPbGf9/VRAX8D2B4PjzCpfX540I= github.com/containerd/log v0.1.0/go.mod h1:VRRf09a7mHDIRezVKTRCrOq78v577GXq3bSa3EhrzVo= github.com/containerd/stargz-snapshotter/estargz v0.14.3 h1:OqlDCK3ZVUO6C3B/5FSkDwbkEETK84kQgEeFwDC+62k= github.com/containerd/stargz-snapshotter/estargz v0.14.3/go.mod h1:KY//uOCIkSuNAHhJogcZtrNHdKrA99/FCCRjE3HD36o= -github.com/containerd/typeurl v1.0.2 h1:Chlt8zIieDbzQFzXzAeBEF92KhExuE4p9p92/QmY7aY= -github.com/containerd/typeurl v1.0.2/go.mod h1:9trJWW2sRlGub4wZJRTW83VtbOLS6hwcDZXTn6oPz9s= github.com/containerd/typeurl/v2 v2.1.1 h1:3Q4Pt7i8nYwy2KmQWIw2+1hTvwTE/6w9FqcttATPO/4= github.com/containerd/typeurl/v2 v2.1.1/go.mod h1:IDp2JFvbwZ31H8dQbEIY7sDl2L3o3HZj1hsSQlywkQ0= github.com/cpuguy83/go-md2man/v2 v2.0.3/go.mod h1:tgQtvFlXSQOSOSIRvRPT7W67SCa46tRHOmNcaadrF8o= @@ -264,8 +262,6 @@ github.com/mitchellh/go-homedir v1.1.0 h1:lukF9ziXFxDFPkA1vsr5zpc1XuPDn/wFntq5mG github.com/mitchellh/go-homedir v1.1.0/go.mod h1:SfyaCUpYCn1Vlf4IUYiD9fPX4A5wJrkLzIz1N1q0pr0= github.com/mitchellh/ioprogress v0.0.0-20180201004757-6a23b12fa88e h1:Qa6dnn8DlasdXRnacluu8HzPts0S1I9zvvUPDbBnXFI= github.com/mitchellh/ioprogress v0.0.0-20180201004757-6a23b12fa88e/go.mod h1:waEya8ee1Ro/lgxpVhkJI4BVASzkm3UZqkx/cFJiYHM= -github.com/moby/buildkit v0.11.6 h1:VYNdoKk5TVxN7k4RvZgdeM4GOyRvIi4Z8MXOY7xvyUs= -github.com/moby/buildkit v0.11.6/go.mod h1:GCqKfHhz+pddzfgaR7WmHVEE3nKKZMMDPpK8mh3ZLv4= github.com/moby/buildkit v0.12.2 h1:B7guBgY6sfk4dBlv/ORUxyYlp0UojYaYyATgtNwSCXc= github.com/moby/buildkit v0.12.2/go.mod h1:adB4y0SxxX8trnrY+oEulb48ODLqPO6pKMF0ppGcCoI= github.com/moby/patternmatcher v0.6.0 h1:GmP9lR19aU5GqSSFko+5pRqHi+Ohk1O69aFiKkVGiPk= From b7d6bc0cb34a165af2230011b88c1a21f40ee7f4 Mon Sep 17 00:00:00 2001 From: Parthiba-Hazra Date: Mon, 29 Jan 2024 20:00:37 +0530 Subject: [PATCH 7/9] Renaming the `--tag` flag to `--previous-image` to better reflect its purpose. If previous-image flag set then workingImage will be based off of opts.PreviousImage in the rebaser.Rebase Signed-off-by: Parthiba-Hazra fix merge conflicts Signed-off-by: Parthiba-Hazra resolve merge conflict Signed-off-by: Parthiba-Hazra --- go.mod | 2 +- go.sum | 9 ++++----- internal/commands/rebase.go | 2 +- internal/commands/rebase_test.go | 15 +++++++++------ pkg/client/rebase.go | 20 ++++++-------------- 5 files changed, 21 insertions(+), 27 deletions(-) diff --git a/go.mod b/go.mod index 0630bd8ed4..ac75c04009 100644 --- a/go.mod +++ b/go.mod @@ -75,7 +75,7 @@ require ( github.com/containerd/containerd v1.7.7 // indirect github.com/containerd/log v0.1.0 // indirect github.com/containerd/stargz-snapshotter/estargz v0.14.3 // indirect - github.com/containerd/typeurl/v2 v2.1.1 // indirect + github.com/containerd/typeurl v1.0.2 // indirect github.com/cyphar/filepath-securejoin v0.2.4 // indirect github.com/dimchansky/utfbom v1.1.1 // indirect github.com/distribution/reference v0.5.0 // indirect diff --git a/go.sum b/go.sum index 4e1b008b36..caa20b082c 100644 --- a/go.sum +++ b/go.sum @@ -106,8 +106,8 @@ github.com/containerd/log v0.1.0 h1:TCJt7ioM2cr/tfR8GPbGf9/VRAX8D2B4PjzCpfX540I= github.com/containerd/log v0.1.0/go.mod h1:VRRf09a7mHDIRezVKTRCrOq78v577GXq3bSa3EhrzVo= github.com/containerd/stargz-snapshotter/estargz v0.14.3 h1:OqlDCK3ZVUO6C3B/5FSkDwbkEETK84kQgEeFwDC+62k= github.com/containerd/stargz-snapshotter/estargz v0.14.3/go.mod h1:KY//uOCIkSuNAHhJogcZtrNHdKrA99/FCCRjE3HD36o= -github.com/containerd/typeurl/v2 v2.1.1 h1:3Q4Pt7i8nYwy2KmQWIw2+1hTvwTE/6w9FqcttATPO/4= -github.com/containerd/typeurl/v2 v2.1.1/go.mod h1:IDp2JFvbwZ31H8dQbEIY7sDl2L3o3HZj1hsSQlywkQ0= +github.com/containerd/typeurl v1.0.2 h1:Chlt8zIieDbzQFzXzAeBEF92KhExuE4p9p92/QmY7aY= +github.com/containerd/typeurl v1.0.2/go.mod h1:9trJWW2sRlGub4wZJRTW83VtbOLS6hwcDZXTn6oPz9s= github.com/cpuguy83/go-md2man/v2 v2.0.3/go.mod h1:tgQtvFlXSQOSOSIRvRPT7W67SCa46tRHOmNcaadrF8o= github.com/creack/pty v1.1.18 h1:n56/Zwd5o6whRC5PMGretI4IdRLlmBXYNjScPaBgsbY= github.com/creack/pty v1.1.18/go.mod h1:MOBLtS5ELjhRRrroQr9kyvTxUAFNvYEK993ew/Vr4O4= @@ -262,8 +262,8 @@ github.com/mitchellh/go-homedir v1.1.0 h1:lukF9ziXFxDFPkA1vsr5zpc1XuPDn/wFntq5mG github.com/mitchellh/go-homedir v1.1.0/go.mod h1:SfyaCUpYCn1Vlf4IUYiD9fPX4A5wJrkLzIz1N1q0pr0= github.com/mitchellh/ioprogress v0.0.0-20180201004757-6a23b12fa88e h1:Qa6dnn8DlasdXRnacluu8HzPts0S1I9zvvUPDbBnXFI= github.com/mitchellh/ioprogress v0.0.0-20180201004757-6a23b12fa88e/go.mod h1:waEya8ee1Ro/lgxpVhkJI4BVASzkm3UZqkx/cFJiYHM= -github.com/moby/buildkit v0.12.2 h1:B7guBgY6sfk4dBlv/ORUxyYlp0UojYaYyATgtNwSCXc= -github.com/moby/buildkit v0.12.2/go.mod h1:adB4y0SxxX8trnrY+oEulb48ODLqPO6pKMF0ppGcCoI= +github.com/moby/buildkit v0.11.6 h1:VYNdoKk5TVxN7k4RvZgdeM4GOyRvIi4Z8MXOY7xvyUs= +github.com/moby/buildkit v0.11.6/go.mod h1:GCqKfHhz+pddzfgaR7WmHVEE3nKKZMMDPpK8mh3ZLv4= github.com/moby/patternmatcher v0.6.0 h1:GmP9lR19aU5GqSSFko+5pRqHi+Ohk1O69aFiKkVGiPk= github.com/moby/patternmatcher v0.6.0/go.mod h1:hDPoyOpDY7OrrMDLaYoY3hf52gNCR/YOUYxkhApJIxc= github.com/moby/sys/sequential v0.5.0 h1:OPvI35Lzn9K04PBbCLW0g4LcFAJgHsvXsRyewg5lXtc= @@ -502,7 +502,6 @@ google.golang.org/appengine v1.6.8 h1:IhEN5q69dyKagZPYMSdIjS2HqprW324FRQZJcGqPAs google.golang.org/appengine v1.6.8/go.mod h1:1jJ3jBArFh5pcgW8gCtRJnepW8FzD1V44FJffLiz/Ds= google.golang.org/protobuf v1.26.0-rc.1/go.mod h1:jlhhOSvTdKEhbULTjvd4ARK9grFBp09yW+WbY/TyQbw= google.golang.org/protobuf v1.26.0/go.mod h1:9q0QmTI4eRPtz6boOQmLYwt+qCgq0jsYwAQnmE0givc= -google.golang.org/protobuf v1.27.1/go.mod h1:9q0QmTI4eRPtz6boOQmLYwt+qCgq0jsYwAQnmE0givc= google.golang.org/protobuf v1.31.0 h1:g0LDEJHgrBl9N9r17Ru3sqWhkIx2NB67okBHPwC7hs8= google.golang.org/protobuf v1.31.0/go.mod h1:HV8QOd/L58Z+nl8r43ehVNZIU/HEI6OcFqwMG9pJV4I= gopkg.in/alecthomas/kingpin.v2 v2.2.6/go.mod h1:FMv+mEhP44yOT+4EoQTLFTRgOQ1FBLkstjWtayDeSgw= diff --git a/internal/commands/rebase.go b/internal/commands/rebase.go index 79afada61a..4544724d34 100644 --- a/internal/commands/rebase.go +++ b/internal/commands/rebase.go @@ -49,7 +49,7 @@ func Rebase(logger logging.Logger, cfg config.Config, pack PackClient) *cobra.Co cmd.Flags().BoolVar(&opts.Publish, "publish", false, "Publish the rebased application image directly to the container registry specified in , instead of the daemon. The previous application image must also reside in the registry.") cmd.Flags().StringVar(&opts.RunImage, "run-image", "", "Run image to use for rebasing") cmd.Flags().StringVar(&policy, "pull-policy", "", "Pull policy to use. Accepted values are always, never, and if-not-present. The default is always") - cmd.Flags().StringSliceVarP(&opts.Tags, "tag", "t", nil, "Comma-separated list of tag to apply to the rebased image") + cmd.Flags().StringVar(&opts.PreviousImage, "previous-image", "", "Image reference to use as the previous base image for rebase") cmd.Flags().StringVar(&opts.ReportDestinationDir, "report-output-dir", "", "Path to export build report.toml.\nOmitting the flag yield no report file.") cmd.Flags().BoolVar(&opts.Force, "force", false, "Perform rebase operation without target validation (only available for API >= 0.12)") diff --git a/internal/commands/rebase_test.go b/internal/commands/rebase_test.go index c4833d87b4..a3bdf05f5e 100644 --- a/internal/commands/rebase_test.go +++ b/internal/commands/rebase_test.go @@ -158,7 +158,9 @@ func testRebaseCommand(t *testing.T, when spec.G, it spec.S) { }) }) }) - when("image name and tags are provided", func() { + when("image name and previous image are provided", func() { + var expectedOpts client.RebaseOptions + it.Before(func() { runImage := "test/image" testMirror1 := "example.com/some/run1" @@ -171,8 +173,8 @@ func testRebaseCommand(t *testing.T, when spec.G, it spec.S) { command = commands.Rebase(logger, cfg, mockClient) repoName = "test/repo-image" - tags := []string{"tag1"} - opts = client.RebaseOptions{ + previousImage := "example.com/previous-image:tag" // Example of previous image with tag + opts := client.RebaseOptions{ RepoName: repoName, Publish: false, PullPolicy: image.PullAlways, @@ -180,16 +182,17 @@ func testRebaseCommand(t *testing.T, when spec.G, it spec.S) { AdditionalMirrors: map[string][]string{ runImage: {testMirror1, testMirror2}, }, - Tags: tags, + PreviousImage: previousImage, } + expectedOpts = opts }) it("works", func() { mockClient.EXPECT(). - Rebase(gomock.Any(), gomock.Eq(opts)). + Rebase(gomock.Any(), gomock.Eq(expectedOpts)). Return(nil) - command.SetArgs([]string{repoName, "--tag", "tag1"}) + command.SetArgs([]string{repoName, "--previous-image", "example.com/previous-image:tag"}) h.AssertNil(t, command.Execute()) }) }) diff --git a/pkg/client/rebase.go b/pkg/client/rebase.go index 36072f9a01..20fc3c060e 100644 --- a/pkg/client/rebase.go +++ b/pkg/client/rebase.go @@ -5,7 +5,6 @@ import ( "fmt" "os" "path/filepath" - "strings" "github.com/BurntSushi/toml" "github.com/buildpacks/lifecycle/phase" @@ -49,11 +48,9 @@ type RebaseOptions struct { Force bool // Tags to be applied to the rebased image. - Tags []string + PreviousImage string } -const tagDelim = ":" - // Rebase updates the run image layers in an app image. // This operation mutates the image specified in opts. func (c *Client) Rebase(ctx context.Context, opts RebaseOptions) error { @@ -62,18 +59,13 @@ func (c *Client) Rebase(ctx context.Context, opts RebaseOptions) error { return errors.Wrapf(err, "invalid image name '%s'", opts.RepoName) } - appImageName := opts.RepoName + repoName := opts.RepoName - if len(opts.Tags) > 0 { - parts := strings.SplitN(appImageName, tagDelim, 2) - if len(parts) == 2 { - appImageName = fmt.Sprintf("%s:%s", parts[0], opts.Tags[0]) - } else { - appImageName = fmt.Sprintf("%s:%s", appImageName, opts.Tags[0]) - } + if opts.PreviousImage != "" { + repoName = opts.PreviousImage } - appImage, err := c.imageFetcher.Fetch(ctx, appImageName, image.FetchOptions{Daemon: !opts.Publish, PullPolicy: opts.PullPolicy}) + appImage, err := c.imageFetcher.Fetch(ctx, repoName, image.FetchOptions{Daemon: !opts.Publish, PullPolicy: opts.PullPolicy}) if err != nil { return err } @@ -131,7 +123,7 @@ func (c *Client) Rebase(ctx context.Context, opts RebaseOptions) error { c.logger.Infof("Rebasing %s on run image %s", style.Symbol(appImage.Name()), style.Symbol(baseImage.Name())) rebaser := &phase.Rebaser{Logger: c.logger, PlatformAPI: build.SupportedPlatformAPIVersions.Latest(), Force: opts.Force} - report, err := rebaser.Rebase(appImage, baseImage, appImage.Name(), nil) + report, err := rebaser.Rebase(appImage, baseImage, opts.RepoName, nil) if err != nil { return err } From c6749c7b9488c0545256296568128a1ecbc97f1e Mon Sep 17 00:00:00 2001 From: Parthiba-Hazra Date: Mon, 29 Jan 2024 21:04:34 +0530 Subject: [PATCH 8/9] fix merge conflicts Signed-off-by: Parthiba-Hazra update documentation for PreviousImage filed in RebaseOptions Signed-off-by: Parthiba-Hazra update docs Signed-off-by: Parthiba-Hazra --- go.mod | 1 + go.sum | 5 ++ internal/builder/builder.go | 10 ++- internal/builder/builder_test.go | 5 +- internal/commands/build.go | 2 +- internal/commands/builder_create.go | 5 +- internal/commands/buildpack_package.go | 2 +- internal/commands/create_builder.go | 2 +- internal/commands/extension_package.go | 2 +- internal/commands/package_buildpack.go | 2 +- internal/commands/rebase.go | 2 +- pkg/buildpack/build_module_info.go | 11 ++- pkg/buildpack/managed_collection.go | 98 ++---------------------- pkg/buildpack/managed_collection_test.go | 31 -------- pkg/client/create_builder.go | 2 +- pkg/client/create_builder_test.go | 13 +--- pkg/client/rebase.go | 2 +- 17 files changed, 43 insertions(+), 152 deletions(-) diff --git a/go.mod b/go.mod index ac75c04009..0897aa9079 100644 --- a/go.mod +++ b/go.mod @@ -76,6 +76,7 @@ require ( github.com/containerd/log v0.1.0 // indirect github.com/containerd/stargz-snapshotter/estargz v0.14.3 // indirect github.com/containerd/typeurl v1.0.2 // indirect + github.com/containerd/typeurl/v2 v2.1.1 // indirect github.com/cyphar/filepath-securejoin v0.2.4 // indirect github.com/dimchansky/utfbom v1.1.1 // indirect github.com/distribution/reference v0.5.0 // indirect diff --git a/go.sum b/go.sum index caa20b082c..5bd9465469 100644 --- a/go.sum +++ b/go.sum @@ -108,6 +108,8 @@ github.com/containerd/stargz-snapshotter/estargz v0.14.3 h1:OqlDCK3ZVUO6C3B/5FSk github.com/containerd/stargz-snapshotter/estargz v0.14.3/go.mod h1:KY//uOCIkSuNAHhJogcZtrNHdKrA99/FCCRjE3HD36o= github.com/containerd/typeurl v1.0.2 h1:Chlt8zIieDbzQFzXzAeBEF92KhExuE4p9p92/QmY7aY= github.com/containerd/typeurl v1.0.2/go.mod h1:9trJWW2sRlGub4wZJRTW83VtbOLS6hwcDZXTn6oPz9s= +github.com/containerd/typeurl/v2 v2.1.1 h1:3Q4Pt7i8nYwy2KmQWIw2+1hTvwTE/6w9FqcttATPO/4= +github.com/containerd/typeurl/v2 v2.1.1/go.mod h1:IDp2JFvbwZ31H8dQbEIY7sDl2L3o3HZj1hsSQlywkQ0= github.com/cpuguy83/go-md2man/v2 v2.0.3/go.mod h1:tgQtvFlXSQOSOSIRvRPT7W67SCa46tRHOmNcaadrF8o= github.com/creack/pty v1.1.18 h1:n56/Zwd5o6whRC5PMGretI4IdRLlmBXYNjScPaBgsbY= github.com/creack/pty v1.1.18/go.mod h1:MOBLtS5ELjhRRrroQr9kyvTxUAFNvYEK993ew/Vr4O4= @@ -264,6 +266,8 @@ github.com/mitchellh/ioprogress v0.0.0-20180201004757-6a23b12fa88e h1:Qa6dnn8Dla github.com/mitchellh/ioprogress v0.0.0-20180201004757-6a23b12fa88e/go.mod h1:waEya8ee1Ro/lgxpVhkJI4BVASzkm3UZqkx/cFJiYHM= github.com/moby/buildkit v0.11.6 h1:VYNdoKk5TVxN7k4RvZgdeM4GOyRvIi4Z8MXOY7xvyUs= github.com/moby/buildkit v0.11.6/go.mod h1:GCqKfHhz+pddzfgaR7WmHVEE3nKKZMMDPpK8mh3ZLv4= +github.com/moby/buildkit v0.12.2 h1:B7guBgY6sfk4dBlv/ORUxyYlp0UojYaYyATgtNwSCXc= +github.com/moby/buildkit v0.12.2/go.mod h1:adB4y0SxxX8trnrY+oEulb48ODLqPO6pKMF0ppGcCoI= github.com/moby/patternmatcher v0.6.0 h1:GmP9lR19aU5GqSSFko+5pRqHi+Ohk1O69aFiKkVGiPk= github.com/moby/patternmatcher v0.6.0/go.mod h1:hDPoyOpDY7OrrMDLaYoY3hf52gNCR/YOUYxkhApJIxc= github.com/moby/sys/sequential v0.5.0 h1:OPvI35Lzn9K04PBbCLW0g4LcFAJgHsvXsRyewg5lXtc= @@ -502,6 +506,7 @@ google.golang.org/appengine v1.6.8 h1:IhEN5q69dyKagZPYMSdIjS2HqprW324FRQZJcGqPAs google.golang.org/appengine v1.6.8/go.mod h1:1jJ3jBArFh5pcgW8gCtRJnepW8FzD1V44FJffLiz/Ds= google.golang.org/protobuf v1.26.0-rc.1/go.mod h1:jlhhOSvTdKEhbULTjvd4ARK9grFBp09yW+WbY/TyQbw= google.golang.org/protobuf v1.26.0/go.mod h1:9q0QmTI4eRPtz6boOQmLYwt+qCgq0jsYwAQnmE0givc= +google.golang.org/protobuf v1.27.1/go.mod h1:9q0QmTI4eRPtz6boOQmLYwt+qCgq0jsYwAQnmE0givc= google.golang.org/protobuf v1.31.0 h1:g0LDEJHgrBl9N9r17Ru3sqWhkIx2NB67okBHPwC7hs8= google.golang.org/protobuf v1.31.0/go.mod h1:HV8QOd/L58Z+nl8r43ehVNZIU/HEI6OcFqwMG9pJV4I= gopkg.in/alecthomas/kingpin.v2 v2.2.6/go.mod h1:FMv+mEhP44yOT+4EoQTLFTRgOQ1FBLkstjWtayDeSgw= diff --git a/internal/builder/builder.go b/internal/builder/builder.go index ba405fb40c..88330845f2 100644 --- a/internal/builder/builder.go +++ b/internal/builder/builder.go @@ -103,6 +103,7 @@ type BuilderOption func(*options) error type options struct { toFlatten buildpack.FlattenModuleInfos + labels map[string]string } // FromImage constructs a builder from a builder image @@ -170,13 +171,20 @@ func constructBuilder(img imgutil.Image, newName string, errOnMissingLabel bool, return bldr, nil } -func Flatten(modules buildpack.FlattenModuleInfos) BuilderOption { +func WithFlattened(modules buildpack.FlattenModuleInfos) BuilderOption { return func(o *options) error { o.toFlatten = modules return nil } } +func WithLabels(labels map[string]string) BuilderOption { + return func(o *options) error { + o.labels = labels + return nil + } +} + func constructLifecycleDescriptor(metadata Metadata) LifecycleDescriptor { return CompatDescriptor(LifecycleDescriptor{ Info: LifecycleInfo{ diff --git a/internal/builder/builder_test.go b/internal/builder/builder_test.go index 56b4e926d8..80d39d03ff 100644 --- a/internal/builder/builder_test.go +++ b/internal/builder/builder_test.go @@ -1876,10 +1876,7 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) { when("buildpacks to be flattened are NOT 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.Flatten(flattenModules)) + bldr, err = builder.New(builderImage, "some-builder") h.AssertNil(t, err) // Let's add the buildpacks diff --git a/internal/commands/build.go b/internal/commands/build.go index 26f48f9476..9b8845d65d 100644 --- a/internal/commands/build.go +++ b/internal/commands/build.go @@ -249,7 +249,7 @@ func buildCommandFlags(cmd *cobra.Command, buildFlags *BuildFlags, cfg config.Co cmd.Flags().StringVar(&buildFlags.Network, "network", "", "Connect detect and build containers to network") cmd.Flags().StringArrayVar(&buildFlags.PreBuildpacks, "pre-buildpack", []string{}, "Buildpacks to prepend to the groups in the builder's order") cmd.Flags().StringArrayVar(&buildFlags.PostBuildpacks, "post-buildpack", []string{}, "Buildpacks to append to the groups in the builder's order") - cmd.Flags().BoolVar(&buildFlags.Publish, "publish", false, "This flag triggers the buildpack to publish the built image to a container registry after the build process is complete. This means that the resulting image is pushed to a specified registry, making it available for deployment and use by others.") + cmd.Flags().BoolVar(&buildFlags.Publish, "publish", false, "Publish the application image directly to the container registry specified in , instead of the daemon. The run image must also reside in the registry.") cmd.Flags().StringVar(&buildFlags.DockerHost, "docker-host", "", `Address to docker daemon that will be exposed to the build container. If not set (or set to empty string) the standard socket location will be used. diff --git a/internal/commands/builder_create.go b/internal/commands/builder_create.go index 879cb0ab03..0a591bfed1 100644 --- a/internal/commands/builder_create.go +++ b/internal/commands/builder_create.go @@ -23,6 +23,7 @@ type BuilderCreateFlags struct { Registry string Policy string Flatten []string + Label map[string]string } // CreateBuilder creates a builder image, based on a builder config @@ -96,6 +97,7 @@ Creating a custom builder allows you to control what buildpacks are used and wha Registry: flags.Registry, PullPolicy: pullPolicy, Flatten: toFlatten, + Labels: flags.Label, }); err != nil { return err } @@ -110,9 +112,10 @@ Creating a custom builder allows you to control what buildpacks are used and wha cmd.Flags().MarkHidden("buildpack-registry") } cmd.Flags().StringVarP(&flags.BuilderTomlPath, "config", "c", "", "Path to builder TOML file (required)") - cmd.Flags().BoolVar(&flags.Publish, "publish", false, "This flag triggers the buildpack to publish the built image to a container registry after the build process is complete. This means that the resulting image is pushed to a specified registry, making it available for deployment and use by others.") + cmd.Flags().BoolVar(&flags.Publish, "publish", false, "Publish the builder directly to the container registry specified in , 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().StringSliceVar(&flags.Flatten, "flatten", nil, "List of buildpacks to flatten together into a single layer (format: '@,@'") + cmd.Flags().StringToStringVarP(&flags.Label, "label", "l", nil, "Labels to add to the builder image, in the form of '='") AddHelpFlag(cmd, "create") return cmd diff --git a/internal/commands/buildpack_package.go b/internal/commands/buildpack_package.go index d69a9d6e40..adb95c3e03 100644 --- a/internal/commands/buildpack_package.go +++ b/internal/commands/buildpack_package.go @@ -131,7 +131,7 @@ func BuildpackPackage(logger logging.Logger, cfg config.Config, packager Buildpa cmd.Flags().StringVarP(&flags.PackageTomlPath, "config", "c", "", "Path to package TOML config") cmd.Flags().StringVarP(&flags.Format, "format", "f", "", `Format to save package as ("image" or "file")`) - cmd.Flags().BoolVar(&flags.Publish, "publish", false, `This flag triggers the buildpack to publish the built image to a container registry after the build process is complete. This means that the resulting image is pushed to a specified registry, making it available for deployment and use by others. (applies to "--format=image" only)`) + cmd.Flags().BoolVar(&flags.Publish, "publish", false, `Publish the buildpack directly to the container registry specified in , instead of the daemon (applies to "--format=image" only).`) 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().StringVarP(&flags.Path, "path", "p", "", "Path to the Buildpack that needs to be packaged") cmd.Flags().StringVarP(&flags.BuildpackRegistry, "buildpack-registry", "r", "", "Buildpack Registry name") diff --git a/internal/commands/create_builder.go b/internal/commands/create_builder.go index ff7128e3e2..9999392ef6 100644 --- a/internal/commands/create_builder.go +++ b/internal/commands/create_builder.go @@ -89,7 +89,7 @@ Creating a custom builder allows you to control what buildpacks are used and wha cmd.Flags().MarkHidden("buildpack-registry") } cmd.Flags().StringVarP(&flags.BuilderTomlPath, "config", "c", "", "Path to builder TOML file (required)") - cmd.Flags().BoolVar(&flags.Publish, "publish", false, "This flag triggers the buildpack to publish the built image to a container registry after the build process is complete. This means that the resulting image is pushed to a specified registry, making it available for deployment and use by others.") + cmd.Flags().BoolVar(&flags.Publish, "publish", false, "Publish the builder directly to the container registry specified in , 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") return cmd } diff --git a/internal/commands/extension_package.go b/internal/commands/extension_package.go index 85ad7569df..0415823e57 100644 --- a/internal/commands/extension_package.go +++ b/internal/commands/extension_package.go @@ -102,7 +102,7 @@ func ExtensionPackage(logger logging.Logger, cfg config.Config, packager Extensi // flags will be added here cmd.Flags().StringVarP(&flags.PackageTomlPath, "config", "c", "", "Path to package TOML config") cmd.Flags().StringVarP(&flags.Format, "format", "f", "", `Format to save package as ("image" or "file")`) - cmd.Flags().BoolVar(&flags.Publish, "publish", false, `This flag triggers the buildpack to publish the built image to a container registry after the build process is complete. This means that the resulting image is pushed to a specified registry, making it available for deployment and use by others. (applies to "--format=image" only)`) + cmd.Flags().BoolVar(&flags.Publish, "publish", false, `Publish the extension directly to the container registry specified in , instead of the daemon (applies to "--format=image" only).`) cmd.Flags().StringVar(&flags.Policy, "pull-policy", "", "Pull policy to use. Accepted values are always, never, and if-not-present. The default is always") AddHelpFlag(cmd, "package") return cmd diff --git a/internal/commands/package_buildpack.go b/internal/commands/package_buildpack.go index 6b1e339e9c..1bd17c4561 100644 --- a/internal/commands/package_buildpack.go +++ b/internal/commands/package_buildpack.go @@ -85,7 +85,7 @@ func PackageBuildpack(logger logging.Logger, cfg config.Config, packager Buildpa cmd.Flags().StringVarP(&flags.PackageTomlPath, "config", "c", "", "Path to package TOML config (required)") cmd.Flags().StringVarP(&flags.Format, "format", "f", "", `Format to save package as ("image" or "file")`) - cmd.Flags().BoolVar(&flags.Publish, "publish", false, `This flag triggers the buildpack to publish the built image to a container registry after the build process is complete. This means that the resulting image is pushed to a specified registry, making it available for deployment and use by others. (applies to "--format=image" only)`) + cmd.Flags().BoolVar(&flags.Publish, "publish", false, `Publish the buildpack directly to the container registry specified in , instead of the daemon (applies to "--format=image" only).`) 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().StringVarP(&flags.BuildpackRegistry, "buildpack-registry", "r", "", "Buildpack Registry name") diff --git a/internal/commands/rebase.go b/internal/commands/rebase.go index 4544724d34..d0622f9db7 100644 --- a/internal/commands/rebase.go +++ b/internal/commands/rebase.go @@ -49,7 +49,7 @@ func Rebase(logger logging.Logger, cfg config.Config, pack PackClient) *cobra.Co cmd.Flags().BoolVar(&opts.Publish, "publish", false, "Publish the rebased application image directly to the container registry specified in , instead of the daemon. The previous application image must also reside in the registry.") cmd.Flags().StringVar(&opts.RunImage, "run-image", "", "Run image to use for rebasing") cmd.Flags().StringVar(&policy, "pull-policy", "", "Pull policy to use. Accepted values are always, never, and if-not-present. The default is always") - cmd.Flags().StringVar(&opts.PreviousImage, "previous-image", "", "Image reference to use as the previous base image for rebase") + cmd.Flags().StringVar(&opts.PreviousImage, "previous-image", "", "Image to rebase. Set to a particular tag reference, digest reference, or (when performing a daemon build) image ID. Use this flag in combination with to avoid replacing the original image.") cmd.Flags().StringVar(&opts.ReportDestinationDir, "report-output-dir", "", "Path to export build report.toml.\nOmitting the flag yield no report file.") cmd.Flags().BoolVar(&opts.Force, "force", false, "Perform rebase operation without target validation (only available for API >= 0.12)") diff --git a/pkg/buildpack/build_module_info.go b/pkg/buildpack/build_module_info.go index f846cd489c..1549579cd2 100644 --- a/pkg/buildpack/build_module_info.go +++ b/pkg/buildpack/build_module_info.go @@ -49,15 +49,18 @@ func parseBuildpackName(names string) (ModuleInfos, error) { ids := strings.Split(names, ",") for _, id := range ids { if strings.Count(id, "@") != 1 { - return nil, errors.Errorf("invalid format %s; please use '@' to add buildpacks to be flatten", id) + return nil, errors.Errorf("invalid format %s; please use '@' to add buildpacks to be flattened", id) } bpFullName := strings.Split(id, "@") - if len(bpFullName) != 2 { + idFromName := strings.TrimSpace(bpFullName[0]) + versionFromName := strings.TrimSpace(bpFullName[1]) + if idFromName == "" || versionFromName == "" { return nil, errors.Errorf("invalid format %s; '' and '' must be specified", id) } + bpID := dist.ModuleInfo{ - ID: bpFullName[0], - Version: bpFullName[1], + ID: idFromName, + Version: versionFromName, } buildModuleInfos = append(buildModuleInfos, bpID) } diff --git a/pkg/buildpack/managed_collection.go b/pkg/buildpack/managed_collection.go index a4113a669b..5896caeb63 100644 --- a/pkg/buildpack/managed_collection.go +++ b/pkg/buildpack/managed_collection.go @@ -1,15 +1,15 @@ package buildpack -// ManagedCollection defines the required behavior to deal with build modules when adding then to an OCI image. +// ManagedCollection keeps track of build modules and the manner in which they should be added to an OCI image (as flattened or exploded). type ManagedCollection interface { - // AllModules returns all build modules handle by the manager + // AllModules returns all build modules handled by the manager. AllModules() []BuildModule // ExplodedModules returns all build modules that will be added to the output artifact as a single layer // containing a single module. ExplodedModules() []BuildModule - // AddModules determines whether the explodedModules must be added as flattened or not. + // AddModules adds module information to the collection as flattened or not, depending on how the collection is configured. AddModules(main BuildModule, deps ...BuildModule) // FlattenedModules returns all build modules that will be added to the output artifact as a single layer @@ -35,7 +35,9 @@ func (f *managedCollection) FlattenedModules() [][]BuildModule { func (f *managedCollection) AllModules() []BuildModule { all := f.explodedModules - all = append(all, f.flattenedModules...) + for _, modules := range f.flattenedModules { + all = append(all, modules...) + } return all } @@ -56,94 +58,6 @@ type managedCollectionV1 struct { flattenAll bool } -func NewManagedCollectionV1(flattenAll bool) ManagedCollection { - return &managedCollectionV1{ - flattenAll: flattenAll, - managedCollection: managedCollection{ - explodedModules: []BuildModule{}, - flattenedModules: [][]BuildModule{}, - }, - } -} - -func (f *managedCollectionV1) AddModules(main BuildModule, deps ...BuildModule) { - if !f.flattenAll { - // default behavior - f.explodedModules = append(f.explodedModules, append([]BuildModule{main}, deps...)...) - } else { - // flatten all - f.flattenedModules = append(f.flattenedModules, append([]BuildModule{main}, deps...)...) - } - return all -} - -func NewManagedCollectionV2(modules FlattenModuleInfos) ManagedCollection { - flattenGroups := 0 - if modules != nil { - flattenGroups = len(modules.FlattenModules()) - } - - return &managedCollectionV2{ - flattenModuleInfos: modules, - managedCollection: managedCollection{ - explodedModules: []BuildModule{}, - flattenedModules: make([][]BuildModule, flattenGroups), - }, - } -} - -// managedCollectionV2 can be used when the build modules to be flattened are known at the point of initialization. -// The flattened build modules are provided when the collection is initialized and the collection will take care of -// keeping them in the correct group (flattened or exploded) once they are added. -type managedCollectionV2 struct { - managedCollection - flattenModuleInfos FlattenModuleInfos -} - -func (ff *managedCollectionV2) flattenGroups() []ModuleInfos { - return ff.flattenModuleInfos.FlattenModules() -} - -func (ff *managedCollectionV2) AddModules(main BuildModule, deps ...BuildModule) { - var allModules []BuildModule - allModules = append(allModules, append([]BuildModule{main}, deps...)...) - for _, module := range allModules { - if ff.flattenModuleInfos != nil && len(ff.flattenGroups()) > 0 { - pos := ff.flattenedLayerFor(module) - if pos >= 0 { - ff.flattenedModules[pos] = append(ff.flattenedModules[pos], module) - } else { - // this module must not be flattened - ff.explodedModules = append(ff.explodedModules, module) - } - } else { - // we don't want to flatten anything - ff.explodedModules = append(ff.explodedModules, module) - } - } -} - -// flattenedLayerFor given a module it will try to determine to which row (group) this module must be added to in order to -// be flattened. If it is not found, it means, the module must no me flattened at all -func (ff *managedCollectionV2) flattenedLayerFor(module BuildModule) int { - // flattenModuleInfos to be flattened are representing a two-dimension array. where each row represents a group of - // flattenModuleInfos that must be flattened together in the same layer. - for i, flattenGroup := range ff.flattenGroups() { - for _, buildModuleInfo := range flattenGroup.BuildModule() { - if buildModuleInfo.FullName() == module.Descriptor().Info().FullName() { - return i - } - } - } - return -1 -} - -// managedCollectionV1 can be used to flatten all the flattenModuleInfos or none of them. -type managedCollectionV1 struct { - managedCollection - flattenAll bool -} - // NewManagedCollectionV1 will create a manager instance responsible for flattening Buildpack Packages. func NewManagedCollectionV1(flattenAll bool) ManagedCollection { return &managedCollectionV1{ diff --git a/pkg/buildpack/managed_collection_test.go b/pkg/buildpack/managed_collection_test.go index 95e94ae8ab..96357f61c5 100644 --- a/pkg/buildpack/managed_collection_test.go +++ b/pkg/buildpack/managed_collection_test.go @@ -30,7 +30,6 @@ func testModuleManager(t *testing.T, when spec.G, it spec.S) { * bp31 */ var ( -<<<<<<< HEAD moduleManager buildpack.ManagedCollection compositeBP1 buildpack.BuildModule bp1 buildpack.BuildModule @@ -41,17 +40,6 @@ func testModuleManager(t *testing.T, when spec.G, it spec.S) { bp31 buildpack.BuildModule flattenBuildModules buildpack.FlattenModuleInfos err error -======= - moduleManager buildpack.ManagedCollection - compositeBP1 buildpack.BuildModule - bp1 buildpack.BuildModule - compositeBP2 buildpack.BuildModule - bp21 buildpack.BuildModule - bp22 buildpack.BuildModule - compositeBP3 buildpack.BuildModule - bp31 buildpack.BuildModule - err error ->>>>>>> 70a41dc4 (Implementing RFC-0123) ) it.Before(func() { @@ -150,20 +138,12 @@ func testModuleManager(t *testing.T, when spec.G, it spec.S) { }) when("manager is configured in flatten mode", func() { -<<<<<<< HEAD when("V1 is used", func() { when("flatten all", func() { it.Before(func() { moduleManager = buildpack.NewManagedCollectionV1(true) moduleManager.AddModules(compositeBP1, []buildpack.BuildModule{bp1, compositeBP2, bp21, bp22, compositeBP3, bp31}...) }) -======= - when("flatten all", func() { - it.Before(func() { - moduleManager = buildpack.NewManagedCollectionV1(true) - moduleManager.AddModules(compositeBP1, []buildpack.BuildModule{bp1, compositeBP2, bp21, bp22, compositeBP3, bp31}...) - }) ->>>>>>> 0edc1a25 (adding feedback from review) when("#FlattenedModules", func() { it("returns one flatten module (1 layer)", func() { @@ -257,20 +237,9 @@ func testModuleManager(t *testing.T, when spec.G, it spec.S) { }) when("manager is not configured in flatten mode", func() { -<<<<<<< HEAD when("V1 is used", func() { it.Before(func() { moduleManager = buildpack.NewManagedCollectionV1(false) -======= - it.Before(func() { - moduleManager = buildpack.NewManagedCollectionV1(false) - }) - - when("#ExplodedModules", func() { - it("returns nil when no explodedModules are added", func() { - modules := moduleManager.ExplodedModules() - h.AssertEq(t, len(modules), 0) ->>>>>>> 0edc1a25 (adding feedback from review) }) when("#ExplodedModules", func() { diff --git a/pkg/client/create_builder.go b/pkg/client/create_builder.go index 2b452c22e3..ee1062bc4f 100644 --- a/pkg/client/create_builder.go +++ b/pkg/client/create_builder.go @@ -155,7 +155,7 @@ func (c *Client) createBaseBuilder(ctx context.Context, opts CreateBuilderOption var builderOpts []builder.BuilderOption if opts.Flatten != nil && len(opts.Flatten.FlattenModules()) > 0 { - builderOpts = append(builderOpts, builder.Flatten(opts.Flatten)) + builderOpts = append(builderOpts, builder.WithFlattened(opts.Flatten)) } if opts.Labels != nil && len(opts.Labels) > 0 { builderOpts = append(builderOpts, builder.WithLabels(opts.Labels)) diff --git a/pkg/client/create_builder_test.go b/pkg/client/create_builder_test.go index 37880b67e7..34f5bc7aa8 100644 --- a/pkg/client/create_builder_test.go +++ b/pkg/client/create_builder_test.go @@ -1083,7 +1083,6 @@ func testCreateBuilder(t *testing.T, when spec.G, it spec.S) { }) when("flatten all", func() { - var err error it("creates 1 layer for all buildpacks", func() { prepareFetcherWithRunImages() opts.Flatten, err = buildpack.ParseFlattenBuildModules([]string{"flatten/bp-1@1,flatten/bp-2@2,flatten/bp-4@4,flatten/bp-6@6,flatten/bp-7@7,flatten/bp-3@3,flatten/bp-5@5"}) @@ -1097,19 +1096,11 @@ func testCreateBuilder(t *testing.T, when spec.G, it spec.S) { }) }) -<<<<<<< HEAD - when("with exclude", func() { - it("creates 1 layer for buildpacks and 1 layer for buildpack excluded", func() { + when("only some modules are flattened", func() { + it("creates 1 layer for buildpacks [1,2,3,4,5,6] and 1 layer for buildpack [7]", func() { prepareFetcherWithRunImages() opts.Flatten, err = buildpack.ParseFlattenBuildModules([]string{"flatten/bp-1@1,flatten/bp-2@2,flatten/bp-4@4,flatten/bp-6@6,flatten/bp-3@3,flatten/bp-5@5"}) h.AssertNil(t, err) -======= - when("only some modules are flattened", func() { - it("creates 1 layer for buildpacks and 1 layer for buildpack excluded", func() { - prepareFetcherWithRunImages() - opts.Flatten, err = buildpack.ParseFlattenBuildModules([]string{"flatten/bp-1@1,flatten/bp-2@2,flatten/bp-4@4,flatten/bp-6@6,flatten/bp-3@3,flatten/bp-5@5"}) - h.AssertNil(t, err) ->>>>>>> 0edc1a25 (adding feedback from review) successfullyCreateFlattenBuilder() diff --git a/pkg/client/rebase.go b/pkg/client/rebase.go index 20fc3c060e..f493f6184a 100644 --- a/pkg/client/rebase.go +++ b/pkg/client/rebase.go @@ -47,7 +47,7 @@ type RebaseOptions struct { // validated (will not have any effect if API < 0.12). Force bool - // Tags to be applied to the rebased image. + // Image reference to use as the previous image for rebase. PreviousImage string } From d7559e24408f5c0a35dd8206a5678203d4ed2d5e Mon Sep 17 00:00:00 2001 From: Parthiba-Hazra Date: Fri, 2 Feb 2024 21:00:32 +0530 Subject: [PATCH 9/9] add test cases for new changes in Rebase function Signed-off-by: Parthiba-Hazra --- pkg/client/rebase_test.go | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/pkg/client/rebase_test.go b/pkg/client/rebase_test.go index 8068f231d0..ba6a9df86b 100644 --- a/pkg/client/rebase_test.go +++ b/pkg/client/rebase_test.go @@ -267,6 +267,38 @@ func testRebase(t *testing.T, when spec.G, it spec.S) { }) }) }) + when("previous image is provided", func() { + it("fetches the image using the previous image name", func() { + h.AssertNil(t, subject.Rebase(context.TODO(), RebaseOptions{ + RepoName: "new/app", + PreviousImage: "some/app", + })) + args := fakeImageFetcher.FetchCalls["some/app"] + h.AssertNotNil(t, args) + h.AssertEq(t, args.Daemon, true) + }) + }) + + when("previous image is set to new image name", func() { + it("returns error if Fetch function fails", func() { + err := subject.Rebase(context.TODO(), RebaseOptions{ + RepoName: "some/app", + PreviousImage: "new/app", + }) + h.AssertError(t, err, "image 'new/app' does not exist on the daemon: not found") + }) + }) + + when("previous image is not provided", func() { + it("fetches the image using the repo name", func() { + h.AssertNil(t, subject.Rebase(context.TODO(), RebaseOptions{ + RepoName: "some/app", + })) + args := fakeImageFetcher.FetchCalls["some/app"] + h.AssertNotNil(t, args) + h.AssertEq(t, args.Daemon, true) + }) + }) }) }) }