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

Add SSL support #54

Closed
sameersbn opened this issue Apr 26, 2014 · 5 comments
Closed

Add SSL support #54

sameersbn opened this issue Apr 26, 2014 · 5 comments

Comments

@sameersbn
Copy link
Owner

@arno Creating an issue for the SSL support work. At the moment the SSL related comments are all over the place.

I just pulled your branch. I see you have given a lot of SSL configuration options. I think its an overkill.

My changes are only a couple of lines (maybe 10 lines or so) I will publish my branch and maybe you can check and we can add whatever else is required.
#53

@sameersbn
Copy link
Owner Author

@arnos i have published my ssl-support branch. There were a lot of use cases to take care of and I finally managed to get it done. I had to hack into the gitlab code to get things working.

Anyways, can you test the ssl support and confirm that it works for you? Instructions can be found in the README.

@arnos
Copy link

arnos commented Apr 28, 2014

Must have missed something a I thought you were pulling my branch?

I'm a bit concerned with the fact you had to modify gitlab code as their
SSL support is pretty much out of the box.

I'll check it as soon as I can.

On Monday, April 28, 2014, Sameer Naik notifications@github.com wrote:

@arnos https://github.com/arnos i have published my ssl-support branch.
There were a lot of use cases to take care of and I finally managed to get
done. I had to hack into the gitlab code to get things working.

Anyways, can you test the ssl support and confirm that it works for you?
Instructions can be found in the READMEhttps://github.com/sameersbn/docker-gitlab/blob/feature/ssl-support/README.md#ssl
.


Reply to this email directly or view it on GitHubhttps://github.com//issues/54#issuecomment-41593806
.

@sameersbn
Copy link
Owner Author

@arnos its true that gitlab SSL support is supposed to work out of the box. But since we are containerizing the application I had to make minor and harmless edits to get it working for the various use cases.

There are a lot of topologies that have to be handled and various scenarios like

  • using a load balancer like haproxy
  • ensuring that git operations work with https
  • making sure everything works when the default ports are not used, etc.

Hopefully, I think I have taken care of all scenario's.

@sameersbn
Copy link
Owner Author

@arnos i was just thinking.
maybe we should remove all ssl configurations and also remove the nginx server and instead expose the unicorn worker on port 80 so that users can configure the SSL configuration in the host.

Right now this is the general use case:

On the host you install a load balancer such as hipache/haproxy or nginx. If you are enabling SSL support then it (SSL) has to also be configured at the load balancer and as such the internal SSL configuration is more or less pointless.

Inside the container we are running an nginx server which proxies connections to the unicorn workers. So in essence we have a stack that looks like this:
request -> load balancer -> nginx -> unicorn

If we remove the internal nginx, then the stack would look like this
request -> load balancer -> unicorn

By doing this change we will effectively be converting the gitlab application from a web server to a app server, which i think fits better with docker and as a side effect your access log collection will happen at the host.

Another problem we would be solving with this change is, if i am using nginx as my load balancer and if I want to change the client_max_body_size, then I have to configure this on the internal nginx server as well as at the load balancer (just found this out today). But with this change we only end up configuring this setting at one place.

The only downside to this that I can see is that you will not be able to enable ssl support if you decide to use the application without a load balancer, which I think should be fine.

what do you think?

@arnos
Copy link

arnos commented May 13, 2014

as long as it is defined how to enable SSL from unicorn it should be fine.
again this goes back to issue #46

With an installer experience, you can ask the user if they have a load
balancer or not and if not offer to setup an nginx loadbalancer container.

On Tue, May 13, 2014 at 2:08 PM, Sameer Naik notifications@github.comwrote:

@arnos https://github.com/arnos i was just thinking.
maybe we should remove all ssl configurations and also remove the nginx
server and instead expose the unicorn worker on port 80 so that users can
configure the SSL configuration in the host.

Right now this is the general use case:

On the host you install a load balancer such as hipache/haproxy or nginx.
If you are enabling SSL support then it (SSL) has to also be configured at
the load balancer as well and as such the internal SSL configuration is
more or less pointless.

Inside the container we are running an nginx server which proxies
connections to the unicorn workers. So in essence we have a stack that
looks like this:
request -> load balancer -> nginx -> unicorn

If we remove the internal nginx, then the stack would look like this
request -> load balancer -> unicorn

By doing this change we will effectively be converting the gitlab
application from a web server to a app server, which i think fits better
with docker and as a side effect your access log collection will happen at
the host.

Another problem we would be solving with this change is, if i am using
nginx as my load balancer and if I want to change the client_max_body_size,
then I have to configure this on the internal nginx server as well as at
the load balancer (just found this out today). But with this change we only
end up configuring this setting at one place.

The only downside to this that I can see is that you will not be able to
enable ssl support if you decide to use the application without a load
balancer, which I think should be fine.


Reply to this email directly or view it on GitHubhttps://github.com//issues/54#issuecomment-42990999
.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants