Finish prom_scraper configuration

- Remove unnecessary locations from nginx.conf.erb

All locations removed from nginx config, for port which is only used for the metrics endpoint.
Replaced catch-all servername with internal servername.

- Adjust unit tests

Put tests into correct context
Fix naming of contexts (rubocop)

- Adjust nginx.conf.erb template for better indentation and less blank lines
- Improve scrape TLS file templates

jobs/cloud_controller_ng/templates/nginx.conf.erb

@@ -30,15 +30,16 @@ http {
 
   keepalive_timeout    <%= p("cc.server_keepalive_timeout") %> 20;
 
-  <% if_p("cc.nginx_rate_limit_general") do %>
+<% if_p("cc.nginx_rate_limit_general") do -%>
   limit_req_zone $http_authorization zone=all:10m rate=<%=p("cc.nginx_rate_limit_general")['limit'] %>;
-  <% end %>
+<% end -%>
+
+<% if_p("cc.nginx_rate_limit_zones") do -%>
+  <% p("cc.nginx_rate_limit_zones").each do |zone| -%>
+limit_req_zone $http_authorization zone=<%= zone['name'] %>:10m rate=<%= zone['limit'] %>;
+  <% end -%>
+<% end -%>
 
-  <% if_p("cc.nginx_rate_limit_zones") do %>
-    <% p("cc.nginx_rate_limit_zones").each do |zone| %>
-  limit_req_zone $http_authorization zone=<%= zone['name'] %>:10m rate=<%= zone['limit'] %>;
-    <% end %>
-  <% end %>
   limit_req_status 429;
 
   client_max_body_size <%= p("cc.client_max_body_size") %>; #already enforced upstream/but doesn't hurt.
@@ -62,8 +63,8 @@ http {
     server_name  _;
     server_name_in_redirect off;
     <% if p("request_timeout_in_seconds").to_i > 0 %>
-      proxy_send_timeout          <%= p("request_timeout_in_seconds") %>;
-      proxy_read_timeout          <%= p("request_timeout_in_seconds") %>;
+    proxy_send_timeout          <%= p("request_timeout_in_seconds") %>;
+    proxy_read_timeout          <%= p("request_timeout_in_seconds") %>;
     <% end %>
     proxy_buffering             off;
     proxy_set_header            Host $host;
@@ -72,7 +73,6 @@ http {
     proxy_redirect              off;
     proxy_connect_timeout       10;
 
-
     location /internal/v4/ {
       proxy_pass http://cloud_controller;
     }
@@ -118,16 +118,16 @@ http {
     }
   }
 
-<% if_p("cc.prom_scraper_tls.public_cert", "cc.prom_scraper_tls.private_key", "cc.prom_scraper_tls.ca_cert") do %>
+<% if_p("cc.prom_scraper_tls.public_cert", "cc.prom_scraper_tls.private_key", "cc.prom_scraper_tls.ca_cert") do -%>
   server {
     listen <%= p("cc.prom_metrics_server_tls_port") %> ssl;
     include prom_scraper_mtls.conf;
 
-    server_name  "--"; # This is yet another invalid catch-all name. See the docs at: http://nginx.org/en/docs/http/server_names.html . search for "In catch-all server examples the strange name “_” can be seen:"
+    server_name  "<%= p("cc.internal_service_hostname") %>";
     server_name_in_redirect off;
     <% if p("request_timeout_in_seconds").to_i > 0 %>
-      proxy_send_timeout          <%= p("request_timeout_in_seconds") %>;
-      proxy_read_timeout          <%= p("request_timeout_in_seconds") %>;
+    proxy_send_timeout          <%= p("request_timeout_in_seconds") %>;
+    proxy_read_timeout          <%= p("request_timeout_in_seconds") %>;
     <% end %>
     proxy_buffering             off;
     proxy_set_header            Host $host;
@@ -136,81 +136,44 @@ http {
     proxy_redirect              off;
     proxy_connect_timeout       10;
 
-
-    location /internal/v4/ {
-      proxy_pass http://cloud_controller;
-    }
-
-    location ~ /internal/v3/staging/.*/(droplet_completed|build_completed) {
-      proxy_pass http://cloud_controller;
-    }
-
-    location ~ /internal/v4/(droplets|buildpack_cache)/.*/upload {
-      # Allow download the droplets and buildpacks
-      if ($request_method = GET){
-        proxy_pass http://cloud_controller;
-      }
-
-      # Allow large uploads
-      client_max_body_size <%= p("cc.app_bits_max_body_size") %>; #already enforced upstream/but doesn't hurt.
-
-      # Pass altered request body to this location
-      upload_pass   @cc_uploads;
-
-      # Store files to this directory
-      upload_store /var/vcap/data/cloud_controller_ng/tmp/staged_droplet_uploads;
-
-      # Allow uploaded files to be read only by user
-      upload_store_access user:r;
-
-      # Set specified fields in request body
-      upload_set_form_field "droplet_path" $upload_tmp_path;
-
-      #on any error, delete uploaded files.
-      upload_cleanup 400-505;
-    }
-
-    include local_blobstore_downloads.conf;
-
-    # Pass altered request body to a backend
-    location @cc_uploads {
+    location /internal/v4/metrics {
       proxy_pass http://cloud_controller;
     }
   }
-<% end %>
+<% end -%>
 
   # This block handles public endpoints over TLS
   server {
-  listen <%= p("cc.public_tls.port") %> ssl;
-  include nginx_server_public_tls.conf;
+    listen <%= p("cc.public_tls.port") %> ssl;
+    include nginx_server_public_tls.conf;
 
-  server_name  _;
-  server_name_in_redirect off;
-  <% if p("request_timeout_in_seconds").to_i > 0 %>
+    server_name  _;
+    server_name_in_redirect off;
+    <% if p("request_timeout_in_seconds").to_i > 0 %>
     proxy_send_timeout          <%= p("request_timeout_in_seconds") %>;
     proxy_read_timeout          <%= p("request_timeout_in_seconds") %>;
-  <% end %>
+    <% end -%>
 
-  include local_blobstore_downloads.conf;
-  include nginx_external_endpoints.conf;
+    include local_blobstore_downloads.conf;
+    include nginx_external_endpoints.conf;
   }
 
   <% unless p('temporary_disable_non_tls_endpoints')  %>
   # This block handles public endpoints over non-TLS secured HTTP
   # This is required for backwards compatibility during a rolling-deploy
   server {
-  <% if p("cc.nginx.ip").empty? %>
-    listen    <%= p("cc.external_port") %>;
-  <% else %>
-    listen    <%= p("cc.nginx.ip") %>:<%= p("cc.external_port") %>;
-  <% end %>
+  <% if p("cc.nginx.ip").empty? -%>
+  listen    <%= p("cc.external_port") %>;
+  <% else -%>
+  listen    <%= p("cc.nginx.ip") %>:<%= p("cc.external_port") %>;
+  <% end -%>
 
     server_name  _;
     server_name_in_redirect off;
     <% if p("request_timeout_in_seconds").to_i > 0 %>
-      proxy_send_timeout          <%= p("request_timeout_in_seconds") %>;
-      proxy_read_timeout          <%= p("request_timeout_in_seconds") %>;
-    <% end %>
+    proxy_send_timeout          <%= p("request_timeout_in_seconds") %>;
+    proxy_read_timeout          <%= p("request_timeout_in_seconds") %>;
+    <% end -%>
 
     include local_blobstore_downloads.conf;
     include nginx_external_endpoints.conf;

jobs/cloud_controller_ng/templates/scrape.crt.erb

@@ -1,4 +1 @@
-<% if_p('cc.prom_scraper_tls.public_cert') do %>
-<%= p('cc.prom_scraper_tls.public_cert') %>
-<% end %>
-
+<%= p('cc.prom_scraper_tls.public_cert', '') %>

jobs/cloud_controller_ng/templates/scrape.key.erb

@@ -1,3 +1 @@
-<% if_p('cc.prom_scraper_tls.private_key') do %>
-<%= p('cc.prom_scraper_tls.private_key') %>
-<% end %>
+<%= p('cc.prom_scraper_tls.private_key', '') %>

jobs/cloud_controller_ng/templates/scrape_ca.crt.erb

@@ -1,3 +1 @@
-<% if_p("cc.prom_scraper_tls.ca_cert") do %>
-<%= p("cc.prom_scraper_tls.ca_cert") %>
-<% end %>
+<%= p('cc.prom_scraper_tls.ca_cert', '') %>

spec/cloud_controller_ng/nginx_spec.rb

@@ -30,38 +30,63 @@ module Test
               expect(@rendered_file).to include('log_format main escape=json')
             end
           end
-        end
-      end
 
-      context 'prom_scraper with all propeties set' do
-        let(:manifest_properties) { { 'cc' => { 'prom_scraper_tls' => { 'public_cert' => 'a public cert', 'private_key' => 'a private key', 'ca_cert' => 'an authority'  } } } }
+          context 'when nginx_rate_limit_general is configured' do
+            let(:manifest_properties) { { 'cc' => { 'nginx_rate_limit_general' => { 'limit' => '200000' } } } }
 
-        it 'renders prom scraper server' do
-          expect(@rendered_file).to include('include prom_scraper_mtls.conf')
-        end
-      end
+            it 'renders limit_req_zone' do
+              expect(@rendered_file).to include('limit_req_zone $http_authorization zone=all:10m rate=200000;')
+            end
+          end
 
-      context 'prom_scraper with public_cert not set' do
-        let(:manifest_properties) { { 'cc' => { 'prom_scraper_tls' => { 'private_key' => 'a private key', 'ca_cert' => 'an authority'  } } } }
+          context 'when nginx_rate_limit_zones are configured' do
+            let(:manifest_properties) { { 'cc' => { 'nginx_rate_limit_zones' => [{ 'name' => 'zone_a', 'limit' => '10000' }, { 'name' => 'zone_b', 'limit' => '5000' }] } } }
 
-        it 'does not render prom scraper server' do
-          expect(@rendered_file).not_to include('include prom_scraper_mtls.conf')
-        end
-      end
+            it 'renders both zones' do
+              expect(@rendered_file).to include('limit_req_zone $http_authorization zone=zone_a:10m rate=10000;')
+              expect(@rendered_file).to include('limit_req_zone $http_authorization zone=zone_b:10m rate=5000;')
+            end
+          end
 
-      context 'prom_scraper with private_key not set' do
-        let(:manifest_properties) { { 'cc' => { 'prom_scraper_tls' => { 'public_cert' => 'a public cert', 'ca_cert' => 'an authority'  } } } }
+          context 'when nginx.ip is configured' do
+            let(:manifest_properties) { { 'cc' => { 'nginx' => { 'ip' => '192.168.200.1' } } } }
 
-        it 'does not render prom scraper server' do
-          expect(@rendered_file).not_to include('include prom_scraper_mtls.conf')
-        end
-      end
+            it 'renders nginx.ip' do
+              expect(@rendered_file).to include('listen    192.168.200.1:9022;')
+            end
+          end
 
-      context 'prom_scraper with ca_cert not set' do
-        let(:manifest_properties) { { 'cc' => { 'prom_scraper_tls' => { 'public_cert' => 'a public cert', 'private_key' => 'a private key'  } } } }
+          context 'when all properties of prom_scraper are set' do
+            let(:manifest_properties) { { 'cc' => { 'prom_scraper_tls' => { 'public_cert' => 'a public cert', 'private_key' => 'a private key', 'ca_cert' => 'an authority' } } } }
 
-        it 'does not render prom scraper server' do
-          expect(@rendered_file).not_to include('include prom_scraper_mtls.conf')
+            it 'renders prom scraper server' do
+              expect(@rendered_file).to include('include prom_scraper_mtls.conf')
+            end
+          end
+
+          context 'when public_cert of prom_scraper is not set' do
+            let(:manifest_properties) { { 'cc' => { 'prom_scraper_tls' => { 'private_key' => 'a private key', 'ca_cert' => 'an authority' } } } }
+
+            it 'does not render prom scraper server' do
+              expect(@rendered_file).not_to include('include prom_scraper_mtls.conf')
+            end
+          end
+
+          context 'when private_key of prom_scraper is not set' do
+            let(:manifest_properties) { { 'cc' => { 'prom_scraper_tls' => { 'public_cert' => 'a public cert', 'ca_cert' => 'an authority' } } } }
+
+            it 'does not render prom scraper server' do
+              expect(@rendered_file).not_to include('include prom_scraper_mtls.conf')
+            end
+          end
+
+          context 'when ca_cert of prom_scraper is not set' do
+            let(:manifest_properties) { { 'cc' => { 'prom_scraper_tls' => { 'public_cert' => 'a public cert', 'private_key' => 'a private key' } } } }
+
+            it 'does not render prom scraper server' do
+              expect(@rendered_file).not_to include('include prom_scraper_mtls.conf')
+            end
+          end
         end
       end
     end