diff --git a/README.md b/README.md index d9c73aa0..24e2f0a9 100644 --- a/README.md +++ b/README.md @@ -43,27 +43,29 @@ To use `container-diff analyze` to perform analysis on a single image, you need ``` container-diff analyze [Run all analyzers] -container-diff analyze -d [History] -container-diff analyze -f [File System] -container-diff analyze -p [Pip] -container-diff analyze -a [Apt] -container-diff analyze -n [Node] +container-diff analyze --types=history [History] +container-diff analyze --types=file [File System] +container-diff analyze --types=pip [Pip] +container-diff analyze --types=apt [Apt] +container-diff analyze --types=node [Node] +container-diff analyze --types=apt,node [Apt and Node] +# --types=,,,... ``` To use container-diff to perform a diff analysis on two images, you need two Docker images (in the form of an ID, tarball, or URL from a repo). Once you have those images, you can run any of the following differs: ``` container-diff diff [Run all differs] -container-diff diff -d [History] -container-diff diff -f [File System] -container-diff diff -p [Pip] -container-diff diff -a [Apt] -container-diff diff -n [Node] +container-diff diff --types=history [History] +container-diff diff --types=file [File System] +container-diff diff --types=pip [Pip] +container-diff diff --types=apt [Apt] +container-diff diff --types=node [Node] ``` You can similarly run many analyzers at once: ``` -container-diff diff -d -a -n [History, Apt, and Node] +container-diff diff --types=history,apt,node [History, Apt, and Node] ``` All of the analyzer flags with their long versions can be seen below: @@ -228,7 +230,7 @@ To run container-diff on image IDs, docker must be installed. ## Example Run ``` -$ container-diff gcr.io/google-appengine/python:2017-07-21-123058 gcr.io/google-appengine/python:2017-06-29-190410 -a -n -p +$ container-diff diff gcr.io/google-appengine/python:2017-07-21-123058 gcr.io/google-appengine/python:2017-06-29-190410 --types=apt,node,pip -----AptDiffer----- @@ -372,7 +374,3 @@ This is where you define how your analyzer should output for a human readable fo 5. Add your analyzer to the `analyses` map in [differs.go](https://github.com/GoogleCloudPlatform/container-diff/blob/master/differs/differs.go#L22) with the corresponding Analyzer struct as the value. - - - - diff --git a/cmd/analyze.go b/cmd/analyze.go index 8ea3a5d7..55c877b5 100644 --- a/cmd/analyze.go +++ b/cmd/analyze.go @@ -4,6 +4,7 @@ import ( "errors" "fmt" "os" + "strings" "github.com/GoogleCloudPlatform/container-diff/differs" "github.com/GoogleCloudPlatform/container-diff/utils" @@ -16,37 +17,29 @@ var analyzeCmd = &cobra.Command{ Short: "Analyzes an image: [image]", Long: `Analyzes an image using the specifed analyzers as indicated via flags (see documentation for available ones).`, Run: func(cmd *cobra.Command, args []string) { - if validArgs, err := validateArgs(args, checkAnalyzeArgNum, checkArgType); !validArgs { + if err := validateArgs(args, checkAnalyzeArgNum, checkArgType); err != nil { glog.Error(err.Error()) os.Exit(1) } - analyzeArgs := []string{} - allAnalyzers := getAllAnalyzers() - for _, name := range allAnalyzers { - if *analyzeFlagMap[name] == true { - analyzeArgs = append(analyzeArgs, name) - } - } - - // If no analyzers are specified, perform them all as the default - if len(analyzeArgs) == 0 { - analyzeArgs = allAnalyzers + if err := checkIfValidAnalyzer(types); err != nil { + glog.Error(err) + os.Exit(1) } - - if err := analyzeImage(args[0], analyzeArgs); err != nil { + if err := analyzeImage(args[0], strings.Split(types, ",")); err != nil { glog.Error(err) os.Exit(1) } }, } -func checkAnalyzeArgNum(args []string) (bool, error) { +func checkAnalyzeArgNum(args []string) error { var errMessage string if len(args) != 1 { errMessage = "'analyze' requires one image as an argument: container analyze [image]" - return false, errors.New(errMessage) + glog.Errorf(errMessage) + return errors.New(errMessage) } - return true, nil + return nil } func analyzeImage(imageArg string, analyzerArgs []string) error { diff --git a/cmd/analyze_test.go b/cmd/analyze_test.go index 8c74de1f..3f429f21 100644 --- a/cmd/analyze_test.go +++ b/cmd/analyze_test.go @@ -12,8 +12,8 @@ var analyzeArgNumTests = []testpair{ func TestAnalyzeArgNum(t *testing.T) { for _, test := range analyzeArgNumTests { - valid, err := checkAnalyzeArgNum(test.input) - if valid != test.expected_output { + err := checkAnalyzeArgNum(test.input) + if (err == nil) != test.expected_output { if test.expected_output { t.Errorf("Got unexpected error: %s", err) } else { diff --git a/cmd/diff.go b/cmd/diff.go index f8124fc9..fdcab75c 100644 --- a/cmd/diff.go +++ b/cmd/diff.go @@ -4,6 +4,7 @@ import ( "errors" "fmt" "os" + "strings" "sync" "github.com/GoogleCloudPlatform/container-diff/differs" @@ -17,37 +18,28 @@ var diffCmd = &cobra.Command{ Short: "Compare two images: [image1] [image2]", Long: `Compares two images using the specifed analyzers as indicated via flags (see documentation for available ones).`, Run: func(cmd *cobra.Command, args []string) { - if validArgs, err := validateArgs(args, checkDiffArgNum, checkArgType); !validArgs { + if err := validateArgs(args, checkDiffArgNum, checkArgType); err != nil { glog.Error(err.Error()) os.Exit(1) } - analyzeArgs := []string{} - allAnalyzers := getAllAnalyzers() - for _, name := range allAnalyzers { - if *analyzeFlagMap[name] == true { - analyzeArgs = append(analyzeArgs, name) - } - } - - // If no analyzers are specified, perform them all as the default - if len(analyzeArgs) == 0 { - analyzeArgs = allAnalyzers + if err := checkIfValidAnalyzer(types); err != nil { + glog.Error(err) + os.Exit(1) } - - if err := diffImages(args[0], args[1], analyzeArgs); err != nil { + if err := diffImages(args[0], args[1], strings.Split(types, ",")); err != nil { glog.Error(err) os.Exit(1) } }, } -func checkDiffArgNum(args []string) (bool, error) { +func checkDiffArgNum(args []string) error { var errMessage string if len(args) != 2 { errMessage = "'diff' requires two images as arguments: container diff [image1] [image2]" - return false, errors.New(errMessage) + return errors.New(errMessage) } - return true, nil + return nil } func diffImages(image1Arg, image2Arg string, diffArgs []string) error { diff --git a/cmd/diff_test.go b/cmd/diff_test.go index af1cdda7..bd7fd3b2 100644 --- a/cmd/diff_test.go +++ b/cmd/diff_test.go @@ -13,8 +13,8 @@ var diffArgNumTests = []testpair{ func TestDiffArgNum(t *testing.T) { for _, test := range diffArgNumTests { - valid, err := checkDiffArgNum(test.input) - if valid != test.expected_output { + err := checkDiffArgNum(test.input) + if (err == nil) != test.expected_output { if test.expected_output { t.Errorf("Got unexpected error: %s", err) } else { diff --git a/cmd/root.go b/cmd/root.go index 35928353..e031d9c4 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -1,40 +1,28 @@ package cmd import ( - "bytes" "context" - "errors" goflag "flag" "fmt" "os" "reflect" "sort" + "strings" + "github.com/GoogleCloudPlatform/container-diff/differs" "github.com/GoogleCloudPlatform/container-diff/utils" "github.com/docker/docker/client" "github.com/golang/glog" + "github.com/pkg/errors" "github.com/spf13/cobra" "github.com/spf13/pflag" ) var json bool -var eng bool var save bool -var apt bool -var node bool -var file bool -var history bool -var pip bool - -var analyzeFlagMap = map[string]*bool{ - "apt": &apt, - "node": &node, - "file": &file, - "history": &history, - "pip": &pip, -} +var types string -type validatefxn func(args []string) (bool, error) +type validatefxn func(args []string) error var RootCmd = &cobra.Command{ Use: "container-diff", @@ -90,24 +78,13 @@ func cleanupImage(image utils.Image) { } } -func getAllAnalyzers() []string { - allAnalyzers := []string{} - for name := range analyzeFlagMap { - allAnalyzers = append(allAnalyzers, name) - } - return allAnalyzers -} - -func validateArgs(args []string, validatefxns ...validatefxn) (bool, error) { +func validateArgs(args []string, validatefxns ...validatefxn) error { for _, validatefxn := range validatefxns { - valid, err := validatefxn(args) - if err != nil { - return false, err - } else if !valid { - return false, nil + if err := validatefxn(args); err != nil { + return err } } - return true, nil + return nil } func checkImage(arg string) bool { @@ -117,20 +94,27 @@ func checkImage(arg string) bool { return true } -func checkArgType(args []string) (bool, error) { - var buffer bytes.Buffer - valid := true +func checkArgType(args []string) error { for _, arg := range args { if !checkImage(arg) { - valid = false errMessage := fmt.Sprintf("Argument %s is not an image ID, URL, or tar\n", args[0]) - buffer.WriteString(errMessage) + glog.Errorf(errMessage) + return errors.New(errMessage) } } - if !valid { - return false, errors.New(buffer.String()) + return nil +} + +func checkIfValidAnalyzer(flagtypes string) error { + analyzers := strings.Split(flagtypes, ",") + for _, name := range analyzers { + if _, exists := differs.Analyzers[name]; !exists { + errMessage := fmt.Sprintf("Argument %s is not an image ID, URL, or tar\n", name) + glog.Errorf(errMessage) + return errors.New(errMessage) + } } - return true, nil + return nil } func remove(path string, dir bool) string { @@ -157,11 +141,7 @@ func init() { func addSharedFlags(cmd *cobra.Command) { cmd.Flags().BoolVarP(&json, "json", "j", false, "JSON Output defines if the diff should be returned in a human readable format (false) or a JSON (true).") - cmd.Flags().BoolVarP(&pip, "pip", "p", false, "Set this flag to use the pip differ.") - cmd.Flags().BoolVarP(&node, "node", "n", false, "Set this flag to use the node differ.") - cmd.Flags().BoolVarP(&apt, "apt", "a", false, "Set this flag to use the apt differ.") - cmd.Flags().BoolVarP(&file, "file", "f", false, "Set this flag to use the file differ.") - cmd.Flags().BoolVarP(&history, "history", "d", false, "Set this flag to use the dockerfile history differ.") + cmd.Flags().StringVarP(&types, "types", "t", "", "This flag sets the list of analyzer types to use. It expects a comma separated list of supported analyzers.") cmd.Flags().BoolVarP(&save, "save", "s", false, "Set this flag to save rather than remove the final image filesystems on exit.") cmd.Flags().BoolVarP(&utils.SortSize, "order", "o", false, "Set this flag to sort any file/package results by descending size. Otherwise, they will be sorted by name.") } diff --git a/cmd/root_test.go b/cmd/root_test.go index 0a5ae772..0821e1e9 100644 --- a/cmd/root_test.go +++ b/cmd/root_test.go @@ -19,8 +19,8 @@ var argTypeTests = []testpair{ func TestArgType(t *testing.T) { for _, test := range argTypeTests { - valid, err := checkArgType(test.input) - if valid != test.expected_output { + err := checkArgType(test.input) + if (err == nil) != test.expected_output { if test.expected_output { t.Errorf("Got unexpected error: %s", err) } else { diff --git a/differs/differs.go b/differs/differs.go index 976b12b4..65a96896 100644 --- a/differs/differs.go +++ b/differs/differs.go @@ -25,7 +25,7 @@ type Analyzer interface { Analyze(image utils.Image) (utils.Result, error) } -var analyzers = map[string]Analyzer{ +var Analyzers = map[string]Analyzer{ "history": HistoryAnalyzer{}, "file": FileAnalyzer{}, "apt": AptAnalyzer{}, @@ -84,7 +84,7 @@ func (req SingleRequest) GetAnalysis() (map[string]utils.Result, error) { func GetAnalyzers(analyzeNames []string) (analyzeFuncs []Analyzer, err error) { for _, name := range analyzeNames { - if a, exists := analyzers[name]; exists { + if a, exists := Analyzers[name]; exists { analyzeFuncs = append(analyzeFuncs, a) } else { glog.Errorf("Unknown analyzer/differ specified", name) diff --git a/tests/integration_test.go b/tests/integration_test.go index da33a8f3..82fb0e48 100644 --- a/tests/integration_test.go +++ b/tests/integration_test.go @@ -61,7 +61,7 @@ func TestDiffAndAnalysis(t *testing.T) { description string imageA string imageB string - differFlag string + differFlags []string subcommand string //TODO: Don't consume a json file @@ -72,7 +72,7 @@ func TestDiffAndAnalysis(t *testing.T) { subcommand: "diff", imageA: diffBase, imageB: diffModified, - differFlag: "-f", + differFlags: []string{"--types=file"}, expectedFile: "file_diff_expected.json", }, { @@ -80,7 +80,7 @@ func TestDiffAndAnalysis(t *testing.T) { subcommand: "diff", imageA: aptBase, imageB: aptModified, - differFlag: "-a", + differFlags: []string{"--types=apt"}, expectedFile: "apt_diff_expected.json", }, { @@ -88,7 +88,7 @@ func TestDiffAndAnalysis(t *testing.T) { subcommand: "diff", imageA: nodeBase, imageB: nodeModified, - differFlag: "-n", + differFlags: []string{"--types=node"}, expectedFile: "node_diff_order_expected.json", }, { @@ -96,7 +96,7 @@ func TestDiffAndAnalysis(t *testing.T) { subcommand: "diff", imageA: multiBase, imageB: multiModified, - differFlag: "-npa", + differFlags: []string{"--types=node,pip,apt"}, expectedFile: "multi_diff_expected.json", }, { @@ -104,7 +104,7 @@ func TestDiffAndAnalysis(t *testing.T) { subcommand: "diff", imageA: diffBase, imageB: diffModified, - differFlag: "-d", + differFlags: []string{"--types=history"}, expectedFile: "hist_diff_expected.json", }, { @@ -112,35 +112,35 @@ func TestDiffAndAnalysis(t *testing.T) { subcommand: "diff", imageA: aptBase, imageB: aptModified, - differFlag: "-ao", + differFlags: []string{"--types=apt", "-o"}, expectedFile: "apt_sorted_diff_expected.json", }, { description: "apt analysis", subcommand: "analyze", imageA: aptModified, - differFlag: "-a", + differFlags: []string{"--types=apt"}, expectedFile: "apt_analysis_expected.json", }, { description: "file sorted analysis", subcommand: "analyze", imageA: diffModified, - differFlag: "-fo", + differFlags: []string{"--types=file", "-o"}, expectedFile: "file_sorted_analysis_expected.json", }, { description: "pip analysis", subcommand: "analyze", imageA: pipModified, - differFlag: "-p", + differFlags: []string{"--types=pip"}, expectedFile: "pip_analysis_expected.json", }, { description: "node analysis", subcommand: "analyze", imageA: nodeModified, - differFlag: "-n", + differFlags: []string{"--types=node"}, expectedFile: "node_analysis_expected.json", }, } @@ -150,7 +150,7 @@ func TestDiffAndAnalysis(t *testing.T) { if test.imageB != "" { args = append(args, test.imageB) } - args = append(args, test.differFlag) + args = append(args, test.differFlags...) args = append(args, "-j") actual, err := runner.Run(args...) if err != nil {