Merge pull request #926 from aemengo/feature/package-buildpack-os-con…

…figuration

Move package `os` to be configured via `package.toml`

acceptance/acceptance_test.go

@@ -235,9 +235,9 @@ func testWithoutSpecificBuilderRequirement(
 
 	when("package-buildpack", func() {
 		var (
-			tmpDir                  string
-			simplePackageConfigPath string
-			buildpackManager        buildpacks.BuildpackManager
+			tmpDir                         string
+			buildpackManager               buildpacks.BuildpackManager
+			simplePackageConfigFixtureName = "package.toml"
 		)
 
 		it.Before(func() {
@@ -250,9 +250,6 @@ func testWithoutSpecificBuilderRequirement(
 			tmpDir, err = ioutil.TempDir("", "package-buildpack-tests")
 			assert.Nil(err)
 
-			simplePackageConfigPath = filepath.Join(tmpDir, "package.toml")
-			h.CopyFile(t, pack.FixtureManager().FixtureLocation("package.toml"), simplePackageConfigPath)
-
 			buildpackManager = buildpacks.NewBuildpackManager(t, assert)
 			buildpackManager.PrepareBuildpacks(tmpDir, buildpacks.SimpleLayersParent, buildpacks.SimpleLayers)
 		})
@@ -268,7 +265,7 @@ func testWithoutSpecificBuilderRequirement(
 
 		}
 
-		generateAggregatePackageToml := func(buildpackURI, nestedPackageName string) string {
+		generateAggregatePackageToml := func(buildpackURI, nestedPackageName, os string) string {
 			t.Helper()
 			packageTomlFile, err := ioutil.TempFile(tmpDir, "package_aggregate-*.toml")
 			assert.Nil(err)
@@ -279,6 +276,7 @@ func testWithoutSpecificBuilderRequirement(
 				map[string]interface{}{
 					"BuildpackURI": buildpackURI,
 					"PackageName":  nestedPackageName,
+					"OS":           os,
 				},
 			)
 
@@ -290,7 +288,9 @@ func testWithoutSpecificBuilderRequirement(
 		when("no --format is provided", func() {
 			it("creates the package as image", func() {
 				packageName := "test/package-" + h.RandString(10)
-				output := pack.RunSuccessfully("package-buildpack", packageName, "-c", simplePackageConfigPath)
+				packageTomlPath := generatePackageTomlWithOS(t, assert, pack, tmpDir, simplePackageConfigFixtureName, dockerHostOS())
+
+				output := pack.RunSuccessfully("package-buildpack", packageName, "-c", packageTomlPath)
 				assertions.NewOutputAssertionManager(t, output).ReportsPackageCreation(packageName)
 				defer h.DockerRmi(dockerCli, packageName)
 
@@ -304,7 +304,8 @@ func testWithoutSpecificBuilderRequirement(
 				nestedPackageName := "test/package-" + h.RandString(10)
 				packageName := "test/package-" + h.RandString(10)
 
-				aggregatePackageToml := generateAggregatePackageToml("simple-layers-parent-buildpack.tgz", nestedPackageName)
+				packageTomlPath := generatePackageTomlWithOS(t, assert, pack, tmpDir, simplePackageConfigFixtureName, dockerHostOS())
+				aggregatePackageToml := generateAggregatePackageToml("simple-layers-parent-buildpack.tgz", nestedPackageName, dockerHostOS())
 
 				packageBuildpack := buildpacks.NewPackageImage(
 					t,
@@ -317,7 +318,7 @@ func testWithoutSpecificBuilderRequirement(
 							t,
 							pack,
 							nestedPackageName,
-							simplePackageConfigPath,
+							packageTomlPath,
 							buildpacks.WithRequiredBuildpacks(buildpacks.SimpleLayers),
 						),
 					),
@@ -333,27 +334,26 @@ func testWithoutSpecificBuilderRequirement(
 				it("publishes image to registry", func() {
 					h.SkipIf(t, !pack.Supports("package-buildpack --os"), "os not supported")
 
+					packageTomlPath := generatePackageTomlWithOS(t, assert, pack, tmpDir, simplePackageConfigFixtureName, dockerHostOS())
 					nestedPackageName := registryConfig.RepoName("test/package-" + h.RandString(10))
 
 					nestedPackage := buildpacks.NewPackageImage(
 						t,
 						pack,
 						nestedPackageName,
-						simplePackageConfigPath,
+						packageTomlPath,
 						buildpacks.WithRequiredBuildpacks(buildpacks.SimpleLayers),
 						buildpacks.WithPublish(),
-						buildpacks.WithOS(dockerHostOS()),
 					)
 					buildpackManager.PrepareBuildpacks(tmpDir, nestedPackage)
 					defer h.DockerRmi(dockerCli, nestedPackageName)
 
-					aggregatePackageToml := generateAggregatePackageToml("simple-layers-parent-buildpack.tgz", nestedPackageName)
+					aggregatePackageToml := generateAggregatePackageToml("simple-layers-parent-buildpack.tgz", nestedPackageName, dockerHostOS())
 					packageName := registryConfig.RepoName("test/package-" + h.RandString(10))
 					output := pack.RunSuccessfully(
 						"package-buildpack", packageName,
 						"-c", aggregatePackageToml,
 						"--publish",
-						"--os", dockerHostOS(),
 					)
 					defer h.DockerRmi(dockerCli, packageName)
 					assertions.NewOutputAssertionManager(t, output).ReportsPackagePublished(packageName)
@@ -370,17 +370,18 @@ func testWithoutSpecificBuilderRequirement(
 
 			when("--pull-policy=never", func() {
 				it("should use local image", func() {
+					packageTomlPath := generatePackageTomlWithOS(t, assert, pack, tmpDir, simplePackageConfigFixtureName, dockerHostOS())
 					nestedPackageName := "test/package-" + h.RandString(10)
 					nestedPackage := buildpacks.NewPackageImage(
 						t,
 						pack,
 						nestedPackageName,
-						simplePackageConfigPath,
+						packageTomlPath,
 						buildpacks.WithRequiredBuildpacks(buildpacks.SimpleLayers),
 					)
 					buildpackManager.PrepareBuildpacks(tmpDir, nestedPackage)
 					defer h.DockerRmi(dockerCli, nestedPackageName)
-					aggregatePackageToml := generateAggregatePackageToml("simple-layers-parent-buildpack.tgz", nestedPackageName)
+					aggregatePackageToml := generateAggregatePackageToml("simple-layers-parent-buildpack.tgz", nestedPackageName, dockerHostOS())
 
 					packageName := registryConfig.RepoName("test/package-" + h.RandString(10))
 					defer h.DockerRmi(dockerCli, packageName)
@@ -396,18 +397,19 @@ func testWithoutSpecificBuilderRequirement(
 				})
 
 				it("should not pull image from registry", func() {
+					packageTomlPath := generatePackageTomlWithOS(t, assert, pack, tmpDir, simplePackageConfigFixtureName, dockerHostOS())
 					nestedPackageName := registryConfig.RepoName("test/package-" + h.RandString(10))
 					nestedPackage := buildpacks.NewPackageImage(
 						t,
 						pack,
 						nestedPackageName,
-						simplePackageConfigPath,
+						packageTomlPath,
 						buildpacks.WithPublish(),
 						buildpacks.WithRequiredBuildpacks(buildpacks.SimpleLayers),
 					)
 					buildpackManager.PrepareBuildpacks(tmpDir, nestedPackage)
 					defer h.DockerRmi(dockerCli, nestedPackageName)
-					aggregatePackageToml := generateAggregatePackageToml("simple-layers-parent-buildpack.tgz", nestedPackageName)
+					aggregatePackageToml := generateAggregatePackageToml("simple-layers-parent-buildpack.tgz", nestedPackageName, dockerHostOS())
 
 					packageName := registryConfig.RepoName("test/package-" + h.RandString(10))
 					defer h.DockerRmi(dockerCli, packageName)
@@ -428,11 +430,12 @@ func testWithoutSpecificBuilderRequirement(
 			})
 
 			it("creates the package", func() {
+				packageTomlPath := generatePackageTomlWithOS(t, assert, pack, tmpDir, simplePackageConfigFixtureName, dockerHostOS())
 				destinationFile := filepath.Join(tmpDir, "package.cnb")
 				output, err := pack.Run(
 					"package-buildpack", destinationFile,
 					"--format", "file",
-					"-c", simplePackageConfigPath,
+					"-c", packageTomlPath,
 				)
 				assert.Nil(err)
 				assertions.NewOutputAssertionManager(t, output).ReportsPackageCreation(destinationFile)
@@ -538,16 +541,17 @@ func testWithoutSpecificBuilderRequirement(
 						fmt.Sprintf("buildpack-%s.cnb", h.RandString(8)),
 					)
 
+					packageTomlPath := generatePackageTomlWithOS(t, assert, pack, tmpDir, "package_for_build_cmd.toml", dockerHostOS())
+
 					packageFile := buildpacks.NewPackageFile(
 						t,
 						pack,
 						packageFileLocation,
-						pack.FixtureManager().FixtureLocation("package_for_build_cmd.toml"),
+						packageTomlPath,
 						buildpacks.WithRequiredBuildpacks(
 							buildpacks.FolderSimpleLayersParent,
 							buildpacks.FolderSimpleLayers,
 						),
-						buildpacks.WithOS(dockerHostOS()),
 					)
 
 					buildpackManager.PrepareBuildpacks(tmpDir, packageFile)
@@ -570,13 +574,14 @@ func testWithoutSpecificBuilderRequirement(
 		when("buildpack image", func() {
 			when("inspect-buildpack", func() {
 				it("succeeds", func() {
+					packageTomlPath := generatePackageTomlWithOS(t, assert, pack, tmpDir, "package_for_build_cmd.toml", dockerHostOS())
 					packageImageName := registryConfig.RepoName("buildpack-" + h.RandString(8))
 
 					packageImage := buildpacks.NewPackageImage(
 						t,
 						pack,
 						packageImageName,
-						pack.FixtureManager().FixtureLocation("package_for_build_cmd.toml"),
+						packageTomlPath,
 						buildpacks.WithRequiredBuildpacks(
 							buildpacks.FolderSimpleLayersParent,
 							buildpacks.FolderSimpleLayers,
@@ -1327,7 +1332,6 @@ func testAcceptance(
 										buildpacks.FolderSimpleLayersParent,
 										buildpacks.FolderSimpleLayers,
 									),
-									buildpacks.WithOS(dockerHostOS()),
 								)
 
 								buildpackManager.PrepareBuildpacks(tmpDir, packageFile)
@@ -2389,13 +2393,14 @@ func createBuilder(
 		buildpacks.ReadEnv,
 	}
 
+	packageTomlPath := generatePackageTomlWithOS(t, assert, pack, tmpDir, "package.toml", dockerHostOS())
 	packageImageName := registryConfig.RepoName("simple-layers-package-image-buildpack-" + h.RandString(8))
 
 	packageImageBuildpack := buildpacks.NewPackageImage(
 		t,
 		pack,
 		packageImageName,
-		pack.FixtureManager().FixtureLocation("package.toml"),
+		packageTomlPath,
 		buildpacks.WithRequiredBuildpacks(buildpacks.SimpleLayers),
 	)
 
@@ -2449,6 +2454,32 @@ func createBuilder(
 	return bldr, nil
 }
 
+func generatePackageTomlWithOS(
+	t *testing.T,
+	assert h.AssertionManager,
+	pack *invoke.PackInvoker,
+	tmpDir string,
+	fixtureName string,
+	platform_os string,
+) string {
+	t.Helper()
+
+	packageTomlFile, err := ioutil.TempFile(tmpDir, "package-*.toml")
+	assert.Nil(err)
+
+	pack.FixtureManager().TemplateFixtureToFile(
+		fixtureName,
+		packageTomlFile,
+		map[string]interface{}{
+			"OS": platform_os,
+		},
+	)
+
+	assert.Nil(packageTomlFile.Close())
+
+	return packageTomlFile.Name()
+}
+
 func createStack(t *testing.T, dockerCli client.CommonAPIClient, runImageMirror string) error {
 	t.Helper()
 	t.Log("creating stack images...")

acceptance/buildpacks/manager.go

@@ -53,7 +53,6 @@ func (b BuildpackManager) PrepareBuildpacks(destination string, buildpacks ...Te
 }
 
 type Modifiable interface {
-	SetOS(string)
 	SetPublish()
 	SetBuildpacks([]TestBuildpack)
 }
@@ -70,9 +69,3 @@ func WithPublish() PackageModifier {
 		p.SetPublish()
 	}
 }
-
-func WithOS(osVal string) PackageModifier {
-	return func(p Modifiable) {
-		p.SetOS(osVal)
-	}
-}

acceptance/buildpacks/package_file_buildpack.go

@@ -21,11 +21,6 @@ type PackageFile struct {
 	destination          string
 	sourceConfigLocation string
 	buildpacks           []TestBuildpack
-	os                   string
-}
-
-func (p *PackageFile) SetOS(os string) {
-	p.os = os
 }
 
 func (p *PackageFile) SetBuildpacks(buildpacks []TestBuildpack) {
@@ -81,10 +76,6 @@ func (p PackageFile) Prepare(sourceDir, _ string) error {
 		"--format", "file",
 	}
 
-	if p.os != "" {
-		packArgs = append(packArgs, "--os", p.os)
-	}
-
 	output := p.pack.RunSuccessfully("package-buildpack", packArgs...)
 
 	if !strings.Contains(output, fmt.Sprintf("Successfully created package '%s'", p.destination)) {

acceptance/buildpacks/package_image_buildpack.go

@@ -23,11 +23,6 @@ type PackageImage struct {
 	sourceConfigLocation string
 	buildpacks           []TestBuildpack
 	publish              bool
-	os                   string
-}
-
-func (p *PackageImage) SetOS(os string) {
-	p.os = os
 }
 
 func (p *PackageImage) SetBuildpacks(buildpacks []TestBuildpack) {
@@ -88,10 +83,6 @@ func (p PackageImage) Prepare(sourceDir, _ string) error {
 		packArgs = append(packArgs, "--publish")
 	}
 
-	if p.os != "" {
-		packArgs = append(packArgs, "--os", p.os)
-	}
-
 	output := p.pack.RunSuccessfully("package-buildpack", packArgs...)
 
 	assertOutput := assertions.NewOutputAssertionManager(p.testObject, output)

acceptance/testdata/pack_fixtures/package.toml

@@ -1,2 +1,5 @@
 [buildpack]
-uri = "simple-layers-buildpack.tgz"
+uri = "simple-layers-buildpack.tgz"
+
+[platform]
+os = "{{ .OS }}"

acceptance/testdata/pack_fixtures/package_aggregate.toml

@@ -2,4 +2,7 @@
 uri = "{{ .BuildpackURI }}"
 
 [[dependencies]]
-image = "{{ .PackageName }}"
+image = "{{ .PackageName }}"
+
+[platform]
+os = "{{ .OS }}"

acceptance/testdata/pack_fixtures/package_for_build_cmd.toml

@@ -2,4 +2,7 @@
 uri = "simple-layers-parent-buildpack"
 
 [[dependencies]]
-uri = "simple-layers-buildpack"
+uri = "simple-layers-buildpack"
+
+[platform]
+os = "{{ .OS }}"

buildpackage/config_reader.go

@@ -16,6 +16,7 @@ import (
 type Config struct {
 	Buildpack    dist.BuildpackURI `toml:"buildpack"`
 	Dependencies []dist.ImageOrURI `toml:"dependencies"`
+	Platform     dist.Platform     `toml:"platform"`
 }
 
 // NewConfigReader returns an instance of ConfigReader. It does not take any parameters.
@@ -51,6 +52,15 @@ func (r *ConfigReader) Read(path string) (Config, error) {
 		return packageConfig, errors.Errorf("missing %s configuration", style.Symbol("buildpack.uri"))
 	}
 
+	if packageConfig.Platform.OS == "" {
+		packageConfig.Platform.OS = "linux"
+	}
+
+	if packageConfig.Platform.OS != "linux" && packageConfig.Platform.OS != "windows" {
+		return packageConfig, errors.Errorf("invalid %s configuration: only [%s, %s] is permitted",
+			style.Symbol("platform.os"), style.Symbol("linux"), style.Symbol("windows"))
+	}
+
 	configDir, err := filepath.Abs(filepath.Dir(path))
 	if err != nil {
 		return packageConfig, err