fixing a little bug when creating a builder without target flag on da…

…rwin Signed-off-by: Juan Bustamante <jbustamante@vmware.com>
buildpacks · May 21, 2024 · dad414b · dad414b
1 parent 3f3b24d
commit dad414b
Show file tree

Hide file tree

Showing 4 changed files with 53 additions and 17 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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",