From ed575659100531c0d9c8adb3d221a9c27d826979 Mon Sep 17 00:00:00 2001
From: Alex Goodman <wagoodman@gmail.com>
Date: Tue, 19 Nov 2019 09:59:57 -0800
Subject: [PATCH 1/3] fix nil log dest; clean tar paths
---
cmd/root.go | 2 +-
dive/filetree/cache.go | 93 --------------
dive/filetree/comparer.go | 175 ++++++++++++++++++++++++++
dive/filetree/efficiency.go | 7 +-
dive/filetree/file_tree.go | 40 +++---
dive/filetree/file_tree_test.go | 32 +++--
dive/filetree/path_error.go | 39 ++++++
dive/image/docker/image_archive.go | 7 +-
runtime/run.go | 13 +-
runtime/ui/app.go | 6 +-
runtime/ui/controller.go | 11 +-
runtime/ui/view/filetree.go | 2 +-
runtime/ui/viewmodel/filetree.go | 6 +-
runtime/ui/viewmodel/filetree_test.go | 13 +-
14 files changed, 304 insertions(+), 142 deletions(-)
delete mode 100644 dive/filetree/cache.go
create mode 100644 dive/filetree/comparer.go
create mode 100644 dive/filetree/path_error.go
diff --git a/cmd/root.go b/cmd/root.go
index c58e46e..680632c 100644
--- a/cmd/root.go
+++ b/cmd/root.go
@@ -121,6 +121,7 @@ func initLogging() {
if viper.GetBool("log.enabled") {
logFileObj, err = os.OpenFile(viper.GetString("log.path"), os.O_WRONLY|os.O_APPEND|os.O_CREATE, 0644)
+ log.SetOutput(logFileObj)
} else {
log.SetOutput(ioutil.Discard)
}
@@ -139,7 +140,6 @@ func initLogging() {
}
log.SetLevel(level)
- log.SetOutput(logFileObj)
log.Debug("Starting Dive...")
}
diff --git a/dive/filetree/cache.go b/dive/filetree/cache.go
deleted file mode 100644
index a7f0fc4..0000000
--- a/dive/filetree/cache.go
+++ /dev/null
@@ -1,93 +0,0 @@
-package filetree
-
-import (
- "github.com/sirupsen/logrus"
-)
-
-type TreeCacheKey struct {
- bottomTreeStart, bottomTreeStop, topTreeStart, topTreeStop int
-}
-
-type TreeCache struct {
- refTrees []*FileTree
- cache map[TreeCacheKey]*FileTree
-}
-
-func (cache *TreeCache) Get(bottomTreeStart, bottomTreeStop, topTreeStart, topTreeStop int) (*FileTree, error) {
- key := TreeCacheKey{bottomTreeStart, bottomTreeStop, topTreeStart, topTreeStop}
- if value, exists := cache.cache[key]; exists {
- return value, nil
- }
-
- value, err := cache.buildTree(key)
- if err != nil {
- return nil, err
- }
- cache.cache[key] = value
- return value, nil
-}
-
-func (cache *TreeCache) buildTree(key TreeCacheKey) (*FileTree, error) {
- newTree, err := StackTreeRange(cache.refTrees, key.bottomTreeStart, key.bottomTreeStop)
- if err != nil {
- return nil, err
- }
- for idx := key.topTreeStart; idx <= key.topTreeStop; idx++ {
- err := newTree.CompareAndMark(cache.refTrees[idx])
- if err != nil {
- logrus.Errorf("unable to build tree: %+v", err)
- return nil, err
- }
- }
- return newTree, nil
-}
-
-func (cache *TreeCache) Build() error {
- var bottomTreeStart, bottomTreeStop, topTreeStart, topTreeStop int
-
- // case 1: layer compare (top tree SIZE is fixed (BUT floats forward), Bottom tree SIZE changes)
- for selectIdx := 0; selectIdx < len(cache.refTrees); selectIdx++ {
- bottomTreeStart = 0
- topTreeStop = selectIdx
-
- if selectIdx == 0 {
- bottomTreeStop = selectIdx
- topTreeStart = selectIdx
- } else {
- bottomTreeStop = selectIdx - 1
- topTreeStart = selectIdx
- }
-
- _, err := cache.Get(bottomTreeStart, bottomTreeStop, topTreeStart, topTreeStop)
- if err != nil {
- return err
- }
- }
-
- // case 2: aggregated compare (bottom tree is ENTIRELY fixed, top tree SIZE changes)
- for selectIdx := 0; selectIdx < len(cache.refTrees); selectIdx++ {
- bottomTreeStart = 0
- topTreeStop = selectIdx
- if selectIdx == 0 {
- bottomTreeStop = selectIdx
- topTreeStart = selectIdx
- } else {
- bottomTreeStop = 0
- topTreeStart = 1
- }
-
- _, err := cache.Get(bottomTreeStart, bottomTreeStop, topTreeStart, topTreeStop)
- if err != nil {
- return err
- }
- }
- return nil
-}
-
-func NewFileTreeCache(refTrees []*FileTree) TreeCache {
-
- return TreeCache{
- refTrees: refTrees,
- cache: make(map[TreeCacheKey]*FileTree),
- }
-}
diff --git a/dive/filetree/comparer.go b/dive/filetree/comparer.go
new file mode 100644
index 0000000..8780da3
--- /dev/null
+++ b/dive/filetree/comparer.go
@@ -0,0 +1,175 @@
+package filetree
+
+import (
+ "fmt"
+ "github.com/sirupsen/logrus"
+)
+
+type TreeIndexKey struct {
+ bottomTreeStart, bottomTreeStop, topTreeStart, topTreeStop int
+}
+
+func NewTreeIndexKey(bottomTreeStart, bottomTreeStop, topTreeStart, topTreeStop int) TreeIndexKey {
+ return TreeIndexKey{
+ bottomTreeStart: bottomTreeStart,
+ bottomTreeStop: bottomTreeStop,
+ topTreeStart: topTreeStart,
+ topTreeStop: topTreeStop,
+ }
+}
+
+func (index TreeIndexKey) String() string {
+ if index.bottomTreeStart == index.bottomTreeStop && index.topTreeStart == index.topTreeStop {
+ return fmt.Sprintf("Index(%d:%d)", index.bottomTreeStart, index.topTreeStart)
+ } else if index.bottomTreeStart == index.bottomTreeStop {
+ return fmt.Sprintf("Index(%d:%d-%d)", index.bottomTreeStart, index.topTreeStart, index.topTreeStop)
+ } else if index.topTreeStart == index.topTreeStop {
+ return fmt.Sprintf("Index(%d-%d:%d)", index.bottomTreeStart, index.bottomTreeStop, index.topTreeStart)
+ }
+ return fmt.Sprintf("Index(%d-%d:%d-%d)", index.bottomTreeStart, index.bottomTreeStop, index.topTreeStart, index.topTreeStop)
+}
+
+type Comparer struct {
+ refTrees []*FileTree
+ trees map[TreeIndexKey]*FileTree
+ pathErrors map[TreeIndexKey][]PathError
+}
+
+func NewComparer(refTrees []*FileTree) Comparer {
+ return Comparer{
+ refTrees: refTrees,
+ trees: make(map[TreeIndexKey]*FileTree),
+ pathErrors: make(map[TreeIndexKey][]PathError),
+ }
+}
+
+func (cmp *Comparer) GetPathErrors(key TreeIndexKey) ([]PathError, error) {
+ _, pathErrors, err := cmp.get(key)
+ if err != nil {
+ return nil, err
+ }
+ return pathErrors, nil
+}
+
+func (cmp *Comparer) GetTree(key TreeIndexKey) (*FileTree, error) {
+ //func (cmp *Comparer) GetTree(bottomTreeStart, bottomTreeStop, topTreeStart, topTreeStop int) (*FileTree, []PathError, error) {
+ //key := TreeIndexKey{bottomTreeStart, bottomTreeStop, topTreeStart, topTreeStop}
+
+ if value, exists := cmp.trees[key]; exists {
+ return value, nil
+ }
+
+ value, pathErrors, err := cmp.get(key)
+ if err != nil {
+ return nil, err
+ }
+ cmp.trees[key] = value
+ cmp.pathErrors[key] = pathErrors
+ return value, nil
+}
+
+func (cmp *Comparer) get(key TreeIndexKey) (*FileTree, []PathError, error) {
+ newTree, pathErrors, err := StackTreeRange(cmp.refTrees, key.bottomTreeStart, key.bottomTreeStop)
+ if err != nil {
+ return nil, nil, err
+ }
+ for idx := key.topTreeStart; idx <= key.topTreeStop; idx++ {
+ markPathErrors, err := newTree.CompareAndMark(cmp.refTrees[idx])
+ pathErrors = append(pathErrors, markPathErrors...)
+ if err != nil {
+ logrus.Errorf("error while building tree: %+v", err)
+ return nil, nil, err
+ }
+ }
+ return newTree, pathErrors, nil
+}
+
+// case 1: layer compare (top tree SIZE is fixed (BUT floats forward), Bottom tree SIZE changes)
+func (cmp *Comparer) NaturalIndexes() <-chan TreeIndexKey {
+ indexes := make(chan TreeIndexKey)
+
+ go func() {
+ defer close(indexes)
+
+ var bottomTreeStart, bottomTreeStop, topTreeStart, topTreeStop int
+
+ for selectIdx := 0; selectIdx < len(cmp.refTrees); selectIdx++ {
+ bottomTreeStart = 0
+ topTreeStop = selectIdx
+
+ if selectIdx == 0 {
+ bottomTreeStop = selectIdx
+ topTreeStart = selectIdx
+ } else {
+ bottomTreeStop = selectIdx - 1
+ topTreeStart = selectIdx
+ }
+
+ indexes <- TreeIndexKey{
+ bottomTreeStart: bottomTreeStart,
+ bottomTreeStop: bottomTreeStop,
+ topTreeStart: topTreeStart,
+ topTreeStop: topTreeStop,
+ }
+ }
+ }()
+ return indexes
+
+}
+
+// case 2: aggregated compare (bottom tree is ENTIRELY fixed, top tree SIZE changes)
+func (cmp *Comparer) AggregatedIndexes() <-chan TreeIndexKey {
+ indexes := make(chan TreeIndexKey)
+
+ go func() {
+ defer close(indexes)
+
+ var bottomTreeStart, bottomTreeStop, topTreeStart, topTreeStop int
+
+ for selectIdx := 0; selectIdx < len(cmp.refTrees); selectIdx++ {
+ bottomTreeStart = 0
+ topTreeStop = selectIdx
+ if selectIdx == 0 {
+ bottomTreeStop = selectIdx
+ topTreeStart = selectIdx
+ } else {
+ bottomTreeStop = 0
+ topTreeStart = 1
+ }
+
+ indexes <- TreeIndexKey{
+ bottomTreeStart: bottomTreeStart,
+ bottomTreeStop: bottomTreeStop,
+ topTreeStart: topTreeStart,
+ topTreeStop: topTreeStop,
+ }
+ }
+ }()
+ return indexes
+
+}
+
+func (cmp *Comparer) BuildCache() (errors []error) {
+ for index := range cmp.NaturalIndexes() {
+ pathError, _ := cmp.GetPathErrors(index)
+ if pathError != nil {
+ for _, path := range pathError {
+ errors = append(errors, fmt.Errorf("path error at layer index %s: %s", index, path))
+ }
+ }
+ _, err := cmp.GetTree(index)
+ if err != nil {
+ errors = append(errors, err)
+ return errors
+ }
+ }
+
+ for index := range cmp.AggregatedIndexes() {
+ _, err := cmp.GetTree(index)
+ if err != nil {
+ errors = append(errors, err)
+ return errors
+ }
+ }
+ return errors
+}
diff --git a/dive/filetree/efficiency.go b/dive/filetree/efficiency.go
index 24ad3b5..cde56cb 100644
--- a/dive/filetree/efficiency.go
+++ b/dive/filetree/efficiency.go
@@ -62,7 +62,12 @@ func Efficiency(trees []*FileTree) (float64, EfficiencySlice) {
sizeBytes += curNode.Data.FileInfo.Size
return nil
}
- stackedTree, err := StackTreeRange(trees, 0, currentTree-1)
+ stackedTree, failedPaths, err := StackTreeRange(trees, 0, currentTree-1)
+ if failedPaths != nil {
+ for _, path := range failedPaths {
+ logrus.Errorf(path.String())
+ }
+ }
if err != nil {
logrus.Errorf("unable to stack tree range: %+v", err)
return err
diff --git a/dive/filetree/file_tree.go b/dive/filetree/file_tree.go
index 11311fc..f818559 100644
--- a/dive/filetree/file_tree.go
+++ b/dive/filetree/file_tree.go
@@ -204,22 +204,23 @@ func (tree *FileTree) VisitDepthParentFirst(visitor Visitor, evaluator VisitEval
}
// Stack takes two trees and combines them together. This is done by "stacking" the given tree on top of the owning tree.
-func (tree *FileTree) Stack(upper *FileTree) error {
+func (tree *FileTree) Stack(upper *FileTree) (failed []PathError, stackErr error) {
graft := func(node *FileNode) error {
if node.IsWhiteout() {
err := tree.RemovePath(node.Path())
if err != nil {
- return fmt.Errorf("cannot remove node %s: %v", node.Path(), err.Error())
+ failed = append(failed, NewPathError(node.Path(), ActionAdd, err))
}
} else {
- newNode, _, err := tree.AddPath(node.Path(), node.Data.FileInfo)
+ _, _, err := tree.AddPath(node.Path(), node.Data.FileInfo)
if err != nil {
- return fmt.Errorf("cannot add node %s: %v", newNode.Path(), err.Error())
+ failed = append(failed, NewPathError(node.Path(), ActionRemove, err))
}
}
return nil
}
- return upper.VisitDepthChildFirst(graft, nil)
+ stackErr = upper.VisitDepthChildFirst(graft, nil)
+ return failed, stackErr
}
// GetNode fetches a single node when given a slash-delimited string from root ('/') to the desired node (e.g. '/a/node/path')
@@ -293,17 +294,18 @@ type compareMark struct {
}
// CompareAndMark marks the FileNodes in the owning (lower) tree with DiffType annotations when compared to the given (upper) tree.
-func (tree *FileTree) CompareAndMark(upper *FileTree) error {
+func (tree *FileTree) CompareAndMark(upper *FileTree) ([]PathError, error) {
// always compare relative to the original, unaltered tree.
originalTree := tree
modifications := make([]compareMark, 0)
+ failed := make([]PathError, 0)
graft := func(upperNode *FileNode) error {
if upperNode.IsWhiteout() {
err := tree.markRemoved(upperNode.Path())
if err != nil {
- return fmt.Errorf("cannot remove upperNode %s: %v", upperNode.Path(), err.Error())
+ failed = append(failed, NewPathError(upperNode.Path(), ActionRemove, err))
}
return nil
}
@@ -315,7 +317,8 @@ func (tree *FileTree) CompareAndMark(upper *FileTree) error {
if originalLowerNode == nil {
_, newNodes, err := tree.AddPath(upperNode.Path(), upperNode.Data.FileInfo)
if err != nil {
- return fmt.Errorf("cannot add new upperNode %s: %v", upperNode.Path(), err.Error())
+ failed = append(failed, NewPathError(upperNode.Path(), ActionAdd, err))
+ return nil
}
for idx := len(newNodes) - 1; idx >= 0; idx-- {
newNode := newNodes[idx]
@@ -334,7 +337,7 @@ func (tree *FileTree) CompareAndMark(upper *FileTree) error {
// we must visit from the leaves upwards to ensure that diff types can be derived from and assigned to children
err := upper.VisitDepthChildFirst(graft, nil)
if err != nil {
- return err
+ return failed, err
}
// take note of the comparison results on each note in the owning tree.
@@ -342,19 +345,19 @@ func (tree *FileTree) CompareAndMark(upper *FileTree) error {
if pair.final > 0 {
err = pair.lowerNode.AssignDiffType(pair.final)
if err != nil {
- return err
+ return failed, err
}
} else if pair.lowerNode.Data.DiffType == Unmodified {
err = pair.lowerNode.deriveDiffType(pair.tentative)
if err != nil {
- return err
+ return failed, err
}
}
// persist the upper's payload on the owning tree
pair.lowerNode.Data.FileInfo = *pair.upperNode.Data.FileInfo.Copy()
}
- return nil
+ return failed, nil
}
// markRemoved annotates the FileNode at the given path as Removed.
@@ -367,15 +370,18 @@ func (tree *FileTree) markRemoved(path string) error {
}
// StackTreeRange combines an array of trees into a single tree
-func StackTreeRange(trees []*FileTree, start, stop int) (*FileTree, error) {
-
+func StackTreeRange(trees []*FileTree, start, stop int) (*FileTree, []PathError, error) {
+ errors := make([]PathError, 0)
tree := trees[0].Copy()
for idx := start; idx <= stop; idx++ {
- err := tree.Stack(trees[idx])
+ failedPaths, err := tree.Stack(trees[idx])
+ if len(failedPaths) > 0 {
+ errors = append(errors, failedPaths...)
+ }
if err != nil {
logrus.Errorf("could not stack tree range: %v", err)
- return nil, err
+ return nil, nil, err
}
}
- return tree, nil
+ return tree, errors, nil
}
diff --git a/dive/filetree/file_tree_test.go b/dive/filetree/file_tree_test.go
index d9f9677..15b95e5 100644
--- a/dive/filetree/file_tree_test.go
+++ b/dive/filetree/file_tree_test.go
@@ -307,12 +307,16 @@ func TestStack(t *testing.T) {
t.Errorf("expected no node on whiteout file add, but got %v", node)
}
- err = tree1.Stack(tree2)
+ failedPaths, err := tree1.Stack(tree2)
if err != nil {
t.Errorf("Could not stack refTrees: %v", err)
}
+ if len(failedPaths) > 0 {
+ t.Errorf("expected no filepath errors, got %d", len(failedPaths))
+ }
+
expected :=
`├── etc
│ └── nginx
@@ -415,10 +419,13 @@ func TestCompareWithNoChanges(t *testing.T) {
t.Errorf("could not setup test: %v", err)
}
}
- err := lowerTree.CompareAndMark(upperTree)
+ failedPaths, err := lowerTree.CompareAndMark(upperTree)
if err != nil {
t.Errorf("could not setup test: %v", err)
}
+ if len(failedPaths) > 0 {
+ t.Errorf("expected no filepath errors, got %d", len(failedPaths))
+ }
asserter := func(n *FileNode) error {
if n.Path() == "/" {
return nil
@@ -463,10 +470,13 @@ func TestCompareWithAdds(t *testing.T) {
}
failedAssertions := []error{}
- err := lowerTree.CompareAndMark(upperTree)
+ failedPaths, err := lowerTree.CompareAndMark(upperTree)
if err != nil {
t.Errorf("Expected tree compare to have no errors, got: %v", err)
}
+ if len(failedPaths) > 0 {
+ t.Errorf("expected no filepath errors, got %d", len(failedPaths))
+ }
asserter := func(n *FileNode) error {
p := n.Path()
@@ -577,11 +587,13 @@ func TestCompareWithChanges(t *testing.T) {
changedPaths = append(changedPaths, chownPath)
- err = lowerTree.CompareAndMark(upperTree)
+ failedPaths, err := lowerTree.CompareAndMark(upperTree)
if err != nil {
t.Errorf("unable to compare and mark: %+v", err)
}
-
+ if len(failedPaths) > 0 {
+ t.Errorf("expected no filepath errors, got %d", len(failedPaths))
+ }
failedAssertions := []error{}
asserter := func(n *FileNode) error {
p := n.Path()
@@ -642,10 +654,13 @@ func TestCompareWithRemoves(t *testing.T) {
}
}
- err := lowerTree.CompareAndMark(upperTree)
+ failedPaths, err := lowerTree.CompareAndMark(upperTree)
if err != nil {
t.Errorf("could not setup test: %v", err)
}
+ if len(failedPaths) > 0 {
+ t.Errorf("expected no filepath errors, got %d", len(failedPaths))
+ }
failedAssertions := []error{}
asserter := func(n *FileNode) error {
p := n.Path()
@@ -745,7 +760,10 @@ func TestStackRange(t *testing.T) {
}
}
trees := []*FileTree{lowerTree, upperTree, tree}
- _, err = StackTreeRange(trees, 0, 2)
+ _, failedPaths, err := StackTreeRange(trees, 0, 2)
+ if len(failedPaths) > 0 {
+ t.Errorf("expected no filepath errors, got %d", len(failedPaths))
+ }
if err != nil {
t.Fatal(err)
}
diff --git a/dive/filetree/path_error.go b/dive/filetree/path_error.go
new file mode 100644
index 0000000..32f8772
--- /dev/null
+++ b/dive/filetree/path_error.go
@@ -0,0 +1,39 @@
+package filetree
+
+import "fmt"
+
+const (
+ ActionAdd FileAction = iota
+ ActionRemove
+)
+
+type FileAction int
+
+func (fa FileAction) String() string {
+ switch fa {
+ case ActionAdd:
+ return "add"
+ case ActionRemove:
+ return "remove"
+ default:
+ return "<unknown file action>"
+ }
+}
+
+type PathError struct {
+ Path string
+ Action FileAction
+ Err error
+}
+
+func NewPathError(path string, action FileAction, err error) PathError {
+ return PathError{
+ Path: path,
+ Action: action,
+ Err: err,
+ }
+}
+
+func (pe PathError) String() string {
+ return fmt.Sprintf("unable to %s '%s': %+v", pe.Action.String(), pe.Path, pe.Err)
+}
diff --git a/dive/image/docker/image_archive.go b/dive/image/docker/image_archive.go
index 7b2f806..7e91172 100644
--- a/dive/image/docker/image_archive.go
+++ b/dive/image/docker/image_archive.go
@@ -8,6 +8,7 @@ import (
"io"
"io/ioutil"
"os"
+ "path"
"strings"
)
@@ -119,7 +120,11 @@ func getFileList(tarReader *tar.Reader) ([]filetree.FileInfo, error) {
return nil, err
}
- name := header.Name
+ // always ensure relative path notations are not parsed as part of the filename
+ name := path.Clean(header.Name)
+ if name == "." {
+ continue
+ }
switch header.Typeflag {
case tar.TypeXGlobalHeader:
diff --git a/runtime/run.go b/runtime/run.go
index 0c8d08a..9998ed6 100644
--- a/runtime/run.go
+++ b/runtime/run.go
@@ -87,10 +87,13 @@ func run(enableUi bool, options Options, imageResolver image.Resolver, events ev
} else {
events.message(utils.TitleFormat("Building cache..."))
- cache := filetree.NewFileTreeCache(analysis.RefTrees)
- err := cache.Build()
- if err != nil {
- events.exitWithErrorMessage("cannot build cache tree", err)
+ treeStack := filetree.NewComparer(analysis.RefTrees)
+ errors := treeStack.BuildCache()
+ if errors != nil {
+ for _, err := range errors {
+ events.message(" " + err.Error())
+ }
+ events.exitWithError(fmt.Errorf("file tree has path errors"))
return
}
@@ -102,7 +105,7 @@ func run(enableUi bool, options Options, imageResolver image.Resolver, events ev
// enough sleep will prevent this behavior (todo: remove this hack)
time.Sleep(100 * time.Millisecond)
- err = ui.Run(analysis, cache)
+ err = ui.Run(analysis, treeStack)
if err != nil {
events.exitWithError(err)
return
diff --git a/runtime/ui/app.go b/runtime/ui/app.go
index ec7f5ad..0d77116 100644
--- a/runtime/ui/app.go
+++ b/runtime/ui/app.go
@@ -24,7 +24,7 @@ var (
appSingleton *app
)
-func newApp(gui *gocui.Gui, analysis *image.AnalysisResult, cache filetree.TreeCache) (*app, error) {
+func newApp(gui *gocui.Gui, analysis *image.AnalysisResult, cache filetree.Comparer) (*app, error) {
var err error
once.Do(func() {
var theControls *Controller
@@ -118,7 +118,7 @@ func (a *app) quit() error {
}
// Run is the UI entrypoint.
-func Run(analysis *image.AnalysisResult, cache filetree.TreeCache) error {
+func Run(analysis *image.AnalysisResult, treeStack filetree.Comparer) error {
var err error
g, err := gocui.NewGui(gocui.OutputNormal)
@@ -127,7 +127,7 @@ func Run(analysis *image.AnalysisResult, cache filetree.TreeCache) error {
}
defer g.Close()
- _, err = newApp(g, analysis, cache)
+ _, err = newApp(g, analysis, treeStack)
if err != nil {
return err
}
diff --git a/runtime/ui/controller.go b/runtime/ui/controller.go
index 0eef2c2..4a2fb70 100644
--- a/runtime/ui/controller.go
+++ b/runtime/ui/controller.go
@@ -20,7 +20,7 @@ type Controller struct {
lookup map[string]view.Renderer
}
-func NewCollection(g *gocui.Gui, analysis *image.AnalysisResult, cache filetree.TreeCache) (*Controller, error) {
+func NewCollection(g *gocui.Gui, analysis *image.AnalysisResult, cache filetree.Comparer) (*Controller, error) {
var err error
controller := &Controller{
@@ -34,10 +34,11 @@ func NewCollection(g *gocui.Gui, analysis *image.AnalysisResult, cache filetree.
}
controller.lookup[controller.Layer.Name()] = controller.Layer
- treeStack, err := filetree.StackTreeRange(analysis.RefTrees, 0, 0)
- if err != nil {
- return nil, err
- }
+ //treeStack, err := filetree.StackTreeRange(analysis.RefTrees, 0, 0)
+ //if err != nil {
+ // return nil, err
+ //}
+ treeStack := analysis.RefTrees[0]
controller.Tree, err = view.NewFileTreeView("filetree", g, treeStack, analysis.RefTrees, cache)
if err != nil {
return nil, err
diff --git a/runtime/ui/view/filetree.go b/runtime/ui/view/filetree.go
index 3185be4..d460892 100644
--- a/runtime/ui/view/filetree.go
+++ b/runtime/ui/view/filetree.go
@@ -41,7 +41,7 @@ type FileTree struct {
}
// NewFileTreeView creates a new view object attached the the global [gocui] screen object.
-func NewFileTreeView(name string, gui *gocui.Gui, tree *filetree.FileTree, refTrees []*filetree.FileTree, cache filetree.TreeCache) (controller *FileTree, err error) {
+func NewFileTreeView(name string, gui *gocui.Gui, tree *filetree.FileTree, refTrees []*filetree.FileTree, cache filetree.Comparer) (controller *FileTree, err error) {
controller = new(FileTree)
controller.listeners = make([]ViewOptionChangeListener, 0)
diff --git a/runtime/ui/viewmodel/filetree.go b/runtime/ui/viewmodel/filetree.go
index 75cb469..fe7f00d 100644
--- a/runtime/ui/viewmodel/filetree.go
+++ b/runtime/ui/viewmodel/filetree.go
@@ -19,7 +19,7 @@ type FileTree struct {
ModelTree *filetree.FileTree
ViewTree *filetree.FileTree
RefTrees []*filetree.FileTree
- cache filetree.TreeCache
+ cache filetree.Comparer
CollapseAll bool
ShowAttributes bool
@@ -35,7 +35,7 @@ type FileTree struct {
}
// NewFileTreeViewModel creates a new view object attached the the global [gocui] screen object.
-func NewFileTreeViewModel(tree *filetree.FileTree, refTrees []*filetree.FileTree, cache filetree.TreeCache) (treeViewModel *FileTree, err error) {
+func NewFileTreeViewModel(tree *filetree.FileTree, refTrees []*filetree.FileTree, cache filetree.Comparer) (treeViewModel *FileTree, err error) {
treeViewModel = new(FileTree)
// populate main fields
@@ -101,7 +101,7 @@ func (vm *FileTree) SetTreeByLayer(bottomTreeStart, bottomTreeStop, topTreeStart
if topTreeStop > len(vm.RefTrees)-1 {
return fmt.Errorf("invalid layer index given: %d of %d", topTreeStop, len(vm.RefTrees)-1)
}
- newTree, err := vm.cache.Get(bottomTreeStart, bottomTreeStop, topTreeStart, topTreeStop)
+ newTree, err := vm.cache.GetTree(filetree.NewTreeIndexKey(bottomTreeStart, bottomTreeStop, topTreeStart, topTreeStop))
if err != nil {
logrus.Errorf("unable to fetch layer tree from cache: %+v", err)
return err
diff --git a/runtime/ui/viewmodel/filetree_test.go b/runtime/ui/viewmodel/filetree_test.go
index c90f0d7..4e1c0c0 100644
--- a/runtime/ui/viewmodel/filetree_test.go
+++ b/runtime/ui/viewmodel/filetree_test.go
@@ -76,15 +76,18 @@ func assertTestData(t *testing.T, actualBytes []byte) {
func initializeTestViewModel(t *testing.T) *FileTree {
result := docker.TestAnalysisFromArchive(t, "../../../.data/test-docker-image.tar")
- cache := filetree.NewFileTreeCache(result.RefTrees)
- err := cache.Build()
- if err != nil {
- t.Fatalf("%s: unable to build cache: %+v", t.Name(), err)
+ cache := filetree.NewComparer(result.RefTrees)
+ errors := cache.BuildCache()
+ if len(errors) > 0 {
+ t.Fatalf("%s: unable to build cache: %d errors", t.Name(), len(errors))
}
format.Selected = color.New(color.ReverseVideo, color.Bold).SprintFunc()
- treeStack, err := filetree.StackTreeRange(result.RefTrees, 0, 0)
+ treeStack, failedPaths, err := filetree.StackTreeRange(result.RefTrees, 0, 0)
+ if len(failedPaths) > 0 {
+ t.Errorf("expected no filepath errors, got %d", len(failedPaths))
+ }
if err != nil {
t.Fatalf("%s: unable to stack trees: %v", t.Name(), err)
}
From 17742e7b6cb39406bb875f41d3ae79212ec8859c Mon Sep 17 00:00:00 2001
From: Alex Goodman <wagoodman@gmail.com>
Date: Tue, 19 Nov 2019 10:13:26 -0800
Subject: [PATCH 2/3] add test for formatting/rejecting relative path in
filetree
---
dive/filetree/file_tree.go | 11 ++++++++---
dive/filetree/file_tree_test.go | 34 +++++++++++++++++++++++++++++++++
2 files changed, 42 insertions(+), 3 deletions(-)
diff --git a/dive/filetree/file_tree.go b/dive/filetree/file_tree.go
index f818559..4333e25 100644
--- a/dive/filetree/file_tree.go
+++ b/dive/filetree/file_tree.go
@@ -2,6 +2,7 @@ package filetree
import (
"fmt"
+ "path"
"sort"
"strings"
@@ -240,8 +241,12 @@ func (tree *FileTree) GetNode(path string) (*FileNode, error) {
}
// AddPath adds a new node to the tree with the given payload
-func (tree *FileTree) AddPath(path string, data FileInfo) (*FileNode, []*FileNode, error) {
- nodeNames := strings.Split(strings.Trim(path, "/"), "/")
+func (tree *FileTree) AddPath(filepath string, data FileInfo) (*FileNode, []*FileNode, error) {
+ filepath = path.Clean(filepath)
+ if filepath == "." {
+ return nil, nil, fmt.Errorf("cannot add relative path '%s'", filepath)
+ }
+ nodeNames := strings.Split(strings.Trim(filepath, "/"), "/")
node := tree.Root
addedNodes := make([]*FileNode, 0)
for idx, name := range nodeNames {
@@ -264,7 +269,7 @@ func (tree *FileTree) AddPath(path string, data FileInfo) (*FileNode, []*FileNod
if node == nil {
// the child could not be added
- return node, addedNodes, fmt.Errorf(fmt.Sprintf("could not add child node: '%s' (path:'%s')", name, path))
+ return node, addedNodes, fmt.Errorf(fmt.Sprintf("could not add child node: '%s' (path:'%s')", name, filepath))
}
}
diff --git a/dive/filetree/file_tree_test.go b/dive/filetree/file_tree_test.go
index 15b95e5..c1bc37c 100644
--- a/dive/filetree/file_tree_test.go
+++ b/dive/filetree/file_tree_test.go
@@ -125,6 +125,40 @@ func TestStringBetween(t *testing.T) {
}
+func TestRejectPurelyRelativePath(t *testing.T) {
+ tree := NewFileTree()
+ _, _, err := tree.AddPath("./etc/nginx/nginx.conf", FileInfo{})
+ if err != nil {
+ t.Errorf("could not setup test: %v", err)
+ }
+ _, _, err = tree.AddPath("./", FileInfo{})
+
+ if err == nil {
+ t.Errorf("expected to reject relative path, but did not")
+ }
+
+}
+
+func TestAddRelativePath(t *testing.T) {
+ tree := NewFileTree()
+ _, _, err := tree.AddPath("./etc/nginx/nginx.conf", FileInfo{})
+ if err != nil {
+ t.Errorf("could not setup test: %v", err)
+ }
+
+ expected :=
+ `└── etc
+ └── nginx
+ └── nginx.conf
+`
+ actual := tree.String(false)
+
+ if expected != actual {
+ t.Errorf("Expected tree string:\n--->%s<---\nGot:\n--->%s<---", expected, actual)
+ }
+
+}
+
func TestAddPath(t *testing.T) {
tree := NewFileTree()
_, _, err := tree.AddPath("/etc/nginx/nginx.conf", FileInfo{})
From 6b659d9ef47c6bf8af9c704b166de0e1438db5cc Mon Sep 17 00:00:00 2001
From: Alex Goodman <wagoodman@gmail.com>
Date: Wed, 20 Nov 2019 15:02:53 -0800
Subject: [PATCH 3/3] linting
---
dive/filetree/comparer.go | 2 +-
dive/filetree/efficiency.go | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/dive/filetree/comparer.go b/dive/filetree/comparer.go
index 8780da3..1df52a7 100644
--- a/dive/filetree/comparer.go
+++ b/dive/filetree/comparer.go
@@ -152,7 +152,7 @@ func (cmp *Comparer) AggregatedIndexes() <-chan TreeIndexKey {
func (cmp *Comparer) BuildCache() (errors []error) {
for index := range cmp.NaturalIndexes() {
pathError, _ := cmp.GetPathErrors(index)
- if pathError != nil {
+ if len(pathError) > 0 {
for _, path := range pathError {
errors = append(errors, fmt.Errorf("path error at layer index %s: %s", index, path))
}
diff --git a/dive/filetree/efficiency.go b/dive/filetree/efficiency.go
index cde56cb..cdb6b4f 100644
--- a/dive/filetree/efficiency.go
+++ b/dive/filetree/efficiency.go
@@ -63,7 +63,7 @@ func Efficiency(trees []*FileTree) (float64, EfficiencySlice) {
return nil
}
stackedTree, failedPaths, err := StackTreeRange(trees, 0, currentTree-1)
- if failedPaths != nil {
+ if len(failedPaths) > 0 {
for _, path := range failedPaths {
logrus.Errorf(path.String())
}