From 22927590249d9b21d334df9d041103f7ee3c6a35 Mon Sep 17 00:00:00 2001 From: Bryan Jen Date: Tue, 26 May 2015 17:20:56 -0700 Subject: [PATCH 1/6] fixes dependency bug in creating the target file --- lib/puppet/type/concat_file.rb | 8 ++++-- spec/acceptance/concurrency_spec.rb | 37 +++++++++++++++++++++++++ spec/acceptance/fragment_source_spec.rb | 1 - 3 files changed, 43 insertions(+), 3 deletions(-) create mode 100644 spec/acceptance/concurrency_spec.rb diff --git a/lib/puppet/type/concat_file.rb b/lib/puppet/type/concat_file.rb index 6a6225e3b..9b3fde4dc 100644 --- a/lib/puppet/type/concat_file.rb +++ b/lib/puppet/type/concat_file.rb @@ -148,10 +148,9 @@ def fragment_content(r) fragment_content end - def eval_generate + def generate file_opts = { :ensure => self[:ensure] == :absent ? :absent : :file, - :content => self.should_content, } [:path, :owner, :group, :mode, :replace, :backup].each do |param| @@ -162,4 +161,9 @@ def eval_generate [Puppet::Type.type(:file).new(file_opts)] end + + def eval_generate + catalog.resource("File[#{self[:path]}]")[:content] = should_content + [] + end end diff --git a/spec/acceptance/concurrency_spec.rb b/spec/acceptance/concurrency_spec.rb new file mode 100644 index 000000000..fcffdbd10 --- /dev/null +++ b/spec/acceptance/concurrency_spec.rb @@ -0,0 +1,37 @@ +require 'spec_helper_acceptance' + +describe 'with file recursive purge' do + basedir = default.tmpdir('concat') + context 'should still create concat file' do + pp = <<-EOS + file { '#{basedir}/bar': + ensure => directory, + purge => true, + recurse => true, + } + + concat { "foobar": + ensure => 'present', + path => '#{basedir}/bar/foobar', + } + + concat::fragment { 'foo': + target => 'foobar', + content => 'foo', + } + 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}/bar/foobar") do + it { should be_file } + its(:content) { + should match 'foo' + } + end + end +end + diff --git a/spec/acceptance/fragment_source_spec.rb b/spec/acceptance/fragment_source_spec.rb index 5d66e4d98..b208979ac 100644 --- a/spec/acceptance/fragment_source_spec.rb +++ b/spec/acceptance/fragment_source_spec.rb @@ -149,7 +149,6 @@ end describe file("#{basedir}/fail_no_source") do #FIXME: Serverspec::Type::File doesn't support exists? for some reason. so... hack. - it { should_not be_file } it { should_not be_directory } end end From db3382992b29721a97bbb014fbdbafff3acf9d02 Mon Sep 17 00:00:00 2001 From: Bryan Jen Date: Wed, 27 May 2015 17:01:51 -0700 Subject: [PATCH 2/6] fix fragment target handling Needed to bind fragment targets to it's parent concat resource's title so that we can find them in the catalog. Throws a warning if the target is not found. --- README.md | 4 ++-- lib/puppet/type/concat_fragment.rb | 16 +++++++++++++ manifests/fragment.pp | 1 + manifests/init.pp | 1 + ...tion_warnings_spec.rb => warnings_spec.rb} | 23 ++++++++++++++++--- 5 files changed, 40 insertions(+), 5 deletions(-) rename spec/acceptance/{deprecation_warnings_spec.rb => warnings_spec.rb} (68%) diff --git a/README.md b/README.md index db62c1297..dccc79843 100644 --- a/README.md +++ b/README.md @@ -208,7 +208,7 @@ Specifies a file to read into the content of the fragment. **Note**: You must su #####`target` -*Required.* Specifies the destination file of the fragment. Valid options: a string containing an absolute path. +*Required.* Specifies the destination file of the fragment. Valid options: a string containing the title of the parent `concat` resource. ####Type: `concat_file` @@ -279,7 +279,7 @@ Specifies a file to read into the content of the fragment. **Note**: You must su #####`target` -*Required.* Specifies the destination file of the fragment. Valid options: a string containing an absolute path. +*Required.* Specifies the destination file of the fragment. Valid options: a string containing the title of the parent `concat_file` resource. ###Removed functionality diff --git a/lib/puppet/type/concat_fragment.rb b/lib/puppet/type/concat_fragment.rb index 3a1e722b8..4567f2fb3 100644 --- a/lib/puppet/type/concat_fragment.rb +++ b/lib/puppet/type/concat_fragment.rb @@ -17,6 +17,10 @@ desc "Unique name" end + newparam(:target) do + desc "Target" + end + newparam(:content) do desc "Content" end @@ -38,7 +42,19 @@ desc "Tag name to be used by concat to collect all concat_fragments by tag name" end + autorequire(:file) do + unless catalog.resource("Concat_file[#{self[:target]}]") + warning "Target Concat_file[#{self[:target]}] not found in the catalog" + end + end + validate do + # Check if target is set + fail Puppet::ParseError, "Target not set" if self[:target].nil? + + # Check if tag is set + fail Puppet::ParseError, "Tag not set" if self[:tag].nil? + # Check if either source or content is set. raise error if none is set fail Puppet::ParseError, "Set either 'source' or 'content'" if self[:source].nil? && self[:content].nil? diff --git a/manifests/fragment.pp b/manifests/fragment.pp index 9cfae6613..cc336435e 100644 --- a/manifests/fragment.pp +++ b/manifests/fragment.pp @@ -47,6 +47,7 @@ $safe_target_name = regsubst($target, '[/:\n\s]', '_', 'GM') concat_fragment { $name: + target => $target, tag => $safe_target_name, order => $order, content => $content, diff --git a/manifests/init.pp b/manifests/init.pp index 5bf53fc83..6e97a952e 100644 --- a/manifests/init.pp +++ b/manifests/init.pp @@ -107,6 +107,7 @@ if $_append_header { concat_fragment { "${name}_header": + target => $name, tag => $safe_name, content => $warn_message, order => '0', diff --git a/spec/acceptance/deprecation_warnings_spec.rb b/spec/acceptance/warnings_spec.rb similarity index 68% rename from spec/acceptance/deprecation_warnings_spec.rb rename to spec/acceptance/warnings_spec.rb index 8a689a76b..89b3c3c57 100644 --- a/spec/acceptance/deprecation_warnings_spec.rb +++ b/spec/acceptance/warnings_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper_acceptance' -describe 'deprecation warnings' do +describe 'warnings' do basedir = default.tmpdir('concat') shared_examples 'has_warning' do |pp, w| @@ -10,7 +10,7 @@ end end - context 'concat force parameter' do + context 'concat force parameter deprecation' do pp = <<-EOS concat { '#{basedir}/file': force => false, @@ -25,7 +25,7 @@ it_behaves_like 'has_warning', pp, w end - context 'concat::fragment ensure parameter' do + context 'concat::fragment ensure parameter deprecation' do context 'target file exists' do pp = <<-EOS concat { '#{basedir}/file': @@ -41,4 +41,21 @@ it_behaves_like 'has_warning', pp, w end end + + context 'concat::fragment target not found' do + context 'target not found' do + pp = <<-EOS + concat { 'file': + path => '#{basedir}/file', + } + concat::fragment { 'foo': + target => '#{basedir}/file', + content => 'bar', + } + EOS + w = 'not found in the catalog' + + it_behaves_like 'has_warning', pp, w + end + end end From 78fdb4e794e0858c058694a5db8790fb08fe853f Mon Sep 17 00:00:00 2001 From: Bryan Jen Date: Thu, 28 May 2015 16:51:35 -0700 Subject: [PATCH 3/6] adds file autorequire autorequire will add parent directories of the concat_file to the dependency chain. So they can be created in the right order if they do not exist. --- lib/puppet/type/concat_file.rb | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/lib/puppet/type/concat_file.rb b/lib/puppet/type/concat_file.rb index 9b3fde4dc..79776c5bd 100644 --- a/lib/puppet/type/concat_file.rb +++ b/lib/puppet/type/concat_file.rb @@ -89,6 +89,22 @@ def exists? end.compact end + # Copied from puppet's file type + # Autorequire the nearest ancestor directory found in the catalog. + autorequire(:file) do + req = [] + path = Pathname.new(self[:path]) + if !path.root? + # Start at our parent, to avoid autorequiring ourself + parents = path.parent.enum_for(:ascend) + if found = parents.find { |p| catalog.resource(:file, p.to_s) } + req << found.to_s + end + end + + req + end + def should_content return @generated_content if @generated_content @generated_content = "" From 64b9288ac77ccfb51701845e0375690373c4419c Mon Sep 17 00:00:00 2001 From: Bryan Jen Date: Fri, 29 May 2015 09:09:07 -0700 Subject: [PATCH 4/6] fix defaulted force behavior With Concat 2.0.x we've deprecated the -force parameter and the resulting bug caused concat files that have no content/fragments to overwrite the target file with an empty file. Concat should only create an empty file if no file exists, and if the file exists, then it should not overwrite it with an empty file. --- lib/puppet/type/concat_file.rb | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/puppet/type/concat_file.rb b/lib/puppet/type/concat_file.rb index 9b3fde4dc..0744fadd9 100644 --- a/lib/puppet/type/concat_file.rb +++ b/lib/puppet/type/concat_file.rb @@ -163,7 +163,11 @@ def generate end def eval_generate - catalog.resource("File[#{self[:path]}]")[:content] = should_content + content = should_content + + if !content.nil? and !content.empty? + catalog.resource("File[#{self[:path]}]")[:content] = content + end [] end end From 5267d3587c50e574cb96feda857f7f5b74e4e1ae Mon Sep 17 00:00:00 2001 From: Bryan Jen Date: Mon, 1 Jun 2015 15:30:04 -0700 Subject: [PATCH 5/6] 1.2.3 Release prep --- CHANGELOG.md | 9 +++++++++ metadata.json | 6 +++--- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 59204bb83..8ef82026d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,15 @@ This is a major release. Includes re-implementation of concat to use native Type ####Bugfixes - Fixes a bug in alpha ordering of fragments. +======= +##2015-06-02 - Supported Release 1.2.3 +###Summary + +This release includes a README fix to document correct behavior of fragment target parameter. + +####Bugfixes + +- README Fix to correctly document how a fragment $target param should work. ##2015-05-12 - Supported Release 1.2.2 ###Summary diff --git a/metadata.json b/metadata.json index 92fb8a182..a4a9e301b 100644 --- a/metadata.json +++ b/metadata.json @@ -7,9 +7,6 @@ "source": "https://github.com/puppetlabs/puppetlabs-concat", "project_page": "https://github.com/puppetlabs/puppetlabs-concat", "issues_url": "https://tickets.puppetlabs.com/browse/MODULES", - "dependencies": [ - {"name":"puppetlabs/stdlib","version_requirement":">= 4.5.0 < 5.0.0"} - ], "operatingsystem_support": [ { "operatingsystem": "RedHat", @@ -106,5 +103,8 @@ "name": "puppet", "version_requirement": "3.x" } + ], + "dependencies": [ + {"name":"puppetlabs/stdlib","version_requirement":">= 3.2.0 < 5.0.0"} ] } From d6ee5a8606d9a5161c6f4d1e9bd9dd833701d9c1 Mon Sep 17 00:00:00 2001 From: Bryan Jen Date: Mon, 1 Jun 2015 15:41:42 -0700 Subject: [PATCH 6/6] Release 2.0.1 prep --- CHANGELOG.md | 11 +++++++++++ metadata.json | 2 +- 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8ef82026d..87a0443c3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,14 @@ +======= +##2015-06-02 - Supported Release 2.0.1 +###Summary + +This is a bugfix release. + +####Bugfixes +- Fixes dependency graphing with concurrent modification of the same file. +- Fixes handling fragment target. +- Fixes the defaulted force behavior to handle empty concats correctly. + ======= ##2015-05-12 - Supported Release 2.0.0 ###Summary diff --git a/metadata.json b/metadata.json index a4a9e301b..31d9144d7 100644 --- a/metadata.json +++ b/metadata.json @@ -1,6 +1,6 @@ { "name": "puppetlabs-concat", - "version": "2.0.0", + "version": "2.0.1", "author": "Puppet Labs", "summary": "Construct files from multiple fragments.", "license": "Apache-2.0",