fragment can be local file that might not always exist

[wbean1]

In the 1.0 release, I could have a fragment (similar to the example in README)

  concat::fragment { "motd_local":
    target  => 'motd',
    order   => 90,
    ensure  => "/etc/motd.local",
  }

and if that file didn't exist locally, it didn't matter.

Keeping that syntax, I now get:

Error: Could not retrieve catalog from remote server: Error 400 on SERVER: validate_re(): "/etc/motd.local" does not match "^$|^present$|^absent$|^file$|^directory$" at /etc/puppet/modules/concat/manifests/fragment.pp:39 on node ____

If I try to use source, I get:

Error: /File[/var/lib/puppet/concat/motd/fragments/90_motd_local]: Could not evaluate: Could not retrieve information from environment devl source(s) file:/etc/motd.local

[jhoblitt]

This was an accidental regression caused by introducing parameter validation. #111 fixed the README but there's some blog posts that refer to ensure => path syntax. I searched github a few weeks ago and couldn't find examples of that syntax so I've been trying to decide if we should revert that validation and add a warning or leave it as is (so as not to thrash on the semantics since it's been like this for a month). @apenney Any thoughts on that?

I will add some tests for for the failure that's being described here.

[ripienaar]

fwiw this is a deliberate feature, not documented but def something thats wanted.

[jhoblitt]

@wbean1 Could you post the exact snippet that's failing for you with the source parameter? There is good system test coverage of that functionality already: https://github.com/puppetlabs/puppetlabs-concat/blob/master/spec/system/fragment_list_source_spec.rb

@ripienaar what's the history on how ensure => path and source => path came to be supported? The behind the scenes workings are different but the semantics should be the same. ensure => path is now frowned upon with the file type and it would be nice to not have to support this when the eventual transition to a type/provider is made.

[ripienaar]

perhaps by accident? this module comes from 0.24.x days, lots of behaviour was left up to discovery rather than specification.

What I mean with the feature being needed though is that its highly desirable to be able to pull in fragments on the local file system that are not managed by puppet which this enables.

[jhoblitt]

@wbean1 Any information you could provide on reproducing the failure you saw with the source parameter would be helpful.

[jhoblitt]

I was working on a PR to change the validation into a warning and while writing system tests for the warning I uncovered a nasty behaviour. It appears that there is an issue with using the ensure/symlink syntax and then trying to change the fragment back to a plain file. What happens is a symlink is left behind in the fragment dir that's not over written when the file resource for the fragment has ensure => present set.

Warning: /Stage[main]//Concat::Fragment[foo]/File[/var/lib/puppet/concat/_tmp_concat_file/fragments/10_foo]: Ensure set to :present but file type is link so no content will be synced

[wbean1]

the feature being needed though is that its highly desirable to be able to pull in fragments on the local file system that are not managed by puppet which this enables

This is exactly the need. I understand why local changes are not in the 'spirit' of puppet, but this was example usage...

  concat::fragment { "motd_local":
    target  => 'motd',
    order   => 90,
    ensure  => "/etc/motd.local",
  }

generates the validate_re() exception previously posted

 concat::fragment { "motd_local":
    target  => 'motd',
    order   => 90,
    source  => "/etc/motd.local",
  }

generates (when "/etc/motd.local" does not exist on the local system)

Error: /Stage[main]/Motd/Concat::Fragment[motd_local]/File[/var/lib/puppet/concat/motd/fragments/90_motd_local]: Could not evaluate: Could not retrieve information from environment lab source(s) file:/etc/motd.local

[jhoblitt]

The intent is to allow the ensure => /target syntax again but with a deprecation warning. I'm still playing with fixes for the symlink replacement bug but my preferred solution would be not to allow non-existent files.

I will note that the behaviour your asking for is very un-puppet like. Generally, the way to do something like that would be to create a fact for the existence of /etc/motd.local and branching on it.

[jhoblitt]

@wbean1 #117 has been merged and should allow the semantics your expecting but with a deprecation warning. Thank you for reporting the regression!