Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

feat: proxy protocol support#2587

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
buchdag merged 3 commits intonginx-proxy:mainfromantoniomika:am/proxy-protocol
Jul 27, 2025

Conversation

@antoniomika
Copy link
Contributor

Similar to#1487, I took a look at adding proxy-protocol support to this project.

The changes included here add a newENABLE_PROXY_PROTOCOL env var, which will enable proxy-protocol support for all docker-gen created listeners. Because settingproxy_protocol on alisten enables the functionality for allserver's that use the same port, I opted to make this option global.

Unlike the other PR, I decided to map thex-forwarded-for (andx-forwarded-port) entry to conform with current use-cases. If setting the real ip module settings is desired, there already exists methods for adding other configurations to the template.

I also added tests to verify this functionalityDOES NOT break or modify any of the current proxy header use cases. This feature is opt-in, as nginx will stop responding to HTTP requests that aren't made using the proxy protocol.

@antoniomikaantoniomika changed the titleAm/proxy protocolProxy Protocol SupportFeb 18, 2025
@buchdagbuchdag added the type/featPR for a new feature labelFeb 22, 2025
@bvli
Copy link

I'd like to see this feature implemented as well.. Are there any reason for not merging this PR?

{{- end }}
{{ template "access_log" (dict "Enable" $globals.config.enable_access_log) }}
listen {{ $globals.config.external_http_port }} {{ $default_server }};
listen {{ $globals.config.external_http_port }} {{- $default_server }} {{- $proxy_protocol }};
Copy link
Member

Choose a reason for hiding this comment

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

question: I'm not sure of what's get rendered here ifdefault_server andproxy_protocol both have a non blank value, aren't the two strings concatenated because of{{- ? 🤔

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

Nope, everything looks correct because of this:https://github.com/nginx-proxy/nginx-proxy/pull/2587/files/6baf7bbf57785df273cd5ed5871937b9a0a1da2d#diff-a2a77c71854a4489d896f49c59a5f864e5b1e290f6b1ba16f9f39c1a32829c57R887

I made this change so the spacing doesn't look all wonky if neither are enabled.

Here's a test if you're curious:

  1. Run a proxy-protocol test case like so:
~$ make build-nginx-proxy-test-alpine build-webserver&& docker compose -f ./test/compose.base.yml -f ./test/test_proxy_protocol/test_proxy-protocol-global-enabled.yml up -d
  1. Exec the container and check the proxy lines look okay:
~$ dockerexec nginx-proxy nginx -T2>&1| grep'listen.*proxy_protocol'    listen 80 proxy_protocol;    listen 443 ssl proxy_protocol;    listen 80 proxy_protocol;    listen 443 ssl proxy_protocol;
  1. Add a default server by addingDEFAULT_HOST: "proxy-protocol-global-enabled.nginx-proxy.tld" to thenginx-proxy service' environment in./test/test_proxy_protocol/test_proxy-protocol-global-enabled.yml. Rerun the command in 1.
  2. Re-grep to check the result:
~$ dockerexec nginx-proxy nginx -T2>&1| grep'listen.*proxy_protocol'    listen 80 default_server proxy_protocol;    listen 443 ssl default_server proxy_protocol;
  1. Disable proxy protocol by setting theENABLE_PROXY_PROTOCOL environment var to false in the same file above. Rerun the command in step 1 and grep fordefault_server.
~$ dockerexec nginx-proxy nginx -T2>&1| grep'listen.*default_server'    listen 80 default_server;    listen 443 ssl default_server;
  1. Finally with both disabled:
    listen 80;    listen 443 ssl;    listen 80;    listen 443 ssl;

Rerunning the steps above on main, default server disabled looks like this:

    listen 80;    listen 443 ssl;    listen 80 ;    listen 443 ssl ;

And enabled like this:

    listen 80 default_server;    listen 443 ssl default_server;

Copy link
Member

Choose a reason for hiding this comment

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

So you've actually fixed the pre existing spacing issue, thank you 👍

@buchdag
Copy link
Member

@bvli I have close to zero practical knowledge about the proxy protocol, so it's a bit hard for me to evaluate or test this.

Aside from a few minor refactor and a question (ping@antoniomika) the PR looks fine though.

@antoniomika
Copy link
ContributorAuthor

Thanks for the review@buchdag! I addressed your comments and squashed/rebased the PR. Let me know if there's anything else I can help with.

@buchdag
Copy link
Member

Thanks for the PR and the explanations@antoniomika 👍

antoniomika reacted with heart emoji

@buchdagbuchdag merged commit6ebf9d6 intonginx-proxy:mainJul 27, 2025
3 of 6 checks passed
@buchdagbuchdag changed the titleProxy Protocol Supportfeat: proxy Protocol SupportAug 19, 2025
@buchdagbuchdag changed the titlefeat: proxy Protocol Supportfeat: proxy protocol supportAug 19, 2025
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@buchdagbuchdagbuchdag approved these changes

+1 more reviewer

@SchoNieSchoNieSchoNie left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

type/featPR for a new feature

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@antoniomika@bvli@buchdag@SchoNie

[8]ページ先頭

©2009-2025 Movatter.jp