From 8472efdbd1efcb4eea9872baf56e2473a0dd970f Mon Sep 17 00:00:00 2001 From: Valentin Rothberg Date: Tue, 8 Dec 2020 14:24:34 +0100 Subject: pkg/copy: add parsing API Add an API for parsing user input into a possibly specified container and path. This allows for sharing the parsing code between the local and the remote client (and bindings) in the future. Signed-off-by: Valentin Rothberg --- pkg/copy/parse.go | 61 +++++++++++++++++++++++++++++++++++ pkg/domain/infra/abi/cp.go | 60 ++++++++++++++-------------------- pkg/domain/infra/tunnel/containers.go | 3 +- 3 files changed, 88 insertions(+), 36 deletions(-) create mode 100644 pkg/copy/parse.go diff --git a/pkg/copy/parse.go b/pkg/copy/parse.go new file mode 100644 index 000000000..39e0e1547 --- /dev/null +++ b/pkg/copy/parse.go @@ -0,0 +1,61 @@ +package copy + +import ( + "strings" + + "github.com/pkg/errors" +) + +// ParseSourceAndDestination parses the source and destination input into a +// possibly specified container and path. The input format is described in +// podman-cp(1) as "[nameOrID:]path". Colons in paths are supported as long +// they start with a dot or slash. +// +// It returns, in order, the source container and path, followed by the +// destination container and path, and an error. Note that exactly one +// container must be specified. +func ParseSourceAndDestination(source, destination string) (string, string, string, string, error) { + sourceContainer, sourcePath := parseUserInput(source) + destContainer, destPath := parseUserInput(destination) + + numContainers := 0 + if len(sourceContainer) > 0 { + numContainers++ + } + if len(destContainer) > 0 { + numContainers++ + } + + if numContainers != 1 { + return "", "", "", "", errors.Errorf("invalid arguments %q, %q: exactly 1 container expected but %d specified", source, destination, numContainers) + } + + if len(sourcePath) == 0 || len(destPath) == 0 { + return "", "", "", "", errors.Errorf("invalid arguments %q, %q: you must specify paths", source, destination) + } + + return sourceContainer, sourcePath, destContainer, destPath, nil +} + +// parseUserInput parses the input string and returns, if specified, the name +// or ID of the container and the path. The input format is described in +// podman-cp(1) as "[nameOrID:]path". Colons in paths are supported as long +// they start with a dot or slash. +func parseUserInput(input string) (container string, path string) { + if len(input) == 0 { + return + } + path = input + + // If the input starts with a dot or slash, it cannot refer to a + // container. + if input[0] == '.' || input[0] == '/' { + return + } + + if spl := strings.SplitN(path, ":", 2); len(spl) == 2 { + container = spl[0] + path = spl[1] + } + return +} diff --git a/pkg/domain/infra/abi/cp.go b/pkg/domain/infra/abi/cp.go index 9409df743..7cc3005c1 100644 --- a/pkg/domain/infra/abi/cp.go +++ b/pkg/domain/infra/abi/cp.go @@ -2,46 +2,53 @@ package abi import ( "context" - "strings" "github.com/containers/podman/v2/libpod" "github.com/containers/podman/v2/pkg/copy" "github.com/containers/podman/v2/pkg/domain/entities" - "github.com/pkg/errors" ) func (ic *ContainerEngine) ContainerCp(ctx context.Context, source, dest string, options entities.ContainerCpOptions) error { - srcCtr, srcPath := parsePath(ic.Libpod, source) - destCtr, destPath := parsePath(ic.Libpod, dest) - - if srcCtr != nil && destCtr != nil { - return errors.Errorf("invalid arguments %q, %q: you must use just one container", source, dest) + // Parse user input. + sourceContainerStr, sourcePath, destContainerStr, destPath, err := copy.ParseSourceAndDestination(source, dest) + if err != nil { + return err } - if srcCtr == nil && destCtr == nil { - return errors.Errorf("invalid arguments %q, %q: you must specify one container", source, dest) + + // Look up containers. + var sourceContainer, destContainer *libpod.Container + if len(sourceContainerStr) > 0 { + sourceContainer, err = ic.Libpod.LookupContainer(sourceContainerStr) + if err != nil { + return err + } } - if len(srcPath) == 0 || len(destPath) == 0 { - return errors.Errorf("invalid arguments %q, %q: you must specify paths", source, dest) + if len(destContainerStr) > 0 { + destContainer, err = ic.Libpod.LookupContainer(destContainerStr) + if err != nil { + return err + } } var sourceItem, destinationItem copy.CopyItem - var err error - // Copy from the container to the host. - if srcCtr != nil { - sourceItem, err = copy.CopyItemForContainer(srcCtr, srcPath, options.Pause, true) + + // Source ... container OR host. + if sourceContainer != nil { + sourceItem, err = copy.CopyItemForContainer(sourceContainer, sourcePath, options.Pause, true) defer sourceItem.CleanUp() if err != nil { return err } } else { - sourceItem, err = copy.CopyItemForHost(srcPath, true) + sourceItem, err = copy.CopyItemForHost(sourcePath, true) if err != nil { return err } } - if destCtr != nil { - destinationItem, err = copy.CopyItemForContainer(destCtr, destPath, options.Pause, false) + // Destination ... container OR host. + if destContainer != nil { + destinationItem, err = copy.CopyItemForContainer(destContainer, destPath, options.Pause, false) defer destinationItem.CleanUp() if err != nil { return err @@ -57,20 +64,3 @@ func (ic *ContainerEngine) ContainerCp(ctx context.Context, source, dest string, // Copy from the host to the container. return copy.Copy(&sourceItem, &destinationItem, options.Extract) } - -func parsePath(runtime *libpod.Runtime, path string) (*libpod.Container, string) { - if len(path) == 0 { - return nil, "" - } - if path[0] == '.' || path[0] == '/' { // A path cannot point to a container. - return nil, path - } - pathArr := strings.SplitN(path, ":", 2) - if len(pathArr) == 2 { - ctr, err := runtime.LookupContainer(pathArr[0]) - if err == nil { - return ctr, pathArr[1] - } - } - return nil, path -} diff --git a/pkg/domain/infra/tunnel/containers.go b/pkg/domain/infra/tunnel/containers.go index 3584668c7..e65fef0a4 100644 --- a/pkg/domain/infra/tunnel/containers.go +++ b/pkg/domain/infra/tunnel/containers.go @@ -732,7 +732,8 @@ func (ic *ContainerEngine) ContainerPort(ctx context.Context, nameOrID string, o } func (ic *ContainerEngine) ContainerCp(ctx context.Context, source, dest string, options entities.ContainerCpOptions) error { - return errors.New("not implemented") + return nil + // return containers.Copy(ic.ClientCxt, source, dest, options) } // Shutdown Libpod engine -- cgit v1.2.3-54-g00ecf From c2a5011c0d65cfd6c1ab5d6eef9778551ed56860 Mon Sep 17 00:00:00 2001 From: Valentin Rothberg Date: Tue, 8 Dec 2020 16:18:49 +0100 Subject: archive: move stat-header handling into copy package Move handling the stat header into `pkg/copy`. All copy-related should ideally be located in this package to increase locality and reduce scattering where possible. Signed-off-by: Valentin Rothberg --- pkg/api/handlers/compat/containers_archive.go | 45 +-------------------- pkg/copy/copy.go | 1 - pkg/copy/fileinfo.go | 56 +++++++++++++++++++++++++++ pkg/copy/item.go | 13 ------- 4 files changed, 58 insertions(+), 57 deletions(-) create mode 100644 pkg/copy/fileinfo.go diff --git a/pkg/api/handlers/compat/containers_archive.go b/pkg/api/handlers/compat/containers_archive.go index 223eb2cd5..bb84f86be 100644 --- a/pkg/api/handlers/compat/containers_archive.go +++ b/pkg/api/handlers/compat/containers_archive.go @@ -1,13 +1,8 @@ package compat import ( - "bytes" - "encoding/base64" - "encoding/json" "fmt" "net/http" - "os" - "time" "github.com/containers/podman/v2/libpod" "github.com/containers/podman/v2/libpod/define" @@ -71,12 +66,12 @@ func handleHeadAndGet(w http.ResponseWriter, r *http.Request, decoder *schema.De utils.Error(w, "Not found.", http.StatusNotFound, errors.Wrapf(err, "error stating container path %q", query.Path)) return } - statHeader, err := fileInfoToDockerStats(info) + statHeader, err := copy.EncodeFileInfo(info) if err != nil { utils.Error(w, "Something went wrong", http.StatusInternalServerError, err) return } - w.Header().Add("X-Docker-Container-Path-Stat", statHeader) + w.Header().Add(copy.XDockerContainerPathStatHeader, statHeader) // Our work is done when the user is interested in the header only. if r.Method == http.MethodHead { @@ -98,42 +93,6 @@ func handleHeadAndGet(w http.ResponseWriter, r *http.Request, decoder *schema.De } } -func fileInfoToDockerStats(info *copy.FileInfo) (string, error) { - dockerStats := struct { - Name string `json:"name"` - Size int64 `json:"size"` - Mode os.FileMode `json:"mode"` - ModTime time.Time `json:"mtime"` - LinkTarget string `json:"linkTarget"` - }{ - Name: info.Name, - Size: info.Size, - Mode: info.Mode, - ModTime: info.ModTime, - LinkTarget: info.LinkTarget, - } - - jsonBytes, err := json.Marshal(&dockerStats) - if err != nil { - return "", errors.Wrap(err, "failed to serialize file stats") - } - - buff := bytes.NewBuffer(make([]byte, 0, 128)) - base64encoder := base64.NewEncoder(base64.StdEncoding, buff) - - _, err = base64encoder.Write(jsonBytes) - if err != nil { - return "", err - } - - err = base64encoder.Close() - if err != nil { - return "", err - } - - return buff.String(), nil -} - func handlePut(w http.ResponseWriter, r *http.Request, decoder *schema.Decoder, runtime *libpod.Runtime) { query := struct { Path string `schema:"path"` diff --git a/pkg/copy/copy.go b/pkg/copy/copy.go index 0e68eb450..3993b532e 100644 --- a/pkg/copy/copy.go +++ b/pkg/copy/copy.go @@ -114,7 +114,6 @@ func enforceCopyRules(source, destination *CopyItem) error { return nil } - // Source is a *stream*. if source.info.IsStream { if !(destination.info.IsDir || destination.info.IsStream) { return errors.New("destination must be a directory or stream when copying from a stream") diff --git a/pkg/copy/fileinfo.go b/pkg/copy/fileinfo.go new file mode 100644 index 000000000..08b4eb377 --- /dev/null +++ b/pkg/copy/fileinfo.go @@ -0,0 +1,56 @@ +package copy + +import ( + "encoding/base64" + "encoding/json" + "net/http" + "os" + "strings" + "time" + + "github.com/pkg/errors" +) + +// XDockerContainerPathStatHeader is the *key* in http headers pointing to the +// base64 encoded JSON payload of stating a path in a container. +const XDockerContainerPathStatHeader = "X-Docker-Container-Path-Stat" + +// FileInfo describes a file or directory and is returned by +// (*CopyItem).Stat(). +type FileInfo struct { + Name string `json:"name"` + Size int64 `json:"size"` + Mode os.FileMode `json:"mode"` + ModTime time.Time `json:"mtime"` + IsDir bool `json:"isDir"` + IsStream bool `json:"isStream"` + LinkTarget string `json:"linkTarget"` +} + +// EncodeFileInfo serializes the specified FileInfo as a base64 encoded JSON +// payload. Intended for Docker compat. +func EncodeFileInfo(info *FileInfo) (string, error) { + buf, err := json.Marshal(&info) + if err != nil { + return "", errors.Wrap(err, "failed to serialize file stats") + } + return base64.URLEncoding.EncodeToString(buf), nil +} + +// ExtractFileInfoFromHeader extracts a base64 encoded JSON payload of a +// FileInfo in the http header. If no such header entry is found, nil is +// returned. Intended for Docker compat. +func ExtractFileInfoFromHeader(header *http.Header) (*FileInfo, error) { + rawData := header.Get(XDockerContainerPathStatHeader) + if len(rawData) == 0 { + return nil, nil + } + + info := FileInfo{} + base64Decoder := base64.NewDecoder(base64.URLEncoding, strings.NewReader(rawData)) + if err := json.NewDecoder(base64Decoder).Decode(&info); err != nil { + return nil, err + } + + return &info, nil +} diff --git a/pkg/copy/item.go b/pkg/copy/item.go index db6bca610..df8bf30b9 100644 --- a/pkg/copy/item.go +++ b/pkg/copy/item.go @@ -5,7 +5,6 @@ import ( "os" "path/filepath" "strings" - "time" buildahCopiah "github.com/containers/buildah/copier" "github.com/containers/buildah/pkg/chrootuser" @@ -75,18 +74,6 @@ type CopyItem struct { // deferFunc allows for returning functions that must be deferred at call sites. type deferFunc func() -// FileInfo describes a file or directory and is returned by -// (*CopyItem).Stat(). -type FileInfo struct { - Name string `json:"name"` - Size int64 `json:"size"` - Mode os.FileMode `json:"mode"` - ModTime time.Time `json:"mtime"` - IsDir bool `json:"isDir"` - IsStream bool `json:"isStream"` - LinkTarget string `json:"linkTarget"` -} - // Stat returns the FileInfo. func (item *CopyItem) Stat() (*FileInfo, error) { return &item.info, item.statError -- cgit v1.2.3-54-g00ecf From a12323884fd576063ba8f5785a3c9c2e48140c7f Mon Sep 17 00:00:00 2001 From: Valentin Rothberg Date: Wed, 9 Dec 2020 12:18:14 +0100 Subject: pkg/copy: introduce a Copier Introduce a `Copier` object to separate the copy-rule enforcement from copying. That allows for a better error reporting of the REST API. Signed-off-by: Valentin Rothberg --- pkg/api/handlers/compat/containers_archive.go | 19 ++++++-- pkg/copy/copy.go | 67 ++++++++++++++++++++------- pkg/domain/infra/abi/cp.go | 6 ++- 3 files changed, 70 insertions(+), 22 deletions(-) diff --git a/pkg/api/handlers/compat/containers_archive.go b/pkg/api/handlers/compat/containers_archive.go index bb84f86be..d8197415c 100644 --- a/pkg/api/handlers/compat/containers_archive.go +++ b/pkg/api/handlers/compat/containers_archive.go @@ -10,6 +10,7 @@ import ( "github.com/containers/podman/v2/pkg/copy" "github.com/gorilla/schema" "github.com/pkg/errors" + "github.com/sirupsen/logrus" ) func Archive(w http.ResponseWriter, r *http.Request) { @@ -86,11 +87,16 @@ func handleHeadAndGet(w http.ResponseWriter, r *http.Request, decoder *schema.De return } - w.WriteHeader(http.StatusOK) - if err := copy.Copy(&source, &destination, false); err != nil { + copier, err := copy.GetCopier(&source, &destination, false) + if err != nil { utils.Error(w, "Something went wrong", http.StatusInternalServerError, err) return } + w.WriteHeader(http.StatusOK) + if err := copier.Copy(); err != nil { + logrus.Errorf("Error during copy: %v", err) + return + } } func handlePut(w http.ResponseWriter, r *http.Request, decoder *schema.Decoder, runtime *libpod.Runtime) { @@ -129,9 +135,14 @@ func handlePut(w http.ResponseWriter, r *http.Request, decoder *schema.Decoder, return } - w.WriteHeader(http.StatusOK) - if err := copy.Copy(&source, &destination, false); err != nil { + copier, err := copy.GetCopier(&source, &destination, false) + if err != nil { utils.Error(w, "Something went wrong", http.StatusInternalServerError, err) return } + w.WriteHeader(http.StatusOK) + if err := copier.Copy(); err != nil { + logrus.Errorf("Error during copy: %v", err) + return + } } diff --git a/pkg/copy/copy.go b/pkg/copy/copy.go index 3993b532e..13893deb2 100644 --- a/pkg/copy/copy.go +++ b/pkg/copy/copy.go @@ -25,31 +25,61 @@ import ( // // **************************************************************************** -// Copy the source item to destination. Use extract to untar the source if -// it's a tar archive. -func Copy(source *CopyItem, destination *CopyItem, extract bool) error { +// Copier copies data from a source to a destination CopyItem. +type Copier struct { + copyFunc func() error + cleanUpFuncs []deferFunc +} + +// cleanUp releases resources the Copier may hold open. +func (c *Copier) cleanUp() { + for _, f := range c.cleanUpFuncs { + f() + } +} + +// Copy data from a source to a destination CopyItem. +func (c *Copier) Copy() error { + defer c.cleanUp() + return c.copyFunc() +} + +// GetCopiers returns a Copier to copy the source item to destination. Use +// extract to untar the source if it's a tar archive. +func GetCopier(source *CopyItem, destination *CopyItem, extract bool) (*Copier, error) { + copier := &Copier{} + // First, do the man-page dance. See podman-cp(1) for details. if err := enforceCopyRules(source, destination); err != nil { - return err + return nil, err } // Destination is a stream (e.g., stdout or an http body). if destination.info.IsStream { // Source is a stream (e.g., stdin or an http body). if source.info.IsStream { - _, err := io.Copy(destination.writer, source.reader) - return err + copier.copyFunc = func() error { + _, err := io.Copy(destination.writer, source.reader) + return err + } + return copier, nil } root, glob, err := source.buildahGlobs() if err != nil { - return err + return nil, err } - return buildahCopiah.Get(root, "", source.getOptions(), []string{glob}, destination.writer) + copier.copyFunc = func() error { + return buildahCopiah.Get(root, "", source.getOptions(), []string{glob}, destination.writer) + } + return copier, nil } // Destination is either a file or a directory. if source.info.IsStream { - return buildahCopiah.Put(destination.root, destination.resolved, source.putOptions(), source.reader) + copier.copyFunc = func() error { + return buildahCopiah.Put(destination.root, destination.resolved, source.putOptions(), source.reader) + } + return copier, nil } tarOptions := &archive.TarOptions{ @@ -71,33 +101,36 @@ func Copy(source *CopyItem, destination *CopyItem, extract bool) error { var tarReader io.ReadCloser if extract && archive.IsArchivePath(source.resolved) { if !destination.info.IsDir { - return errors.Errorf("cannot extract archive %q to file %q", source.original, destination.original) + return nil, errors.Errorf("cannot extract archive %q to file %q", source.original, destination.original) } reader, err := os.Open(source.resolved) if err != nil { - return err + return nil, err } - defer reader.Close() + copier.cleanUpFuncs = append(copier.cleanUpFuncs, func() { reader.Close() }) // The stream from stdin may be compressed (e.g., via gzip). decompressedStream, err := archive.DecompressStream(reader) if err != nil { - return err + return nil, err } - defer decompressedStream.Close() + copier.cleanUpFuncs = append(copier.cleanUpFuncs, func() { decompressedStream.Close() }) tarReader = decompressedStream } else { reader, err := archive.TarWithOptions(source.resolved, tarOptions) if err != nil { - return err + return nil, err } - defer reader.Close() + copier.cleanUpFuncs = append(copier.cleanUpFuncs, func() { reader.Close() }) tarReader = reader } - return buildahCopiah.Put(root, dir, source.putOptions(), tarReader) + copier.copyFunc = func() error { + return buildahCopiah.Put(root, dir, source.putOptions(), tarReader) + } + return copier, nil } // enforceCopyRules enforces the rules for copying from a source to a diff --git a/pkg/domain/infra/abi/cp.go b/pkg/domain/infra/abi/cp.go index 7cc3005c1..362053cce 100644 --- a/pkg/domain/infra/abi/cp.go +++ b/pkg/domain/infra/abi/cp.go @@ -62,5 +62,9 @@ func (ic *ContainerEngine) ContainerCp(ctx context.Context, source, dest string, } // Copy from the host to the container. - return copy.Copy(&sourceItem, &destinationItem, options.Extract) + copier, err := copy.GetCopier(&sourceItem, &destinationItem, options.Extract) + if err != nil { + return err + } + return copier.Copy() } -- cgit v1.2.3-54-g00ecf