Skip to content

ENV section should be before tha RUN in Dockerfile#6450

Closed
FelipeRuizGarcia wants to merge 2 commits into
docker:masterfrom
FelipeRuizGarcia:patch-1
Closed

ENV section should be before tha RUN in Dockerfile#6450
FelipeRuizGarcia wants to merge 2 commits into
docker:masterfrom
FelipeRuizGarcia:patch-1

Conversation

@FelipeRuizGarcia

Copy link
Copy Markdown

Im learning Docker and Im following the get-staterd guide.
Im behind a proxy.

Following the guide such as is document, when I run root@felipe# docker build -t happydocker . I get a error

Sending build context to Docker daemon 45.75MB
Step 1/14 : FROM python:2.7-slim
---> b16fde09c92c
Step 2/14 : WORKDIR /app
---> Using cache
---> 2842bfbc0a0a
Step 3/14 : ADD . /app
---> 23dad6764edc
Step 4/14 : RUN pip install --trusted-host pypi.python.org -r requirements.txt
---> Running in e137a0f36fdb
Collecting Flask (from -r requirements.txt (line 1))
Retrying (Retry(total=4, connect=None, read=None, redirect=None, status=None)) after connection broken by 'ConnectTimeoutError(<pip._vendor.urllib3.connection.VerifiedHTTPSConnection object at 0x7fcf5f021910>, 'Connection to pypi.python.org timed out. (connect timeout=15)')': /simple/flask/

The librarys indicate in requirements.txt file are not installed for error connection : This is a signal that proxy ENV are not passed for the app container yet.
This because in the Dockerfile first execute RUN command and the import the ENV variables.

Install any needed packages specified in requirements.txt

RUN pip install --trusted-host pypi.python.org -r requirements.txt

Define environment variable

ENV NAME World
ENV http_proxy host:port

If first indicate ENV variables in the that going to be installed in the container app and then RUN command in the Dockerfile, the image is create succesfully.

Proposed changes

Unreleased project version (optional)

Related issues (optional)

Im learning Docker and Im following the get-staterd guide.
Im behind a proxy.

Following the guide such as is document, when I run  `root@felipe# docker build -t happydocker  .  ` I get a error 

Sending build context to Docker daemon  45.75MB
Step 1/14 : FROM python:2.7-slim
 ---> b16fde09c92c
Step 2/14 : WORKDIR /app
 ---> Using cache
 ---> 2842bfbc0a0a
Step 3/14 : ADD . /app
 ---> 23dad6764edc
Step 4/14 : RUN pip install --trusted-host pypi.python.org -r requirements.txt
 ---> Running in e137a0f36fdb
Collecting Flask (from -r requirements.txt (line 1))
  Retrying (Retry(total=4, connect=None, read=None, redirect=None, status=None)) after connection broken by 'ConnectTimeoutError(<pip._vendor.urllib3.connection.VerifiedHTTPSConnection object at 0x7fcf5f021910>, 'Connection to pypi.python.org timed out. (connect timeout=15)')': /simple/flask/

The librarys indicate in requirements.txt file are not installed for error connection : This is a signal that proxy ENV are not passed for the app container yet.
This because in the Dockerfile first execute RUN command and the import the ENV variables.

# Install any needed packages specified in requirements.txt
RUN pip install --trusted-host pypi.python.org -r requirements.txt
# Define environment variable
ENV NAME World
ENV http_proxy host:port

If first indicate ENV variables in the that going to be installed in the container app and then RUN command in the Dockerfile, the image is create succesfully.
Comment thread get-started/part2.md Outdated
ADD . /app

# Define environment variable
ENV NAME World

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think the $NAME env-var has to be set before the RUN, as this is only used at runtime (when the container is started)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What we should change though, is the section about setting a proxy through an ENV instruction. That's not best practice, because doing so will bake your proxy configuration into the image, which means that everyone that will try to run your image will also be using the proxy you configured (which is likely unique to your environment).

Instead, we should describe:

  • using --build-arg to set a proxy during build time
  • use the --env option at runtime
  • refer to the proxy configuration in the CLI configuration file

ping @gbarr01

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A beginner behind a proxy following the guide its going toget the error because in these guide does not mention about the option for docker build command behind a proxy.

I think this should be mentioned in some part of guide.
What you think ?

@gbarr01 gbarr01 Apr 18, 2018

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, @thaJeztah ,

I'm struggling to understand the use of ARG and --build-arg. I tested its use with the NAME variable and it's not working as I expect. The docs say: An ARG instruction can optionally include a default value... If an ARG instruction has a default value and if there is no value passed at build-time, the builder uses the default. I'm not seeing this.

For example, this code does not display "Moby" but "None".

Dockerfile
ARG NAME="Moby"

Build and Run

docker build --build-arg NAME  --tag hellomoby:v1 .
docker run --publish 4000:80 hellomoby:v1

When I tweak the build-time to docker build --build-arg NAME="Moby" --tag hellomoby:v2 ., it's still "None".

Only when I run, docker run --env NAME="Moby" --publish 4000:80 hellomoby:v2, does it display as "Moby". And when I remove ARG from Dockerfile altogether, with the same run command, it also displays as "Moby."

I cannot see how ARG and --build-arg is having any effect here. Can you see what I'm doing wrong? Seems like using --env at runtime is good enough.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gbarr01 let me catch up with you IRL to try and explain (probably faster than writing all down)

@FelipeRuizGarcia FelipeRuizGarcia changed the title ENV section must be before tha RUN in Dockerfile ENV section should be before tha RUN in Dockerfile Apr 14, 2018
@gbarr01 gbarr01 self-assigned this Apr 16, 2018
@gbarr01 gbarr01 added area/tutorials area/build Relates to Dockerfiles or docker build command labels Apr 26, 2018
@ahh-docker ahh-docker assigned ahh-docker and unassigned gbarr01 Sep 18, 2018
@ahh-docker

Copy link
Copy Markdown
Contributor

@thaJeztah can you please let me know if there's any more changes with this? Let me know and I'll get it updated. Thanks!

@ahh-docker

Copy link
Copy Markdown
Contributor

@thaJeztah Can I get a look at this again? Need to know if this is LGTM or a thumbs down. Thanks!

@thaJeztah thaJeztah left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left some comments inline; the additional comment about the ENV vars doesn't make the situation worse than it is now, but the section should really be rewritten, as it's promoting bad practice.

Comment thread get-started/part2.md
ENV NAME World

# Make port 80 available to the world outside this container
EXPOSE 80

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moving this line doesn't look related to this PR

Comment thread get-started/part2.md
# Define environment variable
ENV NAME World

# Make port 80 available to the world outside this container

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not introduced in this PR, but this description is not correct. EXPOSE does not make the port accessible, it only advertises / adds metadata about what port(s) the container listens on (according to the image's author).

Something like "Advertise that the container is listening on port 80" (not sure about "advertise") would be better (but could be a separate PR)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about 'Publicizes that the container is listening on port 80'?

Comment thread get-started/part2.md
> proxy servers:
> proxy servers and make sure that `ENV` section are before that `RUN`, this to
> set up the variables enviroment before run everything needed.
> command section :

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is "correct", but this section should really be rewritten (see my other comment; #6450 (comment)).

Setting proxy-servers in the image through ENV is definitely not the recommended approach; those variables will persist in the image, which means that the image can only be run in the environment in which they are built.

Information about the proxy-server can leak security-sensitive information (name of the proxy-server, or even name/password if the proxy server requires authentication).

This section should describe using build-args instead (--build-arg HTTP_PROXY=http://myyproxy.example.com), or even how to set defaults in the CLI configuration file.

Build-args were designed for this purpose; the proxy information will only be available during the docker build itself, but won't persist in the image after that (including leaking through docker image history).

@paigehargrave

Copy link
Copy Markdown
Contributor

@thaJeztah - Hi Sebastian, I've attempted to wordsmith something to cover the use of --build-arg, can you take a look and see what else is needed? Thanks! (Please ignore any formatting SNAFUs - just trying to nail down content.)

Suggested rewrite of Proxy server settings info:

Proxy servers can block connections to your web app once it's up and running.
If you are behind a proxy server, you can specify proxy server host and port information in one of the following ways:

  1. Use the build-args command in your Dockerfile for build-time argument passing.

docker build --build-arg HTTP_PROXY=http://myyproxy.example.com

The proxy information is only available during the docker build itself, and doesn’t persist in the resulting image (including leaking through docker image history).
This is similar to how docker run -e works. Refer to the docker run documentation f additional details.
You can use the --build-arg flag without a value, in which case the value from the local environment is propagated into the Docker container being built.
Using this flag will not alter the output you see when the ARG lines from the Dockerfile are echoed during the build process. The ARG instruction lets Dockerfile authors define values that users can set at build-time using the --build-arg flag. See the Dockerfile reference for more information about ARG.

2)Set defaults in the CLI configuration file. (Should we include this? If so, can we include an example?)

Warning: Using ENV instructions in a Dockerfile to set proxy-server values in the image is not recommended, because those variable values persist in the intermediate and final images. This is not desirable for the following reasons:
The resulting image can only be run in the environment in which it is built.
Information about the proxy-server can leak security-sensitive information (name of the proxy-server, or even the name and password if the proxy server requires authentication).

@bermudezmt

Copy link
Copy Markdown
Contributor

@thaJeztah I know this PR has gotten unwieldy. 😆 Can you please review comment from @paigehargrave (#6450 (comment)) which hopefully addresses your requested changes? Thanks!

@ghost

ghost commented Feb 22, 2019

Copy link
Copy Markdown

@thaJeztah I know this PR has gotten unwieldy. 😆 Can you please review comment from @paigehargrave (#6450 (comment)) which hopefully addresses your requested changes? Thanks!

Nudged in Slack this morning.

@ghost ghost assigned paigehargrave and unassigned ahh-docker Feb 22, 2019
@ghost ghost added OP/+21 days and removed OP/+14 days labels Feb 22, 2019
@thaJeztah

Copy link
Copy Markdown
Member

You can use the --build-arg flag without a value

That one might need an example, to illustrate what we mean with "without a value";

export HTTPS_PROXY=https://proxy.corp.example.com:8080

docker build --build-arg HTTPS_PROXY  ......

2)Set defaults in the CLI configuration file. (Should we include this? If so, can we include an example?)

I'm thinking if we should include all this information (about proxies) in the "getting started". The information is valuable, but overall, it seems a bit out of place for a "getting started".

There's an existing section on proxy configuration (for the client) here: https://docs.docker.com/network/proxy/, but it may be hard to find ("configure the daemon and client"). Thinking of dedicating a more standalone section about that, to describe daemon configuration, client configuration, as well as setting proxies at runtime and for build.

Other parts in the documentation can link to that section so that we don't have to repeat the information.

@paigehargrave paigehargrave mentioned this pull request Feb 26, 2019
@paigehargrave

Copy link
Copy Markdown
Contributor

@thaJeztah Hi Sebastian - I created #8352 to try to capture your suggested changes. Hopefully what I did is a reasonable start. PTAL and many thanks again for your input!

It looks like additional information is needed for:

  • use the --env option at runtime
  • refer to the proxy configuration in the CLI configuration file

@bermudezmt

Copy link
Copy Markdown
Contributor

Closing in favor of #8352 .

@bermudezmt bermudezmt closed this Mar 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/build Relates to Dockerfiles or docker build command

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants