Skip to content

Commit

Permalink
Refactors Windows package-buildpack to use baselayer shim
Browse files Browse the repository at this point in the history
Signed-off-by: Micah Young <[email protected]>
  • Loading branch information
Micah Young committed Oct 12, 2020
1 parent 8b006ab commit ac3a6c3
Show file tree
Hide file tree
Showing 7 changed files with 249 additions and 243 deletions.
2 changes: 0 additions & 2 deletions acceptance/acceptance_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -237,8 +237,6 @@ func testWithoutSpecificBuilderRequirement(
"pack does not support 'package-buildpack'",
)

h.SkipIf(t, dockerHostOS() == "windows", "These tests are not yet compatible with Windows-based containers")

var err error
tmpDir, err = ioutil.TempDir("", "package-buildpack-tests")
assert.Nil(err)
Expand Down
4 changes: 3 additions & 1 deletion build_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2235,7 +2235,9 @@ func newWindowsImage(name, topLayerSha string, identifier imgutil.Identifier) *f
result := fakes.NewImage(name, topLayerSha, identifier)
arch, _ := result.Architecture()
osVersion, _ := result.OSVersion()
result.SetPlatform("windows", osVersion, arch)
result.SetOS("windows")
result.SetOSVersion(osVersion)
result.SetArchitecture(arch)
return result
}

Expand Down
8 changes: 4 additions & 4 deletions create_builder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -349,7 +349,7 @@ func testCreateBuilder(t *testing.T, when spec.G, it spec.S) {

prepareFetcherWithRunImages()

fakeBuildImage.SetPlatform("windows", "0123", "amd64")
h.AssertNil(t, fakeBuildImage.SetOS("windows"))
mockImageFetcher.EXPECT().Fetch(gomock.Any(), "some/build-image", true, config.PullAlways).Return(fakeBuildImage, nil)

err = packClientWithExperimental.CreateBuilder(context.TODO(), opts)
Expand All @@ -361,7 +361,7 @@ func testCreateBuilder(t *testing.T, when spec.G, it spec.S) {
it("fails", func() {
prepareFetcherWithRunImages()

fakeBuildImage.SetPlatform("windows", "0123", "amd64")
h.AssertNil(t, fakeBuildImage.SetOS("windows"))
mockImageFetcher.EXPECT().Fetch(gomock.Any(), "some/build-image", true, config.PullAlways).Return(fakeBuildImage, nil)

err := subject.CreateBuilder(context.TODO(), opts)
Expand Down Expand Up @@ -428,7 +428,7 @@ func testCreateBuilder(t *testing.T, when spec.G, it spec.S) {
prepareFetcherWithRunImages()
opts.Config.Lifecycle.URI = ""
opts.Config.Lifecycle.Version = "3.4.5"
fakeBuildImage.SetPlatform("windows", "0123", "amd64")
h.AssertNil(t, fakeBuildImage.SetOS("windows"))

mockDownloader.EXPECT().Download(
gomock.Any(),
Expand Down Expand Up @@ -480,7 +480,7 @@ func testCreateBuilder(t *testing.T, when spec.G, it spec.S) {
prepareFetcherWithRunImages()
opts.Config.Lifecycle.URI = ""
opts.Config.Lifecycle.Version = ""
fakeBuildImage.SetPlatform("windows", "0123", "amd64")
h.AssertNil(t, fakeBuildImage.SetOS("windows"))

mockDownloader.EXPECT().Download(
gomock.Any(),
Expand Down
6 changes: 3 additions & 3 deletions internal/build/phase_config_provider_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ func testPhaseConfigProvider(t *testing.T, when spec.G, it spec.S) {
when("building for Windows", func() {
it("sets process isolation", func() {
fakeBuilderImage := ifakes.NewImage("fake-builder", "", nil)
fakeBuilderImage.SetPlatform("windows", "", "")
h.AssertNil(t, fakeBuilderImage.SetOS("windows"))
fakeBuilder, err := fakes.NewFakeBuilder(fakes.WithImage(fakeBuilderImage))
h.AssertNil(t, err)
lifecycle := newTestLifecycleExec(t, false, fakes.WithBuilder(fakeBuilder))
Expand Down Expand Up @@ -141,7 +141,7 @@ func testPhaseConfigProvider(t *testing.T, when spec.G, it spec.S) {
when("building for Windows", func() {
it("sets daemon access on the config", func() {
fakeBuilderImage := ifakes.NewImage("fake-builder", "", nil)
fakeBuilderImage.SetPlatform("windows", "", "")
h.AssertNil(t, fakeBuilderImage.SetOS("windows"))
fakeBuilder, err := fakes.NewFakeBuilder(fakes.WithImage(fakeBuilderImage))
h.AssertNil(t, err)
lifecycle := newTestLifecycleExec(t, false, fakes.WithBuilder(fakeBuilder))
Expand Down Expand Up @@ -242,7 +242,7 @@ func testPhaseConfigProvider(t *testing.T, when spec.G, it spec.S) {
when("building for Windows", func() {
it("sets root user on the config", func() {
fakeBuilderImage := ifakes.NewImage("fake-builder", "", nil)
fakeBuilderImage.SetPlatform("windows", "", "")
h.AssertNil(t, fakeBuilderImage.SetOS("windows"))
fakeBuilder, err := fakes.NewFakeBuilder(fakes.WithImage(fakeBuilderImage))
h.AssertNil(t, err)
lifecycle := newTestLifecycleExec(t, false, fakes.WithBuilder(fakeBuilder))
Expand Down
103 changes: 68 additions & 35 deletions internal/buildpackage/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,12 @@ package buildpackage
import (
"archive/tar"
"compress/gzip"
"context"
"io"
"io/ioutil"
"os"

"github.com/buildpacks/imgutil/layer"

"github.com/buildpacks/imgutil"
v1 "github.com/google/go-containerregistry/pkg/v1"
"github.com/google/go-containerregistry/pkg/v1/empty"
Expand All @@ -15,24 +17,18 @@ import (
"github.com/google/go-containerregistry/pkg/v1/tarball"
"github.com/pkg/errors"

pubcfg "github.com/buildpacks/pack/config"
"github.com/buildpacks/pack/internal/archive"
"github.com/buildpacks/pack/internal/dist"
"github.com/buildpacks/pack/internal/stack"
"github.com/buildpacks/pack/internal/style"
)

const windowsPackageBase = "mcr.microsoft.com/windows/nanoserver:1809-amd64" // TODO: Should this be hard-coded?

type ImageFactory interface {
NewImage(repoName string, local bool) (imgutil.Image, error)
}

type ImageFetcher interface {
Fetch(ctx context.Context, name string, daemon bool, pullPolicy pubcfg.PullPolicy) (imgutil.Image, error)
}

type WorkableImage interface {
SetOS(string) error
SetLabel(string, string) error
AddLayerWithDiffID(path, diffID string) error
}
Expand All @@ -55,12 +51,22 @@ func (i *layoutImage) SetLabel(key string, val string) error {
return err
}

func (i *layoutImage) SetOS(osVal string) error {
configFile, err := i.ConfigFile()
if err != nil {
return err
}
configFile.OS = osVal
i.Image, err = mutate.ConfigFile(i.Image, configFile)
return err
}

func (i *layoutImage) AddLayerWithDiffID(path, _ string) error {
layer, err := tarball.LayerFromFile(path, tarball.WithCompressionLevel(gzip.DefaultCompression))
tarLayer, err := tarball.LayerFromFile(path, tarball.WithCompressionLevel(gzip.DefaultCompression))
if err != nil {
return err
}
i.Image, err = mutate.AppendLayers(i.Image, layer)
i.Image, err = mutate.AppendLayers(i.Image, tarLayer)
if err != nil {
return errors.Wrap(err, "add layer")
}
Expand All @@ -70,16 +76,12 @@ func (i *layoutImage) AddLayerWithDiffID(path, _ string) error {
type PackageBuilder struct {
buildpack dist.Buildpack
dependencies []dist.Buildpack
imageOS string
imageFactory ImageFactory
imageFetcher ImageFetcher
}

func NewBuilder(imageOS string, imageFactory ImageFactory, imageFetcher ImageFetcher) *PackageBuilder {
func NewBuilder(imageFactory ImageFactory) *PackageBuilder {
return &PackageBuilder{
imageOS: imageOS,
imageFactory: imageFactory,
imageFetcher: imageFetcher,
}
}

Expand All @@ -91,14 +93,24 @@ func (b *PackageBuilder) AddDependency(buildpack dist.Buildpack) {
b.dependencies = append(b.dependencies, buildpack)
}

func (b *PackageBuilder) finalizeImage(tmpDir string, image WorkableImage) error {
func (b *PackageBuilder) finalizeImage(image WorkableImage, imageOS, tmpDir string) error {
if err := dist.SetLabel(image, MetadataLabel, &Metadata{
BuildpackInfo: b.buildpack.Descriptor().Info,
Stacks: b.resolvedStacks(),
}); err != nil {
return err
}

if err := image.SetOS(imageOS); err != nil {
return err
}

if imageOS == "windows" {
if err := addWindowsShimBaseLayer(image, tmpDir); err != nil {
return err
}
}

bpLayers := dist.BuildpackLayers{}
for _, bp := range append(b.dependencies, b.buildpack) {
bpLayerTar, err := dist.BuildpackToLayerTar(tmpDir, bp)
Expand Down Expand Up @@ -128,6 +140,39 @@ func (b *PackageBuilder) finalizeImage(tmpDir string, image WorkableImage) error
return nil
}

func addWindowsShimBaseLayer(image WorkableImage, tmpDir string) error {
baseLayerFile, err := ioutil.TempFile(tmpDir, "windows-baselayer")
if err != nil {
return err
}
defer baseLayerFile.Close()

baseLayer, err := layer.WindowsBaseLayer()
if err != nil {
return err
}

if _, err := io.Copy(baseLayerFile, baseLayer); err != nil {
return err
}

if err := baseLayerFile.Close(); err != nil {
return err
}

baseLayerPath := baseLayerFile.Name()
diffID, err := dist.LayerDiffID(baseLayerPath)
if err != nil {
return err
}

if err := image.AddLayerWithDiffID(baseLayerPath, diffID.String()); err != nil {
return err
}

return nil
}

func (b *PackageBuilder) validate() error {
if b.buildpack == nil {
return errors.New("buildpack must be set")
Expand Down Expand Up @@ -159,7 +204,7 @@ func (b *PackageBuilder) resolvedStacks() []dist.Stack {
return stacks
}

func (b *PackageBuilder) SaveAsFile(path string) error {
func (b *PackageBuilder) SaveAsFile(path, imageOS string) error {
if err := b.validate(); err != nil {
return err
}
Expand All @@ -174,7 +219,7 @@ func (b *PackageBuilder) SaveAsFile(path string) error {
}
defer os.RemoveAll(tmpDir)

if err := b.finalizeImage(tmpDir, layoutImage); err != nil {
if err := b.finalizeImage(layoutImage, imageOS, tmpDir); err != nil {
return err
}

Expand Down Expand Up @@ -204,26 +249,14 @@ func (b *PackageBuilder) SaveAsFile(path string) error {
return archive.WriteDirToTar(tw, layoutDir, "/", 0, 0, 0755, true, nil)
}

func (b *PackageBuilder) SaveAsImage(ctx context.Context, repoName string, publish bool, pullPolicy pubcfg.PullPolicy) (imgutil.Image, error) {
func (b *PackageBuilder) SaveAsImage(repoName string, publish bool, imageOS string) (imgutil.Image, error) {
if err := b.validate(); err != nil {
return nil, err
}

var (
image imgutil.Image
err error
)
if b.imageOS == "windows" {
image, err = b.imageFetcher.Fetch(ctx, windowsPackageBase, !publish, pullPolicy)
if err != nil {
return nil, errors.Wrapf(err, "fetching base image")
}
image.Rename(repoName)
} else {
image, err = b.imageFactory.NewImage(repoName, !publish)
if err != nil {
return nil, errors.Wrapf(err, "creating image")
}
image, err := b.imageFactory.NewImage(repoName, !publish)
if err != nil {
return nil, errors.Wrapf(err, "creating image")
}

tmpDir, err := ioutil.TempDir("", "package-buildpack")
Expand All @@ -232,7 +265,7 @@ func (b *PackageBuilder) SaveAsImage(ctx context.Context, repoName string, publi
}
defer os.RemoveAll(tmpDir)

if err := b.finalizeImage(tmpDir, image); err != nil {
if err := b.finalizeImage(image, imageOS, tmpDir); err != nil {
return nil, err
}

Expand Down
Loading

0 comments on commit ac3a6c3

Please sign in to comment.