Skip to content
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

feat: use USER env var in Dockerfile #155

Merged
merged 4 commits into from Jun 28, 2023
Merged

feat: use USER env var in Dockerfile #155

merged 4 commits into from Jun 28, 2023

Conversation

JeremyTubongbanua
Copy link
Member

What I did

  • Made ENV USER=atsign
  • Then replaced atsign with $USER

Was this the correct approach?

@cpswan
Copy link
Member

cpswan commented May 24, 2023

Rather than a line by line review I've made a bunch of edits and pushed a commit with them in.

@JeremyTubongbanua the main things I've changed are:

  • braces around where ENV variables are used
  • replace /$USER with ${HOMEDIR} where it's a directory we're referring to

@cconstab can you also review before we merge this please

@JeremyTubongbanua
Copy link
Member Author

JeremyTubongbanua commented May 24, 2023

Thanks @cpswan

Do environment variables work inside of the strings at ENTRYPOINT?

I got this when I ran a dummy Dockerfile.

Output:

Hello, ${USER} -it --name tmp

Dockerfile:

FROM ubuntu

ENV USER='jeremy123'

ENTRYPOINT [ "echo", "Hello, ${USER}" ]

Full Docker output:

[+] Building 0.0s (5/5) FINISHED                                                                                                                                                                         
 => [internal] load .dockerignore                                                                                                                                                                   0.0s
 => => transferring context: 2B                                                                                                                                                                     0.0s
 => [internal] load build definition from Dockerfile                                                                                                                                                0.0s
 => => transferring dockerfile: 111B                                                                                                                                                                0.0s
 => [internal] load metadata for docker.io/library/ubuntu:latest                                                                                                                                    0.0s
 => CACHED [1/1] FROM docker.io/library/ubuntu                                                                                                                                                      0.0s
 => exporting to image                                                                                                                                                                              0.0s
 => => exporting layers                                                                                                                                                                             0.0s
 => => writing image sha256:00784b0f36c3923b9a4b92e2ee891aacf62d2bb60319d1d1bb37d8430de383e1                                                                                                        0.0s
 => => naming to docker.io/library/tmp                                                                                                                                                              0.0s
Hello, ${USER} -it --name tmp

@cpswan
Copy link
Member

cpswan commented May 24, 2023

Good catch @JeremyTubongbanua ENV vars don't work inside an exec style ENTRYPOINT

https://stackoverflow.com/a/37904830/3830789

XavierChanth
XavierChanth previously approved these changes Jun 28, 2023
@JeremyTubongbanua JeremyTubongbanua merged commit 0de5cd4 into trunk Jun 28, 2023
1 check passed
@XavierChanth XavierChanth deleted the edit-docker branch July 6, 2023 15:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants