This change changes the default for noOverwriteDirNonDir to be true
internally, with the intent to change the default at the API to follow
accordingly.
The `AllowOverwriteDirWithFile` option in the Client was added when
reimplementing the CLI using the API Client lib in [moby@1b2b91b].
Before that refactor, the `noOverwriteDirNonDir` query argument
[would be set unconditionally][1] by the CLI, with no options to
control the behavior.
The `noOverwriteDirNonDir` query parameter was added in [moby@db9cc91] to
set the `NoOverwriteDirNonDir` option that was implemented in pkg/archive
in [moby@a74799b].
It was added in [PR13171-comment2], following a discussion on the risk
of replacing a directory with a file and vice-versa in [PR13171-comment].
> In my latest changes from yesterday:
>
> - Removed the `GET stat-path` endpoint and added a `HEAD` handler to
> the `archive-path` endpoint. Updated the api docs to reflect this.
> Also moved api docs changes from `v1.19` to `v1.20`.
> - Added a `NoOverwriteDirNonDir` flag to `archive.TarOptions` to indicate
> that we do not want to overwrite a directory with a non-directory (and
> vice versa) when unpacking an archive.
> - Added a corresponding but optional `noOverwriteDirNonDir` parameter
> to the `PUT extract-to-dir` endpoint to specify desired behavior.
>
> These changes combine to keep the behavior we want
It's unclear why these were added as an *option* and why it was implemented
as opt-in (not opt-out), as overwriting a file with a directory (or vice-versa)
would generally be unexpected behavior.
[1]: 8c9ad7b818/api/client/cp.go (L345-L346)
[moby@1b2b91b]: 1b2b91ba43
[moby@a74799b]: a74799b701
[moby@db9cc91]: db9cc91a9e
[PR13171-comment]: https://github.com/moby/moby/pull/13171#issuecomment-106559765
[PR13171-comment2]: https://github.com/moby/moby/pull/13171#issuecomment-108538643
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
These comments were added to enforce using the correct import path for
our packages ("github.com/docker/docker", not "github.com/moby/moby").
However, when working in go module mode (not GOPATH / vendor), they have
no effect, so their impact is limited.
Remove these imports in preparation of migrating our code to become an
actual go module.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
- remove named error-returns
- make error-handling slightly more idiomatic (check for non-nil errors)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Commit ebcb7d6b40 removed code that string-
matched errors to return the appropriate error-type. As part of that, it
defaulted to using a "System" (status 500) error.
We need to verify codepaths used by the archive functions, but let's
start with detecting invalid parameter errors, and preserve those.
This patch updates the code backing the following endpoints to return
a 400 (Invalid Parameter) error when present;
- `HEAD /containers/{name:.*}/archive`
- `GET /containers/{name:.*}/archive`
- `PUT /containers/{name:.*}/archive`
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This endpoint was deprecated in API v1.20 (Docker Engine v1.8.0) in
commit db9cc91a9e, in favor of the
`PUT /containers/{id}/archive` and `HEAD /containers/{id}/archive`
endpoints, and disabled in API v1.24 (Docker Engine v1.12.0) through
commit 428328908d.
This patch removes the endpoint, and the associated `daemon.ContainerCopy`
method in the backend.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
It is only applicable to Windows so it does not need to be called from
platform-generic code. Fix locking in the Windows implementation.
Signed-off-by: Cory Snider <csnider@mirantis.com>
The Linux implementation needs to diverge significantly from the Windows
one in order to fix platform-specific bugs. Cut the generic
implementation out of daemon/archive.go and paste identical, verbatim
copies of that implementation into daemon/archive_{windows,linux}.go to
make it easier to compare the progression of changes to the respective
implementations through Git history.
Signed-off-by: Cory Snider <csnider@mirantis.com>
It was only used in a single location, and the ErrExtractPointNotDirectory was
not checked for, or used as a sentinel error.
This error was introduced in c32dde5baa. It was
never used as a sentinel error, but from that commit, it looks like it was added
as a package variable to mirror already existing errors defined at the package
level.
This patch removes the exported variable, and replaces the error with an
errdefs.InvalidParameter(), so that the API also returns the correct (400)
status code.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
It was only used in a single location, and the ErrRootFSReadOnly was not checked
for, or used as a sentinel error.
This error was introduced in c32dde5baa, originally
named `ErrContainerRootfsReadonly`. It was never used as a sentinel error, but
from that commit, it looks like it was added as a package variable to mirror
the coding style of already existing errors defined at the package level.
This patch removes the exported variable, and replaces the error with an
errdefs.InvalidParameter(), so that the API also returns the correct (400)
status code.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Now that the type of Container.BaseFS has been reverted to a string,
values can never implement the extractor or archiver interfaces. Rip out
the dead code to support archiving and unarchiving through those
interfcaes.
Signed-off-by: Cory Snider <csnider@mirantis.com>
The Driver abstraction was needed for Linux Containers on Windows,
support for which has since been removed.
There is no direct equivalent to Lchmod() in the standard library so
continue to use the containerd/continuity version.
Signed-off-by: Cory Snider <csnider@mirantis.com>
Before 7a7357da, archive.TarResourceRebase was being used to copy files
and folders from the container. That function splits the source path
into a dirname + basename pair to support copying a file:
if you wanted to tar `dir/file` it would tar from `dir` the file `file`
(as part of the IncludedFiles option).
However, that path splitting logic was kept for folders as well, which
resulted in weird inputs to archive.TarWithOptions:
if you wanted to tar `dir1/dir2` it would tar from `dir1` the directory
`dir2` (as part of IncludedFiles option).
Although it was weird, it worked fine until we started chrooting into
the container rootfs when doing a `docker cp` with container source set
to `/` (cf 3029e765).
The fix is to only do the path splitting logic if the source is a file.
Unfortunately, 7a7357da added support for LCOW by duplicating some of
this subtle logic. Ideally we would need to do more refactoring of the
archive codebase to properly encapsulate these behaviors behind well-
documented APIs.
This fix does not do that. Instead, it fixes the issue inline.
Signed-off-by: Tibor Vass <tibor@docker.com>
Previously only unpack operations were supported with chroot.
This adds chroot support for packing operations.
This prevents potential breakouts when copying data from a container.
Signed-off-by: Brian Goff <cpuguy83@gmail.com>
This is useful for preventing CVE-2018-15664 where a malicious container
process can take advantage of a race on symlink resolution/sanitization.
Before this change chrootarchive would chroot to the destination
directory which is attacker controlled. With this patch we always chroot
to the container's root which is not attacker controlled.
Signed-off-by: Brian Goff <cpuguy83@gmail.com>
Instead of having to create a bunch of custom error types that are doing
nothing but wrapping another error in sub-packages, use a common helper
to create errors of the requested type.
e.g. instead of re-implementing this over and over:
```go
type notFoundError struct {
cause error
}
func(e notFoundError) Error() string {
return e.cause.Error()
}
func(e notFoundError) NotFound() {}
func(e notFoundError) Cause() error {
return e.cause
}
```
Packages can instead just do:
```
errdefs.NotFound(err)
```
Signed-off-by: Brian Goff <cpuguy83@gmail.com>
This enables docker cp and ADD/COPY docker build support for LCOW.
Originally, the graphdriver.Get() interface returned a local path
to the container root filesystem. This does not work for LCOW, so
the Get() method now returns an interface that LCOW implements to
support copying to and from the container.
Signed-off-by: Akash Gupta <akagup@microsoft.com>
Use strongly typed errors to set HTTP status codes.
Error interfaces are defined in the api/errors package and errors
returned from controllers are checked against these interfaces.
Errors can be wraeped in a pkg/errors.Causer, as long as somewhere in the
line of causes one of the interfaces is implemented. The special error
interfaces take precedence over Causer, meaning if both Causer and one
of the new error interfaces are implemented, the Causer is not
traversed.
Signed-off-by: Brian Goff <cpuguy83@gmail.com>
Commit the rwLayer to get the correct DiffID
Refacator copy in thebuilder
move more code into exportImage
cleanup some windows tests
Release the newly commited layer.
Set the imageID on the buildStage after exporting a new image.
Move archiver to BuildManager.
Have ReleaseableLayer.Commit return a layer
and store the Image from exportImage in the local imageSources cache
Remove NewChild from image interface.
Signed-off-by: Daniel Nephin <dnephin@docker.com>
Add CreateImage() to the daemon
Refactor daemon.Comit() and expose a Image.NewChild()
Update copy to use IDMappings.
Signed-off-by: Daniel Nephin <dnephin@docker.com>
There is no case which would resolve in this error. The root user always exists, and if the id maps are empty, the default value of 0 is correct.
Signed-off-by: Daniel Nephin <dnephin@docker.com>
Removes 3 methods from the builder.Backend interface
Remove the coupling between imageContexts, imageMounts and the builder.
Signed-off-by: Daniel Nephin <dnephin@docker.com>
Redefine a better interface for remote context dependency.
Separate Dockerfile build instruction from remote context.
Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
This changes the long-standing bug of copy operations not preserving the
UID/GID information after the files arrive to the container.
Signed-off-by: Erik Hollensbe <github@hollensbe.org>
Fixes#22564
When an error occurs on mount, there should not be any call later to
unmount. This can throw off refcounting in the underlying driver
unexpectedly.
Consider these two cases:
```
$ docker run -v foo:/bar busybox true
```
```
$ docker run -v foo:/bar -w /foo busybox true
```
In the first case, if mounting `foo` fails, the volume driver will not
get a call to unmount (this is the incorrect behavior).
In the second case, the volume driver will not get a call to unmount
(correct behavior).
This occurs because in the first case, `/bar` does not exist in the
container, and as such there is no call to `volume.Mount()` during the
`create` phase. It will error out during the `start` phase.
In the second case `/bar` is created before dealing with the volume
because of the `-w`. Because of this, when the volume is being setup
docker will try to copy the image path contents in the volume, in which
case it will attempt to mount the volume and fail. This happens during
the `create` phase. This makes it so the container will not be created
(or at least fully created) and the user gets the error on `create`
instead of `start`. The error handling is different in these two phases.
Changed to only send `unmount` if the volume is mounted.
While investigating the cause of the reported issue I found some odd
behavior in unmount calls so I've cleaned those up a bit here as well.
Signed-off-by: Brian Goff <cpuguy83@gmail.com>
This corrects `docker cp` behavior when user namespaces are enabled.
Instead of chown'ing copied-in files to real root (0,0), the code
queries for the remapped root uid & gid and sets the chown option
properly.
Docker-DCO-1.1-Signed-off-by: Phil Estes <estesp@linux.vnet.ibm.com> (github: estesp)
Currently, daemonbuilder package (part of daemon) implemented the
builder backend. However, it was a very thin wrapper around daemon
methods and caused an implementation dependency for api/server build
endpoint. api/server buildrouter should only know about the backend
implementing the /build API endpoint.
Removing daemonbuilder involved moving build specific methods to
respective files in the daemon, where they fit naturally.
Signed-off-by: Anusha Ragunathan <anusha@docker.com>
So other packages don't need to import the daemon package when they
want to use this struct.
Signed-off-by: David Calavera <david.calavera@gmail.com>
Signed-off-by: Tibor Vass <tibor@docker.com>
