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

New backward compatible multiport syntax for VIRTUAL_PORT variable #2130

Closed
wants to merge 1 commit into from

Conversation

pini-gh
Copy link
Contributor

@pini-gh pini-gh commented Jan 9, 2023

This pull request is intended to support containers exposing more than one entrypoint through different ports. I've discussed this topic in issue #1504.

New VIRTUAL_PORT syntax

VIRTUAL_PORT = [ <virtual_port> | <multiport> ];
multiport    = port, { ",",  port };
port         = <virtual_port> [ ":", <virtual_path> [ ":", <virtual_dest> ]];

As many nginx upstream definitions are created as specified ports. The same goes for location blocks. Routing is performed based on the virtual path specified for each port.

Example

VIRTUAL_HOST: "multiport.example.com"
VIRTUAL_PORT: "9220:~ ^/(admin|fonts?|images|webmin)/,10901,20901:/ws2p,30901:/gva/playground"

would produce one nginx upstream definition per port, and as many location blocs:

# multiport.example.com:10901
upstream multiport.example.com-10901 {
        ## Can be connected with "docker-gen-bridge" network
        # blah
        server 172.29.0.5:10901;
}
# multiport.example.com:20901/ws2p
upstream multiport.example.com-5c7ebef820fe004e45e3af1d0c47971594d028b2-20901 {
        ## Can be connected with "docker-gen-bridge" network
        # blah
        server 172.29.0.5:20901;
}
# multiport.example.com:30901/gva/playground
upstream multiport.example.com-1f02ce2421b17d828edaabfc3014360891bb0be3-30901 {
        ## Can be connected with "docker-gen-bridge" network
        # blah
        server 172.29.0.5:30901;
}
# multiport.example.com:9220~ ^/(admin|fonts?|images|webmin)/
upstream multiport.example.com-cae8bfc2ea1fe6bb6fda08727ab065e8fed98aa2-9220 {
        ## Can be connected with "docker-gen-bridge" network
        # blah
        server 172.29.0.5:9220;
}
server {
        server_name multiport.example.com;
        listen 80 ;
        access_log /var/log/nginx/access.log vhost;
        location / {
                proxy_pass http://multiport.example.com-10901;
        }
        location /ws2p {
                proxy_pass http://multiport.example.com-5c7ebef820fe004e45e3af1d0c47971594d028b2-20901;
        }
        location /gva/playground {
                proxy_pass http://multiport.example.com-1f02ce2421b17d828edaabfc3014360891bb0be3-30901;
        }
        location ~ ^/(admin|fonts?|images|webmin)/ {
                proxy_pass http://multiport.example.com-cae8bfc2ea1fe6bb6fda08727ab065e8fed98aa2-9220;
        }
}

Backward compatible

Legacy VIRTUAL_PORT syntax with only one port still works the very same way. I've ran the full test suite with only 3 unrelated errors (which I reproduce with version 1.0.4 as well):

FAILED test/test_ssl/test_dhparam.py::test_custom_dhparam_is_supported_per_site - Exception: Failed to process CLI request:
ERROR test/test_raw-ip-vhost.py::test_raw_ipv4_vhost_forwards_to_web1
ERROR test/test_raw-ip-vhost.py::test_raw_ipv6_vhost_forwards_to_web2

Added related documentation

See section 'Multiport Syntax' of the README.

Specific test case

I've added a test case in folder test/test_multiport_syntax. It features a web server exposing 4 ports. All these ports are reachable through different paths. This test case runs successfully on my box.

@pini-gh pini-gh changed the title Pini multiport syntax New backward compatible multiport syntax for VIRTUAL_PORT variable Jan 9, 2023
@pini-gh pini-gh force-pushed the pini-multiport-syntax branch 2 times, most recently from 5e6d117 to 58cb570 Compare January 11, 2023 01:02
@pini-gh
Copy link
Contributor Author

pini-gh commented Jan 11, 2023

Hi @buchdag, I thin this PR is ready for review now. Thanks in advance.

@rhansen
Copy link
Collaborator

rhansen commented Jan 11, 2023

I'm having trouble parsing the proposed syntax. Can you express it in ABNF?

Also, see my comment in #1504.

@pini-gh
Copy link
Contributor Author

pini-gh commented Jan 12, 2023

@rhansen:

I'm having trouble parsing the proposed syntax. Can you express it in ABNF?

This is EBNF but for the leaf items <virtual_port>, <virtual_path>, and <virtual_dest> which accept the same values as the legacy VIRTUAL_PORT, VIRTUAL_PATH, and VIRTUAL_DEST variables.

Here is how I would put it in ABNF:

VIRTUAL_PORT = [<virtual_port> / <multiport>]
multiport    = port *("," port)
port         = <virtual_port> [":" <virtual_path> [":" <virtual_dest>]]

@rhansen
Copy link
Collaborator

rhansen commented Jan 12, 2023

Apologies, I somehow misread the colon separator as a comma which confused me greatly. I understand now.

@pini-gh
Copy link
Contributor Author

pini-gh commented Jan 20, 2023

Rebased on 'main'.

@buchdag
Copy link
Member

buchdag commented Jan 20, 2023

@pini-gh I like this. I'll try to review it this week-end. Do you remember (of the top of your head, I'll search myself later) issues that would be solved by this PR or other PR that implements the same feature ?

@pini-gh
Copy link
Contributor Author

pini-gh commented Jan 20, 2023

@buchdag Thanks

Browsing the issues I've found these ones : #560, #911, #939, #1042, #1066, #1112, #1382, #1463, #1504, #2050.
Please double-check; I may have misinterpreted some of them.

Some of them ask explicitly for accessing the same container through different vhosts / vports. I think this PR is relevant for them as well because it brings a different solution for the very same problem: one container exposing several ports.

EDIT: I forgot to answer about other PRs. Here they are: #259, #1405. And I know @rhansen is working on something similar. See his comments in #1504.

@pini-gh pini-gh force-pushed the pini-multiport-syntax branch 3 times, most recently from b1a92ec to 05e923e Compare January 24, 2023 19:59
@pini-gh pini-gh force-pushed the pini-multiport-syntax branch 2 times, most recently from 65c9374 to 316f1f6 Compare February 6, 2023 15:53
@VincentSC
Copy link

What is the status for this? Any help needed?

@rhansen
Copy link
Collaborator

rhansen commented Feb 14, 2023

What is the status for this? Any help needed?

@VincentSC See the discussion in #1504. Two things need to happen: the dust needs to settle on the desired syntax, and the code needs infrastructure changes to support whatever syntax we end up with. I've been slowly chipping away at the prerequisite code changes in separate pull requests.

@ironrai
Copy link

ironrai commented Sep 18, 2023

Is there hope that this will be resumed in the near future? I need it a lot

@pini-gh
Copy link
Contributor Author

pini-gh commented Sep 19, 2023

Is there hope that this will be resumed in the near future? I need it a lot

You can use image pinidh/nginx-proxy:1.3.1-pini2.

@pini-gh pini-gh force-pushed the pini-multiport-syntax branch 2 times, most recently from 75cc452 to ed248c6 Compare December 12, 2023 20:00
@buchdag buchdag added type/feat PR for a new feature scope/multiport Issue or PR related to multi port proxying labels Dec 19, 2023
@pini-gh pini-gh force-pushed the pini-multiport-syntax branch 2 times, most recently from 3a36672 to 4c647ab Compare December 22, 2023 11:01
@pini-gh pini-gh force-pushed the pini-multiport-syntax branch 2 times, most recently from 104863a to aa22257 Compare March 11, 2024 17:25
Syntax:
  VIRTUAL_PORT = [ <virtual_port> | <multiport> ];
  multiport    = port, { ",",  port };
  port         = <virtual_port> [ ":", <virtual_path> [ ":", <virtual_dest> ]];

Example with multiport syntax:
  VIRTUAL_HOST: "multiport.example.com"
  VIRTUAL_PORT: "9220:~ ^/(admin|fonts?|images|webmin)/,10901,20901:/ws2p,30901:/gva/playground"

Produces:
  # multiport.example.com:10901
  upstream multiport.example.com-10901 {
      # Exposed ports: [{   10901  tcp } {   20901  tcp } {   30901  tcp } {   9220  tcp }]
      # Default virtual port: 80
      # VIRTUAL_PORT: 10901
      ## Can be connected with "docker-gen-bridge" network
      # blah
      server 172.29.0.5:10901;
  }
  # multiport.example.com:20901/ws2p
  upstream multiport.example.com-5c7ebef820fe004e45e3af1d0c47971594d028b2-20901 {
      # Exposed ports: [{   10901  tcp } {   20901  tcp } {   30901  tcp } {   9220  tcp }]
      # Default virtual port: 80
      # VIRTUAL_PORT: 20901
      ## Can be connected with "docker-gen-bridge" network
      # blah
      server 172.29.0.5:20901;
  }
  # multiport.example.com:30901/gva/playground
  upstream multiport.example.com-1f02ce2421b17d828edaabfc3014360891bb0be3-30901 {
      # Exposed ports: [{   10901  tcp } {   20901  tcp } {   30901  tcp } {   9220  tcp }]
      # Default virtual port: 80
      # VIRTUAL_PORT: 30901
      ## Can be connected with "docker-gen-bridge" network
      # blah
      server 172.29.0.5:30901;
  }
  # multiport.example.com:9220~ ^/(admin|fonts?|images|webmin)/
  upstream multiport.example.com-cae8bfc2ea1fe6bb6fda08727ab065e8fed98aa2-9220 {
      # Exposed ports: [{   10901  tcp } {   20901  tcp } {   30901  tcp } {   9220  tcp }]
      # Default virtual port: 80
      # VIRTUAL_PORT: 9220
      ## Can be connected with "docker-gen-bridge" network
      # blah
      server 172.29.0.5:9220;
  }
  server {
      server_name multiport.example.com;
      access_log /var/log/nginx/access.log vhost;
      listen 80 ;
      location / {
          proxy_pass http://multiport.example.com-10901;
      }
      location /ws2p {
          proxy_pass http://multiport.example.com-5c7ebef820fe004e45e3af1d0c47971594d028b2-20901/;
      }
      location /gva/playground {
          proxy_pass http://multiport.example.com-1f02ce2421b17d828edaabfc3014360891bb0be3-30901;
      }
      location ~ ^/(admin|fonts?|images|webmin)/ {
          proxy_pass http://multiport.example.com-cae8bfc2ea1fe6bb6fda08727ab065e8fed98aa2-9220;
      }
  }

This feature is discussed in that upstream issue:
nginx-proxy#1504
@buchdag
Copy link
Member

buchdag commented May 9, 2024

Superseded by #2434

@buchdag buchdag closed this May 9, 2024
@pini-gh
Copy link
Contributor Author

pini-gh commented May 12, 2024

Closing this PR now that #2434 is merged.

@pini-gh pini-gh deleted the pini-multiport-syntax branch May 13, 2024 12:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
scope/multiport Issue or PR related to multi port proxying type/feat PR for a new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants