[46] secure connections to backends

[yanns]

Fix https://github.com/hjr3/weldr/issues/46

@hjr3 can you check this? If it's ok, I can rebase it. The problem I have by testing that is that we do not support hostname now. The certificate validation fails with IP.

[hjr3]

first glance at this looks good. i will look more in depth when i get home later tonight.

i think hyper provides some sort of DNS functionality. i will look into that more.

[yanns]

Yes we could let the user configure hostnames, and use https://github.com/hyperium/hyper/blob/master/src/client/dns.rs to resolve them.

Should we add this in this PR, or in another?

[hjr3]

@yanns looks good! thank you for the work. i am working on the pool refactor right now. once i get the abstraction working right, we can add some sort of enum type to allow a SocketAddr or a hostname.

It is not ideal that we have to do the hostname lookup each time we make a backend request. Ideally, we have a dns resolver local to the proxy that honors the TTL. We can save that for a future time though!

Rebase and let's get this merged.

[yanns]

One question: instead of cloning handle2:

        let handle2 = handle.clone();
        let work = listener.incoming().for_each(move |(socket, addr)| {
            let client = Client::configure()
                .connector(HttpsConnector::new(4, &handle2.clone()))
                .build(&handle2.clone());
            let service = Proxy {
                client: client,
                pool: pool.clone(),
            };

            let http = Http::new();
            http.bind_connection(&handle2, socket, addr, service);
            Ok(())
        });

we could use it directly:

        let handle2 = handle.clone();
        let work = listener.incoming().for_each(move |(socket, addr)| {
            let client = Client::configure()
                .connector(HttpsConnector::new(4, &handle2))
                .build(&handle2);
            let service = Proxy {
                client: client,
                pool: pool.clone(),
            };

            let http = Http::new();
            http.bind_connection(&handle2, socket, addr, service);
            Ok(())
        });

My intuition would prefer avoiding cloning is good. What is your opinion?

[yanns]

About DNS, I agree with you that we should not resolve the DNS on each call. My approach would be to resolve the DNS on startup, and then honour the TTL with another thread (management thread?) But we could start with a naive approach, and later optimize it.

[hjr3]

@yanns yes, you can remove clone() from &handle2. The Handle type uses Rc, so clone is very cheap. That being said, if we can avoid it then we should. I assumed that code wanted ownership of the handle. I looked more closely and it does not. Good catch.

[yanns]

OK I removed the clone() completely.