Merge pull request #18 from cftorres/FinalFSFix

Fixed images sourced from tars and IDs to have final filesystem

Author: cftorres

differs/fileDiff.go

@@ -1,11 +1,9 @@
 package differs
 
 import (
-	"os"
 	"sort"
 
 	"github.com/GoogleCloudPlatform/container-diff/utils"
-	"github.com/golang/glog"
 )
 
 type FileDiffer struct {
@@ -23,21 +21,11 @@ func diffImageFiles(image1, image2 utils.Image) (utils.DirDiff, error) {
 
 	var diff utils.DirDiff
 
-	target1 := "j1.json"
-	err := utils.DirToJSON(img1, target1, true)
+	img1Dir, err := utils.GetDirectory(img1, true)
 	if err != nil {
 		return diff, err
 	}
-	target2 := "j2.json"
-	err = utils.DirToJSON(img2, target2, true)
-	if err != nil {
-		return diff, err
-	}
-	img1Dir, err := utils.GetDirectory(target1)
-	if err != nil {
-		return diff, err
-	}
-	img2Dir, err := utils.GetDirectory(target2)
+	img2Dir, err := utils.GetDirectory(img2, true)
 	if err != nil {
 		return diff, err
 	}
@@ -52,15 +40,7 @@ func diffImageFiles(image1, image2 utils.Image) (utils.DirDiff, error) {
 		Image2: image2.Source,
 		Adds:   adds,
 		Dels:   dels,
-	}
-
-	err = os.Remove(target1)
-	if err != nil {
-		glog.Error(err)
-	}
-	err = os.Remove(target2)
-	if err != nil {
-		glog.Error(err)
+		Mods:   []string{},
 	}
 	return diff, nil
 }

differs/pipDiff.go

@@ -21,9 +21,9 @@ func (d PipDiffer) Diff(image1, image2 utils.Image) (utils.DiffResult, error) {
 	return diff, err
 }
 
-func getPythonVersion(pathToLayer string) ([]string, error) {
+func getPythonVersion(pathToImage string) ([]string, error) {
 	matches := []string{}
-	libPath := filepath.Join(pathToLayer, "usr/local/lib")
+	libPath := filepath.Join(pathToImage, "usr/local/lib")
 	libContents, err := ioutil.ReadDir(libPath)
 	if err != nil {
 		return matches, err
@@ -59,9 +59,7 @@ func (d PipDiffer) getPackages(image utils.Image) (map[string]map[string]utils.P
 	pythonPaths := []string{}
 	if !reflect.DeepEqual(utils.ConfigSchema{}, image.Config) {
 		paths := getPythonPaths(image.Config.Config.Env)
-		for _, p := range paths {
-			pythonPaths = append(pythonPaths, p)
-		}
+		pythonPaths = append(pythonPaths, paths...)
 	}
 	pythonVersions, err := getPythonVersion(path)
 	if err != nil {
@@ -77,7 +75,7 @@ func (d PipDiffer) getPackages(image utils.Image) (map[string]map[string]utils.P
 	for _, pythonPath := range pythonPaths {
 		contents, err := ioutil.ReadDir(pythonPath)
 		if err != nil {
-			// python version folder doesn't have a site-packages folder
+			// python version folder doesn't have a site-packages folder or PYTHONPATH doesn't exist
 			continue
 		}
 

tests/file_diff_expected.json

@@ -8,7 +8,7 @@
       "Dels": [
         "/home/test"
       ],
-      "Mods": null
+      "Mods": []
     }
   }
 ]

tests/multi_diff_expected.json

@@ -101,21 +101,6 @@
         }
       },
       "InfoDiff": [
-        {
-          "Package": "wheel",
-          "Info1": [
-            {
-              "Version": "0.29.0",
-              "Size": "103509"
-            }
-          ],
-          "Info2": [
-            {
-              "Version": "0.29.0",
-              "Size": "137451"
-            }
-          ]
-        },
         {
           "Package": "mock",
           "Info1": [
@@ -145,6 +130,21 @@
               "Size": "1157078"
             }
           ]
+        },
+        {
+          "Package": "wheel",
+          "Info1": [
+            {
+              "Version": "0.29.0",
+              "Size": "103509"
+            }
+          ],
+          "Info2": [
+            {
+              "Version": "0.29.0",
+              "Size": "137451"
+            }
+          ]
         }
       ]
     }

utils/docker_utils.go

@@ -5,9 +5,13 @@ import (
 	"bytes"
 	"context"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"io"
+	"io/ioutil"
+	"os"
 	"os/exec"
+	"path/filepath"
 	"regexp"
 	"strings"
 	"syscall"
@@ -36,6 +40,7 @@ func ValidDockerVersion() (bool, error) {
 	}
 	return false, nil
 }
+
 func getImagePullResponse(image string, response []Event) (string, error) {
 	var imageDigest string
 	for _, event := range response {
@@ -303,3 +308,59 @@ func getImageConfig(image string) (container.Config, error) {
 	}
 	return config, nil
 }
+
+func getLayersFromManifest(manifestPath string) ([]string, error) {
+	type Manifest struct {
+		Layers []string
+	}
+
+	manifestJSON, err := ioutil.ReadFile(manifestPath)
+	if err != nil {
+		errMsg := fmt.Sprintf("Could not open manifest to get layer order: %s", err)
+		return []string{}, errors.New(errMsg)
+	}
+
+	var imageManifest []Manifest
+	err = json.Unmarshal(manifestJSON, &imageManifest)
+	if err != nil {
+		errMsg := fmt.Sprintf("Could not unmarshal manifest to get layer order: %s", err)
+		return []string{}, errors.New(errMsg)
+	}
+	return imageManifest[0].Layers, nil
+}
+
+func unpackDockerSave(tarPath string, target string) error {
+	if _, ok := os.Stat(target); ok != nil {
+		os.MkdirAll(target, 0777)
+	}
+
+	tempLayerDir := target + "-temp"
+	err := UnTar(tarPath, tempLayerDir)
+	if err != nil {
+		errMsg := fmt.Sprintf("Could not unpack saved Docker image %s: %s", tarPath, err)
+		return errors.New(errMsg)
+	}
+
+	manifest := filepath.Join(tempLayerDir, "manifest.json")
+	layers, err := getLayersFromManifest(manifest)
+	if err != nil {
+		return err
+	}
+
+	for _, layer := range layers {
+		layerTar := filepath.Join(tempLayerDir, layer)
+		if _, err := os.Stat(layerTar); err != nil {
+			glog.Infof("Did not unpack layer %s because no layer.tar found", layer)
+			continue
+		}
+		err = UnTar(layerTar, target)
+		if err != nil {
+			glog.Errorf("Could not unpack layer %s: %s", layer, err)
+		}
+	}
+	err = os.RemoveAll(tempLayerDir)
+	if err != nil {
+		glog.Errorf("Error deleting temp image layer filesystem: %s", err)
+	}
+	return nil
+}

utils/fs_utils.go

@@ -2,11 +2,11 @@ package utils
 
 import (
 	"bytes"
-	"encoding/json"
 	"fmt"
 	"io/ioutil"
 	"os"
 	"path/filepath"
+	"strings"
 
 	"github.com/golang/glog"
 )
@@ -22,18 +22,33 @@ func GetDirectorySize(path string) (int64, error) {
 	return size, err
 }
 
-func GetDirectory(dirpath string) (Directory, error) {
-	dirfile, err := ioutil.ReadFile(dirpath)
-	if err != nil {
-		return Directory{}, err
-	}
+// GetDirectoryContents converts the directory starting at the provided path into a Directory struct.
+func GetDirectory(path string, deep bool) (Directory, error) {
+	var directory Directory
+	directory.Root = path
+	var err error
+	if deep {
+		walkFn := func(currPath string, info os.FileInfo, err error) error {
+			newContent := strings.TrimPrefix(currPath, directory.Root)
+			if newContent != "" {
+				directory.Content = append(directory.Content, newContent)
+			}
+			return nil
+		}
 
-	var dir Directory
-	err = json.Unmarshal(dirfile, &dir)
-	if err != nil {
-		return Directory{}, err
+		err = filepath.Walk(path, walkFn)
+	} else {
+		contents, err := ioutil.ReadDir(path)
+		if err != nil {
+			return directory, err
+		}
+
+		for _, file := range contents {
+			fileName := "/" + file.Name()
+			directory.Content = append(directory.Content, fileName)
+		}
 	}
-	return dir, nil
+	return directory, err
 }
 
 // Checks for content differences between files of the same name from different directories
@@ -53,6 +68,22 @@ func GetModifiedEntries(d1, d2 Directory) []string {
 			glog.Errorf("Error checking directory entry %s: %s\n", f, err)
 			continue
 		}
+		f2stat, err := os.Stat(f2path)
+		if err != nil {
+			glog.Errorf("Error checking directory entry %s: %s\n", f, err)
+			continue
+		}
+
+		// If the directory entry in question is a tar, verify that the two have the same size
+		if isTar(f1path) {
+			if f1stat.Size() != f2stat.Size() {
+				modified = append(modified, f)
+			}
+			continue
+		}
+
+		// If the directory entry is not a tar and not a directory, then it's a file so make sure the file contents are the same
+		// Note: We skip over directory entries because to compare directories, we compare their contents
 		if !f1stat.IsDir() {
 			same, err := checkSameFile(f1path, f2path)
 			if err != nil {
@@ -83,12 +114,20 @@ type DirDiff struct {
 	Mods   []string
 }
 
-func compareDirEntries(d1, d2 Directory) DirDiff {
+// DiffDirectory takes the diff of two directories, assuming both are completely unpacked
+func DiffDirectory(d1, d2 Directory) (DirDiff, bool) {
 	adds := GetAddedEntries(d1, d2)
 	dels := GetDeletedEntries(d1, d2)
 	mods := GetModifiedEntries(d1, d2)
 
-	return DirDiff{d1.Root, d2.Root, adds, dels, mods}
+	var same bool
+	if len(adds) == 0 && len(dels) == 0 && len(mods) == 0 {
+		same = true
+	} else {
+		same = false
+	}
+
+	return DirDiff{d1.Root, d2.Root, adds, dels, mods}, same
 }
 
 func checkSameFile(f1name, f2name string) (bool, error) {
@@ -121,7 +160,3 @@ func checkSameFile(f1name, f2name string) (bool, error) {
 	}
 	return true, nil
 }
-
-func DiffDirectory(d1, d2 Directory) DirDiff {
-	return compareDirEntries(d1, d2)
-}

utils/fs_utils_test.go

@@ -68,25 +68,42 @@ func TestGetModifiedEntries(t *testing.T) {
 }
 
 func TestGetDirectory(t *testing.T) {
-	type dirtestpair struct {
-		input            string
-		expected_success bool
+	tests := []struct {
+		descrip  string
+		path     string
+		expected Directory
+		deep     bool
+	}{
+		{
+			descrip: "deep",
+			path:    "testTars/la-croix3-full",
+			expected: Directory{
+				Root:    "testTars/la-croix3-full",
+				Content: []string{"/lime.txt", "/nest", "/nest/f1.txt", "/nested-dir", "/nested-dir/f2.txt", "/passionfruit.txt", "/peach-pear.txt"},
+			},
+			deep: true,
+		},
+		{
+			descrip: "shallow",
+			path:    "testTars/la-croix3-full",
+			expected: Directory{
+				Root:    "testTars/la-croix3-full",
+				Content: []string{"/lime.txt", "/nest", "/nested-dir", "/passionfruit.txt", "/peach-pear.txt"},
+			},
+			deep: false,
+		},
 	}
+	for _, testCase := range tests {
+		dir, err := GetDirectory(testCase.path, testCase.deep)
+		if err != nil {
+			t.Errorf("Error converting directory to Directory struct")
+		}
 
-	var dirtests = []dirtestpair{
-		{"test_files/dir.json", true},
-		{"test_files/dir_bad.json", false},
-		{"nonexistentpath", false},
-		{"", false},
-	}
+		actualDir := dir
+		expectedDir := testCase.expected
 
-	for _, test := range dirtests {
-		_, err := GetDirectory(test.input)
-		if err != nil && test.expected_success {
-			t.Errorf("Got unexpected error: %s", err)
-		}
-		if err == nil && !test.expected_success {
-			t.Errorf("Expected error but got none")
+		if !reflect.DeepEqual(actualDir, expectedDir) {
+			t.Errorf("%s test was incorrect\nExpected: %s\nGot: %s", testCase.descrip, expectedDir, actualDir)
 		}
 	}
 }