search

macadmins / puppet-sal_client

Configuration of a Sal client using Puppet
Apache License 2.0
2 stars 5 forks source link

Add option to disable passing plist as binary to avoid PSON render #16

Open gavinelder opened 1 year ago

gavinelder commented 1 year ago

Hey 👋 ,

I found that the binary argument in file_source => plist($profile, binary) seems unnecessary. Including it causes the Puppet-generated catalog to contain binary info, which can make the JSON compile fail and fall back to PSON which is now deprecated.

I've tested it without the binary argument, and everything seems to work fine, Looking at puppet-munki which does not use binary there is no reported issues with using this method. https://github.com/airbnb/puppet-munki/blob/92598857d676a4d48e05501b23a6a9664c5b7c2e/manifests/config.pp#L112-L116

However, I might be missing some context for why it's there in the first place. If anyone knows the reason or believes this will cause an issues, please let me know.

grahamgilbert commented 1 year ago

It is there because big log entries cause older versions of puppet to crash. If we are making this change, we need to test back to older versions and document the oldest supported version.

gavinelder commented 1 year ago

Thanks @grahamgilbert for the context , I have now changed this to an optional value to enable or disable the passing of the content as binary or not.

The default remains true as to keep behaviour for existing users and to not break backwards compatibility removing the need to test against older versions & to not change the behaviours expected from this puppet module.