From ac74035286b9f39ab690ff7bf8fc47fd2d7384dd Mon Sep 17 00:00:00 2001 From: William Yardley Date: Wed, 12 Apr 2017 11:14:19 -0700 Subject: [PATCH] (#862) Use vendor defaults for fastcgi / uwsgi params. Rename fastcgi_params to fastcgi.conf. Don't manage files if set to non-default path. --- manifests/resource/location.pp | 13 +++-- manifests/resource/server.pp | 13 +++-- spec/defines/resource_location_spec.rb | 50 ++++++++++++++++++- spec/defines/resource_server_spec.rb | 34 ++++++++++++- .../{fastcgi_params.erb => fastcgi.conf.erb} | 9 ++-- templates/server/locations/fastcgi.erb | 2 + templates/server/uwsgi_params.erb | 29 ++++++----- 7 files changed, 117 insertions(+), 33 deletions(-) rename templates/server/{fastcgi_params.erb => fastcgi.conf.erb} (83%) diff --git a/manifests/resource/location.pp b/manifests/resource/location.pp index 7e62861fd..56dd41906 100644 --- a/manifests/resource/location.pp +++ b/manifests/resource/location.pp @@ -173,12 +173,12 @@ Optional[String] $fastcgi = undef, Optional[String] $fastcgi_index = undef, Optional[Hash] $fastcgi_param = undef, - String $fastcgi_params = "${::nginx::conf_dir}/fastcgi_params", + Optional[String] $fastcgi_params = "${::nginx::conf_dir}/fastcgi.conf", Optional[String] $fastcgi_script = undef, Optional[String] $fastcgi_split_path = undef, Optional[String] $uwsgi = undef, Optional[Hash] $uwsgi_param = undef, - String $uwsgi_params = "${nginx::config::conf_dir}/uwsgi_params", + Optional[String] $uwsgi_params = "${nginx::config::conf_dir}/uwsgi_params", Optional[String] $uwsgi_read_timeout = undef, Boolean $ssl = false, Boolean $ssl_only = false, @@ -248,15 +248,18 @@ $config_file = "${server_dir}/${server_sanitized}.conf" - if $ensure == present and $fastcgi != undef and !defined(File[$fastcgi_params]) { + # Only try to manage these files if they're the default one (as you presumably + # usually don't want the default template if you're using a custom file. + + if $ensure == present and $fastcgi != undef and !defined(File[$fastcgi_params]) and $fastcgi_params == "${::nginx::conf_dir}/fastcgi.conf" { file { $fastcgi_params: ensure => present, mode => '0644', - content => template('nginx/server/fastcgi_params.erb'), + content => template('nginx/server/fastcgi.conf.erb'), } } - if $ensure == present and $uwsgi != undef and !defined(File[$uwsgi_params]) { + if $ensure == present and $uwsgi != undef and !defined(File[$uwsgi_params]) and $uwsgi_params == "${::nginx::conf_dir}/uwsgi_params" { file { $uwsgi_params: ensure => present, mode => '0644', diff --git a/manifests/resource/server.pp b/manifests/resource/server.pp index 0825f96e9..a0f6406a6 100644 --- a/manifests/resource/server.pp +++ b/manifests/resource/server.pp @@ -194,10 +194,10 @@ Optional[String] $fastcgi = undef, Optional[String] $fastcgi_index = undef, $fastcgi_param = undef, - String $fastcgi_params = "${::nginx::conf_dir}/fastcgi_params", + Optional[String] $fastcgi_params = "${::nginx::conf_dir}/fastcgi.conf", Optional[String] $fastcgi_script = undef, Optional[String] $uwsgi = undef, - String $uwsgi_params = "${nginx::config::conf_dir}/uwsgi_params", + Optional[String] $uwsgi_params = "${nginx::config::conf_dir}/uwsgi_params", Optional[String] $uwsgi_read_timeout = undef, Array $index_files = [ 'index.html', @@ -380,15 +380,18 @@ $root = $www_root } - if $fastcgi != undef and !defined(File[$fastcgi_params]) { + # Only try to manage these files if they're the default one (as you presumably + # usually don't want the default template if you're using a custom file. + + if $fastcgi != undef and !defined(File[$fastcgi_params]) and $fastcgi_params == "${::nginx::conf_dir}/fastcgi.conf" { file { $fastcgi_params: ensure => present, mode => '0644', - content => template('nginx/server/fastcgi_params.erb'), + content => template('nginx/server/fastcgi.conf.erb'), } } - if $uwsgi != undef and !defined(File[$uwsgi_params]) { + if $uwsgi != undef and !defined(File[$uwsgi_params]) and $uwsgi_params == "${::nginx::conf_dir}/uwsgi_params" { file { $uwsgi_params: ensure => present, mode => '0644', diff --git a/spec/defines/resource_location_spec.rb b/spec/defines/resource_location_spec.rb index bb66631a6..bbb8d7162 100644 --- a/spec/defines/resource_location_spec.rb +++ b/spec/defines/resource_location_spec.rb @@ -22,7 +22,7 @@ it { is_expected.to contain_class('nginx::config') } it { is_expected.to contain_concat__fragment('server1-500-33c6aa94600c830ad2d316bb4db36724').with_content(%r{location rspec-test}) } - it { is_expected.not_to contain_file('/etc/nginx/fastcgi_params') } + it { is_expected.not_to contain_file('/etc/nginx/fastcgi.conf') } it { is_expected.not_to contain_concat__fragment('server1-800-rspec-test-ssl') } it { is_expected.not_to contain_file('/etc/nginx/rspec-test_htpasswd') } end @@ -844,7 +844,41 @@ context 'when fastcgi => "localhost:9000"' do let(:params) { { fastcgi: 'localhost:9000', server: 'server1' } } - it { is_expected.to contain_file('/etc/nginx/fastcgi_params').with_mode('0644') } + it { is_expected.to contain_file('/etc/nginx/fastcgi.conf').with_mode('0644') } + end + + context 'when fastcgi_params is non-default' do + let(:params) do + { + location: 'location', + fastcgi: 'localhost:9000', + fastcgi_params: '/etc/nginx/mycustomparams', + server: 'server1' + } + end + + it { is_expected.not_to contain_file('/etc/nginx/mycustomparams') } + it do + is_expected.to contain_concat__fragment('server1-500-' + Digest::MD5.hexdigest(params[:location].to_s)). + with_content(%r{include\s+/etc/nginx/mycustomparams;}) + end + end + + context 'when fastcgi_params is undef' do + let(:params) do + { + location: 'location', + fastcgi: 'localhost:9000', + fastcgi_params: nil, + server: 'server1' + } + end + + it { is_expected.not_to contain_file('/etc/nginx/fastcgi.conf') } + it do + is_expected.to contain_concat__fragment('server1-500-' + Digest::MD5.hexdigest(params[:location].to_s)). + without_content(%r{include\s+/etc/nginx/fastcgi.conf;}) + end end context 'when uwsgi => "unix:/home/project/uwsgi.socket"' do @@ -853,6 +887,18 @@ it { is_expected.to contain_file('/etc/nginx/uwsgi_params') } end + context 'when uwsgi_params is non-default' do + let(:params) do + { + uwsgi: 'uwsgi_upstream', + uwsgi_params: '/etc/nginx/bogusparams', + server: 'server1' + } + end + + it { is_expected.not_to contain_file('/etc/nginx/uwsgi_params') } + end + context 'when ssl_only => true' do let(:params) { { ssl_only: true, server: 'server1', www_root: '/' } } it { is_expected.not_to contain_concat__fragment('server1-500-' + Digest::MD5.hexdigest('rspec-test')) } diff --git a/spec/defines/resource_server_spec.rb b/spec/defines/resource_server_spec.rb index 7b6203225..620f31dc7 100644 --- a/spec/defines/resource_server_spec.rb +++ b/spec/defines/resource_server_spec.rb @@ -39,7 +39,7 @@ it { is_expected.to contain_concat__fragment("#{title}-header").with_content(%r{error_log\s+/var/log/nginx/www\.rspec\.example\.com\.error\.log}) } it { is_expected.to contain_concat__fragment("#{title}-footer") } it { is_expected.to contain_nginx__resource__location("#{title}-default") } - it { is_expected.not_to contain_file('/etc/nginx/fastcgi_params') } + it { is_expected.not_to contain_file('/etc/nginx/fastcgi.conf') } it do is_expected.to contain_file("#{title}.conf symlink").with('ensure' => 'link', 'path' => "/etc/nginx/sites-enabled/#{title}.conf", @@ -1002,7 +1002,28 @@ default_params.merge(fastcgi: 'localhost:9000') end - it { is_expected.to contain_file('/etc/nginx/fastcgi_params').with_mode('0644') } + it { is_expected.to contain_nginx__resource__location("#{title}-default").with_fastcgi_params('/etc/nginx/fastcgi.conf') } + it { is_expected.to contain_file('/etc/nginx/fastcgi.conf').with_mode('0644') } + end + + context 'when fastcgi_params is non-default' do + let :params do + default_params.merge(fastcgi: 'localhost:9000', + fastcgi_params: '/etc/nginx/mycustomparams') + end + + it { is_expected.to contain_nginx__resource__location("#{title}-default").with_fastcgi_params('/etc/nginx/mycustomparams') } + it { is_expected.not_to contain_file('/etc/nginx/mycustomparams') } + end + + context 'when fastcgi_params is not defined' do + let :params do + default_params.merge(fastcgi: 'localhost:9000', + fastcgi_params: nil) + end + + it { is_expected.to contain_nginx__resource__location("#{title}-default").with_fastcgi_params('nil') } + it { is_expected.not_to contain_file('/etc/nginx/fastcgi.conf') } end context 'when fastcgi_index => "index.php"' do @@ -1029,6 +1050,15 @@ it { is_expected.to contain_file('/etc/nginx/uwsgi_params').with_mode('0644') } end + context 'when uwsgi_params is non-default' do + let :params do + default_params.merge(uwsgi: 'uwsgi_upstream', + uwsgi_params: '/etc/nginx/bogusparams') + end + + it { is_expected.not_to contain_file('/etc/nginx/bogusparams') } + end + context 'when listen_port == ssl_port but ssl = false' do let :params do default_params.merge(listen_port: 80, diff --git a/templates/server/fastcgi_params.erb b/templates/server/fastcgi.conf.erb similarity index 83% rename from templates/server/fastcgi_params.erb rename to templates/server/fastcgi.conf.erb index 127e4a526..8e678c790 100644 --- a/templates/server/fastcgi_params.erb +++ b/templates/server/fastcgi.conf.erb @@ -1,16 +1,18 @@ # This file managed by puppet on host <%= @fqdn %> +fastcgi_param SCRIPT_FILENAME $document_root$fastcgi_script_name; fastcgi_param QUERY_STRING $query_string; fastcgi_param REQUEST_METHOD $request_method; fastcgi_param CONTENT_TYPE $content_type; fastcgi_param CONTENT_LENGTH $content_length; -fastcgi_param SCRIPT_FILENAME $request_filename; fastcgi_param SCRIPT_NAME $fastcgi_script_name; fastcgi_param REQUEST_URI $request_uri; fastcgi_param DOCUMENT_URI $document_uri; fastcgi_param DOCUMENT_ROOT $document_root; fastcgi_param SERVER_PROTOCOL $server_protocol; +fastcgi_param REQUEST_SCHEME $scheme; +fastcgi_param HTTPS $https if_not_empty; fastcgi_param GATEWAY_INTERFACE CGI/1.1; fastcgi_param SERVER_SOFTWARE nginx/$nginx_version; @@ -21,10 +23,5 @@ fastcgi_param SERVER_ADDR $server_addr; fastcgi_param SERVER_PORT $server_port; fastcgi_param SERVER_NAME $server_name; -fastcgi_param HTTPS $https; - # PHP only, required if PHP was built with --enable-force-cgi-redirect fastcgi_param REDIRECT_STATUS 200; - -# Mitigate httpoxy, see https://httpoxy.org/#fix-now -fastcgi_param HTTP_PROXY ""; diff --git a/templates/server/locations/fastcgi.erb b/templates/server/locations/fastcgi.erb index c2856643d..6178ba502 100644 --- a/templates/server/locations/fastcgi.erb +++ b/templates/server/locations/fastcgi.erb @@ -2,7 +2,9 @@ <% if defined? @www_root -%> root <%= @www_root %>; <% end -%> +<% if defined? @fastcgi_params -%> include <%= @fastcgi_params %>; +<% end -%> fastcgi_pass <%= @fastcgi %>; <% if @fastcgi_index -%> diff --git a/templates/server/uwsgi_params.erb b/templates/server/uwsgi_params.erb index 86b9a2de7..3725828dd 100644 --- a/templates/server/uwsgi_params.erb +++ b/templates/server/uwsgi_params.erb @@ -1,15 +1,18 @@ # This file managed by puppet on host <%= @fqdn %> -uwsgi_param QUERY_STRING $query_string; -uwsgi_param REQUEST_METHOD $request_method; -uwsgi_param CONTENT_TYPE $content_type; -uwsgi_param CONTENT_LENGTH $content_length; -uwsgi_param REQUEST_URI $request_uri; -uwsgi_param PATH_INFO $document_uri; -uwsgi_param DOCUMENT_ROOT $document_root; -uwsgi_param SERVER_PROTOCOL $server_protocol; -uwsgi_param REMOTE_ADDR $remote_addr; -uwsgi_param REMOTE_PORT $remote_port; -uwsgi_param SERVER_ADDR $server_addr; -uwsgi_param SERVER_PORT $server_port; -uwsgi_param SERVER_NAME $server_name; +uwsgi_param QUERY_STRING $query_string; +uwsgi_param REQUEST_METHOD $request_method; +uwsgi_param CONTENT_TYPE $content_type; +uwsgi_param CONTENT_LENGTH $content_length; + +uwsgi_param REQUEST_URI $request_uri; +uwsgi_param PATH_INFO $document_uri; +uwsgi_param DOCUMENT_ROOT $document_root; +uwsgi_param SERVER_PROTOCOL $server_protocol; +uwsgi_param REQUEST_SCHEME $scheme; +uwsgi_param HTTPS $https if_not_empty; + +uwsgi_param REMOTE_ADDR $remote_addr; +uwsgi_param REMOTE_PORT $remote_port; +uwsgi_param SERVER_PORT $server_port; +uwsgi_param SERVER_NAME $server_name;