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

chore: inheriting proxy env vars from host when building with docker #2192

Merged
merged 2 commits into from
Oct 13, 2023

Conversation

AlseinX
Copy link
Contributor

@AlseinX AlseinX commented Oct 12, 2023

Modified the build script to pass HTTP_PROXY environment variables from the host machine to the build container, so that developers under GFW would use their custom proxy to build the project.

Copy link
Collaborator

@Kailai-Wang Kailai-Wang left a comment

Choose a reason for hiding this comment

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

It's great that you proactively submit PRs to tackle the problem you met 👍

Have you tried this?
https://docs.docker.com/network/proxy/

Like using a customised docker config for it - as it doesn't apply to everyone, maybe it's a good idea to keep the change local too

docker/Dockerfile Outdated Show resolved Hide resolved
scripts/build-docker.sh Show resolved Hide resolved
@Kailai-Wang
Copy link
Collaborator

Kailai-Wang commented Oct 12, 2023

It also seems that even without definition inside Dockerfile, the env HTTP_RPOXY can be recognised:

docker build --no-cache --progress=plain  --build-arg HTTP_PROXY="http://proxy.example.com:3128" -t tmp -<<EOF
FROM alpine
RUN env | grep -i _PROXY
EOF

#0 building with "desktop-linux" instance using docker driver

#1 [internal] load .dockerignore
#1 transferring context: 2B done
#1 DONE 0.0s

#2 [internal] load build definition from Dockerfile
#2 transferring dockerfile: 74B done
#2 DONE 0.0s

#3 [internal] load metadata for docker.io/library/alpine:latest
#3 DONE 0.7s

#4 [1/2] FROM docker.io/library/alpine@sha256:eece025e432126ce23f223450a0326fbebde39cdf496a85d8c016293fc851978
#4 CACHED

#5 [2/2] RUN env | grep -i _PROXY
#5 0.160 http_proxy=http://proxy.example.com:3128
#5 0.160 HTTP_PROXY=http://proxy.example.com:3128
#5 DONE 0.2s

#6 exporting to image
#6 exporting layers 0.0s done
#6 writing image sha256:61266719edeac635a4e8a606d67321e8484c20224296759db620c7f617e108c2 done
#6 naming to docker.io/library/tmp done
#6 DONE 0.0s

@AlseinX
Copy link
Contributor Author

AlseinX commented Oct 13, 2023

It also seems that even without definition inside Dockerfile, the env HTTP_RPOXY can be recognised:

I performed a test removing the definitions inside Dockerfile, within the server under GFW.
It fails to recognise the proxy settings and seems not to work exactly as documented.

@Kailai-Wang
Copy link
Collaborator

I performed a test removing the definitions inside Dockerfile, within the server under GFW.
It fails to recognise the proxy settings and seems not to work exactly as documented

This is utterly strange.
It should at least get those envs assigned inside your docker, what's output if you add RUN env | grep _PROXY in Dockerfile?

Whether it can actually access host proxy is another story.

@AlseinX
Copy link
Contributor Author

AlseinX commented Oct 13, 2023

I tried and found that --build-arg HTTP_PROXY="..." only sets exact HTTP_PROXY, and others are unset, therefore it does not recognise, which means I have to pass all of these env vars with --build-arg within the build script
I have recommitted to remove modification on dockerfile.

Copy link
Collaborator

@Kailai-Wang Kailai-Wang left a comment

Choose a reason for hiding this comment

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

Thanks!

@Kailai-Wang
Copy link
Collaborator

In my test --network=host is enough for the container to access services on the host via e.g. localhost:<port-number>, but anyway it's not that important

@Kailai-Wang Kailai-Wang merged commit f2977f6 into dev Oct 13, 2023
27 checks passed
@Kailai-Wang Kailai-Wang deleted the alsein/proxy branch October 13, 2023 14:06
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.

2 participants