From 78a0204c13ac464d2258d206fa985330e9a6c66c Mon Sep 17 00:00:00 2001 From: Tomislav Simnett Date: Mon, 29 Apr 2019 17:13:16 +0100 Subject: [PATCH 1/2] Add rewrite_non_www_to_www option --- manifests/resource/server.pp | 3 + spec/defines/resource_server_spec.rb | 92 ++++++++++++++++++++++++++ templates/server/server_header.erb | 19 +++++- templates/server/server_ssl_header.erb | 7 +- 4 files changed, 118 insertions(+), 3 deletions(-) diff --git a/manifests/resource/server.pp b/manifests/resource/server.pp index 654480b8b..2afb41d89 100644 --- a/manifests/resource/server.pp +++ b/manifests/resource/server.pp @@ -77,6 +77,8 @@ # [*www_root*] - Specifies the location on disk for files to be read from. Cannot be set in conjunction with $proxy # [*rewrite_www_to_non_www*] - Adds a server directive and rewrite rule to rewrite www.domain.com to domain.com in order to avoid # duplicate content (SEO); +# [*rewrite_non_www_to_www*] - Adds a server directive and rewrite rule to rewrite domain.com to www.domain.com in order to avoid +# duplicate content (SEO); # [*try_files*] - Specifies the locations for files to be checked as an array. Cannot be used in conjuction with $proxy. # [*proxy_cache*] - This directive sets name of zone for caching. The same zone can be used in multiple places. # [*proxy_cache_key*] - Override the default proxy_cache_key of $scheme$proxy_host$request_uri @@ -225,6 +227,7 @@ Array[String] $server_name = [$name], Optional[String] $www_root = undef, Boolean $rewrite_www_to_non_www = false, + Boolean $rewrite_non_www_to_www = false, Optional[Hash] $location_custom_cfg = undef, Optional[Hash] $location_cfg_prepend = undef, Optional[Hash] $location_cfg_append = undef, diff --git a/spec/defines/resource_server_spec.rb b/spec/defines/resource_server_spec.rb index 47323991e..06db77717 100644 --- a/spec/defines/resource_server_spec.rb +++ b/spec/defines/resource_server_spec.rb @@ -370,6 +370,64 @@ end end end + + context 'with a naked domain title' do + let(:title) { 'rspec.example.com' } + + [ + { + title: 'should not contain non-www to www rewrite', + attr: 'rewrite_non_www_to_www', + value: false, + notmatch: %r{ + ^ + \s+server_name\s+rspec\.example\.com;\n + \s+return\s+301\s+http://www\.rspec\.example\.com\$request_uri; + }x + }, + { + title: 'should contain non-www to www rewrite', + attr: 'rewrite_non_www_to_www', + value: true, + match: %r{ + ^ + \s+server_name\s+rspec\.example\.com;\n + \s+return\s+301\s+http://www\.rspec\.example\.com\$request_uri; + }x + }, + { + title: 'should rewrite non-www servername to www', + attr: 'rewrite_non_www_to_www', + value: true, + match: %r{\s+server_name\s+www.rspec.example.com;} + }, + { + title: 'should not rewrite non-www servername to www', + attr: 'rewrite_non_www_to_www', + value: false, + notmatch: %r{\s+server_name\s+www.rspec.example.com;} + } + ].each do |param| + context "when #{param[:attr]} is #{param[:value]}" do + let(:params) { default_params.merge(param[:attr].to_sym => param[:value]) } + + it { is_expected.to contain_concat__fragment("#{title}-header") } + it param[:title] do + matches = Array(param[:match]) + + if matches.all? { |m| m.is_a? Regexp } + matches.each { |item| is_expected.to contain_concat__fragment("#{title}-header").with_content(item) } + else + lines = catalogue.resource('concat::fragment', "#{title}-header").send(:parameters)[:content].split("\n") + expect(lines & Array(param[:match])).to eq(Array(param[:match])) + end + Array(param[:notmatch]).each do |item| + is_expected.to contain_concat__fragment("#{title}-header").without_content(item) + end + end + end + end + end end describe 'server_footer template content' do @@ -436,6 +494,40 @@ end end + context 'with a naked domain title' do + [ + { + title: 'should not contain non-www to www rewrite', + attr: 'rewrite_non_www_to_www', + value: false, + notmatch: %r{ + ^ + \s+server_name\s+rspec\.example\.com;\n + \s+return\s+301\s+https://www\.rspec\.example\.com\$request_uri; + }x + } + ].each do |param| + context "when #{param[:attr]} is #{param[:value]}" do + let(:params) { default_params.merge(param[:attr].to_sym => param[:value]) } + + it { is_expected.to contain_concat__fragment("#{title}-footer") } + it param[:title] do + matches = Array(param[:match]) + + if matches.all? { |m| m.is_a? Regexp } + matches.each { |item| is_expected.to contain_concat__fragment("#{title}-footer").with_content(item) } + else + lines = catalogue.resource('concat::fragment', "#{title}-footer").send(:parameters)[:content].split("\n") + expect(lines & Array(param[:match])).to eq(Array(param[:match])) + end + Array(param[:notmatch]).each do |item| + is_expected.to contain_concat__fragment("#{title}-footer").without_content(item) + end + end + end + end + end + describe 'server_ssl_header template content' do context 'with ssl' do let :params do diff --git a/templates/server/server_header.erb b/templates/server/server_header.erb index ca58dd6f8..919515140 100644 --- a/templates/server/server_header.erb +++ b/templates/server/server_header.erb @@ -1,5 +1,5 @@ # MANAGED BY PUPPET -<% if @rewrite_www_to_non_www -%> +<% if @rewrite_www_to_non_www || @rewrite_non_www_to_www -%> <%- @server_name.each do |s| -%> server { <%- if @listen_ip.is_a?(Array) then -%> @@ -19,12 +19,21 @@ server { <%- end -%> <%- end -%> <%= scope.function_template(["nginx/server/server_ipv6_listen.erb"]) %> +<%- if @rewrite_www_to_non_www -%> server_name www.<%= s.gsub(/^www\./, '') %>; <%- if @ssl_redirect or @ssl_only -%> return 301 https://<%= s.gsub(/^www\./, '') %><% if @_ssl_redirect_port.to_i != 443 %>:<%= @_ssl_redirect_port %><% end %>$request_uri; <%- else -%> return 301 http://<%= s.gsub(/^www\./, '') %>$request_uri; <%- end -%> +<%- elsif @rewrite_non_www_to_www -%> + server_name <%= s %>; + <%- if @ssl_redirect or @ssl_only -%> + return 301 https://www.<%= s %><% if @_ssl_redirect_port.to_i != 443 %>:<%= @_ssl_redirect_port %><% end %>$request_uri; + <%- else -%> + return 301 http://www.<%= s %>$request_uri; + <%- end -%> +<%- end -%> } <% end -%> @@ -47,7 +56,13 @@ server { <%- end -%> <%- end -%> <%= scope.function_template(["nginx/server/server_ipv6_listen.erb"]) %> - server_name <%= @rewrite_www_to_non_www ? @server_name.join(" ").gsub(/(^| )(www\.)?(?=[a-z0-9])/, '') : @server_name.join(" ") %>; +<%- if @rewrite_www_to_non_www -%> + server_name <%= @server_name.join(" ").gsub(/(^| )(www\.)?(?=[a-z0-9])/, '') %>; +<%- elsif @rewrite_non_www_to_www -%> + server_name <%= @server_name.join(" ").gsub(/(^| )(?=[a-z0-9])/, 'www.') %>; +<%- else %> + server_name <%= @server_name.join(" ") %>; +<%- end -%> <%- if instance_variables.any? { |iv| iv.to_s.include? 'auth_basic' } -%> <%- if defined? @auth_basic -%> auth_basic "<%= @auth_basic %>"; diff --git a/templates/server/server_ssl_header.erb b/templates/server/server_ssl_header.erb index 3832637e7..66ecd3cea 100644 --- a/templates/server/server_ssl_header.erb +++ b/templates/server/server_ssl_header.erb @@ -1,5 +1,5 @@ # MANAGED BY PUPPET -<% if @rewrite_www_to_non_www -%> +<% if @rewrite_www_to_non_www || @rewrite_non_www_to_www -%> <%- @server_name.each do |s| -%> server { <%- if @listen_ip.is_a?(Array) then -%> @@ -10,8 +10,13 @@ server { listen <%= @listen_ip %>:<%= @ssl_port %> <% if @ssl_listen_option %>ssl<% end %><% if @http2 == 'on' %> http2<% end %><% if @spdy == 'on' %> spdy<% end %><% if @listen_options %> <%= @listen_options %><% end %>; <%- end -%> <%= scope.function_template(["nginx/server/server_ssl_ipv6_listen.erb"]) %> +<%- if @rewrite_www_to_non_www -%> server_name www.<%= s.gsub(/^www\./, '') %>; return 301 https://<%= s.gsub(/^www\./, '') %>$request_uri; +<%- elsif @rewrite_non_www_to_www %> + server_name <%= s.gsub(/^www\./, '') %>; + return 301 https://www.<%= s %>$request_uri; +<%- end -%> <%= scope.function_template(["nginx/server/server_ssl_settings.erb"]) %> From 0f80c183e13e4b63fda3a9f442b3007ee2c04a96 Mon Sep 17 00:00:00 2001 From: Tomislav Simnett Date: Mon, 10 Jun 2019 09:26:51 +0100 Subject: [PATCH 2/2] Add validation so both rewrite_www_to_non_www and rewrite_non_www_to_www are not set --- manifests/resource/server.pp | 4 ++++ spec/defines/resource_server_spec.rb | 12 ++++++++++++ 2 files changed, 16 insertions(+) diff --git a/manifests/resource/server.pp b/manifests/resource/server.pp index 2afb41d89..09fa62ab9 100644 --- a/manifests/resource/server.pp +++ b/manifests/resource/server.pp @@ -279,6 +279,10 @@ fail('You must include the nginx base class before using any defined resources') } + if $rewrite_www_to_non_www == true and $rewrite_non_www_to_www == true { + fail('You must not set both $rewrite_www_to_non_www and $rewrite_non_www_to_www to true') + } + # Variables if $nginx::confd_only { $server_dir = "${nginx::conf_dir}/conf.d" diff --git a/spec/defines/resource_server_spec.rb b/spec/defines/resource_server_spec.rb index 06db77717..efde6b3b7 100644 --- a/spec/defines/resource_server_spec.rb +++ b/spec/defines/resource_server_spec.rb @@ -61,6 +61,18 @@ end end + describe 'with both $rewrite_www_to_non_www and $rewrite_non_www_to_www enabled' do + let(:params) do + default_params.merge(rewrite_non_www_to_www: true, rewrite_www_to_non_www: true) + end + + it do + is_expected.to compile.and_raise_error( + %r{You must not set both \$rewrite_www_to_non_www and \$rewrite_non_www_to_www to true} + ) + end + end + describe 'server_header template content' do [ {