From 6c69b3e75ee38fbf26ea76e7f0548c38b5eb7745 Mon Sep 17 00:00:00 2001
From: Jarkko Oranen <oranenj@iki.fi>
Date: Thu, 6 Apr 2017 20:40:57 +0300
Subject: [PATCH] Avoid spurious location block when redirecting to SSL in
 another server block

Fixes #1029
---
 manifests/resource/server.pp         |  8 ++++++--
 spec/defines/resource_server_spec.rb | 22 ++++++++++++++++++++--
 2 files changed, 26 insertions(+), 4 deletions(-)

diff --git a/manifests/resource/server.pp b/manifests/resource/server.pp
index f8ab7e015..db6125794 100644
--- a/manifests/resource/server.pp
+++ b/manifests/resource/server.pp
@@ -370,7 +370,7 @@
     $_ssl_redirect_port = $ssl_port
   }
 
-  # Suppress unneded stuff in non-SSL location block when certain conditions are
+  # Suppress unneeded stuff in non-SSL location block when certain conditions are
   # met.
   if (($ssl == true) and ($ssl_port == $listen_port)) or ($ssl_redirect) {
     $ssl_only = true
@@ -378,7 +378,11 @@
     $ssl_only = false
   }
 
-  if $use_default_location == true {
+  # If we're redirecting to SSL, the default location block is useless, *unless*
+  # SSL is enabled for this server
+  # either       and    ssl -> true
+  # ssl redirect and no ssl -> false
+  if ($ssl_redirect != true or $ssl == true) and $use_default_location == true {
     # Create the default location reference for the server
     nginx::resource::location {"${name_sanitized}-default":
       ensure                      => $ensure,
diff --git a/spec/defines/resource_server_spec.rb b/spec/defines/resource_server_spec.rb
index de588f14a..1463a50ad 100644
--- a/spec/defines/resource_server_spec.rb
+++ b/spec/defines/resource_server_spec.rb
@@ -898,12 +898,30 @@
         it { is_expected.to contain_concat__fragment("#{title}-header").with_content(%r{  return 301 https://\$host:9787\$request_uri;}) }
       end
 
-      context 'ssl_redirect should set ssl_only' do
-        let(:params) { { ssl_redirect: true } }
+      context 'ssl_redirect should set ssl_only when ssl => true' do
+        let(:params) do
+          {
+            ssl_redirect: true,
+            ssl: true,
+            ssl_key: 'dummy.key',
+            ssl_cert: 'dummy.crt'
+          }
+        end
 
         it { is_expected.to contain_nginx__resource__location("#{title}-default").with_ssl_only(true) }
       end
 
+      context 'ssl_redirect should not include default location when ssl => false' do
+        let(:params) do
+          {
+            ssl_redirect: true,
+            ssl: false
+          }
+        end
+
+        it { is_expected.not_to contain_nginx__resource__location("#{title}-default") }
+      end
+
       context 'SSL cert and key are both set to fully qualified paths' do
         let(:params) { { ssl: true, ssl_cert: '/tmp/foo.crt', ssl_key: '/tmp/foo.key:' } }