Skip to content

Commit

Permalink
fixing a little bug when creating a builder without target flag on da…
Browse files Browse the repository at this point in the history
…rwin

Signed-off-by: Juan Bustamante <[email protected]>
  • Loading branch information
jjbustamante committed May 21, 2024
1 parent 3f3b24d commit dad414b
Show file tree
Hide file tree
Showing 4 changed files with 53 additions and 17 deletions.
2 changes: 2 additions & 0 deletions pkg/buildpack/downloader_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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),
Expand Down
31 changes: 25 additions & 6 deletions pkg/client/create_builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
15 changes: 8 additions & 7 deletions pkg/client/package_buildpack.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"fmt"
"path/filepath"

"github.com/docker/docker/api/types"
"github.com/pkg/errors"

pubbldpkg "github.com/buildpacks/pack/buildpackage"
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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
Expand Down
22 changes: 18 additions & 4 deletions pkg/client/package_buildpack_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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),
Expand All @@ -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),
Expand All @@ -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(),
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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",
Expand Down

0 comments on commit dad414b

Please sign in to comment.