From 83e94572b2a3676a02747135776f1439217b839d Mon Sep 17 00:00:00 2001 From: Natalie Arellano Date: Thu, 12 Oct 2023 11:30:07 -0400 Subject: [PATCH] When downloading buildpacks or extensions for `pack build` or `pack builder create`, ensure the os/arch matches the builder Signed-off-by: Natalie Arellano --- pkg/buildpack/downloader.go | 15 +++++++++++++-- pkg/buildpack/downloader_test.go | 24 +++++++++++++++--------- pkg/client/build.go | 13 ++++++++++--- pkg/client/build_test.go | 2 ++ pkg/client/create_builder.go | 12 +++++++++--- pkg/client/create_builder_test.go | 14 ++++++++++++-- 6 files changed, 61 insertions(+), 19 deletions(-) diff --git a/pkg/buildpack/downloader.go b/pkg/buildpack/downloader.go index 41a5a61cdb..454afda0cb 100644 --- a/pkg/buildpack/downloader.go +++ b/pkg/buildpack/downloader.go @@ -67,6 +67,9 @@ type DownloadOptions struct { // The OS of the builder image ImageOS string + // The OS/Architecture to download + Platform string + // Deprecated: the older alternative to buildpack URI ImageName string @@ -102,7 +105,11 @@ func (c *buildpackDownloader) Download(ctx context.Context, moduleURI string, op case PackageLocator: imageName := ParsePackageLocator(moduleURI) c.logger.Debugf("Downloading %s from image: %s", kind, style.Symbol(imageName)) - mainBP, depBPs, err = extractPackaged(ctx, kind, imageName, c.imageFetcher, image.FetchOptions{Daemon: opts.Daemon, PullPolicy: opts.PullPolicy}) + mainBP, depBPs, err = extractPackaged(ctx, kind, imageName, c.imageFetcher, image.FetchOptions{ + Daemon: opts.Daemon, + PullPolicy: opts.PullPolicy, + Platform: opts.Platform, + }) if err != nil { return nil, nil, errors.Wrapf(err, "extracting from registry %s", style.Symbol(moduleURI)) } @@ -113,7 +120,11 @@ func (c *buildpackDownloader) Download(ctx context.Context, moduleURI string, op return nil, nil, errors.Wrapf(err, "locating in registry: %s", style.Symbol(moduleURI)) } - mainBP, depBPs, err = extractPackaged(ctx, kind, address, c.imageFetcher, image.FetchOptions{Daemon: opts.Daemon, PullPolicy: opts.PullPolicy}) + mainBP, depBPs, err = extractPackaged(ctx, kind, address, c.imageFetcher, image.FetchOptions{ + Daemon: opts.Daemon, + PullPolicy: opts.PullPolicy, + Platform: opts.Platform, + }) if err != nil { return nil, nil, errors.Wrapf(err, "extracting from registry %s", style.Symbol(moduleURI)) } diff --git a/pkg/buildpack/downloader_test.go b/pkg/buildpack/downloader_test.go index 16c30b6e10..2aefdb571d 100644 --- a/pkg/buildpack/downloader_test.go +++ b/pkg/buildpack/downloader_test.go @@ -127,8 +127,12 @@ func testBuildpackDownloader(t *testing.T, when spec.G, it spec.S) { downloadOptions = buildpack.DownloadOptions{ImageOS: "linux"} ) - shouldFetchPackageImageWith := func(demon bool, pull image.PullPolicy) { - mockImageFetcher.EXPECT().Fetch(gomock.Any(), packageImage.Name(), image.FetchOptions{Daemon: demon, PullPolicy: pull}).Return(packageImage, nil) + shouldFetchPackageImageWith := func(demon bool, pull image.PullPolicy, platform string) { + mockImageFetcher.EXPECT().Fetch(gomock.Any(), packageImage.Name(), image.FetchOptions{ + Daemon: demon, + PullPolicy: pull, + Platform: platform, + }).Return(packageImage, nil) } when("package image lives in cnb registry", func() { @@ -141,11 +145,12 @@ func testBuildpackDownloader(t *testing.T, when spec.G, it spec.S) { downloadOptions = buildpack.DownloadOptions{ RegistryName: "some-registry", ImageOS: "linux", + Platform: "linux/amd64", Daemon: true, PullPolicy: image.PullAlways, } - shouldFetchPackageImageWith(true, image.PullAlways) + shouldFetchPackageImageWith(true, image.PullAlways, "linux/amd64") mainBP, _, err := buildpackDownloader.Download(context.TODO(), "urn:cnb:registry:example/foo@1.1.0", downloadOptions) h.AssertNil(t, err) h.AssertEq(t, mainBP.Descriptor().Info().ID, "example/foo") @@ -161,7 +166,7 @@ func testBuildpackDownloader(t *testing.T, when spec.G, it spec.S) { PullPolicy: image.PullAlways, } - shouldFetchPackageImageWith(true, image.PullAlways) + shouldFetchPackageImageWith(true, image.PullAlways, "") mainBP, _, err := buildpackDownloader.Download(context.TODO(), "example/foo@1.1.0", downloadOptions) h.AssertNil(t, err) h.AssertEq(t, mainBP.Descriptor().Info().ID, "example/foo") @@ -185,10 +190,11 @@ func testBuildpackDownloader(t *testing.T, when spec.G, it spec.S) { Daemon: true, PullPolicy: image.PullAlways, ImageOS: "linux", + Platform: "linux/amd64", ImageName: "some/package:tag", } - shouldFetchPackageImageWith(true, image.PullAlways) + shouldFetchPackageImageWith(true, image.PullAlways, "linux/amd64") mainBP, _, err := buildpackDownloader.Download(context.TODO(), "", downloadOptions) h.AssertNil(t, err) h.AssertEq(t, mainBP.Descriptor().Info().ID, "example/foo") @@ -204,7 +210,7 @@ func testBuildpackDownloader(t *testing.T, when spec.G, it spec.S) { PullPolicy: image.PullAlways, } - shouldFetchPackageImageWith(true, image.PullAlways) + shouldFetchPackageImageWith(true, image.PullAlways, "") mainBP, _, err := buildpackDownloader.Download(context.TODO(), "", downloadOptions) h.AssertNil(t, err) h.AssertEq(t, mainBP.Descriptor().Info().ID, "example/foo") @@ -220,7 +226,7 @@ func testBuildpackDownloader(t *testing.T, when spec.G, it spec.S) { PullPolicy: image.PullAlways, } - shouldFetchPackageImageWith(false, image.PullAlways) + shouldFetchPackageImageWith(false, image.PullAlways, "") mainBP, _, err := buildpackDownloader.Download(context.TODO(), "", downloadOptions) h.AssertNil(t, err) h.AssertEq(t, mainBP.Descriptor().Info().ID, "example/foo") @@ -234,7 +240,7 @@ func testBuildpackDownloader(t *testing.T, when spec.G, it spec.S) { Daemon: false, PullPolicy: image.PullAlways, } - shouldFetchPackageImageWith(false, image.PullAlways) + shouldFetchPackageImageWith(false, image.PullAlways, "") mainBP, _, err := buildpackDownloader.Download(context.TODO(), packageImage.Name(), downloadOptions) h.AssertNil(t, err) h.AssertEq(t, mainBP.Descriptor().Info().ID, "example/foo") @@ -250,7 +256,7 @@ func testBuildpackDownloader(t *testing.T, when spec.G, it spec.S) { PullPolicy: image.PullNever, } - shouldFetchPackageImageWith(false, image.PullNever) + shouldFetchPackageImageWith(false, image.PullNever, "") mainBP, _, err := buildpackDownloader.Download(context.TODO(), "", downloadOptions) h.AssertNil(t, err) h.AssertEq(t, mainBP.Descriptor().Info().ID, "example/foo") diff --git a/pkg/client/build.go b/pkg/client/build.go index c94e862dfd..a46cab5e80 100644 --- a/pkg/client/build.go +++ b/pkg/client/build.go @@ -1032,13 +1032,19 @@ func (c *Client) fetchBuildpack(ctx context.Context, bp string, relativeBaseDir Version: version, } default: - imageOS, err := builderImage.OS() + builderOS, err := builderImage.OS() if err != nil { - return nil, nil, errors.Wrapf(err, "getting OS from %s", style.Symbol(builderImage.Name())) + return nil, nil, errors.Wrapf(err, "getting builder OS") + } + + builderArch, err := builderImage.Architecture() + if err != nil { + return nil, nil, errors.Wrapf(err, "getting builder architecture") } downloadOptions := buildpack.DownloadOptions{ RegistryName: registry, - ImageOS: imageOS, + ImageOS: builderOS, + Platform: fmt.Sprintf("%s/%s", builderOS, builderArch), RelativeBaseDir: relativeBaseDir, Daemon: !publish, PullPolicy: pullPolicy, @@ -1076,6 +1082,7 @@ func (c *Client) fetchBuildpackDependencies(ctx context.Context, bp string, pack mainBP, deps, err := c.buildpackDownloader.Download(ctx, dep.URI, buildpack.DownloadOptions{ RegistryName: downloadOptions.RegistryName, ImageOS: downloadOptions.ImageOS, + Platform: downloadOptions.Platform, Daemon: downloadOptions.Daemon, PullPolicy: downloadOptions.PullPolicy, RelativeBaseDir: filepath.Join(bp, packageCfg.Buildpack.URI), diff --git a/pkg/client/build_test.go b/pkg/client/build_test.go index 2f43619c53..ed5693f7c9 100644 --- a/pkg/client/build_test.go +++ b/pkg/client/build_test.go @@ -1240,6 +1240,8 @@ api = "0.2" Version: "child.buildpack.version", }, }) + args := fakeImageFetcher.FetchCalls[fakePackage.Name()] + h.AssertEq(t, args.Platform, "linux/amd64") }) it("fails when no metadata label on package", func() { diff --git a/pkg/client/create_builder.go b/pkg/client/create_builder.go index 28cd2fa7c5..dc03d73d6b 100644 --- a/pkg/client/create_builder.go +++ b/pkg/client/create_builder.go @@ -256,14 +256,20 @@ func (c *Client) addExtensionsToBuilder(ctx context.Context, opts CreateBuilderO func (c *Client) addConfig(ctx context.Context, kind string, config pubbldr.ModuleConfig, opts CreateBuilderOptions, bldr *builder.Builder) error { c.logger.Debugf("Looking up %s %s", kind, style.Symbol(config.DisplayString())) - imageOS, err := bldr.Image().OS() + builderOS, err := bldr.Image().OS() if err != nil { - return errors.Wrapf(err, "getting OS from %s", style.Symbol(bldr.Image().Name())) + return errors.Wrapf(err, "getting builder OS") } + builderArch, err := bldr.Image().Architecture() + if err != nil { + return errors.Wrapf(err, "getting builder architecture") + } + mainBP, depBPs, err := c.buildpackDownloader.Download(ctx, config.URI, buildpack.DownloadOptions{ Daemon: !opts.Publish, ImageName: config.ImageName, - ImageOS: imageOS, + ImageOS: builderOS, + Platform: fmt.Sprintf("%s/%s", builderOS, builderArch), ModuleKind: kind, PullPolicy: opts.PullPolicy, RegistryName: opts.Registry, diff --git a/pkg/client/create_builder_test.go b/pkg/client/create_builder_test.go index 3b4cfc41ef..ea0c95a939 100644 --- a/pkg/client/create_builder_test.go +++ b/pkg/client/create_builder_test.go @@ -843,12 +843,22 @@ func testCreateBuilder(t *testing.T, when spec.G, it spec.S) { buildpackBlob := blob.NewBlob(filepath.Join("testdata", "buildpack-api-0.4")) bp, err := buildpack.FromBuildpackRootBlob(buildpackBlob, archive.DefaultTarWriterFactory()) h.AssertNil(t, err) - mockBuildpackDownloader.EXPECT().Download(gomock.Any(), "https://example.fake/bp-one-with-api-4.tgz", gomock.Any()).Return(bp, bpDependencies, nil) + mockBuildpackDownloader.EXPECT().Download(gomock.Any(), "https://example.fake/bp-one-with-api-4.tgz", gomock.Any()).DoAndReturn( + func(ctx context.Context, buildpackURI string, opts buildpack.DownloadOptions) (buildpack.BuildModule, []buildpack.BuildModule, error) { + // test options + h.AssertEq(t, opts.Platform, "linux/amd64") + return bp, bpDependencies, nil + }) extensionBlob := blob.NewBlob(filepath.Join("testdata", "extension-api-0.9")) extension, err := buildpack.FromExtensionRootBlob(extensionBlob, archive.DefaultTarWriterFactory()) h.AssertNil(t, err) - mockBuildpackDownloader.EXPECT().Download(gomock.Any(), "https://example.fake/ext-one-with-api-9.tgz", gomock.Any()).Return(extension, nil, nil) + mockBuildpackDownloader.EXPECT().Download(gomock.Any(), "https://example.fake/ext-one-with-api-9.tgz", gomock.Any()).DoAndReturn( + func(ctx context.Context, buildpackURI string, opts buildpack.DownloadOptions) (buildpack.BuildModule, []buildpack.BuildModule, error) { + // test options + h.AssertEq(t, opts.Platform, "linux/amd64") + return extension, nil, nil + }) successfullyCreateDeterministicBuilder()