From 9186211bb20c4155662178bb851f0ceb3e4002af Mon Sep 17 00:00:00 2001 From: Andrew Teixeira Date: Fri, 2 Sep 2016 16:22:54 -0400 Subject: [PATCH] * Add .rspec for better test output * Add the ability to pass certificate/key data in as "content" * Add "ensure" to certs::site so that we can remove certs/keys as well * Fix bug with service notification code * Add beginning spec tests for certs::site * Fix cert/key source issue, and add placeholder to site spec * Add the ability to add chain or ca by content * Add SPEC tests for ca/chain files * Add SPEC tests for merging certificate with chain/ca * Add lots more tests --- .gitignore | 2 + .rspec | 1 + README.md | 11 +- manifests/init.pp | 2 +- manifests/params.pp | 10 +- manifests/site.pp | 170 +++++++++++------ metadata.json | 5 +- spec/classes/init_spec.rb | 30 ++- spec/defines/site_spec.rb | 391 ++++++++++++++++++++++++++++++++++++++ 9 files changed, 553 insertions(+), 69 deletions(-) create mode 100644 .rspec create mode 100644 spec/defines/site_spec.rb diff --git a/.gitignore b/.gitignore index 9a0fa70..879212b 100644 --- a/.gitignore +++ b/.gitignore @@ -12,3 +12,5 @@ coverage .librarian .tmp +.DS_Store +tmp/* diff --git a/.rspec b/.rspec new file mode 100644 index 0000000..49d5710 --- /dev/null +++ b/.rspec @@ -0,0 +1 @@ +--format documentation diff --git a/README.md b/README.md index d3374e5..245c090 100644 --- a/README.md +++ b/README.md @@ -193,7 +193,7 @@ e.g. *'puppet:///site_certs'* will search for the mount point defined in `filese ##### `cert_ext` The extension of the certificate file. -Optional value. **Default: crt.** +Optional value. **Default: '.crt'.** ##### `cert_path` Location where the certificate files will be stored on the managed node. @@ -204,6 +204,11 @@ Optional value. Defaults: - **FreeBSD**: `/usr/local/etc/apache24` - **Gentoo**: `/etc/ssl/apache2` +##### `key_ext` +The extension of the private key file. + +Optional value. **Default: '.key'.** + ##### `key_path` Location where the private keys will be stored on the managed node. @@ -317,7 +322,7 @@ Optional value. **Default: false.** ## Limitations This module is CI tested against [open source Puppet](https://docs.puppetlabs.com/puppet) on: -- Centos 5 and 6 +- CentOS 5 and 6 - RHEL 5, 6, and 7 This module also provides functions for other distributions and operating systems, such as FreeBSD and Gentoo, but is not formally tested on them and are subject to regressions. @@ -326,7 +331,7 @@ No issues have been identified as of yet. ## Release Notes -### 1.0.0 (September 2, 2016) +### 1.0.0 (September 6, 2016) #### Summary * Introducing new features, primarily an option to merge certificates for services that require it * Adding Vagrant support for testing using Puppet 4 diff --git a/manifests/init.pp b/manifests/init.pp index 0ad8bda..7397c3e 100644 --- a/manifests/init.pp +++ b/manifests/init.pp @@ -58,7 +58,7 @@ validate_string($_ca_ext) validate_absolute_path($_ca_path) - if $service != '' { + if $service != undef { validate_string($service) } diff --git a/manifests/params.pp b/manifests/params.pp index ebc10b2..bdf5321 100644 --- a/manifests/params.pp +++ b/manifests/params.pp @@ -13,7 +13,7 @@ # # [cert_ext] # The extension of the certificate file. -# Optional value. Default: crt. +# Optional value. Default: '.crt'. # # [cert_path] # Location where the certificate files will be stored on the managed node. @@ -23,6 +23,10 @@ # - '/usr/local/etc/apache24' on FreeBSD-based systems # - '/etc/ssl/apache2' on Gentoo-based systems # +# [key_ext] +# The extension of the private key file. +# Optional value. Default: '.key'. +# # [key_path] # Location where the private keys will be stored on the managed node. # Optional value. Defaults: @@ -146,10 +150,10 @@ } $cert_ext = '.crt' $key_ext = '.key' - $chain_name = '' + $chain_name = undef $chain_ext = $cert_ext $chain_path = $cert_path - $ca_name = '' + $ca_name = undef $ca_ext = $cert_ext $ca_path = $cert_path $owner = 'root' diff --git a/manifests/site.pp b/manifests/site.pp index 4b1cc72..526113e 100644 --- a/manifests/site.pp +++ b/manifests/site.pp @@ -50,6 +50,9 @@ # define certs::site( $source_path = undef, + $cert_content = undef, + $key_content = undef, + $ensure = 'present', $cert_ext = undef, $cert_path = undef, $key_ext = undef, @@ -59,11 +62,13 @@ $chain_ext = undef, $chain_path = undef, $chain_source_path = $source_path, + $chain_content = undef, $ca_cert = false, $ca_name = undef, $ca_ext = undef, $ca_path = undef, $ca_source_path = $source_path, + $ca_content = undef, $service = undef, $owner = undef, $group = undef, @@ -82,10 +87,22 @@ if ($name == undef) { fail('You must provide a name value for the site to certs::site.') } - if ($source_path == undef) { - fail('You must provide a source_path for the SSL files to certs::site.') + if ($source_path == undef and ($cert_content == undef or $key_content == undef)) { + fail('You must provide a source_path or cert_content/key_content combination for the SSL files to certs::site.') } + if ($source_path and ($cert_content or $key_content)) { + fail('You can only provide $source_path or $cert_content/$key_content, not both.') + } + + if !$source_path { + if !($cert_content and $key_content) { + fail('If source_path is not set, $cert_content and $key_content must both be set.') + } + } + + validate_re($ensure, '^(present|absent)$') + $_cert_ext = pick_default($cert_ext, $::certs::_cert_ext) $_cert_path = pick_default($cert_path, $::certs::_cert_path) $_key_ext = pick_default($key_ext, $::certs::_key_ext) @@ -111,7 +128,7 @@ validate_string($_ca_ext) validate_absolute_path($_ca_path) - if $service != '' { + if $service != undef { validate_string($service) } @@ -131,8 +148,16 @@ if ($chain_name == undef) { fail('You must provide a chain_name value for the cert chain to certs::site.') } - if ($chain_source_path == undef) { - fail('You must provide a chain_source_path for the SSL files to certs::site.') + $chain = "${chain_name}${_chain_ext}" + + if $chain_content == undef { + if ($chain_source_path == undef) { + fail('You must provide a chain_source_path for the SSL files to certs::site.') + } + + $chain_source = "${chain_source_path}/${chain}" + } else { + $chain_source = undef } } @@ -141,9 +166,16 @@ if ($ca_name == undef) { fail('You must provide a ca_name value for the CA cert to certs::site.') } + $ca = "${ca_name}${_ca_ext}" + + if $ca_content == undef { + if ($ca_source_path == undef) { + fail('You must provide a ca_source_path for the SSL files to certs::site.') + } - if ($ca_source_path == undef) { - fail('You must provide a ca_source_path for the SSL files to certs::site.') + $ca_source = "${ca_source_path}/${ca}" + } else { + $ca_source = undef } } @@ -152,19 +184,14 @@ $cert = "${name}${_cert_ext}" $key = "${name}${_key_ext}" - if $cert_chain { - $chain = "${chain_name}${_chain_ext}" - } - if $ca_cert { - $ca = "${ca_name}${_ca_ext}" - } - - if $service != '' { + if $service != undef { if defined(Service[$service]) { $service_notify = Service[$service] } else { $service_notify = undef } + } else { + $service_notify = undef } if !defined(File[$_cert_path]) { @@ -207,65 +234,89 @@ } } - if $merge_chain { - concat { "${name}_cert_merged": - ensure => 'present', - ensure_newline => true, - backup => false, - path => "${_cert_path}/${cert}", - owner => $_owner, - group => $_group, - mode => $_cert_mode, - require => File[$_cert_path], - notify => $service_notify, - } + if $source_path == undef { + $cert_source = undef + $key_source = undef + } else { + $cert_source = "${source_path}/${cert}" + $key_source = "${source_path}/${key}" + } - concat::fragment { "${cert}_certificate": - target => "${name}_cert_merged", - source => "${source_path}/${cert}", - order => '01' - } + if $ensure == 'present' { + if $merge_chain { + concat { "${name}_cert_merged": + ensure => 'present', + ensure_newline => true, + backup => false, + path => "${_cert_path}/${cert}", + owner => $_owner, + group => $_group, + mode => $_cert_mode, + require => File[$_cert_path], + notify => $service_notify, + } - if $cert_chain { - concat::fragment { "${cert}_chain": - target => "${name}_cert_merged", - source => "${chain_source_path}/${chain}", - order => '50' + concat::fragment { "${cert}_certificate": + target => "${name}_cert_merged", + source => $cert_source, + content => $cert_content, + order => '01' } - } - if $ca_cert { - concat::fragment { "${cert}_ca": - target => "${name}_cert_merged", - source => "${ca_source_path}/${ca}", - order => '90' + + if $cert_chain { + concat::fragment { "${cert}_chain": + target => "${name}_cert_merged", + source => $chain_source, + content => $chain_content, + order => '50' + } + } + if $ca_cert { + concat::fragment { "${cert}_ca": + target => "${name}_cert_merged", + source => $ca_source, + content => $ca_content, + order => '90' + } + } + } else { + file { "${_cert_path}/${cert}": + ensure => file, + source => $cert_source, + content => $cert_content, + owner => $_owner, + group => $_group, + mode => $_cert_mode, + require => File[$_cert_path], + notify => $service_notify, } } - } else { - file { "${_cert_path}/${cert}": + + file { "${_key_path}/${key}": ensure => file, - source => "${source_path}/${cert}", + source => $key_source, + content => $key_content, owner => $_owner, group => $_group, - mode => $_cert_mode, - require => File[$_cert_path], + mode => $_key_mode, + require => File[$_key_path], notify => $service_notify, } - } + } else { + file { "${_cert_path}/${cert}": + ensure => $ensure, + } - file { "${_key_path}/${key}": - ensure => file, - source => "${source_path}/${key}", - owner => $_owner, - group => $_group, - mode => $_key_mode, - require => File[$_key_path], - notify => $service_notify, + file { "${_key_path}/${key}": + ensure => $ensure, + } } if ($cert_chain and !defined(File["${_chain_path}/${chain}"])) { file { "${_chain_path}/${chain}": ensure => file, - source => "${chain_source_path}/${chain}", + source => $chain_source, + content => $chain_content, owner => $_owner, group => $_group, mode => $_cert_mode, @@ -277,7 +328,8 @@ if ($ca_cert and !defined(File["${_ca_path}/${ca}"])) { file { "${_ca_path}/${ca}": ensure => file, - source => "${ca_source_path}/${ca}", + source => $ca_source, + content => $ca_content, owner => $_owner, group => $_group, mode => $_cert_mode, diff --git a/metadata.json b/metadata.json index baa2d34..8cda108 100644 --- a/metadata.json +++ b/metadata.json @@ -1,7 +1,10 @@ { "name": "broadinstitute-certs", "version": "1.0.0", - "author": "Riccardo Calixte ", + "author": [ + "Riccardo Calixte ", + "Andrew Teixeira :class do - [ 'Debian', 'RedHat' ].each do |osfamily| + [ 'Debian', 'FreeBSD', 'Gentoo', 'RedHat' ].each do |osfamily| context "on #{osfamily}" do @@ -12,7 +12,6 @@ :operatingsystem => 'Debian', :lsbdistid => 'Debian', :lsbdistcodename => 'wheezy', - :kernelrelease => '3.2.0-4-amd64', :operatingsystemrelease => '7.3', :operatingsystemmajrelease => '7', } } @@ -22,6 +21,33 @@ end end + if osfamily == 'FreeBSD' + let(:facts) { { + :osfamily => osfamily, + :operatingsystem => 'FreeBSD', + :operatingsystemrelease => '10.0-RELEASE-p18', + :operatingsystemmajrelease => '10', + } } + + context 'with defaults for all parameters' do + it { should contain_class('certs') } + end + + end + + if osfamily == 'Gentoo' + let(:facts) { { + :osfamily => osfamily, + :operatingsystem => 'Gentoo', + :operatingsystemrelease => '3.16.1-gentoo', + } } + + context 'with defaults for all parameters' do + it { should contain_class('certs') } + end + + end + if osfamily == 'RedHat' let(:facts) { { :osfamily => osfamily, diff --git a/spec/defines/site_spec.rb b/spec/defines/site_spec.rb new file mode 100644 index 0000000..4fcebd7 --- /dev/null +++ b/spec/defines/site_spec.rb @@ -0,0 +1,391 @@ +require 'spec_helper' + +describe 'certs::site', :type => :define do + let (:title) { 'base.example.org' } + + [ 'Debian', 'FreeBSD', 'Gentoo', 'RedHat', 'Suse' ].each do |osfamily| + + context "on #{osfamily}" do + # This define requires the certs class, so make sure it's defined + let :pre_condition do + 'class { "certs": }' + end + + if osfamily == 'Debian' + let(:facts) { { + :osfamily => 'Debian', + :operatingsystem => 'Debian', + :lsbdistid => 'Debian', + :lsbdistcodename => 'wheezy', + :operatingsystemrelease => '7.3', + :operatingsystemmajrelease => '7', + } } + + context 'with only cert and key content set' do + let(:params) { + { + :cert_content => 'cert1111', + :key_content => 'key1111', + :service => :undef, + } + } + + it { should contain_file('/etc/ssl/certs').with_ensure('directory') } + it { should contain_file('/etc/ssl/private').with_ensure('directory') } + it { should contain_file('/etc/ssl/certs/base.example.org.crt').with_group('root') } + it { should contain_file('/etc/ssl/private/base.example.org.key').with_group('root') } + end + + end + + if osfamily == 'FreeBSD' + let(:facts) { { + :osfamily => osfamily, + :operatingsystem => 'FreeBSD', + :operatingsystemrelease => '10.0-RELEASE-p18', + :operatingsystemmajrelease => '10', + } } + + context 'with only cert and key content set' do + let(:params) { + { + :cert_content => 'cert1111', + :key_content => 'key1111', + :service => :undef, + } + } + + it { should contain_file('/usr/local/etc/apache24').with_ensure('directory') } + it { should contain_file('/usr/local/etc/apache24').with_group('wheel') } + it { should contain_file('/usr/local/etc/apache24/base.example.org.crt').with_group('wheel') } + it { should contain_file('/usr/local/etc/apache24/base.example.org.key').with_group('wheel') } + end + + end + + if osfamily == 'Gentoo' + let(:facts) { { + :osfamily => osfamily, + :operatingsystem => 'Gentoo', + :operatingsystemrelease => '3.16.1-gentoo', + } } + + context 'with only cert and key content set' do + let(:params) { + { + :cert_content => 'cert1111', + :key_content => 'key1111', + :service => :undef, + } + } + + it { should contain_file('/etc/ssl/apache2').with_ensure('directory') } + it { should contain_file('/etc/ssl/apache2').with_group('wheel') } + it { should contain_file('/etc/ssl/apache2/base.example.org.crt').with_group('wheel') } + it { should contain_file('/etc/ssl/apache2/base.example.org.key').with_group('wheel') } + end + + end + + if osfamily == 'RedHat' + let(:facts) { { + :osfamily => osfamily, + :operatingsystem => 'RedHat', + :operatingsystemrelease => '7.2', + :operatingsystemmajrelease => '7', + } } + + context 'with only cert and key content set' do + let(:params) { + { + :cert_content => 'cert1111', + :key_content => 'key1111', + :service => :undef, + } + } + + it { should contain_file('/etc/pki/tls/certs').with_ensure('directory') } + it { should contain_file('/etc/pki/tls/private').with_ensure('directory') } + it { should contain_file('/etc/pki/tls/certs/base.example.org.crt').with_group('root') } + it { should contain_file('/etc/pki/tls/private/base.example.org.key').with_group('root') } + end + + end + end + end + + context "on Debian-like setup for the remaining tests" do + + let(:facts) { { + :osfamily => 'Debian', + :operatingsystem => 'Debian', + :lsbdistid => 'Debian', + :lsbdistcodename => 'wheezy', + :operatingsystemrelease => '7.3', + :operatingsystemmajrelease => '7', + } } + + # This define requires the certs class, so make sure it's defined + let :pre_condition do + 'class { "certs": + cert_path => "/etc/ssl/certs", + key_path => "/etc/ssl/private", + }' + end + + context 'with only cert and key content set' do + let(:params) { + { + :cert_content => 'cert1111', + :key_content => 'key1111', + :service => :undef, + } + } + + it { should contain_file('/etc/ssl/certs/base.example.org.crt').with_content(/cert1111/) } + it { should contain_file('/etc/ssl/private/base.example.org.key').with_content(/key1111/) } + end + + context 'with ensure => absent' do + let(:params) { + { + :cert_content => 'cert', + :key_content => 'key', + :service => :undef, + :ensure => 'absent' + } + } + + it { should contain_file('/etc/ssl/certs/base.example.org.crt').with_ensure('absent') } + it { should contain_file('/etc/ssl/private/base.example.org.key').with_ensure('absent') } + end + + context 'with CA cert' do + let(:params) { + { + :cert_content => 'cert1111', + :key_content => 'key1111', + :service => :undef, + :ca_cert => true, + :ca_name => 'ca', + :ca_content => 'ca2222', + :ensure => 'present' + } + } + + it { should contain_file('/etc/ssl/certs/base.example.org.crt').with_content(/cert1111/) } + it { should contain_file('/etc/ssl/private/base.example.org.key').with_content(/key1111/) } + it { should contain_file('/etc/ssl/certs/ca.crt').with_content(/ca2222/) } + end + + context 'with chain cert' do + let(:params) { + { + :cert_content => 'cert1111', + :key_content => 'key1111', + :service => :undef, + :ca_cert => true, + :ca_name => 'chain', + :ca_content => 'chain3333', + :ensure => 'present' + } + } + + it { should contain_file('/etc/ssl/certs/base.example.org.crt').with_content(/cert1111/) } + it { should contain_file('/etc/ssl/private/base.example.org.key').with_content(/key1111/) } + it { should contain_file('/etc/ssl/certs/chain.crt').with_content(/chain3333/) } + end + + context 'with merge_chain set to true with CA' do + let(:params) { + { + :cert_content => 'cert1111', + :key_content => 'key1111', + :service => :undef, + :ensure => 'present', + :ca_cert => true, + :ca_name => 'ca', + :ca_content => 'ca2222', + :merge_chain => true + } + } + + it { should contain_concat('base.example.org_cert_merged') } + it { + should contain_concat__fragment('base.example.org.crt_certificate').with( + :content => /cert1111/ ) + } + it { + should contain_concat__fragment('base.example.org.crt_ca').with( + :content => /ca2222/ ) + } + it { should contain_file('/etc/ssl/private/base.example.org.key').with_content(/key1111/) } + it { should contain_file('/etc/ssl/certs/ca.crt').with_content(/ca2222/) } + end + + context 'with merge_chain set to true with chain' do + let(:params) { + { + :cert_content => 'cert1111', + :key_content => 'key1111', + :service => :undef, + :ensure => 'present', + :cert_chain => true, + :chain_name => 'chain', + :chain_content => 'chain3333', + :merge_chain => true + } + } + + it { should contain_concat('base.example.org_cert_merged') } + it { + should contain_concat__fragment('base.example.org.crt_certificate').with( + :content => /cert1111/ ) + } + it { + should contain_concat__fragment('base.example.org.crt_chain').with( + :content => /chain3333/ ) + } + it { should contain_file('/etc/ssl/private/base.example.org.key').with_content(/key1111/) } + it { should contain_file('/etc/ssl/certs/chain.crt').with_content(/chain3333/) } + end + + context 'with merge_chain set to true with CA and chain' do + let(:params) { + { + :cert_content => 'cert1111', + :key_content => 'key1111', + :service => :undef, + :ensure => 'present', + :cert_chain => true, + :chain_name => 'chain', + :chain_content => 'chain3333', + :ca_cert => true, + :ca_name => 'ca', + :ca_content => 'ca2222', + :merge_chain => true + } + } + + it { should contain_concat('base.example.org_cert_merged') } + it { + should contain_concat__fragment('base.example.org.crt_certificate').with( + :content => /cert1111/ ) + } + it { + should contain_concat__fragment('base.example.org.crt_ca').with( + :content => /ca2222/ ) + } + it { + should contain_concat__fragment('base.example.org.crt_chain').with( + :content => /chain3333/ ) + } + it { should contain_file('/etc/ssl/private/base.example.org.key').with_content(/key1111/) } + it { should contain_file('/etc/ssl/certs/ca.crt').with_content(/ca2222/) } + it { should contain_file('/etc/ssl/certs/chain.crt').with_content(/chain3333/) } + end + + context 'with a custom user set' do + let(:params) { + { + :cert_content => 'cert1111', + :key_content => 'key1111', + :service => :undef, + :ensure => 'present', + :cert_chain => true, + :chain_name => 'chain', + :chain_content => 'chain3333', + :ca_cert => true, + :ca_name => 'ca', + :ca_content => 'ca2222', + :owner => 'newowner', + } + } + + it { should contain_file('/etc/ssl/certs/base.example.org.crt').with_owner('newowner') } + it { should contain_file('/etc/ssl/private/base.example.org.key').with_owner('newowner') } + it { should contain_file('/etc/ssl/certs/ca.crt').with_owner('newowner') } + it { should contain_file('/etc/ssl/certs/chain.crt').with_owner('newowner') } + end + + context 'with a custom group set' do + let(:params) { + { + :cert_content => 'cert1111', + :key_content => 'key1111', + :service => :undef, + :ensure => 'present', + :cert_chain => true, + :chain_name => 'chain', + :chain_content => 'chain3333', + :ca_cert => true, + :ca_name => 'ca', + :ca_content => 'ca2222', + :group => 'newgroup', + } + } + + it { should contain_file('/etc/ssl/certs/base.example.org.crt').with_group('newgroup') } + it { should contain_file('/etc/ssl/private/base.example.org.key').with_group('newgroup') } + it { should contain_file('/etc/ssl/certs/ca.crt').with_group('newgroup') } + it { should contain_file('/etc/ssl/certs/chain.crt').with_group('newgroup') } + end + + context 'with a custom file and directory modes set' do + let(:params) { + { + :cert_content => 'cert1111', + :key_content => 'key1111', + :service => :undef, + :ensure => 'present', + :cert_chain => true, + :chain_name => 'chain', + :chain_content => 'chain3333', + :ca_cert => true, + :ca_name => 'ca', + :ca_content => 'ca2222', + :cert_mode => '0700', + :key_mode => '0400', + :cert_dir_mode => '0500', + :key_dir_mode => '0500', + } + } + + it { should contain_file('/etc/ssl/certs').with_ensure('directory') } + it { should contain_file('/etc/ssl/certs').with_mode('0500') } + it { should contain_file('/etc/ssl/private').with_ensure('directory') } + it { should contain_file('/etc/ssl/private').with_mode('0500') } + + it { should contain_file('/etc/ssl/certs/base.example.org.crt').with_mode('0700') } + it { should contain_file('/etc/ssl/private/base.example.org.key').with_mode('0400') } + it { should contain_file('/etc/ssl/certs/ca.crt').with_mode('0700') } + it { should contain_file('/etc/ssl/certs/chain.crt').with_mode('0700') } + end + + context 'with custom extensions set' do + let(:params) { + { + :cert_content => 'cert1111', + :key_content => 'key1111', + :service => :undef, + :ensure => 'present', + :cert_chain => true, + :chain_name => 'chain', + :chain_content => 'chain3333', + :ca_cert => true, + :ca_name => 'ca', + :ca_content => 'ca2222', + :cert_ext => '.pem', + :key_ext => '.pem', + :ca_ext => '.pem', + :chain_ext => '.pem', + } + } + + it { should contain_file('/etc/ssl/certs/base.example.org.pem') } + it { should contain_file('/etc/ssl/private/base.example.org.pem') } + it { should contain_file('/etc/ssl/certs/ca.pem') } + it { should contain_file('/etc/ssl/certs/chain.pem') } + end + end +end