Package: release.debian.org
Severity: normal
User: release.debian....@packages.debian.org
Usertags: unblock

Please unblock package puppet

A bug (http://bugs.debian.org/698294)  prevents the use of symbolic links at
the source when using puppet to distribute files; they fail with a checksum
error.  This is a regression from puppet in stable.  Puppet Labs recently
included the fix for this bug into their 2.7.x maintenance branch.

A test with puppet 2.7.18-3 (with the bug) and 2.7.18-4 (bug fixed) shows the
following:

,----[ setup ]
| # echo testing > testfile
| # ln -s testfile testlink
| # md5sum testfile 
| eb1a3227cdc3fedbaec2fe38bf6c044a  testfile
`----

,----[ test with 2.7.18-3 ]
| # dpkg -i 
/home/ssm/Debian/pbuilder/unstable_result/{puppet-common,puppetmaster-common,puppetmaster}_2.7.18-3_all.deb
| # puppet resource file $(pwd)/newfile ensure=file links=follow 
source=$(pwd)/testlink mode=0644
| err: /File[/root/newfile]/ensure: change from absent to present
| failed: Could not rename temporary file /root/newfile.puppettmp_1534
| to /root/newfile: File written to disk did not match checksum;
| discarding changes ( vs {md5}d41d8cd98f00b204e9800998ecf8427e)
| file { '/root/newfile':
|   ensure => 'absent',
| }
`----

,----[ test with 2.7.18-4 ]
| # dpkg -i 
/home/ssm/Debian/pbuilder/unstable_result/{puppet-common,puppetmaster-common,puppetmaster}_2.7.18-4_all.deb
| # puppet resource file $(pwd)/newfile ensure=file links=follow 
source=$(pwd)/testlink mode=0644
| notice: /File[/root/newfile]/ensure: defined content as 
'{md5}eb1a3227cdc3fedbaec2fe38bf6c044a'
| file { '/root/newfile':
|   ensure  => 'file',
|   content => '{md5}eb1a3227cdc3fedbaec2fe38bf6c044a',
|   group   => '0',
|   mode    => '644',
|   owner   => '0',
| }
`----

The list of patches are taken from
https://github.com/puppetlabs/puppet/pull/1532, minus one patch for a newer
puppet version, and area added to the packaging as follows (debdiff also
attached):

* fix-symlink-1-ee4c6f7c697737aa919b9f90436ab0cc69934b03
  fixes broken tests, which prevented the fix from being accepted.

* fix-symlink-2-1b0e812ad9e33b3cc148fac30a28490f60f40c63
  the actual bugfix. Two lines changed. :)

* fix-symlink-3-3a00ed468617c17b5a527c68cfc37d7d1fddaa72
  updates tests to reflect new behaviour

* fix-symlink-4-1d8a76e060f610a9db20cf1bdd4ff95dddba9309
  add acceptance test

Since we also package the tests as "puppet-testsuite", the patches updating the
tests are included.

unblock puppet/2.7.18-4

-- System Information:
Debian Release: 7.0
  APT prefers testing
  APT policy: (900, 'testing'), (800, 'unstable'), (1, 'experimental')
Architecture: amd64 (x86_64)

Kernel: Linux 3.2.0-4-amd64 (SMP w/4 CPU cores)
Locale: LANG=en_US.UTF-8, LC_CTYPE=en_US.UTF-8 (charmap=UTF-8)
Shell: /bin/sh linked to /bin/dash
diff -Nru puppet-2.7.18/debian/changelog puppet-2.7.18/debian/changelog
--- puppet-2.7.18/debian/changelog	2013-03-13 15:46:03.000000000 +0100
+++ puppet-2.7.18/debian/changelog	2013-03-15 21:43:41.000000000 +0100
@@ -1,3 +1,10 @@
+puppet (2.7.18-4) unstable; urgency=low
+
+  * Import upstream patch to fix puppet's handling of symbolic links 
+    (Closes: #698294)
+
+ -- Stig Sandbeck Mathisen <s...@debian.org>  Fri, 15 Mar 2013 20:32:40 +0100
+
 puppet (2.7.18-3) unstable; urgency=high
 
   * Add patch to fix puppet vulnerabilities (CVE-2013-1640, CVE-2013-1652,
diff -Nru puppet-2.7.18/debian/patches/fix-symlink-1-ee4c6f7c697737aa919b9f90436ab0cc69934b03 puppet-2.7.18/debian/patches/fix-symlink-1-ee4c6f7c697737aa919b9f90436ab0cc69934b03
--- puppet-2.7.18/debian/patches/fix-symlink-1-ee4c6f7c697737aa919b9f90436ab0cc69934b03	1970-01-01 01:00:00.000000000 +0100
+++ puppet-2.7.18/debian/patches/fix-symlink-1-ee4c6f7c697737aa919b9f90436ab0cc69934b03	2013-03-15 21:43:41.000000000 +0100
@@ -0,0 +1,128 @@
+commit ee4c6f7c697737aa919b9f90436ab0cc69934b03
+Author: Chris Boot <c...@tiger-computing.co.uk>
+Date:   Mon Mar 11 15:30:14 2013 +0000
+
+    (#7680) Update tests for changed behaviour after bugfix
+    
+    A number of tests were broken before the bugfix:
+    * incorrectly checking the file mode (missing .should)
+    * operating on a non-existent source symlink
+    
+    Enabled all of the #10315 tests which now pass, some of which needed
+    editing due to the expected behaviour now being different for
+    links => follow.
+
+--- a/spec/integration/type/file_spec.rb
++++ b/spec/integration/type/file_spec.rb
+@@ -234,22 +234,20 @@
+ 
+             describe "that is readable" do
+               it "should set the executable bits when creating the destination (#10315)" do
+-                pending "bug #10315"
+-
+                 catalog.add_resource described_class.new(:path => path, :source => link, :mode => 0666, :links => :follow)
+                 catalog.apply
+ 
++                File.should be_directory(path)
+                 (get_mode(path) & 07777).should == 0777
+               end
+ 
+               it "should set the executable bits when overwriting the destination (#10315)" do
+-                pending "bug #10315"
+-
+                 FileUtils.touch(path)
+ 
+-                catalog.add_resource described_class.new(:path => path, :source => link, :mode => 0666, :links => :follow)
++                catalog.add_resource described_class.new(:path => path, :source => link, :mode => 0666, :links => :follow, :backup => false)
+                 catalog.apply
+ 
++                File.should be_directory(path)
+                 (get_mode(path) & 07777).should == 0777
+               end
+             end
+@@ -264,37 +262,41 @@
+                 set_mode(0700, target)
+               end
+ 
+-              it "should not set executable bits when creating the destination (#10315)" do
+-                pending "bug #10315"
+-
++              it "should set executable bits when creating the destination (#10315)" do
+                 catalog.add_resource described_class.new(:path => path, :source => link, :mode => 0666, :links => :follow)
+                 catalog.apply
+ 
+-                (get_mode(path) & 07777).should == 0666
++                File.should be_directory(path)
++                (get_mode(path) & 07777).should == 0777
+               end
+ 
+-              it "should not set executable bits when overwriting the destination" do
++              it "should set executable bits when overwriting the destination" do
+                 FileUtils.touch(path)
+ 
+-                catalog.add_resource described_class.new(:path => path, :source => link, :mode => 0666, :links => :follow)
++                catalog.add_resource described_class.new(:path => path, :source => link, :mode => 0666, :links => :follow, :backup => false)
+                 catalog.apply
+ 
+-                (get_mode(path) & 07777).should == 0666
++                File.should be_directory(path)
++                (get_mode(path) & 07777).should == 0777
+               end
+             end
+           end
+ 
+           describe "to a file" do
+-            let(:target) { tmpfile('file_target') }
++            let(:link_target) { tmpfile('file_target') }
+ 
+-            it "should create the file, not a symlink (#2817, #10315)" do
+-              pending "bug #2817, #10315"
++            before :each do
++              FileUtils.touch(link_target)
+ 
++              File.symlink(link_target, link)
++            end
++
++            it "should create the file, not a symlink (#2817, #10315)" do
+               catalog.add_resource described_class.new(:path => path, :source => link, :mode => 0600, :links => :follow)
+               catalog.apply
+ 
+               File.should be_file(path)
+-              (get_mode(path) & 07777) == 0600
++              (get_mode(path) & 07777).should == 0600
+             end
+ 
+             it "should overwrite the file" do
+@@ -304,7 +306,7 @@
+               catalog.apply
+ 
+               File.should be_file(path)
+-              (get_mode(path) & 07777) == 0600
++              (get_mode(path) & 07777).should == 0600
+             end
+           end
+ 
+@@ -326,13 +328,11 @@
+ 
+             describe "when following all links" do
+               it "should create the destination and apply executable bits (#10315)" do
+-                pending "bug #10315"
+-
+                 catalog.add_resource described_class.new(:path => path, :source => link, :mode => 0600, :links => :follow)
+                 catalog.apply
+ 
+                 File.should be_directory(path)
+-                (get_mode(path) & 07777) == 0777
++                (get_mode(path) & 07777).should == 0700
+               end
+ 
+               it "should overwrite the destination and apply executable bits" do
+@@ -342,7 +342,7 @@
+                 catalog.apply
+ 
+                 File.should be_directory(path)
+-                (get_mode(path) & 07777) == 0777
++                (get_mode(path) & 0111).should == 0100
+               end
+             end
+           end
diff -Nru puppet-2.7.18/debian/patches/fix-symlink-2-1b0e812ad9e33b3cc148fac30a28490f60f40c63 puppet-2.7.18/debian/patches/fix-symlink-2-1b0e812ad9e33b3cc148fac30a28490f60f40c63
--- puppet-2.7.18/debian/patches/fix-symlink-2-1b0e812ad9e33b3cc148fac30a28490f60f40c63	1970-01-01 01:00:00.000000000 +0100
+++ puppet-2.7.18/debian/patches/fix-symlink-2-1b0e812ad9e33b3cc148fac30a28490f60f40c63	2013-03-15 21:43:41.000000000 +0100
@@ -0,0 +1,31 @@
+commit 1b0e812ad9e33b3cc148fac30a28490f60f40c63
+Author: Chris Boot <c...@tiger-computing.co.uk>
+Date:   Thu Mar 14 17:56:39 2013 +0000
+
+    Send the :links option to the file server
+    
+    Previously, the server was never asked to follow links when 'links =>
+    follow' was used, so the server always provided the metadata for the
+    link itself. With this patch, the server correctly follows the link
+    server-side and the client can apply the catalog correctly.
+
+--- a/lib/puppet/type/file/source.rb
++++ b/lib/puppet/type/file/source.rb
+@@ -101,7 +101,7 @@
+       return @content if @content
+       raise Puppet::DevError, "No source for content was stored with the metadata" unless metadata.source
+ 
+-      unless tmp = Puppet::FileServing::Content.indirection.find(metadata.source)
++      unless tmp = Puppet::FileServing::Content.indirection.find(metadata.source, :links => resource[:links])
+         fail "Could not find any content at %s" % metadata.source
+       end
+       @content = tmp.content
+@@ -154,7 +154,7 @@
+       return nil unless value
+       value.each do |source|
+         begin
+-          if data = Puppet::FileServing::Metadata.indirection.find(source)
++          if data = Puppet::FileServing::Metadata.indirection.find(source, :links => resource[:links])
+             @metadata = data
+             @metadata.source = source
+             break
diff -Nru puppet-2.7.18/debian/patches/fix-symlink-3-3a00ed468617c17b5a527c68cfc37d7d1fddaa72 puppet-2.7.18/debian/patches/fix-symlink-3-3a00ed468617c17b5a527c68cfc37d7d1fddaa72
--- puppet-2.7.18/debian/patches/fix-symlink-3-3a00ed468617c17b5a527c68cfc37d7d1fddaa72	1970-01-01 01:00:00.000000000 +0100
+++ puppet-2.7.18/debian/patches/fix-symlink-3-3a00ed468617c17b5a527c68cfc37d7d1fddaa72	2013-03-15 21:43:41.000000000 +0100
@@ -0,0 +1,73 @@
+commit 3a00ed468617c17b5a527c68cfc37d7d1fddaa72
+Author: Chris Boot <c...@tiger-computing.co.uk>
+Date:   Thu Mar 14 19:05:50 2013 +0000
+
+    Update Puppet::FileServing::Metadata tests
+    
+    Provide a stub for resource[:links] => :manage
+    Expect :find to be called with :links => :manage
+
+--- a/spec/unit/type/file/source_spec.rb
++++ b/spec/unit/type/file/source_spec.rb
+@@ -92,6 +92,7 @@
+   describe "when returning the metadata" do
+     before do
+       @metadata = stub 'metadata', :source= => nil
++      @resource.stubs(:[]).with(:links).returns :manage
+     end
+ 
+     it "should return already-available metadata" do
+@@ -107,22 +108,22 @@
+ 
+     it "should collect its metadata using the Metadata class if it is not already set" do
+       @source = source.new(:resource => @resource, :value => @foobar)
+-      Puppet::FileServing::Metadata.indirection.expects(:find).with(@foobar_uri).returns @metadata
++      Puppet::FileServing::Metadata.indirection.expects(:find).with(@foobar_uri, :links => :manage).returns @metadata
+       @source.metadata
+     end
+ 
+     it "should use the metadata from the first found source" do
+       metadata = stub 'metadata', :source= => nil
+       @source = source.new(:resource => @resource, :value => [@foobar, @feebooz])
+-      Puppet::FileServing::Metadata.indirection.expects(:find).with(@foobar_uri).returns nil
+-      Puppet::FileServing::Metadata.indirection.expects(:find).with(@feebooz_uri).returns metadata
++      Puppet::FileServing::Metadata.indirection.expects(:find).with(@foobar_uri, :links => :manage).returns nil
++      Puppet::FileServing::Metadata.indirection.expects(:find).with(@feebooz_uri, :links => :manage).returns metadata
+       @source.metadata.should equal(metadata)
+     end
+ 
+     it "should store the found source as the metadata's source" do
+       metadata = mock 'metadata'
+       @source = source.new(:resource => @resource, :value => @foobar)
+-      Puppet::FileServing::Metadata.indirection.expects(:find).with(@foobar_uri).returns metadata
++      Puppet::FileServing::Metadata.indirection.expects(:find).with(@foobar_uri, :links => :manage).returns metadata
+ 
+       metadata.expects(:source=).with(@foobar_uri)
+       @source.metadata
+@@ -130,7 +131,7 @@
+ 
+     it "should fail intelligently if an exception is encountered while querying for metadata" do
+       @source = source.new(:resource => @resource, :value => @foobar)
+-      Puppet::FileServing::Metadata.indirection.expects(:find).with(@foobar_uri).raises RuntimeError
++      Puppet::FileServing::Metadata.indirection.expects(:find).with(@foobar_uri, :links => :manage).raises RuntimeError
+ 
+       @source.expects(:fail).raises ArgumentError
+       lambda { @source.metadata }.should raise_error(ArgumentError)
+@@ -138,7 +139,7 @@
+ 
+     it "should fail if no specified sources can be found" do
+       @source = source.new(:resource => @resource, :value => @foobar)
+-      Puppet::FileServing::Metadata.indirection.expects(:find).with(@foobar_uri).returns nil
++      Puppet::FileServing::Metadata.indirection.expects(:find).with(@foobar_uri, :links => :manage).returns nil
+ 
+       @source.expects(:fail).raises RuntimeError
+ 
+@@ -319,7 +320,7 @@
+       before(:each) do
+         metadata = Puppet::FileServing::Metadata.new(path, :source => uri, 'type' => 'file')
+         #metadata = stub('remote', :ftype => "file", :source => uri)
+-        Puppet::FileServing::Metadata.indirection.stubs(:find).with(uri).returns metadata
++        Puppet::FileServing::Metadata.indirection.stubs(:find).with(uri, :links => :manage).returns metadata
+         resource[:source] = uri
+       end
+ 
diff -Nru puppet-2.7.18/debian/patches/fix-symlink-4-1d8a76e060f610a9db20cf1bdd4ff95dddba9309 puppet-2.7.18/debian/patches/fix-symlink-4-1d8a76e060f610a9db20cf1bdd4ff95dddba9309
--- puppet-2.7.18/debian/patches/fix-symlink-4-1d8a76e060f610a9db20cf1bdd4ff95dddba9309	1970-01-01 01:00:00.000000000 +0100
+++ puppet-2.7.18/debian/patches/fix-symlink-4-1d8a76e060f610a9db20cf1bdd4ff95dddba9309	2013-03-15 21:43:41.000000000 +0100
@@ -0,0 +1,42 @@
+commit 1d8a76e060f610a9db20cf1bdd4ff95dddba9309
+Author: Adrien Thebo <g...@somethingsinistral.net>
+Date:   Thu Mar 14 13:42:22 2013 -0700
+
+    (#7680) Add acceptance test for file links => follow
+
+--- /dev/null
++++ b/acceptance/tests/resource/file/ticket_7680-follow-symlinks.rb
+@@ -0,0 +1,33 @@
++test_name "#7680: 'links => follow' should use the file source content"
++confine :except, :platform => 'windows'
++
++agents.each do |agent|
++
++  step "Create file content"
++  real_source = agent.tmpfile('follow_links_source')
++  dest        = agent.tmpfile('follow_links_dest')
++  symlink     = agent.tmpfile('follow_links_symlink')
++
++  on agent, "echo 'This is the real content' > #{real_source}"
++  on agent, "ln -sf #{real_source} #{symlink}"
++
++  manifest = <<-MANIFEST
++    file { '#{dest}':
++      ensure => file,
++      source => '#{symlink}',
++      links  => follow,
++    }
++  MANIFEST
++  apply_manifest_on(agent, manifest, :trace => true)
++
++  on agent, "cat #{dest}" do
++    assert_match /This is the real content/, stdout
++  end
++
++  step "Cleanup"
++  [real_source, dest, symlink].each do |file|
++    on agent, "rm -f '#{file}'"
++  end
++end
++
++
diff -Nru puppet-2.7.18/debian/patches/series puppet-2.7.18/debian/patches/series
--- puppet-2.7.18/debian/patches/series	2013-03-13 15:46:03.000000000 +0100
+++ puppet-2.7.18/debian/patches/series	2013-03-15 21:43:41.000000000 +0100
@@ -1,4 +1,8 @@
 2.7.18-CVE-Rollup.patch
 2.7.x-unit-test-fix.patch
+fix-symlink-1-ee4c6f7c697737aa919b9f90436ab0cc69934b03
+fix-symlink-2-1b0e812ad9e33b3cc148fac30a28490f60f40c63
+fix-symlink-3-3a00ed468617c17b5a527c68cfc37d7d1fddaa72
+fix-symlink-4-1d8a76e060f610a9db20cf1bdd4ff95dddba9309
 apache2-passenger-template
 fix_logcheck

Reply via email to