New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Proposal: docker build read Dockerfile from stdin #27393
Comments
@dsheets perhaps I misunderstood, but
Or is this to build using the local build context, but getting only the dockerfile from stdin? |
@thaJeztah sorry I wasn't clearer. Your second explanation is correct. This proposal is to have the build context indicated by local path, git repository, or URL and the Dockerfile only to be read from stdin and supply the build instructions (or override existing build instructions) from the context. The use case that prompted this proposal is the first in the above use case list -- stacking anonymous images in a shared engine CI environment. It is much easier and more natural to pipe around text than tar files and there are many common tools to source/modify/compile text compared to tar files without creating temporary files or directories. |
@dsheets clear! Sounds reasonable to me (although the documentation may need a rewrite as all possible combinations become quite complex) |
Actually I am not sure what the current behaviour is, is |
I believe it should still work - as long as the -f points to a file in the context/git-repo. It should allow for you to build from a Dockerfile that’s not in the root.
|
I think this is a bit inconsistent, as I believe I think it would be better to allow multiple contexts which are appended so you could do |
For the local directory case, Currently, a local context can only be a directory (not a tarball or a Dockerfile). This isn't currently consistent but the suggestion to stack contexts would have to expand that functionality as well. If you have use cases for stacked contexts, I recommend a separate proposal outlining them. I don't see any reason why the |
@justincormack |
I think currently
|
@thaJeztah's comment about being able to indirectly reference a Dockerfile from outside of the build context is a really interesting usecase - I'd love to hear about some usecase/need for that one. My initial reaction to this is that it could lead to a bit of confusion since we would support "-" being an option of -f as well as in place of the build context location (last param). This isn't necessary an issue since we would need to restrict the user from specifying it in both places, but just something to think about. Would still like time to think about this, but I'm leaning towards saying we should add this since it pretty consistent with other tools that support similar options and it could open up some interesting usecases. For example: |
I would like to be able to use a Docker file outside a tarball build On 19 Oct 2016 15:28, "Doug Davis" notifications@github.com wrote:
|
You mean multiple |
@justincormack where do you see the incompatibility? Actually, the more I think about this... @dsheets how do you see this working? Right now the Dockerfile is passed as part of the context, not as an independent entity to the REST API. |
@duglin your @justincormack using @thaJeztah Do you mean multiple positional arguments to the "context"? I don't know what multiple |
@dsheets yea, I know its disallowed today but my point is that once you figure out how to support stdin then there's no reason to disallow it anymore - we can just treat any file outside of the build context as coming from "stdin". In your prototype, did you end up creating a Dockerfile in the context? Not sure you can do that w/o also creating a .dockerignore with Dockerfile in there, or overriding any existing one. This scares me because then you'd be modifying the user's files and we shouldn't do that since we have no idea what they might be doing with them. |
I guess one more safe option, but a bit hacky, would be to put the Dockerfile contents into a well defined, but hidden and pretty unlikely to overlap with a user's, file - such as |
@thaJeztah no, |
@justincormack @duglin @dsheets I don't really see a reason why we would disallow a Dockerfile that 's outside of the build-context. As long as we treat the Dockerfile as if it's at the root of the context (so doesn't have access to files outside of it), I don't see why we would reject that. So basically;
Sounds like something we could support. The |
Well, I think that fails because using '-' as the build context means that there is no context and stdin is the Dockerfile, not the context. Or am I not folllowing? |
@thaJeztah I agree that we could definitely support arbitrary Dockerfile paths with the tar stream rewriting I use to put stdin into the context. When I make a PR, I will put that in a secondary commit. @duglin I don't understand what you are referring to re: @justincormack It would be cool if you could pass a local tarball as a build context to go in the direction that you are thinking. Unfortunately, your We could have a method for packing both tarballs and dockerfiles into stdin (multi-part MIME?!) but that is quite an extension and design exercise. |
What's wrong with relevant: https://codereview.appspot.com/22170044/#msg2 On Wed, Oct 19, 2016 at 12:23 PM, David Sheets notifications@github.com
|
@erikh |
The special-casing would not be required if the out-of-context path suggestion by @thaJeztah was used. I think at that point only the |
right, special casing would be needed for the context, but I haven't tested but I'm pretty sure there's no barrier here. On Wed, Oct 19, 2016 at 1:08 PM, David Sheets notifications@github.com
|
As I said, With @thaJeztah's suggestion to rewrite the context tar stream for out-of-context Dockerfile references, all of those changes would be made. Portability and convention would be the reasons (and good ones IMO) to support |
this works just fine on 1.12.2:
Just verified. Anyways, I'm tapping out. On Wed, Oct 19, 2016 at 2:34 PM, David Sheets notifications@github.com
|
@erikh, that's interesting. This works on 1.12.2 on Linux but not on OS X... thanks for the encouragement! |
It doesn't always work on Linux, either --
This seems quite brittle and does not actually satisfy the specification in this proposal. I'm currently inclined to believe that this disgusting magic is courtesy of the shell's I/O redirection rewriting
@erikh, or others, do you have any idea what is going on here? The source code (and my understanding) supports the error case only. I'll go hunting for this bogus magic tomorrow, I suppose... |
Your shell replaces stdin (fd 0) with an open file which
This doesn't work on OS X (or likely other BSDs) because This means that
Essentially, using |
I think github's mail system died, so I'll echo my thoughts directly into the form, hopefully they do not deliver duplicates later, sorry in advance. I think I see part of the problem. I agree, it's confusing behavior at best, but I think it's exposing a breakout of the context so I wanted to explain. I tried this for giggles:
https://github.com/docker/docker/blob/master/builder/context.go#L219 seems to be the relevant portion of code here. How we qualify https://github.com/docker/docker/blob/master/builder/context.go#L249-L251 specifically doesn't seem to check if, say, the path is Additionally, I don't know if docker is doing anything fancy but I checked: package main
import (
"fmt"
"os"
)
func main() {
f, err := os.Open("/dev/stdin")
if err != nil {
panic("wut")
}
fmt.Println(f.Fd())
} This prints "3" which is what I expect it to. Maybe docker is seeing stdin first and ignoring the -f option? |
@tonistiigi did some testing, and may be able to add more info there |
Unsure if it's useful, but I've been using this in my path handling code on my own projects (could replace 249-251) paths := filepath.SplitList(rel)
for _, path := range paths {
if path == ".." {
return nil, createException(m, fmt.Sprintf("Cannot use relative path %s because it may fall below the root build directory", source))
}
} |
@erikh symlinks are resolved prior to path relativization and context-escape checking. On Linux, the kernel creates a symlink chain |
I think I see part of the problem. I agree, it's confusing behavior at I tried this for giggles with the repo @ https://github.com/erikh/box:
https://github.com/docker/docker/blob/master/builder/context.go#L219 seems https://github.com/docker/docker/blob/master/builder/context.go#L249-L251 On Wed, Oct 19, 2016 at 4:52 PM, David Sheets notifications@github.com
|
I don't know if docker is doing anything fancy but I checked: package main
import (
"fmt"
"os"
)
func main() {
f, err := os.Open("/dev/stdin")
if err != nil {
panic("wut")
}
fmt.Println(f.Fd())
} This prints "3' which is what I expect it to. Maybe docker is seeing stdin |
I think that would be great. I could finally stick a placeholder in the FROM directive for multiple distros that are closely related, cat & sed the dockerfile with the right distro plugged in to docker build (instead of generating a temporary dockerfile for each distro) and still have access to the context for COPY operations. Atm, I have a bunch of centos 6 & centos 7 dockerfiles that look exactly the same, except for a differing FROM directive. |
@Magnitus- Sorry ... Not sure if this is still relevant to you but one way I got around the issue of multiple docker files was simply tagging the base image before building So it'd look like this docker tag centos:6 mybaseimg Dockerfile: |
I'm ok with both allowing If there are no objections(didn't spot in comments) and nobody has already done work on this, it can be assigned to me. |
Implements proposal moby#27393 that describes the meaning of '-f -' in a `docker build` command. For all context types except reading a context from stdin, '-f -' will replace any file called 'Dockerfile' in the context's tar stream with the contents of stdin. If the context itself is read from stdin, '-f -' continues to read the Dockerfile called '-' from this context. Fixes moby#27393. Signed-off-by: David Sheets <sheets@alum.mit.edu>
@tonistiigi I've pushed the |
A problem with setting |
just some history... when -f was added it was decided that the value of "-f" was relative to the current dir (unless its an abs path). |
I propose that
docker build
accept-f -
to indicate that the Dockerfile to be used is read from the STDIN of thedocker
client process.Use cases
docker build
is now a versatile shell scripting command as Dockerfiles can participate in shell pipelines decoupled from file system contextsNew capabilities
Design considerations
-
stdin reader interface already exists in the context path argument so any-f -
implementation will have to accommodate that-
in place of file paths is a common UNIX idiom to indicate that what would have been read from the path specified should instead be read from stdin; this makes havingdocker
participate in UNIX pipelines much easierCases
builder.GetContextFromReader
(builder/context.go
) is used to read a context (or Dockerfile only) from stdin: nothing will change, a file called-
will still be sourced from the stdin tarball (or still ignored in the case ofDockerfile
only on stdin)builder.GetContextFromGitURL
(builder/context.go
) is used to read a context from a git URL and the resultant context will have itsDockerfile
rewritten and used for the buildbuilder.GetContextFromURL
(builder/context.go
) is used to read a context from a remote URL and the resultant context will have itsDockerfile
rewritten and used for the build (or ignored in the case of aDockerfile
only context)builder.GetContextFromLocalDir
(builder/context.go
) is used to read a context from a local directory and the resultant context will have itsDockerfile
rewritten and used for the buildImplementation
I've prototyped this functionality by rewriting the tar stream of the context before the trusted pull rewriting. If there is interest in this functionality, I will clean up my prototype and submit a PR. I welcome your ideas regarding this or similar features. Thanks!
The text was updated successfully, but these errors were encountered: