Skip to content

Commit

Permalink
Return 2 when certificates are not found during requests
Browse files Browse the repository at this point in the history
The ipa tool has nearly since epoch returned 2 for the case of
entry not found.

The certificate processing raises a separate error,
CertificateOperationsError, when something goes wrong.
This returns 1.

With the introduction of the JSON API most requests will get
a proper HTTP return code representing what went wrong. In this
case we can use 404 to determine if the request resulted in
a NotFound therefore can eventually return a 2 and be
consistent in return values.

Related: https://pagure.io/freeipa/issue/9562

Signed-off-by: Rob Crittenden <rcritten@redhat.com>
Reviewed-By: Florence Blanc-Renaud <flo@redhat.com>
  • Loading branch information
rcritten authored and flo-renaud committed Apr 2, 2024
1 parent a9bb811 commit 5d3c6b7
Show file tree
Hide file tree
Showing 2 changed files with 4 additions and 2 deletions.
4 changes: 3 additions & 1 deletion ipaserver/plugins/dogtag.py
Original file line number Diff line number Diff line change
Expand Up @@ -689,7 +689,7 @@ def raise_certificate_operation_error(self, func_name, err_msg=None, detail=None
:param err_msg: diagnostic error message, if not supplied it will be
'Unable to communicate with CMS'
:param detail: extra information that will be appended to err_msg
inside a parenthesis
inside a parenthesis. This may be an HTTP status msg
Raise a CertificateOperationError and log the error message.
"""
Expand All @@ -701,6 +701,8 @@ def raise_certificate_operation_error(self, func_name, err_msg=None, detail=None
err_msg = u'%s (%s)' % (err_msg, detail)

logger.error('%s.%s(): %s', type(self).__name__, func_name, err_msg)
if detail == 404:
raise errors.NotFound(reason=err_msg)
raise errors.CertificateOperationError(error=err_msg)

def _request(self, url, port, **kw):
Expand Down
2 changes: 1 addition & 1 deletion ipatests/test_xmlrpc/test_cert_plugin.py
Original file line number Diff line number Diff line change
Expand Up @@ -558,6 +558,6 @@ def test_revoke_and_remove_hold(self):

def test_remove_hold_nonexistent_cert(self):
# remove hold must print 'Certificate ID xx not found'
with pytest.raises(errors.CertificateOperationError,
with pytest.raises(errors.NotFound,
match=r'Certificate ID 0x.* not found'):
api.Command['cert_remove_hold'](9999)

0 comments on commit 5d3c6b7

Please sign in to comment.