From 681c876736217d1092b1519c854aeb97ce1bdc76 Mon Sep 17 00:00:00 2001 From: Juan Bustamante Date: Thu, 25 May 2023 17:25:22 -0500 Subject: [PATCH 01/13] Moving buildpack package --flatten command to be experimental. We still need how it affects the distribution spec Signed-off-by: Juan Bustamante --- internal/commands/buildpack_package.go | 27 ++++++++++++++++----- internal/commands/buildpack_package_test.go | 20 ++++++++++++--- internal/commands/package_buildpack.go | 2 +- 3 files changed, 38 insertions(+), 11 deletions(-) diff --git a/internal/commands/buildpack_package.go b/internal/commands/buildpack_package.go index 50fe2a132d..d772ef578c 100644 --- a/internal/commands/buildpack_package.go +++ b/internal/commands/buildpack_package.go @@ -54,7 +54,7 @@ func BuildpackPackage(logger logging.Logger, cfg config.Config, packager Buildpa "and they can be included in the configs used in `pack builder create` and `pack buildpack package`. For more " + "on how to package a buildpack, see: https://buildpacks.io/docs/buildpack-author-guide/package-a-buildpack/.", RunE: logError(logger, func(cmd *cobra.Command, args []string) error { - if err := validateBuildpackPackageFlags(&flags); err != nil { + if err := validateBuildpackPackageFlags(cfg, &flags); err != nil { return err } @@ -96,6 +96,10 @@ func BuildpackPackage(logger logging.Logger, cfg config.Config, packager Buildpa logger.Warnf("%s is not a valid extension for a packaged buildpack. Packaged buildpacks must have a %s extension", style.Symbol(ext), style.Symbol(client.CNBExtension)) } } + if flags.Flatten { + logger.Warn("Creating a flattened Buildpack package could break the distribution specification. Please use it with caution.") + } + if err := packager.PackageBuildpack(cmd.Context(), client.PackageBuildpackOptions{ RelativeBaseDir: relativeBaseDir, Name: name, @@ -134,11 +138,16 @@ func BuildpackPackage(logger logging.Logger, cfg config.Config, packager Buildpa cmd.Flags().BoolVar(&flags.Flatten, "flatten", false, "Flatten the buildpack into a single layer") cmd.Flags().StringSliceVarP(&flags.FlattenExclude, "flatten-exclude", "e", nil, "Buildpacks to exclude from flattening, in the form of '@'") cmd.Flags().IntVar(&flags.Depth, "depth", -1, "Max depth to flatten.\nOmission of this flag or values < 0 will flatten the entire tree.") + if !cfg.Experimental { + cmd.Flags().MarkHidden("flatten") + cmd.Flags().MarkHidden("depth") + cmd.Flags().MarkHidden("flatten-exclude") + } AddHelpFlag(cmd, "package") return cmd } -func validateBuildpackPackageFlags(p *BuildpackPackageFlags) error { +func validateBuildpackPackageFlags(cfg config.Config, p *BuildpackPackageFlags) error { if p.Publish && p.Policy == image.PullNever.String() { return errors.Errorf("--publish and --pull-policy never cannot be used together. The --publish flag requires the use of remote images.") } @@ -146,10 +155,16 @@ func validateBuildpackPackageFlags(p *BuildpackPackageFlags) error { return errors.Errorf("--config and --path cannot be used together. Please specify the relative path to the Buildpack directory in the package config file.") } - if p.Flatten && len(p.FlattenExclude) > 0 { - for _, exclude := range p.FlattenExclude { - if strings.Count(exclude, "@") != 1 { - return errors.Errorf("invalid format %s; please use '@' to exclude buildpack from flattening", exclude) + if p.Flatten { + if !cfg.Experimental { + return client.NewExperimentError("Flattening a buildpack package currently experimental.") + } + + if len(p.FlattenExclude) > 0 { + for _, exclude := range p.FlattenExclude { + if strings.Count(exclude, "@") != 1 { + return errors.Errorf("invalid format %s; please use '@' to exclude buildpack from flattening", exclude) + } } } } diff --git a/internal/commands/buildpack_package_test.go b/internal/commands/buildpack_package_test.go index dded63bf95..6b0c0babe5 100644 --- a/internal/commands/buildpack_package_test.go +++ b/internal/commands/buildpack_package_test.go @@ -121,13 +121,25 @@ func testPackageCommand(t *testing.T, when spec.G, it spec.S) { }) }) when("flatten is set to true", func() { - when("flatten exclude doesn't have format @", func() { + when("experimental is true", func() { + when("flatten exclude doesn't have format @", func() { + it("errors with a descriptive message", func() { + cmd := packageCommand(withClientConfig(config.Config{Experimental: true}), withBuildpackPackager(fakeBuildpackPackager)) + cmd.SetArgs([]string{"test", "-f", "file", "--flatten", "--flatten-exclude", "some-buildpack"}) + + err := cmd.Execute() + h.AssertError(t, err, fmt.Sprintf("invalid format %s; please use '@' to exclude buildpack from flattening", "some-buildpack")) + }) + }) + }) + + when("experimental is false", func() { it("errors with a descriptive message", func() { - cmd := packageCommand(withBuildpackPackager(fakeBuildpackPackager)) - cmd.SetArgs([]string{"test", "-f", "file", "--flatten", "--flatten-exclude", "some-buildpack"}) + cmd := packageCommand(withClientConfig(config.Config{Experimental: false}), withBuildpackPackager(fakeBuildpackPackager)) + cmd.SetArgs([]string{"test", "-f", "file", "--flatten"}) err := cmd.Execute() - h.AssertError(t, err, fmt.Sprintf("invalid format %s; please use '@' to exclude buildpack from flattening", "some-buildpack")) + h.AssertError(t, err, "Flattening a buildpack package currently experimental.") }) }) }) diff --git a/internal/commands/package_buildpack.go b/internal/commands/package_buildpack.go index c0dfac2609..e251833361 100644 --- a/internal/commands/package_buildpack.go +++ b/internal/commands/package_buildpack.go @@ -33,7 +33,7 @@ func PackageBuildpack(logger logging.Logger, cfg config.Config, packager Buildpa RunE: logError(logger, func(cmd *cobra.Command, args []string) error { deprecationWarning(logger, "package-buildpack", "buildpack package") - if err := validateBuildpackPackageFlags(&flags); err != nil { + if err := validateBuildpackPackageFlags(cfg, &flags); err != nil { return err } From d4545c7a585570598ea70d92a0d9095db020188e Mon Sep 17 00:00:00 2001 From: Juan Bustamante Date: Fri, 26 May 2023 08:45:47 -0500 Subject: [PATCH 02/13] Apply suggestions from code review Co-authored-by: Natalie Arellano Signed-off-by: Juan Bustamante --- internal/commands/buildpack_package.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/commands/buildpack_package.go b/internal/commands/buildpack_package.go index d772ef578c..f33107a633 100644 --- a/internal/commands/buildpack_package.go +++ b/internal/commands/buildpack_package.go @@ -97,7 +97,7 @@ func BuildpackPackage(logger logging.Logger, cfg config.Config, packager Buildpa } } if flags.Flatten { - logger.Warn("Creating a flattened Buildpack package could break the distribution specification. Please use it with caution.") + logger.Warn("Flattening a buildpack package could break the distribution specification. Please use it with caution.") } if err := packager.PackageBuildpack(cmd.Context(), client.PackageBuildpackOptions{ @@ -157,7 +157,7 @@ func validateBuildpackPackageFlags(cfg config.Config, p *BuildpackPackageFlags) if p.Flatten { if !cfg.Experimental { - return client.NewExperimentError("Flattening a buildpack package currently experimental.") + return client.NewExperimentError("Flattening a buildpack package is currently experimental.") } if len(p.FlattenExclude) > 0 { From 2e1b3a52343ba9ddde0976a14c4e77d21f882e3f Mon Sep 17 00:00:00 2001 From: Natalie Arellano Date: Tue, 30 May 2023 17:09:00 -0400 Subject: [PATCH 03/13] When deriving stack.toml from the new run image information 2 we should write the file with the correct schema Signed-off-by: Natalie Arellano --- internal/builder/builder.go | 2 +- internal/builder/builder_test.go | 12 ++++++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/internal/builder/builder.go b/internal/builder/builder.go index 7b66ad891b..a69f7d8021 100644 --- a/internal/builder/builder.go +++ b/internal/builder/builder.go @@ -1083,7 +1083,7 @@ func (b *Builder) stackLayer(dest string) (string, error) { if b.metadata.Stack.RunImage.Image != "" { err = toml.NewEncoder(buf).Encode(b.metadata.Stack) } else if len(b.metadata.RunImages) > 0 { - err = toml.NewEncoder(buf).Encode(b.metadata.RunImages[0]) + err = toml.NewEncoder(buf).Encode(StackMetadata{RunImage: b.metadata.RunImages[0]}) } if err != nil { return "", errors.Wrapf(err, "failed to marshal stack.toml") diff --git a/internal/builder/builder_test.go b/internal/builder/builder_test.go index 26ad0c144e..69cf7947da 100644 --- a/internal/builder/builder_test.go +++ b/internal/builder/builder_test.go @@ -1583,6 +1583,18 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) { ) }) + it("adds the stack.toml to the image", func() { + layerTar, err := baseImage.FindLayerWithPath("/cnb/stack.toml") + h.AssertNil(t, err) + h.AssertOnTarEntry(t, layerTar, "/cnb/stack.toml", + h.ContentEquals(`[run-image] + image = "some/run" + mirrors = ["some/mirror", "other/mirror"] +`), + h.HasModTime(archive.NormalizedDateTime), + ) + }) + it("adds the run image to the metadata", func() { label, err := baseImage.Label("io.buildpacks.builder.metadata") h.AssertNil(t, err) From 63d15924cd5ed46acd859d3ed1ee50fb89c8dcf8 Mon Sep 17 00:00:00 2001 From: Natalie Arellano Date: Tue, 30 May 2023 17:54:32 -0400 Subject: [PATCH 04/13] Create new volume cache for kaniko instead of re-using build cache This removes the unnecessary restriction that the build cache must be a volume cache in order to use extensions. Signed-off-by: Natalie Arellano --- internal/build/lifecycle_execution.go | 49 +++++++--------------- internal/build/lifecycle_execution_test.go | 35 ++++++++++------ pkg/cache/cache_opts.go | 30 ++++++------- 3 files changed, 53 insertions(+), 61 deletions(-) diff --git a/internal/build/lifecycle_execution.go b/internal/build/lifecycle_execution.go index 485508aa60..5608839809 100644 --- a/internal/build/lifecycle_execution.go +++ b/internal/build/lifecycle_execution.go @@ -195,6 +195,7 @@ func (l *LifecycleExecution) Run(ctx context.Context, phaseFactoryCreator PhaseF } launchCache := cache.NewVolumeCache(l.opts.Image, l.opts.Cache.Launch, "launch", l.docker) + kanikoCache := cache.NewVolumeCache(l.opts.Image, l.opts.Cache.Kaniko, "kaniko", l.docker) if !l.opts.UseCreator { if l.platformAPI.LessThan("0.7") { @@ -222,7 +223,7 @@ func (l *LifecycleExecution) Run(ctx context.Context, phaseFactoryCreator PhaseF l.logger.Info(style.Step("RESTORING")) if l.opts.ClearCache && l.PlatformAPI().LessThan("0.10") { l.logger.Info("Skipping 'restore' due to clearing cache") - } else if err := l.Restore(ctx, buildCache, phaseFactory); err != nil { + } else if err := l.Restore(ctx, buildCache, kanikoCache, phaseFactory); err != nil { return err } @@ -230,7 +231,7 @@ func (l *LifecycleExecution) Run(ctx context.Context, phaseFactoryCreator PhaseF if l.platformAPI.AtLeast("0.10") && l.hasExtensionsForBuild() { group.Go(func() error { l.logger.Info(style.Step("EXTENDING (BUILD)")) - return l.ExtendBuild(ctx, buildCache, phaseFactory) + return l.ExtendBuild(ctx, kanikoCache, phaseFactory) }) } else { group.Go(func() error { @@ -249,7 +250,7 @@ func (l *LifecycleExecution) Run(ctx context.Context, phaseFactoryCreator PhaseF if l.platformAPI.AtLeast("0.12") && l.hasExtensionsForRun() { group.Go(func() error { l.logger.Info(style.Step("EXTENDING (RUN)")) - return l.ExtendRun(ctx, buildCache, phaseFactory) + return l.ExtendRun(ctx, kanikoCache, phaseFactory) }) } @@ -258,7 +259,7 @@ func (l *LifecycleExecution) Run(ctx context.Context, phaseFactoryCreator PhaseF } l.logger.Info(style.Step("EXPORTING")) - return l.Export(ctx, buildCache, launchCache, phaseFactory) + return l.Export(ctx, buildCache, launchCache, kanikoCache, phaseFactory) } if l.platformAPI.AtLeast("0.10") && l.hasExtensions() { @@ -421,7 +422,7 @@ func (l *LifecycleExecution) Detect(ctx context.Context, phaseFactory PhaseFacto return detect.Run(ctx) } -func (l *LifecycleExecution) Restore(ctx context.Context, buildCache Cache, phaseFactory PhaseFactory) error { +func (l *LifecycleExecution) Restore(ctx context.Context, buildCache Cache, kanikoCache Cache, phaseFactory PhaseFactory) error { // build up flags and ops var flags []string if l.opts.ClearCache { @@ -454,12 +455,7 @@ func (l *LifecycleExecution) Restore(ctx context.Context, buildCache Cache, phas registryImages = append(registryImages, l.opts.BuilderImage) } - switch buildCache.Type() { - case cache.Volume: - kanikoCacheBindOp = WithBinds(fmt.Sprintf("%s:%s", buildCache.Name(), l.mountPaths.kanikoCacheDir())) - default: - return fmt.Errorf("build cache must be volume cache when building with extensions") - } + kanikoCacheBindOp = WithBinds(fmt.Sprintf("%s:%s", kanikoCache.Name(), l.mountPaths.kanikoCacheDir())) } // for auths @@ -645,18 +641,9 @@ func (l *LifecycleExecution) Build(ctx context.Context, phaseFactory PhaseFactor return build.Run(ctx) } -func (l *LifecycleExecution) ExtendBuild(ctx context.Context, buildCache Cache, phaseFactory PhaseFactory) error { +func (l *LifecycleExecution) ExtendBuild(ctx context.Context, kanikoCache Cache, phaseFactory PhaseFactory) error { flags := []string{"-app", l.mountPaths.appDir()} - // set kaniko cache opt - var kanikoCacheBindOp PhaseConfigProviderOperation - switch buildCache.Type() { - case cache.Volume: - kanikoCacheBindOp = WithBinds(fmt.Sprintf("%s:%s", buildCache.Name(), l.mountPaths.kanikoCacheDir())) - default: - return fmt.Errorf("build cache must be volume cache when building with extensions") - } - configProvider := NewPhaseConfigProvider( "extender", l, @@ -667,7 +654,7 @@ func (l *LifecycleExecution) ExtendBuild(ctx context.Context, buildCache Cache, WithFlags(flags...), WithNetwork(l.opts.Network), WithRoot(), - kanikoCacheBindOp, + WithBinds(fmt.Sprintf("%s:%s", kanikoCache.Name(), l.mountPaths.kanikoCacheDir())), ) extend := phaseFactory.New(configProvider) @@ -675,18 +662,9 @@ func (l *LifecycleExecution) ExtendBuild(ctx context.Context, buildCache Cache, return extend.Run(ctx) } -func (l *LifecycleExecution) ExtendRun(ctx context.Context, buildCache Cache, phaseFactory PhaseFactory) error { +func (l *LifecycleExecution) ExtendRun(ctx context.Context, kanikoCache Cache, phaseFactory PhaseFactory) error { flags := []string{"-app", l.mountPaths.appDir(), "-kind", "run"} - // set kaniko cache opt - var kanikoCacheBindOp PhaseConfigProviderOperation - switch buildCache.Type() { - case cache.Volume: - kanikoCacheBindOp = WithBinds(fmt.Sprintf("%s:%s", buildCache.Name(), l.mountPaths.kanikoCacheDir())) - default: - return fmt.Errorf("build cache must be volume cache when building with extensions") - } - configProvider := NewPhaseConfigProvider( "extender", l, @@ -699,7 +677,7 @@ func (l *LifecycleExecution) ExtendRun(ctx context.Context, buildCache Cache, ph WithRoot(), WithImage(l.runImageAfterExtensions()), WithBinds(fmt.Sprintf("%s:%s", filepath.Join(l.tmpDir, "cnb"), l.mountPaths.cnbDir())), - kanikoCacheBindOp, + WithBinds(fmt.Sprintf("%s:%s", kanikoCache.Name(), l.mountPaths.kanikoCacheDir())), ) extend := phaseFactory.New(configProvider) @@ -717,19 +695,21 @@ func determineDefaultProcessType(platformAPI *api.Version, providedValue string) return providedValue } -func (l *LifecycleExecution) Export(ctx context.Context, buildCache, launchCache Cache, phaseFactory PhaseFactory) error { +func (l *LifecycleExecution) Export(ctx context.Context, buildCache, launchCache, kanikoCache Cache, phaseFactory PhaseFactory) error { flags := []string{ "-app", l.mountPaths.appDir(), "-cache-dir", l.mountPaths.cacheDir(), } expEnv := NullOp() + kanikoCacheBindOp := NullOp() if l.platformAPI.LessThan("0.12") { flags = append(flags, "-stack", l.mountPaths.stackPath()) } else { flags = append(flags, "-run", l.mountPaths.runPath()) if l.hasExtensionsForRun() { expEnv = WithEnv("CNB_EXPERIMENTAL_MODE=warn") + kanikoCacheBindOp = WithBinds(fmt.Sprintf("%s:%s", kanikoCache.Name(), l.mountPaths.kanikoCacheDir())) } } @@ -771,6 +751,7 @@ func (l *LifecycleExecution) Export(ctx context.Context, buildCache, launchCache WithRoot(), WithNetwork(l.opts.Network), cacheBindOp, + kanikoCacheBindOp, WithContainerOperations(WriteStackToml(l.mountPaths.stackPath(), l.opts.Builder.Stack(), l.os)), WithContainerOperations(WriteRunToml(l.mountPaths.runPath(), l.opts.Builder.RunImages(), l.os)), WithContainerOperations(WriteProjectMetadata(l.mountPaths.projectPath(), l.opts.ProjectMetadata, l.os)), diff --git a/internal/build/lifecycle_execution_test.go b/internal/build/lifecycle_execution_test.go index 3a10d241e5..5ac97f2e7e 100644 --- a/internal/build/lifecycle_execution_test.go +++ b/internal/build/lifecycle_execution_test.go @@ -69,6 +69,7 @@ func testLifecycleExecution(t *testing.T, when spec.G, it spec.S) { lifecycle *build.LifecycleExecution fakeBuildCache = newFakeVolumeCache() fakeLaunchCache *fakes.FakeCache + fakeKanikoCache *fakes.FakeCache fakePhase *fakes.FakePhase fakePhaseFactory *fakes.FakePhaseFactory fakeFetcher fakeImageFetcher @@ -164,6 +165,10 @@ func testLifecycleExecution(t *testing.T, when spec.G, it spec.S) { fakeLaunchCache.ReturnForType = cache.Volume fakeLaunchCache.ReturnForName = "some-launch-cache" + fakeKanikoCache = fakes.NewFakeCache() + fakeKanikoCache.ReturnForType = cache.Volume + fakeKanikoCache.ReturnForName = "some-kaniko-cache" + fakePhase = &fakes.FakePhase{} fakePhaseFactory = fakes.NewFakePhaseFactory(fakes.WhichReturnsForNew(fakePhase)) }) @@ -1637,7 +1642,7 @@ func testLifecycleExecution(t *testing.T, when spec.G, it spec.S) { when("#Restore", func() { it.Before(func() { - err := lifecycle.Restore(context.Background(), fakeBuildCache, fakePhaseFactory) + err := lifecycle.Restore(context.Background(), fakeBuildCache, fakeKanikoCache, fakePhaseFactory) h.AssertNil(t, err) lastCallIndex := len(fakePhaseFactory.NewCalledWithProvider) - 1 @@ -1749,7 +1754,7 @@ func testLifecycleExecution(t *testing.T, when spec.G, it spec.S) { it("does not provide -build-image or /kaniko bind", func() { h.AssertSliceNotContains(t, configProvider.ContainerConfig().Cmd, "-build-image") - h.AssertSliceNotContains(t, configProvider.HostConfig().Binds, "some-cache:/kaniko") + h.AssertSliceNotContains(t, configProvider.HostConfig().Binds, "some-kaniko-cache:/kaniko") }) }) @@ -1759,7 +1764,7 @@ func testLifecycleExecution(t *testing.T, when spec.G, it spec.S) { it("provides -build-image and /kaniko bind", func() { h.AssertSliceContainsInOrder(t, configProvider.ContainerConfig().Cmd, "-build-image", providedBuilderImage) h.AssertSliceContains(t, configProvider.ContainerConfig().Env, "CNB_REGISTRY_AUTH={}") - h.AssertSliceContains(t, configProvider.HostConfig().Binds, "some-cache:/kaniko") + h.AssertSliceContains(t, configProvider.HostConfig().Binds, "some-kaniko-cache:/kaniko") }) }) }) @@ -1769,7 +1774,7 @@ func testLifecycleExecution(t *testing.T, when spec.G, it spec.S) { it("does not provide -build-image or /kaniko bind", func() { h.AssertSliceNotContains(t, configProvider.ContainerConfig().Cmd, "-build-image") - h.AssertSliceNotContains(t, configProvider.HostConfig().Binds, "some-cache:/kaniko") + h.AssertSliceNotContains(t, configProvider.HostConfig().Binds, "some-kaniko-cache:/kaniko") }) }) }) @@ -1783,7 +1788,7 @@ func testLifecycleExecution(t *testing.T, when spec.G, it spec.S) { platformAPI = api.MustParse("0.12") it("provides /kaniko bind", func() { - h.AssertSliceContains(t, configProvider.HostConfig().Binds, "some-cache:/kaniko") + h.AssertSliceContains(t, configProvider.HostConfig().Binds, "some-kaniko-cache:/kaniko") }) }) @@ -1791,7 +1796,7 @@ func testLifecycleExecution(t *testing.T, when spec.G, it spec.S) { platformAPI = api.MustParse("0.11") it("does not provide /kaniko bind", func() { - h.AssertSliceNotContains(t, configProvider.HostConfig().Binds, "some-cache:/kaniko") + h.AssertSliceNotContains(t, configProvider.HostConfig().Binds, "some-kaniko-cache:/kaniko") }) }) }) @@ -1800,7 +1805,7 @@ func testLifecycleExecution(t *testing.T, when spec.G, it spec.S) { platformAPI = api.MustParse("0.12") it("does not provide /kaniko bind", func() { - h.AssertSliceNotContains(t, configProvider.HostConfig().Binds, "some-cache:/kaniko") + h.AssertSliceNotContains(t, configProvider.HostConfig().Binds, "some-kaniko-cache:/kaniko") }) }) }) @@ -1843,7 +1848,7 @@ func testLifecycleExecution(t *testing.T, when spec.G, it spec.S) { when("#ExtendBuild", func() { it.Before(func() { - err := lifecycle.ExtendBuild(context.Background(), fakeBuildCache, fakePhaseFactory) + err := lifecycle.ExtendBuild(context.Background(), fakeKanikoCache, fakePhaseFactory) h.AssertNil(t, err) lastCallIndex := len(fakePhaseFactory.NewCalledWithProvider) - 1 @@ -1865,7 +1870,7 @@ func testLifecycleExecution(t *testing.T, when spec.G, it spec.S) { it("configures the phase with binds", func() { expectedBinds := providedVolumes - expectedBinds = append(expectedBinds, "some-cache:/kaniko") + expectedBinds = append(expectedBinds, "some-kaniko-cache:/kaniko") h.AssertSliceContains(t, configProvider.HostConfig().Binds, expectedBinds...) }) @@ -1885,7 +1890,7 @@ func testLifecycleExecution(t *testing.T, when spec.G, it spec.S) { when("#ExtendRun", func() { it.Before(func() { - err := lifecycle.ExtendRun(context.Background(), fakeBuildCache, fakePhaseFactory) + err := lifecycle.ExtendRun(context.Background(), fakeKanikoCache, fakePhaseFactory) h.AssertNil(t, err) lastCallIndex := len(fakePhaseFactory.NewCalledWithProvider) - 1 @@ -1921,7 +1926,7 @@ func testLifecycleExecution(t *testing.T, when spec.G, it spec.S) { it("configures the phase with binds", func() { expectedBinds := providedVolumes - expectedBinds = append(expectedBinds, "some-cache:/kaniko", fmt.Sprintf("%s:/cnb", filepath.Join(tmpDir, "cnb"))) + expectedBinds = append(expectedBinds, "some-kaniko-cache:/kaniko", fmt.Sprintf("%s:/cnb", filepath.Join(tmpDir, "cnb"))) h.AssertSliceContains(t, configProvider.HostConfig().Binds, expectedBinds...) }) @@ -1941,7 +1946,7 @@ func testLifecycleExecution(t *testing.T, when spec.G, it spec.S) { when("#Export", func() { it.Before(func() { - err := lifecycle.Export(context.Background(), fakeBuildCache, fakeLaunchCache, fakePhaseFactory) + err := lifecycle.Export(context.Background(), fakeBuildCache, fakeLaunchCache, fakeKanikoCache, fakePhaseFactory) h.AssertNil(t, err) lastCallIndex := len(fakePhaseFactory.NewCalledWithProvider) - 1 @@ -1986,6 +1991,12 @@ func testLifecycleExecution(t *testing.T, when spec.G, it spec.S) { it("sets CNB_EXPERIMENTAL_MODE=warn in the environment", func() { h.AssertSliceContains(t, configProvider.ContainerConfig().Env, "CNB_EXPERIMENTAL_MODE=warn") }) + + it("configures the phase with binds", func() { + expectedBinds := []string{"some-cache:/cache", "some-launch-cache:/launch-cache", "some-kaniko-cache:/kaniko"} + + h.AssertSliceContains(t, configProvider.HostConfig().Binds, expectedBinds...) + }) }) }) }) diff --git a/pkg/cache/cache_opts.go b/pkg/cache/cache_opts.go index d95c4a8854..07e0f57fd6 100644 --- a/pkg/cache/cache_opts.go +++ b/pkg/cache/cache_opts.go @@ -18,6 +18,7 @@ type CacheInfo struct { type CacheOpts struct { Build CacheInfo Launch CacheInfo + Kaniko CacheInfo } const ( @@ -139,22 +140,21 @@ func sanitize(c *CacheOpts) error { } } - if c.Build.Format == CacheBind || c.Launch.Format == CacheBind { - var ( - resolvedPath string - err error - ) - if c.Build.Format == CacheBind { - if resolvedPath, err = filepath.Abs(c.Build.Source); err != nil { - return errors.Wrap(err, "resolve absolute path") - } - c.Build.Source = filepath.Join(resolvedPath, "build-cache") - } else { - if resolvedPath, err = filepath.Abs(c.Launch.Source); err != nil { - return errors.Wrap(err, "resolve absolute path") - } - c.Launch.Source = filepath.Join(resolvedPath, "launch-cache") + var ( + resolvedPath string + err error + ) + if c.Build.Format == CacheBind { + if resolvedPath, err = filepath.Abs(c.Build.Source); err != nil { + return errors.Wrap(err, "resolve absolute path") + } + c.Build.Source = filepath.Join(resolvedPath, "build-cache") + } + if c.Launch.Format == CacheBind { + if resolvedPath, err = filepath.Abs(c.Launch.Source); err != nil { + return errors.Wrap(err, "resolve absolute path") } + c.Launch.Source = filepath.Join(resolvedPath, "launch-cache") } return nil } From 37747efaf195582f76037616337cdd338a0515e0 Mon Sep 17 00:00:00 2001 From: Juan Bustamante Date: Fri, 26 May 2023 08:46:46 -0500 Subject: [PATCH 05/13] Fixing unit test after updating the error message Adding test case for warning message Signed-off-by: Juan Bustamante --- internal/commands/buildpack_package_test.go | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/internal/commands/buildpack_package_test.go b/internal/commands/buildpack_package_test.go index 6b0c0babe5..838c7a6dd3 100644 --- a/internal/commands/buildpack_package_test.go +++ b/internal/commands/buildpack_package_test.go @@ -131,6 +131,23 @@ func testPackageCommand(t *testing.T, when spec.G, it spec.S) { h.AssertError(t, err, fmt.Sprintf("invalid format %s; please use '@' to exclude buildpack from flattening", "some-buildpack")) }) }) + + when("no exclusions", func() { + it("creates package with correct image name and warns flatten is being used", func() { + cmd := packageCommand( + withClientConfig(config.Config{Experimental: true}), + withBuildpackPackager(fakeBuildpackPackager), + withLogger(logger), + ) + cmd.SetArgs([]string{"my-flatten-image", "-f", "file", "--flatten"}) + err := cmd.Execute() + h.AssertNil(t, err) + + receivedOptions := fakeBuildpackPackager.CreateCalledWithOptions + h.AssertEq(t, receivedOptions.Name, "my-flatten-image.cnb") + h.AssertContains(t, outBuf.String(), "Flattening a buildpack package could break the distribution specification. Please use it with caution.") + }) + }) }) when("experimental is false", func() { @@ -139,7 +156,7 @@ func testPackageCommand(t *testing.T, when spec.G, it spec.S) { cmd.SetArgs([]string{"test", "-f", "file", "--flatten"}) err := cmd.Execute() - h.AssertError(t, err, "Flattening a buildpack package currently experimental.") + h.AssertError(t, err, "Flattening a buildpack package is currently experimental.") }) }) }) From 7325122315df8850b785e7d5bd478b5fd86c960f Mon Sep 17 00:00:00 2001 From: Joe Kimmel Date: Wed, 7 Jun 2023 15:59:55 -0700 Subject: [PATCH 06/13] only add the stack arg to lifecycle for older platform APIs Signed-off-by: Joe Kimmel --- internal/build/lifecycle_execution.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/internal/build/lifecycle_execution.go b/internal/build/lifecycle_execution.go index 485508aa60..527776a1ff 100644 --- a/internal/build/lifecycle_execution.go +++ b/internal/build/lifecycle_execution.go @@ -569,8 +569,10 @@ func (l *LifecycleExecution) Analyze(ctx context.Context, buildCache, launchCach if l.opts.RunImage != "" { args = append([]string{"-run-image", l.opts.RunImage}, args...) } - args = append([]string{"-stack", l.mountPaths.stackPath()}, args...) - stackOp = WithContainerOperations(WriteStackToml(l.mountPaths.stackPath(), l.opts.Builder.Stack(), l.os)) + if l.platformAPI.LessThan("0.12") { + args = append([]string{"-stack", l.mountPaths.stackPath()}, args...) + stackOp = WithContainerOperations(WriteStackToml(l.mountPaths.stackPath(), l.opts.Builder.Stack(), l.os)) + } runOp = WithContainerOperations(WriteRunToml(l.mountPaths.runPath(), l.opts.Builder.RunImages(), l.os)) } From 57b70cd8abc63363c9b0ca1453e12c6bbc628795 Mon Sep 17 00:00:00 2001 From: Joe Kimmel Date: Thu, 8 Jun 2023 09:29:59 -0700 Subject: [PATCH 07/13] either add -stack or -run flags when invoking lifecycle (with @natalieparellano) Signed-off-by: Joe Kimmel --- internal/build/lifecycle_execution.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/internal/build/lifecycle_execution.go b/internal/build/lifecycle_execution.go index 527776a1ff..6a75cf45f4 100644 --- a/internal/build/lifecycle_execution.go +++ b/internal/build/lifecycle_execution.go @@ -572,8 +572,10 @@ func (l *LifecycleExecution) Analyze(ctx context.Context, buildCache, launchCach if l.platformAPI.LessThan("0.12") { args = append([]string{"-stack", l.mountPaths.stackPath()}, args...) stackOp = WithContainerOperations(WriteStackToml(l.mountPaths.stackPath(), l.opts.Builder.Stack(), l.os)) + } else { + args = append([]string{"-run", l.mountPaths.runPath()}, args...) + runOp = WithContainerOperations(WriteRunToml(l.mountPaths.runPath(), l.opts.Builder.RunImages(), l.os)) } - runOp = WithContainerOperations(WriteRunToml(l.mountPaths.runPath(), l.opts.Builder.RunImages(), l.os)) } flagsOp := WithFlags(flags...) From c84939bfd7a3fd9a94370442d384d16ea46bab23 Mon Sep 17 00:00:00 2001 From: Natalie Arellano Date: Thu, 15 Jun 2023 12:10:47 -0400 Subject: [PATCH 08/13] Fix small issue with deprecated BOM display Extensions don't output legacy BOMs so including them in the BOM display is a bit misleading. Signed-off-by: Natalie Arellano --- internal/inspectimage/bom_display.go | 8 -------- internal/inspectimage/writer/bom_json_test.go | 2 -- internal/inspectimage/writer/bom_yaml_test.go | 2 -- 3 files changed, 12 deletions(-) diff --git a/internal/inspectimage/bom_display.go b/internal/inspectimage/bom_display.go index e8a85c34e2..28e273288d 100644 --- a/internal/inspectimage/bom_display.go +++ b/internal/inspectimage/bom_display.go @@ -19,7 +19,6 @@ type BOMEntryDisplay struct { Version string `toml:"version,omitempty" json:"version,omitempty" yaml:"version,omitempty"` Metadata map[string]interface{} `toml:"metadata" json:"metadata" yaml:"metadata"` Buildpack dist.ModuleRef `json:"buildpacks" yaml:"buildpacks" toml:"buildpacks"` - Extension dist.ModuleRef `json:"extensions" yaml:"extensions" toml:"extensions"` } func NewBOMDisplay(info *client.ImageInfo) []BOMEntryDisplay { @@ -68,13 +67,6 @@ func displayBOMWithExtension(bom []buildpack.BOMEntry) []BOMEntryDisplay { }, Optional: entry.Buildpack.Optional, }, - Extension: dist.ModuleRef{ - ModuleInfo: dist.ModuleInfo{ - ID: entry.Buildpack.ID, - Version: entry.Buildpack.Version, - }, - Optional: entry.Buildpack.Optional, - }, }) } diff --git a/internal/inspectimage/writer/bom_json_test.go b/internal/inspectimage/writer/bom_json_test.go index 9ff336333c..5e0b8f8fb0 100644 --- a/internal/inspectimage/writer/bom_json_test.go +++ b/internal/inspectimage/writer/bom_json_test.go @@ -45,7 +45,6 @@ func testJSONBOM(t *testing.T, when spec.G, it spec.S) { } } }, - "extensions": {}, "buildpacks": { "id": "test.bp.one.remote", "version": "1.0.0" @@ -68,7 +67,6 @@ func testJSONBOM(t *testing.T, when spec.G, it spec.S) { } } }, - "extensions": {}, "buildpacks": { "id": "test.bp.one.remote", "version": "1.0.0" diff --git a/internal/inspectimage/writer/bom_yaml_test.go b/internal/inspectimage/writer/bom_yaml_test.go index 4f7ae982cf..855d065166 100644 --- a/internal/inspectimage/writer/bom_yaml_test.go +++ b/internal/inspectimage/writer/bom_yaml_test.go @@ -41,7 +41,6 @@ local: int: 456 nested: string: '' - extensions: {} buildpacks: id: test.bp.one.remote version: 1.0.0 @@ -57,7 +56,6 @@ remote: int: 123 nested: string: anotherString - extensions: {} buildpacks: id: test.bp.one.remote version: 1.0.0 From 3be39f99a9ce42855c2d66f89463389d7b744cdf Mon Sep 17 00:00:00 2001 From: Natalie Arellano Date: Thu, 15 Jun 2023 16:30:31 -0400 Subject: [PATCH 09/13] Fix acceptance Signed-off-by: Natalie Arellano --- acceptance/testdata/pack_fixtures/report_output.txt | 2 +- internal/builder/lifecycle.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/acceptance/testdata/pack_fixtures/report_output.txt b/acceptance/testdata/pack_fixtures/report_output.txt index d3a0b650ba..0222943d76 100644 --- a/acceptance/testdata/pack_fixtures/report_output.txt +++ b/acceptance/testdata/pack_fixtures/report_output.txt @@ -2,7 +2,7 @@ Pack: Version: {{ .Version }} OS/Arch: {{ .OS }}/{{ .Arch }} -Default Lifecycle Version: 0.17.0-pre.2 +Default Lifecycle Version: 0.17.0-rc.1 Supported Platform APIs: 0.3, 0.4, 0.5, 0.6, 0.7, 0.8, 0.9, 0.10, 0.11, 0.12 diff --git a/internal/builder/lifecycle.go b/internal/builder/lifecycle.go index 5b33533674..001d3ea107 100644 --- a/internal/builder/lifecycle.go +++ b/internal/builder/lifecycle.go @@ -14,7 +14,7 @@ import ( // A snapshot of the latest tested lifecycle version values const ( - DefaultLifecycleVersion = "0.17.0-pre.2" + DefaultLifecycleVersion = "0.17.0-rc.1" DefaultBuildpackAPIVersion = "0.2" ) From c965586df803dbfeaa6fca95ba00468914f8c4c0 Mon Sep 17 00:00:00 2001 From: Natalie Arellano Date: Thu, 15 Jun 2023 17:08:42 -0400 Subject: [PATCH 10/13] Fix extension Signed-off-by: Natalie Arellano --- .../mock_buildpacks/0.2/read-env-extension/bin/generate | 3 +++ 1 file changed, 3 insertions(+) diff --git a/acceptance/testdata/mock_buildpacks/0.2/read-env-extension/bin/generate b/acceptance/testdata/mock_buildpacks/0.2/read-env-extension/bin/generate index a68dcef3cc..8421473953 100755 --- a/acceptance/testdata/mock_buildpacks/0.2/read-env-extension/bin/generate +++ b/acceptance/testdata/mock_buildpacks/0.2/read-env-extension/bin/generate @@ -24,5 +24,8 @@ FROM \${base_image} USER root RUN echo "Hello World" > /from-ext.txt + +ARG user_id +USER \${user_id} EOL fi From 57fd8c10bab34ab60e3942d5c31da74f3b950990 Mon Sep 17 00:00:00 2001 From: Natalie Arellano Date: Thu, 15 Jun 2023 17:21:47 -0400 Subject: [PATCH 11/13] Fix acceptance The fixture changed, so its SHA changed Signed-off-by: Natalie Arellano --- acceptance/acceptance_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/acceptance/acceptance_test.go b/acceptance/acceptance_test.go index dffa9b4f4c..a02dd5e182 100644 --- a/acceptance/acceptance_test.go +++ b/acceptance/acceptance_test.go @@ -797,12 +797,12 @@ func testAcceptance( it("creates builder", func() { // Linux containers (including Linux containers on Windows) extSimpleLayersDiffID := "sha256:b9e4a0ddfb650c7aa71d1e6aceea1665365e409b3078bfdc1e51c2b07ab2b423" - extReadEnvDiffID := "sha256:ab7419c5e0b1a0789bd07cef2ed0573ec6e98eb05d7f05eb95d4f035243e331c" + extReadEnvDiffID := "sha256:6801a0398d023ff06a43c4fd03ef325a45daaa4540fc3ee140e2fb22bf5143a7" bpSimpleLayersDiffID := "sha256:285ff6683c99e5ae19805f6a62168fb40dca64d813c53b782604c9652d745c70" bpReadEnvDiffID := "sha256:dd1e0efcbf3f08b014ef6eff9cfe7a9eac1cf20bd9b6a71a946f0a74575aa56f" if imageManager.HostOS() == "windows" { // Windows containers on Windows extSimpleLayersDiffID = "sha256:a063cf949b9c267133e451ac8cd95b4e77571bb7c629dd817461dca769170810" - extReadEnvDiffID = "sha256:a4e7f114efa3692939974da9c9f08e47b3fdb5c779688dc8f5a950e0f804bef1" + extReadEnvDiffID = "sha256:4c37e22595762315d28805f32e1f5fd48b4ddfc293ef7b41e0e609a4241b8479" bpSimpleLayersDiffID = "sha256:ccd1234cc5685e8a412b70c5f9a8e7b584b8e4f2a20c987ec242c9055de3e45e" bpReadEnvDiffID = "sha256:8b22a7742ffdfbdd978787c6937456b68afb27c3585a3903048be7434d251e3f" } From e618ace9f3347ae82036f1e9fa7032f3e61bff94 Mon Sep 17 00:00:00 2001 From: Natalie Arellano Date: Fri, 16 Jun 2023 13:07:11 -0400 Subject: [PATCH 12/13] Fallback to old behavior of re-using the build cache on older platform api Without this change, users would be required to upgrade their lifecycle to 0.17.0 when consuming pack 0.30.0 if using build image extension, as the restorer would fail to write to the /kaniko directory. Signed-off-by: Natalie Arellano --- internal/build/lifecycle_execution.go | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/internal/build/lifecycle_execution.go b/internal/build/lifecycle_execution.go index a2837b13d7..fb39fa1356 100644 --- a/internal/build/lifecycle_execution.go +++ b/internal/build/lifecycle_execution.go @@ -195,7 +195,6 @@ func (l *LifecycleExecution) Run(ctx context.Context, phaseFactoryCreator PhaseF } launchCache := cache.NewVolumeCache(l.opts.Image, l.opts.Cache.Launch, "launch", l.docker) - kanikoCache := cache.NewVolumeCache(l.opts.Image, l.opts.Cache.Kaniko, "kaniko", l.docker) if !l.opts.UseCreator { if l.platformAPI.LessThan("0.7") { @@ -220,6 +219,27 @@ func (l *LifecycleExecution) Run(ctx context.Context, phaseFactoryCreator PhaseF } } + var kanikoCache Cache + if l.PlatformAPI().AtLeast("0.12") { + // lifecycle 0.17.0 (introduces support for Platform API 0.12) and above will ensure that + // this volume is owned by the CNB user, + // and hence the restorer (after dropping privileges) will be able to write to it. + kanikoCache = cache.NewVolumeCache(l.opts.Image, l.opts.Cache.Kaniko, "kaniko", l.docker) + } else { + switch { + case buildCache.Type() == cache.Volume: + // Re-use the build cache as the kaniko cache. Earlier versions of the lifecycle (0.16.x and below) + // already ensure this volume is owned by the CNB user. + kanikoCache = buildCache + case l.hasExtensionsForBuild(): + // We need a usable kaniko cache, so error in this case. + return fmt.Errorf("build cache must be volume cache when building with extensions") + default: + // The kaniko cache is unused, so it doesn't matter that it's not usable. + kanikoCache = cache.NewVolumeCache(l.opts.Image, l.opts.Cache.Kaniko, "kaniko", l.docker) + } + } + l.logger.Info(style.Step("RESTORING")) if l.opts.ClearCache && l.PlatformAPI().LessThan("0.10") { l.logger.Info("Skipping 'restore' due to clearing cache") From edc796f1fb7ff36afd73ae9c6b41ccc41a38b7ea Mon Sep 17 00:00:00 2001 From: Natalie Arellano Date: Fri, 16 Jun 2023 13:14:42 -0400 Subject: [PATCH 13/13] Add unit test Signed-off-by: Natalie Arellano --- internal/build/lifecycle_execution_test.go | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/internal/build/lifecycle_execution_test.go b/internal/build/lifecycle_execution_test.go index 5ac97f2e7e..e3673f072e 100644 --- a/internal/build/lifecycle_execution_test.go +++ b/internal/build/lifecycle_execution_test.go @@ -1401,6 +1401,21 @@ func testLifecycleExecution(t *testing.T, when spec.G, it spec.S) { }) }) + when("platform >= 0.12", func() { + platformAPI = api.MustParse("0.12") + + it("passes run", func() { + h.AssertIncludeAllExpectedPatterns(t, + configProvider.ContainerConfig().Cmd, + []string{"-run", "/layers/run.toml"}, + ) + h.AssertSliceNotContains(t, + configProvider.ContainerConfig().Cmd, + "-stack", + ) + }) + }) + when("publish", func() { providedPublish = true