puppet: Simplify uploads handling.

`uploads-route.noserve` and `uploads-route.internal` contained
identical location blocks for `/upload`, since differentiation was
necessary for Trusty until 33c941407b72; move the now-common sections
into `app`.

This the only differences between internal and S3 serving as a single
block which should be included or not based on config; move it to a
file which may or may not be placed in `app.d/`.

puppet/zulip/files/nginx/zulip-include-frontend/app

@@ -54,8 +54,7 @@ location / {
 # web, and thus need API headers added.  We can't easily collapse
 # these blocks with the /api block, because regular expressions take
 # priority over paths in nginx's order-of-operations, and we don't
-# want to override the tornado configuration for /api/v1/events.  The
+# want to override the tornado configuration for /api/v1/events.
-# last is handled via uploads-route.
 location /thumbnail {
     include /etc/nginx/zulip-include/api_headers;
 
@@ -66,6 +65,11 @@ location /avatar {
 
     include uwsgi_params;
 }
+location /user_uploads {
+    include /etc/nginx/zulip-include/api_headers;
+
+    include uwsgi_params;
+}
 
 # Send all API routes not covered above to Django via uWSGI
 location /api/ {
@@ -74,5 +78,4 @@ location /api/ {
     include uwsgi_params;
 }
 
-include /etc/nginx/zulip-include/uploads.route;
 include /etc/nginx/zulip-include/app.d/*.conf;

puppet/zulip/files/nginx/zulip-include-maybe/uploads-internal.conf

@@ -5,13 +5,3 @@ location /serve_uploads {
     include /etc/nginx/zulip-include/uploads.types;
     alias /home/zulip/uploads/files;
 }
-
-# This Django route not under /api is shared between mobile and web
-# and thus needs API headers added, in addition to the configuration
-# required to have this URL be served by Django.
-
-location /user_uploads {
-    include /etc/nginx/zulip-include/api_headers;
-
-    include uwsgi_params;
-}

puppet/zulip/files/nginx/zulip-include-maybe/uploads-route.noserve

@@ -1,9 +0,0 @@
-# This Django route not under /api is shared between mobile and web
-# and thus needs API headers added, in addition to the configuration
-# required to have this URL be served by Django.
-
-location /user_uploads {
-    include /etc/nginx/zulip-include/api_headers;
-
-    include uwsgi_params;
-}

puppet/zulip/manifests/nginx.pp

@@ -29,23 +29,38 @@ class zulip::nginx {
     notify  => Service['nginx'],
   }
 
+  # Configuration for how uploaded files are served.  The default is
+  # to serve uploaded files using using the `nginx` `internal`
+  # feature via django-sendfile2, which basically does an internal
+  # redirect and returns the file content from nginx in an
+  # HttpResponse that would otherwise have been a redirect.
+  #
+  # For installations using S3 to serve uploaded files, we want Django
+  # to handle the /serve_uploads route, so that it can do authentication
+  # and serve a redirect.
   $no_serve_uploads = zulipconf('application_server', 'no_serve_uploads', '')
-  if $no_serve_uploads != '' {
+  if $no_serve_uploads == '' {
-    # If we're not serving uploads locally, set the appropriate API headers for it.
+    file { '/etc/nginx/zulip-include/app.d/uploads-internal.conf':
-    $uploads_route = 'puppet:///modules/zulip/nginx/zulip-include-maybe/uploads-route.noserve'
-  } else {
-    $uploads_route = 'puppet:///modules/zulip/nginx/zulip-include-maybe/uploads-route.internal'
-  }
-
-  file { '/etc/nginx/zulip-include/uploads.route':
       ensure  => file,
       require => Package[$zulip::common::nginx],
       owner   => 'root',
       group   => 'root',
       mode    => '0644',
       notify  => Service['nginx'],
-    source  => $uploads_route,
+      source  => 'puppet:///modules/zulip/nginx/zulip-include-maybe/uploads-internal.conf',
     }
+  } else {
+    file { '/etc/nginx/zulip-include/app.d/uploads-internal.conf':
+      ensure  => absent,
+    }
+  }
+
+  # Removed 2021-04 in Zulip 4.0; these lines can be removed in Zulip
+  # version 5.0 and later.
+  file { '/etc/nginx/zulip-include/uploads.route':
+    ensure  => absent,
+  }
+
 
   file { '/etc/nginx/dhparam.pem':
     ensure  => file,