From 51bd49fcc3773a8268e192c4c02d6a56418e4bce Mon Sep 17 00:00:00 2001 From: Bryan Jen Date: Wed, 25 Mar 2015 11:48:51 -0700 Subject: [PATCH] refactor concat to wrap electrical/file_concat --- .fixtures.yml | 5 +- .gitignore | 1 + Gemfile | 5 +- files/concatfragments.rb | 153 ----------- lib/facter/concat_basedir.rb | 11 - .../parser/functions/concat_getparam.rb | 35 --- lib/puppet/parser/functions/concat_is_bool.rb | 22 -- manifests/fragment.pp | 100 +------ manifests/init.pp | 208 +++------------ manifests/setup.pp | 64 ----- metadata.json | 13 +- spec/acceptance/backup_spec.rb | 2 +- spec/acceptance/concat_spec.rb | 64 ----- spec/acceptance/deprecation_warnings_spec.rb | 238 ----------------- spec/acceptance/empty_spec.rb | 23 -- spec/acceptance/fragment_source_spec.rb | 2 +- spec/acceptance/newline_spec.rb | 67 ----- spec/acceptance/nodesets/default.yml | 8 +- spec/acceptance/order_spec.rb | 4 +- spec/acceptance/replace_spec.rb | 3 +- .../{warn_spec.rb => warn_header_spec.rb} | 8 +- spec/spec_helper_acceptance.rb | 14 +- spec/unit/classes/concat_setup_spec.rb | 98 ------- spec/unit/defines/concat_fragment_spec.rb | 233 +---------------- spec/unit/defines/concat_spec.rb | 247 ++++-------------- spec/unit/facts/concat_basedir_spec.rb | 18 -- 26 files changed, 143 insertions(+), 1503 deletions(-) delete mode 100644 files/concatfragments.rb delete mode 100644 lib/facter/concat_basedir.rb delete mode 100644 lib/puppet/parser/functions/concat_getparam.rb delete mode 100644 lib/puppet/parser/functions/concat_is_bool.rb delete mode 100644 manifests/setup.pp delete mode 100644 spec/acceptance/deprecation_warnings_spec.rb delete mode 100644 spec/acceptance/empty_spec.rb delete mode 100644 spec/acceptance/newline_spec.rb rename spec/acceptance/{warn_spec.rb => warn_header_spec.rb} (94%) delete mode 100644 spec/unit/classes/concat_setup_spec.rb delete mode 100644 spec/unit/facts/concat_basedir_spec.rb diff --git a/.fixtures.yml b/.fixtures.yml index 67added0a..fb0f76a6f 100644 --- a/.fixtures.yml +++ b/.fixtures.yml @@ -2,6 +2,9 @@ fixtures: repositories: 'stdlib': repo: 'git://github.com/puppetlabs/puppetlabs-stdlib.git' - ref: '4.2.0' + ref: '4.5.1' + 'file_concat': + repo: 'git://github.com/electrical/puppet-lib-file_concat.git' + branch: '1.0.0' symlinks: 'concat': '#{source_dir}' diff --git a/.gitignore b/.gitignore index b5db85e05..ef01482d1 100644 --- a/.gitignore +++ b/.gitignore @@ -7,3 +7,4 @@ spec/fixtures/ coverage/ .idea/ *.iml +*.swp diff --git a/Gemfile b/Gemfile index cc4048aec..f7319c221 100644 --- a/Gemfile +++ b/Gemfile @@ -12,13 +12,14 @@ end group :development, :unit_tests do gem 'rake', :require => false - gem 'rspec-core', '3.1.7', :require => false - gem 'rspec-puppet', '~> 1.0', :require => false + gem 'rspec-core', '~> 3.1.7', :require => false + gem 'rspec-puppet', '~> 2.0', :require => false gem 'puppetlabs_spec_helper', :require => false gem 'puppet-lint', :require => false gem 'simplecov', :require => false gem 'puppet_facts', :require => false gem 'json', :require => false + gem 'pry' end beaker_version = ENV['BEAKER_VERSION'] diff --git a/files/concatfragments.rb b/files/concatfragments.rb deleted file mode 100644 index b16f3e13c..000000000 --- a/files/concatfragments.rb +++ /dev/null @@ -1,153 +0,0 @@ -#!/usr/bin/env ruby -# Script to concat files to a config file. -# -# Given a directory like this: -# /path/to/conf.d -# |-- fragments -# | |-- 00_named.conf -# | |-- 10_domain.net -# | `-- zz_footer -# -# The script supports a test option that will build the concat file to a temp location and -# use /usr/bin/cmp to verify if it should be run or not. This would result in the concat happening -# twice on each run but gives you the option to have an unless option in your execs to inhibit rebuilds. -# -# Without the test option and the unless combo your services that depend on the final file would end up -# restarting on each run, or in other manifest models some changes might get missed. -# -# OPTIONS: -# -o The file to create from the sources -# -d The directory where the fragments are kept -# -t Test to find out if a build is needed, basically concats the files to a temp -# location and compare with what's in the final location, return codes are designed -# for use with unless on an exec resource -# -w Add a shell style comment at the top of the created file to warn users that it -# is generated by puppet -# -f Enables the creation of empty output files when no fragments are found -# -n Sort the output numerically rather than the default alpha sort -# -# the command: -# -# concatfragments.rb -o /path/to/conffile.cfg -d /path/to/conf.d -# -# creates /path/to/conf.d/fragments.concat and copies the resulting -# file to /path/to/conffile.cfg. The files will be sorted alphabetically -# pass the -n switch to sort numerically. -# -# The script does error checking on the various dirs and files to make -# sure things don't fail. -require 'optparse' -require 'fileutils' - -settings = { - :outfile => "", - :workdir => "", - :test => false, - :force => false, - :warn => "", - :sortarg => "", - :newline => false -} - -OptionParser.new do |opts| - opts.banner = "Usage: #{$0} [options]" - opts.separator "Specific options:" - - opts.on("-o", "--outfile OUTFILE", String, "The file to create from the sources") do |o| - settings[:outfile] = o - end - - opts.on("-d", "--workdir WORKDIR", String, "The directory where the fragments are kept") do |d| - settings[:workdir] = d - end - - opts.on("-t", "--test", "Test to find out if a build is needed") do - settings[:test] = true - end - - opts.separator "Other options:" - opts.on("-w", "--warn WARNMSG", String, - "Add a shell style comment at the top of the created file to warn users that it is generated by puppet") do |w| - settings[:warn] = w - end - - opts.on("-f", "--force", "Enables the creation of empty output files when no fragments are found") do - settings[:force] = true - end - - opts.on("-n", "--sort", "Sort the output numerically rather than the default alpha sort") do - settings[:sortarg] = "-n" - end - - opts.on("-l", "--line", "Append a newline") do - settings[:newline] = true - end -end.parse! - -# do we have -o? -raise 'Please specify an output file with -o' unless !settings[:outfile].empty? - -# do we have -d? -raise 'Please specify fragments directory with -d' unless !settings[:workdir].empty? - -# can we write to -o? -if File.file?(settings[:outfile]) - if !File.writable?(settings[:outfile]) - raise "Cannot write to #{settings[:outfile]}" - end -else - if !File.writable?(File.dirname(settings[:outfile])) - raise "Cannot write to dirname #{File.dirname(settings[:outfile])} to create #{settings[:outfile]}" - end -end - -# do we have a fragments subdir inside the work dir? -if !File.directory?(File.join(settings[:workdir], "fragments")) && !File.executable?(File.join(settings[:workdir], "fragments")) - raise "Cannot access the fragments directory" -end - -# are there actually any fragments? -if (Dir.entries(File.join(settings[:workdir], "fragments")) - %w{ . .. }).empty? - if !settings[:force] - raise "The fragments directory is empty, cowardly refusing to make empty config files" - end -end - -Dir.chdir(settings[:workdir]) - -if settings[:warn].empty? - File.open("fragments.concat", 'w') { |f| f.write("") } -else - File.open("fragments.concat", 'w') { |f| f.write("#{settings[:warn]}\n") } -end - -# find all the files in the fragments directory, sort them numerically and concat to fragments.concat in the working dir -open('fragments.concat', 'a') do |f| - fragments = Dir.entries("fragments").sort - if settings[:sortarg] == '-n' - fragments = fragments.sort_by { |v| v.split('_').map(&:to_i) } - end - fragments.each { |entry| - if File.file?(File.join("fragments", entry)) - f << File.read(File.join("fragments", entry)) - - # append a newline if we were asked to (invoked with -l) - if settings[:newline] - f << "\n" - end - - end - } -end - -if !settings[:test] - # This is a real run, copy the file to outfile - FileUtils.cp 'fragments.concat', settings[:outfile] -else - # Just compare the result to outfile to help the exec decide - if FileUtils.cmp 'fragments.concat', settings[:outfile] - exit 0 - else - exit 1 - end -end diff --git a/lib/facter/concat_basedir.rb b/lib/facter/concat_basedir.rb deleted file mode 100644 index bfac07102..000000000 --- a/lib/facter/concat_basedir.rb +++ /dev/null @@ -1,11 +0,0 @@ -# == Fact: concat_basedir -# -# A custom fact that sets the default location for fragments -# -# "${::vardir}/concat/" -# -Facter.add("concat_basedir") do - setcode do - File.join(Puppet[:vardir],"concat") - end -end diff --git a/lib/puppet/parser/functions/concat_getparam.rb b/lib/puppet/parser/functions/concat_getparam.rb deleted file mode 100644 index 1757bdc54..000000000 --- a/lib/puppet/parser/functions/concat_getparam.rb +++ /dev/null @@ -1,35 +0,0 @@ -# Test whether a given class or definition is defined -require 'puppet/parser/functions' - -Puppet::Parser::Functions.newfunction(:concat_getparam, - :type => :rvalue, - :doc => <<-'ENDOFDOC' -Takes a resource reference and name of the parameter and -returns value of resource's parameter. - -*Examples:* - - define example_resource($param) { - } - - example_resource { "example_resource_instance": - param => "param_value" - } - - concat_getparam(Example_resource["example_resource_instance"], "param") - -Would return: param_value -ENDOFDOC -) do |vals| - reference, param = vals - raise(ArgumentError, 'Must specify a reference') unless reference - raise(ArgumentError, 'Must specify name of a parameter') unless param and param.instance_of? String - - return '' if param.empty? - - if resource = findresource(reference.to_s) - return resource[param] if resource[param] - end - - return '' -end diff --git a/lib/puppet/parser/functions/concat_is_bool.rb b/lib/puppet/parser/functions/concat_is_bool.rb deleted file mode 100644 index c2c2a9f2e..000000000 --- a/lib/puppet/parser/functions/concat_is_bool.rb +++ /dev/null @@ -1,22 +0,0 @@ -# -# concat_is_bool.rb -# - -module Puppet::Parser::Functions - newfunction(:concat_is_bool, :type => :rvalue, :doc => <<-EOS -Returns true if the variable passed to this function is a boolean. - EOS - ) do |arguments| - - raise(Puppet::ParseError, "concat_is_bool(): Wrong number of arguments " + - "given (#{arguments.size} for 1)") if arguments.size != 1 - - type = arguments[0] - - result = type.is_a?(TrueClass) || type.is_a?(FalseClass) - - return result - end -end - -# vim: set ts=2 sw=2 et : diff --git a/manifests/fragment.pp b/manifests/fragment.pp index f9ee2a7ca..5a3e31043 100644 --- a/manifests/fragment.pp +++ b/manifests/fragment.pp @@ -1,6 +1,6 @@ # == Define: concat::fragment # -# Puts a file fragment into a directory previous setup using concat +# Creates a file_fragment in the catalogue # # === Options: # @@ -13,115 +13,37 @@ # [*order*] # By default all files gets a 10_ prefix in the directory you can set it to # anything else using this to influence the order of the content in the file -# [*ensure*] -# Present/Absent or destination to a file to include another file -# [*mode*] -# Deprecated -# [*owner*] -# Deprecated -# [*group*] -# Deprecated -# [*backup*] -# Deprecated # define concat::fragment( $target, $content = undef, $source = undef, $order = '10', - $ensure = undef, - $mode = undef, - $owner = undef, - $group = undef, - $backup = undef ) { validate_string($target) validate_string($content) if !(is_string($source) or is_array($source)) { fail('$source is not a string or an Array.') } + if !(is_string($order) or is_integer($order)) { fail('$order is not a string or integer.') } elsif (is_string($order) and $order =~ /[:\n\/]/) { fail("Order cannot contain '/', ':', or '\n'.") } - if $mode { - warning('The $mode parameter to concat::fragment is deprecated and has no effect') - } - if $owner { - warning('The $owner parameter to concat::fragment is deprecated and has no effect') - } - if $group { - warning('The $group parameter to concat::fragment is deprecated and has no effect') - } - if $backup { - warning('The $backup parameter to concat::fragment is deprecated and has no effect') - } - $my_backup = concat_getparam(Concat[$target], 'backup') - if $ensure == undef { - $my_ensure = concat_getparam(Concat[$target], 'ensure') - } else { - if ! ($ensure in [ 'present', 'absent' ]) { - warning('Passing a value other than \'present\' or \'absent\' as the $ensure parameter to concat::fragment is deprecated. If you want to use the content of a file as a fragment please use the $source parameter.') - } - $my_ensure = $ensure - } - - include concat::setup - - $safe_name = regsubst($name, '[/:\n]', '_', 'GM') - $safe_target_name = regsubst($target, '[/:\n]', '_', 'GM') - $concatdir = $concat::setup::concatdir - $fragdir = "${concatdir}/${safe_target_name}" - $fragowner = $concat::setup::fragment_owner - $fraggroup = $concat::setup::fragment_group - $fragmode = $concat::setup::fragment_mode - - # The file type's semantics are problematic in that ensure => present will - # not over write a pre-existing symlink. We are attempting to provide - # backwards compatiblity with previous concat::fragment versions that - # supported the file type's ensure => /target syntax - - # be paranoid and only allow the fragment's file resource's ensure param to - # be file, absent, or a file target - $safe_ensure = $my_ensure ? { - '' => 'file', - undef => 'file', - 'file' => 'file', - 'present' => 'file', - 'absent' => 'absent', - default => $my_ensure, - } - # if it looks line ensure => /target syntax was used, fish that out - if ! ($my_ensure in ['', 'present', 'absent', 'file' ]) { - $ensure_target = $my_ensure - } else { - $ensure_target = undef - } - - # the file type's semantics only allows one of: ensure => /target, content, - # or source - if ($ensure_target and $source) or - ($ensure_target and $content) or - ($source and $content) { - fail('You cannot specify more than one of $content, $source, $ensure => /target') - } - - if ! ($content or $source or $ensure_target) { + if ! ($content or $source) { crit('No content, source or symlink specified') + } elsif ($content and $source) { + fail("Can't use 'source' and 'content' at the same time") } - file { "${fragdir}/fragments/${order}_${safe_name}": - ensure => $safe_ensure, - owner => $fragowner, - group => $fraggroup, - mode => $fragmode, - source => $source, + $safe_target_name = regsubst($target, '[/:\n\s]', '_', 'GM') + + file_fragment { $name: + tag => $safe_target_name, + order => $order, content => $content, - backup => $my_backup, - replace => true, - alias => "concat_fragment_${name}", - notify => Exec["concat_${target}"] + source => $source, } } diff --git a/manifests/init.pp b/manifests/init.pp index c44c03284..305b3d976 100644 --- a/manifests/init.pp +++ b/manifests/init.pp @@ -16,34 +16,21 @@ # Who will own the file # [*mode*] # The mode of the final file -# [*force*] -# Enables creating empty files if no fragments are present -# [*warn*] +# [*warn_header*] # Adds a normal shell style comment top of the file indicating that it is # built by puppet -# [*force*] # [*backup*] # Controls the filebucketing behavior of the final file and see File type # reference for its use. Defaults to 'puppet' # [*replace*] # Whether to replace a file that already exists on the local system # [*order*] -# [*ensure_newline*] -# [*gnu*] -# Deprecated +# Select whether to order associated fragments by 'alpha' or 'numeric'. +# Defaults to 'alpha'. # # === Actions: -# * Creates fragment directories if it didn't exist already -# * Executes the concatfragments.rb script to build the final file, this -# script will create directory/fragments.concat. Execution happens only -# when: -# * The directory changes -# * fragments.concat != final destination, this means rebuilds will happen -# whenever someone changes or deletes the final file. Checking is done -# using /usr/bin/cmp. -# * The Exec gets notified by something else - like the concat::fragment -# define -# * Copies the file over to the final destination using a file resource +# * Creates a file_concat resource from the electrical/puppet-lib-file_concat library. +# * Creates file_fragment resources from electrical/puppet-lib-file_concat # # === Aliases: # @@ -52,213 +39,80 @@ # * The final file can be referenced as File["/path/to/file"] or # File["concat_/path/to/file"] # + define concat( $ensure = 'present', $path = $name, $owner = undef, $group = undef, $mode = '0644', - $warn = false, - $force = false, + $warn_header = false, $backup = 'puppet', $replace = true, $order = 'alpha', - $ensure_newline = false, $validate_cmd = undef, - $gnu = undef ) { validate_re($ensure, '^present$|^absent$') validate_absolute_path($path) validate_string($owner) validate_string($group) validate_string($mode) - if ! (is_string($warn) or $warn == true or $warn == false) { - fail('$warn is not a string or boolean') + if ! (is_string($warn_header) or $warn_header == true or $warn_header == false) { + fail('$warn_header is not a string or boolean') } - validate_bool($force) - if ! concat_is_bool($backup) and ! is_string($backup) { + if ! is_bool($backup) and ! is_string($backup) { fail('$backup must be string or bool!') } validate_bool($replace) validate_re($order, '^alpha$|^numeric$') - validate_bool($ensure_newline) if $validate_cmd and ! is_string($validate_cmd) { fail('$validate_cmd must be a string') } - if $gnu { - warning('The $gnu parameter to concat is deprecated and has no effect') - } - - include concat::setup - $safe_name = regsubst($name, '[/:]', '_', 'G') - $concatdir = $concat::setup::concatdir - $fragdir = "${concatdir}/${safe_name}" - $concat_name = 'fragments.concat.out' - $script_command = $concat::setup::script_command - $default_warn_message = '# This file is managed by Puppet. DO NOT EDIT.' - $bool_warn_message = 'Using stringified boolean values (\'true\', \'yes\', \'on\', \'false\', \'no\', \'off\') to represent boolean true/false as the $warn parameter to concat is deprecated and will be treated as the warning message in a future release' + $safe_name = regsubst($name, '[/:\n\s]', '_', 'G') + $default_warn_message = "# This file is managed by Puppet. DO NOT EDIT.\n" - case $warn { + case $warn_header { true: { $warn_message = $default_warn_message - } - 'true', 'yes', 'on': { - warning($bool_warn_message) - $warn_message = $default_warn_message + $append_header = true } false: { $warn_message = '' } - 'false', 'no', 'off': { - warning($bool_warn_message) - $warn_message = '' - } default: { - $warn_message = $warn + $warn_message = $warn_header + $append_header = true } } - $warnmsg_escaped = regsubst($warn_message, '\'', '\'\\\'\'', 'G') - $warnflag = $warnmsg_escaped ? { - '' => '', - default => "-w '${warnmsg_escaped}'" - } - - $forceflag = $force ? { - true => '-f', - false => '', - } - - $orderflag = $order ? { - 'numeric' => '-n', - 'alpha' => '', - } - - $newlineflag = $ensure_newline ? { - true => '-l', - false => '', - } - - File { - backup => $backup, - } - - # reset poisoned Exec defaults - Exec { - user => undef, - group => undef, - } - if $ensure == 'present' { - file { $fragdir: - ensure => directory, - mode => '0750', - } - - file { "${fragdir}/fragments": - ensure => directory, - mode => '0750', - force => true, - ignore => ['.svn', '.git', '.gitignore'], - notify => Exec["concat_${name}"], - purge => true, - recurse => true, - } - - file { "${fragdir}/fragments.concat": - ensure => present, - mode => '0640', - } - - file { "${fragdir}/${concat_name}": - ensure => present, - mode => '0640', - } - - file { $name: - ensure => present, - owner => $owner, + file_concat { $name: + tag => $safe_name, + path => $path, + owner => $user, group => $group, mode => $mode, replace => $replace, - path => $path, - alias => "concat_${name}", - source => "${fragdir}/${concat_name}", backup => $backup, + order => $order, + validate_cmd => $validate_cmd, } - # Only newer versions of puppet 3.x support the validate_cmd parameter - if $validate_cmd { - File[$name] { - validate_cmd => $validate_cmd, - } - } - - # remove extra whitespace from string interpolation to make testing easier - $command = strip(regsubst("${script_command} -o \"${fragdir}/${concat_name}\" -d \"${fragdir}\" ${warnflag} ${forceflag} ${orderflag} ${newlineflag}", '\s+', ' ', 'G')) - - # make sure ruby is in the path for PE - if defined('$is_pe') and $::is_pe { - if $::kernel == 'windows' { - $command_path = "${::env_windows_installdir}/bin:${::path}" - } else { - $command_path = "/opt/puppet/bin:${::path}" + if $append_header { + file_fragment { "#{$name}_header": + tag => $safe_name, + content => $warn_message, + order => '0', } - } else { - $command_path = $::path - } - - # if puppet is running as root, this exec should also run as root to allow - # the concatfragments.rb script to potentially be installed in path that - # may not be accessible by a target non-root owner. - exec { "concat_${name}": - alias => "concat_${fragdir}", - command => $command, - notify => File[$name], - subscribe => File[$fragdir], - unless => "${command} -t", - path => $command_path, - require => [ - File[$fragdir], - File["${fragdir}/fragments"], - File["${fragdir}/fragments.concat"], - ], } } else { - file { [ - $fragdir, - "${fragdir}/fragments", - "${fragdir}/fragments.concat", - "${fragdir}/${concat_name}" - ]: - ensure => absent, - force => true, - } - - file { $path: - ensure => absent, + file_concat { $name: + ensure => $ensure, + tag => $safe_name, + path => $path, backup => $backup, } - - $absent_exec_command = $::kernel ? { - 'windows' => 'cmd.exe /c exit 0', - default => 'true', - } - - $absent_exec_path = $::kernel ? { - 'windows' => $::path, - default => '/bin:/usr/bin', - } - - # Need to have an unless here for idempotency. - exec { "concat_${name}": - alias => "concat_${fragdir}", - command => $absent_exec_command, - unless => $absent_exec_command, - path => $absent_exec_path, - } } } -# vim:sw=2:ts=2:expandtab:textwidth=79 diff --git a/manifests/setup.pp b/manifests/setup.pp deleted file mode 100644 index c5aedd82b..000000000 --- a/manifests/setup.pp +++ /dev/null @@ -1,64 +0,0 @@ -# === Class: concat::setup -# -# Sets up the concat system. This is a private class. -# -# [$concatdir] -# is where the fragments live and is set on the fact concat_basedir. -# Since puppet should always manage files in $concatdir and they should -# not be deleted ever, /tmp is not an option. -# -# It also copies out the concatfragments.{sh,rb} file to ${concatdir}/bin -# -class concat::setup { - if $caller_module_name != $module_name { - warning("${name} is deprecated as a public API of the ${module_name} module and should no longer be directly included in the manifest.") - } - - if $::concat_basedir { - $concatdir = $::concat_basedir - } else { - fail ('$concat_basedir not defined. Try running again with pluginsync=true on the [master] and/or [main] section of your node\'s \'/etc/puppet/puppet.conf\'.') - } - - # owner,group and mode of fragment files (on windows owner and access rights should - # be inherited from concatdir and not explicitly set to avoid problems) - $fragment_owner = $::osfamily ? { 'windows' => undef, default => $::id } - $fragment_mode = $::osfamily ? { 'windows' => undef, default => '0640' } - # test on gid fact availability to support older facter versions - if defined('$gid') and $::gid and $::osfamily != 'Windows' { - $fragment_group = $::gid - } else { - $fragment_group = undef - } - - $script_name = 'concatfragments.rb' - - $script_path = "${concatdir}/bin/${script_name}" - - $default_owner = $::osfamily ? { 'windows' => undef, default => $::id } - - $default_group = $default_owner ? { 'root' => '0', default => undef } - - $script_mode = $::osfamily ? { 'windows' => undef, default => '0755' } - - $script_command = $::osfamily? { - 'windows' => "ruby.exe '${script_path}'", - 'openbsd' => "/usr/local/bin/ruby21 '${script_path}'", - default => $script_path - } - - file { $script_path: - ensure => file, - owner => $default_owner, - group => $default_group, - mode => $script_mode, - source => "puppet:///modules/concat/${script_name}", - } - - file { [ $concatdir, "${concatdir}/bin" ]: - ensure => directory, - owner => $default_owner, - group => $default_group, - mode => '0755', - } -} diff --git a/metadata.json b/metadata.json index 9d7a46dd0..49c24c602 100644 --- a/metadata.json +++ b/metadata.json @@ -1,6 +1,6 @@ { "name": "puppetlabs-concat", - "version": "1.2.0", + "version": "2.0.0", "author": "Puppet Labs", "summary": "Construct files from multiple fragments.", "license": "Apache-2.0", @@ -105,6 +105,13 @@ } ], "dependencies": [ - {"name":"puppetlabs/stdlib","version_requirement":">= 3.2.0 < 5.0.0"} - ] + { + "name": "puppetlabs/stdlib", + "version_requirement": ">= 3.2.0 < 5.0.0" + }, + { + "name": "electrical/file_concat", + "version_requirement": ">= 1.0.0" + }, + ], } diff --git a/spec/acceptance/backup_spec.rb b/spec/acceptance/backup_spec.rb index 1d3a5dfe3..0a737a556 100644 --- a/spec/acceptance/backup_spec.rb +++ b/spec/acceptance/backup_spec.rb @@ -26,7 +26,7 @@ it 'applies the manifest twice with "Filebucketed" stdout and no stderr' do apply_manifest(pp, :catch_failures => true) do |r| - expect(r.stdout).to match(/Filebucketed #{basedir}\/file to puppet with sum 0140c31db86293a1a1e080ce9b91305f/) # sum is for file contents of 'old contents' + expect(r.stdout).to match(/Filebucketed #{basedir}\/file to puppet with sum 0140c31db86293a1a1e080ce9b91305f/) end apply_manifest(pp, :catch_changes => true) end diff --git a/spec/acceptance/concat_spec.rb b/spec/acceptance/concat_spec.rb index c5c972774..c281954b7 100644 --- a/spec/acceptance/concat_spec.rb +++ b/spec/acceptance/concat_spec.rb @@ -38,56 +38,6 @@ apply_manifest(pp, :catch_failures => true) apply_manifest(pp, :catch_changes => true) end - - describe file("#{vardir}/concat") do - it { should be_directory } - it { should be_owned_by username } - it("should be mode", :unless => (fact('osfamily') == 'AIX' or fact('osfamily') == 'windows')) { - should be_mode 755 - } - end - describe file("#{vardir}/concat/bin") do - it { should be_directory } - it { should be_owned_by username } - it("should be mode", :unless => (fact('osfamily') == 'AIX' or fact('osfamily') == 'windows')) { - should be_mode 755 - } - end - describe file("#{vardir}/concat/bin/#{scriptname}") do - it { should be_file } - it { should be_owned_by username } - it("should be mode", :unless => (fact('osfamily') == 'AIX' or fact('osfamily') == 'windows')) { - should be_mode 755 - } - end - describe file("#{vardir}/concat/#{safe_basedir}_file") do - it { should be_directory } - it { should be_owned_by username } - it("should be mode", :unless => (fact('osfamily') == 'AIX' or fact('osfamily') == 'windows')) { - should be_mode 750 - } - end - describe file("#{vardir}/concat/#{safe_basedir}_file/fragments") do - it { should be_directory } - it { should be_owned_by username } - it("should be mode", :unless => (fact('osfamily') == 'AIX' or fact('osfamily') == 'windows')) { - should be_mode 750 - } - end - describe file("#{vardir}/concat/#{safe_basedir}_file/fragments.concat") do - it { should be_file } - it { should be_owned_by username } - it("should be mode", :unless => (fact('osfamily') == 'AIX' or fact('osfamily') == 'windows')) { - should be_mode 640 - } - end - describe file("#{vardir}/concat/#{safe_basedir}_file/fragments.concat.out") do - it { should be_file } - it { should be_owned_by username } - it("should be mode", :unless => (fact('osfamily') == 'AIX' or fact('osfamily') == 'windows')) { - should be_mode 640 - } - end end context 'owner/group root' do @@ -133,20 +83,6 @@ should match '2' } end - describe file("#{vardir}/concat/#{safe_basedir}_file/fragments/01_1") do - it { should be_file } - it { should be_owned_by username } - it("should be mode", :unless => (fact('osfamily') == 'AIX' or fact('osfamily') == 'windows')) { - should be_mode 640 - } - end - describe file("#{vardir}/concat/#{safe_basedir}_file/fragments/02_2") do - it { should be_file } - it { should be_owned_by username } - it("should be mode", :unless => (fact('osfamily') == 'AIX' or fact('osfamily') == 'windows')) { - should be_mode 640 - } - end end context 'ensure' do diff --git a/spec/acceptance/deprecation_warnings_spec.rb b/spec/acceptance/deprecation_warnings_spec.rb deleted file mode 100644 index 11133ea9f..000000000 --- a/spec/acceptance/deprecation_warnings_spec.rb +++ /dev/null @@ -1,238 +0,0 @@ -require 'spec_helper_acceptance' - -describe 'deprecation warnings' do - basedir = default.tmpdir('concat') - - shared_examples 'has_warning' do |pp, w| - it 'applies the manifest twice with a stderr regex' do - expect(apply_manifest(pp, :catch_failures => true).stderr).to match(/#{Regexp.escape(w)}/m) - expect(apply_manifest(pp, :catch_changes => true).stderr).to match(/#{Regexp.escape(w)}/m) - end - end - - context 'concat gnu parameter' do - pp = <<-EOS - concat { '#{basedir}/file': - gnu => 'foo', - } - concat::fragment { 'foo': - target => '#{basedir}/file', - content => 'bar', - } - EOS - w = 'The $gnu parameter to concat is deprecated and has no effect' - - it_behaves_like 'has_warning', pp, w - end - - context 'concat warn parameter =>' do - ['true', 'yes', 'on'].each do |warn| - context warn do - pp = <<-EOS - concat { '#{basedir}/file': - warn => '#{warn}', - } - concat::fragment { 'foo': - target => '#{basedir}/file', - content => 'bar', - } - EOS - w = 'Using stringified boolean values (\'true\', \'yes\', \'on\', \'false\', \'no\', \'off\') to represent boolean true/false as the $warn parameter to concat is deprecated and will be treated as the warning message in a future release' - - it_behaves_like 'has_warning', pp, w - - describe file("#{basedir}/file") do - it { should be_file } - its(:content) { - should match '# This file is managed by Puppet. DO NOT EDIT.' - should match 'bar' - } - end - end - end - - ['false', 'no', 'off'].each do |warn| - context warn do - pp = <<-EOS - concat { '#{basedir}/file': - warn => '#{warn}', - } - concat::fragment { 'foo': - target => '#{basedir}/file', - content => 'bar', - } - EOS - w = 'Using stringified boolean values (\'true\', \'yes\', \'on\', \'false\', \'no\', \'off\') to represent boolean true/false as the $warn parameter to concat is deprecated and will be treated as the warning message in a future release' - - it_behaves_like 'has_warning', pp, w - - describe file("#{basedir}/file") do - it { should be_file } - its(:content) { - should_not match '# This file is managed by Puppet. DO NOT EDIT.' - should match 'bar' - } - end - end - end - end - - context 'concat::fragment ensure parameter', :unless => fact('osfamily') == 'windows' do - context 'target file exists' do - before(:all) do - pp = <<-EOS - file { '#{basedir}': - ensure => directory, - } - file { '#{basedir}/file1': - content => "file1 contents\n", - } - EOS - apply_manifest(pp) - end - - pp = <<-EOS - concat { '#{basedir}/file': } - concat::fragment { 'foo': - target => '#{basedir}/file', - ensure => '#{basedir}/file1', - } - EOS - w = 'Passing a value other than \'present\' or \'absent\' as the $ensure parameter to concat::fragment is deprecated. If you want to use the content of a file as a fragment please use the $source parameter.' - - it_behaves_like 'has_warning', pp, w - - describe file("#{basedir}/file") do - it { should be_file } - its(:content) { should match 'file1 contents' } - end - - describe 'the fragment can be changed from a symlink to a plain file', :unless => (fact("osfamily") == "windows") do - pp = <<-EOS - concat { '#{basedir}/file': } - concat::fragment { 'foo': - target => '#{basedir}/file', - content => 'new content', - } - EOS - - it 'applies the manifest twice with no stderr' do - apply_manifest(pp, :catch_failures => true) - apply_manifest(pp, :catch_changes => true) - end - - describe file("#{basedir}/file") do - it { should be_file } - its(:content) { - should match 'new content' - should_not match 'file1 contents' - } - end - end - end # target file exists - - context 'target does not exist', :unless => fact('osfamily') == 'windows' do - pp = <<-EOS - concat { '#{basedir}/file': } - concat::fragment { 'foo': - target => '#{basedir}/file', - ensure => '#{basedir}/file1', - } - EOS - w = 'Passing a value other than \'present\' or \'absent\' as the $ensure parameter to concat::fragment is deprecated. If you want to use the content of a file as a fragment please use the $source parameter.' - - it_behaves_like 'has_warning', pp, w - - describe file("#{basedir}/file") do - it { should be_file } - end - - describe 'the fragment can be changed from a symlink to a plain file', :unless => (fact('osfamily') == 'windows') do - pp = <<-EOS - concat { '#{basedir}/file': } - concat::fragment { 'foo': - target => '#{basedir}/file', - content => 'new content', - } - EOS - - it 'applies the manifest twice with no stderr' do - apply_manifest(pp, :catch_failures => true) - apply_manifest(pp, :catch_changes => true) - end - - describe file("#{basedir}/file") do - it { should be_file } - its(:content) { should match 'new content' } - end - end - end # target file exists - - end # concat::fragment ensure parameter - - context 'concat::fragment mode parameter' do - pp = <<-EOS - concat { '#{basedir}/file': } - concat::fragment { 'foo': - target => '#{basedir}/file', - content => 'bar', - mode => 'bar', - } - EOS - w = 'The $mode parameter to concat::fragment is deprecated and has no effect' - - it_behaves_like 'has_warning', pp, w - end - - context 'concat::fragment owner parameter' do - pp = <<-EOS - concat { '#{basedir}/file': } - concat::fragment { 'foo': - target => '#{basedir}/file', - content => 'bar', - owner => 'bar', - } - EOS - w = 'The $owner parameter to concat::fragment is deprecated and has no effect' - - it_behaves_like 'has_warning', pp, w - end - - context 'concat::fragment group parameter' do - pp = <<-EOS - concat { '#{basedir}/file': } - concat::fragment { 'foo': - target => '#{basedir}/file', - content => 'bar', - group => 'bar', - } - EOS - w = 'The $group parameter to concat::fragment is deprecated and has no effect' - - it_behaves_like 'has_warning', pp, w - end - - context 'concat::fragment backup parameter' do - pp = <<-EOS - concat { '#{basedir}/file': } - concat::fragment { 'foo': - target => '#{basedir}/file', - content => 'bar', - backup => 'bar', - } - EOS - w = 'The $backup parameter to concat::fragment is deprecated and has no effect' - - it_behaves_like 'has_warning', pp, w - end - - context 'include concat::setup' do - pp = <<-EOS - include concat::setup - EOS - w = 'concat::setup is deprecated as a public API of the concat module and should no longer be directly included in the manifest.' - - it_behaves_like 'has_warning', pp, w - end - -end diff --git a/spec/acceptance/empty_spec.rb b/spec/acceptance/empty_spec.rb deleted file mode 100644 index 4ab6e1e60..000000000 --- a/spec/acceptance/empty_spec.rb +++ /dev/null @@ -1,23 +0,0 @@ -require 'spec_helper_acceptance' - -describe 'concat force empty parameter' do - basedir = default.tmpdir('concat') - context 'should run successfully' do - pp = <<-EOS - concat { '#{basedir}/file': - mode => '0644', - force => true, - } - EOS - - it 'applies the manifest twice with no stderr' do - apply_manifest(pp, :catch_failures => true) - apply_manifest(pp, :catch_changes => true) - end - - describe file("#{basedir}/file") do - it { should be_file } - its(:content) { should_not match /1\n2/ } - end - end -end diff --git a/spec/acceptance/fragment_source_spec.rb b/spec/acceptance/fragment_source_spec.rb index a174e02b4..5d66e4d98 100644 --- a/spec/acceptance/fragment_source_spec.rb +++ b/spec/acceptance/fragment_source_spec.rb @@ -145,7 +145,7 @@ EOS it 'applies the manifest with resource failures' do - apply_manifest(pp, :expect_failures => true) + expect(apply_manifest(pp, :catch_failures => true).stderr).to match(/Failed to generate additional resources using 'eval_generate'/) end describe file("#{basedir}/fail_no_source") do #FIXME: Serverspec::Type::File doesn't support exists? for some reason. so... hack. diff --git a/spec/acceptance/newline_spec.rb b/spec/acceptance/newline_spec.rb deleted file mode 100644 index c1fa16a10..000000000 --- a/spec/acceptance/newline_spec.rb +++ /dev/null @@ -1,67 +0,0 @@ -require 'spec_helper_acceptance' - -describe 'concat ensure_newline parameter' do - basedir = default.tmpdir('concat') - context '=> false' do - before(:all) do - pp = <<-EOS - file { '#{basedir}': - ensure => directory - } - EOS - - apply_manifest(pp) - end - pp = <<-EOS - concat { '#{basedir}/file': - ensure_newline => false, - } - concat::fragment { '1': - target => '#{basedir}/file', - content => '1', - } - concat::fragment { '2': - target => '#{basedir}/file', - content => '2', - } - EOS - - it 'applies the manifest twice with no stderr' do - apply_manifest(pp, :catch_failures => true) - apply_manifest(pp, :catch_changes => true) - end - - describe file("#{basedir}/file") do - it { should be_file } - its(:content) { should match '12' } - end - end - - context '=> true' do - pp = <<-EOS - concat { '#{basedir}/file': - ensure_newline => true, - } - concat::fragment { '1': - target => '#{basedir}/file', - content => '1', - } - concat::fragment { '2': - target => '#{basedir}/file', - content => '2', - } - EOS - - it 'applies the manifest twice with no stderr' do - apply_manifest(pp, :catch_failures => true) - apply_manifest(pp, :catch_changes => true) - end - - describe file("#{basedir}/file") do - it { should be_file } - its(:content) { - should match /1\n2\n/ - } - end - end -end diff --git a/spec/acceptance/nodesets/default.yml b/spec/acceptance/nodesets/default.yml index b82685158..4e2cb809e 100644 --- a/spec/acceptance/nodesets/default.yml +++ b/spec/acceptance/nodesets/default.yml @@ -1,10 +1,10 @@ HOSTS: - centos-66-x64: + centos-65-x64: roles: - master platform: el-6-x86_64 - box : puppetlabs/centos-6.6-64-nocm - box_url : http://puppet-vagrant-boxes.puppetlabs.com/centos-6.6-64-nocm + box : centos-65-x64-vbox436-nocm + box_url : http://puppet-vagrant-boxes.puppetlabs.com/centos-65-x64-virtualbox-nocm.box hypervisor : vagrant CONFIG: - type: git + type: foss diff --git a/spec/acceptance/order_spec.rb b/spec/acceptance/order_spec.rb index c158dd880..b746b1c9c 100644 --- a/spec/acceptance/order_spec.rb +++ b/spec/acceptance/order_spec.rb @@ -37,7 +37,7 @@ end describe 'alpha' do - it_behaves_like 'sortby', 'alpha', /string10string1string2/ + it_behaves_like 'sortby', 'alpha', /string1string10string2/ end describe 'numeric' do @@ -83,7 +83,7 @@ end end describe 'alpha' do - it_should_behave_like 'order_by', 'alpha', /string2string1string3/ + it_should_behave_like 'order_by', 'alpha', /string3string2string1/ end describe 'numeric' do it_should_behave_like 'order_by', 'numeric', /string3string2string1/ diff --git a/spec/acceptance/replace_spec.rb b/spec/acceptance/replace_spec.rb index bd597ed19..68d7383fe 100644 --- a/spec/acceptance/replace_spec.rb +++ b/spec/acceptance/replace_spec.rb @@ -226,7 +226,7 @@ end end - # XXX concat's force param currently enables the creation of empty files + # XXX # when there are no fragments, and the replace param will only replace # files and symlinks, not directories. The semantics either need to be # changed, extended, or a new param introduced to control directory @@ -234,7 +234,6 @@ context 'should succeed', :pending => 'not yet implemented' do pp = <<-EOS concat { '#{basedir}/file': - force => true, } concat::fragment { '1': diff --git a/spec/acceptance/warn_spec.rb b/spec/acceptance/warn_header_spec.rb similarity index 94% rename from spec/acceptance/warn_spec.rb rename to spec/acceptance/warn_header_spec.rb index 2788607c8..52615e1d7 100644 --- a/spec/acceptance/warn_spec.rb +++ b/spec/acceptance/warn_header_spec.rb @@ -1,11 +1,11 @@ require 'spec_helper_acceptance' -describe 'concat warn =>' do +describe 'concat warn_header =>' do basedir = default.tmpdir('concat') context 'true should enable default warning message' do pp = <<-EOS concat { '#{basedir}/file': - warn => true, + warn_header => true, } concat::fragment { '1': @@ -38,7 +38,7 @@ context 'false should not enable default warning message' do pp = <<-EOS concat { '#{basedir}/file': - warn => false, + warn_header => false, } concat::fragment { '1': @@ -71,7 +71,7 @@ context '# foo should overide default warning message' do pp = <<-EOS concat { '#{basedir}/file': - warn => '# foo', + warn_header => "# foo\n", } concat::fragment { '1': diff --git a/spec/spec_helper_acceptance.rb b/spec/spec_helper_acceptance.rb index da994f8df..1a3ffc0ad 100644 --- a/spec/spec_helper_acceptance.rb +++ b/spec/spec_helper_acceptance.rb @@ -16,21 +16,29 @@ hosts.each do |host| on hosts, "mkdir -p #{host['distmoduledir']}" if host['platform'] =~ /sles-1/i || host['platform'] =~ /solaris-1/i - get_stdlib = <<-EOS + get_deps = <<-EOS package{'wget':} - exec{'download': + exec{'download-stdlib': command => "wget -P /root/ https://forgeapi.puppetlabs.com/v3/files/puppetlabs-stdlib-4.5.1.tar.gz --no-check-certificate", path => ['/opt/csw/bin/','/usr/bin/'] } + exec{'download-file_concat': + command => "wget -P /root/ https://forgeapi.puppetlabs.com/v3/files/electrical-file_concat-1.0.0.tar.gz --no-check-certificate", + path => ['/opt/csw/bin/','/usr/bin/'] + } EOS - apply_manifest_on(host, get_stdlib) + apply_manifest_on(host, get_deps) # have to use force otherwise it checks ssl cert even though it is a local file on host, puppet('module install /root/puppetlabs-stdlib-4.5.1.tar.gz --force --ignore-dependencies'), {:acceptable_exit_codes => [0, 1]} + on host, puppet('module install /root/electrical-file_concat-1.0.0.tar.gz --force --ignore-dependencies'), {:acceptable_exit_codes => [0, 1]} elsif host['platform'] =~ /windows/i on host, shell('curl -k -o c:/puppetlabs-stdlib-4.5.1.tar.gz https://forgeapi.puppetlabs.com/v3/files/puppetlabs-stdlib-4.5.1.tar.gz') on host, puppet('module install c:/puppetlabs-stdlib-4.5.1.tar.gz --force --ignore-dependencies'), {:acceptable_exit_codes => [0, 1]} + on host, shell('curl -k -o c:/electrical-file_concat-1.0.0.tar.gz https://forgeapi.puppetlabs.com/v3/files/electrical-file_concat-1.0.0.tar.gz') + on host, puppet('module install c:/electrical-file_concat-1.0.0.tar.gz --force --ignore-dependencies'), {:acceptable_exit_codes => [0, 1]} else on host, puppet('module install puppetlabs-stdlib'), {:acceptable_exit_codes => [0, 1]} + on host, puppet('module install electrical-file_concat'), {:acceptable_exit_codes => [0, 1]} end end end diff --git a/spec/unit/classes/concat_setup_spec.rb b/spec/unit/classes/concat_setup_spec.rb deleted file mode 100644 index c6ff93e0a..000000000 --- a/spec/unit/classes/concat_setup_spec.rb +++ /dev/null @@ -1,98 +0,0 @@ -require 'spec_helper' - -describe 'concat::setup', :type => :class do - - shared_examples 'setup' do |concatdir| - concatdir = '/foo' if concatdir.nil? - - let(:facts) do - { - :concat_basedir => concatdir, - :caller_module_name => 'Test', - :osfamily => 'Debian', - :id => 'root', - :is_pe => false, - } - end - - it do - should contain_file("#{concatdir}/bin/concatfragments.rb").with({ - :mode => '0755', - :owner => 'root', - :group => 0, - :source => 'puppet:///modules/concat/concatfragments.rb', - }) - end - - [concatdir, "#{concatdir}/bin"].each do |file| - it do - should contain_file(file).with({ - :ensure => 'directory', - :mode => '0755', - :owner => 'root', - :group => 0, - }) - end - end - end - - context 'facts' do - context 'concat_basedir =>' do - context '/foo' do - it_behaves_like 'setup', '/foo' - end - end - end # facts - - context 'deprecated as a public class' do - it 'should create a warning' do - skip('rspec-puppet support for testing warning()') - end - end - - context "on osfamily Solaris" do - concatdir = '/foo' - let(:facts) do - { - :concat_basedir => concatdir, - :caller_module_name => 'Test', - :osfamily => 'Solaris', - :id => 'root', - :is_pe => false, - } - end - - it do - should contain_file("#{concatdir}/bin/concatfragments.rb").with({ - :ensure => 'file', - :owner => 'root', - :group => 0, - :mode => '0755', - :source => 'puppet:///modules/concat/concatfragments.rb', - }) - end - end # on osfamily Solaris - - context "on osfamily windows" do - concatdir = '/foo' - let(:facts) do - { - :concat_basedir => concatdir, - :caller_module_name => 'Test', - :osfamily => 'windows', - :id => 'batman', - :is_pe => false, - } - end - - it do - should contain_file("#{concatdir}/bin/concatfragments.rb").with({ - :ensure => 'file', - :owner => nil, - :group => nil, - :mode => nil, - :source => 'puppet:///modules/concat/concatfragments.rb', - }) - end - end # on osfamily windows -end diff --git a/spec/unit/defines/concat_fragment_spec.rb b/spec/unit/defines/concat_fragment_spec.rb index 6cf3e4296..98be58a18 100644 --- a/spec/unit/defines/concat_fragment_spec.rb +++ b/spec/unit/defines/concat_fragment_spec.rb @@ -9,50 +9,21 @@ :content => nil, :source => nil, :order => 10, - :ensure => 'present', }.merge(params) - safe_name = title.gsub(/[\/\n]/, '_') - safe_target_name = p[:target].gsub(/[\/\n]/, '_') - concatdir = '/var/lib/puppet/concat' - fragdir = "#{concatdir}/#{safe_target_name}" id = 'root' gid = 'root' - if p[:ensure] == 'absent' - safe_ensure = p[:ensure] - else - safe_ensure = 'file' - end let(:title) { title } - let(:facts) do - { - :concat_basedir => concatdir, - :id => id, - :gid => gid, - :osfamily => 'Debian', - :path => '/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin', - :is_pe => false, - } - end let(:params) { params } let(:pre_condition) do "concat{ '#{p[:target]}': }" end it do - should contain_class('concat::setup') should contain_concat(p[:target]) - should contain_file("#{fragdir}/fragments/#{p[:order]}_#{safe_name}").with({ - :ensure => safe_ensure, - :owner => id, - :group => gid, - :mode => '0640', - :source => p[:source], - :content => p[:content], - :alias => "concat_fragment_#{title}", - :backup => 'puppet', - }) + should contain_file_concat(p[:target]) + should contain_file_fragment(title) end end @@ -60,6 +31,7 @@ ['0', '1', 'a', 'z'].each do |title| it_behaves_like 'fragment', title, { :target => '/etc/motd', + :content => "content for #{title}" } end end # title @@ -69,42 +41,21 @@ context target do it_behaves_like 'fragment', target, { :target => '/etc/motd', + :content => "content for #{target}" } end end context 'false' do let(:title) { 'motd_header' } - let(:facts) {{ :concat_basedir => '/tmp', :is_pe => false }} let(:params) {{ :target => false }} it 'should fail' do - expect { should }.to raise_error(Puppet::Error, /is not a string/) + expect { catalogue }.to raise_error(Puppet::Error, /is not a string/) end end end # target => - context 'ensure =>' do - ['present', 'absent'].each do |ens| - context ens do - it_behaves_like 'fragment', 'motd_header', { - :ensure => ens, - :target => '/etc/motd', - } - end - end - - context 'any value other than \'present\' or \'absent\'' do - let(:title) { 'motd_header' } - let(:facts) {{ :concat_basedir => '/tmp', :is_pe => false }} - let(:params) {{ :ensure => 'invalid', :target => '/etc/motd' }} - - it 'should create a warning' do - skip('rspec-puppet support for testing warning()') - end - end - end # ensure => - context 'content =>' do ['', 'ashp is our hero'].each do |content| context content do @@ -117,11 +68,10 @@ context 'false' do let(:title) { 'motd_header' } - let(:facts) {{ :concat_basedir => '/tmp', :is_pe => false }} let(:params) {{ :content => false, :target => '/etc/motd' }} it 'should fail' do - expect { should }.to raise_error(Puppet::Error, /is not a string/) + expect { catalogue }.to raise_error(Puppet::Error, /is not a string/) end end end # content => @@ -138,193 +88,28 @@ context 'false' do let(:title) { 'motd_header' } - let(:facts) {{ :concat_basedir => '/tmp', :is_pe => false }} let(:params) {{ :source => false, :target => '/etc/motd' }} it 'should fail' do - expect { should }.to raise_error(Puppet::Error, /is not a string or an Array/) + expect { catalogue }.to raise_error(Puppet::Error, /is not a string or an Array/) end end end # source => - context 'order =>' do - ['', '42', 'a', 'z'].each do |order| - context '\'\'' do - it_behaves_like 'fragment', 'motd_header', { - :order => order, - :target => '/etc/motd', - } - end - end - - context 'false' do - let(:title) { 'motd_header' } - let(:facts) {{ :concat_basedir => '/tmp', :is_pe => false }} - let(:params) {{ :order => false, :target => '/etc/motd' }} - - it 'should fail' do - expect { should }.to raise_error(Puppet::Error, /is not a string or integer/) - end - end - - context '123:456' do - let(:title) { 'motd_header' } - let(:facts) {{ :concat_basedir => '/tmp', :is_pe => false }} - let(:params) {{ :order => '123:456', :target => '/etc/motd' }} - - it 'should fail' do - expect { should }.to raise_error(Puppet::Error, /cannot contain/) - end - end - context '123/456' do - let(:title) { 'motd_header' } - let(:facts) {{ :concat_basedir => '/tmp', :is_pe => false }} - let(:params) {{ :order => '123/456', :target => '/etc/motd' }} - - it 'should fail' do - expect { should }.to raise_error(Puppet::Error, /cannot contain/) - end - end - context '123\n456' do - let(:title) { 'motd_header' } - let(:facts) {{ :concat_basedir => '/tmp', :is_pe => false }} - let(:params) {{ :order => "123\n456", :target => '/etc/motd' }} - - it 'should fail' do - expect { should }.to raise_error(Puppet::Error, /cannot contain/) - end - end - end # order => - context 'more than one content source' do - error_msg = 'You cannot specify more than one of $content, $source, $ensure => /target' - - context 'ensure => target and source' do - let(:title) { 'motd_header' } - let(:facts) do - { - :concat_basedir => '/tmp', - :osfamily => 'Debian', - :id => 'root', - :is_pe => false, - :gid => 'root', - } - end - let(:params) do - { - :target => '/etc/motd', - :ensure => '/foo', - :source => '/bar', - } - end - - it 'should fail' do - expect { should }.to raise_error(Puppet::Error, /#{Regexp.escape(error_msg)}/m) - end - end - - context 'ensure => target and content' do - let(:title) { 'motd_header' } - let(:facts) do - { - :concat_basedir => '/tmp', - :osfamily => 'Debian', - :id => 'root', - :is_pe => false, - :gid => 'root', - } - end - let(:params) do - { - :target => '/etc/motd', - :ensure => '/foo', - :content => 'bar', - } - end - - it 'should fail' do - expect { should }.to raise_error(Puppet::Error, /#{Regexp.escape(error_msg)}/m) - end - end - context 'source and content' do let(:title) { 'motd_header' } - let(:facts) do - { - :concat_basedir => '/tmp', - :osfamily => 'Debian', - :id => 'root', - :is_pe => false, - :gid => 'root', - } - end let(:params) do { - :target => '/etc/motd', + :target => '/etc/motd', :source => '/foo', :content => 'bar', } end it 'should fail' do - expect { should }.to raise_error(Puppet::Error, /#{Regexp.escape(error_msg)}/m) + expect { catalogue }.to raise_error(Puppet::Error, /Can't use 'source' and 'content' at the same time/) end end - end # more than one content source - - describe 'deprecated parameter' do - context 'mode =>' do - context '1755' do - it_behaves_like 'fragment', 'motd_header', { - :mode => '1755', - :target => '/etc/motd', - } - - it 'should create a warning' do - skip('rspec-puppet support for testing warning()') - end - end - end # mode => - - context 'owner =>' do - context 'apenny' do - it_behaves_like 'fragment', 'motd_header', { - :owner => 'apenny', - :target => '/etc/motd', - } - - it 'should create a warning' do - skip('rspec-puppet support for testing warning()') - end - end - end # owner => - - context 'group =>' do - context 'apenny' do - it_behaves_like 'fragment', 'motd_header', { - :group => 'apenny', - :target => '/etc/motd', - } - - it 'should create a warning' do - skip('rspec-puppet support for testing warning()') - end - end - end # group => - - context 'backup =>' do - context 'foo' do - it_behaves_like 'fragment', 'motd_header', { - :backup => 'foo', - :target => '/etc/motd', - } - - it 'should create a warning' do - skip('rspec-puppet support for testing warning()') - end - end - end # backup => - end # deprecated params - end diff --git a/spec/unit/defines/concat_spec.rb b/spec/unit/defines/concat_spec.rb index 115a0f55b..176f24a37 100644 --- a/spec/unit/defines/concat_spec.rb +++ b/spec/unit/defines/concat_spec.rb @@ -13,20 +13,15 @@ :owner => nil, :group => nil, :mode => '0644', - :warn => false, - :force => false, + :warn_header => false, + #:force => false, :backup => 'puppet', :replace => true, - :order => 'alpha', - :ensure_newline => false, - :validate_cmd => nil, }.merge(params) safe_name = title.gsub('/', '_') - concatdir = '/var/lib/puppet/concat' - fragdir = "#{concatdir}/#{safe_name}" concat_name = 'fragments.concat.out' - default_warn_message = '# This file is managed by Puppet. DO NOT EDIT.' + default_warn_message = "# This file is managed by Puppet. DO NOT EDIT.\n" file_defaults = { :backup => p[:backup], @@ -36,7 +31,6 @@ let(:params) { params } let(:facts) do { - :concat_basedir => concatdir, :id => id, :osfamily => 'Debian', :path => '/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin', @@ -47,117 +41,23 @@ if p[:ensure] == 'present' it do - should contain_file(fragdir).with(file_defaults.merge({ - :ensure => 'directory', - :mode => '0750', - })) - end - - it do - should contain_file("#{fragdir}/fragments").with(file_defaults.merge({ - :ensure => 'directory', - :mode => '0750', - :force => true, - :ignore => ['.svn', '.git', '.gitignore'], - :purge => true, - :recurse => true, - })) - end - - [ - "#{fragdir}/fragments.concat", - "#{fragdir}/#{concat_name}", - ].each do |file| - it do - should contain_file(file).with(file_defaults.merge({ - :ensure => 'present', - :mode => '0640', - })) - end - end - - it do - should contain_file(title).with(file_defaults.merge({ + should contain_concat(title).with(file_defaults.merge({ :ensure => 'present', :owner => p[:owner], :group => p[:group], :mode => p[:mode], - :replace => p[:replace], :path => p[:path], - :alias => "concat_#{title}", - :source => "#{fragdir}/#{concat_name}", - :validate_cmd => p[:validate_cmd], :backup => p[:backup], + :replace => p[:replace], })) end - - cmd = "#{concatdir}/bin/concatfragments.rb " + - "-o \"#{concatdir}/#{safe_name}/fragments.concat.out\" " + - "-d \"#{concatdir}/#{safe_name}\"" - - # flag order: fragdir, warnflag, forceflag, orderflag, newlineflag - if p.has_key?(:warn) - case p[:warn] - when TrueClass - message = default_warn_message - when 'true', 'yes', 'on' - # should generate a stringified boolean warning - message = default_warn_message - when FalseClass - message = nil - when 'false', 'no', 'off' - # should generate a stringified boolean warning - message = nil - else - message = p[:warn] - end - - unless message.nil? - cmd += " -w \'#{message}\'" - end - end - - cmd += " -f" if p[:force] - cmd += " -n" if p[:order] == 'numeric' - cmd += " -l" if p[:ensure_newline] == true - - it do - should contain_exec("concat_#{title}").with({ - :alias => "concat_#{fragdir}", - :command => cmd, - :unless => "#{cmd} -t", - }) - end else - [ - fragdir, - "#{fragdir}/fragments", - "#{fragdir}/fragments.concat", - "#{fragdir}/#{concat_name}", - ].each do |file| - it do - should contain_file(file).with(file_defaults.merge({ - :ensure => 'absent', - :force => true, - })) - end - end - it do - should contain_file(title).with(file_defaults.merge({ + should contain_concat(title).with(file_defaults.merge({ :ensure => 'absent', :backup => p[:backup], })) end - - it do - should contain_exec("concat_#{title}").with({ - :alias => "concat_#{fragdir}", - :command => 'true', - :unless => 'true', - :path => '/bin:/usr/bin', - }) - end end end @@ -175,14 +75,14 @@ context title do let(:title) { title } it 'should fail' do - expect { should }.to raise_error(Puppet::Error, /is not an absolute path/) + expect { catalogue }.to raise_error(Puppet::Error, /is not an absolute path/) end end end end context 'with path param' do - ['./foo', 'foo', 'foo/bar'].each do |title| + ['/foo', 'foo', 'foo/bar'].each do |title| context title do it_behaves_like 'concat', title, { :path => '/etc/foo.bar' } end @@ -205,7 +105,7 @@ let(:title) { '/etc/foo.bar' } let(:params) {{ :ensure => 'invalid' }} it 'should fail' do - expect { should }.to raise_error(Puppet::Error, /#{Regexp.escape('does not match "^present$|^absent$"')}/) + expect { catalogue }.to raise_error(Puppet::Error, /#{Regexp.escape('does not match "^present$|^absent$"')}/) end end end # ensure => @@ -220,7 +120,7 @@ let(:title) { '/etc/foo.bar' } let(:params) {{ :path => path }} it 'should fail' do - expect { should }.to raise_error(Puppet::Error, /is not an absolute path/) + expect { catalogue }.to raise_error(Puppet::Error, /is not an absolute path/) end end end @@ -235,7 +135,7 @@ let(:title) { '/etc/foo.bar' } let(:params) {{ :owner => false }} it 'should fail' do - expect { should }.to raise_error(Puppet::Error, /is not a string/) + expect { catalogue }.to raise_error(Puppet::Error, /is not a string/) end end end # owner => @@ -249,7 +149,7 @@ let(:title) { '/etc/foo.bar' } let(:params) {{ :group => false }} it 'should fail' do - expect { should }.to raise_error(Puppet::Error, /is not a string/) + expect { catalogue }.to raise_error(Puppet::Error, /is not a string/) end end end # group => @@ -263,22 +163,22 @@ let(:title) { '/etc/foo.bar' } let(:params) {{ :mode => false }} it 'should fail' do - expect { should }.to raise_error(Puppet::Error, /is not a string/) + expect { catalogue }.to raise_error(Puppet::Error, /is not a string/) end end end # mode => - context 'warn =>' do + context 'warn_header =>' do [true, false, '# foo'].each do |warn| context warn do - it_behaves_like 'concat', '/etc/foo.bar', { :warn => warn } + it_behaves_like 'concat', '/etc/foo.bar', { :warn_header => warn } end end context '(stringified boolean)' do ['true', 'yes', 'on', 'false', 'no', 'off'].each do |warn| context warn do - it_behaves_like 'concat', '/etc/foo.bar', { :warn => warn } + it_behaves_like 'concat', '/etc/foo.bar', { :warn_header => warn } it 'should create a warning' do skip('rspec-puppet support for testing warning()') @@ -289,28 +189,28 @@ context '123' do let(:title) { '/etc/foo.bar' } - let(:params) {{ :warn => 123 }} + let(:params) {{ :warn_header => 123 }} it 'should fail' do - expect { should }.to raise_error(Puppet::Error, /is not a string or boolean/) + expect { catalogue }.to raise_error(Puppet::Error, /is not a string or boolean/) end end end # warn => - context 'force =>' do - [true, false].each do |force| - context force do - it_behaves_like 'concat', '/etc/foo.bar', { :force => force } - end - end - - context '123' do - let(:title) { '/etc/foo.bar' } - let(:params) {{ :force => 123 }} - it 'should fail' do - expect { should }.to raise_error(Puppet::Error, /is not a boolean/) - end - end - end # force => + #context 'force =>' do + # [true, false].each do |force| + # context force do + # it_behaves_like 'concat', '/etc/foo.bar', { :force => force } + # end + # end + + # context '123' do + # let(:title) { '/etc/foo.bar' } + # let(:params) {{ :force => 123 }} + # it 'should fail' do + # expect { catalogue }.to raise_error(Puppet::Error, /is not a boolean/) + # end + # end + #end # force => context 'backup =>' do context 'reverse' do @@ -329,7 +229,7 @@ let(:title) { '/etc/foo.bar' } let(:params) {{ :backup => [] }} it 'should fail' do - expect { should }.to raise_error(Puppet::Error, /backup must be string or bool/) + expect { catalogue }.to raise_error(Puppet::Error, /backup must be string or bool/) end end end # backup => @@ -345,71 +245,24 @@ let(:title) { '/etc/foo.bar' } let(:params) {{ :replace => 123 }} it 'should fail' do - expect { should }.to raise_error(Puppet::Error, /is not a boolean/) + expect { catalogue }.to raise_error(Puppet::Error, /is not a boolean/) end end end # replace => - context 'order =>' do - ['alpha', 'numeric'].each do |order| - context order do - it_behaves_like 'concat', '/etc/foo.bar', { :order => order } - end - end - - context 'invalid' do - let(:title) { '/etc/foo.bar' } - let(:params) {{ :order => 'invalid' }} - it 'should fail' do - expect { should }.to raise_error(Puppet::Error, /#{Regexp.escape('does not match "^alpha$|^numeric$"')}/) - end - end - end # order => - - context 'ensure_newline =>' do - [true, false].each do |ensure_newline| - context 'true' do - it_behaves_like 'concat', '/etc/foo.bar', { :ensure_newline => ensure_newline} - end - end - - context '123' do - let(:title) { '/etc/foo.bar' } - let(:params) {{ :ensure_newline => 123 }} - it 'should fail' do - expect { should }.to raise_error(Puppet::Error, /is not a boolean/) - end - end - end # ensure_newline => - - context 'validate_cmd =>' do - context '/usr/bin/test -e %' do - it_behaves_like 'concat', '/etc/foo.bar', { :validate_cmd => '/usr/bin/test -e %' } - end - - [ 1234, true ].each do |cmd| - context cmd do - let(:title) { '/etc/foo.bar' } - let(:params) {{ :validate_cmd => cmd }} - it 'should fail' do - expect { should }.to raise_error(Puppet::Error, /\$validate_cmd must be a string/) - end - end - end - end # validate_cmd => - - describe 'deprecated parameter' do - context 'gnu =>' do - context 'foo' do - it_behaves_like 'concat', '/etc/foo.bar', { :gnu => 'foo'} - - it 'should create a warning' do - skip('rspec-puppet support for testing warning()') - end - end - end - end - + #context 'order =>' do + # ['alpha', 'numeric'].each do |order| + # context order do + # it_behaves_like 'concat', '/etc/foo.bar', { :order => order } + # end + # end + + # context 'invalid' do + # let(:title) { '/etc/foo.bar' } + # let(:params) {{ :order => 'invalid' }} + # it 'should fail' do + # expect { catalogue }.to raise_error(Puppet::Error, /#{Regexp.escape('does not match "^alpha$|^numeric$"')}/) + # end + # end + #end # order => end - -# vim:sw=2:ts=2:expandtab:textwidth=79 diff --git a/spec/unit/facts/concat_basedir_spec.rb b/spec/unit/facts/concat_basedir_spec.rb deleted file mode 100644 index 41bc90f15..000000000 --- a/spec/unit/facts/concat_basedir_spec.rb +++ /dev/null @@ -1,18 +0,0 @@ -require 'spec_helper' - -describe 'concat_basedir', :type => :fact do - before(:each) { Facter.clear } - - context 'Puppet[:vardir] ==' do - it '/var/lib/puppet' do - Puppet.stubs(:[]).with(:vardir).returns('/var/lib/puppet') - Facter.fact(:concat_basedir).value.should == '/var/lib/puppet/concat' - end - - it '/home/apenny/.puppet/var' do - Puppet.stubs(:[]).with(:vardir).returns('/home/apenny/.puppet/var') - Facter.fact(:concat_basedir).value.should == '/home/apenny/.puppet/var/concat' - end - end - -end