From 20ff7c16022793c707f6c2b8fb38a801870bc0e2 Mon Sep 17 00:00:00 2001 From: Rob Crittenden Date: Wed, 8 Feb 2023 10:42:58 -0500 Subject: [PATCH] Fix setting values of 0 in ACME pruning Replace comparisons of "if value" with "if value is not None" in order to handle 0. Add a short reference to the man page to indicat that a cert or request retention time of 0 means remove at the next execution. Also indicate that the search time limit is in seconds. Fixes: https://pagure.io/freeipa/issue/9325 Signed-off-by: Rob Crittenden Reviewed-By: Francisco Trivino --- doc/designs/expired_certificate_pruning.md | 4 ++-- install/tools/man/ipa-acme-manage.1 | 8 +++---- ipaserver/install/ipa_acme_manage.py | 28 +++++++++++----------- 3 files changed, 20 insertions(+), 20 deletions(-) diff --git a/doc/designs/expired_certificate_pruning.md b/doc/designs/expired_certificate_pruning.md index a23e452696ba2a150c4ad5a3e57360ae0a16a338..35ead7b00145b5df44caf542cba277f0e6e08b6a 100644 --- a/doc/designs/expired_certificate_pruning.md +++ b/doc/designs/expired_certificate_pruning.md @@ -67,11 +67,11 @@ There are four values each that can be managed for pruning certificates and requ * expired cert/incomplete request time * time unit * LDAP search size limit -* LDAP search time limit +* LDAP search time limit (in seconds) The first two configure when an expired certificate or incomplete request will be deleted. The unit can be one of: minute, hour, day, year. By default it is 30 days. -The LDAP limits control how many entries are returned and how long the search can take. By default it is 1000 entries and unlimited time. +The LDAP limits control how many entries are returned and how long the search can take. By default it is 1000 entries and unlimited time (0 == unlimited, unit is seconds). ### Configuration settings diff --git a/install/tools/man/ipa-acme-manage.1 b/install/tools/man/ipa-acme-manage.1 index e6cec4e4a7fd460c514a72456a2dc9a2e3682ebd..b8383c14f482698d2bcc8b08f0c0bf5882c3c298 100644 --- a/install/tools/man/ipa-acme-manage.1 +++ b/install/tools/man/ipa-acme-manage.1 @@ -79,7 +79,7 @@ For example, "0 0 1 * *" schedules the job to run at 12:00am on the first day of each month. .TP \fB\-\-certretention=CERTRETENTION\fR -Certificate retention time. The default is 30. +Certificate retention time. The default is 30. A value of 0 will remove expired certificates with no delay. .TP \fB\-\-certretentionunit=CERTRETENTIONUNIT\fR Certificate retention units. Valid units are: minute, hour, day, year. @@ -89,10 +89,10 @@ The default is days. LDAP search size limit searching for expired certificates. The default is 1000. This is a client-side limit. There may be additional server-side limitations. .TP \fB\-\-certsearchtimelimit=CERTSEARCHTIMELIMIT\fR -LDAP search time limit searching for expired certificates. The default is 0, no limit. This is a client-side limit. There may be additional server-side limitations. +LDAP search time limit (seconds) searching for expired certificates. The default is 0, no limit. This is a client-side limit. There may be additional server-side limitations. .TP \fB\-\-requestretention=REQUESTRETENTION\fR -Request retention time. The default is 30. +Request retention time. The default is 30. A value of 0 will remove expired requests with no delay. .TP \fB\-\-requestretentionunit=REQUESTRETENTIONUNIT\fR Request retention units. Valid units are: minute, hour, day, year. @@ -102,7 +102,7 @@ The default is days. LDAP search size limit searching for unfulfilled requests. The default is 1000. There may be additional server-side limitations. .TP \fB\-\-requestsearchtimelimit=REQUESTSEARCHTIMELIMIT\fR -LDAP search time limit searching for unfulfilled requests. The default is 0, no limit. There may be additional server-side limitations. +LDAP search time limit (seconds) searching for unfulfilled requests. The default is 0, no limit. There may be additional server-side limitations. .TP \fB\-\-config\-show\fR Show the current pruning configuration diff --git a/ipaserver/install/ipa_acme_manage.py b/ipaserver/install/ipa_acme_manage.py index b7b2111d9edcec2580aa4a485d7a7340146ff065..e7c35ff6fb5b7a30ac9e2c0c18f8db805cf06ee9 100644 --- a/ipaserver/install/ipa_acme_manage.py +++ b/ipaserver/install/ipa_acme_manage.py @@ -207,14 +207,14 @@ class IPAACMEManage(AdminTool): self.options.enable, self.options.disable, self.options.cron, - self.options.certretention, + self.options.certretention is not None, self.options.certretentionunit, - self.options.requestretention, + self.options.requestretention is not None, self.options.requestretentionunit, - self.options.certsearchsizelimit, - self.options.certsearchtimelimit, - self.options.requestsearchsizelimit, - self.options.requestsearchtimelimit, + self.options.certsearchsizelimit is not None, + self.options.certsearchtimelimit is not None, + self.options.requestsearchsizelimit is not None, + self.options.requestsearchtimelimit is not None, ] ) and (self.options.config_show or self.options.run) @@ -226,7 +226,7 @@ class IPAACMEManage(AdminTool): elif self.options.cron: if len(self.options.cron.split()) != 5: self.option_parser.error("Invalid format for --cron") - # dogtag does no validation when setting an option so + # dogtag does no validation when setting this option so # do the minimum. The dogtag cron is limited compared to # crontab(5). opt = self.options.cron.split() @@ -255,7 +255,7 @@ class IPAACMEManage(AdminTool): 'pki-server', command, f'{prefix}.{directive}' ] - if value: + if value is not None: args.extend([str(value)]) logger.debug(args) result = run(args, raiseonerr=False, capture_output=True, @@ -350,28 +350,28 @@ class IPAACMEManage(AdminTool): # pki-server ca-config-set can only set one option at a time so # loop through all the options and set what is there. - if self.options.certretention: + if self.options.certretention is not None: ca_config_set('certRetentionTime', self.options.certretention) if self.options.certretentionunit: ca_config_set('certRetentionUnit', self.options.certretentionunit) - if self.options.certsearchtimelimit: + if self.options.certsearchtimelimit is not None: ca_config_set('certSearchTimeLimit', self.options.certsearchtimelimit) - if self.options.certsearchsizelimit: + if self.options.certsearchsizelimit is not None: ca_config_set('certSearchSizeLimit', self.options.certsearchsizelimit) - if self.options.requestretention: + if self.options.requestretention is not None: ca_config_set('requestRetentionTime', self.options.requestretention) if self.options.requestretentionunit: ca_config_set('requestRetentionUnit', self.options.requestretentionunit) - if self.options.requestsearchsizelimit: + if self.options.requestsearchsizelimit is not None: ca_config_set('requestSearchSizeLimit', self.options.requestsearchsizelimit) - if self.options.requestsearchtimelimit: + if self.options.requestsearchtimelimit is not None: ca_config_set('requestSearchTimeLimit', self.options.requestsearchtimelimit) if self.options.cron: -- 2.39.1