Valian / docker-nginx-auto-ssl

Docker image for automatic generation of SSL certs using Let's encrypt and Open Resty
https://hub.docker.com/r/valian/docker-nginx-auto-ssl/
MIT License
411 stars 102 forks source link

added websocket proxying support #10

Closed tverdohleb closed 5 years ago

tverdohleb commented 6 years ago

@Valian do you have any thoughts about this PR?

Valian commented 6 years ago

@tverdohleb First, thanks for this PR!

I would change two things here:

  1. You are always adding Upgrade header. It should be done only for ws requests, like in the following snippet:

    http {
    map $http_upgrade $connection_upgrade {
        default upgrade;
        '' close;
    }
    
    upstream websocket {
        server 192.168.100.10:8010;
    }
    
    server {
        listen 8020;
        location / {
            proxy_pass http://websocket;
            proxy_http_version 1.1;
            proxy_set_header Upgrade $http_upgrade;
            proxy_set_header Connection $connection_upgrade;
        }
    }
    }

    Also, I believe proxy_http_version 1.1; directive is needed, as I have plans to add http 2.0 support

  2. Could you also add a short note to README that this repository support websockets?

Sorry for a late reply, I have problems with GH notifications.

srghma commented 5 years ago

any update?

Valian commented 5 years ago

Replaced and implemented by #22