diff --git a/acceptance/acceptance_test.go b/acceptance/acceptance_test.go index 2268ab780..5708d5d4a 100644 --- a/acceptance/acceptance_test.go +++ b/acceptance/acceptance_test.go @@ -569,7 +569,7 @@ func testAcceptance(t *testing.T, when spec.G, it spec.S, packFixturesDir, packP var packageImageName string it.Before(func() { - packageImageName = createPackage(t, + packageImageName = packageBuildpack(t, filepath.Join(configDir, "package_for_build_cmd.toml"), packPath, lifecycleDescriptor, @@ -1063,7 +1063,7 @@ func testAcceptance(t *testing.T, when spec.G, it spec.S, packFixturesDir, packP h.AssertNil(t, os.RemoveAll(tmpDir)) }) - createPackageLocally := func(absConfigPath string) string { + packageBuildpackLocally := func(absConfigPath string) string { t.Helper() packageName := "test/package-" + h.RandString(10) output, err := h.RunE(subjectPack("package-buildpack", packageName, "-p", absConfigPath)) @@ -1072,7 +1072,7 @@ func testAcceptance(t *testing.T, when spec.G, it spec.S, packFixturesDir, packP return packageName } - createPackageRemotely := func(absConfigPath string) string { + packageBuildpackRemotely := func(absConfigPath string) string { t.Helper() packageName := registryConfig.RepoName("test/package-" + h.RandString(10)) output, err := h.RunE(subjectPack("package-buildpack", packageName, "-p", absConfigPath, "--publish")) @@ -1108,20 +1108,20 @@ func testAcceptance(t *testing.T, when spec.G, it spec.S, packFixturesDir, packP it("creates the package", func() { t.Log("package w/ only buildpacks") - nestedPackageName := createPackageLocally(filepath.Join(tmpDir, "package.toml")) + nestedPackageName := packageBuildpackLocally(filepath.Join(tmpDir, "package.toml")) defer h.DockerRmi(dockerCli, nestedPackageName) assertImageExistsLocally(nestedPackageName) t.Log("package w/ buildpacks and packages") aggregatePackageToml := generateAggregatePackageToml("simple-layers-parent-buildpack.tgz", nestedPackageName) - packageName := createPackageLocally(aggregatePackageToml) + packageName := packageBuildpackLocally(aggregatePackageToml) defer h.DockerRmi(dockerCli, packageName) assertImageExistsLocally(packageName) }) when("--publish", func() { it("publishes image to registry", func() { - nestedPackageName := createPackageRemotely(filepath.Join(tmpDir, "package.toml")) + nestedPackageName := packageBuildpackRemotely(filepath.Join(tmpDir, "package.toml")) defer h.DockerRmi(dockerCli, nestedPackageName) aggregatePackageToml := generateAggregatePackageToml("simple-layers-parent-buildpack.tgz", nestedPackageName) @@ -1142,7 +1142,7 @@ func testAcceptance(t *testing.T, when spec.G, it spec.S, packFixturesDir, packP when("--no-pull", func() { it("should use local image", func() { - nestedPackage := createPackageLocally(filepath.Join(tmpDir, "package.toml")) + nestedPackage := packageBuildpackLocally(filepath.Join(tmpDir, "package.toml")) defer h.DockerRmi(dockerCli, nestedPackage) aggregatePackageToml := generateAggregatePackageToml("simple-layers-parent-buildpack.tgz", nestedPackage) @@ -1156,7 +1156,7 @@ func testAcceptance(t *testing.T, when spec.G, it spec.S, packFixturesDir, packP }) it("should not pull image from registry", func() { - nestedPackage := createPackageRemotely(filepath.Join(tmpDir, "package.toml")) + nestedPackage := packageBuildpackRemotely(filepath.Join(tmpDir, "package.toml")) defer h.DockerRmi(dockerCli, nestedPackage) aggregatePackageToml := generateAggregatePackageToml("simple-layers-parent-buildpack.tgz", nestedPackage) @@ -1166,6 +1166,15 @@ func testAcceptance(t *testing.T, when spec.G, it spec.S, packFixturesDir, packP h.AssertError(t, err, fmt.Sprintf("image '%s' does not exist on the daemon", nestedPackage)) }) }) + + when("package.toml is invalid", func() { + it("displays an error", func() { + h.CopyFile(t, filepath.Join(packFixturesDir, "invalid_package.toml"), filepath.Join(tmpDir, "invalid_package.toml")) + + _, err := h.RunE(subjectPack("package-buildpack", "some-package", "-p", filepath.Join(tmpDir, "invalid_package.toml"))) + h.AssertError(t, err, "reading config:") + }) + }) }) when("report", func() { @@ -1431,7 +1440,7 @@ func createBuilder(t *testing.T, runImageMirror, configDir, packPath, lifecycleP } // CREATE PACKAGE - packageImageName := createPackage(t, + packageImageName := packageBuildpack(t, filepath.Join(configDir, "package.toml"), packPath, lifecycleDescriptor, @@ -1481,7 +1490,7 @@ func createBuilder(t *testing.T, runImageMirror, configDir, packPath, lifecycleP return bldr } -func createPackage(t *testing.T, configPath, packPath string, lifecycleDescriptor builder.LifecycleDescriptor, repoName string, buildpacks []string) string { +func packageBuildpack(t *testing.T, configPath, packPath string, lifecycleDescriptor builder.LifecycleDescriptor, repoName string, buildpacks []string) string { t.Helper() t.Log("creating package image...") diff --git a/acceptance/testdata/pack_fixtures/invalid_package.toml b/acceptance/testdata/pack_fixtures/invalid_package.toml new file mode 100644 index 000000000..a015b08c2 --- /dev/null +++ b/acceptance/testdata/pack_fixtures/invalid_package.toml @@ -0,0 +1 @@ +[buildpack] diff --git a/buildpackage/config_reader.go b/buildpackage/config_reader.go new file mode 100644 index 000000000..11ab63f4e --- /dev/null +++ b/buildpackage/config_reader.go @@ -0,0 +1,107 @@ +package buildpackage + +import ( + "path/filepath" + "strings" + + "github.com/BurntSushi/toml" + "github.com/pkg/errors" + + "github.com/buildpacks/pack/internal/dist" + "github.com/buildpacks/pack/internal/paths" + "github.com/buildpacks/pack/internal/style" +) + +// Config encapsulates the possible configuration options for buildpackage creation. +type Config struct { + Buildpack dist.BuildpackURI `toml:"buildpack"` + Dependencies []dist.ImageOrURI `toml:"dependencies"` +} + +// NewConfigReader returns an instance of ConfigReader. It does not take any parameters. +func NewConfigReader() *ConfigReader { + return &ConfigReader{} +} + +// ConfigReader implements a Read method for buildpackage configuration which parses and validates buildpackage +// configuration from a toml file. +type ConfigReader struct{} + +// Read reads and validates a buildpackage configuration from the file path provided and returns the +// configuration and any error that occurred during reading or validation. +func (r *ConfigReader) Read(path string) (Config, error) { + config := Config{} + + tomlMetadata, err := toml.DecodeFile(path, &config) + if err != nil { + return config, errors.Wrap(err, "decoding toml") + } + + undecodedKeys := tomlMetadata.Undecoded() + if len(undecodedKeys) > 0 { + unusedKeys := map[string]interface{}{} + for _, key := range undecodedKeys { + keyName := key.String() + + parent := strings.Split(keyName, ".")[0] + + if _, ok := unusedKeys[parent]; !ok { + unusedKeys[keyName] = nil + } + } + + var errorKeys []string + for errorKey := range unusedKeys { + errorKeys = append(errorKeys, style.Symbol(errorKey)) + } + + pluralizedElement := "element" + if len(errorKeys) > 1 { + pluralizedElement += "s" + } + + return config, errors.Errorf("unknown configuration %s %s in %s", + pluralizedElement, + strings.Join(errorKeys, ", "), + style.Symbol(path), + ) + } + + if config.Buildpack.URI == "" { + return config, errors.Errorf("missing %s configuration", style.Symbol("buildpack.uri")) + } + + configDir, err := filepath.Abs(filepath.Dir(path)) + if err != nil { + return config, err + } + + absPath, err := paths.ToAbsolute(config.Buildpack.URI, configDir) + if err != nil { + return config, errors.Wrapf(err, "getting absolute path for %s", style.Symbol(config.Buildpack.URI)) + } + config.Buildpack.URI = absPath + + for i := range config.Dependencies { + uri := config.Dependencies[i].URI + if uri != "" { + absPath, err := paths.ToAbsolute(uri, configDir) + if err != nil { + return config, errors.Wrapf(err, "getting absolute path for %s", style.Symbol(uri)) + } + + config.Dependencies[i].URI = absPath + } + + dep := config.Dependencies[i] + if dep.URI != "" && dep.ImageName != "" { + return config, errors.Errorf( + "dependency configured with both %s and %s", + style.Symbol("uri"), + style.Symbol("image"), + ) + } + } + + return config, nil +} diff --git a/buildpackage/config_reader_test.go b/buildpackage/config_reader_test.go new file mode 100644 index 000000000..b622b0162 --- /dev/null +++ b/buildpackage/config_reader_test.go @@ -0,0 +1,262 @@ +package buildpackage_test + +import ( + "io/ioutil" + "os" + "path/filepath" + "testing" + + "github.com/buildpacks/pack/buildpackage" + "github.com/buildpacks/pack/internal/paths" + + "github.com/heroku/color" + "github.com/sclevine/spec" + "github.com/sclevine/spec/report" + + h "github.com/buildpacks/pack/testhelpers" +) + +func TestBuildpackageConfigReader(t *testing.T) { + color.Disable(true) + defer color.Disable(false) + spec.Run(t, "Buildpackage Config Reader", testBuildpackageConfigReader, spec.Parallel(), spec.Report(report.Terminal{})) +} + +func testBuildpackageConfigReader(t *testing.T, when spec.G, it spec.S) { + when("#Read", func() { + var tmpDir string + + it.Before(func() { + var err error + tmpDir, err = ioutil.TempDir("", "buildpackage-config-test") + h.AssertNil(t, err) + }) + + it.After(func() { + os.RemoveAll(tmpDir) + }) + + it("returns correct config when provided toml file is valid", func() { + configFile := filepath.Join(tmpDir, "package.toml") + + err := ioutil.WriteFile(configFile, []byte(validPackageToml), os.ModePerm) + h.AssertNil(t, err) + + packageConfigReader := buildpackage.NewConfigReader() + + config, err := packageConfigReader.Read(configFile) + h.AssertNil(t, err) + + 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 an error when toml decode fails", func() { + configFile := filepath.Join(tmpDir, "package.toml") + + err := ioutil.WriteFile(configFile, []byte(brokenPackageToml), os.ModePerm) + h.AssertNil(t, err) + + packageConfigReader := buildpackage.NewConfigReader() + + _, err = packageConfigReader.Read(configFile) + h.AssertNotNil(t, err) + + h.AssertError(t, err, "decoding toml") + }) + + it("expands relative file uris to absolute paths relative to config file", func() { + configFile := filepath.Join(tmpDir, "package.toml") + + err := ioutil.WriteFile(configFile, []byte(relativePathsPackageToml), os.ModePerm) + h.AssertNil(t, err) + + packageConfigReader := buildpackage.NewConfigReader() + + config, err := packageConfigReader.Read(configFile) + h.AssertNil(t, err) + + expectedURI, err := paths.FilePathToURI(filepath.Join(tmpDir, "bp", "a")) + h.AssertNil(t, err) + h.AssertEq(t, config.Buildpack.URI, expectedURI) + + expectedURI, err = paths.FilePathToURI(filepath.Join(tmpDir, "bp", "b")) + h.AssertNil(t, err) + h.AssertEq(t, config.Dependencies[0].URI, expectedURI) + }) + + it("returns an error when buildpack uri is invalid", func() { + configFile := filepath.Join(tmpDir, "package.toml") + + err := ioutil.WriteFile(configFile, []byte(invalidBPURIPackageToml), os.ModePerm) + h.AssertNil(t, err) + + packageConfigReader := buildpackage.NewConfigReader() + + _, err = packageConfigReader.Read(configFile) + h.AssertNotNil(t, err) + h.AssertError(t, err, "getting absolute path for") + h.AssertError(t, err, "https@@@@@@://example.com/bp/a.tgz") + }) + + it("returns an error when dependency uri is invalid", func() { + configFile := filepath.Join(tmpDir, "package.toml") + + err := ioutil.WriteFile(configFile, []byte(invalidDepURIPackageToml), os.ModePerm) + h.AssertNil(t, err) + + packageConfigReader := buildpackage.NewConfigReader() + + _, err = packageConfigReader.Read(configFile) + h.AssertNotNil(t, err) + h.AssertError(t, err, "getting absolute path for") + h.AssertError(t, err, "https@@@@@@://example.com/bp/b.tgz") + }) + + it("returns an error when unknown array table is present", func() { + configFile := filepath.Join(tmpDir, "package.toml") + + err := ioutil.WriteFile(configFile, []byte(invalidDepTablePackageToml), os.ModePerm) + h.AssertNil(t, err) + + packageConfigReader := buildpackage.NewConfigReader() + + _, err = packageConfigReader.Read(configFile) + h.AssertNotNil(t, err) + h.AssertError(t, err, "unknown configuration element") + h.AssertError(t, err, "dependenceis") + h.AssertNotContains(t, err.Error(), ".image") + h.AssertError(t, err, configFile) + }) + + it("returns an error when unknown buildpack key is present", func() { + configFile := filepath.Join(tmpDir, "package.toml") + + err := ioutil.WriteFile(configFile, []byte(unknownBPKeyPackageToml), os.ModePerm) + h.AssertNil(t, err) + + packageConfigReader := buildpackage.NewConfigReader() + + _, err = packageConfigReader.Read(configFile) + h.AssertNotNil(t, err) + h.AssertError(t, err, "unknown configuration element") + h.AssertError(t, err, "buildpack.url") + h.AssertError(t, err, configFile) + }) + + it("returns an error when multiple unknown keys are present", func() { + configFile := filepath.Join(tmpDir, "package.toml") + + err := ioutil.WriteFile(configFile, []byte(multipleUnknownKeysPackageToml), os.ModePerm) + h.AssertNil(t, err) + + packageConfigReader := buildpackage.NewConfigReader() + + _, err = packageConfigReader.Read(configFile) + h.AssertNotNil(t, err) + h.AssertError(t, err, "unknown configuration elements") + h.AssertError(t, err, "'buildpack.url'") + h.AssertError(t, err, "', '") + h.AssertError(t, err, "'dependenceis'") + }) + + it("returns an error when both dependency options are configured", func() { + configFile := filepath.Join(tmpDir, "package.toml") + + err := ioutil.WriteFile(configFile, []byte(conflictingDependencyKeysPackageToml), os.ModePerm) + h.AssertNil(t, err) + + packageConfigReader := buildpackage.NewConfigReader() + + _, err = packageConfigReader.Read(configFile) + h.AssertNotNil(t, err) + h.AssertError(t, err, "dependency configured with both 'uri' and 'image'") + }) + + it("returns an error no buildpack is configured", func() { + configFile := filepath.Join(tmpDir, "package.toml") + + err := ioutil.WriteFile(configFile, []byte(missingBuildpackPackageToml), os.ModePerm) + h.AssertNil(t, err) + + packageConfigReader := buildpackage.NewConfigReader() + + _, err = packageConfigReader.Read(configFile) + h.AssertNotNil(t, err) + h.AssertError(t, err, "missing 'buildpack.uri' configuration") + }) + }) +} + +const validPackageToml = ` +[buildpack] +uri = "https://example.com/bp/a.tgz" + +[[dependencies]] +uri = "https://example.com/bp/b.tgz" +` + +const brokenPackageToml = ` +[buildpack # missing closing bracket +uri = "https://example.com/bp/a.tgz" + +[dependencies]] # missing opening bracket +uri = "https://example.com/bp/b.tgz" +` + +const relativePathsPackageToml = ` +[buildpack] +uri = "bp/a" + +[[dependencies]] +uri = "bp/b" +` + +const invalidBPURIPackageToml = ` +[buildpack] +uri = "https@@@@@@://example.com/bp/a.tgz" +` + +const invalidDepURIPackageToml = ` +[buildpack] +uri = "noop-buildpack.tgz" + +[[dependencies]] +uri = "https@@@@@@://example.com/bp/b.tgz" +` + +const invalidDepTablePackageToml = ` +[buildpack] +uri = "noop-buildpack.tgz" + +[[dependenceis]] # Notice: this is misspelled +image = "some/package-dep" +` + +const unknownBPKeyPackageToml = ` +[buildpack] +url = "noop-buildpack.tgz" +` + +const multipleUnknownKeysPackageToml = ` +[buildpack] +url = "noop-buildpack.tgz" + +[[dependenceis]] # Notice: this is misspelled +image = "some/package-dep" +` + +const conflictingDependencyKeysPackageToml = ` +[buildpack] +uri = "noop-buildpack.tgz" + +[[dependencies]] +uri = "bp/b" +image = "some/package-dep" +` + +const missingBuildpackPackageToml = ` +[[dependencies]] +uri = "bp/b" +` diff --git a/cmd/pack/main.go b/cmd/pack/main.go index a36bbe092..141a68968 100644 --- a/cmd/pack/main.go +++ b/cmd/pack/main.go @@ -8,6 +8,7 @@ import ( "github.com/spf13/cobra" "github.com/buildpacks/pack" + "github.com/buildpacks/pack/buildpackage" "github.com/buildpacks/pack/cmd" "github.com/buildpacks/pack/internal/commands" "github.com/buildpacks/pack/internal/config" @@ -63,7 +64,7 @@ func main() { rootCmd.AddCommand(commands.InspectImage(logger, &cfg, &packClient)) rootCmd.AddCommand(commands.CreateBuilder(logger, &packClient)) - rootCmd.AddCommand(commands.PackageBuildpack(logger, &packClient)) + rootCmd.AddCommand(commands.PackageBuildpack(logger, &packClient, buildpackage.NewConfigReader())) rootCmd.AddCommand(commands.SetRunImagesMirrors(logger, cfg)) rootCmd.AddCommand(commands.InspectBuilder(logger, cfg, &packClient)) rootCmd.AddCommand(commands.SetDefaultBuilder(logger, cfg, &packClient)) diff --git a/create_builder_test.go b/create_builder_test.go index c8035d89a..d743a81b5 100644 --- a/create_builder_test.go +++ b/create_builder_test.go @@ -21,10 +21,10 @@ import ( "github.com/buildpacks/pack" pubbldr "github.com/buildpacks/pack/builder" + pubbldpkg "github.com/buildpacks/pack/buildpackage" "github.com/buildpacks/pack/internal/api" "github.com/buildpacks/pack/internal/blob" "github.com/buildpacks/pack/internal/builder" - "github.com/buildpacks/pack/internal/buildpackage" "github.com/buildpacks/pack/internal/dist" ifakes "github.com/buildpacks/pack/internal/fakes" "github.com/buildpacks/pack/internal/image" @@ -502,7 +502,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: buildpackage.Config{ + Config: pubbldpkg.Config{ Buildpack: dist.BuildpackURI{URI: createBuildpack(bpd)}, }, Publish: true, diff --git a/internal/buildpackage/buildpackage.go b/internal/buildpackage/buildpackage.go index 845d7f00e..45f0a204b 100644 --- a/internal/buildpackage/buildpackage.go +++ b/internal/buildpackage/buildpackage.go @@ -6,11 +6,6 @@ import ( const MetadataLabel = "io.buildpacks.buildpackage.metadata" -type Config struct { - Buildpack dist.BuildpackURI `toml:"buildpack"` - Dependencies []dist.ImageOrURI `toml:"dependencies"` -} - type Metadata struct { dist.BuildpackInfo Stacks []dist.Stack `toml:"stacks" json:"stacks"` diff --git a/internal/buildpackage/testdata/package.toml b/internal/buildpackage/testdata/package.toml new file mode 100644 index 000000000..8cd6d3894 --- /dev/null +++ b/internal/buildpackage/testdata/package.toml @@ -0,0 +1,11 @@ +[buildpack] +uri = "https://example.com/bp/a.tgz" + +[[dependencies]] +uri = "https://example.com/bp/b.tgz" + +[[dependencies]] +uri = "bp/c" + +[[dependencies]] +image = "registry.example.com/bp/d" diff --git a/internal/commands/fakes/fake_buildpack_packager.go b/internal/commands/fakes/fake_buildpack_packager.go new file mode 100644 index 000000000..22ca8244e --- /dev/null +++ b/internal/commands/fakes/fake_buildpack_packager.go @@ -0,0 +1,17 @@ +package fakes + +import ( + "context" + + "github.com/buildpacks/pack" +) + +type FakeBuildpackPackager struct { + CreateCalledWithOptions pack.PackageBuildpackOptions +} + +func (c *FakeBuildpackPackager) PackageBuildpack(ctx context.Context, opts pack.PackageBuildpackOptions) error { + c.CreateCalledWithOptions = opts + + return nil +} diff --git a/internal/commands/fakes/fake_package_config_reader.go b/internal/commands/fakes/fake_package_config_reader.go new file mode 100644 index 000000000..e0610a33d --- /dev/null +++ b/internal/commands/fakes/fake_package_config_reader.go @@ -0,0 +1,30 @@ +package fakes + +import ( + pubbldpkg "github.com/buildpacks/pack/buildpackage" +) + +type FakePackageConfigReader struct { + ReadCalledWithArg string + ReadReturnConfig pubbldpkg.Config + ReadReturnError error +} + +func (r *FakePackageConfigReader) Read(path string) (pubbldpkg.Config, error) { + r.ReadCalledWithArg = path + + return r.ReadReturnConfig, r.ReadReturnError +} + +func NewFakePackageConfigReader(ops ...func(*FakePackageConfigReader)) *FakePackageConfigReader { + fakePackageConfigReader := &FakePackageConfigReader{ + ReadReturnConfig: pubbldpkg.Config{}, + ReadReturnError: nil, + } + + for _, op := range ops { + op(fakePackageConfigReader) + } + + return fakePackageConfigReader +} diff --git a/internal/commands/package_buildpack.go b/internal/commands/package_buildpack.go index 50b1e18b6..f1e8f4b4d 100644 --- a/internal/commands/package_buildpack.go +++ b/internal/commands/package_buildpack.go @@ -1,15 +1,13 @@ package commands import ( - "path/filepath" + "context" - "github.com/BurntSushi/toml" "github.com/pkg/errors" "github.com/spf13/cobra" "github.com/buildpacks/pack" - "github.com/buildpacks/pack/internal/buildpackage" - "github.com/buildpacks/pack/internal/paths" + pubbldpkg "github.com/buildpacks/pack/buildpackage" "github.com/buildpacks/pack/internal/style" "github.com/buildpacks/pack/logging" ) @@ -20,7 +18,15 @@ type PackageBuildpackFlags struct { NoPull bool } -func PackageBuildpack(logger logging.Logger, client PackClient) *cobra.Command { +type PackageConfigReader interface { + Read(path string) (pubbldpkg.Config, error) +} + +type BuildpackPackager interface { + PackageBuildpack(ctx context.Context, options pack.PackageBuildpackOptions) error +} + +func PackageBuildpack(logger logging.Logger, client BuildpackPackager, packageConfigReader PackageConfigReader) *cobra.Command { var flags PackageBuildpackFlags ctx := createCancellableContext() cmd := &cobra.Command{ @@ -28,7 +34,7 @@ func PackageBuildpack(logger logging.Logger, client PackClient) *cobra.Command { Args: cobra.ExactArgs(1), Short: "Package buildpack", RunE: logError(logger, func(cmd *cobra.Command, args []string) error { - config, err := ReadPackageConfig(flags.PackageTomlPath) + config, err := packageConfigReader.Read(flags.PackageTomlPath) if err != nil { return errors.Wrap(err, "reading config") } @@ -58,37 +64,3 @@ func PackageBuildpack(logger logging.Logger, client PackClient) *cobra.Command { return cmd } - -func ReadPackageConfig(path string) (buildpackage.Config, error) { - config := buildpackage.Config{} - - configDir, err := filepath.Abs(filepath.Dir(path)) - if err != nil { - return config, err - } - - _, err = toml.DecodeFile(path, &config) - if err != nil { - return config, errors.Wrapf(err, "reading config %s", path) - } - - absPath, err := paths.ToAbsolute(config.Buildpack.URI, configDir) - if err != nil { - return config, errors.Wrapf(err, "getting absolute path for %s", style.Symbol(config.Buildpack.URI)) - } - config.Buildpack.URI = absPath - - for i := range config.Dependencies { - uri := config.Dependencies[i].URI - if uri != "" { - absPath, err := paths.ToAbsolute(uri, configDir) - if err != nil { - return config, errors.Wrapf(err, "getting absolute path for %s", style.Symbol(uri)) - } - - config.Dependencies[i].URI = absPath - } - } - - return config, nil -} diff --git a/internal/commands/package_buildpack_test.go b/internal/commands/package_buildpack_test.go new file mode 100644 index 000000000..c6f67b8b1 --- /dev/null +++ b/internal/commands/package_buildpack_test.go @@ -0,0 +1,166 @@ +package commands_test + +import ( + "bytes" + "fmt" + "testing" + + "github.com/heroku/color" + "github.com/pkg/errors" + "github.com/sclevine/spec" + "github.com/sclevine/spec/report" + "github.com/spf13/cobra" + + pubbldpkg "github.com/buildpacks/pack/buildpackage" + "github.com/buildpacks/pack/internal/commands" + "github.com/buildpacks/pack/internal/commands/fakes" + "github.com/buildpacks/pack/internal/dist" + "github.com/buildpacks/pack/internal/logging" + h "github.com/buildpacks/pack/testhelpers" +) + +func TestPackageBuildpackCommand(t *testing.T) { + color.Disable(true) + defer color.Disable(false) + spec.Run(t, "Commands", testPackageBuildpackCommand, spec.Parallel(), spec.Report(report.Terminal{})) +} + +func testPackageBuildpackCommand(t *testing.T, when spec.G, it spec.S) { + when("PackageBuildpack#Execute", func() { + it("reads package config from the configured path", func() { + fakePackageConfigReader := fakes.NewFakePackageConfigReader() + expectedConfigPath := "/path/to/some/file" + + packageBuildpackCommand := packageBuildpackCommand( + withConfigReader(fakePackageConfigReader), + withConfigPath(expectedConfigPath), + ) + + err := packageBuildpackCommand.Execute() + h.AssertNil(t, err) + + h.AssertEq(t, fakePackageConfigReader.ReadCalledWithArg, expectedConfigPath) + }) + + it("logs an error and exits when package toml is invalid", func() { + outBuf := &bytes.Buffer{} + expectedErr := errors.New("it went wrong") + + packageBuildpackCommand := packageBuildpackCommand( + withLogger(logging.NewLogWithWriters(outBuf, outBuf)), + withConfigReader( + fakes.NewFakePackageConfigReader(whereReadReturns(pubbldpkg.Config{}, expectedErr)), + ), + ) + + err := packageBuildpackCommand.Execute() + h.AssertNotNil(t, err) + + h.AssertContains(t, outBuf.String(), fmt.Sprintf("ERROR: reading config: %s", expectedErr)) + }) + + it("creates package with correct image name", func() { + fakeBuildpackPackager := &fakes.FakeBuildpackPackager{} + + packageBuildpackCommand := packageBuildpackCommand( + withImageName("my-specific-image"), + withBuildpackPackager(fakeBuildpackPackager), + ) + + err := packageBuildpackCommand.Execute() + h.AssertNil(t, err) + + receivedOptions := fakeBuildpackPackager.CreateCalledWithOptions + + h.AssertEq(t, receivedOptions.Name, "my-specific-image") + }) + + it("creates package with config returned by the reader", func() { + fakeBuildpackPackager := &fakes.FakeBuildpackPackager{} + + myConfig := pubbldpkg.Config{ + Buildpack: dist.BuildpackURI{URI: "test"}, + } + + packageBuildpackCommand := packageBuildpackCommand( + withBuildpackPackager(fakeBuildpackPackager), + withConfigReader(fakes.NewFakePackageConfigReader(whereReadReturns(myConfig, nil))), + ) + + err := packageBuildpackCommand.Execute() + h.AssertNil(t, err) + + receivedOptions := fakeBuildpackPackager.CreateCalledWithOptions + + h.AssertEq(t, receivedOptions.Config, myConfig) + }) + }) +} + +type packageCommandConfig struct { + logger *logging.LogWithWriters + configReader *fakes.FakePackageConfigReader + buildpackPackager *fakes.FakeBuildpackPackager + + imageName string + configPath string +} + +type packageCommandOption func(config *packageCommandConfig) + +func packageBuildpackCommand(ops ...packageCommandOption) *cobra.Command { + config := &packageCommandConfig{ + logger: logging.NewLogWithWriters(&bytes.Buffer{}, &bytes.Buffer{}), + configReader: fakes.NewFakePackageConfigReader(), + buildpackPackager: &fakes.FakeBuildpackPackager{}, + + imageName: "some-image-name", + configPath: "/path/to/some/file", + } + + for _, op := range ops { + op(config) + } + + cmd := commands.PackageBuildpack(config.logger, config.buildpackPackager, config.configReader) + cmd.SetArgs([]string{config.imageName, "--package-config", config.configPath}) + + return cmd +} + +func withLogger(logger *logging.LogWithWriters) packageCommandOption { + return func(config *packageCommandConfig) { + config.logger = logger + } +} + +func withConfigReader(reader *fakes.FakePackageConfigReader) packageCommandOption { + return func(config *packageCommandConfig) { + config.configReader = reader + } +} + +func withBuildpackPackager(creator *fakes.FakeBuildpackPackager) packageCommandOption { + return func(config *packageCommandConfig) { + config.buildpackPackager = creator + } +} + +func withImageName(name string) packageCommandOption { + return func(config *packageCommandConfig) { + config.imageName = name + } +} + +func withConfigPath(path string) packageCommandOption { + return func(config *packageCommandConfig) { + config.configPath = path + } +} + +func whereReadReturns(config pubbldpkg.Config, err error) func(*fakes.FakePackageConfigReader) { + return func(r *fakes.FakePackageConfigReader) { + r.ReadReturnConfig = config + r.ReadReturnError = err + } +} diff --git a/package_buildpack.go b/package_buildpack.go index 26d67983e..56a47262a 100644 --- a/package_buildpack.go +++ b/package_buildpack.go @@ -5,6 +5,7 @@ import ( "github.com/pkg/errors" + pubbldpkg "github.com/buildpacks/pack/buildpackage" "github.com/buildpacks/pack/internal/buildpackage" "github.com/buildpacks/pack/internal/dist" "github.com/buildpacks/pack/internal/style" @@ -12,7 +13,7 @@ import ( type PackageBuildpackOptions struct { Name string - Config buildpackage.Config + Config pubbldpkg.Config Publish bool NoPull bool } diff --git a/package_buildpack_test.go b/package_buildpack_test.go index 45cee5e3c..4780bd42c 100644 --- a/package_buildpack_test.go +++ b/package_buildpack_test.go @@ -14,8 +14,8 @@ import ( "github.com/sclevine/spec/report" "github.com/buildpacks/pack" + pubbldpkg "github.com/buildpacks/pack/buildpackage" "github.com/buildpacks/pack/internal/api" - "github.com/buildpacks/pack/internal/buildpackage" "github.com/buildpacks/pack/internal/dist" ifakes "github.com/buildpacks/pack/internal/fakes" "github.com/buildpacks/pack/internal/image" @@ -77,7 +77,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: buildpackage.Config{ + Config: pubbldpkg.Config{ Buildpack: dist.BuildpackURI{URI: createBuildpack(dist.BuildpackDescriptor{ API: api.MustParse("0.2"), Info: dist.BuildpackInfo{ID: "bp.nested", Version: "2.3.4"}, @@ -115,7 +115,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: buildpackage.Config{ + Config: pubbldpkg.Config{ Buildpack: dist.BuildpackURI{URI: createBuildpack(dist.BuildpackDescriptor{ API: api.MustParse("0.2"), Info: dist.BuildpackInfo{ID: "bp.1", Version: "1.2.3"}, @@ -141,7 +141,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: buildpackage.Config{ + Config: pubbldpkg.Config{ Buildpack: dist.BuildpackURI{URI: createBuildpack(dist.BuildpackDescriptor{ API: api.MustParse("0.2"), Info: dist.BuildpackInfo{ID: "bp.1", Version: "1.2.3"}, @@ -167,7 +167,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: buildpackage.Config{ + Config: pubbldpkg.Config{ Buildpack: dist.BuildpackURI{URI: createBuildpack(dist.BuildpackDescriptor{ API: api.MustParse("0.2"), Info: dist.BuildpackInfo{ID: "bp.1", Version: "1.2.3"}, @@ -192,7 +192,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: buildpackage.Config{ + Config: pubbldpkg.Config{ Buildpack: dist.BuildpackURI{URI: createBuildpack(dist.BuildpackDescriptor{ API: api.MustParse("0.2"), Info: dist.BuildpackInfo{ID: "bp.1", Version: "1.2.3"}, @@ -214,7 +214,7 @@ func testPackageBuildpack(t *testing.T, when spec.G, it spec.S) { h.AssertError(t, subject.PackageBuildpack(context.TODO(), pack.PackageBuildpackOptions{ Name: "", - Config: buildpackage.Config{ + Config: pubbldpkg.Config{ Buildpack: dist.BuildpackURI{URI: createBuildpack(dist.BuildpackDescriptor{ API: api.MustParse("0.2"), Info: dist.BuildpackInfo{ID: "bp.1", Version: "1.2.3"},