From 5f24b01e13cfc076239a0370e00bcff4d5fda649 Mon Sep 17 00:00:00 2001 From: Natalie Arellano Date: Wed, 1 May 2024 15:50:35 -0400 Subject: [PATCH 01/13] Run build containers with userns=host Fixes https://github.com/buildpacks/pack-private/issues/26 Signed-off-by: Natalie Arellano --- internal/build/phase_config_provider.go | 4 +++- internal/build/phase_config_provider_test.go | 1 + 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/internal/build/phase_config_provider.go b/internal/build/phase_config_provider.go index 15434c7a70..bd1e281fef 100644 --- a/internal/build/phase_config_provider.go +++ b/internal/build/phase_config_provider.go @@ -34,9 +34,11 @@ type PhaseConfigProvider struct { } func NewPhaseConfigProvider(name string, lifecycleExec *LifecycleExecution, ops ...PhaseConfigProviderOperation) *PhaseConfigProvider { + hostConf := new(container.HostConfig) + hostConf.UsernsMode = "host" provider := &PhaseConfigProvider{ ctrConf: new(container.Config), - hostConf: new(container.HostConfig), + hostConf: hostConf, name: name, os: lifecycleExec.os, infoWriter: logging.GetWriterForLevel(lifecycleExec.logger, logging.InfoLevel), diff --git a/internal/build/phase_config_provider_test.go b/internal/build/phase_config_provider_test.go index ed54a6ea1e..6512054704 100644 --- a/internal/build/phase_config_provider_test.go +++ b/internal/build/phase_config_provider_test.go @@ -59,6 +59,7 @@ func testPhaseConfigProvider(t *testing.T, when spec.G, it spec.S) { h.AssertSliceContainsMatch(t, phaseConfigProvider.HostConfig().Binds, "pack-app-.*:/workspace") h.AssertEq(t, phaseConfigProvider.HostConfig().Isolation, container.IsolationEmpty) + h.AssertEq(t, phaseConfigProvider.HostConfig().UsernsMode, container.UsernsMode("host")) }) when("building for Windows", func() { From 1ab72dd552a9feb3176e7565655960a474c7d3d0 Mon Sep 17 00:00:00 2001 From: Natalie Arellano Date: Thu, 2 May 2024 10:29:29 -0400 Subject: [PATCH 02/13] Use the untrusted flow when buildpacks are added to a trusted builder Fixes https://github.com/buildpacks/pack-private/issues/21 Signed-off-by: Natalie Arellano --- pkg/client/build.go | 9 +++++++++ pkg/client/build_test.go | 29 ++++++++++++++++++++++++++--- 2 files changed, 35 insertions(+), 3 deletions(-) diff --git a/pkg/client/build.go b/pkg/client/build.go index 43c82e0196..5429ed86e9 100644 --- a/pkg/client/build.go +++ b/pkg/client/build.go @@ -400,6 +400,10 @@ func (c *Client) Build(ctx context.Context, opts BuildOptions) error { // Get the platform API version to use lifecycleVersion := bldr.LifecycleDescriptor().Info.Version useCreator := supportsCreator(lifecycleVersion) && opts.TrustBuilder(opts.Builder) + if useCreator && hasAdditionalModules(opts) { + c.logger.Warnf("Builder is trusted but additional modules were added; using the untrusted (5 phases) build flow") + useCreator = false + } var ( lifecycleOptsLifecycleImage string lifecycleAPIs []string @@ -703,6 +707,11 @@ func (c *Client) Build(ctx context.Context, opts BuildOptions) error { return c.logImageNameAndSha(ctx, opts.Publish, imageRef) } +func hasAdditionalModules(opts BuildOptions) bool { + return !(len(opts.Buildpacks) == 0 && len(opts.Extensions) == 0 && + len(opts.PreBuildpacks) == 0 && len(opts.PostBuildpacks) == 0) +} + func extractSupportedLifecycleApis(labels map[string]string) ([]string, error) { // sample contents of labels: // {io.buildpacks.builder.metadata:\"{\"lifecycle\":{\"version\":\"0.15.3\"},\"api\":{\"buildpack\":\"0.2\",\"platform\":\"0.3\"}}", diff --git a/pkg/client/build_test.go b/pkg/client/build_test.go index b01cb9b2be..85265d2f2b 100644 --- a/pkg/client/build_test.go +++ b/pkg/client/build_test.go @@ -2112,9 +2112,6 @@ api = "0.2" h.AssertEq(t, args.PullPolicy, image.PullAlways) h.AssertEq(t, args.Platform, "linux/amd64") }) - it("uses the api versions of the lifecycle image", func() { - h.AssertTrue(t, true) - }) it("parses the versions correctly", func() { fakeLifecycleImage.SetLabel("io.buildpacks.lifecycle.apis", "{\"platform\":{\"deprecated\":[\"0.1\",\"0.2\",\"0.3\",\"0.4\",\"0.5\",\"0.6\"],\"supported\":[\"0.7\",\"0.8\",\"0.9\",\"0.10\",\"0.11\",\"0.12\"]}}") @@ -2154,6 +2151,32 @@ api = "0.2" args := fakeImageFetcher.FetchCalls[fakeLifecycleImage.Name()] h.AssertNil(t, args) }) + + when("additional buildpacks were added", func() { + it("uses the 5 phases with the lifecycle image", func() { + additionalBP := ifakes.CreateBuildpackTar(t, tmpDir, dist.BuildpackDescriptor{ + WithAPI: api.MustParse("0.3"), + WithInfo: dist.ModuleInfo{ + ID: "buildpack.add.1.id", + Version: "buildpack.add.1.version", + }, + WithStacks: []dist.Stack{{ID: defaultBuilderStackID}}, + WithOrder: nil, + }) + + h.AssertNil(t, subject.Build(context.TODO(), BuildOptions{ + Image: "some/app", + Builder: defaultBuilderName, + Publish: true, + TrustBuilder: func(string) bool { return true }, + Buildpacks: []string{additionalBP}, + })) + h.AssertEq(t, fakeLifecycle.Opts.UseCreator, false) + h.AssertEq(t, fakeLifecycle.Opts.LifecycleImage, fakeLifecycleImage.Name()) + + h.AssertContains(t, outBuf.String(), "Builder is trusted but additional modules were added; using the untrusted (5 phases) build flow") + }) + }) }) when("lifecycle doesn't support creator", func() { From 0d0a4958ff85f8fa54557c91971603d0e804fc39 Mon Sep 17 00:00:00 2001 From: Natalie Arellano Date: Thu, 2 May 2024 11:03:45 -0400 Subject: [PATCH 03/13] Warn if NOT --pull-policy=always in container Fixes https://github.com/buildpacks/pack-private/issues/20 Signed-off-by: Natalie Arellano --- go.mod | 1 + go.sum | 10 ++++++---- pkg/client/build.go | 12 ++++++++++++ pkg/client/build_test.go | 32 ++++++++++++++++++++++++++++++++ 4 files changed, 51 insertions(+), 4 deletions(-) diff --git a/go.mod b/go.mod index 1d77f5a645..a6cb78c80b 100644 --- a/go.mod +++ b/go.mod @@ -2,6 +2,7 @@ module github.com/buildpacks/pack require ( github.com/BurntSushi/toml v1.3.2 + github.com/GoogleContainerTools/kaniko v1.21.1 github.com/Masterminds/semver v1.5.0 github.com/Microsoft/go-winio v0.6.2 github.com/apex/log v1.9.0 diff --git a/go.sum b/go.sum index 7cc9ae8caa..87db8eec90 100644 --- a/go.sum +++ b/go.sum @@ -31,6 +31,8 @@ github.com/Azure/go-autorest/tracing v0.6.0 h1:TYi4+3m5t6K48TGI9AUdb+IzbnSxvnvUM github.com/Azure/go-autorest/tracing v0.6.0/go.mod h1:+vhtPC754Xsa23ID7GlGsrdKBpUA79WCAKPPZVC2DeU= github.com/BurntSushi/toml v1.3.2 h1:o7IhLm0Msx3BaB+n3Ag7L8EVlByGnpq14C4YWiu/gL8= github.com/BurntSushi/toml v1.3.2/go.mod h1:CxXYINrC8qIiEnFrOxCa7Jy5BFHlXnUU2pbicEuybxQ= +github.com/GoogleContainerTools/kaniko v1.21.1 h1:Q77TGiuSRopS1FvZY9Bzu9Wp9VYlpP6zU+/mu08/COs= +github.com/GoogleContainerTools/kaniko v1.21.1/go.mod h1:5kbaXGmhHLNc2Zzi/P1Se0qhFYDvK8R62QJh/O0n8rk= github.com/Masterminds/semver v1.5.0 h1:H65muMkzWKEuNDnfl9d70GUjFniHKHRbFPGBuZ3QEww= github.com/Masterminds/semver v1.5.0/go.mod h1:MB6lktGJrhw8PrUyiEoblNEGEQ+RzHPF078ddwwvV3Y= github.com/Microsoft/go-winio v0.5.2/go.mod h1:WpS1mjBmmwHBEWmogvA2mj8546UReBk4v8QkMxJ6pZY= @@ -522,8 +524,8 @@ golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8T golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= google.golang.org/appengine v1.1.0/go.mod h1:EbEs0AVv82hx2wNQdGPgUI5lhzA/G0D9YwlJXL52JkM= google.golang.org/genproto v0.0.0-20240213162025-012b6fc9bca9 h1:9+tzLLstTlPTRyJTh+ah5wIMsBW5c4tQwGTN3thOW9Y= -google.golang.org/genproto/googleapis/api v0.0.0-20231016165738-49dd2c1f3d0b h1:CIC2YMXmIhYw6evmhPxBKJ4fmLbOFtXQN/GV3XOZR8k= -google.golang.org/genproto/googleapis/api v0.0.0-20231016165738-49dd2c1f3d0b/go.mod h1:IBQ646DjkDkvUIsVq/cc03FUFQ9wbZu7yE396YcL870= +google.golang.org/genproto/googleapis/api v0.0.0-20240221002015-b0ce06bbee7c h1:9g7erC9qu44ks7UK4gDNlnk4kOxZG707xKm4jVniy6o= +google.golang.org/genproto/googleapis/api v0.0.0-20240221002015-b0ce06bbee7c/go.mod h1:5iCWqnniDlqZHrd3neWVTOwvh/v6s3232omMecelax8= google.golang.org/genproto/googleapis/rpc v0.0.0-20240213162025-012b6fc9bca9 h1:hZB7eLIaYlW9qXRfCq/qDaPdbeY3757uARz5Vvfv+cY= google.golang.org/genproto/googleapis/rpc v0.0.0-20240213162025-012b6fc9bca9/go.mod h1:YUWgXUFRPfoYK1IHMuxH5K6nPEXSCzIMljnQ59lLRCk= google.golang.org/grpc v1.61.1 h1:kLAiWrZs7YeDM6MumDe7m3y4aM6wacLzM1Y/wiLP9XY= @@ -551,5 +553,5 @@ gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C gopkg.in/yaml.v3 v3.0.0-20200605160147-a5ece683394c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= -gotest.tools/v3 v3.0.3 h1:4AuOwCGf4lLR9u3YOe2awrHygurzhO/HeQ6laiA6Sx0= -gotest.tools/v3 v3.0.3/go.mod h1:Z7Lb0S5l+klDB31fvDQX8ss/FlKDxtlFlw3Oa8Ymbl8= +gotest.tools/v3 v3.4.0 h1:ZazjZUfuVeZGLAmlKKuyv3IKP5orXcwtOwDQH6YVr6o= +gotest.tools/v3 v3.4.0/go.mod h1:CtbdzLSsqVhDgMtKsx03ird5YTGB3ar27v0u/yKBW5g= diff --git a/pkg/client/build.go b/pkg/client/build.go index 43c82e0196..44e086f415 100644 --- a/pkg/client/build.go +++ b/pkg/client/build.go @@ -16,6 +16,7 @@ import ( "strings" "time" + "github.com/GoogleContainerTools/kaniko/pkg/util/proc" "github.com/Masterminds/semver" "github.com/buildpacks/imgutil" "github.com/buildpacks/imgutil/layout" @@ -55,6 +56,10 @@ const ( minLifecycleVersionSupportingCreatorWithExtensions = "0.19.0" ) +var RunningInContainer = func() bool { + return proc.GetContainerRuntime(0, 0) != proc.RuntimeNotFound +} + // LifecycleExecutor executes the lifecycle which satisfies the Cloud Native Buildpacks Lifecycle specification. // Implementations of the Lifecycle must execute the following phases by calling the // phase-specific lifecycle binary in order: @@ -290,6 +295,13 @@ var IsTrustedBuilderFunc = func(b string) bool { func (c *Client) Build(ctx context.Context, opts BuildOptions) error { var pathsConfig layoutPathConfig + if RunningInContainer() && !(opts.PullPolicy == image.PullAlways) { + c.logger.Warnf("Detected pack is running in a container; if using a shared docker host, failing to pull build inputs from a remote registry is insecure - " + + "other tenants may have compromised build inputs stored in the daemon." + + "This configuration is insecure and may become unsupported in the future." + + "Re-run with '--pull-policy=always' to silence this warning.") + } + imageRef, err := c.parseReference(opts) if err != nil { return errors.Wrapf(err, "invalid image name '%s'", opts.Image) diff --git a/pkg/client/build_test.go b/pkg/client/build_test.go index b01cb9b2be..a6c61b3615 100644 --- a/pkg/client/build_test.go +++ b/pkg/client/build_test.go @@ -2284,6 +2284,38 @@ api = "0.2" }) }) + when("containerized pack", func() { + it.Before(func() { + RunningInContainer = func() bool { + return true + } + }) + + when("--pull-policy=always", func() { + it("does not warn", func() { + h.AssertNil(t, subject.Build(context.TODO(), BuildOptions{ + Image: "some/app", + Builder: defaultBuilderName, + PullPolicy: image.PullAlways, + })) + + h.AssertNotContains(t, outBuf.String(), "failing to pull build inputs from a remote registry is insecure") + }) + }) + + when("not --pull-policy=always", func() { + it("warns", func() { + h.AssertNil(t, subject.Build(context.TODO(), BuildOptions{ + Image: "some/app", + Builder: defaultBuilderName, + PullPolicy: image.PullNever, + })) + + h.AssertContains(t, outBuf.String(), "failing to pull build inputs from a remote registry is insecure") + }) + }) + }) + when("always", func() { it("uses pulls the builder and run image before using them", func() { h.AssertNil(t, subject.Build(context.TODO(), BuildOptions{ From b24c122640222072e1620f3a168ab4fab1012a09 Mon Sep 17 00:00:00 2001 From: Natalie Arellano Date: Thu, 2 May 2024 13:42:06 -0400 Subject: [PATCH 04/13] Also consider buildpacks that are added from project descriptor (but exclude inline buildpacks) Signed-off-by: Natalie Arellano --- pkg/client/build.go | 73 ++++++++++++++++++++++++---------------- pkg/client/build_test.go | 55 ++++++++++++++++++++++++++++++ 2 files changed, 99 insertions(+), 29 deletions(-) diff --git a/pkg/client/build.go b/pkg/client/build.go index 5429ed86e9..28489b4322 100644 --- a/pkg/client/build.go +++ b/pkg/client/build.go @@ -372,7 +372,7 @@ func (c *Client) Build(ctx context.Context, opts BuildOptions) error { return err } - fetchedBPs, order, err := c.processBuildpacks(ctx, bldr.Image(), bldr.Buildpacks(), bldr.Order(), bldr.StackID, opts) + fetchedBPs, nInlineBPs, order, err := c.processBuildpacks(ctx, bldr.Image(), bldr.Buildpacks(), bldr.Order(), bldr.StackID, opts) if err != nil { return err } @@ -400,7 +400,16 @@ func (c *Client) Build(ctx context.Context, opts BuildOptions) error { // Get the platform API version to use lifecycleVersion := bldr.LifecycleDescriptor().Info.Version useCreator := supportsCreator(lifecycleVersion) && opts.TrustBuilder(opts.Builder) - if useCreator && hasAdditionalModules(opts) { + hasAdditionalModules := func() bool { + if len(fetchedBPs) == 0 && len(fetchedExs) == 0 { + return false + } + if len(fetchedBPs) == nInlineBPs && len(fetchedExs) == 0 { + return false + } + return true + }() + if useCreator && hasAdditionalModules { c.logger.Warnf("Builder is trusted but additional modules were added; using the untrusted (5 phases) build flow") useCreator = false } @@ -707,11 +716,6 @@ func (c *Client) Build(ctx context.Context, opts BuildOptions) error { return c.logImageNameAndSha(ctx, opts.Publish, imageRef) } -func hasAdditionalModules(opts BuildOptions) bool { - return !(len(opts.Buildpacks) == 0 && len(opts.Extensions) == 0 && - len(opts.PreBuildpacks) == 0 && len(opts.PostBuildpacks) == 0) -} - func extractSupportedLifecycleApis(labels map[string]string) ([]string, error) { // sample contents of labels: // {io.buildpacks.builder.metadata:\"{\"lifecycle\":{\"version\":\"0.15.3\"},\"api\":{\"buildpack\":\"0.2\",\"platform\":\"0.3\"}}", @@ -1064,18 +1068,21 @@ func (c *Client) processProxyConfig(config *ProxyConfig) ProxyConfig { // ---------- // - group: // - A -func (c *Client) processBuildpacks(ctx context.Context, builderImage imgutil.Image, builderBPs []dist.ModuleInfo, builderOrder dist.Order, stackID string, opts BuildOptions) (fetchedBPs []buildpack.BuildModule, order dist.Order, err error) { +func (c *Client) processBuildpacks(ctx context.Context, builderImage imgutil.Image, builderBPs []dist.ModuleInfo, builderOrder dist.Order, stackID string, opts BuildOptions) (fetchedBPs []buildpack.BuildModule, nInlineBPs int, order dist.Order, err error) { relativeBaseDir := opts.RelativeBaseDir declaredBPs := opts.Buildpacks - // declare buildpacks provided by project descriptor when no buildpacks are declared + // Buildpacks from --buildpack override buildpacks from project descriptor if len(declaredBPs) == 0 && len(opts.ProjectDescriptor.Build.Buildpacks) != 0 { relativeBaseDir = opts.ProjectDescriptorBaseDir for _, bp := range opts.ProjectDescriptor.Build.Buildpacks { - buildpackLocator, err := getBuildpackLocator(bp, stackID) + buildpackLocator, isInline, err := getBuildpackLocator(bp, stackID) if err != nil { - return nil, nil, err + return nil, 0, nil, err + } + if isInline { + nInlineBPs++ } declaredBPs = append(declaredBPs, buildpackLocator) } @@ -1085,7 +1092,7 @@ func (c *Client) processBuildpacks(ctx context.Context, builderImage imgutil.Ima for _, bp := range declaredBPs { locatorType, err := buildpack.GetLocatorType(bp, relativeBaseDir, builderBPs) if err != nil { - return nil, nil, err + return nil, 0, nil, err } switch locatorType { @@ -1095,7 +1102,7 @@ func (c *Client) processBuildpacks(ctx context.Context, builderImage imgutil.Ima order = builderOrder case len(order) > 1: // This should only ever be possible if they are using from=builder twice which we don't allow - return nil, nil, errors.New("buildpacks from builder can only be defined once") + return nil, 0, nil, errors.New("buildpacks from builder can only be defined once") default: newOrder := dist.Order{} groupToAdd := order[0].Group @@ -1109,7 +1116,7 @@ func (c *Client) processBuildpacks(ctx context.Context, builderImage imgutil.Ima default: newFetchedBPs, moduleInfo, err := c.fetchBuildpack(ctx, bp, relativeBaseDir, builderImage, builderBPs, opts, buildpack.KindBuildpack) if err != nil { - return fetchedBPs, order, err + return fetchedBPs, 0, order, err } fetchedBPs = append(fetchedBPs, newFetchedBPs...) order = appendBuildpackToOrder(order, *moduleInfo) @@ -1119,20 +1126,28 @@ func (c *Client) processBuildpacks(ctx context.Context, builderImage imgutil.Ima if (len(order) == 0 || len(order[0].Group) == 0) && len(builderOrder) > 0 { preBuildpacks := opts.PreBuildpacks postBuildpacks := opts.PostBuildpacks + // Pre-buildpacks from --pre-buildpack override pre-buildpacks from project descriptor if len(preBuildpacks) == 0 && len(opts.ProjectDescriptor.Build.Pre.Buildpacks) > 0 { for _, bp := range opts.ProjectDescriptor.Build.Pre.Buildpacks { - buildpackLocator, err := getBuildpackLocator(bp, stackID) + buildpackLocator, isInline, err := getBuildpackLocator(bp, stackID) if err != nil { - return nil, nil, errors.Wrap(err, "get pre-buildpack locator") + return nil, 0, nil, errors.Wrap(err, "get pre-buildpack locator") + } + if isInline { + nInlineBPs++ } preBuildpacks = append(preBuildpacks, buildpackLocator) } } + // Post-buildpacks from --post-buildpack override post-buildpacks from project descriptor if len(postBuildpacks) == 0 && len(opts.ProjectDescriptor.Build.Post.Buildpacks) > 0 { for _, bp := range opts.ProjectDescriptor.Build.Post.Buildpacks { - buildpackLocator, err := getBuildpackLocator(bp, stackID) + buildpackLocator, isInline, err := getBuildpackLocator(bp, stackID) if err != nil { - return nil, nil, errors.Wrap(err, "get post-buildpack locator") + return nil, 0, nil, errors.Wrap(err, "get post-buildpack locator") + } + if isInline { + nInlineBPs++ } postBuildpacks = append(postBuildpacks, buildpackLocator) } @@ -1143,7 +1158,7 @@ func (c *Client) processBuildpacks(ctx context.Context, builderImage imgutil.Ima for _, bp := range preBuildpacks { newFetchedBPs, moduleInfo, err := c.fetchBuildpack(ctx, bp, relativeBaseDir, builderImage, builderBPs, opts, buildpack.KindBuildpack) if err != nil { - return fetchedBPs, order, err + return fetchedBPs, 0, order, err } fetchedBPs = append(fetchedBPs, newFetchedBPs...) order = prependBuildpackToOrder(order, *moduleInfo) @@ -1152,7 +1167,7 @@ func (c *Client) processBuildpacks(ctx context.Context, builderImage imgutil.Ima for _, bp := range postBuildpacks { newFetchedBPs, moduleInfo, err := c.fetchBuildpack(ctx, bp, relativeBaseDir, builderImage, builderBPs, opts, buildpack.KindBuildpack) if err != nil { - return fetchedBPs, order, err + return fetchedBPs, 0, order, err } fetchedBPs = append(fetchedBPs, newFetchedBPs...) order = appendBuildpackToOrder(order, *moduleInfo) @@ -1160,7 +1175,7 @@ func (c *Client) processBuildpacks(ctx context.Context, builderImage imgutil.Ima } } - return fetchedBPs, order, nil + return fetchedBPs, nInlineBPs, order, nil } func (c *Client) fetchBuildpack(ctx context.Context, bp string, relativeBaseDir string, builderImage imgutil.Image, builderBPs []dist.ModuleInfo, opts BuildOptions, kind string) ([]buildpack.BuildModule, *dist.ModuleInfo, error) { @@ -1250,26 +1265,26 @@ func (c *Client) fetchBuildpackDependencies(ctx context.Context, bp string, pack return nil, err } -func getBuildpackLocator(bp projectTypes.Buildpack, stackID string) (string, error) { +func getBuildpackLocator(bp projectTypes.Buildpack, stackID string) (locator string, isInline bool, err error) { switch { case bp.ID != "" && bp.Script.Inline != "" && bp.URI == "": if bp.Script.API == "" { - return "", errors.New("Missing API version for inline buildpack") + return "", false, errors.New("Missing API version for inline buildpack") } pathToInlineBuildpack, err := createInlineBuildpack(bp, stackID) if err != nil { - return "", errors.Wrap(err, "Could not create temporary inline buildpack") + return "", false, errors.Wrap(err, "Could not create temporary inline buildpack") } - return pathToInlineBuildpack, nil + return pathToInlineBuildpack, true, nil case bp.URI != "": - return bp.URI, nil + return bp.URI, false, nil case bp.ID != "" && bp.Version != "": - return fmt.Sprintf("%s@%s", bp.ID, bp.Version), nil + return fmt.Sprintf("%s@%s", bp.ID, bp.Version), false, nil case bp.ID != "" && bp.Version == "": - return bp.ID, nil + return bp.ID, false, nil default: - return "", errors.New("Invalid buildpack definition") + return "", false, errors.New("Invalid buildpack definition") } } diff --git a/pkg/client/build_test.go b/pkg/client/build_test.go index 85265d2f2b..01b2989053 100644 --- a/pkg/client/build_test.go +++ b/pkg/client/build_test.go @@ -2176,6 +2176,61 @@ api = "0.2" h.AssertContains(t, outBuf.String(), "Builder is trusted but additional modules were added; using the untrusted (5 phases) build flow") }) + + when("from project descriptor", func() { + it("uses the 5 phases with the lifecycle image", func() { + additionalBP := ifakes.CreateBuildpackTar(t, tmpDir, dist.BuildpackDescriptor{ + WithAPI: api.MustParse("0.3"), + WithInfo: dist.ModuleInfo{ + ID: "buildpack.add.1.id", + Version: "buildpack.add.1.version", + }, + WithStacks: []dist.Stack{{ID: defaultBuilderStackID}}, + WithOrder: nil, + }) + + h.AssertNil(t, subject.Build(context.TODO(), BuildOptions{ + Image: "some/app", + Builder: defaultBuilderName, + Publish: true, + TrustBuilder: func(string) bool { return true }, + ProjectDescriptor: projectTypes.Descriptor{Build: projectTypes.Build{ + Buildpacks: []projectTypes.Buildpack{{ + URI: additionalBP, + }}, + }}, + })) + h.AssertEq(t, fakeLifecycle.Opts.UseCreator, false) + h.AssertEq(t, fakeLifecycle.Opts.LifecycleImage, fakeLifecycleImage.Name()) + + h.AssertContains(t, outBuf.String(), "Builder is trusted but additional modules were added; using the untrusted (5 phases) build flow") + }) + + when("inline buildpack", func() { + it("uses the creator with the provided builder", func() { + h.AssertNil(t, subject.Build(context.TODO(), BuildOptions{ + Image: "some/app", + Builder: defaultBuilderName, + Publish: true, + TrustBuilder: func(string) bool { return true }, + ProjectDescriptor: projectTypes.Descriptor{Build: projectTypes.Build{ + Buildpacks: []projectTypes.Buildpack{{ + ID: "buildpack.add.1.id", + Version: "buildpack.add.1.version", + Script: projectTypes.Script{ + API: "0.10", + Inline: "echo hello", + }, + }}, + }}, + })) + h.AssertEq(t, fakeLifecycle.Opts.UseCreator, true) + + args := fakeImageFetcher.FetchCalls[fakeLifecycleImage.Name()] + h.AssertNil(t, args) + }) + }) + }) }) }) From 1e7aad8d057a2cd46f86e41f9d9feae472c4c97c Mon Sep 17 00:00:00 2001 From: Natalie Arellano Date: Thu, 2 May 2024 15:23:43 -0400 Subject: [PATCH 05/13] Skip WCOW failing test Signed-off-by: Natalie Arellano --- acceptance/acceptance_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/acceptance/acceptance_test.go b/acceptance/acceptance_test.go index 6b030cb04c..eb856b5072 100644 --- a/acceptance/acceptance_test.go +++ b/acceptance/acceptance_test.go @@ -1284,6 +1284,7 @@ func testAcceptance( it.Before(func() { h.SkipIf(t, os.Getenv("DOCKER_HOST") != "", "cannot mount volume when DOCKER_HOST is set") + h.SkipIf(t, imageManager.HostOS() == "windows", "These tests are broken on Windows Containers on Windows when not using the creator; see https://github.com/buildpacks/pack/issues/2147") if imageManager.HostOS() == "windows" { volumeRoot = `c:\` From 1cc1cc8dd9978eb321ed5e6c1d99250311df5781 Mon Sep 17 00:00:00 2001 From: Natalie Arellano Date: Thu, 27 Jun 2024 14:24:02 -0400 Subject: [PATCH 06/13] Initial implementation (still has TODOs) Signed-off-by: Natalie Arellano --- internal/config/config.go | 9 +++++ pkg/cache/volume_cache.go | 64 +++++++++++++++++++++++++++++- pkg/cache/volume_cache_test.go | 71 +++++++++++++++++++++++++++++++++- 3 files changed, 142 insertions(+), 2 deletions(-) diff --git a/internal/config/config.go b/internal/config/config.go index de370ef727..c0e61ade88 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -23,6 +23,7 @@ type Config struct { LifecycleImage string `toml:"lifecycle-image,omitempty"` RegistryMirrors map[string]string `toml:"registry-mirrors,omitempty"` LayoutRepositoryDir string `toml:"layout-repo-dir,omitempty"` + VolumeKeys map[string]string `toml:"volume-keys,omitempty"` // TODO: move to own struct } type Registry struct { @@ -58,6 +59,14 @@ func DefaultConfigPath() (string, error) { return filepath.Join(home, "config.toml"), nil } +func DefaultVolumeKeysPath() (string, error) { + home, err := PackHome() + if err != nil { + return "", errors.Wrap(err, "getting pack home") + } + return filepath.Join(home, "volume-keys.toml"), nil +} + func PackHome() (string, error) { packHome := os.Getenv("PACK_HOME") if packHome == "" { diff --git a/pkg/cache/volume_cache.go b/pkg/cache/volume_cache.go index b274d3d3ff..bc6121a4aa 100644 --- a/pkg/cache/volume_cache.go +++ b/pkg/cache/volume_cache.go @@ -2,16 +2,21 @@ package cache import ( "context" + "crypto/rand" "crypto/sha256" "fmt" + "os" "strings" "github.com/docker/docker/client" "github.com/google/go-containerregistry/pkg/name" + "github.com/buildpacks/pack/internal/config" "github.com/buildpacks/pack/internal/paths" ) +const EnvVolumeKey = "PACK_VOLUME_KEY" + type VolumeCache struct { docker DockerClient volume string @@ -20,7 +25,11 @@ type VolumeCache struct { func NewVolumeCache(imageRef name.Reference, cacheType CacheInfo, suffix string, dockerClient DockerClient) *VolumeCache { var volumeName string if cacheType.Source == "" { - sum := sha256.Sum256([]byte(imageRef.Name())) + volumeKey, err := getVolumeKey(imageRef) + if err != nil { + // TODO + } + sum := sha256.Sum256([]byte(imageRef.Name() + volumeKey)) // TODO: investigate if there are better ways to do this vol := paths.FilterReservedNames(fmt.Sprintf("%s-%x", sanitizedRef(imageRef), sum[:6])) volumeName = fmt.Sprintf("pack-cache-%s.%s", vol, suffix) } else { @@ -33,6 +42,59 @@ func NewVolumeCache(imageRef name.Reference, cacheType CacheInfo, suffix string, } } +func getVolumeKey(imageRef name.Reference) (string, error) { + var foundKey string + + // first, look for key in env + + foundKey = os.Getenv(EnvVolumeKey) + if foundKey != "" { + return foundKey, nil + } + + // then, look for key in existing config + + volumeKeysPath, err := config.DefaultVolumeKeysPath() + if err != nil { + return "", err + } + cfg, err := config.Read(volumeKeysPath) + if err != nil { + return "", err + } + + foundKey = cfg.VolumeKeys[imageRef.Name()] + if foundKey != "" { + return foundKey, nil + } + + // finally, create new key and store it in config + + newKey := randString(20) + if cfg.VolumeKeys == nil { + cfg.VolumeKeys = make(map[string]string) + } + cfg.VolumeKeys[imageRef.Name()] = newKey + if err = config.Write(cfg, volumeKeysPath); err != nil { + return "", err + } + + return newKey, nil +} + +// Returns a string iwith lowercase a-z, of length n +func randString(n int) string { + b := make([]byte, n) + _, err := rand.Read(b) + if err != nil { + panic(err) + } + for i := range b { + b[i] = 'a' + (b[i] % 26) + } + return string(b) +} + func (c *VolumeCache) Name() string { return c.volume } diff --git a/pkg/cache/volume_cache_test.go b/pkg/cache/volume_cache_test.go index 5ae1e14a92..ee05fa12cc 100644 --- a/pkg/cache/volume_cache_test.go +++ b/pkg/cache/volume_cache_test.go @@ -2,9 +2,12 @@ package cache_test import ( "context" + "os" + "path/filepath" "strings" "testing" + "github.com/buildpacks/pack/internal/config" "github.com/buildpacks/pack/pkg/cache" "github.com/docker/docker/api/types/filters" @@ -24,7 +27,7 @@ func TestVolumeCache(t *testing.T) { color.Disable(true) defer color.Disable(false) - spec.Run(t, "VolumeCache", testCache, spec.Parallel(), spec.Report(report.Terminal{})) + spec.Run(t, "VolumeCache", testCache, spec.Sequential(), spec.Report(report.Terminal{})) } func testCache(t *testing.T, when spec.G, it spec.S) { @@ -118,6 +121,72 @@ func testCache(t *testing.T, when spec.G, it spec.S) { h.AssertContains(t, subject.Name(), "fedora_httpd_version1.0") h.AssertTrue(t, names.RestrictedNamePattern.MatchString(subject.Name())) }) + + when("PACK_VOLUME_KEY", func() { + when("is set", func() { + it.After(func() { + h.AssertNil(t, os.Unsetenv("PACK_VOLUME_KEY")) + }) + + it("uses it to construct the volume name", func() { + ref, err := name.ParseReference("my/repo:some-tag", name.WeakValidation) + h.AssertNil(t, err) + + nameFromNewKey := cache.NewVolumeCache(ref, cache.CacheInfo{}, "some-suffix", dockerClient) // sources a new key + h.AssertNil(t, os.Setenv("PACK_VOLUME_KEY", "some-volume-key")) + nameFromEnvKey := cache.NewVolumeCache(ref, cache.CacheInfo{}, "some-suffix", dockerClient) // sources key from env + h.AssertNotEq(t, nameFromNewKey.Name(), nameFromEnvKey.Name()) + }) + }) + + when("is unset", func() { + var tmpPackHome string + + it.Before(func() { + var err error + tmpPackHome, err = os.MkdirTemp("", "") + h.AssertNil(t, err) + h.AssertNil(t, os.Setenv("PACK_HOME", tmpPackHome)) + }) + + it.After(func() { + h.AssertNil(t, os.RemoveAll(tmpPackHome)) + }) + + when("~/.pack/volume-keys.toml contains key for repo name", func() { + it("sources the key from ~/.pack/volume-keys.toml", func() { + ref, err := name.ParseReference("my/repo:some-tag", name.WeakValidation) + h.AssertNil(t, err) + + nameFromNewKey := cache.NewVolumeCache(ref, cache.CacheInfo{}, "some-suffix", dockerClient) // sources a new key + + cfgContents := ` +[volume-keys] +"index.docker.io/my/repo:some-tag" = "SOME_VOLUME_KEY" +` + h.AssertNil(t, os.WriteFile(filepath.Join(tmpPackHome, "volume-keys.toml"), []byte(cfgContents), 0755)) // overrides the key that was set + + nameFromConfigKey := cache.NewVolumeCache(ref, cache.CacheInfo{}, "some-suffix", dockerClient) // sources key from config + h.AssertNotEq(t, nameFromNewKey.Name(), nameFromConfigKey.Name()) + }) + }) + + when("~/.pack/volume-keys.toml missing key for repo name", func() { + it("generates a new key and saves it to ~/.pack/volume-keys.toml", func() { + ref, err := name.ParseReference("my/repo:some-tag", name.WeakValidation) + h.AssertNil(t, err) + + nameFromNewKey := cache.NewVolumeCache(ref, cache.CacheInfo{}, "some-suffix", dockerClient) // sources a new key + nameFromConfigKey := cache.NewVolumeCache(ref, cache.CacheInfo{}, "some-suffix", dockerClient) // sources same key from config + h.AssertEq(t, nameFromNewKey.Name(), nameFromConfigKey.Name()) + + cfg, err := config.Read(filepath.Join(tmpPackHome, "volume-keys.toml")) + h.AssertNil(t, err) + h.AssertNotNil(t, cfg.VolumeKeys["index.docker.io/my/repo:some-tag"]) + }) + }) + }) + }) }) when("volume cache name is not empty", func() { From 8e0d859317f88569fd09a656471ab31f8df1c0af Mon Sep 17 00:00:00 2001 From: Natalie Arellano Date: Thu, 27 Jun 2024 15:01:39 -0400 Subject: [PATCH 07/13] Resolve some TODOs Signed-off-by: Natalie Arellano --- acceptance/acceptance_test.go | 12 +++--- internal/build/lifecycle_execution.go | 21 +++++++-- internal/config/config.go | 16 ++++++- pkg/cache/volume_cache.go | 8 ++-- pkg/cache/volume_cache_test.go | 62 +++++++++++++-------------- 5 files changed, 72 insertions(+), 47 deletions(-) diff --git a/acceptance/acceptance_test.go b/acceptance/acceptance_test.go index e7f52815d0..37cb048a05 100644 --- a/acceptance/acceptance_test.go +++ b/acceptance/acceptance_test.go @@ -1162,8 +1162,8 @@ func testAcceptance( ref, err := name.ParseReference(repoName, name.WeakValidation) assert.Nil(err) cacheImage := cache.NewImageCache(ref, dockerCli) - buildCacheVolume := cache.NewVolumeCache(ref, cache.CacheInfo{}, "build", dockerCli) - launchCacheVolume := cache.NewVolumeCache(ref, cache.CacheInfo{}, "launch", dockerCli) + buildCacheVolume, _ := cache.NewVolumeCache(ref, cache.CacheInfo{}, "build", dockerCli) + launchCacheVolume, _ := cache.NewVolumeCache(ref, cache.CacheInfo{}, "launch", dockerCli) cacheImage.Clear(context.TODO()) buildCacheVolume.Clear(context.TODO()) launchCacheVolume.Clear(context.TODO()) @@ -1282,8 +1282,8 @@ func testAcceptance( ref, err := name.ParseReference(repoName, name.WeakValidation) assert.Nil(err) cacheImage := cache.NewImageCache(ref, dockerCli) - buildCacheVolume := cache.NewVolumeCache(ref, cache.CacheInfo{}, "build", dockerCli) - launchCacheVolume := cache.NewVolumeCache(ref, cache.CacheInfo{}, "launch", dockerCli) + buildCacheVolume, _ := cache.NewVolumeCache(ref, cache.CacheInfo{}, "build", dockerCli) + launchCacheVolume, _ := cache.NewVolumeCache(ref, cache.CacheInfo{}, "launch", dockerCli) cacheImage.Clear(context.TODO()) buildCacheVolume.Clear(context.TODO()) launchCacheVolume.Clear(context.TODO()) @@ -3167,8 +3167,8 @@ include = [ "*.jar", "media/mountain.jpg", "/media/person.png", ] imageManager.CleanupImages(origID, repoName, runBefore) ref, err := name.ParseReference(repoName, name.WeakValidation) assert.Nil(err) - buildCacheVolume := cache.NewVolumeCache(ref, cache.CacheInfo{}, "build", dockerCli) - launchCacheVolume := cache.NewVolumeCache(ref, cache.CacheInfo{}, "launch", dockerCli) + buildCacheVolume, _ := cache.NewVolumeCache(ref, cache.CacheInfo{}, "build", dockerCli) + launchCacheVolume, _ := cache.NewVolumeCache(ref, cache.CacheInfo{}, "launch", dockerCli) assert.Succeeds(buildCacheVolume.Clear(context.TODO())) assert.Succeeds(launchCacheVolume.Clear(context.TODO())) }) diff --git a/internal/build/lifecycle_execution.go b/internal/build/lifecycle_execution.go index 2f017dd70f..1042fe0aa3 100644 --- a/internal/build/lifecycle_execution.go +++ b/internal/build/lifecycle_execution.go @@ -179,7 +179,11 @@ func (l *LifecycleExecution) Run(ctx context.Context, phaseFactoryCreator PhaseF } else { switch l.opts.Cache.Build.Format { case cache.CacheVolume: - buildCache = cache.NewVolumeCache(l.opts.Image, l.opts.Cache.Build, "build", l.docker) + var err error + buildCache, err = cache.NewVolumeCache(l.opts.Image, l.opts.Cache.Build, "build", l.docker) + if err != nil { + return err + } l.logger.Debugf("Using build cache volume %s", style.Symbol(buildCache.Name())) case cache.CacheBind: buildCache = cache.NewBindCache(l.opts.Cache.Build, l.docker) @@ -194,7 +198,10 @@ func (l *LifecycleExecution) Run(ctx context.Context, phaseFactoryCreator PhaseF l.logger.Debugf("Build cache %s cleared", style.Symbol(buildCache.Name())) } - launchCache := cache.NewVolumeCache(l.opts.Image, l.opts.Cache.Launch, "launch", l.docker) + launchCache, err := cache.NewVolumeCache(l.opts.Image, l.opts.Cache.Launch, "launch", l.docker) + if err != nil { + return err + } if !l.opts.UseCreator { if l.platformAPI.LessThan("0.7") { @@ -224,7 +231,10 @@ func (l *LifecycleExecution) Run(ctx context.Context, phaseFactoryCreator PhaseF // 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) + kanikoCache, err = cache.NewVolumeCache(l.opts.Image, l.opts.Cache.Kaniko, "kaniko", l.docker) + if err != nil { + return err + } } else { switch { case buildCache.Type() == cache.Volume: @@ -236,7 +246,10 @@ func (l *LifecycleExecution) Run(ctx context.Context, phaseFactoryCreator PhaseF 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) + kanikoCache, err = cache.NewVolumeCache(l.opts.Image, l.opts.Cache.Kaniko, "kaniko", l.docker) + if err != nil { + return err + } } } diff --git a/internal/config/config.go b/internal/config/config.go index c0e61ade88..e15da68f03 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -23,7 +23,10 @@ type Config struct { LifecycleImage string `toml:"lifecycle-image,omitempty"` RegistryMirrors map[string]string `toml:"registry-mirrors,omitempty"` LayoutRepositoryDir string `toml:"layout-repo-dir,omitempty"` - VolumeKeys map[string]string `toml:"volume-keys,omitempty"` // TODO: move to own struct +} + +type VolumeConfig struct { + VolumeKeys map[string]string `toml:"volume-keys,omitempty"` } type Registry struct { @@ -88,7 +91,16 @@ func Read(path string) (Config, error) { return cfg, nil } -func Write(cfg Config, path string) error { +func ReadVolumeKeys(path string) (VolumeConfig, error) { + cfg := VolumeConfig{} + _, err := toml.DecodeFile(path, &cfg) + if err != nil && !os.IsNotExist(err) { + return VolumeConfig{}, errors.Wrapf(err, "failed to read config file at path %s", path) + } + return cfg, nil +} + +func Write(cfg interface{}, path string) error { if err := MkdirAll(filepath.Dir(path)); err != nil { return err } diff --git a/pkg/cache/volume_cache.go b/pkg/cache/volume_cache.go index bc6121a4aa..a9307a34cc 100644 --- a/pkg/cache/volume_cache.go +++ b/pkg/cache/volume_cache.go @@ -22,12 +22,12 @@ type VolumeCache struct { volume string } -func NewVolumeCache(imageRef name.Reference, cacheType CacheInfo, suffix string, dockerClient DockerClient) *VolumeCache { +func NewVolumeCache(imageRef name.Reference, cacheType CacheInfo, suffix string, dockerClient DockerClient) (*VolumeCache, error) { var volumeName string if cacheType.Source == "" { volumeKey, err := getVolumeKey(imageRef) if err != nil { - // TODO + return nil, err } sum := sha256.Sum256([]byte(imageRef.Name() + volumeKey)) // TODO: investigate if there are better ways to do this vol := paths.FilterReservedNames(fmt.Sprintf("%s-%x", sanitizedRef(imageRef), sum[:6])) @@ -39,7 +39,7 @@ func NewVolumeCache(imageRef name.Reference, cacheType CacheInfo, suffix string, return &VolumeCache{ volume: volumeName, docker: dockerClient, - } + }, nil } func getVolumeKey(imageRef name.Reference) (string, error) { @@ -58,7 +58,7 @@ func getVolumeKey(imageRef name.Reference) (string, error) { if err != nil { return "", err } - cfg, err := config.Read(volumeKeysPath) + cfg, err := config.ReadVolumeKeys(volumeKeysPath) if err != nil { return "", err } diff --git a/pkg/cache/volume_cache_test.go b/pkg/cache/volume_cache_test.go index ee05fa12cc..4518a7ce40 100644 --- a/pkg/cache/volume_cache_test.go +++ b/pkg/cache/volume_cache_test.go @@ -43,7 +43,7 @@ func testCache(t *testing.T, when spec.G, it spec.S) { it("adds suffix to calculated name", func() { ref, err := name.ParseReference("my/repo", name.WeakValidation) h.AssertNil(t, err) - subject := cache.NewVolumeCache(ref, cache.CacheInfo{}, "some-suffix", dockerClient) + subject, _ := cache.NewVolumeCache(ref, cache.CacheInfo{}, "some-suffix", dockerClient) if !strings.HasSuffix(subject.Name(), ".some-suffix") { t.Fatalf("Calculated volume name '%s' should end with '.some-suffix'", subject.Name()) } @@ -53,8 +53,8 @@ func testCache(t *testing.T, when spec.G, it spec.S) { ref, err := name.ParseReference("my/repo", name.WeakValidation) h.AssertNil(t, err) - subject := cache.NewVolumeCache(ref, cache.CacheInfo{}, "some-suffix", dockerClient) - expected := cache.NewVolumeCache(ref, cache.CacheInfo{}, "some-suffix", dockerClient) + subject, _ := cache.NewVolumeCache(ref, cache.CacheInfo{}, "some-suffix", dockerClient) + expected, _ := cache.NewVolumeCache(ref, cache.CacheInfo{}, "some-suffix", dockerClient) if subject.Name() != expected.Name() { t.Fatalf("The same repo name should result in the same volume") } @@ -64,11 +64,11 @@ func testCache(t *testing.T, when spec.G, it spec.S) { ref, err := name.ParseReference("my/repo:other-tag", name.WeakValidation) h.AssertNil(t, err) - subject := cache.NewVolumeCache(ref, cache.CacheInfo{}, "some-suffix", dockerClient) + subject, _ := cache.NewVolumeCache(ref, cache.CacheInfo{}, "some-suffix", dockerClient) ref, err = name.ParseReference("my/repo", name.WeakValidation) h.AssertNil(t, err) - notExpected := cache.NewVolumeCache(ref, cache.CacheInfo{}, "some-suffix", dockerClient) + notExpected, _ := cache.NewVolumeCache(ref, cache.CacheInfo{}, "some-suffix", dockerClient) if subject.Name() == notExpected.Name() { t.Fatalf("Different image tags should result in different volumes") } @@ -78,11 +78,11 @@ func testCache(t *testing.T, when spec.G, it spec.S) { ref, err := name.ParseReference("registry.com/my/repo:other-tag", name.WeakValidation) h.AssertNil(t, err) - subject := cache.NewVolumeCache(ref, cache.CacheInfo{}, "some-suffix", dockerClient) + subject, _ := cache.NewVolumeCache(ref, cache.CacheInfo{}, "some-suffix", dockerClient) ref, err = name.ParseReference("my/repo", name.WeakValidation) h.AssertNil(t, err) - notExpected := cache.NewVolumeCache(ref, cache.CacheInfo{}, "some-suffix", dockerClient) + notExpected, _ := cache.NewVolumeCache(ref, cache.CacheInfo{}, "some-suffix", dockerClient) if subject.Name() == notExpected.Name() { t.Fatalf("Different image registries should result in different volumes") } @@ -92,11 +92,11 @@ func testCache(t *testing.T, when spec.G, it spec.S) { ref, err := name.ParseReference("my/repo:latest", name.WeakValidation) h.AssertNil(t, err) - subject := cache.NewVolumeCache(ref, cache.CacheInfo{}, "some-suffix", dockerClient) + subject, _ := cache.NewVolumeCache(ref, cache.CacheInfo{}, "some-suffix", dockerClient) ref, err = name.ParseReference("my/repo", name.WeakValidation) h.AssertNil(t, err) - expected := cache.NewVolumeCache(ref, cache.CacheInfo{}, "some-suffix", dockerClient) + expected, _ := cache.NewVolumeCache(ref, cache.CacheInfo{}, "some-suffix", dockerClient) h.AssertEq(t, subject.Name(), expected.Name()) }) @@ -104,11 +104,11 @@ func testCache(t *testing.T, when spec.G, it spec.S) { ref, err := name.ParseReference("index.docker.io/my/repo", name.WeakValidation) h.AssertNil(t, err) - subject := cache.NewVolumeCache(ref, cache.CacheInfo{}, "some-suffix", dockerClient) + subject, _ := cache.NewVolumeCache(ref, cache.CacheInfo{}, "some-suffix", dockerClient) ref, err = name.ParseReference("my/repo", name.WeakValidation) h.AssertNil(t, err) - expected := cache.NewVolumeCache(ref, cache.CacheInfo{}, "some-suffix", dockerClient) + expected, _ := cache.NewVolumeCache(ref, cache.CacheInfo{}, "some-suffix", dockerClient) h.AssertEq(t, subject.Name(), expected.Name()) }) @@ -116,7 +116,7 @@ func testCache(t *testing.T, when spec.G, it spec.S) { ref, err := name.ParseReference("myregistryhost:5000/fedora/httpd:version1.0", name.WeakValidation) h.AssertNil(t, err) - subject := cache.NewVolumeCache(ref, cache.CacheInfo{}, "some-suffix", dockerClient) + subject, _ := cache.NewVolumeCache(ref, cache.CacheInfo{}, "some-suffix", dockerClient) h.AssertContains(t, subject.Name(), "fedora_httpd_version1.0") h.AssertTrue(t, names.RestrictedNamePattern.MatchString(subject.Name())) @@ -132,9 +132,9 @@ func testCache(t *testing.T, when spec.G, it spec.S) { ref, err := name.ParseReference("my/repo:some-tag", name.WeakValidation) h.AssertNil(t, err) - nameFromNewKey := cache.NewVolumeCache(ref, cache.CacheInfo{}, "some-suffix", dockerClient) // sources a new key + nameFromNewKey, _ := cache.NewVolumeCache(ref, cache.CacheInfo{}, "some-suffix", dockerClient) // sources a new key h.AssertNil(t, os.Setenv("PACK_VOLUME_KEY", "some-volume-key")) - nameFromEnvKey := cache.NewVolumeCache(ref, cache.CacheInfo{}, "some-suffix", dockerClient) // sources key from env + nameFromEnvKey, _ := cache.NewVolumeCache(ref, cache.CacheInfo{}, "some-suffix", dockerClient) // sources key from env h.AssertNotEq(t, nameFromNewKey.Name(), nameFromEnvKey.Name()) }) }) @@ -158,7 +158,7 @@ func testCache(t *testing.T, when spec.G, it spec.S) { ref, err := name.ParseReference("my/repo:some-tag", name.WeakValidation) h.AssertNil(t, err) - nameFromNewKey := cache.NewVolumeCache(ref, cache.CacheInfo{}, "some-suffix", dockerClient) // sources a new key + nameFromNewKey, _ := cache.NewVolumeCache(ref, cache.CacheInfo{}, "some-suffix", dockerClient) // sources a new key cfgContents := ` [volume-keys] @@ -166,7 +166,7 @@ func testCache(t *testing.T, when spec.G, it spec.S) { ` h.AssertNil(t, os.WriteFile(filepath.Join(tmpPackHome, "volume-keys.toml"), []byte(cfgContents), 0755)) // overrides the key that was set - nameFromConfigKey := cache.NewVolumeCache(ref, cache.CacheInfo{}, "some-suffix", dockerClient) // sources key from config + nameFromConfigKey, _ := cache.NewVolumeCache(ref, cache.CacheInfo{}, "some-suffix", dockerClient) // sources key from config h.AssertNotEq(t, nameFromNewKey.Name(), nameFromConfigKey.Name()) }) }) @@ -176,11 +176,11 @@ func testCache(t *testing.T, when spec.G, it spec.S) { ref, err := name.ParseReference("my/repo:some-tag", name.WeakValidation) h.AssertNil(t, err) - nameFromNewKey := cache.NewVolumeCache(ref, cache.CacheInfo{}, "some-suffix", dockerClient) // sources a new key - nameFromConfigKey := cache.NewVolumeCache(ref, cache.CacheInfo{}, "some-suffix", dockerClient) // sources same key from config + nameFromNewKey, _ := cache.NewVolumeCache(ref, cache.CacheInfo{}, "some-suffix", dockerClient) // sources a new key + nameFromConfigKey, _ := cache.NewVolumeCache(ref, cache.CacheInfo{}, "some-suffix", dockerClient) // sources same key from config h.AssertEq(t, nameFromNewKey.Name(), nameFromConfigKey.Name()) - cfg, err := config.Read(filepath.Join(tmpPackHome, "volume-keys.toml")) + cfg, err := config.ReadVolumeKeys(filepath.Join(tmpPackHome, "volume-keys.toml")) h.AssertNil(t, err) h.AssertNotNil(t, cfg.VolumeKeys["index.docker.io/my/repo:some-tag"]) }) @@ -200,7 +200,7 @@ func testCache(t *testing.T, when spec.G, it spec.S) { ref, err := name.ParseReference("my/repo", name.WeakValidation) h.AssertNil(t, err) - subject := cache.NewVolumeCache(ref, cacheInfo, "some-suffix", dockerClient) + subject, _ := cache.NewVolumeCache(ref, cacheInfo, "some-suffix", dockerClient) if volumeName != subject.Name() { t.Fatalf("Volume name '%s' should be same as the name specified '%s'", subject.Name(), volumeName) @@ -211,9 +211,9 @@ func testCache(t *testing.T, when spec.G, it spec.S) { ref, err := name.ParseReference("my/repo", name.WeakValidation) h.AssertNil(t, err) - subject := cache.NewVolumeCache(ref, cacheInfo, "some-suffix", dockerClient) + subject, _ := cache.NewVolumeCache(ref, cacheInfo, "some-suffix", dockerClient) - expected := cache.NewVolumeCache(ref, cacheInfo, "some-suffix", dockerClient) + expected, _ := cache.NewVolumeCache(ref, cacheInfo, "some-suffix", dockerClient) if subject.Name() != expected.Name() { t.Fatalf("The same repo name should result in the same volume") } @@ -223,11 +223,11 @@ func testCache(t *testing.T, when spec.G, it spec.S) { ref, err := name.ParseReference("registry.com/my/repo:other-tag", name.WeakValidation) h.AssertNil(t, err) - subject := cache.NewVolumeCache(ref, cache.CacheInfo{}, "some-suffix", dockerClient) + subject, _ := cache.NewVolumeCache(ref, cache.CacheInfo{}, "some-suffix", dockerClient) ref, err = name.ParseReference("my/repo", name.WeakValidation) h.AssertNil(t, err) - notExpected := cache.NewVolumeCache(ref, cache.CacheInfo{}, "some-suffix", dockerClient) + notExpected, _ := cache.NewVolumeCache(ref, cache.CacheInfo{}, "some-suffix", dockerClient) if subject.Name() == notExpected.Name() { t.Fatalf("Different image registries should result in different volumes") } @@ -237,11 +237,11 @@ func testCache(t *testing.T, when spec.G, it spec.S) { ref, err := name.ParseReference("my/repo:latest", name.WeakValidation) h.AssertNil(t, err) - subject := cache.NewVolumeCache(ref, cache.CacheInfo{}, "some-suffix", dockerClient) + subject, _ := cache.NewVolumeCache(ref, cache.CacheInfo{}, "some-suffix", dockerClient) ref, err = name.ParseReference("my/repo", name.WeakValidation) h.AssertNil(t, err) - expected := cache.NewVolumeCache(ref, cache.CacheInfo{}, "some-suffix", dockerClient) + expected, _ := cache.NewVolumeCache(ref, cache.CacheInfo{}, "some-suffix", dockerClient) h.AssertEq(t, subject.Name(), expected.Name()) }) @@ -249,11 +249,11 @@ func testCache(t *testing.T, when spec.G, it spec.S) { ref, err := name.ParseReference("index.docker.io/my/repo", name.WeakValidation) h.AssertNil(t, err) - subject := cache.NewVolumeCache(ref, cache.CacheInfo{}, "some-suffix", dockerClient) + subject, _ := cache.NewVolumeCache(ref, cache.CacheInfo{}, "some-suffix", dockerClient) ref, err = name.ParseReference("my/repo", name.WeakValidation) h.AssertNil(t, err) - expected := cache.NewVolumeCache(ref, cache.CacheInfo{}, "some-suffix", dockerClient) + expected, _ := cache.NewVolumeCache(ref, cache.CacheInfo{}, "some-suffix", dockerClient) h.AssertEq(t, subject.Name(), expected.Name()) }) @@ -261,7 +261,7 @@ func testCache(t *testing.T, when spec.G, it spec.S) { ref, err := name.ParseReference("myregistryhost:5000/fedora/httpd:version1.0", name.WeakValidation) h.AssertNil(t, err) - subject := cache.NewVolumeCache(ref, cache.CacheInfo{}, "some-suffix", dockerClient) + subject, _ := cache.NewVolumeCache(ref, cache.CacheInfo{}, "some-suffix", dockerClient) h.AssertContains(t, subject.Name(), "fedora_httpd_version1.0") h.AssertTrue(t, names.RestrictedNamePattern.MatchString(subject.Name())) @@ -286,7 +286,7 @@ func testCache(t *testing.T, when spec.G, it spec.S) { ref, err := name.ParseReference(h.RandString(10), name.WeakValidation) h.AssertNil(t, err) - subject = cache.NewVolumeCache(ref, cache.CacheInfo{}, "some-suffix", dockerClient) + subject, _ = cache.NewVolumeCache(ref, cache.CacheInfo{}, "some-suffix", dockerClient) volumeName = subject.Name() }) @@ -324,7 +324,7 @@ func testCache(t *testing.T, when spec.G, it spec.S) { it("returns the cache type", func() { ref, err := name.ParseReference("my/repo", name.WeakValidation) h.AssertNil(t, err) - subject := cache.NewVolumeCache(ref, cache.CacheInfo{}, "some-suffix", dockerClient) + subject, _ := cache.NewVolumeCache(ref, cache.CacheInfo{}, "some-suffix", dockerClient) expected := cache.Volume h.AssertEq(t, subject.Type(), expected) }) From 10aa579c157fbb7cc85cd04696baadf71733a661 Mon Sep 17 00:00:00 2001 From: Natalie Arellano Date: Mon, 1 Jul 2024 12:19:03 -0400 Subject: [PATCH 08/13] Launch build containers in a separate ephemeral Docker bridge network Signed-off-by: Natalie Arellano --- internal/build/docker.go | 2 ++ internal/build/lifecycle_execution.go | 21 +++++++++++ internal/build/lifecycle_execution_test.go | 41 ++++++++++++++++++++++ pkg/client/docker.go | 2 ++ 4 files changed, 66 insertions(+) diff --git a/internal/build/docker.go b/internal/build/docker.go index 6fd7d54fe9..64db04d5df 100644 --- a/internal/build/docker.go +++ b/internal/build/docker.go @@ -23,6 +23,8 @@ type DockerClient interface { ContainerInspect(ctx context.Context, container string) (types.ContainerJSON, error) ContainerRemove(ctx context.Context, container string, options containertypes.RemoveOptions) error CopyToContainer(ctx context.Context, container, path string, content io.Reader, options types.CopyToContainerOptions) error + NetworkCreate(ctx context.Context, name string, options types.NetworkCreate) (types.NetworkCreateResponse, error) + NetworkRemove(ctx context.Context, network string) error } var _ DockerClient = dockerClient.CommonAPIClient(nil) diff --git a/internal/build/lifecycle_execution.go b/internal/build/lifecycle_execution.go index 2f017dd70f..ce8ea021e5 100644 --- a/internal/build/lifecycle_execution.go +++ b/internal/build/lifecycle_execution.go @@ -12,6 +12,7 @@ import ( "github.com/buildpacks/lifecycle/api" "github.com/buildpacks/lifecycle/auth" "github.com/buildpacks/lifecycle/platform/files" + "github.com/docker/docker/api/types" "github.com/google/go-containerregistry/pkg/name" "github.com/pkg/errors" "golang.org/x/sync/errgroup" @@ -165,6 +166,7 @@ func (l *LifecycleExecution) PrevImageName() string { func (l *LifecycleExecution) Run(ctx context.Context, phaseFactoryCreator PhaseFactoryCreator) error { phaseFactory := phaseFactoryCreator(l) + var buildCache Cache if l.opts.CacheImage != "" || (l.opts.Cache.Build.Format == cache.CacheImage) { cacheImageName := l.opts.CacheImage @@ -196,6 +198,25 @@ func (l *LifecycleExecution) Run(ctx context.Context, phaseFactoryCreator PhaseF launchCache := cache.NewVolumeCache(l.opts.Image, l.opts.Cache.Launch, "launch", l.docker) + if l.opts.Network == "" { + // start an ephemeral bridge network + networkName := fmt.Sprintf("pack.local/network/%x", randString(10)) + resp, err := l.docker.NetworkCreate(ctx, networkName, types.NetworkCreate{ + Driver: "bridge", + }) + if err != nil { + return fmt.Errorf("failed to create ephemeral bridge network: %w", err) + } + defer func() { + _ = l.docker.NetworkRemove(ctx, networkName) + }() + l.logger.Debugf("Created ephemeral bridge network %s with ID %s", networkName, resp.ID) + if resp.Warning != "" { + l.logger.Warn(resp.Warning) + } + l.opts.Network = networkName + } + if !l.opts.UseCreator { if l.platformAPI.LessThan("0.7") { l.logger.Info(style.Step("DETECTING")) diff --git a/internal/build/lifecycle_execution_test.go b/internal/build/lifecycle_execution_test.go index 6aa126a25a..09b6d20ba8 100644 --- a/internal/build/lifecycle_execution_test.go +++ b/internal/build/lifecycle_execution_test.go @@ -16,6 +16,7 @@ import ( ifakes "github.com/buildpacks/imgutil/fakes" "github.com/buildpacks/lifecycle/api" "github.com/buildpacks/lifecycle/platform/files" + "github.com/docker/docker/api/types" "github.com/docker/docker/api/types/container" "github.com/docker/docker/client" "github.com/google/go-containerregistry/pkg/authn" @@ -780,6 +781,46 @@ func testLifecycleExecution(t *testing.T, when spec.G, it spec.S) { }) }) + when("network is not provided", func() { + it("creates an ephemeral bridge network", func() { + beforeNetworks := func() int { + networks, err := docker.NetworkList(context.Background(), types.NetworkListOptions{}) + h.AssertNil(t, err) + return len(networks) + }() + + opts := build.LifecycleOptions{ + Image: imageName, + Builder: fakeBuilder, + Termui: fakeTermui, + } + + lifecycle, err := build.NewLifecycleExecution(logger, docker, "some-temp-dir", opts) + h.AssertNil(t, err) + + err = lifecycle.Run(context.Background(), func(execution *build.LifecycleExecution) build.PhaseFactory { + return fakePhaseFactory + }) + h.AssertNil(t, err) + + for _, entry := range fakePhaseFactory.NewCalledWithProvider { + h.AssertContains(t, string(entry.HostConfig().NetworkMode), "pack.local/network/") + h.AssertEq(t, entry.HostConfig().NetworkMode.IsDefault(), false) + h.AssertEq(t, entry.HostConfig().NetworkMode.IsHost(), false) + h.AssertEq(t, entry.HostConfig().NetworkMode.IsNone(), false) + h.AssertEq(t, entry.HostConfig().NetworkMode.IsPrivate(), true) + h.AssertEq(t, entry.HostConfig().NetworkMode.IsUserDefined(), true) + } + + afterNetworks := func() int { + networks, err := docker.NetworkList(context.Background(), types.NetworkListOptions{}) + h.AssertNil(t, err) + return len(networks) + }() + h.AssertEq(t, beforeNetworks, afterNetworks) + }) + }) + when("Error cases", func() { when("passed invalid", func() { it("fails for cache-image", func() { diff --git a/pkg/client/docker.go b/pkg/client/docker.go index 11b0fe1c01..d5a020f476 100644 --- a/pkg/client/docker.go +++ b/pkg/client/docker.go @@ -32,4 +32,6 @@ type DockerClient interface { ContainerWait(ctx context.Context, container string, condition containertypes.WaitCondition) (<-chan containertypes.WaitResponse, <-chan error) ContainerAttach(ctx context.Context, container string, options containertypes.AttachOptions) (types.HijackedResponse, error) ContainerStart(ctx context.Context, container string, options containertypes.StartOptions) error + NetworkCreate(ctx context.Context, name string, options types.NetworkCreate) (types.NetworkCreateResponse, error) + NetworkRemove(ctx context.Context, network string) error } From 2049ae22666ecaed80fc990dd9f2eeaf2ddb40ac Mon Sep 17 00:00:00 2001 From: Natalie Arellano Date: Thu, 2 May 2024 10:09:54 -0400 Subject: [PATCH 09/13] Run build containers with security-opt field set to no-new-privileges=true Fixes https://github.com/buildpacks/pack-private/issues/22 Signed-off-by: Natalie Arellano --- internal/build/phase_config_provider.go | 3 +++ internal/build/phase_config_provider_test.go | 2 ++ 2 files changed, 5 insertions(+) diff --git a/internal/build/phase_config_provider.go b/internal/build/phase_config_provider.go index bd1e281fef..4226928d93 100644 --- a/internal/build/phase_config_provider.go +++ b/internal/build/phase_config_provider.go @@ -36,6 +36,9 @@ type PhaseConfigProvider struct { func NewPhaseConfigProvider(name string, lifecycleExec *LifecycleExecution, ops ...PhaseConfigProviderOperation) *PhaseConfigProvider { hostConf := new(container.HostConfig) hostConf.UsernsMode = "host" + if lifecycleExec.os != "windows" { + hostConf.SecurityOpt = []string{"no-new-privileges=true"} + } provider := &PhaseConfigProvider{ ctrConf: new(container.Config), hostConf: hostConf, diff --git a/internal/build/phase_config_provider_test.go b/internal/build/phase_config_provider_test.go index 6512054704..93cce0ed52 100644 --- a/internal/build/phase_config_provider_test.go +++ b/internal/build/phase_config_provider_test.go @@ -60,6 +60,7 @@ func testPhaseConfigProvider(t *testing.T, when spec.G, it spec.S) { h.AssertEq(t, phaseConfigProvider.HostConfig().Isolation, container.IsolationEmpty) h.AssertEq(t, phaseConfigProvider.HostConfig().UsernsMode, container.UsernsMode("host")) + h.AssertSliceContains(t, phaseConfigProvider.HostConfig().SecurityOpt, "no-new-privileges=true") }) when("building for Windows", func() { @@ -73,6 +74,7 @@ func testPhaseConfigProvider(t *testing.T, when spec.G, it spec.S) { phaseConfigProvider := build.NewPhaseConfigProvider("some-name", lifecycle) h.AssertEq(t, phaseConfigProvider.HostConfig().Isolation, container.IsolationProcess) + h.AssertSliceNotContains(t, phaseConfigProvider.HostConfig().SecurityOpt, "no-new-privileges=true") }) }) From 25f454e57708d4e86606ba8a7289c7920b01d89d Mon Sep 17 00:00:00 2001 From: Natalie Arellano Date: Wed, 10 Jul 2024 11:14:43 -0400 Subject: [PATCH 10/13] Fix macos by not requiring real docker client for lifecycle execution unit tests Signed-off-by: Natalie Arellano --- internal/build/lifecycle_execution_test.go | 24 ++++++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/internal/build/lifecycle_execution_test.go b/internal/build/lifecycle_execution_test.go index 09b6d20ba8..4ab8077122 100644 --- a/internal/build/lifecycle_execution_test.go +++ b/internal/build/lifecycle_execution_test.go @@ -276,7 +276,7 @@ func testLifecycleExecution(t *testing.T, when spec.G, it spec.S) { fakeBuilder *fakes.FakeBuilder outBuf bytes.Buffer logger *logging.LogWithWriters - docker *client.Client + docker *fakeDockerClient fakeTermui *fakes.FakeTermui ) @@ -290,7 +290,7 @@ func testLifecycleExecution(t *testing.T, when spec.G, it spec.S) { fakeBuilder, err = fakes.NewFakeBuilder(fakes.WithSupportedPlatformAPIs([]*api.Version{api.MustParse("0.3")})) h.AssertNil(t, err) logger = logging.NewLogWithWriters(&outBuf, &outBuf) - docker, err = client.NewClientWithOpts(client.FromEnv, client.WithVersion("1.38")) + docker = &fakeDockerClient{} h.AssertNil(t, err) fakePhaseFactory = fakes.NewFakePhaseFactory() }) @@ -2698,6 +2698,26 @@ func (f *fakeImageFetcher) fetchRunImage(name string) error { return nil } +type fakeDockerClient struct { + nNetworks int + build.DockerClient +} + +func (f *fakeDockerClient) NetworkList(ctx context.Context, opts types.NetworkListOptions) ([]types.NetworkResource, error) { + ret := make([]types.NetworkResource, f.nNetworks) + return ret, nil +} + +func (f *fakeDockerClient) NetworkCreate(ctx context.Context, name string, options types.NetworkCreate) (types.NetworkCreateResponse, error) { + f.nNetworks++ + return types.NetworkCreateResponse{}, nil +} + +func (f *fakeDockerClient) NetworkRemove(ctx context.Context, network string) error { + f.nNetworks-- + return nil +} + func newTestLifecycleExecErr(t *testing.T, logVerbose bool, tmpDir string, ops ...func(*build.LifecycleOptions)) (*build.LifecycleExecution, error) { docker, err := client.NewClientWithOpts(client.FromEnv, client.WithVersion("1.38")) h.AssertNil(t, err) From 98ffc46188fb8ef6852c978394306e26857d4c4f Mon Sep 17 00:00:00 2001 From: Natalie Arellano Date: Wed, 10 Jul 2024 11:16:46 -0400 Subject: [PATCH 11/13] Try to fix wcow Signed-off-by: Natalie Arellano --- internal/build/lifecycle_execution.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/internal/build/lifecycle_execution.go b/internal/build/lifecycle_execution.go index ce8ea021e5..bac6a53b9f 100644 --- a/internal/build/lifecycle_execution.go +++ b/internal/build/lifecycle_execution.go @@ -200,12 +200,16 @@ func (l *LifecycleExecution) Run(ctx context.Context, phaseFactoryCreator PhaseF if l.opts.Network == "" { // start an ephemeral bridge network + driver := "bridge" + if l.os == "windows" { + driver = "nat" + } networkName := fmt.Sprintf("pack.local/network/%x", randString(10)) resp, err := l.docker.NetworkCreate(ctx, networkName, types.NetworkCreate{ - Driver: "bridge", + Driver: driver, }) if err != nil { - return fmt.Errorf("failed to create ephemeral bridge network: %w", err) + return fmt.Errorf("failed to create ephemeral %s network: %w", driver, err) } defer func() { _ = l.docker.NetworkRemove(ctx, networkName) From 12049345affc5cddb784f291cbdfab265e58d5b3 Mon Sep 17 00:00:00 2001 From: Aidan Delaney Date: Sun, 9 Jun 2024 10:52:25 +0000 Subject: [PATCH 12/13] Validate build image name Ensure that the user has not requested to build an image with the same name as a builder of a lifecyle image. Signed-off-by: Aidan Delaney Signed-off-by: Aidan Delaney Signed-off-by: Aidan Delaney --- internal/commands/build.go | 61 +++++++++++++++++++++ internal/commands/build_test.go | 33 +++++++++-- internal/commands/config_trusted_builder.go | 10 +++- 3 files changed, 96 insertions(+), 8 deletions(-) diff --git a/internal/commands/build.go b/internal/commands/build.go index d32f96cf8b..18d9f8566b 100644 --- a/internal/commands/build.go +++ b/internal/commands/build.go @@ -1,6 +1,7 @@ package commands import ( + "fmt" "os" "path/filepath" "strconv" @@ -140,6 +141,12 @@ func Build(logger logging.Logger, cfg config.Config, packClient PackClient) *cob } lifecycleImage = ref.Name() } + + err = isForbiddenTag(cfg, inputImageName.Name(), lifecycleImage, builder) + if err != nil { + return errors.Wrapf(err, "forbidden image name") + } + var gid = -1 if cmd.Flags().Changed("gid") { gid = flags.GID @@ -380,3 +387,57 @@ func parseProjectToml(appPath, descriptorPath string, logger logging.Logger) (pr descriptor, err := project.ReadProjectDescriptor(actualPath, logger) return descriptor, actualPath, err } + +func isForbiddenTag(cfg config.Config, input, lifecycle, builder string) error { + inputImage, err := name.ParseReference(input) + if err != nil { + return errors.Wrapf(err, "invalid image name %s", input) + } + + if builder != "" { + builderImage, err := name.ParseReference(builder) + if err != nil { + return errors.Wrapf(err, "parsing builder image %s", builder) + } + if inputImage.Context().RepositoryStr() == builderImage.Context().RepositoryStr() { + return fmt.Errorf("name must not match builder image name") + } + } + + if lifecycle != "" { + lifecycleImage, err := name.ParseReference(lifecycle) + if err != nil { + return errors.Wrapf(err, "parsing lifecycle image %s", lifecycle) + } + if inputImage.Context().RepositoryStr() == lifecycleImage.Context().RepositoryStr() { + return fmt.Errorf("name must not match lifecycle image name") + } + } + + trustedBuilders := getTrustedBuilders(cfg) + for _, trustedBuilder := range trustedBuilders { + builder, err := name.ParseReference(trustedBuilder) + if err != nil { + return err + } + if inputImage.Context().RepositoryStr() == builder.Context().RepositoryStr() { + return fmt.Errorf("name must not match trusted builder name") + } + } + + if inputImage.Context().RepositoryStr() == config.DefaultLifecycleImageRepo { + return fmt.Errorf("name must not match default lifecycle image name") + } + + if cfg.DefaultBuilder != "" { + defaultBuilderImage, err := name.ParseReference(cfg.DefaultBuilder) + if err != nil { + return errors.Wrapf(err, "parsing default builder %s", cfg.DefaultBuilder) + } + if inputImage.Context().RepositoryStr() == defaultBuilderImage.Context().RegistryStr() { + return fmt.Errorf("name must not match default builder image name") + } + } + + return nil +} diff --git a/internal/commands/build_test.go b/internal/commands/build_test.go index 6a59810004..311c820f57 100644 --- a/internal/commands/build_test.go +++ b/internal/commands/build_test.go @@ -126,6 +126,33 @@ func testBuildCommand(t *testing.T, when spec.G, it spec.S) { h.AssertContains(t, outBuf.String(), "Builder 'heroku/builder:22' is trusted") }) }) + + when("the image name matches a builder name", func() { + it("refuses to build", func() { + logger.WantVerbose(true) + command.SetArgs([]string{"heroku/builder:test", "--builder", "heroku/builder:24"}) + h.AssertNotNil(t, command.Execute()) + h.AssertContains(t, outBuf.String(), "name must not match builder image name") + }) + }) + + when("the image name matches a trusted-builder name", func() { + it("refuses to build", func() { + logger.WantVerbose(true) + command.SetArgs([]string{"heroku/builder:test", "--builder", "test", "--trust-builder"}) + h.AssertNotNil(t, command.Execute()) + h.AssertContains(t, outBuf.String(), "name must not match trusted builder name") + }) + }) + + when("the image name matches a lifecycle image name", func() { + it("refuses to build", func() { + logger.WantVerbose(true) + command.SetArgs([]string{"buildpacksio/lifecycle:test", "--builder", "test", "--trust-builder"}) + h.AssertNotNil(t, command.Execute()) + h.AssertContains(t, outBuf.String(), "name must not match default lifecycle image name") + }) + }) }) when("--buildpack-registry flag is specified but experimental isn't set in the config", func() { @@ -766,13 +793,9 @@ builder = "my-builder" when("previous-image flag is provided", func() { when("image is invalid", func() { it("error must be thrown", func() { - mockClient.EXPECT(). - Build(gomock.Any(), EqBuildOptionsWithPreviousImage("previous-image")). - Return(errors.New("")) - command.SetArgs([]string{"--builder", "my-builder", "/x@/y/?!z", "--previous-image", "previous-image"}) err := command.Execute() - h.AssertError(t, err, "failed to build") + h.AssertError(t, err, "forbidden image name") }) }) diff --git a/internal/commands/config_trusted_builder.go b/internal/commands/config_trusted_builder.go index 3de01d3824..f4ad995974 100644 --- a/internal/commands/config_trusted_builder.go +++ b/internal/commands/config_trusted_builder.go @@ -98,9 +98,7 @@ func removeTrustedBuilder(args []string, logger logging.Logger, cfg config.Confi return nil } -func listTrustedBuilders(args []string, logger logging.Logger, cfg config.Config) { - logger.Info("Trusted Builders:") - +func getTrustedBuilders(cfg config.Config) []string { var trustedBuilders []string for _, knownBuilder := range bldr.KnownBuilders { if knownBuilder.Trusted { @@ -113,7 +111,13 @@ func listTrustedBuilders(args []string, logger logging.Logger, cfg config.Config } sort.Strings(trustedBuilders) + return trustedBuilders +} + +func listTrustedBuilders(args []string, logger logging.Logger, cfg config.Config) { + logger.Info("Trusted Builders:") + trustedBuilders := getTrustedBuilders(cfg) for _, builder := range trustedBuilders { logger.Infof(" %s", builder) } From 331ac6d0e711b35b1da4037fe377774d933ddf19 Mon Sep 17 00:00:00 2001 From: Natalie Arellano Date: Mon, 15 Jul 2024 16:33:07 -0400 Subject: [PATCH 13/13] Log a warning (once) when PACK_VOLUME_KEY is unset and running in a container Signed-off-by: Natalie Arellano --- acceptance/acceptance_test.go | 16 +++-- internal/build/lifecycle_execution.go | 8 +-- pkg/cache/volume_cache.go | 20 ++++-- pkg/cache/volume_cache_test.go | 88 +++++++++++++++++---------- 4 files changed, 87 insertions(+), 45 deletions(-) diff --git a/acceptance/acceptance_test.go b/acceptance/acceptance_test.go index 7f7074ece6..f837988e09 100644 --- a/acceptance/acceptance_test.go +++ b/acceptance/acceptance_test.go @@ -38,6 +38,7 @@ import ( "github.com/buildpacks/pack/internal/style" "github.com/buildpacks/pack/pkg/archive" "github.com/buildpacks/pack/pkg/cache" + "github.com/buildpacks/pack/pkg/logging" h "github.com/buildpacks/pack/testhelpers" ) @@ -1162,8 +1163,9 @@ func testAcceptance( ref, err := name.ParseReference(repoName, name.WeakValidation) assert.Nil(err) cacheImage := cache.NewImageCache(ref, dockerCli) - buildCacheVolume, _ := cache.NewVolumeCache(ref, cache.CacheInfo{}, "build", dockerCli) - launchCacheVolume, _ := cache.NewVolumeCache(ref, cache.CacheInfo{}, "launch", dockerCli) + logger := logging.NewSimpleLogger(&bytes.Buffer{}) + buildCacheVolume, _ := cache.NewVolumeCache(ref, cache.CacheInfo{}, "build", dockerCli, logger) + launchCacheVolume, _ := cache.NewVolumeCache(ref, cache.CacheInfo{}, "launch", dockerCli, logger) cacheImage.Clear(context.TODO()) buildCacheVolume.Clear(context.TODO()) launchCacheVolume.Clear(context.TODO()) @@ -1282,8 +1284,9 @@ func testAcceptance( ref, err := name.ParseReference(repoName, name.WeakValidation) assert.Nil(err) cacheImage := cache.NewImageCache(ref, dockerCli) - buildCacheVolume, _ := cache.NewVolumeCache(ref, cache.CacheInfo{}, "build", dockerCli) - launchCacheVolume, _ := cache.NewVolumeCache(ref, cache.CacheInfo{}, "launch", dockerCli) + logger := logging.NewSimpleLogger(&bytes.Buffer{}) + buildCacheVolume, _ := cache.NewVolumeCache(ref, cache.CacheInfo{}, "build", dockerCli, logger) + launchCacheVolume, _ := cache.NewVolumeCache(ref, cache.CacheInfo{}, "launch", dockerCli, logger) cacheImage.Clear(context.TODO()) buildCacheVolume.Clear(context.TODO()) launchCacheVolume.Clear(context.TODO()) @@ -3168,8 +3171,9 @@ include = [ "*.jar", "media/mountain.jpg", "/media/person.png", ] imageManager.CleanupImages(origID, repoName, runBefore) ref, err := name.ParseReference(repoName, name.WeakValidation) assert.Nil(err) - buildCacheVolume, _ := cache.NewVolumeCache(ref, cache.CacheInfo{}, "build", dockerCli) - launchCacheVolume, _ := cache.NewVolumeCache(ref, cache.CacheInfo{}, "launch", dockerCli) + logger := logging.NewSimpleLogger(&bytes.Buffer{}) + buildCacheVolume, _ := cache.NewVolumeCache(ref, cache.CacheInfo{}, "build", dockerCli, logger) + launchCacheVolume, _ := cache.NewVolumeCache(ref, cache.CacheInfo{}, "launch", dockerCli, logger) assert.Succeeds(buildCacheVolume.Clear(context.TODO())) assert.Succeeds(launchCacheVolume.Clear(context.TODO())) }) diff --git a/internal/build/lifecycle_execution.go b/internal/build/lifecycle_execution.go index 620c6eae74..1911c0eed2 100644 --- a/internal/build/lifecycle_execution.go +++ b/internal/build/lifecycle_execution.go @@ -182,7 +182,7 @@ func (l *LifecycleExecution) Run(ctx context.Context, phaseFactoryCreator PhaseF switch l.opts.Cache.Build.Format { case cache.CacheVolume: var err error - buildCache, err = cache.NewVolumeCache(l.opts.Image, l.opts.Cache.Build, "build", l.docker) + buildCache, err = cache.NewVolumeCache(l.opts.Image, l.opts.Cache.Build, "build", l.docker, l.logger) if err != nil { return err } @@ -200,7 +200,7 @@ func (l *LifecycleExecution) Run(ctx context.Context, phaseFactoryCreator PhaseF l.logger.Debugf("Build cache %s cleared", style.Symbol(buildCache.Name())) } - launchCache, err := cache.NewVolumeCache(l.opts.Image, l.opts.Cache.Launch, "launch", l.docker) + launchCache, err := cache.NewVolumeCache(l.opts.Image, l.opts.Cache.Launch, "launch", l.docker, l.logger) if err != nil { return err } @@ -256,7 +256,7 @@ func (l *LifecycleExecution) Run(ctx context.Context, phaseFactoryCreator PhaseF // 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, err = cache.NewVolumeCache(l.opts.Image, l.opts.Cache.Kaniko, "kaniko", l.docker) + kanikoCache, err = cache.NewVolumeCache(l.opts.Image, l.opts.Cache.Kaniko, "kaniko", l.docker, l.logger) if err != nil { return err } @@ -271,7 +271,7 @@ func (l *LifecycleExecution) Run(ctx context.Context, phaseFactoryCreator PhaseF 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, err = cache.NewVolumeCache(l.opts.Image, l.opts.Cache.Kaniko, "kaniko", l.docker) + kanikoCache, err = cache.NewVolumeCache(l.opts.Image, l.opts.Cache.Kaniko, "kaniko", l.docker, l.logger) if err != nil { return err } diff --git a/pkg/cache/volume_cache.go b/pkg/cache/volume_cache.go index a9307a34cc..f414bcd903 100644 --- a/pkg/cache/volume_cache.go +++ b/pkg/cache/volume_cache.go @@ -8,11 +8,13 @@ import ( "os" "strings" + "github.com/GoogleContainerTools/kaniko/pkg/util/proc" "github.com/docker/docker/client" "github.com/google/go-containerregistry/pkg/name" "github.com/buildpacks/pack/internal/config" "github.com/buildpacks/pack/internal/paths" + "github.com/buildpacks/pack/pkg/logging" ) const EnvVolumeKey = "PACK_VOLUME_KEY" @@ -22,14 +24,14 @@ type VolumeCache struct { volume string } -func NewVolumeCache(imageRef name.Reference, cacheType CacheInfo, suffix string, dockerClient DockerClient) (*VolumeCache, error) { +func NewVolumeCache(imageRef name.Reference, cacheType CacheInfo, suffix string, dockerClient DockerClient, logger logging.Logger) (*VolumeCache, error) { var volumeName string if cacheType.Source == "" { - volumeKey, err := getVolumeKey(imageRef) + volumeKey, err := getVolumeKey(imageRef, logger) if err != nil { return nil, err } - sum := sha256.Sum256([]byte(imageRef.Name() + volumeKey)) // TODO: investigate if there are better ways to do this + sum := sha256.Sum256([]byte(imageRef.Name() + volumeKey)) vol := paths.FilterReservedNames(fmt.Sprintf("%s-%x", sanitizedRef(imageRef), sum[:6])) volumeName = fmt.Sprintf("pack-cache-%s.%s", vol, suffix) } else { @@ -42,7 +44,7 @@ func NewVolumeCache(imageRef name.Reference, cacheType CacheInfo, suffix string, }, nil } -func getVolumeKey(imageRef name.Reference) (string, error) { +func getVolumeKey(imageRef name.Reference, logger logging.Logger) (string, error) { var foundKey string // first, look for key in env @@ -70,6 +72,12 @@ func getVolumeKey(imageRef name.Reference) (string, error) { // finally, create new key and store it in config + // if we're running in a container, we should log a warning + // so that we don't always re-create the cache + if RunningInContainer() { + logger.Warnf("%s is unset; set this environment variable to a secret value to avoid creating a new volume cache on every build", EnvVolumeKey) + } + newKey := randString(20) if cfg.VolumeKeys == nil { cfg.VolumeKeys = make(map[string]string) @@ -118,3 +126,7 @@ func sanitizedRef(ref name.Reference) string { result = strings.ReplaceAll(result, "/", "_") return fmt.Sprintf("%s_%s", result, ref.Identifier()) } + +var RunningInContainer = func() bool { + return proc.GetContainerRuntime(0, 0) != proc.RuntimeNotFound +} diff --git a/pkg/cache/volume_cache_test.go b/pkg/cache/volume_cache_test.go index 4518a7ce40..685f34d2be 100644 --- a/pkg/cache/volume_cache_test.go +++ b/pkg/cache/volume_cache_test.go @@ -1,6 +1,7 @@ package cache_test import ( + "bytes" "context" "os" "path/filepath" @@ -9,6 +10,7 @@ import ( "github.com/buildpacks/pack/internal/config" "github.com/buildpacks/pack/pkg/cache" + "github.com/buildpacks/pack/pkg/logging" "github.com/docker/docker/api/types/filters" "github.com/docker/docker/api/types/volume" @@ -31,19 +33,25 @@ func TestVolumeCache(t *testing.T) { } func testCache(t *testing.T, when spec.G, it spec.S) { - var dockerClient client.CommonAPIClient + var ( + dockerClient client.CommonAPIClient + outBuf bytes.Buffer + logger logging.Logger + ) it.Before(func() { var err error dockerClient, err = client.NewClientWithOpts(client.FromEnv, client.WithVersion("1.38")) h.AssertNil(t, err) + logger = logging.NewSimpleLogger(&outBuf) }) + when("#NewVolumeCache", func() { when("volume cache name is empty", func() { it("adds suffix to calculated name", func() { ref, err := name.ParseReference("my/repo", name.WeakValidation) h.AssertNil(t, err) - subject, _ := cache.NewVolumeCache(ref, cache.CacheInfo{}, "some-suffix", dockerClient) + subject, _ := cache.NewVolumeCache(ref, cache.CacheInfo{}, "some-suffix", dockerClient, logger) if !strings.HasSuffix(subject.Name(), ".some-suffix") { t.Fatalf("Calculated volume name '%s' should end with '.some-suffix'", subject.Name()) } @@ -53,8 +61,8 @@ func testCache(t *testing.T, when spec.G, it spec.S) { ref, err := name.ParseReference("my/repo", name.WeakValidation) h.AssertNil(t, err) - subject, _ := cache.NewVolumeCache(ref, cache.CacheInfo{}, "some-suffix", dockerClient) - expected, _ := cache.NewVolumeCache(ref, cache.CacheInfo{}, "some-suffix", dockerClient) + subject, _ := cache.NewVolumeCache(ref, cache.CacheInfo{}, "some-suffix", dockerClient, logger) + expected, _ := cache.NewVolumeCache(ref, cache.CacheInfo{}, "some-suffix", dockerClient, logger) if subject.Name() != expected.Name() { t.Fatalf("The same repo name should result in the same volume") } @@ -64,11 +72,11 @@ func testCache(t *testing.T, when spec.G, it spec.S) { ref, err := name.ParseReference("my/repo:other-tag", name.WeakValidation) h.AssertNil(t, err) - subject, _ := cache.NewVolumeCache(ref, cache.CacheInfo{}, "some-suffix", dockerClient) + subject, _ := cache.NewVolumeCache(ref, cache.CacheInfo{}, "some-suffix", dockerClient, logger) ref, err = name.ParseReference("my/repo", name.WeakValidation) h.AssertNil(t, err) - notExpected, _ := cache.NewVolumeCache(ref, cache.CacheInfo{}, "some-suffix", dockerClient) + notExpected, _ := cache.NewVolumeCache(ref, cache.CacheInfo{}, "some-suffix", dockerClient, logger) if subject.Name() == notExpected.Name() { t.Fatalf("Different image tags should result in different volumes") } @@ -78,11 +86,11 @@ func testCache(t *testing.T, when spec.G, it spec.S) { ref, err := name.ParseReference("registry.com/my/repo:other-tag", name.WeakValidation) h.AssertNil(t, err) - subject, _ := cache.NewVolumeCache(ref, cache.CacheInfo{}, "some-suffix", dockerClient) + subject, _ := cache.NewVolumeCache(ref, cache.CacheInfo{}, "some-suffix", dockerClient, logger) ref, err = name.ParseReference("my/repo", name.WeakValidation) h.AssertNil(t, err) - notExpected, _ := cache.NewVolumeCache(ref, cache.CacheInfo{}, "some-suffix", dockerClient) + notExpected, _ := cache.NewVolumeCache(ref, cache.CacheInfo{}, "some-suffix", dockerClient, logger) if subject.Name() == notExpected.Name() { t.Fatalf("Different image registries should result in different volumes") } @@ -92,11 +100,11 @@ func testCache(t *testing.T, when spec.G, it spec.S) { ref, err := name.ParseReference("my/repo:latest", name.WeakValidation) h.AssertNil(t, err) - subject, _ := cache.NewVolumeCache(ref, cache.CacheInfo{}, "some-suffix", dockerClient) + subject, _ := cache.NewVolumeCache(ref, cache.CacheInfo{}, "some-suffix", dockerClient, logger) ref, err = name.ParseReference("my/repo", name.WeakValidation) h.AssertNil(t, err) - expected, _ := cache.NewVolumeCache(ref, cache.CacheInfo{}, "some-suffix", dockerClient) + expected, _ := cache.NewVolumeCache(ref, cache.CacheInfo{}, "some-suffix", dockerClient, logger) h.AssertEq(t, subject.Name(), expected.Name()) }) @@ -104,11 +112,11 @@ func testCache(t *testing.T, when spec.G, it spec.S) { ref, err := name.ParseReference("index.docker.io/my/repo", name.WeakValidation) h.AssertNil(t, err) - subject, _ := cache.NewVolumeCache(ref, cache.CacheInfo{}, "some-suffix", dockerClient) + subject, _ := cache.NewVolumeCache(ref, cache.CacheInfo{}, "some-suffix", dockerClient, logger) ref, err = name.ParseReference("my/repo", name.WeakValidation) h.AssertNil(t, err) - expected, _ := cache.NewVolumeCache(ref, cache.CacheInfo{}, "some-suffix", dockerClient) + expected, _ := cache.NewVolumeCache(ref, cache.CacheInfo{}, "some-suffix", dockerClient, logger) h.AssertEq(t, subject.Name(), expected.Name()) }) @@ -116,7 +124,7 @@ func testCache(t *testing.T, when spec.G, it spec.S) { ref, err := name.ParseReference("myregistryhost:5000/fedora/httpd:version1.0", name.WeakValidation) h.AssertNil(t, err) - subject, _ := cache.NewVolumeCache(ref, cache.CacheInfo{}, "some-suffix", dockerClient) + subject, _ := cache.NewVolumeCache(ref, cache.CacheInfo{}, "some-suffix", dockerClient, logger) h.AssertContains(t, subject.Name(), "fedora_httpd_version1.0") h.AssertTrue(t, names.RestrictedNamePattern.MatchString(subject.Name())) @@ -132,9 +140,9 @@ func testCache(t *testing.T, when spec.G, it spec.S) { ref, err := name.ParseReference("my/repo:some-tag", name.WeakValidation) h.AssertNil(t, err) - nameFromNewKey, _ := cache.NewVolumeCache(ref, cache.CacheInfo{}, "some-suffix", dockerClient) // sources a new key + nameFromNewKey, _ := cache.NewVolumeCache(ref, cache.CacheInfo{}, "some-suffix", dockerClient, logger) // sources a new key h.AssertNil(t, os.Setenv("PACK_VOLUME_KEY", "some-volume-key")) - nameFromEnvKey, _ := cache.NewVolumeCache(ref, cache.CacheInfo{}, "some-suffix", dockerClient) // sources key from env + nameFromEnvKey, _ := cache.NewVolumeCache(ref, cache.CacheInfo{}, "some-suffix", dockerClient, logger) // sources key from env h.AssertNotEq(t, nameFromNewKey.Name(), nameFromEnvKey.Name()) }) }) @@ -158,7 +166,7 @@ func testCache(t *testing.T, when spec.G, it spec.S) { ref, err := name.ParseReference("my/repo:some-tag", name.WeakValidation) h.AssertNil(t, err) - nameFromNewKey, _ := cache.NewVolumeCache(ref, cache.CacheInfo{}, "some-suffix", dockerClient) // sources a new key + nameFromNewKey, _ := cache.NewVolumeCache(ref, cache.CacheInfo{}, "some-suffix", dockerClient, logger) // sources a new key cfgContents := ` [volume-keys] @@ -166,7 +174,7 @@ func testCache(t *testing.T, when spec.G, it spec.S) { ` h.AssertNil(t, os.WriteFile(filepath.Join(tmpPackHome, "volume-keys.toml"), []byte(cfgContents), 0755)) // overrides the key that was set - nameFromConfigKey, _ := cache.NewVolumeCache(ref, cache.CacheInfo{}, "some-suffix", dockerClient) // sources key from config + nameFromConfigKey, _ := cache.NewVolumeCache(ref, cache.CacheInfo{}, "some-suffix", dockerClient, logger) // sources key from config h.AssertNotEq(t, nameFromNewKey.Name(), nameFromConfigKey.Name()) }) }) @@ -176,14 +184,32 @@ func testCache(t *testing.T, when spec.G, it spec.S) { ref, err := name.ParseReference("my/repo:some-tag", name.WeakValidation) h.AssertNil(t, err) - nameFromNewKey, _ := cache.NewVolumeCache(ref, cache.CacheInfo{}, "some-suffix", dockerClient) // sources a new key - nameFromConfigKey, _ := cache.NewVolumeCache(ref, cache.CacheInfo{}, "some-suffix", dockerClient) // sources same key from config + nameFromNewKey, _ := cache.NewVolumeCache(ref, cache.CacheInfo{}, "some-suffix", dockerClient, logger) // sources a new key + nameFromConfigKey, _ := cache.NewVolumeCache(ref, cache.CacheInfo{}, "some-suffix", dockerClient, logger) // sources same key from config h.AssertEq(t, nameFromNewKey.Name(), nameFromConfigKey.Name()) cfg, err := config.ReadVolumeKeys(filepath.Join(tmpPackHome, "volume-keys.toml")) h.AssertNil(t, err) h.AssertNotNil(t, cfg.VolumeKeys["index.docker.io/my/repo:some-tag"]) }) + + when("containerized pack", func() { + it.Before(func() { + cache.RunningInContainer = func() bool { + return true + } + }) + + it("logs a warning", func() { + ref, err := name.ParseReference("my/repo:some-tag", name.WeakValidation) + h.AssertNil(t, err) + + _, _ = cache.NewVolumeCache(ref, cache.CacheInfo{}, "some-suffix", dockerClient, logger) // sources a new key + _, _ = cache.NewVolumeCache(ref, cache.CacheInfo{}, "some-suffix", dockerClient, logger) // sources same key from config + h.AssertContains(t, outBuf.String(), "PACK_VOLUME_KEY is unset; set this environment variable to a secret value to avoid creating a new volume cache on every build") + h.AssertEq(t, strings.Count(outBuf.String(), "PACK_VOLUME_KEY is unset"), 1) // the second call to NewVolumeCache reads from the config + }) + }) }) }) }) @@ -200,7 +226,7 @@ func testCache(t *testing.T, when spec.G, it spec.S) { ref, err := name.ParseReference("my/repo", name.WeakValidation) h.AssertNil(t, err) - subject, _ := cache.NewVolumeCache(ref, cacheInfo, "some-suffix", dockerClient) + subject, _ := cache.NewVolumeCache(ref, cacheInfo, "some-suffix", dockerClient, logger) if volumeName != subject.Name() { t.Fatalf("Volume name '%s' should be same as the name specified '%s'", subject.Name(), volumeName) @@ -211,9 +237,9 @@ func testCache(t *testing.T, when spec.G, it spec.S) { ref, err := name.ParseReference("my/repo", name.WeakValidation) h.AssertNil(t, err) - subject, _ := cache.NewVolumeCache(ref, cacheInfo, "some-suffix", dockerClient) + subject, _ := cache.NewVolumeCache(ref, cacheInfo, "some-suffix", dockerClient, logger) - expected, _ := cache.NewVolumeCache(ref, cacheInfo, "some-suffix", dockerClient) + expected, _ := cache.NewVolumeCache(ref, cacheInfo, "some-suffix", dockerClient, logger) if subject.Name() != expected.Name() { t.Fatalf("The same repo name should result in the same volume") } @@ -223,11 +249,11 @@ func testCache(t *testing.T, when spec.G, it spec.S) { ref, err := name.ParseReference("registry.com/my/repo:other-tag", name.WeakValidation) h.AssertNil(t, err) - subject, _ := cache.NewVolumeCache(ref, cache.CacheInfo{}, "some-suffix", dockerClient) + subject, _ := cache.NewVolumeCache(ref, cache.CacheInfo{}, "some-suffix", dockerClient, logger) ref, err = name.ParseReference("my/repo", name.WeakValidation) h.AssertNil(t, err) - notExpected, _ := cache.NewVolumeCache(ref, cache.CacheInfo{}, "some-suffix", dockerClient) + notExpected, _ := cache.NewVolumeCache(ref, cache.CacheInfo{}, "some-suffix", dockerClient, logger) if subject.Name() == notExpected.Name() { t.Fatalf("Different image registries should result in different volumes") } @@ -237,11 +263,11 @@ func testCache(t *testing.T, when spec.G, it spec.S) { ref, err := name.ParseReference("my/repo:latest", name.WeakValidation) h.AssertNil(t, err) - subject, _ := cache.NewVolumeCache(ref, cache.CacheInfo{}, "some-suffix", dockerClient) + subject, _ := cache.NewVolumeCache(ref, cache.CacheInfo{}, "some-suffix", dockerClient, logger) ref, err = name.ParseReference("my/repo", name.WeakValidation) h.AssertNil(t, err) - expected, _ := cache.NewVolumeCache(ref, cache.CacheInfo{}, "some-suffix", dockerClient) + expected, _ := cache.NewVolumeCache(ref, cache.CacheInfo{}, "some-suffix", dockerClient, logger) h.AssertEq(t, subject.Name(), expected.Name()) }) @@ -249,11 +275,11 @@ func testCache(t *testing.T, when spec.G, it spec.S) { ref, err := name.ParseReference("index.docker.io/my/repo", name.WeakValidation) h.AssertNil(t, err) - subject, _ := cache.NewVolumeCache(ref, cache.CacheInfo{}, "some-suffix", dockerClient) + subject, _ := cache.NewVolumeCache(ref, cache.CacheInfo{}, "some-suffix", dockerClient, logger) ref, err = name.ParseReference("my/repo", name.WeakValidation) h.AssertNil(t, err) - expected, _ := cache.NewVolumeCache(ref, cache.CacheInfo{}, "some-suffix", dockerClient) + expected, _ := cache.NewVolumeCache(ref, cache.CacheInfo{}, "some-suffix", dockerClient, logger) h.AssertEq(t, subject.Name(), expected.Name()) }) @@ -261,7 +287,7 @@ func testCache(t *testing.T, when spec.G, it spec.S) { ref, err := name.ParseReference("myregistryhost:5000/fedora/httpd:version1.0", name.WeakValidation) h.AssertNil(t, err) - subject, _ := cache.NewVolumeCache(ref, cache.CacheInfo{}, "some-suffix", dockerClient) + subject, _ := cache.NewVolumeCache(ref, cache.CacheInfo{}, "some-suffix", dockerClient, logger) h.AssertContains(t, subject.Name(), "fedora_httpd_version1.0") h.AssertTrue(t, names.RestrictedNamePattern.MatchString(subject.Name())) @@ -286,7 +312,7 @@ func testCache(t *testing.T, when spec.G, it spec.S) { ref, err := name.ParseReference(h.RandString(10), name.WeakValidation) h.AssertNil(t, err) - subject, _ = cache.NewVolumeCache(ref, cache.CacheInfo{}, "some-suffix", dockerClient) + subject, _ = cache.NewVolumeCache(ref, cache.CacheInfo{}, "some-suffix", dockerClient, logger) volumeName = subject.Name() }) @@ -324,7 +350,7 @@ func testCache(t *testing.T, when spec.G, it spec.S) { it("returns the cache type", func() { ref, err := name.ParseReference("my/repo", name.WeakValidation) h.AssertNil(t, err) - subject, _ := cache.NewVolumeCache(ref, cache.CacheInfo{}, "some-suffix", dockerClient) + subject, _ := cache.NewVolumeCache(ref, cache.CacheInfo{}, "some-suffix", dockerClient, logger) expected := cache.Volume h.AssertEq(t, subject.Type(), expected) })