From fb56153ae296efb65cba8f93f11be99d9b0f52a4 Mon Sep 17 00:00:00 2001 From: Nick Kubala Date: Wed, 20 Sep 2017 09:18:56 -0700 Subject: [PATCH 1/9] add support for 'daemon' prefix to specify image in local docker daemon. fix error handling --- Godeps/Godeps.json | 4 + cmd/analyze.go | 17 +- cmd/diff.go | 26 ++- cmd/root.go | 4 +- differs/differs.go | 2 +- pkg/util/cloud_prepper.go | 13 +- pkg/util/daemon_prepper.go | 21 +- pkg/util/image_prep_utils.go | 8 +- pkg/util/image_prepper.go | 34 +++- pkg/util/tar_prepper.go | 2 +- pkg/util/tar_utils.go | 4 +- .../image/docker/daemon/daemon_dest.go | 128 ++++++++++++ .../image/docker/daemon/daemon_src.go | 85 ++++++++ .../image/docker/daemon/daemon_transport.go | 184 ++++++++++++++++++ 14 files changed, 490 insertions(+), 42 deletions(-) create mode 100644 vendor/github.com/containers/image/docker/daemon/daemon_dest.go create mode 100644 vendor/github.com/containers/image/docker/daemon/daemon_src.go create mode 100644 vendor/github.com/containers/image/docker/daemon/daemon_transport.go diff --git a/Godeps/Godeps.json b/Godeps/Godeps.json index 68156557..3fb45cad 100644 --- a/Godeps/Godeps.json +++ b/Godeps/Godeps.json @@ -19,6 +19,10 @@ "ImportPath": "github.com/containers/image/docker", "Rev": "abb4cd79e3427bb2b02a5930814ef2ad19983c24" }, + { + "ImportPath": "github.com/containers/image/docker/daemon", + "Rev": "abb4cd79e3427bb2b02a5930814ef2ad19983c24" + }, { "ImportPath": "github.com/containers/image/docker/policyconfiguration", "Rev": "abb4cd79e3427bb2b02a5930814ef2ad19983c24" diff --git a/cmd/analyze.go b/cmd/analyze.go index 286ee9b8..175b67b1 100644 --- a/cmd/analyze.go +++ b/cmd/analyze.go @@ -56,20 +56,23 @@ func checkAnalyzeArgNum(args []string) error { return nil } -func analyzeImage(imageArg string, analyzerArgs []string) error { +func analyzeImage(imageName string, analyzerArgs []string) error { + cli, err := NewClient() + if err != nil { + return fmt.Errorf("Error getting docker client for differ: %s", err) + } + defer cli.Close() + analyzeTypes, err := differs.GetAnalyzers(analyzerArgs) if err != nil { glog.Error(err.Error()) return errors.New("Could not perform image analysis") } - cli, err := NewClient() - if err != nil { - return fmt.Errorf("Error getting docker client for differ: %s", err) - } - defer cli.Close() + prefixedName := processImageName(imageName) + ip := pkgutil.ImagePrepper{ - Source: imageArg, + Source: prefixedName, Client: cli, } image, err := ip.GetImage() diff --git a/cmd/diff.go b/cmd/diff.go index d0e908bc..bc1e692b 100644 --- a/cmd/diff.go +++ b/cmd/diff.go @@ -35,10 +35,10 @@ var diffCmd = &cobra.Command{ Long: `Compares two images using the specifed analyzers as indicated via flags (see documentation for available ones).`, Args: func(cmd *cobra.Command, args []string) error { if err := validateArgs(args, checkDiffArgNum); err != nil { - return errors.New(err.Error()) + return err } if err := checkIfValidAnalyzer(types); err != nil { - return errors.New(err.Error()) + return err } return nil }, @@ -52,7 +52,7 @@ var diffCmd = &cobra.Command{ func checkDiffArgNum(args []string) error { if len(args) != 2 { - return errors.New("'diff' requires two images as arguments: container diff [image1] [image2]") + return errors.New("'diff' requires two images as arguments: container-diff diff [image1] [image2]") } return nil } @@ -60,13 +60,12 @@ func checkDiffArgNum(args []string) error { func diffImages(image1Arg, image2Arg string, diffArgs []string) error { diffTypes, err := differs.GetAnalyzers(diffArgs) if err != nil { - glog.Error(err.Error()) - return errors.New("Could not perform image diff") + return fmt.Errorf("err msg: %s", err.Error()) } cli, err := NewClient() if err != nil { - return fmt.Errorf("Error getting docker client for differ: %s", err) + return fmt.Errorf("err msg: %s", err) } defer cli.Close() var wg sync.WaitGroup @@ -81,8 +80,10 @@ func diffImages(image1Arg, image2Arg string, diffArgs []string) error { for imageArg := range imageMap { go func(imageName string, imageMap map[string]*pkgutil.Image) { defer wg.Done() + + prefixedName := processImageName(imageName) ip := pkgutil.ImagePrepper{ - Source: imageName, + Source: prefixedName, Client: cli, } image, err := ip.GetImage() @@ -102,8 +103,7 @@ func diffImages(image1Arg, image2Arg string, diffArgs []string) error { req := differs.DiffRequest{*imageMap[image1Arg], *imageMap[image2Arg], diffTypes} diffs, err := req.GetDiff() if err != nil { - glog.Error(err.Error()) - return errors.New("Could not perform image diff") + return fmt.Errorf("err msg: %s", err.Error()) } glog.Info("Retrieving diffs") outputResults(diffs) @@ -116,6 +116,14 @@ func diffImages(image1Arg, image2Arg string, diffArgs []string) error { return nil } +func processImageName(imageName string) string { + if pkgutil.IsTar(imageName) || strings.HasPrefix(imageName, "daemon://") { + return imageName + } + // not a tar and not explicitly local, so force remote + return "remote://" + imageName +} + func init() { RootCmd.AddCommand(diffCmd) addSharedFlags(diffCmd) diff --git a/cmd/root.go b/cmd/root.go index da97d4ac..07cbe87b 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -47,7 +47,7 @@ var RootCmd = &cobra.Command{ func NewClient() (*client.Client, error) { cli, err := client.NewEnvClient() if err != nil { - return nil, fmt.Errorf("Error getting docker client: %s", err) + return nil, fmt.Errorf("err msg: %s", err) } cli.NegotiateAPIVersion(context.Background()) @@ -93,7 +93,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 { diff --git a/differs/differs.go b/differs/differs.go index 26649d53..1f4cc3f4 100644 --- a/differs/differs.go +++ b/differs/differs.go @@ -103,7 +103,7 @@ func GetAnalyzers(analyzeNames []string) (analyzeFuncs []Analyzer, err error) { if a, exists := Analyzers[name]; exists { analyzeFuncs = append(analyzeFuncs, a) } else { - glog.Errorf("Unknown analyzer/differ specified", name) + glog.Errorf("Unknown analyzer/differ specified: %s", name) } } if len(analyzeFuncs) == 0 { diff --git a/pkg/util/cloud_prepper.go b/pkg/util/cloud_prepper.go index c6519c3f..4be130c7 100644 --- a/pkg/util/cloud_prepper.go +++ b/pkg/util/cloud_prepper.go @@ -20,11 +20,14 @@ import ( "regexp" "github.com/containers/image/docker" + "github.com/containers/image/docker/reference" ) +const RemotePrefix = "remote://" + // CloudPrepper prepares images sourced from a Cloud registry type CloudPrepper struct { - ImagePrepper + *ImagePrepper } func (p CloudPrepper) Name() string { @@ -36,12 +39,12 @@ func (p CloudPrepper) GetSource() string { } func (p CloudPrepper) SupportsImage() bool { - pattern := regexp.MustCompile("^.+/.+(:.+){0,1}$") - image := p.ImagePrepper.Source - if exp := pattern.FindString(image); exp != image || CheckTar(image) { + daemonRegex := regexp.MustCompile(DaemonPrefix + ".*") + if match := daemonRegex.MatchString(p.ImagePrepper.Source); match { return false } - return true + _, err := reference.Parse(p.ImagePrepper.Source) + return (err == nil) && !IsTar(p.ImagePrepper.Source) } func (p CloudPrepper) GetFileSystem() (string, error) { diff --git a/pkg/util/daemon_prepper.go b/pkg/util/daemon_prepper.go index acd6a52a..8d9a148c 100644 --- a/pkg/util/daemon_prepper.go +++ b/pkg/util/daemon_prepper.go @@ -18,14 +18,17 @@ package util import ( "context" - "os" - "regexp" + "strings" + "github.com/containers/image/docker/daemon" + "github.com/containers/image/docker/reference" "github.com/golang/glog" ) +const DaemonPrefix = "daemon://" + type DaemonPrepper struct { - ImagePrepper + *ImagePrepper } func (p DaemonPrepper) Name() string { @@ -37,21 +40,17 @@ func (p DaemonPrepper) GetSource() string { } 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 + _, err := reference.Parse(strings.Replace(p.ImagePrepper.Source, DaemonPrefix, "", -1)) + return (err != nil) && !IsTar(p.ImagePrepper.Source) } func (p DaemonPrepper) GetFileSystem() (string, error) { - tarPath, err := saveImageToTar(p.Client, p.Source, p.Source) + ref, err := daemon.ParseReference(p.Source) if err != nil { return "", err } - defer os.Remove(tarPath) - return getImageFromTar(tarPath) + return getFileSystemFromReference(ref, p.Source) } func (p DaemonPrepper) GetConfig() (ConfigSchema, error) { diff --git a/pkg/util/image_prep_utils.go b/pkg/util/image_prep_utils.go index 8eb8daab..23bf614b 100644 --- a/pkg/util/image_prep_utils.go +++ b/pkg/util/image_prep_utils.go @@ -30,10 +30,10 @@ 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} }, +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 Image struct { diff --git a/pkg/util/image_prepper.go b/pkg/util/image_prepper.go index d5f63c45..b1135f03 100644 --- a/pkg/util/image_prepper.go +++ b/pkg/util/image_prepper.go @@ -18,6 +18,8 @@ package util import ( "errors" + "fmt" + "strings" "github.com/docker/docker/client" "github.com/golang/glog" @@ -36,12 +38,42 @@ type Prepper interface { SupportsImage() bool } -func (p ImagePrepper) GetImage() (Image, error) { +func getImage(prepper Prepper) (Image, error) { + imgPath, err := prepper.GetFileSystem() + if err != nil { + return Image{}, fmt.Errorf("error msg: %s", err.Error()) + } + + config, err := prepper.GetConfig() + if err != nil { + return Image{}, fmt.Errorf("error msg: %s", err.Error()) + } + + glog.Infof("Finished prepping image %s", prepper.GetSource()) + return Image{ + Source: prepper.GetSource(), + FSPath: imgPath, + Config: config, + }, nil +} + +func (p *ImagePrepper) GetImage() (Image, error) { glog.Infof("Starting prep for image %s", p.Source) img := p.Source var prepper Prepper + // first, respect prefixes on image names + if strings.HasPrefix(p.Source, "daemon://") { + p.Source = strings.Replace(p.Source, "daemon://", "", -1) + prepper = DaemonPrepper{ImagePrepper: p} + return getImage(prepper) + } else if strings.HasPrefix(p.Source, "remote://") { + p.Source = strings.Replace(p.Source, "remote://", "", -1) + prepper = CloudPrepper{ImagePrepper: p} + return getImage(prepper) + } + for _, prepperConstructor := range orderedPreppers { prepper = prepperConstructor(p) if prepper.SupportsImage() { diff --git a/pkg/util/tar_prepper.go b/pkg/util/tar_prepper.go index 545ef117..13a86fc7 100644 --- a/pkg/util/tar_prepper.go +++ b/pkg/util/tar_prepper.go @@ -29,7 +29,7 @@ import ( ) type TarPrepper struct { - ImagePrepper + *ImagePrepper } func (p TarPrepper) Name() string { diff --git a/pkg/util/tar_utils.go b/pkg/util/tar_utils.go index f57de441..9869f83a 100644 --- a/pkg/util/tar_utils.go +++ b/pkg/util/tar_utils.go @@ -106,7 +106,9 @@ func UnTar(filename string, target string) error { } func IsTar(path string) bool { - return filepath.Ext(path) == ".tar" + return filepath.Ext(path) == ".tar" || + filepath.Ext(path) == ".tar.gz" || + filepath.Ext(path) == ".tgz" } func CheckTar(image string) bool { diff --git a/vendor/github.com/containers/image/docker/daemon/daemon_dest.go b/vendor/github.com/containers/image/docker/daemon/daemon_dest.go new file mode 100644 index 00000000..559e5c71 --- /dev/null +++ b/vendor/github.com/containers/image/docker/daemon/daemon_dest.go @@ -0,0 +1,128 @@ +package daemon + +import ( + "io" + + "github.com/containers/image/docker/reference" + "github.com/containers/image/docker/tarfile" + "github.com/containers/image/types" + "github.com/docker/docker/client" + "github.com/pkg/errors" + "github.com/sirupsen/logrus" + "golang.org/x/net/context" +) + +type daemonImageDestination struct { + ref daemonReference + *tarfile.Destination // Implements most of types.ImageDestination + // For talking to imageLoadGoroutine + goroutineCancel context.CancelFunc + statusChannel <-chan error + writer *io.PipeWriter + // Other state + committed bool // writer has been closed +} + +// newImageDestination returns a types.ImageDestination for the specified image reference. +func newImageDestination(systemCtx *types.SystemContext, ref daemonReference) (types.ImageDestination, error) { + if ref.ref == nil { + return nil, errors.Errorf("Invalid destination docker-daemon:%s: a destination must be a name:tag", ref.StringWithinTransport()) + } + namedTaggedRef, ok := ref.ref.(reference.NamedTagged) + if !ok { + return nil, errors.Errorf("Invalid destination docker-daemon:%s: a destination must be a name:tag", ref.StringWithinTransport()) + } + + c, err := client.NewClient(client.DefaultDockerHost, "1.22", nil, nil) // FIXME: overridable host + if err != nil { + return nil, errors.Wrap(err, "Error initializing docker engine client") + } + + reader, writer := io.Pipe() + // Commit() may never be called, so we may never read from this channel; so, make this buffered to allow imageLoadGoroutine to write status and terminate even if we never read it. + statusChannel := make(chan error, 1) + + ctx, goroutineCancel := context.WithCancel(context.Background()) + go imageLoadGoroutine(ctx, c, reader, statusChannel) + + return &daemonImageDestination{ + ref: ref, + Destination: tarfile.NewDestination(writer, namedTaggedRef), + goroutineCancel: goroutineCancel, + statusChannel: statusChannel, + writer: writer, + committed: false, + }, nil +} + +// imageLoadGoroutine accepts tar stream on reader, sends it to c, and reports error or success by writing to statusChannel +func imageLoadGoroutine(ctx context.Context, c *client.Client, reader *io.PipeReader, statusChannel chan<- error) { + err := errors.New("Internal error: unexpected panic in imageLoadGoroutine") + defer func() { + logrus.Debugf("docker-daemon: sending done, status %v", err) + statusChannel <- err + }() + defer func() { + if err == nil { + reader.Close() + } else { + reader.CloseWithError(err) + } + }() + + resp, err := c.ImageLoad(ctx, reader, true) + if err != nil { + err = errors.Wrap(err, "Error saving image to docker engine") + return + } + defer resp.Body.Close() +} + +// MustMatchRuntimeOS returns true iff the destination can store only images targeted for the current runtime OS. False otherwise. +func (d *daemonImageDestination) MustMatchRuntimeOS() bool { + return true +} + +// Close removes resources associated with an initialized ImageDestination, if any. +func (d *daemonImageDestination) Close() error { + if !d.committed { + logrus.Debugf("docker-daemon: Closing tar stream to abort loading") + // In principle, goroutineCancel() should abort the HTTP request and stop the process from continuing. + // In practice, though, various HTTP implementations used by client.Client.ImageLoad() (including + // https://github.com/golang/net/blob/master/context/ctxhttp/ctxhttp_pre17.go and the + // net/http version with native Context support in Go 1.7) do not always actually immediately cancel + // the operation: they may process the HTTP request, or a part of it, to completion in a goroutine, and + // return early if the context is canceled without terminating the goroutine at all. + // So we need this CloseWithError to terminate sending the HTTP request Body + // immediately, and hopefully, through terminating the sending which uses "Transfer-Encoding: chunked"" without sending + // the terminating zero-length chunk, prevent the docker daemon from processing the tar stream at all. + // Whether that works or not, closing the PipeWriter seems desirable in any case. + d.writer.CloseWithError(errors.New("Aborting upload, daemonImageDestination closed without a previous .Commit()")) + } + d.goroutineCancel() + + return nil +} + +func (d *daemonImageDestination) Reference() types.ImageReference { + return d.ref +} + +// Commit marks the process of storing the image as successful and asks for the image to be persisted. +// WARNING: This does not have any transactional semantics: +// - Uploaded data MAY be visible to others before Commit() is called +// - Uploaded data MAY be removed or MAY remain around if Close() is called without Commit() (i.e. rollback is allowed but not guaranteed) +func (d *daemonImageDestination) Commit() error { + logrus.Debugf("docker-daemon: Closing tar stream") + if err := d.Destination.Commit(); err != nil { + return err + } + if err := d.writer.Close(); err != nil { + return err + } + d.committed = true // We may still fail, but we are done sending to imageLoadGoroutine. + + logrus.Debugf("docker-daemon: Waiting for status") + err := <-d.statusChannel + return err +} diff --git a/vendor/github.com/containers/image/docker/daemon/daemon_src.go b/vendor/github.com/containers/image/docker/daemon/daemon_src.go new file mode 100644 index 00000000..644dbeec --- /dev/null +++ b/vendor/github.com/containers/image/docker/daemon/daemon_src.go @@ -0,0 +1,85 @@ +package daemon + +import ( + "io" + "io/ioutil" + "os" + + "github.com/containers/image/docker/tarfile" + "github.com/containers/image/types" + "github.com/docker/docker/client" + "github.com/pkg/errors" + "golang.org/x/net/context" +) + +const temporaryDirectoryForBigFiles = "/var/tmp" // Do not use the system default of os.TempDir(), usually /tmp, because with systemd it could be a tmpfs. + +type daemonImageSource struct { + ref daemonReference + *tarfile.Source // Implements most of types.ImageSource + tarCopyPath string +} + +type layerInfo struct { + path string + size int64 +} + +// newImageSource returns a types.ImageSource for the specified image reference. +// The caller must call .Close() on the returned ImageSource. +// +// It would be great if we were able to stream the input tar as it is being +// sent; but Docker sends the top-level manifest, which determines which paths +// to look for, at the end, so in we will need to seek back and re-read, several times. +// (We could, perhaps, expect an exact sequence, assume that the first plaintext file +// is the config, and that the following len(RootFS) files are the layers, but that feels +// way too brittle.) +func newImageSource(ctx *types.SystemContext, ref daemonReference) (types.ImageSource, error) { + c, err := client.NewClient(client.DefaultDockerHost, "1.22", nil, nil) // FIXME: overridable host + if err != nil { + return nil, errors.Wrap(err, "Error initializing docker engine client") + } + // Per NewReference(), ref.StringWithinTransport() is either an image ID (config digest), or a !reference.NameOnly() reference. + // Either way ImageSave should create a tarball with exactly one image. + inputStream, err := c.ImageSave(context.TODO(), []string{ref.StringWithinTransport()}) + if err != nil { + return nil, errors.Wrap(err, "Error loading image from docker engine") + } + defer inputStream.Close() + + // FIXME: use SystemContext here. + tarCopyFile, err := ioutil.TempFile(temporaryDirectoryForBigFiles, "docker-daemon-tar") + if err != nil { + return nil, err + } + defer tarCopyFile.Close() + + succeeded := false + defer func() { + if !succeeded { + os.Remove(tarCopyFile.Name()) + } + }() + + if _, err := io.Copy(tarCopyFile, inputStream); err != nil { + return nil, err + } + + succeeded = true + return &daemonImageSource{ + ref: ref, + Source: tarfile.NewSource(tarCopyFile.Name()), + tarCopyPath: tarCopyFile.Name(), + }, nil +} + +// Reference returns the reference used to set up this source, _as specified by the user_ +// (not as the image itself, or its underlying storage, claims). This can be used e.g. to determine which public keys are trusted for this image. +func (s *daemonImageSource) Reference() types.ImageReference { + return s.ref +} + +// Close removes resources associated with an initialized ImageSource, if any. +func (s *daemonImageSource) Close() error { + return os.Remove(s.tarCopyPath) +} diff --git a/vendor/github.com/containers/image/docker/daemon/daemon_transport.go b/vendor/github.com/containers/image/docker/daemon/daemon_transport.go new file mode 100644 index 00000000..41ccd1f1 --- /dev/null +++ b/vendor/github.com/containers/image/docker/daemon/daemon_transport.go @@ -0,0 +1,184 @@ +package daemon + +import ( + "github.com/pkg/errors" + + "github.com/containers/image/docker/reference" + "github.com/containers/image/image" + "github.com/containers/image/transports" + "github.com/containers/image/types" + "github.com/opencontainers/go-digest" +) + +func init() { + transports.Register(Transport) +} + +// Transport is an ImageTransport for images managed by a local Docker daemon. +var Transport = daemonTransport{} + +type daemonTransport struct{} + +// Name returns the name of the transport, which must be unique among other transports. +func (t daemonTransport) Name() string { + return "docker-daemon" +} + +// ParseReference converts a string, which should not start with the ImageTransport.Name prefix, into an ImageReference. +func (t daemonTransport) ParseReference(reference string) (types.ImageReference, error) { + return ParseReference(reference) +} + +// ValidatePolicyConfigurationScope checks that scope is a valid name for a signature.PolicyTransportScopes keys +// (i.e. a valid PolicyConfigurationIdentity() or PolicyConfigurationNamespaces() return value). +// It is acceptable to allow an invalid value which will never be matched, it can "only" cause user confusion. +// scope passed to this function will not be "", that value is always allowed. +func (t daemonTransport) ValidatePolicyConfigurationScope(scope string) error { + // See the explanation in daemonReference.PolicyConfigurationIdentity. + return errors.New(`docker-daemon: does not support any scopes except the default "" one`) +} + +// daemonReference is an ImageReference for images managed by a local Docker daemon +// Exactly one of id and ref can be set. +// For daemonImageSource, both id and ref are acceptable, ref must not be a NameOnly (interpreted as all tags in that repository by the daemon) +// For daemonImageDestination, it must be a ref, which is NamedTagged. +// (We could, in principle, also allow storing images without tagging them, and the user would have to refer to them using the docker image ID = config digest. +// Using the config digest requires the caller to parse the manifest themselves, which is very cumbersome; so, for now, we don’t bother.) +type daemonReference struct { + id digest.Digest + ref reference.Named // !reference.IsNameOnly +} + +// ParseReference converts a string, which should not start with the ImageTransport.Name prefix, into an ImageReference. +func ParseReference(refString string) (types.ImageReference, error) { + // This is intended to be compatible with reference.ParseAnyReference, but more strict about refusing some of the ambiguous cases. + // In particular, this rejects unprefixed digest values (64 hex chars), and sha256 digest prefixes (sha256:fewer-than-64-hex-chars). + + // digest:hexstring is structurally the same as a reponame:tag (meaning docker.io/library/reponame:tag). + // reference.ParseAnyReference interprets such strings as digests. + if dgst, err := digest.Parse(refString); err == nil { + // The daemon explicitly refuses to tag images with a reponame equal to digest.Canonical - but _only_ this digest name. + // Other digest references are ambiguous, so refuse them. + if dgst.Algorithm() != digest.Canonical { + return nil, errors.Errorf("Invalid docker-daemon: reference %s: only digest algorithm %s accepted", refString, digest.Canonical) + } + return NewReference(dgst, nil) + } + + ref, err := reference.ParseNormalizedNamed(refString) // This also rejects unprefixed digest values + if err != nil { + return nil, err + } + if reference.FamiliarName(ref) == digest.Canonical.String() { + return nil, errors.Errorf("Invalid docker-daemon: reference %s: The %s repository name is reserved for (non-shortened) digest references", refString, digest.Canonical) + } + return NewReference("", ref) +} + +// NewReference returns a docker-daemon reference for either the supplied image ID (config digest) or the supplied reference (which must satisfy !reference.IsNameOnly) +func NewReference(id digest.Digest, ref reference.Named) (types.ImageReference, error) { + if id != "" && ref != nil { + return nil, errors.New("docker-daemon: reference must not have an image ID and a reference string specified at the same time") + } + if ref != nil { + if reference.IsNameOnly(ref) { + return nil, errors.Errorf("docker-daemon: reference %s has neither a tag nor a digest", reference.FamiliarString(ref)) + } + // A github.com/distribution/reference value can have a tag and a digest at the same time! + // Most versions of docker/reference do not handle that (ignoring the tag), so reject such input. + // This MAY be accepted in the future. + _, isTagged := ref.(reference.NamedTagged) + _, isDigested := ref.(reference.Canonical) + if isTagged && isDigested { + return nil, errors.Errorf("docker-daemon: references with both a tag and digest are currently not supported") + } + } + return daemonReference{ + id: id, + ref: ref, + }, nil +} + +func (ref daemonReference) Transport() types.ImageTransport { + return Transport +} + +// StringWithinTransport returns a string representation of the reference, which MUST be such that +// reference.Transport().ParseReference(reference.StringWithinTransport()) returns an equivalent reference. +// NOTE: The returned string is not promised to be equal to the original input to ParseReference; +// e.g. default attribute values omitted by the user may be filled in in the return value, or vice versa. +// WARNING: Do not use the return value in the UI to describe an image, it does not contain the Transport().Name() prefix; +// instead, see transports.ImageName(). +func (ref daemonReference) StringWithinTransport() string { + switch { + case ref.id != "": + return ref.id.String() + case ref.ref != nil: + return reference.FamiliarString(ref.ref) + default: // Coverage: Should never happen, NewReference above should refuse such values. + panic("Internal inconsistency: daemonReference has empty id and nil ref") + } +} + +// DockerReference returns a Docker reference associated with this reference +// (fully explicit, i.e. !reference.IsNameOnly, but reflecting user intent, +// not e.g. after redirect or alias processing), or nil if unknown/not applicable. +func (ref daemonReference) DockerReference() reference.Named { + return ref.ref // May be nil +} + +// PolicyConfigurationIdentity returns a string representation of the reference, suitable for policy lookup. +// This MUST reflect user intent, not e.g. after processing of third-party redirects or aliases; +// The value SHOULD be fully explicit about its semantics, with no hidden defaults, AND canonical +// (i.e. various references with exactly the same semantics should return the same configuration identity) +// It is fine for the return value to be equal to StringWithinTransport(), and it is desirable but +// not required/guaranteed that it will be a valid input to Transport().ParseReference(). +// Returns "" if configuration identities for these references are not supported. +func (ref daemonReference) PolicyConfigurationIdentity() string { + // We must allow referring to images in the daemon by image ID, otherwise untagged images would not be accessible. + // But the existence of image IDs means that we can’t truly well namespace the input; the untagged images would have to fall into the default policy, + // which can be unexpected. So, punt. + return "" // This still allows using the default "" scope to define a policy for this transport. +} + +// PolicyConfigurationNamespaces returns a list of other policy configuration namespaces to search +// for if explicit configuration for PolicyConfigurationIdentity() is not set. The list will be processed +// in order, terminating on first match, and an implicit "" is always checked at the end. +// It is STRONGLY recommended for the first element, if any, to be a prefix of PolicyConfigurationIdentity(), +// and each following element to be a prefix of the element preceding it. +func (ref daemonReference) PolicyConfigurationNamespaces() []string { + // See the explanation in daemonReference.PolicyConfigurationIdentity. + return []string{} +} + +// NewImage returns a types.Image for this reference. +// The caller must call .Close() on the returned Image. +func (ref daemonReference) NewImage(ctx *types.SystemContext) (types.Image, error) { + src, err := newImageSource(ctx, ref) + if err != nil { + return nil, err + } + return image.FromSource(src) +} + +// NewImageSource returns a types.ImageSource for this reference, +// asking the backend to use a manifest from requestedManifestMIMETypes if possible. +// nil requestedManifestMIMETypes means manifest.DefaultRequestedManifestMIMETypes. +// The caller must call .Close() on the returned ImageSource. +func (ref daemonReference) NewImageSource(ctx *types.SystemContext, requestedManifestMIMETypes []string) (types.ImageSource, error) { + return newImageSource(ctx, ref) +} + +// NewImageDestination returns a types.ImageDestination for this reference. +// The caller must call .Close() on the returned ImageDestination. +func (ref daemonReference) NewImageDestination(ctx *types.SystemContext) (types.ImageDestination, error) { + return newImageDestination(ctx, ref) +} + +// DeleteImage deletes the named image from the registry, if supported. +func (ref daemonReference) DeleteImage(ctx *types.SystemContext) error { + // Should this just untag the image? Should this stop running containers? + // The semantics is not quite as clear as for remote repositories. + // The user can run (docker rmi) directly anyway, so, for now(?), punt instead of trying to guess what the user meant. + return errors.Errorf("Deleting images not implemented for docker-daemon: images") +} From a005f0c10fe46040564af22b4804c1360f8cd575 Mon Sep 17 00:00:00 2001 From: Nick Kubala Date: Wed, 20 Sep 2017 09:22:33 -0700 Subject: [PATCH 2/9] fix test --- util/image_utils_test.go | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/util/image_utils_test.go b/util/image_utils_test.go index 81d1a0af..d9853769 100644 --- a/util/image_utils_test.go +++ b/util/image_utils_test.go @@ -29,12 +29,11 @@ type imageTestPair struct { func TestCheckImageID(t *testing.T) { for _, test := range []imageTestPair{ - {input: "123456789012", expectedOutput: true}, {input: "gcr.io/repo/image", expectedOutput: false}, {input: "testTars/la-croix1.tar", expectedOutput: false}, } { prepper := pkgutil.DaemonPrepper{ - pkgutil.ImagePrepper{ + &pkgutil.ImagePrepper{ Source: test.input, }, } @@ -51,7 +50,6 @@ func TestCheckImageID(t *testing.T) { func TestCheckImageTar(t *testing.T) { for _, test := range []imageTestPair{ - {input: "123456789012", expectedOutput: false}, {input: "gcr.io/repo/image", expectedOutput: false}, {input: "testTars/la-croix1.tar", expectedOutput: true}, } { @@ -68,12 +66,11 @@ func TestCheckImageTar(t *testing.T) { func TestCheckImageURL(t *testing.T) { for _, test := range []imageTestPair{ - {input: "123456789012", expectedOutput: false}, {input: "gcr.io/repo/image", expectedOutput: true}, {input: "testTars/la-croix1.tar", expectedOutput: false}, } { prepper := pkgutil.CloudPrepper{ - pkgutil.ImagePrepper{ + &pkgutil.ImagePrepper{ Source: test.input, }, } From 369759d377753b221063bd2019864b37bb396198 Mon Sep 17 00:00:00 2001 From: Nick Kubala Date: Wed, 20 Sep 2017 09:26:10 -0700 Subject: [PATCH 3/9] gazelle --- pkg/util/BUILD.bazel | 2 ++ .../image/docker/daemon/BUILD.bazel | 23 +++++++++++++++++++ 2 files changed, 25 insertions(+) create mode 100644 vendor/github.com/containers/image/docker/daemon/BUILD.bazel diff --git a/pkg/util/BUILD.bazel b/pkg/util/BUILD.bazel index 7bd3f420..9949672c 100644 --- a/pkg/util/BUILD.bazel +++ b/pkg/util/BUILD.bazel @@ -16,6 +16,8 @@ go_library( visibility = ["//visibility:public"], deps = [ "//vendor/github.com/containers/image/docker:go_default_library", + "//vendor/github.com/containers/image/docker/daemon:go_default_library", + "//vendor/github.com/containers/image/docker/reference:go_default_library", "//vendor/github.com/containers/image/docker/tarfile:go_default_library", "//vendor/github.com/containers/image/pkg/compression:go_default_library", "//vendor/github.com/containers/image/types:go_default_library", diff --git a/vendor/github.com/containers/image/docker/daemon/BUILD.bazel b/vendor/github.com/containers/image/docker/daemon/BUILD.bazel new file mode 100644 index 00000000..99596d89 --- /dev/null +++ b/vendor/github.com/containers/image/docker/daemon/BUILD.bazel @@ -0,0 +1,23 @@ +load("@io_bazel_rules_go//go:def.bzl", "go_library") + +go_library( + name = "go_default_library", + srcs = [ + "daemon_dest.go", + "daemon_src.go", + "daemon_transport.go", + ], + visibility = ["//visibility:public"], + deps = [ + "//vendor/github.com/containers/image/docker/reference:go_default_library", + "//vendor/github.com/containers/image/docker/tarfile:go_default_library", + "//vendor/github.com/containers/image/image:go_default_library", + "//vendor/github.com/containers/image/transports:go_default_library", + "//vendor/github.com/containers/image/types:go_default_library", + "//vendor/github.com/docker/docker/client:go_default_library", + "//vendor/github.com/opencontainers/go-digest:go_default_library", + "//vendor/github.com/pkg/errors:go_default_library", + "//vendor/github.com/sirupsen/logrus:go_default_library", + "//vendor/golang.org/x/net/context:go_default_library", + ], +) From d4db8beb5959c626b8ead9836166f40b8363684b Mon Sep 17 00:00:00 2001 From: Nick Kubala Date: Wed, 20 Sep 2017 10:15:32 -0700 Subject: [PATCH 4/9] wip; adding e2e tests for local docker images --- tests/integration_test.go | 14 ++++++++++++++ tests/test_analyzer_runs.txt | 4 ---- tests/test_differ_runs.txt | 6 ------ 3 files changed, 14 insertions(+), 10 deletions(-) delete mode 100644 tests/test_analyzer_runs.txt delete mode 100644 tests/test_differ_runs.txt diff --git a/tests/integration_test.go b/tests/integration_test.go index 0add9fe1..662271ef 100644 --- a/tests/integration_test.go +++ b/tests/integration_test.go @@ -26,6 +26,8 @@ import ( "path/filepath" "strings" "testing" + + client "github.com/fsouza/go-dockerclient" ) const ( @@ -43,6 +45,9 @@ const ( multiBase = "gcr.io/gcp-runtimes/multi-base" multiModified = "gcr.io/gcp-runtimes/multi-modified" + + multiBaseLocal = "daemon://gcr.io/gcp-runtimes/multi-base" + multiModifiedLocal = "daemon://gcr.io/gcp-runtimes/multi-modified" ) type ContainerDiffRunner struct { @@ -185,3 +190,12 @@ func TestDiffAndAnalysis(t *testing.T) { }) } } + +func TestMain(m *testing.M) { + // setup + cli := client.NewEnvClient() + err := cli.PullImage(&PullImageOptions{ + + }) + os.Exit(m.Run()) +} \ No newline at end of file diff --git a/tests/test_analyzer_runs.txt b/tests/test_analyzer_runs.txt deleted file mode 100644 index 43615d0b..00000000 --- a/tests/test_analyzer_runs.txt +++ /dev/null @@ -1,4 +0,0 @@ --p pip gcr.io/gcp-runtimes/pip-modified tests/pip_analysis_actual.json --a apt gcr.io/gcp-runtimes/apt-modified tests/apt_analysis_actual.json --n node gcr.io/gcp-runtimes/node-modified tests/node_analysis_actual.json --fo fileOrder gcr.io/gcp-runtimes/diff-modified tests/file_sorted_analysis_actual.json diff --git a/tests/test_differ_runs.txt b/tests/test_differ_runs.txt deleted file mode 100644 index 5b83c7e4..00000000 --- a/tests/test_differ_runs.txt +++ /dev/null @@ -1,6 +0,0 @@ --f file gcr.io/gcp-runtimes/diff-base gcr.io/gcp-runtimes/diff-modified tests/file_diff_actual.json --a apt gcr.io/gcp-runtimes/apt-base gcr.io/gcp-runtimes/apt-modified tests/apt_diff_actual.json --n nodeOrder gcr.io/gcp-runtimes/node-modified:2.0 gcr.io/gcp-runtimes/node-modified tests/node_diff_order_actual.json --npa multi gcr.io/gcp-runtimes/multi-base gcr.io/gcp-runtimes/multi-modified tests/multi_diff_actual.json --d history gcr.io/gcp-runtimes/diff-base gcr.io/gcp-runtimes/diff-modified tests/hist_diff_actual.json --ao aptSort gcr.io/gcp-runtimes/apt-base gcr.io/gcp-runtimes/apt-modified tests/apt_sorted_diff_actual.json From f9d7987d883629a9bb6b167f4e1f3db7b2a467b1 Mon Sep 17 00:00:00 2001 From: Nick Kubala Date: Wed, 20 Sep 2017 11:22:24 -0700 Subject: [PATCH 5/9] add e2e test for local docker images --- tests/integration_test.go | 31 +++++++++++++++++++++++++------ 1 file changed, 25 insertions(+), 6 deletions(-) diff --git a/tests/integration_test.go b/tests/integration_test.go index 662271ef..add745a3 100644 --- a/tests/integration_test.go +++ b/tests/integration_test.go @@ -20,14 +20,17 @@ package tests import ( "bytes" + "context" "fmt" "io/ioutil" + "os" "os/exec" "path/filepath" "strings" "testing" - client "github.com/fsouza/go-dockerclient" + "github.com/docker/docker/client" + "github.com/docker/docker/api/types" ) const ( @@ -120,6 +123,14 @@ func TestDiffAndAnalysis(t *testing.T) { differFlags: []string{"--types=node,pip,apt"}, expectedFile: "multi_diff_expected.json", }, + { + description: "multi differ local", + subcommand: "diff", + imageA: multiBaseLocal, + imageB: multiModifiedLocal, + differFlags: []string{"--types=node,pip,apt"}, + expectedFile: "multi_diff_expected.json", + }, { description: "history differ", subcommand: "diff", @@ -193,9 +204,17 @@ func TestDiffAndAnalysis(t *testing.T) { func TestMain(m *testing.M) { // setup - cli := client.NewEnvClient() - err := cli.PullImage(&PullImageOptions{ - - }) + ctx := context.Background() + cli, _ := client.NewEnvClient() + closer, err := cli.ImagePull(ctx, multiBase, types.ImagePullOptions{}) + if err != nil { + os.Exit(1) + } + closer.Close() + closer, err = cli.ImagePull(ctx, multiModified, types.ImagePullOptions{}) + if err != nil { + os.Exit(1) + } + closer.Close() os.Exit(m.Run()) -} \ No newline at end of file +} From 0293e9a87990fcdaad43595bca81b326b352c6b5 Mon Sep 17 00:00:00 2001 From: Nick Kubala Date: Wed, 20 Sep 2017 13:53:22 -0700 Subject: [PATCH 6/9] remove redundant logic from image prepper. fix error handling --- cmd/diff.go | 13 +++++++------ pkg/util/cloud_prepper.go | 11 +++++++++-- pkg/util/daemon_prepper.go | 16 ++++++++++++++-- pkg/util/image_prepper.go | 12 ------------ 4 files changed, 30 insertions(+), 22 deletions(-) diff --git a/cmd/diff.go b/cmd/diff.go index bc1e692b..add4a8a0 100644 --- a/cmd/diff.go +++ b/cmd/diff.go @@ -60,12 +60,12 @@ func checkDiffArgNum(args []string) error { func diffImages(image1Arg, image2Arg string, diffArgs []string) error { diffTypes, err := differs.GetAnalyzers(diffArgs) if err != nil { - return fmt.Errorf("err msg: %s", err.Error()) + return err } cli, err := NewClient() if err != nil { - return fmt.Errorf("err msg: %s", err) + return err } defer cli.Close() var wg sync.WaitGroup @@ -89,7 +89,7 @@ func diffImages(image1Arg, image2Arg string, diffArgs []string) error { image, err := ip.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) } @@ -111,17 +111,18 @@ 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 } func processImageName(imageName string) string { - if pkgutil.IsTar(imageName) || strings.HasPrefix(imageName, "daemon://") { + if pkgutil.IsTar(imageName) || + strings.HasPrefix(imageName, pkgutil.DaemonPrefix) || + strings.HasPrefix(imageName, pkgutil.RemotePrefix) { return imageName } // not a tar and not explicitly local, so force remote - return "remote://" + imageName + return pkgutil.RemotePrefix + imageName } func init() { diff --git a/pkg/util/cloud_prepper.go b/pkg/util/cloud_prepper.go index 4be130c7..008631ff 100644 --- a/pkg/util/cloud_prepper.go +++ b/pkg/util/cloud_prepper.go @@ -18,6 +18,7 @@ package util import ( "regexp" + "strings" "github.com/containers/image/docker" "github.com/containers/image/docker/reference" @@ -39,12 +40,18 @@ func (p CloudPrepper) GetSource() string { } func (p CloudPrepper) SupportsImage() bool { + // will fail for strings prefixed with 'daemon://' daemonRegex := regexp.MustCompile(DaemonPrefix + ".*") - if match := daemonRegex.MatchString(p.ImagePrepper.Source); match { + if match := daemonRegex.MatchString(p.ImagePrepper.Source); match || IsTar(p.ImagePrepper.Source) { return false } + strippedSource := strings.Replace(p.ImagePrepper.Source, RemotePrefix, "", -1) _, err := reference.Parse(p.ImagePrepper.Source) - return (err == nil) && !IsTar(p.ImagePrepper.Source) + if err != nil { + p.ImagePrepper.Source = strippedSource + return true + } + return false } func (p CloudPrepper) GetFileSystem() (string, error) { diff --git a/pkg/util/daemon_prepper.go b/pkg/util/daemon_prepper.go index 8d9a148c..e74050fb 100644 --- a/pkg/util/daemon_prepper.go +++ b/pkg/util/daemon_prepper.go @@ -18,6 +18,7 @@ package util import ( "context" + "regexp" "strings" "github.com/containers/image/docker/daemon" @@ -40,8 +41,19 @@ func (p DaemonPrepper) GetSource() string { } func (p DaemonPrepper) SupportsImage() bool { - _, err := reference.Parse(strings.Replace(p.ImagePrepper.Source, DaemonPrefix, "", -1)) - return (err != nil) && !IsTar(p.ImagePrepper.Source) + // will fail on strings prefixed with 'remote://' + remoteRegex := regexp.MustCompile(RemotePrefix + ".*") + if match := remoteRegex.MatchString(p.ImagePrepper.Source); match || IsTar(p.ImagePrepper.Source) { + return false + } + strippedSource := strings.Replace(p.ImagePrepper.Source, DaemonPrefix, "", -1) + _, err := reference.Parse(strippedSource) + if err != nil { + // strip prefix off image source for later use + p.ImagePrepper.Source = strippedSource + return true + } + return false } func (p DaemonPrepper) GetFileSystem() (string, error) { diff --git a/pkg/util/image_prepper.go b/pkg/util/image_prepper.go index b1135f03..76503e2e 100644 --- a/pkg/util/image_prepper.go +++ b/pkg/util/image_prepper.go @@ -19,7 +19,6 @@ package util import ( "errors" "fmt" - "strings" "github.com/docker/docker/client" "github.com/golang/glog" @@ -63,17 +62,6 @@ func (p *ImagePrepper) GetImage() (Image, error) { var prepper Prepper - // first, respect prefixes on image names - if strings.HasPrefix(p.Source, "daemon://") { - p.Source = strings.Replace(p.Source, "daemon://", "", -1) - prepper = DaemonPrepper{ImagePrepper: p} - return getImage(prepper) - } else if strings.HasPrefix(p.Source, "remote://") { - p.Source = strings.Replace(p.Source, "remote://", "", -1) - prepper = CloudPrepper{ImagePrepper: p} - return getImage(prepper) - } - for _, prepperConstructor := range orderedPreppers { prepper = prepperConstructor(p) if prepper.SupportsImage() { From 8702bb71f341267d61127b686cdb4d90663454bb Mon Sep 17 00:00:00 2001 From: Nick Kubala Date: Wed, 20 Sep 2017 15:17:02 -0700 Subject: [PATCH 7/9] fix tests --- pkg/util/cloud_prepper.go | 3 ++- pkg/util/daemon_prepper.go | 2 +- tests/integration_test.go | 2 +- util/image_utils_test.go | 8 +++++--- 4 files changed, 9 insertions(+), 6 deletions(-) diff --git a/pkg/util/cloud_prepper.go b/pkg/util/cloud_prepper.go index 008631ff..b2e20f64 100644 --- a/pkg/util/cloud_prepper.go +++ b/pkg/util/cloud_prepper.go @@ -47,7 +47,8 @@ func (p CloudPrepper) SupportsImage() bool { } strippedSource := strings.Replace(p.ImagePrepper.Source, RemotePrefix, "", -1) _, err := reference.Parse(p.ImagePrepper.Source) - if err != nil { + if err == nil { + // strip prefix off image source for later use p.ImagePrepper.Source = strippedSource return true } diff --git a/pkg/util/daemon_prepper.go b/pkg/util/daemon_prepper.go index e74050fb..a9a4d396 100644 --- a/pkg/util/daemon_prepper.go +++ b/pkg/util/daemon_prepper.go @@ -48,7 +48,7 @@ func (p DaemonPrepper) SupportsImage() bool { } strippedSource := strings.Replace(p.ImagePrepper.Source, DaemonPrefix, "", -1) _, err := reference.Parse(strippedSource) - if err != nil { + if err == nil { // strip prefix off image source for later use p.ImagePrepper.Source = strippedSource return true diff --git a/tests/integration_test.go b/tests/integration_test.go index add745a3..c0803c40 100644 --- a/tests/integration_test.go +++ b/tests/integration_test.go @@ -29,8 +29,8 @@ import ( "strings" "testing" - "github.com/docker/docker/client" "github.com/docker/docker/api/types" + "github.com/docker/docker/client" ) const ( diff --git a/util/image_utils_test.go b/util/image_utils_test.go index d9853769..ddd12d77 100644 --- a/util/image_utils_test.go +++ b/util/image_utils_test.go @@ -27,9 +27,10 @@ type imageTestPair struct { expectedOutput bool } -func TestCheckImageID(t *testing.T) { +func TestCheckLocalImage(t *testing.T) { for _, test := range []imageTestPair{ - {input: "gcr.io/repo/image", expectedOutput: false}, + {input: "daemon://gcr.io/repo/image", expectedOutput: true}, + {input: "remote://gcr.io/repo/image", expectedOutput: false}, {input: "testTars/la-croix1.tar", expectedOutput: false}, } { prepper := pkgutil.DaemonPrepper{ @@ -64,9 +65,10 @@ func TestCheckImageTar(t *testing.T) { } } -func TestCheckImageURL(t *testing.T) { +func TestCheckRemoteImage(t *testing.T) { for _, test := range []imageTestPair{ {input: "gcr.io/repo/image", expectedOutput: true}, + {input: "daemon://gcr.io/repo/image", expectedOutput: false}, {input: "testTars/la-croix1.tar", expectedOutput: false}, } { prepper := pkgutil.CloudPrepper{ From 6a1e1698fa30fc611c12a51b980e4545a21e49bb Mon Sep 17 00:00:00 2001 From: Nick Kubala Date: Thu, 21 Sep 2017 11:48:01 -0700 Subject: [PATCH 8/9] allow overriding of image preppers from client. do not mutate imageprepper in validation --- cmd/analyze.go | 20 ++++++++++++++++-- cmd/diff.go | 14 ++----------- pkg/util/cloud_prepper.go | 28 +++++++++----------------- pkg/util/daemon_prepper.go | 39 +++++++++++------------------------- pkg/util/image_prep_utils.go | 12 +++++++---- pkg/util/image_prepper.go | 24 ++-------------------- pkg/util/tar_prepper.go | 8 ++++---- util/image_utils_test.go | 5 +++-- 8 files changed, 59 insertions(+), 91 deletions(-) diff --git a/cmd/analyze.go b/cmd/analyze.go index 175b67b1..869faea5 100644 --- a/cmd/analyze.go +++ b/cmd/analyze.go @@ -69,10 +69,10 @@ func analyzeImage(imageName string, analyzerArgs []string) error { return errors.New("Could not perform image analysis") } - prefixedName := processImageName(imageName) + pkgutil.SetPreppers(getPrepperForImage(imageName)) ip := pkgutil.ImagePrepper{ - Source: prefixedName, + Source: imageName, Client: cli, } image, err := ip.GetImage() @@ -102,6 +102,22 @@ func analyzeImage(imageName string, analyzerArgs []string) error { return nil } +func getPrepperForImage(image string) []func(ip pkgutil.ImagePrepper) pkgutil.Prepper { + if pkgutil.IsTar(image) { + return []func(pkgutil.ImagePrepper) pkgutil.Prepper{ + func(ip pkgutil.ImagePrepper) pkgutil.Prepper { return pkgutil.TarPrepper{ImagePrepper: ip} }, + } + } else if strings.HasPrefix(image, pkgutil.DaemonPrefix) { + return []func(pkgutil.ImagePrepper) pkgutil.Prepper{ + func(ip pkgutil.ImagePrepper) pkgutil.Prepper { return pkgutil.DaemonPrepper{ImagePrepper: ip} }, + } + } + // either has remote prefix or has no prefix, in which case we force remote + return []func(pkgutil.ImagePrepper) pkgutil.Prepper{ + func(ip pkgutil.ImagePrepper) pkgutil.Prepper { return pkgutil.CloudPrepper{ImagePrepper: ip} }, + } +} + func init() { RootCmd.AddCommand(analyzeCmd) addSharedFlags(analyzeCmd) diff --git a/cmd/diff.go b/cmd/diff.go index add4a8a0..6f9e8650 100644 --- a/cmd/diff.go +++ b/cmd/diff.go @@ -81,9 +81,9 @@ func diffImages(image1Arg, image2Arg string, diffArgs []string) error { go func(imageName string, imageMap map[string]*pkgutil.Image) { defer wg.Done() - prefixedName := processImageName(imageName) + pkgutil.SetPreppers(getPrepperForImage(imageName)) ip := pkgutil.ImagePrepper{ - Source: prefixedName, + Source: imageName, Client: cli, } image, err := ip.GetImage() @@ -115,16 +115,6 @@ func diffImages(image1Arg, image2Arg string, diffArgs []string) error { return nil } -func processImageName(imageName string) string { - if pkgutil.IsTar(imageName) || - strings.HasPrefix(imageName, pkgutil.DaemonPrefix) || - strings.HasPrefix(imageName, pkgutil.RemotePrefix) { - return imageName - } - // not a tar and not explicitly local, so force remote - return pkgutil.RemotePrefix + imageName -} - func init() { RootCmd.AddCommand(diffCmd) addSharedFlags(diffCmd) diff --git a/pkg/util/cloud_prepper.go b/pkg/util/cloud_prepper.go index b2e20f64..d8e9ba63 100644 --- a/pkg/util/cloud_prepper.go +++ b/pkg/util/cloud_prepper.go @@ -17,7 +17,6 @@ limitations under the License. package util import ( - "regexp" "strings" "github.com/containers/image/docker" @@ -28,47 +27,40 @@ const RemotePrefix = "remote://" // CloudPrepper prepares images sourced from a Cloud registry type CloudPrepper struct { - *ImagePrepper + ImagePrepper } func (p CloudPrepper) Name() string { return "Cloud Registry" } -func (p CloudPrepper) GetSource() string { - return p.ImagePrepper.Source +func (p CloudPrepper) RawSource() string { + return strings.Replace(p.Source, RemotePrefix, "", -1) } func (p CloudPrepper) SupportsImage() bool { // will fail for strings prefixed with 'daemon://' - daemonRegex := regexp.MustCompile(DaemonPrefix + ".*") - if match := daemonRegex.MatchString(p.ImagePrepper.Source); match || IsTar(p.ImagePrepper.Source) { + if strings.HasPrefix(p.Source, DaemonPrefix) || IsTar(p.Source) { return false } - strippedSource := strings.Replace(p.ImagePrepper.Source, RemotePrefix, "", -1) - _, err := reference.Parse(p.ImagePrepper.Source) - if err == nil { - // strip prefix off image source for later use - p.ImagePrepper.Source = strippedSource - return true - } - return false + _, err := reference.Parse(p.RawSource()) + return (err == nil) } func (p CloudPrepper) GetFileSystem() (string, error) { - ref, err := docker.ParseReference("//" + p.Source) + ref, err := docker.ParseReference("//" + p.RawSource()) if err != nil { return "", err } - return getFileSystemFromReference(ref, p.Source) + return getFileSystemFromReference(ref, p.RawSource()) } func (p CloudPrepper) GetConfig() (ConfigSchema, error) { - ref, err := docker.ParseReference("//" + p.Source) + ref, err := docker.ParseReference("//" + p.RawSource()) if err != nil { return ConfigSchema{}, err } - return getConfigFromReference(ref, p.Source) + return getConfigFromReference(ref, p.RawSource()) } diff --git a/pkg/util/daemon_prepper.go b/pkg/util/daemon_prepper.go index a9a4d396..762e7c85 100644 --- a/pkg/util/daemon_prepper.go +++ b/pkg/util/daemon_prepper.go @@ -18,7 +18,6 @@ package util import ( "context" - "regexp" "strings" "github.com/containers/image/docker/daemon" @@ -29,62 +28,48 @@ import ( const DaemonPrefix = "daemon://" type DaemonPrepper struct { - *ImagePrepper + ImagePrepper } func (p DaemonPrepper) Name() string { return "Local Daemon" } -func (p DaemonPrepper) GetSource() string { - return p.ImagePrepper.Source +func (p DaemonPrepper) RawSource() string { + return strings.Replace(p.Source, DaemonPrefix, "", -1) } func (p DaemonPrepper) SupportsImage() bool { // will fail on strings prefixed with 'remote://' - remoteRegex := regexp.MustCompile(RemotePrefix + ".*") - if match := remoteRegex.MatchString(p.ImagePrepper.Source); match || IsTar(p.ImagePrepper.Source) { + if strings.HasPrefix(p.Source, RemotePrefix) || IsTar(p.Source) { return false } - strippedSource := strings.Replace(p.ImagePrepper.Source, DaemonPrefix, "", -1) - _, err := reference.Parse(strippedSource) - if err == nil { - // strip prefix off image source for later use - p.ImagePrepper.Source = strippedSource - return true - } - return false + _, err := reference.Parse(p.RawSource()) + return (err == nil) } func (p DaemonPrepper) GetFileSystem() (string, error) { - ref, err := daemon.ParseReference(p.Source) + ref, err := daemon.ParseReference(p.RawSource()) if err != nil { return "", err } - return getFileSystemFromReference(ref, p.Source) + return getFileSystemFromReference(ref, p.RawSource()) } func (p DaemonPrepper) GetConfig() (ConfigSchema, error) { - inspect, _, err := p.Client.ImageInspectWithRaw(context.Background(), p.Source) + ref, err := daemon.ParseReference(p.RawSource()) if err != nil { return ConfigSchema{}, err } - config := ConfigObject{ - Env: inspect.Config.Env, - } - history := p.GetHistory() - return ConfigSchema{ - Config: config, - History: history, - }, nil + return getConfigFromReference(ref, p.RawSource()) } func (p DaemonPrepper) GetHistory() []ImageHistoryItem { - history, err := p.Client.ImageHistory(context.Background(), p.Source) + history, err := p.Client.ImageHistory(context.Background(), p.RawSource()) if err != nil { - glog.Error("Could not obtain image history for %s: %s", p.Source, err) + glog.Error("Could not obtain image history for %s: %s", p.RawSource(), err) } historyItems := []ImageHistoryItem{} for _, item := range history { diff --git a/pkg/util/image_prep_utils.go b/pkg/util/image_prep_utils.go index 23bf614b..a1d2e42e 100644 --- a/pkg/util/image_prep_utils.go +++ b/pkg/util/image_prep_utils.go @@ -30,10 +30,14 @@ 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} }, +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} }, +} + +func SetPreppers(preppers []func(ImagePrepper) Prepper) { + orderedPreppers = preppers } type Image struct { diff --git a/pkg/util/image_prepper.go b/pkg/util/image_prepper.go index 76503e2e..b3ff9c37 100644 --- a/pkg/util/image_prepper.go +++ b/pkg/util/image_prepper.go @@ -18,7 +18,6 @@ package util import ( "errors" - "fmt" "github.com/docker/docker/client" "github.com/golang/glog" @@ -31,32 +30,13 @@ type ImagePrepper struct { type Prepper interface { Name() string - GetSource() string + RawSource() string GetFileSystem() (string, error) GetConfig() (ConfigSchema, error) SupportsImage() bool } -func getImage(prepper Prepper) (Image, error) { - imgPath, err := prepper.GetFileSystem() - if err != nil { - return Image{}, fmt.Errorf("error msg: %s", err.Error()) - } - - config, err := prepper.GetConfig() - if err != nil { - return Image{}, fmt.Errorf("error msg: %s", err.Error()) - } - - glog.Infof("Finished prepping image %s", prepper.GetSource()) - return Image{ - Source: prepper.GetSource(), - FSPath: imgPath, - Config: config, - }, nil -} - -func (p *ImagePrepper) GetImage() (Image, error) { +func (p ImagePrepper) GetImage() (Image, error) { glog.Infof("Starting prep for image %s", p.Source) img := p.Source diff --git a/pkg/util/tar_prepper.go b/pkg/util/tar_prepper.go index 13a86fc7..8bd1ecf8 100644 --- a/pkg/util/tar_prepper.go +++ b/pkg/util/tar_prepper.go @@ -29,19 +29,19 @@ import ( ) type TarPrepper struct { - *ImagePrepper + ImagePrepper } func (p TarPrepper) Name() string { return "Tar Archive" } -func (p TarPrepper) GetSource() string { - return p.ImagePrepper.Source +func (p TarPrepper) RawSource() string { + return p.Source } func (p TarPrepper) SupportsImage() bool { - return IsTar(p.ImagePrepper.Source) + return IsTar(p.Source) } func (p TarPrepper) GetFileSystem() (string, error) { diff --git a/util/image_utils_test.go b/util/image_utils_test.go index ddd12d77..79bf43e4 100644 --- a/util/image_utils_test.go +++ b/util/image_utils_test.go @@ -34,7 +34,7 @@ func TestCheckLocalImage(t *testing.T) { {input: "testTars/la-croix1.tar", expectedOutput: false}, } { prepper := pkgutil.DaemonPrepper{ - &pkgutil.ImagePrepper{ + pkgutil.ImagePrepper{ Source: test.input, }, } @@ -69,10 +69,11 @@ func TestCheckRemoteImage(t *testing.T) { for _, test := range []imageTestPair{ {input: "gcr.io/repo/image", expectedOutput: true}, {input: "daemon://gcr.io/repo/image", expectedOutput: false}, + {input: "remote://gcr.io/repo/image", expectedOutput: true}, {input: "testTars/la-croix1.tar", expectedOutput: false}, } { prepper := pkgutil.CloudPrepper{ - &pkgutil.ImagePrepper{ + pkgutil.ImagePrepper{ Source: test.input, }, } From 6de5efb0c90f11d9b7e20e9ee8a76e3b571353cb Mon Sep 17 00:00:00 2001 From: Nick Kubala Date: Thu, 21 Sep 2017 14:08:56 -0700 Subject: [PATCH 9/9] remove ImagePrepper. remove prefix, caller now retrieves correct prepper directly, no inference --- cmd/analyze.go | 27 +++----------- cmd/diff.go | 10 ++--- cmd/root.go | 31 ++++++++++++++++ pkg/util/cloud_prepper.go | 30 ++++++--------- pkg/util/daemon_prepper.go | 33 +++++++---------- pkg/util/image_prep_utils.go | 36 ++++++++++++++---- pkg/util/image_prepper.go | 72 ------------------------------------ pkg/util/tar_prepper.go | 10 +++-- 8 files changed, 99 insertions(+), 150 deletions(-) delete mode 100644 pkg/util/image_prepper.go diff --git a/cmd/analyze.go b/cmd/analyze.go index 869faea5..12cc864e 100644 --- a/cmd/analyze.go +++ b/cmd/analyze.go @@ -69,13 +69,12 @@ func analyzeImage(imageName string, analyzerArgs []string) error { return errors.New("Could not perform image analysis") } - pkgutil.SetPreppers(getPrepperForImage(imageName)) - - ip := pkgutil.ImagePrepper{ - Source: imageName, - 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) @@ -102,22 +101,6 @@ func analyzeImage(imageName string, analyzerArgs []string) error { return nil } -func getPrepperForImage(image string) []func(ip pkgutil.ImagePrepper) pkgutil.Prepper { - if pkgutil.IsTar(image) { - return []func(pkgutil.ImagePrepper) pkgutil.Prepper{ - func(ip pkgutil.ImagePrepper) pkgutil.Prepper { return pkgutil.TarPrepper{ImagePrepper: ip} }, - } - } else if strings.HasPrefix(image, pkgutil.DaemonPrefix) { - return []func(pkgutil.ImagePrepper) pkgutil.Prepper{ - func(ip pkgutil.ImagePrepper) pkgutil.Prepper { return pkgutil.DaemonPrepper{ImagePrepper: ip} }, - } - } - // either has remote prefix or has no prefix, in which case we force remote - return []func(pkgutil.ImagePrepper) pkgutil.Prepper{ - func(ip pkgutil.ImagePrepper) pkgutil.Prepper { return pkgutil.CloudPrepper{ImagePrepper: ip} }, - } -} - func init() { RootCmd.AddCommand(analyzeCmd) addSharedFlags(analyzeCmd) diff --git a/cmd/diff.go b/cmd/diff.go index 6f9e8650..abe6214f 100644 --- a/cmd/diff.go +++ b/cmd/diff.go @@ -81,12 +81,12 @@ func diffImages(image1Arg, image2Arg string, diffArgs []string) error { go func(imageName string, imageMap map[string]*pkgutil.Image) { defer wg.Done() - pkgutil.SetPreppers(getPrepperForImage(imageName)) - 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.Warningf("Diff may be inaccurate: %s", err) diff --git a/cmd/root.go b/cmd/root.go index 07cbe87b..0096aa4d 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -24,7 +24,9 @@ import ( "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" "github.com/pkg/errors" @@ -38,6 +40,11 @@ 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", @@ -106,6 +113,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) } diff --git a/pkg/util/cloud_prepper.go b/pkg/util/cloud_prepper.go index d8e9ba63..ef50720b 100644 --- a/pkg/util/cloud_prepper.go +++ b/pkg/util/cloud_prepper.go @@ -17,50 +17,42 @@ limitations under the License. package util import ( - "strings" - "github.com/containers/image/docker" - "github.com/containers/image/docker/reference" + "github.com/docker/docker/client" ) -const RemotePrefix = "remote://" - // 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) RawSource() string { - return strings.Replace(p.Source, RemotePrefix, "", -1) +func (p CloudPrepper) GetSource() string { + return p.Source } -func (p CloudPrepper) SupportsImage() bool { - // will fail for strings prefixed with 'daemon://' - if strings.HasPrefix(p.Source, DaemonPrefix) || IsTar(p.Source) { - return false - } - _, err := reference.Parse(p.RawSource()) - return (err == nil) +func (p CloudPrepper) GetImage() (Image, error) { + return getImage(p) } func (p CloudPrepper) GetFileSystem() (string, error) { - ref, err := docker.ParseReference("//" + p.RawSource()) + ref, err := docker.ParseReference("//" + p.Source) if err != nil { return "", err } - return getFileSystemFromReference(ref, p.RawSource()) + return getFileSystemFromReference(ref, p.Source) } func (p CloudPrepper) GetConfig() (ConfigSchema, error) { - ref, err := docker.ParseReference("//" + p.RawSource()) + ref, err := docker.ParseReference("//" + p.Source) if err != nil { return ConfigSchema{}, err } - return getConfigFromReference(ref, p.RawSource()) + return getConfigFromReference(ref, p.Source) } diff --git a/pkg/util/daemon_prepper.go b/pkg/util/daemon_prepper.go index 762e7c85..8b01ff5f 100644 --- a/pkg/util/daemon_prepper.go +++ b/pkg/util/daemon_prepper.go @@ -18,58 +18,51 @@ package util import ( "context" - "strings" "github.com/containers/image/docker/daemon" - "github.com/containers/image/docker/reference" + "github.com/docker/docker/client" "github.com/golang/glog" ) -const DaemonPrefix = "daemon://" - type DaemonPrepper struct { - ImagePrepper + Source string + Client *client.Client } func (p DaemonPrepper) Name() string { return "Local Daemon" } -func (p DaemonPrepper) RawSource() string { - return strings.Replace(p.Source, DaemonPrefix, "", -1) +func (p DaemonPrepper) GetSource() string { + return p.Source } -func (p DaemonPrepper) SupportsImage() bool { - // will fail on strings prefixed with 'remote://' - if strings.HasPrefix(p.Source, RemotePrefix) || IsTar(p.Source) { - return false - } - _, err := reference.Parse(p.RawSource()) - return (err == nil) +func (p DaemonPrepper) GetImage() (Image, error) { + return getImage(p) } func (p DaemonPrepper) GetFileSystem() (string, error) { - ref, err := daemon.ParseReference(p.RawSource()) + ref, err := daemon.ParseReference(p.Source) if err != nil { return "", err } - return getFileSystemFromReference(ref, p.RawSource()) + return getFileSystemFromReference(ref, p.Source) } func (p DaemonPrepper) GetConfig() (ConfigSchema, error) { - ref, err := daemon.ParseReference(p.RawSource()) + ref, err := daemon.ParseReference(p.Source) if err != nil { return ConfigSchema{}, err } - return getConfigFromReference(ref, p.RawSource()) + return getConfigFromReference(ref, p.Source) } func (p DaemonPrepper) GetHistory() []ImageHistoryItem { - history, err := p.Client.ImageHistory(context.Background(), p.RawSource()) + history, err := p.Client.ImageHistory(context.Background(), p.Source) if err != nil { - glog.Error("Could not obtain image history for %s: %s", p.RawSource(), err) + glog.Error("Could not obtain image history for %s: %s", p.Source, err) } historyItems := []ImageHistoryItem{} for _, item := range history { diff --git a/pkg/util/image_prep_utils.go b/pkg/util/image_prep_utils.go index a1d2e42e..7aaf84eb 100644 --- a/pkg/util/image_prep_utils.go +++ b/pkg/util/image_prep_utils.go @@ -20,6 +20,7 @@ import ( "archive/tar" "encoding/json" "errors" + "fmt" "io/ioutil" "os" "path/filepath" @@ -30,14 +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} }, -} - -func SetPreppers(preppers []func(ImagePrepper) Prepper) { - orderedPreppers = preppers +type Prepper interface { + Name() string + GetConfig() (ConfigSchema, error) + GetFileSystem() (string, error) + GetImage() (Image, error) + GetSource() string } type Image struct { @@ -59,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)) diff --git a/pkg/util/image_prepper.go b/pkg/util/image_prepper.go deleted file mode 100644 index b3ff9c37..00000000 --- a/pkg/util/image_prepper.go +++ /dev/null @@ -1,72 +0,0 @@ -/* -Copyright 2017 Google, Inc. All rights reserved. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package util - -import ( - "errors" - - "github.com/docker/docker/client" - "github.com/golang/glog" -) - -type ImagePrepper struct { - Source string - Client *client.Client -} - -type Prepper interface { - Name() string - RawSource() string - GetFileSystem() (string, error) - GetConfig() (ConfigSchema, error) - SupportsImage() bool -} - -func (p ImagePrepper) GetImage() (Image, error) { - glog.Infof("Starting prep for image %s", p.Source) - img := p.Source - - var prepper Prepper - - for _, prepperConstructor := range orderedPreppers { - prepper = prepperConstructor(p) - if prepper.SupportsImage() { - break - } - } - - if prepper == nil { - return Image{}, errors.New("Could not retrieve image from source") - } - - imgPath, err := prepper.GetFileSystem() - if err != nil { - return Image{}, err - } - - config, err := prepper.GetConfig() - if err != nil { - glog.Error("Error retrieving History: ", err) - } - - glog.Infof("Finished prepping image %s", p.Source) - return Image{ - Source: img, - FSPath: imgPath, - Config: config, - }, nil -} diff --git a/pkg/util/tar_prepper.go b/pkg/util/tar_prepper.go index 8bd1ecf8..d156ccd7 100644 --- a/pkg/util/tar_prepper.go +++ b/pkg/util/tar_prepper.go @@ -25,23 +25,25 @@ import ( "strings" "github.com/containers/image/docker/tarfile" + "github.com/docker/docker/client" "github.com/golang/glog" ) type TarPrepper struct { - ImagePrepper + Source string + Client *client.Client } func (p TarPrepper) Name() string { return "Tar Archive" } -func (p TarPrepper) RawSource() string { +func (p TarPrepper) GetSource() string { return p.Source } -func (p TarPrepper) SupportsImage() bool { - return IsTar(p.Source) +func (p TarPrepper) GetImage() (Image, error) { + return getImage(p) } func (p TarPrepper) GetFileSystem() (string, error) {