From fcb47d43513fb32f4d57914a220047a047bff8f7 Mon Sep 17 00:00:00 2001 From: William Yardley Date: Tue, 20 Sep 2016 13:48:03 -0700 Subject: [PATCH] add $confd_only option and tests, add more tests to streamhost (see also #879) --- manifests/config.pp | 78 ++++++++---------- manifests/init.pp | 2 + manifests/resource/location.pp | 8 +- manifests/resource/streamhost.pp | 38 +++++---- manifests/resource/vhost.pp | 30 ++++--- spec/classes/config_spec.rb | 114 +++++++++++++++++++++++++-- spec/defines/resource_stream_spec.rb | 11 +++ spec/defines/resource_vhost_spec.rb | 13 +++ templates/conf.d/nginx.conf.erb | 2 + 9 files changed, 217 insertions(+), 79 deletions(-) diff --git a/manifests/config.pp b/manifests/config.pp index 9d5801ec6..91926a494 100644 --- a/manifests/config.pp +++ b/manifests/config.pp @@ -16,6 +16,7 @@ class nginx::config( ### START Module/App Configuration ### $client_body_temp_path = $::nginx::params::client_body_temp_path, + $confd_only = false, $confd_purge = false, $conf_dir = $::nginx::params::conf_dir, $daemon_user = $::nginx::params::daemon_user, @@ -76,7 +77,6 @@ $keepalive_requests = '100', $log_format = {}, $mail = false, - $stream = false, $multi_accept = 'off', $names_hash_bucket_size = '64', $names_hash_max_size = '512', @@ -139,6 +139,7 @@ if ($proxy_conf_template != undef) { warning('The $proxy_conf_template parameter is deprecated and has no effect.') } + validate_bool($confd_only) validate_bool($confd_purge) validate_bool($vhost_purge) if ( $proxy_cache_path != false) { @@ -227,21 +228,25 @@ file { "${conf_dir}/conf.stream.d": ensure => directory, } - if $confd_purge == true { - File["${conf_dir}/conf.stream.d"] { - purge => true, - recurse => true, - } - } file { "${conf_dir}/conf.d": ensure => directory, } - if $confd_purge == true { - File["${conf_dir}/conf.d"] { - purge => true, - recurse => true, - notify => Class['::nginx::service'], + if $confd_purge { + # Err on the side of caution - make sure *both* $vhost_purge and + # $confd_purge are set if $confd_only is set, before purging files + # ${conf_dir}/conf.d + if (($confd_only and $vhost_purge) or !$confd_only) { + File["${conf_dir}/conf.d"] { + purge => true, + recurse => true, + notify => Class['::nginx::service'], + } + File["${conf_dir}/conf.stream.d"] { + purge => true, + recurse => true, + notify => Class['::nginx::service'], + } } } @@ -273,25 +278,35 @@ owner => $daemon_user, } - if $stream { - file { "${conf_dir}/streams-available": + unless $confd_only { + file { "${conf_dir}/sites-available": ensure => directory, owner => $sites_available_owner, group => $sites_available_group, mode => $sites_available_mode, } - - if $vhost_purge == true { - File["${conf_dir}/streams-available"] { + file { "${conf_dir}/sites-enabled": + ensure => directory, + } + if $vhost_purge { + File["${conf_dir}/sites-available"] { + purge => true, + recurse => true, + } + File["${conf_dir}/sites-enabled"] { purge => true, recurse => true, } } - file { "${conf_dir}/streams-enabled": ensure => directory, + owner => $sites_available_owner, + group => $sites_available_group, + mode => $sites_available_mode, + } + file { "${conf_dir}/streams-available": + ensure => directory, } - if $vhost_purge == true { File["${conf_dir}/streams-enabled"] { purge => true, @@ -300,31 +315,6 @@ } } - file { "${conf_dir}/sites-available": - ensure => directory, - owner => $sites_available_owner, - group => $sites_available_group, - mode => $sites_available_mode, - } - - if $vhost_purge == true { - File["${conf_dir}/sites-available"] { - purge => true, - recurse => true, - } - } - - file { "${conf_dir}/sites-enabled": - ensure => directory, - } - - if $vhost_purge == true { - File["${conf_dir}/sites-enabled"] { - purge => true, - recurse => true, - } - } - file { "${conf_dir}/nginx.conf": ensure => file, content => template($conf_template), diff --git a/manifests/init.pp b/manifests/init.pp index 2ea9950b0..25a27cd74 100644 --- a/manifests/init.pp +++ b/manifests/init.pp @@ -76,6 +76,7 @@ ### END Nginx Configuration ### START Module/App Configuration ### + $confd_only = undef, $confd_purge = undef, $conf_dir = undef, $daemon_user = undef, @@ -234,6 +235,7 @@ client_body_temp_path => $client_body_temp_path, client_max_body_size => $client_max_body_size, confd_purge => $confd_purge, + confd_only => $confd_only, conf_dir => $conf_dir, conf_template => $conf_template, daemon_user => $daemon_user, diff --git a/manifests/resource/location.pp b/manifests/resource/location.pp index 79a8b5d25..3c0bbb441 100644 --- a/manifests/resource/location.pp +++ b/manifests/resource/location.pp @@ -363,7 +363,13 @@ } $vhost_sanitized = regsubst($vhost, ' ', '_', 'G') - $config_file = "${::nginx::config::conf_dir}/sites-available/${vhost_sanitized}.conf" + if $::nginx::config::confd_only { + $vhost_dir = "${::nginx::config::conf_dir}/conf.d" + } else { + $vhost_dir = "${::nginx::config::conf_dir}/sites-available" + } + + $config_file = "${vhost_dir}/${vhost_sanitized}.conf" $location_sanitized_tmp = regsubst($location, '\/', '_', 'G') $location_sanitized = regsubst($location_sanitized_tmp, '\\\\', '_', 'G') diff --git a/manifests/resource/streamhost.pp b/manifests/resource/streamhost.pp index 7ec69dbc3..15fdae2fe 100644 --- a/manifests/resource/streamhost.pp +++ b/manifests/resource/streamhost.pp @@ -106,11 +106,15 @@ "${mode} is not valid. It should be 4 digits (0644 by default).") # Variables - $streamhost_dir = "${::nginx::config::conf_dir}/streams-available" - $streamhost_enable_dir = "${::nginx::config::conf_dir}/streams-enabled" - $streamhost_symlink_ensure = $ensure ? { - 'absent' => absent, - default => 'link', + if $::nginx::config::confd_only { + $streamhost_dir = "${::nginx::config::conf_dir}/conf.stream.d" + } else { + $streamhost_dir = "${::nginx::config::conf_dir}/streams-available" + $streamhost_enable_dir = "${::nginx::config::conf_dir}/streams-enabled" + $streamhost_symlink_ensure = $ensure ? { + 'absent' => absent, + default => 'link', + } } $name_sanitized = regsubst($name, ' ', '_', 'G') @@ -134,10 +138,11 @@ } concat { $config_file: - owner => $owner, - group => $group, - mode => $mode, - notify => Class['::nginx::service'], + owner => $owner, + group => $group, + mode => $mode, + notify => Class['::nginx::service'], + require => File[$streamhost_dir], } concat::fragment { "${name_sanitized}-header": @@ -146,12 +151,13 @@ order => '001', } - file{ "${name_sanitized}.conf symlink": - ensure => $streamhost_symlink_ensure, - path => "${streamhost_enable_dir}/${name_sanitized}.conf", - target => $config_file, - require => Concat[$config_file], - notify => Class['::nginx::service'], + unless $::nginx::config::confd_only { + file{ "${name_sanitized}.conf symlink": + ensure => $streamhost_symlink_ensure, + path => "${streamhost_enable_dir}/${name_sanitized}.conf", + target => $config_file, + require => [Concat[$config_file], File[$streamhost_enable_dir]], + notify => Class['::nginx::service'], + } } - } diff --git a/manifests/resource/vhost.pp b/manifests/resource/vhost.pp index 3e17d581b..c3969e117 100644 --- a/manifests/resource/vhost.pp +++ b/manifests/resource/vhost.pp @@ -558,11 +558,15 @@ "${mode} is not valid. It should be 4 digits (0644 by default).") # Variables - $vhost_dir = "${::nginx::config::conf_dir}/sites-available" - $vhost_enable_dir = "${::nginx::config::conf_dir}/sites-enabled" - $vhost_symlink_ensure = $ensure ? { - 'absent' => absent, - default => 'link', + if $::nginx::config::confd_only { + $vhost_dir = "${::nginx::config::conf_dir}/conf.d" + } else { + $vhost_dir = "${::nginx::config::conf_dir}/sites-available" + $vhost_enable_dir = "${::nginx::config::conf_dir}/sites-enabled" + $vhost_symlink_ensure = $ensure ? { + 'absent' => absent, + default => 'link', + } } $name_sanitized = regsubst($name, ' ', '_', 'G') @@ -597,7 +601,7 @@ group => $group, mode => $mode, notify => Class['::nginx::service'], - require => [File[$vhost_dir], File[$vhost_enable_dir]], + require => File[$vhost_dir], } $ssl_only = ($ssl == true) and (($ssl_port + 0) == ($listen_port + 0)) @@ -701,12 +705,14 @@ } } - file{ "${name_sanitized}.conf symlink": - ensure => $vhost_symlink_ensure, - path => "${vhost_enable_dir}/${name_sanitized}.conf", - target => $config_file, - require => [File[$vhost_dir], File[$vhost_enable_dir], Concat[$config_file]], - notify => Class['::nginx::service'], + unless $::nginx::config::confd_only { + file{ "${name_sanitized}.conf symlink": + ensure => $vhost_symlink_ensure, + path => "${vhost_enable_dir}/${name_sanitized}.conf", + target => $config_file, + require => [File[$vhost_dir], File[$vhost_enable_dir], Concat[$config_file]], + notify => Class['::nginx::service'], + } } create_resources('::nginx::resource::map', $string_mappings) diff --git a/spec/classes/config_spec.rb b/spec/classes/config_spec.rb index 063f309a6..94f21dfe5 100644 --- a/spec/classes/config_spec.rb +++ b/spec/classes/config_spec.rb @@ -19,6 +19,15 @@ mode: '0644' ) end + it do + is_expected.to contain_file('/etc/nginx/conf.stream.d').only_with( + path: '/etc/nginx/conf.stream.d', + ensure: 'directory', + owner: 'root', + group: 'root', + mode: '0644' + ) + end it do is_expected.to contain_file('/etc/nginx/conf.mail.d').only_with( path: '/etc/nginx/conf.mail.d', @@ -632,6 +641,24 @@ end end + context 'when confd_only true' do + let(:params) { { confd_only: true } } + it do + is_expected.to contain_file('/etc/nginx/conf.d').without( + %w( + ignore + purge + recurse + ) + ) + is_expected.not_to contain_file('/etc/nginx/sites-available') + is_expected.not_to contain_file('/etc/nginx/sites-enabled') + is_expected.to contain_file('/etc/nginx/nginx.conf').without_content(%r{include /path/to/nginx/sites-enabled/\*;}) + is_expected.not_to contain_file('/etc/nginx/streams-available') + is_expected.not_to contain_file('/etc/nginx/streams-enabled') + end + end + context 'when vhost_purge true' do let(:params) { { vhost_purge: true } } it do @@ -648,6 +675,51 @@ end end + context 'when confd_purge true, vhost_purge true, and confd_only true' do + let(:params) do + { + confd_purge: true, + confd_only: true, + vhost_purge: true + } + end + it do + is_expected.to contain_file('/etc/nginx/conf.d').with( + purge: true, + recurse: true + ) + end + it do + is_expected.to contain_file('/etc/nginx/conf.stream.d').with( + purge: true, + recurse: true + ) + end + end + + context 'when confd_purge true, vhost_purge default (false), confd_only true' do + let(:params) do + { + confd_purge: true, + confd_only: true + } + end + it do + is_expected.to contain_file('/etc/nginx/conf.d').without( + %w( + purge + ) + ) + end + it do + is_expected.to contain_file('/etc/nginx/conf.stream.d').without( + %w( + purge + ) + ) + end + end + context 'when vhost_purge false' do let(:params) { { vhost_purge: false } } it do @@ -677,12 +749,42 @@ ) ) end - end - - context 'when stream true' do - let(:params) { { stream: true } } - it { is_expected.to contain_file('/etc/nginx/streams-available') } - it { is_expected.to contain_file('/etc/nginx/streams-enabled') } + it do + is_expected.to contain_file('/etc/nginx/streams-available').without( + %w( + ignore + purge + recurse + ) + ) + end + it do + is_expected.to contain_file('/etc/nginx/streams-enabled').without( + %w( + ignore + purge + recurse + ) + ) + end + it do + is_expected.to contain_file('/etc/nginx/streams-available').without( + %w( + ignore + purge + recurse + ) + ) + end + it do + is_expected.to contain_file('/etc/nginx/streams-enabled').without( + %w( + ignore + purge + recurse + ) + ) + end end context 'when daemon_user = www-data' do diff --git a/spec/defines/resource_stream_spec.rb b/spec/defines/resource_stream_spec.rb index 29749b365..cd94ef658 100644 --- a/spec/defines/resource_stream_spec.rb +++ b/spec/defines/resource_stream_spec.rb @@ -36,6 +36,17 @@ end end + describe 'when confd_only true' do + let(:pre_condition) { 'class { "nginx": confd_only => true }' } + let(:params) { default_params } + it { is_expected.to contain_class('nginx::config') } + it do + is_expected.to contain_concat("/etc/nginx/conf.stream.d/#{title}.conf").with('owner' => 'root', + 'group' => 'root', + 'mode' => '0644') + end + end + describe 'vhost_header template content' do [ { diff --git a/spec/defines/resource_vhost_spec.rb b/spec/defines/resource_vhost_spec.rb index cbe564aab..b1c1193e7 100644 --- a/spec/defines/resource_vhost_spec.rb +++ b/spec/defines/resource_vhost_spec.rb @@ -43,6 +43,19 @@ end end + describe 'with $confd_only enabled' do + let(:pre_condition) { 'class { "nginx": confd_only => true }' } + let(:params) { default_params } + it { is_expected.to contain_class('nginx::config') } + it do + is_expected.to contain_concat("/etc/nginx/conf.d/#{title}.conf").with('owner' => 'root', + 'group' => 'root', + 'mode' => '0644') + is_expected.not_to contain_file('/etc/nginx/sites-enabled') + is_expected.not_to contain_file('/etc/nginx/sites-available') + end + end + describe 'vhost_header template content' do [ { diff --git a/templates/conf.d/nginx.conf.erb b/templates/conf.d/nginx.conf.erb index b4d6e6b5d..f1082b900 100644 --- a/templates/conf.d/nginx.conf.erb +++ b/templates/conf.d/nginx.conf.erb @@ -176,7 +176,9 @@ http { <% end -%> include <%= @conf_dir %>/conf.d/*.conf; +<% unless @confd_only -%> include <%= @conf_dir %>/sites-enabled/*; +<% end -%> } <% if @mail -%> mail {