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

proxy info from docker config is not respected #1175

Open
BenTheElder opened this issue Dec 18, 2019 · 21 comments
Open

proxy info from docker config is not respected #1175

BenTheElder opened this issue Dec 18, 2019 · 21 comments
Labels
area/provider/docker Issues or PRs related to docker help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. kind/bug Categorizes issue or PR as related to a bug. lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. priority/backlog Higher priority than priority/awaiting-more-evidence. priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete.

Comments

@BenTheElder
Copy link
Member

This should be an easy fix, thanks to @sudeshsh for debugging this with me.

We just need to detect and respect https://docs.docker.com/network/proxy/#configure-the-docker-client

passing in env explicitly will still be expected, but docker will automagically plumb through proxy env, just missing the things kind would otherwise append to noProxy.

/assign
/lifecycle active

@BenTheElder BenTheElder added the kind/bug Categorizes issue or PR as related to a bug. label Dec 18, 2019
@k8s-ci-robot k8s-ci-robot added the lifecycle/active Indicates that an issue or PR is actively being worked on by a contributor. label Dec 18, 2019
@BenTheElder
Copy link
Member Author

filed #1176

@tao12345666333
Copy link
Member

tao12345666333 commented Dec 18, 2019

Just to clarify, the #1176 fix was as described in the https://docs.docker.com/config/daemon/systemd/#httphttps-proxy documentation, not the client side proxy. 😸

@BenTheElder
Copy link
Member Author

@tao12345666333 I'm not sure what you mean by:

Just to clarify, the #1176 fix was as described in the https://docs.docker.com/config/daemon/systemd/#httphttps-proxy documentation, not the client side proxy. smile_cat

?

This fixes the case that you do not set environment variables and instead configure the docker config with proxy details.

@BenTheElder
Copy link
Member Author

We're obtaining whatever proxy settings docker info reports. It is on docker to report sane values there imo :-)

@tao12345666333
Copy link
Member

func getProxyEnvFromDocker(cmder exec.Cmder) (map[string]string, error) {
// get raw fields
lines, err := exec.OutputLines(
cmder.Command(
"docker", "info",
"--format",
// one per line, upper(key)=value
"HTTP_PROXY={{.HTTPSProxy}}\nHTTPS_PROXY={{.HTTPSProxy}}\nNO_PROXY={{.NoProxy}}",
),
)

I mean, the proxy value read by docker info is based on the configuration of the docker daemon, not the .docker / config.

(MoeLove) ➜  ~ grep httpProxy ~/.docker/config.json
            "httpProxy": "http://127.0.0.1:9999"
(MoeLove) ➜  ~ docker info --format {{.HTTPProxy}}

(MoeLove) ➜  ~ docker run --rm alpine:3.10 env |grep -i http_proxy
HTTP_PROXY=http://127.0.0.1:9999
http_proxy=http://127.0.0.1:9999

@BenTheElder
Copy link
Member Author

: |

@aojea
Copy link
Contributor

aojea commented Dec 18, 2019

I really think that is easier to configure environment variables than try to cover this corner case :/

#1175 (comment)

EDIT wrong comment :)

@BenTheElder
Copy link
Member Author

This is not a corner case. This is a well documented mechanism to configure proxy for your containers that we have been ignorant of.

@aojea
Copy link
Contributor

aojea commented Dec 18, 2019

This is not a corner case. This is a well documented mechanism to configure proxy for your containers that we have been ignorant of.

ups, you're totally right https://docs.docker.com/network/proxy/#configure-the-docker-client I was confused because of the docker info thing 😅

@BenTheElder
Copy link
Member Author

BenTheElder commented Dec 18, 2019 via email

@BenTheElder
Copy link
Member Author

punting for a moment to handle some other major improvements but I still intend to finish this.

it looks like unfortunately we need to write some logic to find and parse the docker config accurately.

not a huge deal but I'd hoped to find a way to get docker ... to at least spit out the client config so we don't have to try to locate it the same way... doesn't look like you can.

shouldn't be hard at all, but we should be careful to mimic docker closely here to avoid surprises. in particular we should see how they lookup $HOME.

@tao12345666333
Copy link
Member

If we really need to do this, I personally think that we probably need to deal with the following:

  • check DOCKER_CONFIG env

  • check $HOME/.docker/config

  • parse docker config and get proxies

  • check DOCKER_HOST env etc to get docker's daemon host

  • using docker's daemon host to get value from proxies, if not found we can using default value.

@BenTheElder
Copy link
Member Author

BenTheElder commented Dec 20, 2019 via email

@BenTheElder BenTheElder removed the lifecycle/active Indicates that an issue or PR is actively being worked on by a contributor. label Feb 10, 2020
@BenTheElder BenTheElder removed their assignment Feb 10, 2020
@BenTheElder
Copy link
Member Author

will have to circle back to this one

@BenTheElder BenTheElder added the priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. label Feb 10, 2020
@BenTheElder BenTheElder added this to the 2020 goals milestone Apr 25, 2020
@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jul 24, 2020
@kubernetes-sigs kubernetes-sigs deleted a comment from fejta-bot Jul 24, 2020
@BenTheElder BenTheElder removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jul 24, 2020
@BenTheElder BenTheElder added the area/provider/docker Issues or PRs related to docker label Aug 7, 2020
@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Nov 5, 2020
@tao12345666333
Copy link
Member

/remove-lifecycle stale

@k8s-ci-robot k8s-ci-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Nov 6, 2020
@BenTheElder
Copy link
Member Author

it would be helpful to know more about how docker behaves here, e.g. does it try to merge these settings? If I add proxy details to docker but then I pass the http_proxy env to a container, what value is use?

@tao12345666333
Copy link
Member

Today or tomorrow I will describe this behavior of docker in detail here.

@tao12345666333
Copy link
Member

Sorry for delay.

First of all, it needs to be clear that the environment variables of the container can be set in two ways.

  1. Set directly through --env args.

  2. Configure via client config file ~/.docker/config

The first method has the highest priority.

e.g. set the client proxy to http://127.0.0.1:3001 but pass HTTP_PROXY=http://127.0.0.1:5001 env to container.

The container will have both HTTP_PROXY and http_proxy environment variables.

➜  ~ docker run --rm -e HTTP_PROXY=http://127.0.0.1:5001 alpine env 
PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin
HOSTNAME=c569d42dd38c
HTTP_PROXY=http://127.0.0.1:5001
http_proxy=http://127.0.0.1:3001
HOME=/root

Secondly, the proxy set by the Docker daemon is not added to the container by default.

➜  ~ docker info --format {{.HTTPProxy}}
socks5://127.0.0.1:1080
➜  ~ docker run --rm  -e http_proxy=http://127.0.0.1:5001 alpine env
PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin
HOSTNAME=a84c2638485f
http_proxy=http://127.0.0.1:5001
HOME=/root

@BenTheElder
Copy link
Member Author

Thank you!

We still lack a way to read the settings of 2) ourselves except attempting to match docker config loading (we may have to just do that then).

Maybe we should read from 2) ourselves when using the docker node provider and use it if the HTTP* env are not set 🤔

That seems closest to the docker default behavior with normal containers, we can take what docker set and inject the additional kind no_proxy values. If it is set AND the CLI read the env, then the env are like the user --env to docker run and we can ignore / not bother to load the .docker/config.json

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label May 23, 2021
@k8s-ci-robot k8s-ci-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Jun 22, 2021
@BenTheElder BenTheElder added lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. and removed lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. labels Jun 23, 2021
@kubernetes-sigs kubernetes-sigs deleted a comment from fejta-bot Jun 23, 2021
@kubernetes-sigs kubernetes-sigs deleted a comment from fejta-bot Jun 23, 2021
@kubernetes-sigs kubernetes-sigs deleted a comment from fejta-bot Jun 23, 2021
@BenTheElder BenTheElder added the priority/backlog Higher priority than priority/awaiting-more-evidence. label Jun 23, 2021
@BenTheElder BenTheElder removed this from the 2021 goals milestone Feb 4, 2022
@BenTheElder BenTheElder added the help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. label Feb 4, 2022
@blacksails
Copy link

blacksails commented Feb 11, 2022

I would like to contribute, and I think this could be a good first issue for me.

I don't think we should go past grabbing the client config.

Are you sure about this? In practice that would mean that you wouldn't be able to use the combination of a remote docker daemon, proxy config and kind (assuming that you would need other proxy settings for the remote docker daemon than what is defined in the default proxy config).

@BenTheElder
Copy link
Member Author

Sorry, my own github inbox is a tire fire.

Are you sure about this? In practice that would mean that you wouldn't be able to use the combination of a remote docker daemon, proxy config and kind (assuming that you would need other proxy settings for the remote docker daemon than what is defined in the default proxy config).

Yes, it will be complex and brittle, and the user can always set the *proxy environment variables when invoking kind themselves.

At the moment we have very limited support for remote docker, we try not to break it, but we don't have much in the way of special support for it. kind is intended for use with local clusters, and an alternative to remote docker host is just running kind on the remote host.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/provider/docker Issues or PRs related to docker help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. kind/bug Categorizes issue or PR as related to a bug. lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. priority/backlog Higher priority than priority/awaiting-more-evidence. priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants