Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Switch to using concat{} instead of lots of file{} magic. #167

Merged
merged 18 commits into from
Dec 3, 2013
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,5 @@ pkg/
.librarian/
.tmp/
pkg/
Gemfile.lock
spec/fixtures/
4 changes: 0 additions & 4 deletions .nodeset.yml
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,3 @@ sets:
nodes:
"main.foo.vm":
prefab: 'ubuntu-server-12042-x64'
'sles-11sp1-x64':
nodes:
"main.foo.vm":
prefab: 'sles-11sp1-x64'
2 changes: 1 addition & 1 deletion Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,6 @@ group :rake do
gem 'puppet-blacksmith'
gem 'librarian-puppet-maestrodev'
gem 'rspec-system-puppet', :require => false
gem 'serverspec', :require => false
gem 'serverspec', '~> 0.11.0', :require => false
gem 'rspec-system-serverspec', :require => false
end
96 changes: 0 additions & 96 deletions Gemfile.lock

This file was deleted.

1 change: 1 addition & 0 deletions Modulefile
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,4 @@ project_page 'http://github.com/jfryman/puppet-nginx'

dependency 'puppetlabs/stdlib', '>= 0.1.6'
dependency 'puppetlabs/apt', '>= 1.0.0'
dependency 'puppetlabs/concat', '>= 1.0.0'
1 change: 1 addition & 0 deletions Puppetfile
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,4 @@ forge 'http://forge.puppetlabs.com'

mod 'puppetlabs/stdlib', '>=0.1.6'
mod 'puppetlabs/apt', '>=1.0.0'
mod 'puppetlabs/concat', '>=1.0.0'
2 changes: 2 additions & 0 deletions Puppetfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,11 @@ FORGE
specs:
puppetlabs/apt (1.2.0)
puppetlabs/stdlib (>= 2.2.1)
puppetlabs/concat (1.0.0)
puppetlabs/stdlib (4.1.0)

DEPENDENCIES
puppetlabs/apt (>= 1.0.0)
puppetlabs/concat (>= 1.0.0)
puppetlabs/stdlib (>= 0.1.6)

22 changes: 18 additions & 4 deletions manifests/config.pp
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@
}
if $confd_purge == true {
File["${nginx::params::nx_conf_dir}/conf.d"] {
ignore => 'vhost_autogen.conf',
purge => true,
recurse => true,
}
Expand All @@ -60,12 +59,19 @@
}
if $confd_purge == true {
File["${nginx::params::nx_conf_dir}/conf.mail.d"] {
ignore => 'vhost_autogen.conf',
purge => true,
recurse => true,
}
}

file { "${nginx::params::nx_conf_dir}/conf.d/vhost_autogen.conf":
ensure => absent,
}

file { "${nginx::params::nx_conf_dir}/conf.mail.d/vhost_autogen.conf":
ensure => absent,
}

file {$nginx::config::nx_run_dir:
ensure => directory,
}
Expand All @@ -80,6 +86,14 @@
owner => $nginx::params::nx_daemon_user,
}

file { "${nginx::params::nx_conf_dir}/sites-available":
ensure => directory,
}

file { "${nginx::params::nx_conf_dir}/sites-enabled":
ensure => directory,
}

file { '/etc/nginx/sites-enabled/default':
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you think about ensure => absent to clean up mess from old concat?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It crossed my mind - I left it out since it's a temp folder anyway so things should get wiped out periodically without help from the module.

I'll add it back so people's systems get cleaned up a little.

ensure => absent,
}
Expand All @@ -95,13 +109,13 @@
}

file { "${nginx::config::nx_temp_dir}/nginx.d":
ensure => directory,
ensure => absent,
purge => true,
recurse => true,
}

file { "${nginx::config::nx_temp_dir}/nginx.mail.d":
ensure => directory,
ensure => absent,
purge => true,
recurse => true,
}
Expand Down
11 changes: 7 additions & 4 deletions manifests/resource/location.pp
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,7 @@
'absent' => absent,
default => file,
}
$config_file = "${nginx::config::nx_conf_dir}/sites-available/${vhost}.conf"

$location_sanitized = regsubst($location, '\/', '_', 'G')

Expand Down Expand Up @@ -168,18 +169,20 @@

## Create stubs for vHost File Fragment Pattern
if ($ssl_only != true) {
file {"${nginx::config::nx_temp_dir}/nginx.d/${vhost}-${priority}-${location_sanitized}":
ensure => $ensure_real,
concat::fragment { "${vhost}-${priority}-${location_sanitized}":
target => $config_file,
content => $content_real,
order => $priority,
}
}

## Only create SSL Specific locations if $ssl is true.
if ($ssl == true) {
$ssl_priority = $priority + 300
file {"${nginx::config::nx_temp_dir}/nginx.d/${vhost}-${ssl_priority}-${location_sanitized}-ssl":
ensure => $ensure_real,
concat::fragment {"${vhost}-${ssl_priority}-${location_sanitized}-ssl":
target => $config_file,
content => $content_real,
order => $ssl_priority,
}
}

Expand Down
29 changes: 15 additions & 14 deletions manifests/resource/mailhost.pp
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,8 @@

validate_array($server_name)

$config_file = "${nginx::config::nx_conf_dir}/conf.mail.d/${name}.conf"

# Add IPv6 Logic Check - Nginx service will not start if ipv6 is enabled
# and support does not exist for it in the kernel.
if ($ipv6_enable and !$::ipaddress6) {
Expand All @@ -80,28 +82,27 @@
}
}

# Use the File Fragment Pattern to construct the configuration files.
# Create the base configuration file reference.
concat { $config_file:
owner => 'root',
group => 'root',
mode => '0644',
notify => Class['nginx::service'],
}

if ($listen_port != $ssl_port) {
file { "${nginx::config::nx_temp_dir}/nginx.mail.d/${name}-001":
ensure => $ensure ? {
'absent' => absent,
default => 'file',
},
concat::fragment { "${name}-header":
target => $config_file,
content => template('nginx/mailhost/mailhost.erb'),
notify => Class['nginx::service'],
order => '001',
}
}

# Create SSL File Stubs if SSL is enabled
if ($ssl) {
file { "${nginx::config::nx_temp_dir}/nginx.mail.d/${name}-700-ssl":
ensure => $ensure ? {
'absent' => absent,
default => 'file',
},
concat::fragment { "${name}-ssl":
target => $config_file,
content => template('nginx/mailhost/mailhost_ssl.erb'),
notify => Class['nginx::service'],
order => '700',
}
}
}
61 changes: 39 additions & 22 deletions manifests/resource/vhost.pp
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,15 @@
validate_array($index_files)
validate_array($server_name)

# Variables
$vhost_dir = "${nginx::config::nx_conf_dir}/sites-available"
$vhost_enable_dir = "${nginx::config::nx_conf_dir}/sites-enabled"
$vhost_symlink_ensure = $ensure ? {
'absent' => absent,
default => 'link',
}
$config_file = "${vhost_dir}/${name}.conf"

File {
ensure => $ensure ? {
'absent' => absent,
Expand Down Expand Up @@ -181,6 +190,13 @@
default => $error_log,
}

concat { $config_file:
owner => 'root',
group => 'root',
mode => '0644',
notify => Class['nginx::service'],
}

if ($ssl == true) and ($ssl_port == $listen_port) {
$ssl_only = true
}
Expand Down Expand Up @@ -231,22 +247,21 @@
}
}

# Use the File Fragment Pattern to construct the configuration files.
# Create the base configuration file reference.
if ($listen_port != $ssl_port) {
file { "${nginx::config::nx_temp_dir}/nginx.d/${name}-001":
ensure => $ensure ? {
'absent' => absent,
default => 'file',
},
concat::fragment { "${name}-header":
target => $config_file,
content => template('nginx/vhost/vhost_header.erb'),
notify => Class['nginx::service'],
order => '001',
}
}

# Create a proper file close stub.
if ($listen_port != $ssl_port) {
file { "${nginx::config::nx_temp_dir}/nginx.d/${name}-699": content => template('nginx/vhost/vhost_footer.erb'), }
concat::fragment { "${name}-footer":
target => $config_file,
content => template('nginx/vhost/vhost_footer.erb'),
order => '699',
}
}

# Create SSL File Stubs if SSL is enabled
Expand All @@ -260,25 +275,19 @@
undef => "${nginx::params::nx_logdir}/ssl-${domain_log_name}.error.log",
default => $error_log,
}
file { "${nginx::config::nx_temp_dir}/nginx.d/${name}-700-ssl":
ensure => $ensure ? {
'absent' => absent,
default => 'file',
},

concat::fragment { "${name}-ssl-header":
target => $config_file,
content => template('nginx/vhost/vhost_ssl_header.erb'),
notify => Class['nginx::service'],
order => '700',
}
file { "${nginx::config::nx_temp_dir}/nginx.d/${name}-999-ssl":
ensure => $ensure ? {
'absent' => absent,
default => 'file',
},
concat::fragment { "${name}-ssl-footer":
target => $config_file,
content => template('nginx/vhost/vhost_ssl_footer.erb'),
notify => Class['nginx::service'],
order => '999',
}

#Generate ssl key/cert with provided file-locations

$cert = regsubst($name,' ','_')

# Check if the file has been defined before creating the file to
Expand All @@ -294,4 +303,12 @@
source => $ssl_key,
})
}

file{ "${name}.conf symlink":
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a little confused by this here. I know that this pull incorporates the sites-available and sites-enabled pattern. I like how that's laid out, but I want to say that if I define it in code, that it should be in sites-enabled.

Thoughts?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you suggesting this module just put the vhost file in sites-enabled directly, with no symlink to sites-available? puppetlabs-apache puts the generated config file in sites-available then symlinks from sites-enabled if the vhost is enabled, and removes both the file and symlink if absent. See https://github.com/puppetlabs/puppetlabs-apache/blob/master/manifests/vhost.pp#L426.

Also if there's no symlink there'll be no way to remove a site from sites-enabled since puppetlabs-concat doesn't have $ensure support yet so it'll be impossible to remove the file concat generated.

ensure => $vhost_symlink_ensure,
path => "${vhost_enable_dir}/${name}.conf",
target => $config_file,
require => Concat[$config_file],
notify => Service['nginx'],
}
}
Loading