389-ds-base

Clone
Source Code
GIT

#50014 Ticket 50013 - Log warn instead of ERR when aci target does not exist.
Closed 3 years ago by spichugi. Opened 5 years ago by gparente.
gparente/389-ds-base branch_acl_warn  into  master

file modified
+1 -1 
@@ -147,7 +147,7 @@ 
 

                      slapi_search_internal_pb(temppb);
 

                      slapi_pblock_get(temppb, SLAPI_PLUGIN_INTOP_RESULT, &rc);
 

                      if (rc != LDAP_SUCCESS) {
 

-                         slapi_log_err(SLAPI_LOG_ERR, plugin_name,
 

+                         slapi_log_err(SLAPI_LOG_WARNING, plugin_name,
 

                                        "acl_parse - The ACL target %s does not exist\n", slapi_sdn_get_dn(&targdn));
 

                      }

Bug Description:

This is something we have very often in IPA context and customers are very often asking why there are errors in the logs:

[31/Oct/2018:05:52:23.436616394 -0400] - ERR - NSACLPlugin - acl_parse - The ACL target cn=groups,cn=compat,dc=cgparente,dc=local does not exist
[31/Oct/2018:05:52:23.438951763 -0400] - ERR - NSACLPlugin - acl_parse - The ACL target cn=computers,cn=compat,dc=cgparente,dc=local does not exist

Fix Description:

just log WARN instead of ERR

https://pagure.io/389-ds-base/issue/50013

Author: German Parente gparente@redhat.com

Review by: ???

Thanks @gparente looks good, ack!

What other branches (or RHEL versions) do you need this fix for?

@mreynolds As an extension to this, should logconv check for this message? And should we have a way to "trigger" and aci validation check for this situation? I think that it's good to have this warning, but admins could really easily overlook it, and it won't display for "past" aci's, or acis where the target is removed later.

@mreynolds As an extension to this, should logconv check for this message?

Logconv.pl does not check the errors logs ;-)

And should we have a way to "trigger" and aci validation check for this situation?

The way the old console would do aci syntax checking was to add it and delete it. If the add works the syntax was good! Otherwise it failed. I don't know of any way to do it without creating a new extended op, or writing a cli tool to do syntax checking

@mreynolds Yeah, I think this is more of a future goal than anything. Maybe we should be extending the lib389 healthcheck command to do something like this....

@mreynolds Yeah, I think this is more of a future goal than anything. Maybe we should be extending the lib389 healthcheck command to do something like this....

Yeah maybe, ACI's could definitely be made more user friendly (and have more support)

@mreynolds I will need this to be backported to RHEL7 branch.

@mreynolds , @firstyear : we should not make these aci's invalid. In fact, IPA product set aci's for target entries that do not exist and will exist in the future. A WARN seems to me valid. Or perhaps no message a all since it's not a syntax error. It's just the aci referencing an entry that does not exist. The problem with this is that some customers looks for ERR in the logs to ask the root cause. And in this case we have to say, from support side, that it's just a warning.

Pull-Request has been merged by gparente
5 years ago

I agree with German, it is nothing wrong with acis which taget entries that do not already exist, if you want for example add rights for children of an entry you need this.
But there might be acis unintntionally targeting non existent entries - and this should be logged.
So I think the right solution is to log a message in this case, but not at ERR level

I think I'm saying not that the aci's shouldn't exist, but that the warning going to the log may not always be visible to the administrator who needs to see the warning? I agree that there are valid use cases where the target can not exist and the aci is valid.

389-ds-base is moving from Pagure to Github. This means that new issues and pull requests
will be accepted only in 389-ds-base's github repository.

This pull request has been cloned to Github as issue and is available here:
- https://github.com/389ds/389-ds-base/issues/3073

If you want to continue to work on the PR, please navigate to the github issue,
download the patch from the attachments and file a new pull request.

Thank you for understanding. We apologize for all inconvenience.

Pull-Request has been closed by spichugi
3 years ago
Metadata
None
No Tags
Changes Summary 1
+1 -1
file changed
ldap/servers/plugins/acl/aclparse.c
Powered by Pagure 5.13.3
Documentation • File an Issue • About • SSH Hostkey/Fingerprint
© Red Hat, Inc. and others.