-
Notifications
You must be signed in to change notification settings - Fork 237
Single image analysis #20
Changes from 2 commits
ad78c06
6b359d2
414dd3a
9d4ef87
b453dd4
2d09bc5
f38e8a5
6c212b1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,16 +18,15 @@ import ( | |
|
||
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 save bool | ||
|
||
var diffFlagMap = map[string]*bool{ | ||
var analyzeFlagMap = map[string]*bool{ | ||
"apt": &apt, | ||
"node": &node, | ||
"file": &file, | ||
|
@@ -47,104 +46,168 @@ var RootCmd = &cobra.Command{ | |
|
||
utils.SetDockerEngine(eng) | ||
|
||
img1Arg := args[0] | ||
img2Arg := args[1] | ||
diffArgs := []string{} | ||
allDiffers := getAllDiffers() | ||
for _, name := range allDiffers { | ||
if *diffFlagMap[name] == true { | ||
diffArgs = append(diffArgs, name) | ||
analyzeArgs := []string{} | ||
allAnalyzers := getAllAnalyzers() | ||
for _, name := range allAnalyzers { | ||
if *analyzeFlagMap[name] == true { | ||
analyzeArgs = append(analyzeArgs, name) | ||
} | ||
} | ||
// If no differs are specified, perform all diffs as the default | ||
if len(diffArgs) == 0 { | ||
diffArgs = allDiffers | ||
|
||
// If no differs/analyzers are specified, perform them all as the default | ||
if len(analyzeArgs) == 0 { | ||
analyzeArgs = allAnalyzers | ||
} | ||
|
||
var wg sync.WaitGroup | ||
wg.Add(2) | ||
if len(args) == 1 { | ||
analyzeImage(args[0], analyzeArgs) | ||
} else { | ||
diffImages(args[0], args[1], analyzeArgs) | ||
} | ||
}, | ||
} | ||
|
||
glog.Infof("Starting diff on images %s and %s, using differs: %s", img1Arg, img2Arg, diffArgs) | ||
func diffImages(image1Arg, image2Arg string, diffArgs []string) { | ||
var wg sync.WaitGroup | ||
wg.Add(2) | ||
|
||
var image1, image2 utils.Image | ||
var err error | ||
go func() { | ||
defer wg.Done() | ||
image1, err = utils.ImagePrepper{img1Arg}.GetImage() | ||
if err != nil { | ||
glog.Error(err.Error()) | ||
os.Exit(1) | ||
} | ||
}() | ||
glog.Infof("Starting diff on images %s and %s, using differs: %s", image1Arg, image2Arg, diffArgs) | ||
|
||
go func() { | ||
defer wg.Done() | ||
image2, err = utils.ImagePrepper{img2Arg}.GetImage() | ||
if err != nil { | ||
glog.Error(err.Error()) | ||
os.Exit(1) | ||
} | ||
}() | ||
var image1, image2 utils.Image | ||
var err error | ||
go func() { | ||
defer wg.Done() | ||
image1, err = utils.ImagePrepper{image1Arg}.GetImage() | ||
if err != nil { | ||
glog.Error(err.Error()) | ||
os.Exit(1) | ||
} | ||
}() | ||
|
||
diffTypes, err := differs.GetDiffers(diffArgs) | ||
go func() { | ||
defer wg.Done() | ||
image2, err = utils.ImagePrepper{image2Arg}.GetImage() | ||
if err != nil { | ||
glog.Error(err.Error()) | ||
os.Exit(1) | ||
} | ||
wg.Wait() | ||
|
||
req := differs.DiffRequest{image1, image2, diffTypes} | ||
if diffs, err := req.GetDiff(); err == nil { | ||
// Outputs diff results in alphabetical order by differ name | ||
diffTypes := []string{} | ||
for name := range diffs { | ||
diffTypes = append(diffTypes, name) | ||
} | ||
sort.Strings(diffTypes) | ||
glog.Info("Retrieving diffs") | ||
diffResults := []utils.DiffResult{} | ||
for _, diffType := range diffTypes { | ||
diff := diffs[diffType] | ||
if json { | ||
diffResults = append(diffResults, diff.GetStruct()) | ||
} else { | ||
err = diff.OutputText(diffType) | ||
if err != nil { | ||
glog.Error(err) | ||
} | ||
} | ||
} | ||
}() | ||
|
||
diffTypes, err := differs.GetAnalyzers(diffArgs) | ||
if err != nil { | ||
glog.Error(err.Error()) | ||
os.Exit(1) | ||
} | ||
wg.Wait() | ||
|
||
req := differs.DiffRequest{image1, image2, diffTypes} | ||
if diffs, err := req.GetDiff(); err == nil { | ||
// Outputs diff results in alphabetical order by differ name | ||
sortedTypes := []string{} | ||
for name := range diffs { | ||
sortedTypes = append(sortedTypes, name) | ||
} | ||
sort.Strings(sortedTypes) | ||
glog.Info("Retrieving diffs") | ||
diffResults := []utils.DiffResult{} | ||
for _, diffType := range sortedTypes { | ||
diff := diffs[diffType] | ||
if json { | ||
err = utils.JSONify(diffResults) | ||
diffResults = append(diffResults, diff.GetStruct()) | ||
} else { | ||
err = diff.OutputText(diffType) | ||
if err != nil { | ||
glog.Error(err) | ||
} | ||
} | ||
fmt.Println() | ||
} | ||
if json { | ||
err = utils.JSONify(diffResults) | ||
if err != nil { | ||
glog.Error(err) | ||
} | ||
} | ||
fmt.Println() | ||
if !save { | ||
glog.Info("Removing image file system directories from system") | ||
if !save { | ||
errMsg := remove(image1.FSPath, true) | ||
errMsg += remove(image2.FSPath, true) | ||
if errMsg != "" { | ||
glog.Error(errMsg) | ||
} | ||
errMsg := remove(image1.FSPath, true) | ||
errMsg += remove(image2.FSPath, true) | ||
if errMsg != "" { | ||
glog.Error(errMsg) | ||
} | ||
} else { | ||
dir, _ := os.Getwd() | ||
glog.Infof("Images were saved at %s as %s and %s", dir, image1.FSPath, image2.FSPath) | ||
} | ||
} else { | ||
glog.Error(err.Error()) | ||
os.Exit(1) | ||
} | ||
} | ||
|
||
func analyzeImage(imageArg string, analyzerArgs []string) { | ||
image, err := utils.ImagePrepper{imageArg}.GetImage() | ||
if err != nil { | ||
glog.Error(err.Error()) | ||
os.Exit(1) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. not sure if this is new to this PR, but There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done |
||
} | ||
analyzeTypes, err := differs.GetAnalyzers(analyzerArgs) | ||
if err != nil { | ||
glog.Error(err.Error()) | ||
os.Exit(1) | ||
} | ||
|
||
req := differs.SingleRequest{image, analyzeTypes} | ||
if analyses, err := req.GetAnalysis(); err == nil { | ||
// Outputs analysis results in alphabetical order by differ name | ||
sortedTypes := []string{} | ||
for name := range analyses { | ||
sortedTypes = append(sortedTypes, name) | ||
} | ||
sort.Strings(sortedTypes) | ||
glog.Info("Retrieving diffs") | ||
analyzeResults := []utils.AnalyzeResult{} | ||
for _, analyzeType := range sortedTypes { | ||
analysis := analyses[analyzeType] | ||
if json { | ||
analyzeResults = append(analyzeResults, analysis.GetStruct()) | ||
} else { | ||
dir, _ := os.Getwd() | ||
glog.Infof("Images were saved at %s as %s and %s", dir, image1.FSPath, image2.FSPath) | ||
err = analysis.OutputText(analyzeType) | ||
if err != nil { | ||
glog.Error(err) | ||
} | ||
} | ||
} | ||
if json { | ||
err = utils.JSONify(analyzeResults) | ||
if err != nil { | ||
glog.Error(err) | ||
} | ||
} | ||
fmt.Println() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: use glog/the output buffer here? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh I actually don't even think this is necessary because there's a newline in the template, so I'll just get rid of it... |
||
if !save { | ||
glog.Info("Removing image file system directory from system") | ||
errMsg := remove(image.FSPath, true) | ||
if errMsg != "" { | ||
glog.Error(errMsg) | ||
} | ||
} else { | ||
glog.Error(err.Error()) | ||
os.Exit(1) | ||
dir, _ := os.Getwd() | ||
glog.Infof("Image was saved at %s as %s", dir, image.FSPath) | ||
} | ||
}, | ||
} else { | ||
glog.Error(err.Error()) | ||
os.Exit(1) | ||
} | ||
|
||
} | ||
|
||
func getAllDiffers() []string { | ||
allDiffers := []string{} | ||
for name := range diffFlagMap { | ||
allDiffers = append(allDiffers, name) | ||
func getAllAnalyzers() []string { | ||
allAnalyzers := []string{} | ||
for name := range analyzeFlagMap { | ||
allAnalyzers = append(allAnalyzers, name) | ||
} | ||
return allDiffers | ||
return allAnalyzers | ||
} | ||
|
||
func validateArgs(args []string) (bool, error) { | ||
|
@@ -165,11 +228,11 @@ func validateArgs(args []string) (bool, error) { | |
|
||
func checkArgNum(args []string) (bool, error) { | ||
var errMessage string | ||
if len(args) < 2 { | ||
errMessage = "Too few arguments. Should have two images as arguments: [IMAGE1] [IMAGE2]." | ||
if len(args) < 1 { | ||
errMessage = "Too few arguments. Should have one or two images as arguments." | ||
return false, errors.New(errMessage) | ||
} else if len(args) > 2 { | ||
errMessage = "Too many arguments. Should have two images as arguments: [IMAGE1] [IMAGE2]." | ||
errMessage = "Too many arguments. Should have at most two images as arguments." | ||
return false, errors.New(errMessage) | ||
} else { | ||
return true, nil | ||
|
@@ -186,15 +249,12 @@ func checkImage(arg string) bool { | |
func checkArgType(args []string) (bool, error) { | ||
var buffer bytes.Buffer | ||
valid := true | ||
if !checkImage(args[0]) { | ||
valid = false | ||
errMessage := fmt.Sprintf("Argument %s is not an image ID, URL, or tar\n", args[0]) | ||
buffer.WriteString(errMessage) | ||
} | ||
if !checkImage(args[1]) { | ||
valid = false | ||
errMessage := fmt.Sprintf("Argument %s is not an image ID, URL, or tar\n", args[1]) | ||
buffer.WriteString(errMessage) | ||
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) | ||
} | ||
} | ||
if !valid { | ||
return false, errors.New(buffer.String()) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It makes more sense to me to have analyze as a separate command (such that the logic wouldn't be in root.go) both for clarity and for example in case of future additional commands. If someone adds a command and pattern matches, they seem forced into expanding this if else structure. The primary logic already seems separated out so is there a reason not to pull it into it's own command file?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just thought only one command would be preferable for our tool... @nkubala @aaron-prindle do you have any thoughts as to whether we should split this up into a separate command?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I actually think this structure is fine. We should probably at least put a comment here saying semantically what each case means (
analyze
= one image,diff
= two), and then put something about it in the usage/--help
command. I do like the tool choosing which path to take based on how many images you provide it though, seems intuitive to me