windows: propagate env across ops? about buildkit HOT 11 CLOSED

@jhowardmsft @simonferquel @dnephin @tonistiigi
PTAL if you are interested in?
😅

from buildkit.

Could you elaborate on the proposed solution? I'm not sure I understand

from buildkit.

Problem

(copy-pasted from @simonferquel's Dockerfile proposal for ENV --lazy-expand moby/moby#31525 )

FROM microsoft/nanoserver
ADD .\\bin c:\\myapp
ENV PATH ${PATH};c:\\myapp
RUN echo %PATH%

In this example, the builder does not consider that PATH can have a
living value in the stored base image and overrides its value with
;c:\\myapp. this breaks basically the whole Windows

Solution

The current LLB has this problem: it does not support propagating env from SourceOp (LLB equivalent of Dockerfile FROM) nor other ExecOp (LLB equivalent of RUN):

message ExecOp {
	Meta meta = 1;
	repeated Mount mounts = 2;
}

message Meta {
	repeated string args = 1;
	repeated string env = 2;
	string cwd = 3;
}

My proposal is to add int64 env_input [(gogoproto.customtype) = "InputIndex"] to Meta so that the env can be propagated from other LLB ops.

I also propose adding repeated string env_capturable_keys (e.g. {"PATH"}) to Meta so as to prevent password env var from being leaked into children ops.

from buildkit.

I think we can achieve good performance of this, by capturing in-container persisted env on exec, using a second process in the same container: basically the RUN implementation would looks something like

createContainer()
createAndStartProcess(<run parameters>, containerConfig.User)
waitForProcessEnd()
if platform.HasPersistedEnv(){
  createAndStartProcess(capture-env, containerConfig.User) // important to run this with the same user account
  waitForProcessEnd()
  populateMetadata()
}
removeContainer()

We need to take a look at it, but from what I've observed, the costly part in running a windows container, is creating the container itself and attaching mounts/networks. Running a process in a living container seem very cheap in comparison.

from buildkit.

@simonferquel Thank you for the information, yes, and I think my proposal will work as you suggested.

from buildkit.

I'm not convinced this needs any fix at all. I think the situation is quite similar in linux and we have no users asking alternatives. So why does windows need this special treatment.

I understand that windows has system environment variables and linux doesn't. But 99% of the examples about this problem are about the PATH variable and PATH variable behaves exactly the same in both linux and windows. Almost all linux applications expect PATH variable to be defined so it behaves like a system environment variable. This is why we always set the PATH env in all of the linux images. You can't create an image with the docker build(or with llb client pkg) that doesn't have the PATH variable. For some reason windows does not follow that rule. If I understand correctly then windows base images do not set PATH for some reason. If they did there would not be a problem with modifying it. Next problem that is brought up is that there are programs that are executed that may also change PATH. I don't see it as any different than a linux install script that writes new path to .bashrc. We don't do anything to capture that in linux and users seem to understand that. They know that if they need to modify path variable they need to call ENV. For the other variables, it is even more obvious: for example, all linux users know that extracting go tarball is not enough, you also need to set GOPATH after. There is no magic that adds that automatically.

If windows really needs this feature for some reason I think it can be implemented in a higher level dockerfile feature using nested(#83) or repeated invocation. Afaics --lazy-expand doesn't need exceptions to be implemented with llb, just new lazy-env property in execop meta.

from buildkit.

The problem comes mostly from the fact that many software installers that you would call with RUN commands do modify persisted env variables (ie: JDK installer setup env variables this way, as is the case for many SDKs in the windows commercial c/c++ world).
If you somehow need to add stuff in these variables, you are in a bad situation.

Sent from my Samsung SM-G935F using FastHub

from buildkit.

@simonferquel isn't it possible to run some kind of "export" on the env variables you care about after the install? Maybe even something as simple as writing it to a file, then an entrypoint script can load the env file to set the environment variables for run.

from buildkit.

For the JDK example, exactly the same in linux https://github.com/docker-library/openjdk/blob/9a0822673dffd3e5ba66f18a8547aa60faed6d08/8-jre/alpine/Dockerfile#L27

from buildkit.

@dnephin that is the the current best workaround I know

from buildkit.

Closing as outdated

from buildkit.