8000 refactor: nginx template by buchdag · Pull Request #2550 · nginx-proxy/nginx-proxy · GitHub
[go: up one dir, main page]

Skip to content

refactor: nginx template #2550

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

Merged
merged 4 commits into from
Nov 30, 2024
Merged

refactor: nginx template #2550

merged 4 commits into from
Nov 30, 2024

Conversation

buchdag
Copy link
Member
@buchdag buchdag commented Nov 28, 2024

This PR refactor and clean up some parts of the template:

  • upstream_name was set in$globals.vhosts two times, in the VIRTUAL_HOST and VIRTUAL_HOST_MULTIPORTS loops. It's been deduplicated to the third loop over $globals.vhosts.
  • hard to read expressions like when (hasKey $fooDict $bar) (get $fooDict $bar) "defaultValue" has been replaced with the slightly clearer get $fooDict $bar | default "defaultValue".
  • removal / simplification of the excess logic revolving around setting the port to legacy for containers using VIRTUAL_HOST.

and fix a few things on the debug endpoint:

  • addition of a terminal \n character to the response so that curl | jq on the endpoint work correctly.
  • when a given path + port had more than one container, the whole RuntimeContainer struct of the additional containers where incorrectly kept, resulting in the paths being stripped from the response.

Due to the aforementioned removal of the legacy port logic, this configuration:

services:
  # ...

  app1:
    image: nginx
    environment:
      VIRTUAL_HOST: nginx-proxy.tld
      VIRTUAL_PORT: 8077
  
  app2:
    image: nginx
    environment:
      VIRTUAL_HOST: nginx-proxy.tld
      VIRTUAL_PORT: 8077
  
  app3:
    image: nginx
    environment:
      VIRTUAL_HOST_MULTIPORTS: |-
        nginx-proxy.tld:
          "/":
            port: 8077

Now result in this in the vhost.paths section of the debug endpoint response:

{
   "/":{
      "dest":"",
      "keepalive":"auto",
      "network_tag":"external",
      "ports":{
         "8077":[
            {
               "Name":"compose-app2-1"
            },
            {
               "Name":"compose-app1-1"
            },
            {
               "Name":"compose-app3-1"
            }
         ]
      },
      "proto":"http",
      "upstream":"nginx-proxy.tld"
   }
}

instead of this previously:

{
   "/":{
      "dest":"",
      "keepalive":"auto",
      "network_tag":"external",
      "ports":{
         "8077":[
            {
               "Name":"compose-app3-1"
            }
         ],
         "legacy":[
            {
               "Name":"compose-app2-1"
            },
            {
               "Name":"compose-app1-1"
            }
         ]
      },
      "proto":"http",
      <
8000
span class="pl-ent">"upstream":"nginx-proxy.tld"
   }
}

The resulting nginx configuration is inchanged.

@buchdag buchdag marked this pull request as ready for review November 28, 2024 07:58
@buchdag buchdag merged commit d433b3b into main Nov 30, 2024
4 checks passed
@buchdag buchdag deleted the refactor-template branch November 30, 2024 13:48
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.

1 participant
0