From 20ff7c16022793c707f6c2b8fb38a801870bc0e2 Mon Sep 17 00:00:00 2001
From: Rob Crittenden <rcritten@redhat.com>
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 <rcritten@redhat.com>
Reviewed-By: Francisco Trivino <ftrivino@redhat.com>
---
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