MODULES-1678 - Add show_diff attribute to concat and concat::fragment defined types

[jdkindy]

Add show_diff to concat and concat::fragment types.

[HelenCampbell]

Hi,
Thank you for your work with this so far, this seems like a sensible addition to this module. However you would need to have all unit tests passing for us to have a look at this properly.

[jdkindy]

@HelenCampbell - I fixed the tests. Please take a look to see if there is anything else I need to fix.

[jdkindy]

Found and removed a whitespace change.

[jdkindy]

@HelenCampbell Do I need to submit a new pull request?

[igalic]

is this not moduleynced?

[jdkindy]

I do not know what is meant by 'moduleynced'. I had to add that line in order to get the tests to pass on my local box.

[jdkindy]

Removed line locally and retested successfully, so removed line from the commit. Tests are successful.

[bmjen]

@jdkindy I think a show_diff param would be quite useful for concat, however I am not a fan of adding the corresponding param to the concat::fragment, since the fragment files themselves are supposed to be ingredients to the resulting file vs independently managed file resources. Also, when we do reintroduce concat as a native type, the utility of having a show_diff on fragments will become obsolete.

[jdkindy]

@bmjen The reason for adding the show_diff parameter to concat::fragment resources is that diffs of the file resources defined as part of concat::fragment are included in the reports. Leaving show_diff out of concat::fragment allows secrets to appear in reports. Puppet runs show diffs of both the resultant concat file resource as well as diffs of the concat::fragment file resources.

[Vincent--]

I'd like to have this feature as well (for the same "secret" reason ;-) )
Thanks @jdkindy for the patch !

[bmjen]

@jdkindy I think I understand the reticence towards show_diff and how it corresponds to the fragment files. However, I am still against the exposure of the show_diff param in the fragment resource, as fragments should be internal resources only. I think an appropriate middle ground would be to remove the show_diff param from the fragment.. and force the fragment file to show_diff=false all the time.

Thoughts?

[Vincent--]

I totally agree ! :-)

[jdkindy]

@bmjen I intended for the users to be able to choose fragments if they wished. However, if the plan is for concat to move to a native type and the concat::fragment type will not have a show_diff, I am ok with setting it to false for all concat::fragment resources.

[bmjen]

The alignment here needs to be fixed since show_diff was removed.

[bmjen]

Thanks for the update @jdkindy! A few more comments and we're set for merge!

[jdkindy]

@bmjen I think this commit takes care of all of the issues with the previous one.

[bmjen]

Awesome. Thanks for all the work @jdkindy !

[Vincent--]

Thks @jdkindy :-)