Skip to content

Rewrite section on configuring proxies for containers#13786

Merged
dvdksn merged 1 commit into
docker:mainfrom
thaJeztah:rewrite_proxy_section
May 5, 2023
Merged

Rewrite section on configuring proxies for containers#13786
dvdksn merged 1 commit into
docker:mainfrom
thaJeztah:rewrite_proxy_section

Conversation

@thaJeztah

@thaJeztah thaJeztah commented Nov 4, 2021

Copy link
Copy Markdown
Member

Proposed changes

Unreleased project version (optional)

Related issues (optional)

@thaJeztah

Copy link
Copy Markdown
Member Author

Just pushing this as a draft; this is very unfinished (need to rewrite the last section and also mention upper/lowercase etc)

@netlify

netlify Bot commented Nov 4, 2021

Copy link
Copy Markdown

✔️ Deploy Preview for docsdocker ready!

🔨 Explore the source changes: c07a1dc

🔍 Inspect the deploy log: https://app.netlify.com/sites/docsdocker/deploys/6183bb41ca9524000751e914

😎 Browse the preview: https://deploy-preview-13786--docsdocker.netlify.app

Comment thread network/proxy.md Outdated
"httpProxy": "http://192.168.1.12:3128",
"httpsProxy": "http://192.168.1.12:3128",
"noProxy": "*.test.example.com,.example2.com,127.0.0.0/8"
"httpsProxy": "https://192.168.1.12:3129",

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 think it is supposed to be http here, quoting @djs55:

Confusingly a HTTPS proxy is a HTTP server which accepts CONNECT host:port HTTP requests and establishes a tunnel, which the client then sends TLS back and forth but the proxy doesn't understand (in some ways this is easier than in the HTTP case where the proxy has a bunch of rules about what to do with headers)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I think both should work, although it could depend on the implementation.

It's really a "fuzzy" area, without any (good) standards. e.g. Golang also made a silly (and breaking) change and started to ignore HTTP_PROXY for https connections; https://docs.docker.com/engine/release-notes/#201010

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.

I think

  • the httpProxy (HTTP_PROXY) will be used to proxy http:// connections
  • the httpsProxy (HTTPS_PROXY) will be used to proxy https:// connections

where the connection to the proxy can either be plaintext http://proxy:3128 or itself using TLS https://proxy:3128. So there can be 0, 1 or 2 layers of TLS.

If the underlying connection is TLS, then the only reason to use TLS to talk to the proxy too is to protect the authorization header which could include a base64-encoded password. The destination server is sent in the clear even with TLS (as a Server Name Indication) so the HTTP CONNECT <server:443> part is not secret.

So I think the examples are ok. My main problem with the docs as they are is that there are a confusing number of places where proxies can be configured. I think this PR makes an improvement though!

Comment thread network/proxy.md
| Variable | Dockerfile example | `docker run` example |
|:--------------|:--------------------------------------------------|:----------------------------------------------------|
| `HTTP_PROXY` | `ENV HTTP_PROXY="http://192.168.1.12:3128"` | `--env HTTP_PROXY="http://192.168.1.12:3128"` |
| `HTTPS_PROXY` | `ENV HTTPS_PROXY="https://192.168.1.12:3128"` | `--env HTTPS_PROXY="https://192.168.1.12:3128"` |

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.

Ah that one had https though… 🤔

@thaJeztah

Copy link
Copy Markdown
Member Author

I need to collect more changes and put them together, e.g. #8352 and #12463 (which also isn't correct, as "it depends")

And describe that handling of NO_PROXY is quite different (depending on what software uses it), same as for upper/lowercase cURL, wget, and Golang don't agree on those.

@usha-mandya

Copy link
Copy Markdown
Member

@thaJeztah Is this PR still relevant? Thanks.

@docker-robott

Copy link
Copy Markdown
Collaborator

Thanks for the pull request. We'd like to make our product docs better, but haven’t been able to review all the suggestions.
As our docs have also diverged, we do not have the bandwidth to review and rebase old pull requests.

If the updates are still relevant, review our contribution guidelines and rebase your pull request against the latest version of the docs, then mark it as fresh with a /remove-lifecycle stale comment.
If not, this pull request will be closed in 30 days. This helps our maintainers focus on the active pull requests.

Prevent pull requests from auto-closing with a /lifecycle frozen comment.

/lifecycle stale

@thaJeztah

Copy link
Copy Markdown
Member Author

Reopening for now (still in draft), but still something to finish.

@thaJeztah thaJeztah force-pushed the rewrite_proxy_section branch from c07a1dc to 36df827 Compare April 13, 2023 11:29
@netlify

netlify Bot commented Apr 13, 2023

Copy link
Copy Markdown

Deploy Preview for docsdocker ready!

Name Link
🔨 Latest commit 073cd2f
🔍 Latest deploy log https://app.netlify.com/sites/docsdocker/deploys/6452a1be955eb400084b5bab
😎 Deploy Preview https://deploy-preview-13786--docsdocker.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

Comment thread config/daemon/systemd.md Outdated
@dvdksn dvdksn self-assigned this Apr 13, 2023
@dvdksn dvdksn force-pushed the rewrite_proxy_section branch 3 times, most recently from 7e81ad5 to d1d76c7 Compare April 25, 2023 14:26
@dvdksn dvdksn marked this pull request as ready for review April 27, 2023 13:43
@dvdksn dvdksn self-requested a review as a code owner April 27, 2023 13:43
@dvdksn

dvdksn commented Apr 27, 2023

Copy link
Copy Markdown
Contributor

@thaJeztah I continued on some of the changes you started in this PR. PTAL and let me know if there's more stuff that we should add/change in the scope of this

@dvdksn dvdksn changed the title WIP rewrite section on configuring proxies for containers Rewrite section on configuring proxies for containers May 2, 2023
@thaJeztah

Copy link
Copy Markdown
Member Author

Oh! Thanks, sorry forgot about this one; gonna give this one a look Today.

I see there's one link failure; perhaps we need a custom id / anchor somewhere;

========================================================================
engine/reference/commandline/cli/index.html
  hash does not exist --- engine/reference/commandline/cli/index.html --> /config/daemon/systemd/#httphttps-proxy
engine/reference/commandline/dockerd/index.html
  hash does not exist --- engine/reference/commandline/dockerd/index.html --> /config/daemon/systemd/#httphttps-proxy
========================================================================

@dvdksn

dvdksn commented May 3, 2023

Copy link
Copy Markdown
Contributor

Fixed the broken link upstream in docker/cli#4249 - will need to backport that one

@thaJeztah thaJeztah left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Might need to give this another pass later, but let me post what I had written down, before I forget to submit 😂

Comment thread config/daemon/systemd.md Outdated
Comment thread config/daemon/systemd.md Outdated
Comment thread config/daemon/systemd.md Outdated
Comment thread config/daemon/systemd.md Outdated
Comment thread engine/release-notes/20.10.md Outdated
Comment thread network/proxy.md Outdated
```dockerfile
FROM alpine
ARG HTTPS_PROXY
RUN apk update # uses httpsProxy from ~/.docker/config.json

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

It might be worth also showing an example somewhere where;

  • the proxy is set manually during build (--build-arg)
  • the proxy is set manually during "run" (--env)

perhaps also an example where the proxy is taken from the current environment (this could be a follow-up). Use-case here would be that the host is configured with a proxy; by default, (build-)containers do not inherit those environment variables (by design), but it may be that you need them to have the same proxy configured.

# proxy configuration on my host
env | grep _PROXY
HTTPS_PROXY=https://my-hosts-proxy.example
HTTP_PROXY=http://my-hosts-proxy.example

# run a container that inherits the values from those by setting the name of the env-var, but no value (no `=`);
docker run -e HTTPS_PROXY -e HTTP_PROXY --rm alpine sh -c 'env | grep _PROXY'
HTTPS_PROXY=https://my-hosts-proxy.example
HTTP_PROXY=http://my-hosts-proxy.example

docker build --no-cache --progress=plain --build-arg HTTPS_PROXY --build-arg HTTP_PROXY -<<'EOF'
FROM alpine
RUN env | grep -i _PROXY
EOF

#5 [2/2] RUN env | grep -i _PROXY
#0 0.106 HTTPS_PROXY=https://my-proxy.example
#0 0.106 https_proxy=https://my-proxy.example
#0 0.106 http_proxy=http://my-proxy.example
#0 0.106 HTTP_PROXY=http://my-proxy.example
#5 DONE 0.1s

Comment thread network/proxy.md Outdated
```json
{
"proxies": {
"https://docker-daemon1.example.com": {

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I think this one needs to be tcp://docker-daemon1.example.com (but would have to verify that).

Do you think (for a follow-up?) it would be useful to show an example as well where both "default" and "other is configured?

cat ~/.docker/config.json
{
	"proxies": {
		"default": {
			"httpProxy": "http://192.168.1.12:1111"
		},
		"unix:///Users/thajeztah/.docker/run/docker.sock": {
			"httpProxy": "http://192.168.1.12:2222"
		}
	}
}

And we could show examples, including how it interacts with docker context;

docker context ls
NAME                TYPE                DESCRIPTION                               DOCKER ENDPOINT                                   KUBERNETES ENDPOINT   ORCHESTRATOR
default *           moby                Current DOCKER_HOST based configuration   unix:///var/run/docker.sock
desktop-linux       moby                                                          unix:///Users/thajeztah/.docker/run/docker.sock


# uses "default" (if set)
docker run --rm  alpine sh -c 'env | grep _PROXY'
HTTP_PROXY=http://192.168.1.12:1111

# uses proxy for "unix:///Users/thajeztah/.docker/run/docker.sock"
docker --host unix:///Users/thajeztah/.docker/run/docker.sock run --rm  alpine sh -c 'env | grep _PROXY'
HTTP_PROXY=http://192.168.1.12:2222

# also uses proxy for "unix:///Users/thajeztah/.docker/run/docker.sock"
docker --context=desktop-linux run --rm  alpine sh -c 'env | grep _PROXY'
HTTP_PROXY=http://192.168.1.12:2222

(and, yes, we should integrate that with docker context itself, and have it be part of the context's options)

☝️ I should mention that it looks like there's currently a bug in the CLI, and setting a "per-host" proxy for a ssh:// connection doesn't work (it tries to look up using our "magic" http://docker.example.com hostname, which is used internally.

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.

That's a cool example. Let's add it in a follow-up - for now I can try to clarify the confusion with the key names, and maybe add both a default and a "named" daemon.

TBH, I find contexts a confusing topic overall. Maybe that's an area worth spending some time trying to clarify.

Comment thread network/proxy.md Outdated
Comment thread network/proxy.md Outdated
Comment thread network/proxy.md Outdated
Comment thread engine/release-notes/20.10.md Outdated
Signed-off-by: David Karlsson <david.karlsson@docker.com>

Co-authored-by: Sebastiaan van Stijn <github@gone.nl>
@dvdksn dvdksn force-pushed the rewrite_proxy_section branch from d1d76c7 to 073cd2f Compare May 3, 2023 18:02

@thaJeztah thaJeztah left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

LGTM ❤️

(I can't approve the PR because I opened it, but if you can LGTM it @dvdksn, then we can merge)

@dvdksn dvdksn merged commit 8c8d304 into docker:main May 5, 2023
@thaJeztah thaJeztah deleted the rewrite_proxy_section branch May 5, 2023 08:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants