Skip to content

Generate Nginx includes for advanced configuration (e.g. SSL)#5381

Merged
joshmoore merged 6 commits intoome:developfrom
manics:nginx-includes
Aug 15, 2017
Merged

Generate Nginx includes for advanced configuration (e.g. SSL)#5381
joshmoore merged 6 commits intoome:developfrom
manics:nginx-includes

Conversation

@manics
Copy link
Copy Markdown
Member

@manics manics commented Jul 12, 2017

What this PR does

This creates a minimal nginx configuration file that can be included in a fixed file created by a sysadmin. This allows custom nginx options to be defined once (e.g. SSL options), instead of having to modify the generated web config after every upgrade, and means any Nginx server options can be used.

Testing this PR

  1. Install a production OMERO.web in the standard way
  2. Install Nginx.
  3. Create a omero-web nginx wrapper file with an include statement for the generated omero-web config. This file is exclusively managed by the sysadmin e.g. /etc/nginx/conf.d/omero-web-wrapper.conf:
server {
    listen 80;
    server_name $hostname;

    # SSL configuration ...

    sendfile on;
    client_max_body_size 0;

    # Include generated file from omero web config nginx-location:
    include /opt/omero/web/omero-web-location.include;
}
  1. Generate the minimal omero-web configuration in the location specified by your include statement. The expectation is that this would be routinely regenerated on every OMERO.web upgrade. e.g.
omero web config nginx-location > /opt/omero/web/omero-web-location.include
  1. Start nginx
  2. Start OMERO.web: omero web start
  3. OMERO.web should work as normal

Related reading

Notes

  • omero web config nginx-location generates the location blocks only, other server options are now left for the sysadmin to manage e.g. sendfile on;, client_max_body_size 0;.
  • I've removed the separate upstream block since there was only one server which means it's redundant, and it would be impossible to auto-generate a load-balanced upstream configuration since you'd need to know the addresses of all OMERO.web backends.

@manics manics changed the title Generate Nginx includes for advanced configuration Generate Nginx includes for advanced configuration (e.g. SSL) Jul 12, 2017
@bramalingam
Copy link
Copy Markdown
Member

Followed the steps as suggested, OMERO.web works as expected. Looks good to merge (to me)

@@ -0,0 +1,30 @@
#sendfile on;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps either remove these or extend this comment block to say that such properties should be set by the sysadmin. Perhaps include a full working example?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll add a comment. Can you explain what you mean by a full working example?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

server {
    listen 80;
    server_name $hostname;

    # SSL configuration ...

    sendfile on;
    client_max_body_size 0;

    # Include generated file from omero web config nginx-location:
    include /opt/omero/web/omero-web-location.include;
}

from the description of this PR.

Copy link
Copy Markdown
Member Author

@manics manics Jul 14, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, I get you. Just worried that it's another piece of commented code that will never be tested. It's essentially omero web config nginx with the location blocks replaced by a single include /opt/omero/web/omero-web-location.include; so if we do modify the existing nginx templates we should also modify this comment.

manics added 3 commits July 13, 2017 16:03
Also add a test to check the comment and generated location more or less matches
 the full recommended nginx config.
@manics
Copy link
Copy Markdown
Member Author

manics commented Jul 19, 2017

I've updated the comment, and added a test to ensure the comment in nginx-location matches the generated nginx config

@manics
Copy link
Copy Markdown
Member Author

manics commented Jul 20, 2017

--rebased-to #5387

@kennethgillen
Copy link
Copy Markdown
Member

Can confirm webclient is working with these changes - big 👍 💯 - I'd need guidance on how to run the tests, or let someone else try the test.

@jburel jburel added the develop label Jul 30, 2017
@joshmoore
Copy link
Copy Markdown
Member

joshmoore commented Aug 15, 2017

Using omero-web-docker:

$ perl -i -pe 's/ARG OMERO_VERSION=latest/ARG OMERO_VERSION=OMERO-DEV-merge-build/' Dockerfile
$ docker build -t josh-web .
$ docker run --rm -ti --entrypoint /opt/omero/web/venv/bin/python josh-web /opt/omero/web/OMERO.web/bin/omero web config nginx-location > omero.conf
$ docker run -d --name josh-web -p 4080:4080 josh-web
$ docker run -d --name josh-nginx --link josh-web:web -p 9090:80 -v `pwd`/nginx.conf:/etc/nginx/nginx.conf:ro -v `pwd`/omero.conf:/omero.conf:ro nginx

with nginx.conf:

user  nginx;
worker_processes  1;

error_log  /var/log/nginx/error.log warn;
pid        /var/run/nginx.pid;


events {
    worker_connections  1024;
}


http {
    include       /etc/nginx/mime.types;
    default_type  application/octet-stream;

    log_format  main  '$remote_addr - $remote_user [$time_local] "$request" '
                      '$status $body_bytes_sent "$http_referer" '
                      '"$http_user_agent" "$http_x_forwarded_for"';

    access_log  /var/log/nginx/access.log  main;

    sendfile        on;
    #tcp_nopush     on;

    keepalive_timeout  65;

    #gzip  on;

    include /etc/nginx/conf.d/*.conf;

    server {
      listen 80;
      server_name $hostname;

      sendfile on;
      client_max_body_size 0;

      # Include generated file from omero web config nginx-location:
      include /omero.conf;
    }
}

for a passing docker test (barring statics which is under discussion) 👍

Merging and updating the card for the necessary docs.

@joshmoore joshmoore merged commit 4510ef3 into ome:develop Aug 15, 2017
@manics manics deleted the nginx-includes branch August 15, 2017 10:56
@sbesson sbesson added this to the 5.4.0 milestone Oct 6, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants