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

Fix nginx emerg when TAIGA_REDIRECT_TO_SSL=true #20

Closed
wants to merge 1 commit into from
Closed

Fix nginx emerg when TAIGA_REDIRECT_TO_SSL=true #20

wants to merge 1 commit into from

Conversation

NokiDev
Copy link

@NokiDev NokiDev commented Nov 24, 2019

Was experiencing an issue when using TAIGA_REDIRECT_TO_SSL

Nginx was using two default_server directives leading to an horrible nginx emerg
nginx: [emerg] a duplicate default server for 0.0.0.0:80 in /etc/nginx/nginx.conf:34

I fixed it but not sure how it was intended to work, should we provide a volume mapping for passing certificates ?

Cheers

@zicklag
Copy link
Contributor

zicklag commented Nov 25, 2019

I think we decided that we didn't want to do SSL termination inside the container in favor of doing it in a reverse proxy that is outside of the container. Is that right @blackandred?

@blackandred
Copy link
Contributor

Hi, thank you @NokiDev for the contribution.

Unfortunately this looks like a part of functionality we wanted to delete from container as the container was forked from other project. As part of #16 I attempted to remove SSL functionality from container, as the best way to use SSL is to have a reverse proxy and maintaining such SSL support is painful and difficult to test.

blackandred added a commit that referenced this pull request Nov 27, 2019
@NokiDev
Copy link
Author

NokiDev commented Nov 27, 2019

Oh, okay that makes much more sense thanks for your reply.
It's fine to me, I'm already using a reverse proxy for managing my certificates.

Since you seems to have done the necessary in your commit 953404d38a24e8bf0e7ba179a0bf68751c9e1f5d
I will close this PR

@NokiDev NokiDev closed this Nov 27, 2019
@blackandred
Copy link
Contributor

Thanks 😉

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.

3 participants