Read OS platform from config-file during "pack package-buildpack"

- unit tests

Signed-off-by: Anthony Emengo <[email protected]>

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,10 @@ 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"
+	}
+
 	configDir, err := filepath.Abs(filepath.Dir(path))
 	if err != nil {
 		return packageConfig, err

buildpackage/config_reader_test.go

@@ -46,11 +46,26 @@ func testBuildpackageConfigReader(t *testing.T, when spec.G, it spec.S) {
 			config, err := packageConfigReader.Read(configFile)
 			h.AssertNil(t, err)
 
+			h.AssertEq(t, config.Platform.OS, "some-os")
 			h.AssertEq(t, config.Buildpack.URI, "https://example.com/bp/a.tgz")
 			h.AssertEq(t, len(config.Dependencies), 1)
 			h.AssertEq(t, config.Dependencies[0].URI, "https://example.com/bp/b.tgz")
 		})
 
+		it("returns a config with 'linux' as default when platform is missing", func() {
+			configFile := filepath.Join(tmpDir, "package.toml")
+
+			err := ioutil.WriteFile(configFile, []byte(validPackageWithoutPlatformToml), os.ModePerm)
+			h.AssertNil(t, err)
+
+			packageConfigReader := buildpackage.NewConfigReader()
+
+			config, err := packageConfigReader.Read(configFile)
+			h.AssertNil(t, err)
+
+			h.AssertEq(t, config.Platform.OS, "linux")
+		})
+
 		it("returns an error when toml decode fails", func() {
 			configFile := filepath.Join(tmpDir, "package.toml")
 
@@ -192,6 +207,17 @@ const validPackageToml = `
 [buildpack]
 uri = "https://example.com/bp/a.tgz"
 
+[[dependencies]]
+uri = "https://example.com/bp/b.tgz"
+
+[platform]
+os = "some-os"
+`
+
+const validPackageWithoutPlatformToml = `
+[buildpack]
+uri = "https://example.com/bp/a.tgz"
+
 [[dependencies]]
 uri = "https://example.com/bp/b.tgz"
 `

create_builder_test.go

@@ -762,6 +762,7 @@ func testCreateBuilder(t *testing.T, when spec.G, it spec.S) {
 				h.AssertNil(t, subject.PackageBuildpack(context.TODO(), pack.PackageBuildpackOptions{
 					Name: cnbFile,
 					Config: pubbldpkg.Config{
+						Platform:  dist.Platform{OS: "linux"},
 						Buildpack: dist.BuildpackURI{URI: buildpackPath},
 					},
 					Format: "file",
@@ -842,6 +843,7 @@ func testCreateBuilder(t *testing.T, when spec.G, it spec.S) {
 					h.AssertNil(t, subject.PackageBuildpack(context.TODO(), pack.PackageBuildpackOptions{
 						Name: packageImage.Name(),
 						Config: pubbldpkg.Config{
+							Platform:  dist.Platform{OS: "linux"},
 							Buildpack: dist.BuildpackURI{URI: createBuildpack(bpd)},
 						},
 						Publish: true,
@@ -915,6 +917,7 @@ func testCreateBuilder(t *testing.T, when spec.G, it spec.S) {
 					h.AssertNil(t, subject.PackageBuildpack(context.TODO(), pack.PackageBuildpackOptions{
 						Name: packageImage.Name(),
 						Config: pubbldpkg.Config{
+							Platform:  dist.Platform{OS: "linux"},
 							Buildpack: dist.BuildpackURI{URI: createBuildpack(bpd)},
 						},
 						Publish:    true,

internal/commands/package_buildpack.go

@@ -19,7 +19,6 @@ import (
 type PackageBuildpackFlags struct {
 	PackageTomlPath string
 	Format          string
-	OS              string
 	Publish         bool
 	Policy          string
 }
@@ -72,7 +71,6 @@ func PackageBuildpack(logger logging.Logger, cfg config.Config, client Buildpack
 			if err := client.PackageBuildpack(cmd.Context(), pack.PackageBuildpackOptions{
 				Name:       name,
 				Format:     flags.Format,
-				OS:         flags.OS,
 				Config:     cfg,
 				Publish:    flags.Publish,
 				PullPolicy: pullPolicy,
@@ -93,10 +91,6 @@ func PackageBuildpack(logger logging.Logger, cfg config.Config, client Buildpack
 
 	cmd.Flags().StringVarP(&flags.Format, "format", "f", "", `Format to save package as ("image" or "file")`)
 	cmd.Flags().BoolVar(&flags.Publish, "publish", false, `Publish to registry (applies to "--format=image" only)`)
-	cmd.Flags().StringVar(&flags.OS, "os", "", `Operating system format of the package OCI image: "linux" or "windows" (defaults to "linux", except local images which use the daemon OS)`)
-	if !cfg.Experimental {
-		cmd.Flags().MarkHidden("os")
-	}
 	cmd.Flags().StringVar(&flags.Policy, "pull-policy", "", "Pull policy to use. Accepted values are always, never, and if-not-present. The default is always")
 
 	AddHelpFlag(cmd, "package-buildpack")
@@ -112,9 +106,5 @@ func validatePackageBuildpackFlags(p *PackageBuildpackFlags, cfg config.Config)
 		return errors.Errorf("Please provide a package config path, using --config")
 	}
 
-	if p.OS != "" && !cfg.Experimental {
-		return pack.NewExperimentError("Support for OS flag is currently experimental.")
-	}
-
 	return nil
 }

internal/commands/package_buildpack_test.go

@@ -122,34 +122,6 @@ func testPackageBuildpackCommand(t *testing.T, when spec.G, it spec.S) {
 					h.AssertEq(t, receivedOptions.PullPolicy, pubcfg.PullAlways)
 				})
 			})
-
-			when("--os", func() {
-				when("experimental enabled", func() {
-					it("creates package with correct image name and os", func() {
-						fakeBuildpackPackager := &fakes.FakeBuildpackPackager{}
-
-						packageBuildpackCommand := packageBuildpackCommand(
-							withBuildpackPackager(fakeBuildpackPackager),
-							withExperimental(),
-						)
-
-						packageBuildpackCommand.SetArgs(
-							[]string{
-								"some-image-name",
-								"--config", "/path/to/some/file",
-								"--os", "windows",
-							},
-						)
-
-						err := packageBuildpackCommand.Execute()
-						h.AssertNil(t, err)
-
-						receivedOptions := fakeBuildpackPackager.CreateCalledWithOptions
-
-						h.AssertEq(t, receivedOptions.OS, "windows")
-					})
-				})
-			})
 		})
 	})
 
@@ -250,28 +222,6 @@ func testPackageBuildpackCommand(t *testing.T, when spec.G, it spec.S) {
 				h.AssertError(t, command.Execute(), "parsing pull policy")
 			})
 		})
-
-		when("--os flag is specified but experimental isn't set in the config", func() {
-			it("errors with a descriptive message", func() {
-				fakeBuildpackPackager := &fakes.FakeBuildpackPackager{}
-
-				packageBuildpackCommand := packageBuildpackCommand(
-					withBuildpackPackager(fakeBuildpackPackager),
-				)
-
-				packageBuildpackCommand.SetArgs(
-					[]string{
-						"some-image-name",
-						"--config", "/path/to/some/file",
-						"--os", "windows",
-					},
-				)
-
-				err := packageBuildpackCommand.Execute()
-				h.AssertNotNil(t, err)
-				h.AssertError(t, err, "Support for OS flag is currently experimental")
-			})
-		})
 	})
 }
 
@@ -338,12 +288,6 @@ func withPackageConfigPath(path string) packageCommandOption {
 	}
 }
 
-func withExperimental() packageCommandOption {
-	return func(config *packageCommandConfig) {
-		config.clientConfig.Experimental = true
-	}
-}
-
 func whereReadReturns(config pubbldpkg.Config, err error) func(*fakes.FakePackageConfigReader) {
 	return func(r *fakes.FakePackageConfigReader) {
 		r.ReadReturnConfig = config

internal/dist/dist.go

@@ -17,6 +17,10 @@ type ImageOrURI struct {
 	ImageRef
 }
 
+type Platform struct {
+	OS string `toml:"os"`
+}
+
 type Order []OrderEntry
 
 type OrderEntry struct {

package_buildpack.go

@@ -31,9 +31,6 @@ type PackageBuildpackOptions struct {
 	// Type of output format, The options are the either the const FormatImage, or FormatFile.
 	Format string
 
-	// Type of OCI image to generate in the image or file. The options are "windows" or "linux"
-	OS string
-
 	// Defines the Buildpacks configuration.
 	Config pubbldpkg.Config
 
@@ -51,20 +48,11 @@ func (c *Client) PackageBuildpack(ctx context.Context, opts PackageBuildpackOpti
 		opts.Format = FormatImage
 	}
 
-	if opts.OS == "" {
-		osType, err := c.defaultOSType(ctx, opts.Publish, opts.Format)
-		if err != nil {
-			return errors.Wrap(err, "daemon OS cannot be detected")
-		}
-
-		opts.OS = osType
-	}
-
-	if opts.OS == "windows" && !c.experimental {
+	if opts.Config.Platform.OS == "windows" && !c.experimental {
 		return NewExperimentError("Windows buildpackage support is currently experimental.")
 	}
 
-	writerFactory, err := layer.NewWriterFactory(opts.OS)
+	writerFactory, err := layer.NewWriterFactory(opts.Config.Platform.OS)
 	if err != nil {
 		return errors.Wrap(err, "creating layer writer factory")
 	}
@@ -144,26 +132,13 @@ func (c *Client) PackageBuildpack(ctx context.Context, opts PackageBuildpackOpti
 
 	switch opts.Format {
 	case FormatFile:
-		return packageBuilder.SaveAsFile(opts.Name, opts.OS)
+		return packageBuilder.SaveAsFile(opts.Name, opts.Config.Platform.OS)
 	case FormatImage:
-		_, err = packageBuilder.SaveAsImage(opts.Name, opts.Publish, opts.OS)
+		//TODO: should check if saving linux image on windows daemon ???
+		//
+		_, err = packageBuilder.SaveAsImage(opts.Name, opts.Publish, opts.Config.Platform.OS)
 		return errors.Wrapf(err, "saving image")
 	default:
 		return errors.Errorf("unknown format: %s", style.Symbol(opts.Format))
 	}
 }
-
-func (c *Client) defaultOSType(ctx context.Context, publish bool, format string) (string, error) {
-	if publish || format == FormatFile {
-		c.logger.Warnf(`buildpackage OS unspecified - defaulting to "linux"`)
-
-		return "linux", nil
-	}
-
-	info, err := c.docker.Info(ctx)
-	if err != nil {
-		return "", err
-	}
-
-	return info.OSType, nil
-}