Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
sircubbi opened this issue Sep 11, 2024 · 3 comments
Open

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

sircubbi opened this issue Sep 11, 2024 · 3 comments
Labels
bug Something isn't working

Comments

@sircubbi
Copy link

Affected Puppet, Ruby, OS and module versions/distributions

  • Puppet: 8.9.0
  • Distribution: EL8/EL9
  • Module version: 11.1.0

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

use letsencrypt::renew without any additional parameters

What are you seeing

  • Notice: /Stage[main]/Letsencrypt::Renew/Cron[letsencrypt-renew]/environment: defined 'environment' as (corrective) on every run

What behaviour did you expect instead

  • no corrective changes should be reported

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 Dan33l added the bug Something isn't working label Sep 11, 2024
@Dan33l
Copy link
Member

Dan33l commented Sep 11, 2024

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 9eddbb1 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
Copy link
Member

Dan33l commented Sep 11, 2024

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
Copy link
Member

Dan33l commented Sep 13, 2024

After some tests and pry usage, modifying puppetlabs-cron_core it looks to be idempotent when environment => []
puppetlabs/puppetlabs-cron_core#77

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants