Issue #7680 has been updated by Chris Boot.
Adrien asked me to copy my comments from GitHub to here, so here goes: I have been investigating this issue this afternoon. I believe the tests in question are broken, and do not properly test what they are supposed to test. I have some fixes in [my own branch for this bug](https://github.com/bootc/puppet/compare/master...issue-7680). While doing this fix for the tests relating to this issue, I noticed two other malformed tests and fixed those too as a separate commit. This causes the `should overwrite the destination and apply executable bits` test to start failing because it was a bad test, so my patch marks this as pending Issue #10315. To be clear, the behviour that was being tested is in fact incorrect, but the test does not properly pick this up. After applying my fixes linked above, applying the patch in this pull request does indeed make `should create the file, not a symlink (#2817, #10315)` pass, but as mentioned above this causes `when setting permissions when setting mode for links when following links to a directory that is not readable should not set executable bits when overwriting the destination` to start failing. I'm not sure what the desired behaviour is in the situation that the test creates, but in my opinion either the test is not named correctly or is testing for the wrong thing. The test is called `should not set executable bits when overwriting the destination` but it looks at *all* of the mode bits, not just the executable bits. The current behaviour is to still change the permissions on the target of the File resource, so the starting 0644 is changed to 0666 and the test passes. With the patch in this pull request applied, the permissions on the target are unchanged and remain at 0644 and the test fails. I guess we need to know what the expected behaviour is of a File resource with `links => follow` whose target is an unreadable directory. Personally, I'd expect the behaviour to be the same as though the File resource pointed directly at the unreadable directory instead of the symlink, but it is not. To illustrate: Some preparation: <pre> crb@chris puppet $ mkdir source crb@chris puppet $ chmod 300 source crb@chris puppet $ ln -s source srclink </pre> Now, with the current git master head: <pre> crb@chris puppet $ bundle exec bin/puppet resource file $(pwd)/target links=follow source=$(pwd)/srclink mode=0666 Error: Could not rename temporary file /home/crb/temp/workspace/src/puppet/target.puppettmp_5058 to /home/crb/temp/workspace/src/puppet/target: File written to disk did not match checksum; discarding changes ( vs {md5}d41d8cd98f00b204e9800998ecf8427e) Error: /File[/home/crb/temp/workspace/src/puppet/target]/ensure: change from absent to present failed: Could not rename temporary file /home/crb/temp/workspace/src/puppet/target.puppettmp_5058 to /home/crb/temp/workspace/src/puppet/target: File written to disk did not match checksum; discarding changes ( vs {md5}d41d8cd98f00b204e9800998ecf8427e) file { '/home/crb/temp/workspace/src/puppet/target': ensure => 'absent', } crb@chris puppet $ touch target crb@chris puppet $ bundle exec bin/puppet resource file $(pwd)/target links=follow source=$(pwd)/srclink mode=0666 Notice: /File[/home/crb/temp/workspace/src/puppet/target]/mode: mode changed '0644' to '0666' file { '/home/crb/temp/workspace/src/puppet/target': ensure => 'file', group => '1000', mode => '666', } </pre> Note that the latter part of the above is effectively what the `should not set executable bits when overwriting the destination` test does. Note that the source is a directory, but the target is a file. This is not deleted and re-created, but its permissions are changed. If we instead use the same file resource but point at the unreadable directory directly: <pre> crb@chris puppet $ touch target crb@chris puppet $ bundle exec bin/puppet resource file $(pwd)/target source=$(pwd)/source mode=0666 Notice: /File[/home/crb/temp/workspace/src/puppet/target]/ensure: ensure changed 'file' to 'directory' file { '/home/crb/temp/workspace/src/puppet/target': ensure => 'directory', group => '1000', mode => '777', } crb@chris puppet $ rmdir target crb@chris puppet $ bundle exec bin/puppet resource file $(pwd)/target source=$(pwd)/source mode=0666 Notice: /File[/home/crb/temp/workspace/src/puppet/target]/ensure: created file { '/home/crb/temp/workspace/src/puppet/target': ensure => 'directory', group => '1000', mode => '777', } </pre> Now, with the patch in this pull request: <pre> crb@chris puppet $ bundle exec bin/puppet resource file $(pwd)/target links=follow source=$(pwd)/srclink mode=0666 Error: /File[/home/crb/temp/workspace/src/puppet/target]: Could not evaluate: Got nil value for content Error: Could not run: Got nil value for content crb@chris puppet $ bundle exec bin/puppet resource file $(pwd)/target links=follow source=$(pwd)/srclink mode=0666 Error: /File[/home/crb/temp/workspace/src/puppet/target]: Could not evaluate: Got nil value for content Error: Could not run: Got nil value for content </pre> That, for me, would be the expected behaviour - I expect the operation to fail outright and not touch the file. Are my assumptions / expectations incorrect? If so, please do point me in the right direction, as I really would like to get this issue fixed in the most correct way possible. Thanks, Chris ---------------------------------------- Bug #7680: Checksum missmatch when copying followed symlinks https://projects.puppetlabs.com/issues/7680#change-87049 Author: Mikael Svantesson Status: Code Insufficient Priority: Normal Assignee: Category: file Target version: Affected Puppet version: 2.6.8 Keywords: symlink file Branch: https://github.com/puppetlabs/puppet/pull/1476 When trying to copy a file (using a symlink), Puppet does not calculate the correct checksum for the temporary file. This work in 2.6.7, I have not tried 2.7.x. **Error:** err: /Stage[pre]/Sudo/File[/etc/sudoers]/ensure: change from absent to present failed: Could not rename temporary file /etc/sudoers.puppettmp_4293 to /etc/sudoers: File written to disk did not match checksum; discarding changes ( vs {md5}d41d8cd98f00b204e9800998ecf8427e) at /etc/puppet/modules/sudo/manifests/init.pp:12 at /etc/puppet/modules/sudo/manifests/init.pp:12 **Manifest:** class sudo { package { 'sudo': ensure => installed } file { '/etc/sudoers': ensure => file, source => ["puppet:///modules/sudo/sudoers-${hostname}", 'puppet:///modules/sudo/sudoers'], links => follow, replace => true, owner => 'root', group => 'root', mode => '0440', } } **File structure:** modules/sudo/files |-- sudoers |-- sudoers-host1 `-- sudoers-host2 -> sudoers-host1 -- You have received this notification because you have either subscribed to it, or are involved in it. To change your notification preferences, please click here: http://projects.puppetlabs.com/my/account -- You received this message because you are subscribed to the Google Groups "Puppet Bugs" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To post to this group, send email to [email protected]. Visit this group at http://groups.google.com/group/puppet-bugs?hl=en. For more options, visit https://groups.google.com/groups/opt_out.
