Add cron day of month parameter #146
Conversation
manifests/certonly.pp
Outdated
| @@ -44,6 +44,7 @@ | |||
| Boolean $suppress_cron_output = false, | |||
| Optional[String[1]] $cron_before_command = undef, | |||
| Optional[String[1]] $cron_success_command = undef, | |||
| Optional[Array] $cron_monthday = ['*'], | |||
There was a problem hiding this comment.
Since you provide a default, this can just be Array. Should it also be Array[String[1]]?
There was a problem hiding this comment.
Sure, we could omit the Optional and have just an Array instead. If we use Array[String[1]] then one needs to specify the days in the cron_monthday parameter as strings like ['1', '15']. [1, 15] will not work because puppet will complain with expects a String value, got Integer. What do you think is better? Having Array or Array[String[1]] ?
There was a problem hiding this comment.
I'm a fan of strict data types so perhaps Array[Variant[Integer, String[1]]]
There was a problem hiding this comment.
I couldn't agree more with you. Shall I submit a new commit to this PR including this change?
manifests/certonly.pp
Outdated
| Boolean $suppress_cron_output = false, | ||
| Optional[String[1]] $cron_before_command = undef, | ||
| Optional[String[1]] $cron_success_command = undef, | ||
| Array[Variant[Integer, String[1]]] $cron_monthday = ['*'], |
There was a problem hiding this comment.
Is there a known minimal/maximal value for the integer? That can also be enforced with:
| Array[Variant[Integer, String[1]]] $cron_monthday = ['*'], | |
| Array[Variant[Integer[0, 60], String[1]]] $cron_monthday = ['*'], |
See also: https://puppet.com/docs/puppet/5.4/lang_data_number.html#the-integer-data-type
There was a problem hiding this comment.
Well spotted! Based on the man page of crontab the integer value can be between 0-59. I will commit the change to this branch in a few minutes.
There was a problem hiding this comment.
thanks. I will merge it after it turned green.
bastelfreak
added
enhancement
|
@brigriffin Thank you. |
|
Cheers! |
Pull Request (PR) description
This PR adds support for specifying one or many day(s) of the month when to run the certbot cron. The default is to run everyday (
*in cron syntax) and a user can now specify one or multiple days of the month when to run the certbot cron. For example some people prefer to run it twice per month instead of everyday. This is now possible with this PR.This Pull Request (PR) fixes the following issues
n/a