fix idempotency for empty environment

[Dan33l]

An idempotent issue is present with puppetlabs-cron_core when environment => [] .
We use this into puppet-letsencrypt :
voxpupuli/puppet-letsencrypt#360
https://github.com/voxpupuli/puppet-letsencrypt/blob/master/manifests/renew.pp#L83

environment => [] is used at least in one exemple https://github.com/puppetlabs/puppetlabs-cron_core/blob/main/spec/integration/provider/cron/crontab_spec.rb#L202So it looks expected as a possible usage. I did not found a tests that check idempotency about this usage.

This PR intent a fix of this

[CLAassistant]

All committers have signed the CLA.

[redat00]

Just had the same issue on my personal environment, with the letsencrypt module showing corrective change every time it runs. Your PR fixes the issue.

As a temporary fix, I'll just clone this module and make the fix manually, hopefully it gets merged soon. 🙏

Thanks for your help!

[mhashizume]

Thank you for the PR @Dan33l , we have added it to our team's backlog to review.

[joshcooper]

@Dan33l could you update the tests? It looks like they're failing because they are expecting :absent, but are receiving an empty array:

  -  :environment=>:absent,
  +  :environment=>[],

[mhashizume]

Hi @Dan33l , thank you again for your PR.

As @joshcooper mentioned, the following should get tests passing and get this unblocked:

diff --git a/spec/unit/provider/cron/parsed_spec.rb b/spec/unit/provider/cron/parsed_spec.rb
index 2b95e93..0214213 100644
--- a/spec/unit/provider/cron/parsed_spec.rb
+++ b/spec/unit/provider/cron/parsed_spec.rb
@@ -223,7 +223,7 @@ describe Puppet::Type.type(:cron).provider(:crontab) do
         expect(parameters[0][:special]).to eq(:absent)
         expect(parameters[0][:command]).to match(%r{\$HOME/bin/daily.job >> \$HOME/tmp/out 2>&1})
         expect(parameters[0][:ensure]).to eq(:present)
-        expect(parameters[0][:environment]).to eq(:absent)
+        expect(parameters[0][:environment]).to eq([])
         expect(parameters[0][:user]).to eq(:absent)

         expect(parameters[1][:name]).to match(%r{unmanaged:\$HOME/bin/monthly-\d+})
@@ -235,7 +235,7 @@ describe Puppet::Type.type(:cron).provider(:crontab) do
         expect(parameters[1][:special]).to eq(:absent)
         expect(parameters[1][:command]).to match(%r{\$HOME/bin/monthly})
         expect(parameters[1][:ensure]).to eq(:present)
-        expect(parameters[1][:environment]).to eq(:absent)
+        expect(parameters[1][:environment]).to eq([])
         expect(parameters[1][:user]).to eq(:absent)
         expect(parameters[1][:target]).to eq('foobar')
       end
@@ -259,7 +259,7 @@ describe Puppet::Type.type(:cron).provider(:crontab) do
                        special: :absent,
                        command: '/bin/true',
                        ensure: :present,
-                       environment: :absent,
+                       environment: [],
                        user: :absent,
                        target: 'foobar',
                      },

Would you be able to update the test and sign the CLA?

Thank you!

[mhashizume]

Hi @Dan33l , would you be able to address the concerns I raised in my last comment? Thank you

[lbdemv]

@mhashizume would you be willing to merge a separate PR (that i would create) to fix this issues?
We would like to have this fixed and since @Dan33l seems to be unresponsive to the concerns raised about the failing tests and CLA i would redo his fix and add the necessary changes.

But i don't want do impose myself, if this is not you way to handle this type of issue?

[flokoe]

Any updates on this? We really would appreciate it if this gets fixed one way or another.

[lbdemv]

Any updates @Dan33l / @mhashizume on the test fixes and the CLA? Or my offer to redo it (including CLA/tests?)

[Dan33l]

@joshcooper PR looks now updated accordingly with your comment.

[Dan33l]

@mhashizume PR looks now updated accordingly with your comment.