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

DNS-Name to IP resolution support #779

Open
JanMikes opened this issue Nov 1, 2022 · 8 comments
Open

DNS-Name to IP resolution support #779

JanMikes opened this issue Nov 1, 2022 · 8 comments
Assignees
Labels
z-enhancement ⬆️ Product Enhancement

Comments

@JanMikes
Copy link

JanMikes commented Nov 1, 2022

Hi, referred to the documentation https://unit.nginx.org/configuration/#proxying, i should be able to proxy request to any valid hostname, but dont know how to do it for docker container, defined and run in the same docker-compose file.

What im trying to do is to proxy all http requests from localhost:8080/.well-known/mercure to the mercure service, but i get 400 response from unit when running put request to config:

/usr/local/bin/docker-entrypoint.sh: Looking for configuration snippets in /docker-entrypoint.d/...
/usr/local/bin/docker-entrypoint.sh: Applying configuration /docker-entrypoint.d/config.json
2022/11/01 21:02:15 [error] 12#12 invalid address "mercure"
/usr/local/bin/docker-entrypoint.sh: Error: HTTP response status code is '400'
{
	"error": "Invalid configuration.",
	"detail": "The \"proxy\" address is invalid \"mercure\""
}

docker-compose.yml (simplified):

version: "3"

services:
    web:
        image: unit
        ports:
            - "8080:8080"

    mercure:
        image: dunglas/mercure

config.json (simplified):

{
    "listeners": {
        "*:8080": {
            "pass": "routes"
        }
    },

    "routes": [
        {
            "match": {
                "uri": "/.well-known/mercure"
            },
            "action": {
                "proxy": "http://mercure"
            }
        }
    ]
}

For "proxy": "http://mercure" i tried different values like mercure, mercure:80, http://mercure:80/ and so on, but nothing worked, the result was the same.

I was able to get it working with traefik, but my goal was to not add additional reverse proxy and having Unit used for that.

If i can provide any further details that would help troubleshooting, please let me know.

Thank you!

@tippexs
Copy link
Contributor

tippexs commented Nov 1, 2022

@JanMikes we are currently working on this. At the moment we do not support any DNS or name bases addresses they need to be resolved. Only IPv4/6 and Unix domain sockets are valid inputs. Will flag this as bug / enhancement and let you know as soon as a fix will be out.

For the mean time using the IP will solve this issue. Please note that you will be able to assign a static ip to the container in the docker network. Let me know if this will work for you.

@tippexs tippexs added z-enhancement ⬆️ Product Enhancement z-roadmap labels Nov 1, 2022
@JanMikes
Copy link
Author

JanMikes commented Nov 2, 2022

Thank you for such fast reply, i will be looking forward.

I can confirm that proxying to static IP works fine.

@tippexs tippexs changed the title Unable to proxy to docker container - invalid configuration (invalid is address) DNS-Name to IP resolution support Nov 7, 2022
@travisbell
Copy link

@tippexs As you guys consider supporting DNS resolution, may I ask for a specific feature? It would be really nice if you could respect a record's TTL and refresh addresses as they update. Nginx is missing this feature (it's pay walled as part of Nginx Plus) and is super frustrating to work around. If Unit were to add DNS resolution, I think it makes sense to always do "what's right" (I feel the same way about Nginx of course... 🤷🏼‍♂️).

Thanks!

@Pavlusha311245
Copy link

Any updates? :)

@ac000
Copy link
Member

ac000 commented Apr 24, 2024

This is currently very low priority... we are currently not pushing the proxy capabilities and are concentrating on the application server side of things...

@rcknr
Copy link

rcknr commented Jul 17, 2024

Well, proxy capability is already there. The lack of DNS resolution is making it fairly unpractical. Of course, there are workarounds but why not just add this very important feature right in?

@ac000
Copy link
Member

ac000 commented Jul 17, 2024

Well, proxy capability is already there. The lack of DNS resolution is making it fairly unpractical. Of course, there are workarounds but why not just add this very important feature right in?

Heh, if it were that simple I'm sure we would have.

A while back I did look at lifting the DNS lookup code from nginx, but due to differences in the code bases, I think it really needs attacking from mostly scratch.

Also the proxy code has some issues and has become a lower priority for us in general.

If it's vital to you, you could always try this quick and dirty hack (the real solution is way more involved than this....)

diff --git ./src/nxt_sockaddr.c ./src/nxt_sockaddr.c
index 32941893..2f729532 100644
--- ./src/nxt_sockaddr.c
+++ ./src/nxt_sockaddr.c
@@ -4,6 +4,9 @@
  * Copyright (C) NGINX, Inc.
  */
 
+#include <stdbool.h>
+#include <arpa/inet.h>
+
 #include <nxt_main.h>
 
 
@@ -14,6 +17,7 @@ static u_char *nxt_inet6_ntop(u_char *addr, u_char *buf, u_char *end);
 static nxt_sockaddr_t *nxt_sockaddr_unix_parse(nxt_mp_t *mp, nxt_str_t *addr);
 static nxt_sockaddr_t *nxt_sockaddr_inet6_parse(nxt_mp_t *mp, nxt_str_t *addr);
 static nxt_sockaddr_t *nxt_sockaddr_inet_parse(nxt_mp_t *mp, nxt_str_t *addr);
+static nxt_sockaddr_t *nxt_sockaddr_host_parse(nxt_mp_t *mp, nxt_str_t *addr);
 
 
 nxt_sockaddr_t *
@@ -540,6 +544,20 @@ nxt_sockaddr_parse(nxt_mp_t *mp, nxt_str_t *addr)
 }
 
 
+static inline bool is_hostname(const u_char *str)
+{
+    while (*str) {
+        if ((*str != '.' && *str != ':') && (*str < '0' || *str > '9')) {
+            return true;
+        }
+
+        str++;
+    }
+
+    return false;
+}
+
+
 nxt_sockaddr_t *
 nxt_sockaddr_parse_optport(nxt_mp_t *mp, nxt_str_t *addr)
 {
@@ -556,8 +574,11 @@ nxt_sockaddr_parse_optport(nxt_mp_t *mp, nxt_str_t *addr)
     } else if (addr->start[0] == '[' || nxt_inet6_probe(addr)) {
         sa = nxt_sockaddr_inet6_parse(mp, addr);
 
-    } else {
+    } else if (!is_hostname(addr->start)) {
         sa = nxt_sockaddr_inet_parse(mp, addr);
+
+    } else {
+        sa = nxt_sockaddr_host_parse(mp, addr);
     }
 
     if (nxt_fast_path(sa != NULL)) {
@@ -770,6 +791,65 @@ nxt_sockaddr_inet_parse(nxt_mp_t *mp, nxt_str_t *addr)
 }
 
 
+static nxt_sockaddr_t *
+nxt_sockaddr_host_parse(nxt_mp_t *mp, nxt_str_t *addr)
+{
+    int              err;
+    char             ipaddr[NXT_INET6_ADDR_STR_LEN];
+    char             ipport[NXT_INET6_ADDR_STR_LEN + 8];
+    char             *host, *p;
+    nxt_str_t        str;
+    nxt_sockaddr_t   *sa;
+    struct addrinfo  *res;
+
+    host = strndup((char *)addr->start, addr->length);
+    p = memchr(host, ':', strlen(host));
+    if (p != NULL) {
+        *p = '\0';
+
+        if (strlen(p + 1) > 5) {
+            nxt_thread_log_error(NXT_LOG_ERR, "invalid port in \"%V\"", addr);
+            goto out_free;
+        }
+    }
+
+    err = getaddrinfo(host, NULL, NULL, &res);
+    if (err) {
+        goto out_free;
+    }
+
+    if (res->ai_family == AF_INET) {
+        inet_ntop(AF_INET, &((struct sockaddr_in *)res->ai_addr)->sin_addr,
+                  ipaddr, NXT_INET6_ADDR_STR_LEN);
+
+        sprintf(ipport, "%s%s%s", ipaddr, p ? ":" : "", p ? p + 1 : "");
+
+        /* Do this manually as nxt_length() is using sizeof(ipport) */
+        str.start = (u_char *)ipport;
+        str.length = strlen(ipport);
+        sa = nxt_sockaddr_inet_parse(mp, &str);
+#if (NXT_INET6)
+    } else {
+        inet_ntop(AF_INET6, &((struct sockaddr_in6 *)res->ai_addr)->sin6_addr,
+                  ipaddr, NXT_INET6_ADDR_STR_LEN);
+
+        sprintf(ipport, "[%s]%s%s", ipaddr, p ? ":" : "", p ? p + 1 : "");
+
+        str.start = (u_char *)ipport;
+        str.length = strlen(ipport);
+        sa = nxt_sockaddr_inet6_parse(mp, &str);
+#endif
+    }
+
+    freeaddrinfo(res);
+
+out_free:
+    free(host);
+
+    return sa;
+}
+
+
 in_addr_t
 nxt_inet_addr(u_char *buf, size_t length)
 {

@rcknr
Copy link

rcknr commented Jul 18, 2024

Thanks for sharing! That's a start. Although basic this already covers a lot. I would argue it's worth adding to project's backlog.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
z-enhancement ⬆️ Product Enhancement
Projects
None yet
Development

No branches or pull requests

7 participants