Skip to content

Commit

Permalink
Change from --image and --file to --format
Browse files Browse the repository at this point in the history 
Signed-off-by: Javier Romero <[email protected]>
  • Loading branch information
@jromero
jromero committed Mar 18, 2020
1 parent ac3abd9 commit b5a146f
Show file tree
Hide file tree
Showing 6 changed files with 67 additions and 89 deletions.
27 changes: 15 additions & 12 deletions acceptance/acceptance_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1121,7 +1121,7 @@ func testAcceptance(t *testing.T, when spec.G, it spec.S, packFixturesDir, packP
packageBuildpackLocally := func(absConfigPath string) string {
t.Helper()
packageName := "test/package-" + h.RandString(10)
output, err := h.RunE(subjectPack("package-buildpack", "--image", packageName, "-p", absConfigPath))
output, err := h.RunE(subjectPack("package-buildpack", packageName, "-p", absConfigPath))
h.AssertNil(t, err)
h.AssertContains(t, output, fmt.Sprintf("Successfully created package '%s'", packageName))
return packageName
Expand All @@ -1130,7 +1130,7 @@ func testAcceptance(t *testing.T, when spec.G, it spec.S, packFixturesDir, packP
packageBuildpackRemotely := func(absConfigPath string) string {
t.Helper()
packageName := registryConfig.RepoName("test/package-" + h.RandString(10))
output, err := h.RunE(subjectPack("package-buildpack", "--image", packageName, "-p", absConfigPath, "--publish"))
output, err := h.RunE(subjectPack("package-buildpack", packageName, "-p", absConfigPath, "--publish"))
h.AssertNil(t, err)
h.AssertContains(t, output, fmt.Sprintf("Successfully published package '%s'", packageName))
return packageName
Expand Down Expand Up @@ -1161,21 +1161,19 @@ func testAcceptance(t *testing.T, when spec.G, it spec.S, packFixturesDir, packP
return packageTomlFile.Name()
}

// TODO: Deprecated - remove on next release
when("image name is provided as argument 0", func() {
it("creates the package", func() {
when("no --format is provided", func() {
it("creates the package as image", func() {
packageName := "test/package-" + h.RandString(10)
output, err := h.RunE(subjectPack("package-buildpack", packageName, "-p", filepath.Join(tmpDir, "package.toml")))
h.AssertNil(t, err)
h.AssertContains(t, output, fmt.Sprintf("Successfully created package '%s'", packageName))
h.AssertContains(t, output, `positional argument for image name is deprecated, use "--image" instead`)
defer h.DockerRmi(dockerCli, packageName)

assertImageExistsLocally(packageName)
})
})

when("--image", func() {
when("--format image", func() {
it("creates the package", func() {
t.Log("package w/ only buildpacks")
nestedPackageName := packageBuildpackLocally(filepath.Join(tmpDir, "package.toml"))
Expand All @@ -1197,7 +1195,7 @@ func testAcceptance(t *testing.T, when spec.G, it spec.S, packFixturesDir, packP

packageName := registryConfig.RepoName("test/package-" + h.RandString(10))
defer h.DockerRmi(dockerCli, packageName)
output := h.Run(t, subjectPack("package-buildpack", "--image", packageName, "-p", aggregatePackageToml, "--publish"))
output := h.Run(t, subjectPack("package-buildpack", packageName, "-p", aggregatePackageToml, "--publish"))
h.AssertContains(t, output, fmt.Sprintf("Successfully published package '%s'", packageName))

_, _, err := dockerCli.ImageInspectWithRaw(context.Background(), packageName)
Expand Down Expand Up @@ -1238,13 +1236,16 @@ func testAcceptance(t *testing.T, when spec.G, it spec.S, packFixturesDir, packP
})
})

when("--file", func() {
when("--format cnb", func() {
it.Before(func() {
h.SkipIf(t, !packSupports(packPath, "package-buildpack --format"), "format not supported")
})

it("creates the package", func() {
outputFile := filepath.Join(tmpDir, "package.cnb")
output, err := h.RunE(subjectPack("package-buildpack", "--file", outputFile, "-p", filepath.Join(tmpDir, "package.toml")))
output, err := h.RunE(subjectPack("package-buildpack", outputFile, "--format", "cnb", "-p", filepath.Join(tmpDir, "package.toml")))
h.AssertNil(t, err)
h.AssertContains(t, output, fmt.Sprintf("Successfully created package '%s'", outputFile))

h.AssertTarball(t, outputFile)
})
})
Expand Down Expand Up @@ -1333,7 +1334,9 @@ func resolveRunCombinations(
}

rc.packPath = previousPackPath
rc.packFixturesDir = previousPackFixturesPath
if previousPackFixturesPath != "" {
rc.packFixturesDir = previousPackFixturesPath
}
}

if c.PackCreateBuilder == "previous" {
Expand Down
2 changes: 1 addition & 1 deletion create_builder_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -505,7 +505,7 @@ func testCreateBuilder(t *testing.T, when spec.G, it spec.S) {
}

h.AssertNil(t, subject.PackageBuildpack(context.TODO(), pack.PackageBuildpackOptions{
ImageName: packageImage.Name(),
Name: packageImage.Name(),
Config: pubbldpkg.Config{
Buildpack: dist.BuildpackURI{URI: createBuildpack(bpd)},
},
Expand Down
40 changes: 11 additions & 29 deletions internal/commands/package_buildpack.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,7 @@ import (

type PackageBuildpackFlags struct {
PackageTomlPath string
ImageName string
OutputFile string
Format string
Publish bool
NoPull bool
}
Expand All @@ -32,33 +31,22 @@ func PackageBuildpack(logger logging.Logger, client BuildpackPackager, packageCo
var flags PackageBuildpackFlags
ctx := createCancellableContext()
cmd := &cobra.Command{
Use: `package-buildpack (--image <image-name> | --file <output-file>) --package-config <package-config-path>`,
Use: `package-buildpack <name> --package-config <package-config-path>`,
Short: "Package buildpack in OCI format.",
Args: cobra.MaximumNArgs(1),
PreRunE: func(cmd *cobra.Command, args []string) error {
if len(args) > 0 && args[0] != "" {
logger.Warn(`positional argument for image name is deprecated, use "--image" instead.`)
flags.ImageName = args[0]
}

if flags.ImageName == "" && flags.OutputFile == "" {
return errors.Errorf(`must provide either "--image" or "--file" flag`)
}

return nil
},
Args: cobra.ExactValidArgs(1),
RunE: logError(logger, func(cmd *cobra.Command, args []string) error {
config, err := packageConfigReader.Read(flags.PackageTomlPath)
if err != nil {
return errors.Wrap(err, "reading config")
}

name := args[0]
if err := client.PackageBuildpack(ctx, pack.PackageBuildpackOptions{
ImageName: flags.ImageName,
OutputFile: flags.OutputFile,
Config: config,
Publish: flags.Publish,
NoPull: flags.NoPull,
Name: name,
Format: flags.Format,
Config: config,
Publish: flags.Publish,
NoPull: flags.NoPull,
}); err != nil {
return err
}
Expand All @@ -68,19 +56,13 @@ func PackageBuildpack(logger logging.Logger, client BuildpackPackager, packageCo
action = "published"
}

output := flags.ImageName
if flags.OutputFile != "" {
output = flags.OutputFile
}

logger.Infof("Successfully %s package %s", action, style.Symbol(output))
logger.Infof("Successfully %s package %s", action, style.Symbol(name))
return nil
}),
}
cmd.Flags().StringVarP(&flags.PackageTomlPath, "package-config", "p", "", "Path to package TOML config (required)")
cmd.MarkFlagRequired("package-config")
cmd.Flags().StringVarP(&flags.ImageName, "image", "i", "", "Save package as image")
cmd.Flags().StringVarP(&flags.OutputFile, "file", "f", "", "Save package as file")
cmd.Flags().StringVarP(&flags.Format, "format", "f", "", `Format to save package as ("image" or "cnb")`)
cmd.Flags().BoolVar(&flags.Publish, "publish", false, `Publish to registry (applies to "--image" only)`)
cmd.Flags().BoolVar(&flags.NoPull, "no-pull", false, "Skip pulling packages before use")
AddHelpFlag(cmd, "package-buildpack")
Expand Down
2 changes: 1 addition & 1 deletion internal/commands/package_buildpack_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ func testPackageBuildpackCommand(t *testing.T, when spec.G, it spec.S) {

receivedOptions := fakeBuildpackPackager.CreateCalledWithOptions

h.AssertEq(t, receivedOptions.ImageName, "my-specific-image")
h.AssertEq(t, receivedOptions.Name, "my-specific-image")
})

it("creates package with config returned by the reader", func() {
Expand Down
40 changes: 20 additions & 20 deletions package_buildpack.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,25 +11,26 @@ import (
"github.com/buildpacks/pack/internal/style"
)

const (
FormatImage = "image"
FormatCNB = "cnb"
)

type PackageBuildpackOptions struct {
ImageName string
OutputFile string
Config pubbldpkg.Config
Publish bool
NoPull bool
Name string
Format string
Config pubbldpkg.Config
Publish bool
NoPull bool
}

func (c *Client) PackageBuildpack(ctx context.Context, opts PackageBuildpackOptions) error {
if opts.ImageName == "" && opts.OutputFile == "" {
return errors.New("must provide image name or output file")
}
packageBuilder := buildpackage.NewBuilder(c.imageFactory)

if opts.ImageName != "" && opts.OutputFile != "" {
return errors.New("must only provide one, image name or output file")
if opts.Format == "" {
opts.Format = FormatImage
}

packageBuilder := buildpackage.NewBuilder(c.imageFactory)

bpURI := opts.Config.Buildpack.URI
if bpURI == "" {
return errors.New("buildpack URI must be provided")
Expand Down Expand Up @@ -72,14 +73,13 @@ func (c *Client) PackageBuildpack(ctx context.Context, opts PackageBuildpackOpti
}
}

if opts.OutputFile != "" {
return packageBuilder.SaveAsFile(opts.OutputFile)
}

_, err = packageBuilder.SaveAsImage(opts.ImageName, opts.Publish)
if err != nil {
switch opts.Format {
case FormatCNB:
return packageBuilder.SaveAsFile(opts.Name)
case FormatImage:
_, err = packageBuilder.SaveAsImage(opts.Name, opts.Publish)
return errors.Wrapf(err, "saving image")
default:
return errors.Errorf("unknown format: %s", style.Symbol(opts.Format))
}

return err
}
45 changes: 19 additions & 26 deletions package_buildpack_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ func testPackageBuildpack(t *testing.T, when spec.G, it spec.S) {
mockImageFactory.EXPECT().NewImage(nestedPackage.Name(), false).Return(nestedPackage, nil)

h.AssertNil(t, subject.PackageBuildpack(context.TODO(), pack.PackageBuildpackOptions{
ImageName: nestedPackage.Name(),
Name: nestedPackage.Name(),
Config: pubbldpkg.Config{
Buildpack: dist.BuildpackURI{URI: createBuildpack(dist.BuildpackDescriptor{
API: api.MustParse("0.2"),
Expand Down Expand Up @@ -114,7 +114,7 @@ func testPackageBuildpack(t *testing.T, when spec.G, it spec.S) {
packageImage := shouldCreateLocalPackage()

h.AssertNil(t, subject.PackageBuildpack(context.TODO(), pack.PackageBuildpackOptions{
ImageName: packageImage.Name(),
Name: packageImage.Name(),
Config: pubbldpkg.Config{
Buildpack: dist.BuildpackURI{URI: createBuildpack(dist.BuildpackDescriptor{
API: api.MustParse("0.2"),
Expand All @@ -140,7 +140,7 @@ func testPackageBuildpack(t *testing.T, when spec.G, it spec.S) {
packageImage := shouldCreateRemotePackage()

h.AssertNil(t, subject.PackageBuildpack(context.TODO(), pack.PackageBuildpackOptions{
ImageName: packageImage.Name(),
Name: packageImage.Name(),
Config: pubbldpkg.Config{
Buildpack: dist.BuildpackURI{URI: createBuildpack(dist.BuildpackDescriptor{
API: api.MustParse("0.2"),
Expand All @@ -166,7 +166,7 @@ func testPackageBuildpack(t *testing.T, when spec.G, it spec.S) {
packageImage := shouldCreateRemotePackage()

h.AssertNil(t, subject.PackageBuildpack(context.TODO(), pack.PackageBuildpackOptions{
ImageName: packageImage.Name(),
Name: packageImage.Name(),
Config: pubbldpkg.Config{
Buildpack: dist.BuildpackURI{URI: createBuildpack(dist.BuildpackDescriptor{
API: api.MustParse("0.2"),
Expand All @@ -191,7 +191,7 @@ func testPackageBuildpack(t *testing.T, when spec.G, it spec.S) {
shouldNotFindNestedPackageWhenCallingImageFetcherWith(true, false)

h.AssertError(t, subject.PackageBuildpack(context.TODO(), pack.PackageBuildpackOptions{
ImageName: "some/package",
Name: "some/package",
Config: pubbldpkg.Config{
Buildpack: dist.BuildpackURI{URI: createBuildpack(dist.BuildpackDescriptor{
API: api.MustParse("0.2"),
Expand All @@ -213,7 +213,7 @@ func testPackageBuildpack(t *testing.T, when spec.G, it spec.S) {
mockImageFetcher.EXPECT().Fetch(gomock.Any(), notPackageImage.Name(), true, true).Return(notPackageImage, nil)

h.AssertError(t, subject.PackageBuildpack(context.TODO(), pack.PackageBuildpackOptions{
ImageName: "some/package",
Name: "some/package",
Config: pubbldpkg.Config{
Buildpack: dist.BuildpackURI{URI: createBuildpack(dist.BuildpackDescriptor{
API: api.MustParse("0.2"),
Expand All @@ -228,29 +228,22 @@ func testPackageBuildpack(t *testing.T, when spec.G, it spec.S) {
})
})

when("both image name and output file are provided", func() {
when("unknown format is provided", func() {
it("should error", func() {
err := subject.PackageBuildpack(context.TODO(), pack.PackageBuildpackOptions{
ImageName: "some/image",
OutputFile: "some-file.cnb",
Config: pubbldpkg.Config{},
Publish: false,
NoPull: false,
})
h.AssertError(t, err, "must only provide one, image name or output file")
})
})

when("no image name or output file is provided", func() {
it("should error", func() {
err := subject.PackageBuildpack(context.TODO(), pack.PackageBuildpackOptions{
ImageName: "",
OutputFile: "",
Config: pubbldpkg.Config{},
Publish: false,
NoPull: false,
Name: "some-buildpack",
Format: "invalid-format",
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"},
Stacks: []dist.Stack{{ID: "some.stack.id"}},
})},
},
Publish: false,
NoPull: false,
})
h.AssertError(t, err, "must provide image name or output file")
h.AssertError(t, err, "unknown format: 'invalid-format'")
})
})
}

0 comments on commit b5a146f

Please sign in to comment.