Merge pull request GoogleContainerTools#101 from nkubala/refactor

remove ImagePrepper in favor of processing individual preppers directly

Author: nkubala

.travis.yml

@@ -2,6 +2,11 @@ language: go
 os: linux
 go: 1.8.3
 
+sudo: required
+
+services:
+  - docker
+
 go_import_path: github.com/GoogleCloudPlatform/container-diff
 
 addons:

Makefile

@@ -62,7 +62,7 @@ test: $(BUILD_DIR)/$(PROJECT)
 
 .PHONY: integration
 integration: $(BUILD_DIR)/$(PROJECT)
-	go test -v -tags integration $(REPOPATH)/tests
+	go test -v -tags integration $(REPOPATH)/tests -timeout 20m
 
 .PHONY: release
 release: cross
@@ -71,5 +71,3 @@ release: cross
 .PHONY: clean
 clean:
 	rm -rf $(BUILD_DIR)
-
-

cmd/analyze.go

@@ -56,22 +56,24 @@ func checkAnalyzeArgNum(args []string) error {
 	return nil
 }
 
-func analyzeImage(imageArg string, analyzerArgs []string) error {
+func analyzeImage(imageName string, analyzerArgs []string) error {
 	analyzeTypes, err := differs.GetAnalyzers(analyzerArgs)
 	if err != nil {
 		return err
 	}
 
-	cli, err := NewClient()
+	cli, err := pkgutil.NewClient()
 	if err != nil {
 		return fmt.Errorf("Error getting docker client: %s", err)
 	}
 	defer cli.Close()
-	ip := pkgutil.ImagePrepper{
-		Source: imageArg,
-		Client: cli,
+
+	prepper, err := getPrepperForImage(imageName)
+	if err != nil {
+		return err
 	}
-	image, err := ip.GetImage()
+
+	image, err := prepper.GetImage()
 
 	if !save {
 		defer pkgutil.CleanupImage(image)

cmd/diff.go

@@ -63,7 +63,7 @@ func diffImages(image1Arg, image2Arg string, diffArgs []string) error {
 		return err
 	}
 
-	cli, err := NewClient()
+	cli, err := pkgutil.NewClient()
 	if err != nil {
 		return err
 	}
@@ -77,17 +77,20 @@ func diffImages(image1Arg, image2Arg string, diffArgs []string) error {
 		image1Arg: {},
 		image2Arg: {},
 	}
+	// TODO: fix error handling here
 	for imageArg := range imageMap {
 		go func(imageName string, imageMap map[string]*pkgutil.Image) {
 			defer wg.Done()
-			ip := pkgutil.ImagePrepper{
-				Source: imageName,
-				Client: cli,
+
+			prepper, err := getPrepperForImage(imageName)
+			if err != nil {
+				glog.Error(err)
+				return
 			}
-			image, err := ip.GetImage()
+			image, err := prepper.GetImage()
 			imageMap[imageName] = &image
 			if err != nil {
-				glog.Errorf("Diff may be inaccurate: %s", err.Error())
+				glog.Warningf("Diff may be inaccurate: %s", err)
 			}
 		}(imageArg, imageMap)
 	}
@@ -109,7 +112,6 @@ func diffImages(image1Arg, image2Arg string, diffArgs []string) error {
 	if save {
 		glog.Infof("Images were saved at %s and %s", imageMap[image1Arg].FSPath,
 			imageMap[image2Arg].FSPath)
-
 	}
 	return nil
 }

cmd/root.go

@@ -17,13 +17,14 @@ limitations under the License.
 package cmd
 
 import (
-	"context"
+	"errors"
 	goflag "flag"
 	"fmt"
 	"sort"
 	"strings"
 
 	"github.com/GoogleCloudPlatform/container-diff/differs"
+	pkgutil "github.com/GoogleCloudPlatform/container-diff/pkg/util"
 	"github.com/GoogleCloudPlatform/container-diff/util"
 	"github.com/docker/docker/client"
 	"github.com/golang/glog"
@@ -37,20 +38,22 @@ var types string
 
 type validatefxn func(args []string) error
 
+const (
+	DaemonPrefix = "daemon://"
+	RemotePrefix = "remote://"
+)
+
 var RootCmd = &cobra.Command{
 	Use:   "container-diff",
 	Short: "container-diff is a tool for analyzing and comparing container images",
-	Long:  `container-diff is a CLI tool for analyzing and comparing container images.`,
-}
+	Long: `container-diff is a CLI tool for analyzing and comparing container images.
 
-func NewClient() (*client.Client, error) {
-	cli, err := client.NewEnvClient()
-	if err != nil {
-		return nil, fmt.Errorf("Error getting docker client: %s", err)
-	}
-	cli.NegotiateAPIVersion(context.Background())
+Images can be specified from either a local Docker daemon, or from a remote registry.
+To specify a local image, prefix the image ID with 'daemon://', e.g. 'daemon://gcr.io/foo/bar'.
+To specify a remote image, prefix the image ID with 'remote://', e.g. 'remote://gcr.io/foo/bar'.
+If no prefix is specified, the local daemon will be checked first.
 
-	return cli, nil
+Tarballs can also be specified by simply providing the path to the .tar, .tar.gz, or .tgz file.`,
 }
 
 func outputResults(resultMap map[string]util.Result) {
@@ -92,7 +95,7 @@ func validateArgs(args []string, validatefxns ...validatefxn) error {
 
 func checkIfValidAnalyzer(flagtypes string) error {
 	if flagtypes == "" {
-		return nil
+		return errors.New("Please provide at least one analyzer to run")
 	}
 	analyzers := strings.Split(flagtypes, ",")
 	for _, name := range analyzers {
@@ -103,6 +106,30 @@ func checkIfValidAnalyzer(flagtypes string) error {
 	return nil
 }
 
+func getPrepperForImage(image string) (pkgutil.Prepper, error) {
+	cli, err := client.NewEnvClient()
+	if err != nil {
+		return nil, err
+	}
+	if pkgutil.IsTar(image) {
+		return pkgutil.TarPrepper{
+			Source: image,
+			Client: cli,
+		}, nil
+
+	} else if strings.HasPrefix(image, DaemonPrefix) {
+		return pkgutil.DaemonPrepper{
+			Source: strings.Replace(image, DaemonPrefix, "", -1),
+			Client: cli,
+		}, nil
+	}
+	// either has remote prefix or has no prefix, in which case we force remote
+	return pkgutil.CloudPrepper{
+		Source: strings.Replace(image, RemotePrefix, "", -1),
+		Client: cli,
+	}, nil
+}
+
 func init() {
 	pflag.CommandLine.AddGoFlagSet(goflag.CommandLine)
 }

pkg/util/BUILD.bazel

@@ -8,7 +8,6 @@ go_library(
         "docker_utils.go",
         "fs_utils.go",
         "image_prep_utils.go",
-        "image_prepper.go",
         "image_utils.go",
         "tar_prepper.go",
         "tar_utils.go",

pkg/util/cloud_prepper.go

@@ -17,31 +17,26 @@ limitations under the License.
 package util
 
 import (
-	"regexp"
-
 	"github.com/containers/image/docker"
+	"github.com/docker/docker/client"
 )
 
 // CloudPrepper prepares images sourced from a Cloud registry
 type CloudPrepper struct {
-	ImagePrepper
+	Source string
+	Client *client.Client
 }
 
 func (p CloudPrepper) Name() string {
 	return "Cloud Registry"
 }
 
 func (p CloudPrepper) GetSource() string {
-	return p.ImagePrepper.Source
+	return p.Source
 }
 
-func (p CloudPrepper) SupportsImage() bool {
-	pattern := regexp.MustCompile("^.+/.+(:.+){0,1}$")
-	image := p.ImagePrepper.Source
-	if exp := pattern.FindString(image); exp != image || CheckTar(image) {
-		return false
-	}
-	return true
+func (p CloudPrepper) GetImage() (Image, error) {
+	return getImage(p)
 }
 
 func (p CloudPrepper) GetFileSystem() (string, error) {

pkg/util/daemon_prepper.go

@@ -18,30 +18,28 @@ package util
 
 import (
 	"context"
-	"regexp"
 
 	"github.com/containers/image/docker/daemon"
+
+	"github.com/docker/docker/client"
 	"github.com/golang/glog"
 )
 
 type DaemonPrepper struct {
-	ImagePrepper
+	Source string
+	Client *client.Client
 }
 
 func (p DaemonPrepper) Name() string {
 	return "Local Daemon"
 }
 
 func (p DaemonPrepper) GetSource() string {
-	return p.ImagePrepper.Source
+	return p.Source
 }
 
-func (p DaemonPrepper) SupportsImage() bool {
-	pattern := regexp.MustCompile("[a-z|0-9]{12}")
-	if exp := pattern.FindString(p.ImagePrepper.Source); exp != p.ImagePrepper.Source {
-		return false
-	}
-	return true
+func (p DaemonPrepper) GetImage() (Image, error) {
+	return getImage(p)
 }
 
 func (p DaemonPrepper) GetFileSystem() (string, error) {

pkg/util/docker_utils.go

@@ -39,6 +39,16 @@ type Event struct {
 	} `json:"progressDetail"`
 }
 
+func NewClient() (*client.Client, error) {
+	cli, err := client.NewEnvClient()
+	if err != nil {
+		return nil, fmt.Errorf("Error getting docker client: %s", err)
+	}
+	cli.NegotiateAPIVersion(context.Background())
+
+	return cli, nil
+}
+
 func getLayersFromManifest(manifestPath string) ([]string, error) {
 	type Manifest struct {
 		Layers []string

pkg/util/image_prep_utils.go

@@ -20,6 +20,7 @@ import (
 	"archive/tar"
 	"encoding/json"
 	"errors"
+	"fmt"
 	"io/ioutil"
 	"os"
 	"path/filepath"
@@ -30,10 +31,12 @@ import (
 	"github.com/golang/glog"
 )
 
-var orderedPreppers = []func(ip ImagePrepper) Prepper{
-	func(ip ImagePrepper) Prepper { return DaemonPrepper{ImagePrepper: ip} },
-	func(ip ImagePrepper) Prepper { return CloudPrepper{ImagePrepper: ip} },
-	func(ip ImagePrepper) Prepper { return TarPrepper{ImagePrepper: ip} },
+type Prepper interface {
+	Name() string
+	GetConfig() (ConfigSchema, error)
+	GetFileSystem() (string, error)
+	GetImage() (Image, error)
+	GetSource() string
 }
 
 type Image struct {
@@ -55,6 +58,27 @@ type ConfigSchema struct {
 	History []ImageHistoryItem `json:"history"`
 }
 
+func getImage(p Prepper) (Image, error) {
+	glog.Infof("Retrieving image %s from source %s", p.GetSource(), p.Name())
+	imgPath, err := p.GetFileSystem()
+	if err != nil {
+		return Image{}, err
+	}
+
+	config, err := p.GetConfig()
+	if err != nil {
+		glog.Error("Error retrieving History: ", err)
+	}
+
+	glog.Infof("Finished prepping image %s", p.GetSource())
+	return Image{
+		Source: p.GetSource(),
+		FSPath: imgPath,
+		Config: config,
+	}, nil
+	return Image{}, fmt.Errorf("Could not retrieve image %s from source", p.GetSource())
+}
+
 func getImageFromTar(tarPath string) (string, error) {
 	glog.Info("Extracting image tar to obtain image file system")
 	path := strings.TrimSuffix(tarPath, filepath.Ext(tarPath))