(PUP-9001) Show diff for new files

[lollipopman]

Prior to this commit when :show_diff is set, file diffs are only shown
when modifying an existing file. However, it is often very helpful to
see the diff for a new file as well, either in testing or production
logs.

After this commit diffs are shown for new files as well. Diffs are
performed against NUL(windows) or (/dev/null)(all others) to ensure
compatibility with custom diff commands.

[puppetlabs-jenkins]

Can one of the admins verify this patch?

[lollipopman]

@joshcooper any chance you could review?

[lollipopman]

@joshcooper friendly bump for a review

[binford2k]

I am hesitant about this. Enabling show_diffs with this patch would result in a massive massive massive first run report. And the chances that sensitive information shows up in logs becomes exponentially higher. To be sure, one could alleviate both of those by disabling show_diffs for that run, but it would still be unexpected, will likely break some acceptance testing, and could even overload report storage destinations.

That said, I do see value if that's intentionally what you want. I wonder about changing it to a ternary instead. false, true, and verbose where the added level would give you file creation diffs, but without changing existing behaviors.

[lollipopman]

I am hesitant about this. Enabling show_diffs with this patch would result in a massive massive massive first run report. And the chances that sensitive information shows up in logs becomes exponentially higher. To be sure, one could alleviate both of those by disabling show_diffs for that run, but it would still be unexpected, will likely break some acceptance testing, and could even overload report storage destinations.

Definitely important concerns, thanks for raising them. I traditionally have not used a puppet server, so large reports are not something I have had to deal with.

That said, I do see value if that's intentionally what you want. I wonder about changing it to a ternary instead. false, true, and verbose where the added level would give you file creation diffs, but without changing existing behaviors.

If a ternary is necessary to prevent surprising behavior, I can certainly update the pull request, how about these values:

show_diff:

1. false: No diff (default)
2. true: Diff on changes
3. always: Diff on changes and new files

[CLAassistant]

All committers have signed the CLA.

[kenyon]

This is definitely something I have wanted.