diff --git a/buildpackage/config_reader.go b/buildpackage/config_reader.go index 4d766aa679..d3e07b8380 100644 --- a/buildpackage/config_reader.go +++ b/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 diff --git a/buildpackage/config_reader_test.go b/buildpackage/config_reader_test.go index 0bab3b5837..1f4ce76a5c 100644 --- a/buildpackage/config_reader_test.go +++ b/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" ` diff --git a/create_builder_test.go b/create_builder_test.go index 57cb27d13d..77cbb14092 100644 --- a/create_builder_test.go +++ b/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, diff --git a/internal/commands/package_buildpack.go b/internal/commands/package_buildpack.go index 785e2643c3..7bc84ebc38 100644 --- a/internal/commands/package_buildpack.go +++ b/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 } diff --git a/internal/commands/package_buildpack_test.go b/internal/commands/package_buildpack_test.go index 206ff18dc4..82828ec519 100644 --- a/internal/commands/package_buildpack_test.go +++ b/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 diff --git a/internal/dist/dist.go b/internal/dist/dist.go index b7003bfd4e..e05e5accbf 100644 --- a/internal/dist/dist.go +++ b/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 { diff --git a/package_buildpack.go b/package_buildpack.go index 6d4e1bd399..0b77eb6edf 100644 --- a/package_buildpack.go +++ b/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 -} diff --git a/package_buildpack_test.go b/package_buildpack_test.go index ef9a48c441..f86f14dd3e 100644 --- a/package_buildpack_test.go +++ b/package_buildpack_test.go @@ -84,6 +84,7 @@ func testPackageBuildpack(t *testing.T, when spec.G, it spec.S) { err := subject.PackageBuildpack(context.TODO(), pack.PackageBuildpackOptions{ Name: "Fake-Name", Config: pubbldpkg.Config{ + Platform: dist.Platform{OS: "linux"}, Buildpack: dist.BuildpackURI{URI: ""}, }, Publish: true, @@ -100,6 +101,7 @@ func testPackageBuildpack(t *testing.T, when spec.G, it spec.S) { err := subject.PackageBuildpack(context.TODO(), pack.PackageBuildpackOptions{ Name: "Fake-Name", Config: pubbldpkg.Config{ + Platform: dist.Platform{OS: "linux"}, Buildpack: dist.BuildpackURI{URI: bpURL}, }, Publish: true, @@ -117,6 +119,7 @@ func testPackageBuildpack(t *testing.T, when spec.G, it spec.S) { err := subject.PackageBuildpack(context.TODO(), pack.PackageBuildpackOptions{ Name: "Fake-Name", Config: pubbldpkg.Config{ + Platform: dist.Platform{OS: "linux"}, Buildpack: dist.BuildpackURI{URI: bpURL}, }, Publish: true, @@ -148,6 +151,7 @@ func testPackageBuildpack(t *testing.T, when spec.G, it spec.S) { err := subject.PackageBuildpack(context.TODO(), pack.PackageBuildpackOptions{ Name: "test", Config: pubbldpkg.Config{ + Platform: dist.Platform{OS: "linux"}, Buildpack: dist.BuildpackURI{URI: createBuildpack(packageDescriptor)}, Dependencies: []dist.ImageOrURI{{BuildpackURI: dist.BuildpackURI{URI: dependencyPath}}}, }, @@ -186,6 +190,7 @@ func testPackageBuildpack(t *testing.T, when spec.G, it spec.S) { Format: pack.FormatImage, Name: fakeImage.Name(), Config: pubbldpkg.Config{ + Platform: dist.Platform{OS: daemonOS}, Buildpack: dist.BuildpackURI{URI: createBuildpack(dist.BuildpackDescriptor{ API: api.MustParse("0.2"), Info: dist.BuildpackInfo{ID: "bp.basic", Version: "2.3.4"}, @@ -203,7 +208,6 @@ 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().Info(context.TODO()).Return(types.Info{OSType: "windows"}, nil).AnyTimes() packClientWithoutExperimental, err := pack.NewClient( pack.WithDockerClient(windowsMockDockerClient), @@ -211,7 +215,13 @@ func testPackageBuildpack(t *testing.T, when spec.G, it spec.S) { ) h.AssertNil(t, err) - err = packClientWithoutExperimental.PackageBuildpack(context.TODO(), pack.PackageBuildpackOptions{}) + err = packClientWithoutExperimental.PackageBuildpack(context.TODO(), pack.PackageBuildpackOptions{ + Config: pubbldpkg.Config{ + Platform: dist.Platform{ + OS: "windows", + }, + }, + }) h.AssertError(t, err, "Windows buildpackage support is currently experimental.") }) }) @@ -228,6 +238,7 @@ func testPackageBuildpack(t *testing.T, when spec.G, it spec.S) { h.AssertNil(t, subject.PackageBuildpack(context.TODO(), pack.PackageBuildpackOptions{ Name: nestedPackage.Name(), Config: pubbldpkg.Config{ + Platform: dist.Platform{OS: "linux"}, Buildpack: dist.BuildpackURI{URI: createBuildpack(dist.BuildpackDescriptor{ API: api.MustParse("0.2"), Info: dist.BuildpackInfo{ID: "bp.nested", Version: "2.3.4"}, @@ -267,6 +278,7 @@ func testPackageBuildpack(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(dist.BuildpackDescriptor{ API: api.MustParse("0.2"), Info: dist.BuildpackInfo{ID: "bp.1", Version: "1.2.3"}, @@ -293,6 +305,7 @@ func testPackageBuildpack(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(dist.BuildpackDescriptor{ API: api.MustParse("0.2"), Info: dist.BuildpackInfo{ID: "bp.1", Version: "1.2.3"}, @@ -319,6 +332,7 @@ func testPackageBuildpack(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(dist.BuildpackDescriptor{ API: api.MustParse("0.2"), Info: dist.BuildpackInfo{ID: "bp.1", Version: "1.2.3"}, @@ -344,6 +358,7 @@ func testPackageBuildpack(t *testing.T, when spec.G, it spec.S) { h.AssertError(t, subject.PackageBuildpack(context.TODO(), pack.PackageBuildpackOptions{ Name: "some/package", Config: pubbldpkg.Config{ + Platform: dist.Platform{OS: "linux"}, Buildpack: dist.BuildpackURI{URI: createBuildpack(dist.BuildpackDescriptor{ API: api.MustParse("0.2"), Info: dist.BuildpackInfo{ID: "bp.1", Version: "1.2.3"}, @@ -368,6 +383,7 @@ func testPackageBuildpack(t *testing.T, when spec.G, it spec.S) { h.AssertError(t, subject.PackageBuildpack(context.TODO(), pack.PackageBuildpackOptions{ Name: "some/package", Config: pubbldpkg.Config{ + Platform: dist.Platform{OS: "linux"}, Buildpack: dist.BuildpackURI{URI: createBuildpack(dist.BuildpackDescriptor{ API: api.MustParse("0.2"), Info: dist.BuildpackInfo{ID: "bp.1", Version: "1.2.3"}, @@ -409,8 +425,8 @@ func testPackageBuildpack(t *testing.T, when spec.G, it spec.S) { h.AssertNil(t, packClientWithExperimental.PackageBuildpack(context.TODO(), pack.PackageBuildpackOptions{ Format: pack.FormatFile, Name: packagePath, - OS: imageOS, Config: pubbldpkg.Config{ + Platform: dist.Platform{OS: imageOS}, Buildpack: dist.BuildpackURI{URI: createBuildpack(dist.BuildpackDescriptor{ API: api.MustParse("0.2"), Info: dist.BuildpackInfo{ID: "bp.basic", Version: "2.3.4"}, @@ -466,6 +482,7 @@ func testPackageBuildpack(t *testing.T, when spec.G, it spec.S) { h.AssertNil(t, subject.PackageBuildpack(context.TODO(), pack.PackageBuildpackOptions{ Name: nestedPackage.Name(), Config: pubbldpkg.Config{ + Platform: dist.Platform{OS: "linux"}, Buildpack: dist.BuildpackURI{URI: createBuildpack(childDescriptor)}, }, Publish: true, @@ -481,6 +498,7 @@ func testPackageBuildpack(t *testing.T, when spec.G, it spec.S) { h.AssertNil(t, subject.PackageBuildpack(context.TODO(), pack.PackageBuildpackOptions{ Name: packagePath, Config: pubbldpkg.Config{ + Platform: dist.Platform{OS: "linux"}, Buildpack: dist.BuildpackURI{URI: createBuildpack(packageDescriptor)}, Dependencies: []dist.ImageOrURI{{ImageRef: dist.ImageRef{ImageName: nestedPackage.Name()}}}, }, @@ -500,6 +518,7 @@ func testPackageBuildpack(t *testing.T, when spec.G, it spec.S) { h.AssertNil(t, subject.PackageBuildpack(context.TODO(), pack.PackageBuildpackOptions{ Name: packagePath, Config: pubbldpkg.Config{ + Platform: dist.Platform{OS: "linux"}, Buildpack: dist.BuildpackURI{URI: createBuildpack(packageDescriptor)}, Dependencies: []dist.ImageOrURI{{BuildpackURI: dist.BuildpackURI{URI: createBuildpack(childDescriptor)}}}, }, @@ -521,6 +540,7 @@ func testPackageBuildpack(t *testing.T, when spec.G, it spec.S) { err = subject.PackageBuildpack(context.TODO(), pack.PackageBuildpackOptions{ Name: packagePath, Config: pubbldpkg.Config{ + Platform: dist.Platform{OS: "linux"}, Buildpack: dist.BuildpackURI{URI: createBuildpack(packageDescriptor)}, Dependencies: []dist.ImageOrURI{{BuildpackURI: dist.BuildpackURI{URI: bpURL}}}, }, @@ -543,6 +563,7 @@ func testPackageBuildpack(t *testing.T, when spec.G, it spec.S) { err = subject.PackageBuildpack(context.TODO(), pack.PackageBuildpackOptions{ Name: packagePath, Config: pubbldpkg.Config{ + Platform: dist.Platform{OS: "linux"}, Buildpack: dist.BuildpackURI{URI: createBuildpack(packageDescriptor)}, Dependencies: []dist.ImageOrURI{{BuildpackURI: dist.BuildpackURI{URI: bpURL}}}, }, @@ -576,6 +597,7 @@ func testPackageBuildpack(t *testing.T, when spec.G, it spec.S) { h.AssertNil(t, subject.PackageBuildpack(context.TODO(), pack.PackageBuildpackOptions{ Name: nestedPackage.Name(), Config: pubbldpkg.Config{ + Platform: dist.Platform{OS: "linux"}, Buildpack: dist.BuildpackURI{URI: createBuildpack(childDescriptor)}, }, Publish: true, @@ -591,6 +613,7 @@ func testPackageBuildpack(t *testing.T, when spec.G, it spec.S) { h.AssertNil(t, subject.PackageBuildpack(context.TODO(), pack.PackageBuildpackOptions{ Name: packagePath, Config: pubbldpkg.Config{ + Platform: dist.Platform{OS: "linux"}, Buildpack: dist.BuildpackURI{URI: createBuildpack(packageDescriptor)}, Dependencies: []dist.ImageOrURI{{ImageRef: dist.ImageRef{ImageName: nestedPackage.Name()}}, {BuildpackURI: dist.BuildpackURI{URI: createBuildpack(secondChildDescriptor)}}}, @@ -614,6 +637,7 @@ func testPackageBuildpack(t *testing.T, when spec.G, it spec.S) { h.AssertNil(t, subject.PackageBuildpack(context.TODO(), pack.PackageBuildpackOptions{ Name: dependencyPackagePath, Config: pubbldpkg.Config{ + Platform: dist.Platform{OS: "linux"}, Buildpack: dist.BuildpackURI{URI: createBuildpack(childDescriptor)}, }, PullPolicy: pubcfg.PullAlways, @@ -629,6 +653,7 @@ func testPackageBuildpack(t *testing.T, when spec.G, it spec.S) { h.AssertNil(t, subject.PackageBuildpack(context.TODO(), pack.PackageBuildpackOptions{ Name: packagePath, Config: pubbldpkg.Config{ + Platform: dist.Platform{OS: "linux"}, Buildpack: dist.BuildpackURI{URI: createBuildpack(packageDescriptor)}, Dependencies: []dist.ImageOrURI{{BuildpackURI: dist.BuildpackURI{URI: dependencyPackagePath}}}, }, @@ -651,6 +676,7 @@ func testPackageBuildpack(t *testing.T, when spec.G, it spec.S) { Name: "some-buildpack", Format: "invalid-format", Config: pubbldpkg.Config{ + Platform: dist.Platform{OS: "linux"}, Buildpack: dist.BuildpackURI{URI: createBuildpack(dist.BuildpackDescriptor{ API: api.MustParse("0.2"), Info: dist.BuildpackInfo{ID: "bp.1", Version: "1.2.3"},