Commit graph

427 commits

Author SHA1 Message Date
Tibor Vass
29de017df7
Merge pull request #38599 from thaJeztah/builder_fix_copy_permissions
builder: fix `COPY --from` should preserve ownership
2019-03-22 09:38:13 -07:00
John Howard
20833b06a0 Windows: (WCOW) Generate OCI spec that remote runtime can escape
Signed-off-by: John Howard <jhoward@microsoft.com>

Also fixes https://github.com/moby/moby/issues/22874

This commit is a pre-requisite to moving moby/moby on Windows to using
Containerd for its runtime.

The reason for this is that the interface between moby and containerd
for the runtime is an OCI spec which must be unambigious.

It is the responsibility of the runtime (runhcs in the case of
containerd on Windows) to ensure that arguments are escaped prior
to calling into HCS and onwards to the Win32 CreateProcess call.

Previously, the builder was always escaping arguments which has
led to several bugs in moby. Because the local runtime in
libcontainerd had context of whether or not arguments were escaped,
it was possible to hack around in daemon/oci_windows.go with
knowledge of the context of the call (from builder or not).

With a remote runtime, this is not possible as there's rightly
no context of the caller passed across in the OCI spec. Put another
way, as I put above, the OCI spec must be unambigious.

The other previous limitation (which leads to various subtle bugs)
is that moby is coded entirely from a Linux-centric point of view.

Unfortunately, Windows != Linux. Windows CreateProcess uses a
command line, not an array of arguments. And it has very specific
rules about how to escape a command line. Some interesting reading
links about this are:

https://blogs.msdn.microsoft.com/twistylittlepassagesallalike/2011/04/23/everyone-quotes-command-line-arguments-the-wrong-way/
https://stackoverflow.com/questions/31838469/how-do-i-convert-argv-to-lpcommandline-parameter-of-createprocess
https://docs.microsoft.com/en-us/cpp/cpp/parsing-cpp-command-line-arguments?view=vs-2017

For this reason, the OCI spec has recently been updated to cater
for more natural syntax by including a CommandLine option in
Process.

What does this commit do?

Primary objective is to ensure that the built OCI spec is unambigious.

It changes the builder so that `ArgsEscaped` as commited in a
layer is only controlled by the use of CMD or ENTRYPOINT.

Subsequently, when calling in to create a container from the builder,
if follows a different path to both `docker run` and `docker create`
using the added `ContainerCreateIgnoreImagesArgsEscaped`. This allows
a RUN from the builder to control how to escape in the OCI spec.

It changes the builder so that when shell form is used for RUN,
CMD or ENTRYPOINT, it builds (for WCOW) a more natural command line
using the original as put by the user in the dockerfile, not
the parsed version as a set of args which loses fidelity.
This command line is put into args[0] and `ArgsEscaped` is set
to true for CMD or ENTRYPOINT. A RUN statement does not commit
`ArgsEscaped` to the commited layer regardless or whether shell
or exec form were used.
2019-03-12 18:41:55 -07:00
Sebastiaan van Stijn
6d87f19142
builder: fix COPY --from should preserve ownership
When copying between stages, or copying from an image,
ownership of the copied files should not be changed, unless
the `--chown` option is set (in which case ownership of copied
files should be updated to the specified user/group).

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2019-03-13 00:55:04 +01:00
John Howard
63f9c7784b LCOW: Log stderr on failures
Signed-off-by: John Howard <jhoward@microsoft.com>
2018-09-26 13:23:04 -07:00
mooncake
5160003b4d fix typos
Signed-off-by: mooncake <xcoder@tenxcloud.com>
Signed-off-by: Bily Zhang <xcoder@tenxcloud.com>
2018-09-01 21:26:38 +08:00
Yong Tang
14d5569f19
Merge pull request #37563 from Microsoft/jjh/fix-vso17531561v2
LCOW: Ensure platform is populated on COPY/ADD
2018-08-17 12:51:17 -07:00
Salahuddin Khan
763d839261 Add ADD/COPY --chown flag support to Windows
This implements chown support on Windows. Built-in accounts as well
as accounts included in the SAM database of the container are supported.

NOTE: IDPair is now named Identity and IDMappings is now named
IdentityMapping.

The following are valid examples:
ADD --chown=Guest . <some directory>
COPY --chown=Administrator . <some directory>
COPY --chown=Guests . <some directory>
COPY --chown=ContainerUser . <some directory>

On Windows an owner is only granted the permission to read the security
descriptor and read/write the discretionary access control list. This
fix also grants read/write and execute permissions to the owner.

Signed-off-by: Salahuddin Khan <salah@docker.com>
2018-08-13 21:59:11 -07:00
John Howard
ae6b5828ce LCOW: Ensure platform is populated on COPY/ADD
Signed-off-by: John Howard <jhoward@microsoft.com>
2018-07-30 12:49:33 -07:00
Tibor Vass
9ebed53c54
Merge pull request #37439 from tiborvass/vendor-buildkit
Set BuildKit's ExportedProduct variable to show useful errors in the future
2018-07-17 12:10:23 -07:00
Tibor Vass
81599222fc validate: please vet
Signed-off-by: Tibor Vass <tibor@docker.com>
2018-07-16 21:41:54 +00:00
Yong Tang
492545e139
Merge pull request #37396 from yui-knk/error_when_base_name_resolved_to_blank
Return error if basename is expanded to blank
2018-07-15 15:42:57 -07:00
Yuichiro Kaneko
c9542d313e Return error if basename is expanded to blank
Fix: https://github.com/moby/moby/issues/37325

Signed-off-by: Yuichiro Kaneko <spiketeika@gmail.com>
2018-07-15 22:29:03 +09:00
Yuichiro Kaneko
44e08d8a7d Ensure RUN instruction to run without Healthcheck
Before this commit Healthcheck run if HEALTHCHECK
instruction appears before RUN instruction.
By passing `withoutHealthcheck` to `copyRunConfig`,
always RUN instruction run without Healthcheck.

Fix: https://github.com/moby/moby/issues/37362

Signed-off-by: Yuichiro Kaneko <spiketeika@gmail.com>
2018-07-08 16:49:04 +09:00
Tibor Vass
facad55744 api: Change Platform field back to string (temporary workaround)
This partially reverts https://github.com/moby/moby/pull/37350

Although specs.Platform is desirable in the API, there is more work
to be done on helper functions, namely containerd's platforms.Parse
that assumes the default platform of the Go runtime.

That prevents a client to use the recommended Parse function to
retrieve a specs.Platform object.

With this change, no parsing is expected from the client.

Signed-off-by: Tibor Vass <tibor@docker.com>
2018-07-03 22:33:42 +00:00
Sebastiaan van Stijn
1da7d2eebf
Merge pull request #37368 from tiborvass/buildkit-iidfile
builder: return image ID in API when using buildkit
2018-07-04 00:20:25 +02:00
Tibor Vass
ca8022ec63 builder: return image ID in API when using buildkit
Signed-off-by: Tibor Vass <tibor@docker.com>
2018-07-03 19:11:02 +00:00
Yuichiro Kaneko
9b374801ac Update documents of dispatchAdd
`ADD` does not support git.

Ref: https://github.com/moby/moby/issues/14704#issuecomment-298134333

Signed-off-by: Yuichiro Kaneko <spiketeika@gmail.com>
2018-07-03 08:51:07 +09:00
Tonis Tiigi
f099771665 system: add back lcow validation function
Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
2018-06-27 15:24:26 -07:00
Tonis Tiigi
337ba71fc1 distribution: fix passing platform struct to puller
Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
2018-06-27 14:59:31 -07:00
Tonis Tiigi
81f862a1fe api: fix platform type
Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
2018-06-26 22:59:17 -07:00
Tonis Tiigi
48b93419dc builder: buildkit rebase update
Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
2018-06-26 22:59:17 -07:00
Dennis Chen
7f334d3acf Initial support for OCI multi-platform image
Add the OCI spec compatible image support in client side.

Signed-off-by: Dennis Chen <dennis.chen@arm.com>
2018-06-26 11:39:29 -07:00
John Howard
35193c0e7d LCOW: Auto-select OS
Signed-off-by: John Howard <jhoward@microsoft.com>

Addresses https://github.com/moby/moby/pull/35089#issuecomment-367802698.
This change enables the daemon to automatically select an image under LCOW
that can be used if the API doesn't specify an explicit platform.

For example:

FROM supertest2014/nyan
ADD Dockerfile /

And docker build . will download the linux image (not a multi-manifest image)

And similarly docker pull ubuntu will match linux/amd64
2018-06-26 11:37:43 -07:00
Vincent Demeester
3845728524
Update tests to use gotest.tools 👼
Signed-off-by: Vincent Demeester <vincent@sbr.pm>
2018-06-13 09:04:30 +02:00
Sebastiaan van Stijn
52ea99eda2
Merge pull request #37189 from arm64b/create-intermediate-for-COPY-ADD
Refactor and cleanup the intermediate container creation
2018-06-07 21:20:17 -07:00
Dennis Chen
7f280f6f65 Refactor and cleanup the intermediate container creation
This PR is trying to refactor the `probeAndCreate` and cleanup
related codes based on the refactoring.

Signed-off-by: Dennis Chen <dennis.chen@arm.com>
2018-06-04 12:55:49 +08:00
Tonis Tiigi
c9ebd2f13b vendor: use dockerfile parser from buildkit
Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
2018-06-02 11:10:34 -07:00
Vincent Demeester
3e494c4b93
Merge pull request #37178 from tonistiigi/layer-leak
builder: fix layer leak on multi-stage wildcard copy
2018-05-31 09:39:28 +02:00
Tonis Tiigi
4a18c11bdc builder: fix layer leak on multi-stage wildcard copy
Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
2018-05-30 15:18:40 -07:00
Sebastiaan van Stijn
2628896b5e
Handle multi-digit positional parameters
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2018-05-25 18:51:11 +02:00
Sebastiaan van Stijn
9654e9b6f8
Add detection of "special parameters" for substitution
Detect Special parameters as defined in
http://pubs.opengroup.org/onlinepubs/009695399/utilities/xcu_chap02.html#tag_02_05_02

Treat these as parameters that are not set, instead of
producing an error that a modifier is missing.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2018-05-25 18:20:22 +02:00
Sebastiaan van Stijn
8687a3f4b8
Add more test-cases for positional parameters
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2018-05-25 18:20:10 +02:00
Sebastiaan van Stijn
b80e0309d2
Add line-numbers to asserts
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2018-05-25 18:20:01 +02:00
Sebastiaan van Stijn
334bf3ea76
Fix detection for missing parameter in substitution
`${}`, `${:}` and so on are invalid because there's
no parameter within the brackets; fix detection for
this situation and add/update tests.

There were some existing test-cases that were testing
for the wrong behavior, which are now updated.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2018-05-25 18:19:32 +02:00
Sebastiaan van Stijn
955a6ad95f
builder: fix processing of invalid substitusion syntax
The builder did not detect syntax errors in substitusions in the
Dockerfile, causing those values to be processed incorrectly instead of
producing an error.

Example 1: missing `}`

    docker build --no-cache -<<'EOF'
    FROM busybox
    ARG var=${aaa:-bbb
    RUN echo $var
    EOF

Before:

    Step 3/3 : RUN echo $var
     ---> Running in f06571e77146
    bbb

After:

    Step 2/3 : ARG var=${aaa:-bbb
    failed to process "${aaa:-bbb": syntax error: missing '}'

Example 2: missing closing `}`, no default value

    docker build --no-cache -<<'EOF'
    FROM busybox
    ARG var=${aaa
    RUN echo $var
    EOF

Before:

    Step 2/3 : ARG var=${aaa
    failed to process "${aaa": missing ':' in substitution

After:

    Step 2/3 : ARG var=${aaa
    failed to process "${aaa": syntax error: missing '}'

Example 3: double opening bracket (`{`)

    docker build --no-cache -<<'EOF'
    FROM busybox
    ARG var=${{aaa:-bbb}
    RUN echo $var
    EOF

Before:

    Step 2/3 : ARG var=${{aaa:-bbb}
    failed to process "${{aaa:-bbb}": missing ':' in substitution

After:

    Step 2/3 : ARG var=${{aaa:-bbb}
    failed to process "${{aaa:-bbb}": syntax error: bad substitution

Example 4: double opening bracket (`{`), no default value

    docker build --no-cache -<<'EOF'
    FROM busybox
    ARG var=${{aaa}
    RUN echo $var
    EOF

Before:

    Step 2/3 : ARG var=${{aaa}
    failed to process "${{aaa}": missing ':' in substitution

After:

    Step 2/3 : ARG var=${{aaa}
    failed to process "${{aaa}": syntax error: bad substitution

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2018-05-25 18:19:17 +02:00
Sebastiaan van Stijn
f23c00d870
Various code-cleanup
remove unnescessary import aliases, brackets, and so on.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2018-05-23 17:50:54 +02:00
Yong Tang
9bd5d9912f
Merge pull request #37110 from thaJeztah/remove-testutil-errorcontains
Replace deprecated testutil.ErrorContains()
2018-05-21 11:04:45 -07:00
Sebastiaan van Stijn
8974fd47c7
Merge pull request #37011 from arm64b/ReAdd-LABEL-command-4-target-option
Construct and add 'LABEL' command from 'label' option to last stage
2018-05-21 00:15:02 +02:00
Sebastiaan van Stijn
55bebbaecf
Replace deprecated testutil.ErrorContains()
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2018-05-21 00:13:04 +02:00
Eli Uriegas
80bc8b4766 Remove references to old release process
This hasn't been the way to release Docker for the past year so let's
just remove them altogether

Signed-off-by: Eli Uriegas <eli.uriegas@docker.com>
2018-05-18 18:28:43 +00:00
Dennis Chen
c7b543164d Remove unused 'label' related functions
Since we use `NewLabelCommand()` instead of `addNodesForLabelOption()`
to create the 'LABEL' commands from '--label' options, so all the related
functions should be removed.

Signed-off-by: Dennis Chen <dennis.chen@arm.com>
2018-05-17 09:41:06 +08:00
Dennis Chen
9c238ebd55 Add 'LABEL' command from '--label' to the last stage
This PR is tring to fix issue #36996.

Currently for multi-stage build, if `--target` specified, the `--label` option
will be ignored. The root cause is the last stage build will remove the `LABEL`
command(s) node created from the `--label` option. In order to address this issue,
we can create `LABEL` command(s) and add it/tem to the last stage.

Signed-off-by: Dennis Chen <dennis.chen@arm.com>
2018-05-17 09:40:19 +08:00
Priya Wadhwa
593255ffb0
Export BuildArgs
Signed-off-by: Priya Wadhwa <priyawadhwa@google.com>
2018-05-08 14:57:26 -07:00
Dennis Chen
7c0570473c Fix the target name issue for multi-stage build
This PR is trying to fix issue #36956.

The stage name is case-insensitive by design, so we should use
`strings.EqualFold()` as the comparison method to eliminate the
case sensitive noise.

Also we need to return a pre-defined error code order to avoid below
message like:
"FIXME: Got an API for which error does not match any expected type!!!:
failed to reach build target dev in Dockerfile"

Signed-off-by: Dennis Chen <dennis.chen@arm.com>
2018-05-01 10:28:40 +08:00
Kir Kolyshkin
7d62e40f7e Switch from x/net/context -> context
Since Go 1.7, context is a standard package. Since Go 1.9, everything
that is provided by "x/net/context" is a couple of type aliases to
types in "context".

Many vendored packages still use x/net/context, so vendor entry remains
for now.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2018-04-23 13:52:44 -07:00
Vincent Demeester
a7999aaa53
Skip some tests requires root uid when run as user
Signed-off-by: Vincent Demeester <vincent@sbr.pm>
2018-04-23 10:14:39 +02:00
John Stephens
29fc64b590
Merge pull request #35089 from Microsoft/jjh/fromplatformbuilder
LCOW - Change platform parser directive to FROM statement flag
2018-03-26 14:17:49 -07:00
John Howard
14429056d3 Builder: Review feedback
Signed-off-by: John Howard <jhoward@microsoft.com>
2018-03-19 14:29:36 -07:00
John Howard
317513d698 Builder: Fix CI issues
Signed-off-by: John Howard <jhoward@microsoft.com>
2018-03-19 14:29:36 -07:00
John Howard
69fa84bc3d Builder: Plumbing through platform in FROM statement
Signed-off-by: John Howard <jhoward@microsoft.com>
2018-03-19 14:29:36 -07:00