From dad414b6a50effb0423d34b1d0b4e5ae8546463f Mon Sep 17 00:00:00 2001 From: Juan Bustamante Date: Tue, 21 May 2024 10:43:06 -0500 Subject: [PATCH] fixing a little bug when creating a builder without target flag on darwin Signed-off-by: Juan Bustamante --- pkg/buildpack/downloader_test.go | 2 ++ pkg/client/create_builder.go | 31 ++++++++++++++++++++++------ pkg/client/package_buildpack.go | 15 +++++++------- pkg/client/package_buildpack_test.go | 22 ++++++++++++++++---- 4 files changed, 53 insertions(+), 17 deletions(-) diff --git a/pkg/buildpack/downloader_test.go b/pkg/buildpack/downloader_test.go index 03ce73b0e..3c4d33554 100644 --- a/pkg/buildpack/downloader_test.go +++ b/pkg/buildpack/downloader_test.go @@ -10,6 +10,7 @@ import ( "github.com/buildpacks/imgutil/fakes" "github.com/buildpacks/lifecycle/api" + "github.com/docker/docker/api/types" "github.com/docker/docker/api/types/system" "github.com/golang/mock/gomock" "github.com/heroku/color" @@ -61,6 +62,7 @@ func testBuildpackDownloader(t *testing.T, when spec.G, it spec.S) { var createPackage = func(imageName string) *fakes.Image { packageImage := fakes.NewImage(imageName, "", nil) mockImageFactory.EXPECT().NewImage(packageImage.Name(), false, dist.Target{OS: "linux"}).Return(packageImage, nil) + mockDockerClient.EXPECT().ServerVersion(gomock.Any()).Return(types.Version{Os: "linux", Arch: "amd64"}, nil) pack, err := client.NewClient( client.WithLogger(logger), diff --git a/pkg/client/create_builder.go b/pkg/client/create_builder.go index d2b184f01..fe2bfc50f 100644 --- a/pkg/client/create_builder.go +++ b/pkg/client/create_builder.go @@ -363,19 +363,38 @@ func (c *Client) addConfig(ctx context.Context, kind string, config pubbldr.Modu func (c *Client) processBuilderCreateTargets(ctx context.Context, opts CreateBuilderOptions) ([]dist.Target, error) { var targets []dist.Target - if len(opts.Targets) > 0 { - // when exporting to the daemon, we need to select just one target - if !opts.Publish { - daemonTarget, err := c.daemonTarget(ctx, opts.Targets) + + // For backward compatibility, we must create a target with de default platform used in the Fetch implementation for daemon and remote + // When daemon + // OS: daemonInfo.Os, + // Architecture: daemonInfo.Arch + // When remote || layout + // OS: "linux", + // Architecture: runtime.GOARCH + + if !opts.Publish { + // daemon option + info, err := c.docker.ServerVersion(ctx) + if err != nil { + return targets, err + } + if len(opts.Targets) > 0 { + // when exporting to the daemon, we need to select just one target + daemonTarget, err := c.daemonTarget(info, opts.Targets) if err != nil { return targets, err } targets = append(targets, daemonTarget) } else { - targets = opts.Targets + targets = append(targets, dist.Target{OS: info.Os, Arch: info.Arch}) } } else { - targets = append(targets, dist.Target{OS: runtime.GOOS, Arch: runtime.GOARCH}) + // remote option + if len(opts.Targets) > 0 { + targets = opts.Targets + } else { + targets = append(targets, dist.Target{OS: "linux", Arch: runtime.GOARCH}) + } } return targets, nil } diff --git a/pkg/client/package_buildpack.go b/pkg/client/package_buildpack.go index 854068551..ed08cd8b7 100644 --- a/pkg/client/package_buildpack.go +++ b/pkg/client/package_buildpack.go @@ -5,6 +5,7 @@ import ( "fmt" "path/filepath" + "github.com/docker/docker/api/types" "github.com/pkg/errors" pubbldpkg "github.com/buildpacks/pack/buildpackage" @@ -228,10 +229,14 @@ func (c *Client) downloadBuildpackFromURI(ctx context.Context, uri, relativeBase func (c *Client) processPackageBuildpackTargets(ctx context.Context, opts PackageBuildpackOptions) ([]dist.Target, error) { var targets []dist.Target + info, err := c.docker.ServerVersion(ctx) + if err != nil { + return targets, err + } if len(opts.Targets) > 0 { // when exporting to the daemon, we need to select just one target if !opts.Publish && opts.Format == FormatImage { - daemonTarget, err := c.daemonTarget(ctx, opts.Targets) + daemonTarget, err := c.daemonTarget(info, opts.Targets) if err != nil { return targets, err } @@ -262,12 +267,8 @@ func (c *Client) validateOSPlatform(ctx context.Context, os string, publish bool return nil } -func (c *Client) daemonTarget(ctx context.Context, targets []dist.Target) (dist.Target, error) { - info, err := c.docker.ServerVersion(ctx) - if err != nil { - return dist.Target{}, err - } - +// daemonTarget returns a target that matches with the given daemon os/arch +func (c *Client) daemonTarget(info types.Version, targets []dist.Target) (dist.Target, error) { for _, t := range targets { if t.Arch != "" && t.OS == info.Os && t.Arch == info.Arch { return t, nil diff --git a/pkg/client/package_buildpack_test.go b/pkg/client/package_buildpack_test.go index 3cdaffd6a..9cbdeb94f 100644 --- a/pkg/client/package_buildpack_test.go +++ b/pkg/client/package_buildpack_test.go @@ -11,6 +11,7 @@ import ( "github.com/buildpacks/imgutil" "github.com/buildpacks/imgutil/fakes" "github.com/buildpacks/lifecycle/api" + "github.com/docker/docker/api/types" "github.com/docker/docker/api/types/system" "github.com/golang/mock/gomock" "github.com/google/go-containerregistry/pkg/name" @@ -85,6 +86,10 @@ func testPackageBuildpack(t *testing.T, when spec.G, it spec.S) { } when("buildpack has issues", func() { + it.Before(func() { + mockDockerClient.EXPECT().ServerVersion(gomock.Any()).Return(types.Version{Os: "linux"}, nil) + }) + when("buildpack has no URI", func() { it("should fail", func() { err := subject.PackageBuildpack(context.TODO(), client.PackageBuildpackOptions{ @@ -140,7 +145,7 @@ func testPackageBuildpack(t *testing.T, when spec.G, it spec.S) { it("should fail", func() { dependencyPath := "http://example.com/flawed.file" mockDownloader.EXPECT().Download(gomock.Any(), dependencyPath).Return(blob.NewBlob("no-file.txt"), nil).AnyTimes() - + mockDockerClient.EXPECT().ServerVersion(gomock.Any()).Return(types.Version{Os: "linux"}, nil) mockDockerClient.EXPECT().Info(context.TODO()).Return(system.Info{OSType: "linux"}, nil).AnyTimes() packageDescriptor := dist.BuildpackDescriptor{ @@ -176,7 +181,7 @@ func testPackageBuildpack(t *testing.T, when spec.G, it spec.S) { for _, daemonOS := range []string{"linux", "windows"} { localMockDockerClient := testmocks.NewMockCommonAPIClient(mockController) localMockDockerClient.EXPECT().Info(context.TODO()).Return(system.Info{OSType: daemonOS}, nil).AnyTimes() - + localMockDockerClient.EXPECT().ServerVersion(gomock.Any()).Return(types.Version{Os: daemonOS}, nil) packClientWithExperimental, err := client.NewClient( client.WithDockerClient(localMockDockerClient), client.WithDownloader(mockDownloader), @@ -210,7 +215,7 @@ func testPackageBuildpack(t *testing.T, when spec.G, it spec.S) { it("fails without experimental on Windows daemons", func() { windowsMockDockerClient := testmocks.NewMockCommonAPIClient(mockController) - + windowsMockDockerClient.EXPECT().ServerVersion(gomock.Any()).Return(types.Version{Os: "windows"}, nil) packClientWithoutExperimental, err := client.NewClient( client.WithDockerClient(windowsMockDockerClient), client.WithExperimental(false), @@ -230,7 +235,7 @@ func testPackageBuildpack(t *testing.T, when spec.G, it spec.S) { it("fails for mismatched platform and daemon os", func() { windowsMockDockerClient := testmocks.NewMockCommonAPIClient(mockController) windowsMockDockerClient.EXPECT().Info(context.TODO()).Return(system.Info{OSType: "windows"}, nil).AnyTimes() - + windowsMockDockerClient.EXPECT().ServerVersion(gomock.Any()).Return(types.Version{Os: "windows"}, nil) packClientWithoutExperimental, err := client.NewClient( client.WithDockerClient(windowsMockDockerClient), client.WithExperimental(false), @@ -257,6 +262,7 @@ func testPackageBuildpack(t *testing.T, when spec.G, it spec.S) { mockImageFactory.EXPECT().NewImage(nestedPackage.Name(), false, dist.Target{OS: "linux"}).Return(nestedPackage, nil) mockDockerClient.EXPECT().Info(context.TODO()).Return(system.Info{OSType: "linux"}, nil).AnyTimes() + mockDockerClient.EXPECT().ServerVersion(gomock.Any()).Return(types.Version{Os: "linux"}, nil).AnyTimes() h.AssertNil(t, subject.PackageBuildpack(context.TODO(), client.PackageBuildpackOptions{ Name: nestedPackage.Name(), @@ -402,6 +408,7 @@ func testPackageBuildpack(t *testing.T, when spec.G, it spec.S) { mockImageFetcher.EXPECT().Fetch(gomock.Any(), notPackageImage.Name(), image.FetchOptions{Daemon: true, PullPolicy: image.PullAlways, Target: &dist.Target{OS: "linux"}}).Return(notPackageImage, nil) mockDockerClient.EXPECT().Info(context.TODO()).Return(system.Info{OSType: "linux"}, nil).AnyTimes() + mockDockerClient.EXPECT().ServerVersion(gomock.Any()).Return(types.Version{Os: "linux"}, nil).AnyTimes() h.AssertError(t, subject.PackageBuildpack(context.TODO(), client.PackageBuildpackOptions{ Name: "some/package", @@ -457,6 +464,7 @@ func testPackageBuildpack(t *testing.T, when spec.G, it spec.S) { h.AssertNil(t, err) mockDockerClient.EXPECT().Info(context.TODO()).Return(system.Info{OSType: "linux"}, nil).AnyTimes() + mockDockerClient.EXPECT().ServerVersion(gomock.Any()).Return(types.Version{Os: "linux"}, nil).AnyTimes() name := "basic/package-" + h.RandString(12) fakeImage := fakes.NewImage(name, "", nil) @@ -536,6 +544,8 @@ func testPackageBuildpack(t *testing.T, when spec.G, it spec.S) { repoName = "basic/multi-platform-package-" + h.RandString(12) indexLocalPath = filepath.Join(tmpDir, imgutil.MakeFileSafeName(repoName)) + + mockDockerClient.EXPECT().ServerVersion(gomock.Any()).Return(types.Version{Os: "linux"}, nil).AnyTimes() }) it.After(func() { @@ -838,6 +848,7 @@ func testPackageBuildpack(t *testing.T, when spec.G, it spec.S) { for _, imageOS := range []string{"linux", "windows"} { localMockDockerClient := testmocks.NewMockCommonAPIClient(mockController) localMockDockerClient.EXPECT().Info(context.TODO()).Return(system.Info{OSType: imageOS}, nil).AnyTimes() + localMockDockerClient.EXPECT().ServerVersion(gomock.Any()).Return(types.Version{Os: imageOS}, nil) packClientWithExperimental, err := client.NewClient( client.WithDockerClient(localMockDockerClient), @@ -898,6 +909,8 @@ func testPackageBuildpack(t *testing.T, when spec.G, it spec.S) { tmpDir, err = os.MkdirTemp("", "package-buildpack") h.AssertNil(t, err) + + mockDockerClient.EXPECT().ServerVersion(gomock.Any()).Return(types.Version{Os: "linux"}, nil).AnyTimes() }) it.After(func() { @@ -1189,6 +1202,7 @@ func testPackageBuildpack(t *testing.T, when spec.G, it spec.S) { when("unknown format is provided", func() { it("should error", func() { mockDockerClient.EXPECT().Info(context.TODO()).Return(system.Info{OSType: "linux"}, nil).AnyTimes() + mockDockerClient.EXPECT().ServerVersion(gomock.Any()).Return(types.Version{Os: "linux"}, nil).AnyTimes() err := subject.PackageBuildpack(context.TODO(), client.PackageBuildpackOptions{ Name: "some-buildpack",