Merge pull request #10836 from Luap99/diff

podman diff accept two images or containers

4 files changed, 112 insertions, 91 deletions

diff --git a/cmd/podman/containers/diff.go b/cmd/podman/containers/diff.go
index 0eee85825..7463e96ad 100644
--- a/cmd/podman/containers/diff.go
+++ b/cmd/podman/containers/diff.go
@@ -1,10 +1,11 @@
 package containers
 
 import (
-	"github.com/containers/common/pkg/report"
 	"github.com/containers/podman/v3/cmd/podman/common"
+	"github.com/containers/podman/v3/cmd/podman/diff"
 	"github.com/containers/podman/v3/cmd/podman/registry"
 	"github.com/containers/podman/v3/cmd/podman/validate"
+	"github.com/containers/podman/v3/libpod/define"
 	"github.com/containers/podman/v3/pkg/domain/entities"
 	"github.com/pkg/errors"
 	"github.com/spf13/cobra"
@@ -13,11 +14,11 @@ import (
 var (
 	// podman container _diff_
 	diffCmd = &cobra.Command{
-		Use:               "diff [options] CONTAINER",
-		Args:              validate.IDOrLatestArgs,
+		Use:               "diff [options] CONTAINER [CONTAINER]",
+		Args:              diff.ValidateContainerDiffArgs,
 		Short:             "Inspect changes to the container's file systems",
-		Long:              `Displays changes to the container filesystem's'.  The container will be compared to its parent layer.`,
-		RunE:              diff,
+		Long:              `Displays changes to the container filesystem's'.  The container will be compared to its parent layer or the second argument when given.`,
+		RunE:              diffRun,
 		ValidArgsFunction: common.AutocompleteContainers,
 		Example: `podman container diff myCtr
   podman container diff -l --format json myCtr`,
@@ -33,41 +34,22 @@ func init() {
 
 	diffOpts = &entities.DiffOptions{}
 	flags := diffCmd.Flags()
+
+	// FIXME: Why does this exists? It is not used anywhere.
 	flags.BoolVar(&diffOpts.Archive, "archive", true, "Save the diff as a tar archive")
 	_ = flags.MarkHidden("archive")
 
 	formatFlagName := "format"
-	flags.StringVar(&diffOpts.Format, formatFlagName, "", "Change the output format")
+	flags.StringVar(&diffOpts.Format, formatFlagName, "", "Change the output format (json)")
 	_ = diffCmd.RegisterFlagCompletionFunc(formatFlagName, common.AutocompleteFormat(nil))
 
 	validate.AddLatestFlag(diffCmd, &diffOpts.Latest)
 }
 
-func diff(cmd *cobra.Command, args []string) error {
+func diffRun(cmd *cobra.Command, args []string) error {
 	if len(args) == 0 && !diffOpts.Latest {
 		return errors.New("container must be specified: podman container diff [options [...]] ID-NAME")
 	}
-
-	var id string
-	if len(args) > 0 {
-		id = args[0]
-	}
-	results, err := registry.ContainerEngine().ContainerDiff(registry.GetContext(), id, *diffOpts)
-	if err != nil {
-		return err
-	}
-
-	switch {
-	case report.IsJSON(diffOpts.Format):
-		return common.ChangesToJSON(results)
-	case diffOpts.Format == "":
-		return common.ChangesToTable(results)
-	default:
-		return errors.New("only supported value for '--format' is 'json'")
-	}
-}
-
-func Diff(cmd *cobra.Command, args []string, options entities.DiffOptions) error {
-	diffOpts = &options
-	return diff(cmd, args)
+	diffOpts.Type = define.DiffContainer
+	return diff.Diff(cmd, args, *diffOpts)
 }
diff --git a/cmd/podman/diff.go b/cmd/podman/diff.go
index e2a3cd727..c592b6a22 100644
--- a/cmd/podman/diff.go
+++ b/cmd/podman/diff.go
@@ -1,13 +1,11 @@
 package main
 
 import (
-	"fmt"
-
 	"github.com/containers/podman/v3/cmd/podman/common"
-	"github.com/containers/podman/v3/cmd/podman/containers"
-	"github.com/containers/podman/v3/cmd/podman/images"
+	"github.com/containers/podman/v3/cmd/podman/diff"
 	"github.com/containers/podman/v3/cmd/podman/registry"
 	"github.com/containers/podman/v3/cmd/podman/validate"
+	"github.com/containers/podman/v3/libpod/define"
 	"github.com/containers/podman/v3/pkg/domain/entities"
 	"github.com/spf13/cobra"
 )
@@ -16,13 +14,13 @@ import (
 
 var (
 	// Command: podman _diff_ Object_ID
-	diffDescription = `Displays changes on a container or image's filesystem.  The container or image will be compared to its parent layer.`
+	diffDescription = `Displays changes on a container or image's filesystem.  The container or image will be compared to its parent layer or the second argument when given.`
 	diffCmd         = &cobra.Command{
-		Use:               "diff [options] {CONTAINER|IMAGE}",
-		Args:              validate.IDOrLatestArgs,
+		Use:               "diff [options] {CONTAINER|IMAGE} [{CONTAINER|IMAGE}]",
+		Args:              diff.ValidateContainerDiffArgs,
 		Short:             "Display the changes to the object's file system",
 		Long:              diffDescription,
-		RunE:              diff,
+		RunE:              diffRun,
 		ValidArgsFunction: common.AutocompleteContainersAndImages,
 		Example: `podman diff imageID
   podman diff ctrID
@@ -37,36 +35,18 @@ func init() {
 		Command: diffCmd,
 	})
 	flags := diffCmd.Flags()
+	// FIXME: Why does this exists? It is not used anywhere.
 	flags.BoolVar(&diffOpts.Archive, "archive", true, "Save the diff as a tar archive")
 	_ = flags.MarkHidden("archive")
 
 	formatFlagName := "format"
-	flags.StringVar(&diffOpts.Format, formatFlagName, "", "Change the output format")
+	flags.StringVar(&diffOpts.Format, formatFlagName, "", "Change the output format (json)")
 	_ = diffCmd.RegisterFlagCompletionFunc(formatFlagName, common.AutocompleteFormat(nil))
 
 	validate.AddLatestFlag(diffCmd, &diffOpts.Latest)
 }
 
-func diff(cmd *cobra.Command, args []string) error {
-	// Latest implies looking for a container
-	if diffOpts.Latest {
-		return containers.Diff(cmd, args, diffOpts)
-	}
-
-	options := entities.ContainerExistsOptions{
-		External: true,
-	}
-	if found, err := registry.ContainerEngine().ContainerExists(registry.GetContext(), args[0], options); err != nil {
-		return err
-	} else if found.Value {
-		return containers.Diff(cmd, args, diffOpts)
-	}
-
-	if found, err := registry.ImageEngine().Exists(registry.GetContext(), args[0]); err != nil {
-		return err
-	} else if found.Value {
-		return images.Diff(cmd, args, diffOpts)
-	}
-
-	return fmt.Errorf("%s not found on system", args[0])
+func diffRun(cmd *cobra.Command, args []string) error {
+	diffOpts.Type = define.DiffAll
+	return diff.Diff(cmd, args, diffOpts)
 }
diff --git a/cmd/podman/diff/diff.go b/cmd/podman/diff/diff.go
new file mode 100644
index 000000000..81bbb6c43
--- /dev/null
+++ b/cmd/podman/diff/diff.go
@@ -0,0 +1,79 @@
+package diff
+
+import (
+	"encoding/json"
+	"fmt"
+	"os"
+
+	"github.com/containers/common/pkg/report"
+	"github.com/containers/podman/v3/cmd/podman/registry"
+	"github.com/containers/podman/v3/pkg/domain/entities"
+	"github.com/docker/docker/pkg/archive"
+	"github.com/pkg/errors"
+	"github.com/spf13/cobra"
+)
+
+func Diff(cmd *cobra.Command, args []string, options entities.DiffOptions) error {
+	results, err := registry.ContainerEngine().Diff(registry.GetContext(), args, options)
+	if err != nil {
+		return err
+	}
+
+	switch {
+	case report.IsJSON(options.Format):
+		return changesToJSON(results)
+	case options.Format == "":
+		return changesToTable(results)
+	default:
+		return errors.New("only supported value for '--format' is 'json'")
+	}
+}
+
+type ChangesReportJSON struct {
+	Changed []string `json:"changed,omitempty"`
+	Added   []string `json:"added,omitempty"`
+	Deleted []string `json:"deleted,omitempty"`
+}
+
+func changesToJSON(diffs *entities.DiffReport) error {
+	body := ChangesReportJSON{}
+	for _, row := range diffs.Changes {
+		switch row.Kind {
+		case archive.ChangeAdd:
+			body.Added = append(body.Added, row.Path)
+		case archive.ChangeDelete:
+			body.Deleted = append(body.Deleted, row.Path)
+		case archive.ChangeModify:
+			body.Changed = append(body.Changed, row.Path)
+		default:
+			return errors.Errorf("output kind %q not recognized", row.Kind)
+		}
+	}
+
+	// Pull in configured json library
+	enc := json.NewEncoder(os.Stdout)
+	enc.SetIndent("", "     ")
+	return enc.Encode(body)
+}
+
+func changesToTable(diffs *entities.DiffReport) error {
+	for _, row := range diffs.Changes {
+		fmt.Fprintln(os.Stdout, row.String())
+	}
+	return nil
+}
+
+// IDOrLatestArgs used to validate a nameOrId was provided or the "--latest" flag
+func ValidateContainerDiffArgs(cmd *cobra.Command, args []string) error {
+	given, _ := cmd.Flags().GetBool("latest")
+	if len(args) > 0 && !given {
+		return cobra.RangeArgs(1, 2)(cmd, args)
+	}
+	if len(args) > 0 && given {
+		return errors.New("--latest and containers cannot be used together")
+	}
+	if len(args) == 0 && !given {
+		return errors.Errorf("%q requires a name, id, or the \"--latest\" flag", cmd.CommandPath())
+	}
+	return nil
+}
diff --git a/cmd/podman/images/diff.go b/cmd/podman/images/diff.go
index 2e883d7ae..825aab362 100644
--- a/cmd/podman/images/diff.go
+++ b/cmd/podman/images/diff.go
@@ -1,11 +1,11 @@
 package images
 
 import (
-	"github.com/containers/common/pkg/report"
 	"github.com/containers/podman/v3/cmd/podman/common"
+	"github.com/containers/podman/v3/cmd/podman/diff"
 	"github.com/containers/podman/v3/cmd/podman/registry"
+	"github.com/containers/podman/v3/libpod/define"
 	"github.com/containers/podman/v3/pkg/domain/entities"
-	"github.com/pkg/errors"
 	"github.com/spf13/cobra"
 	"github.com/spf13/pflag"
 )
@@ -13,11 +13,11 @@ import (
 var (
 	// podman container _inspect_
 	diffCmd = &cobra.Command{
-		Use:               "diff [options] IMAGE",
-		Args:              cobra.ExactArgs(1),
+		Use:               "diff [options] IMAGE [IMAGE]",
+		Args:              cobra.RangeArgs(1, 2),
 		Short:             "Inspect changes to the image's file systems",
-		Long:              `Displays changes to the image's filesystem.  The image will be compared to its parent layer.`,
-		RunE:              diff,
+		Long:              `Displays changes to the image's filesystem.  The image will be compared to its parent layer or the second argument when given.`,
+		RunE:              diffRun,
 		ValidArgsFunction: common.AutocompleteImages,
 		Example: `podman image diff myImage
   podman image diff --format json redis:alpine`,
@@ -39,31 +39,11 @@ func diffFlags(flags *pflag.FlagSet) {
 	_ = flags.MarkDeprecated("archive", "Provided for backwards compatibility, has no impact on output.")
 
 	formatFlagName := "format"
-	flags.StringVar(&diffOpts.Format, formatFlagName, "", "Change the output format")
+	flags.StringVar(&diffOpts.Format, formatFlagName, "", "Change the output format (json)")
 	_ = diffCmd.RegisterFlagCompletionFunc(formatFlagName, common.AutocompleteFormat(nil))
 }
 
-func diff(cmd *cobra.Command, args []string) error {
-	if diffOpts.Latest {
-		return errors.New("image diff does not support --latest")
-	}
-
-	results, err := registry.ImageEngine().Diff(registry.GetContext(), args[0], *diffOpts)
-	if err != nil {
-		return err
-	}
-
-	switch {
-	case report.IsJSON(diffOpts.Format):
-		return common.ChangesToJSON(results)
-	case diffOpts.Format == "":
-		return common.ChangesToTable(results)
-	default:
-		return errors.New("only supported value for '--format' is 'json'")
-	}
-}
-
-func Diff(cmd *cobra.Command, args []string, options entities.DiffOptions) error {
-	diffOpts = &options
-	return diff(cmd, args)
+func diffRun(cmd *cobra.Command, args []string) error {
+	diffOpts.Type = define.DiffImage
+	return diff.Diff(cmd, args, *diffOpts)
 }