search

voxpupuli / puppet-letsencrypt

A Puppet module to install the Letsencrypt client and request certificates.
https://forge.puppet.com/puppet/letsencrypt
Apache License 2.0
87 stars 136 forks source link

Corrective changes because of new environment-parameter in renew #360

Open sircubbi opened 2 months ago

sircubbi commented 2 months ago

Affected Puppet, Ruby, OS and module versions/distributions

How to reproduce (e.g Puppet code you use)

use letsencrypt::renew without any additional parameters

What are you seeing

What behaviour did you expect instead

Any additional information you'd like to impart

The change was introducted by #288. When looking at the discussion the new parameter was firstly defaulted to be undef, but later changed to an empty array (for removing any already existing environment). Unfortunately this produces an uncorrective change if no environment is used at all (could be that the cron-ressource is the root-problem).

I would really like to set the new parameter defaulting to undef and restore the previous behaviour.

Dan33l commented 2 months ago

I have some CI using this module with real certificats requested to LE staging CA. Now this CI does not run idempotently. So, i confirm that https://github.com/voxpupuli/puppet-letsencrypt/commit/9eddbb1d60ed9bc4938e8656ea8f32466b8eec25 introduce unexpected behavior.

Trace :

  Info: Using environment 'production'
  Info: Applying configuration version '1726063684'
  Notice: /Stage[main]/Letsencrypt::Renew/Cron[letsencrypt-renew]/environment: defined 'environment' as 
  Notice: Applied catalog in 8.17 seconds
Dan33l commented 2 months ago

The core module puppetlabs-cron_core have a provider that accept value absent for property environment https://github.com/puppetlabs/puppetlabs-cron_core/blob/main/lib/puppet/type/cron.rb#L325.

Perhaps a better default value is absent, instead of empty array.

Edit : When environment is empty (like with []), the provider prefetch :absent: https://github.com/puppetlabs/puppetlabs-cron_core/blob/main/lib/puppet/provider/cron/crontab.rb#L224

So using empty array or absent is equal.

Dan33l commented 2 months ago

After some tests and pry usage, modifying puppetlabs-cron_core it looks to be idempotent when environment => [] https://github.com/puppetlabs/puppetlabs-cron_core/pull/77