Moti Asayag has posted comments on this change. Change subject: engine : No audit log for failed commands ......................................................................
Patch Set 2: (6 comments) https://gerrit.ovirt.org/#/c/37991/2/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CommandBase.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CommandBase.java: Line 791: && internalValidateAndSetQuota(); Line 792: if (!returnValue && getReturnValue().getCanDoActionMessages().size() > 0) { Line 793: String message = String.format("CanDoAction of action '%s' failed for user %s. Reasons: %s", Line 794: getActionType(), getUserName(), Line 795: StringUtils.join(getReturnValue().getCanDoActionMessages(), ',')); i don't think this will be understandable. it might contain place holders. we should use the ErrorTranslator to construct a descent message out of it. Line 796: AuditLogableBase logable = new AuditLogableBase(Guid.isNullOrEmpty(getVdsId()) ? null : getVdsId(), Line 797: Guid.isNullOrEmpty(getVmId()) ? null : getVmId()); Line 798: logable.setUserId(getUserId()); Line 799: logable.setUserName(getUserName()); Line 796: AuditLogableBase logable = new AuditLogableBase(Guid.isNullOrEmpty(getVdsId()) ? null : getVdsId(), Line 797: Guid.isNullOrEmpty(getVmId()) ? null : getVmId()); Line 798: logable.setUserId(getUserId()); Line 799: logable.setUserName(getUserName()); Line 800: logable.setExternal(true); i don't think this is correct - not any event is an external event. Line 801: logable.setOrigin(getOrigin()); Line 802: logable.setCustomEventId(getCustomEventId()); Line 803: logable.setEventFloodInSec(getEventFloodInSec()); Line 804: logable.setVmTemplateId(getVmTemplateId()); Line 797: Guid.isNullOrEmpty(getVmId()) ? null : getVmId()); Line 798: logable.setUserId(getUserId()); Line 799: logable.setUserName(getUserName()); Line 800: logable.setExternal(true); Line 801: logable.setOrigin(getOrigin()); please remove. it seems useless. Line 802: logable.setCustomEventId(getCustomEventId()); Line 803: logable.setEventFloodInSec(getEventFloodInSec()); Line 804: logable.setVmTemplateId(getVmTemplateId()); Line 805: logable.setCustomData(getCustomData()); Line 798: logable.setUserId(getUserId()); Line 799: logable.setUserName(getUserName()); Line 800: logable.setExternal(true); Line 801: logable.setOrigin(getOrigin()); Line 802: logable.setCustomEventId(getCustomEventId()); same Line 803: logable.setEventFloodInSec(getEventFloodInSec()); Line 804: logable.setVmTemplateId(getVmTemplateId()); Line 805: logable.setCustomData(getCustomData()); Line 806: AuditLogDirector.log(logable, AuditLogType.CAN_DO_ACTION_FAILED, message); Line 799: logable.setUserName(getUserName()); Line 800: logable.setExternal(true); Line 801: logable.setOrigin(getOrigin()); Line 802: logable.setCustomEventId(getCustomEventId()); Line 803: logable.setEventFloodInSec(getEventFloodInSec()); i don't think this should be limited. Line 804: logable.setVmTemplateId(getVmTemplateId()); Line 805: logable.setCustomData(getCustomData()); Line 806: AuditLogDirector.log(logable, AuditLogType.CAN_DO_ACTION_FAILED, message); Line 807: log.warn(message); Line 801: logable.setOrigin(getOrigin()); Line 802: logable.setCustomEventId(getCustomEventId()); Line 803: logable.setEventFloodInSec(getEventFloodInSec()); Line 804: logable.setVmTemplateId(getVmTemplateId()); Line 805: logable.setCustomData(getCustomData()); we don't need this info. please remove. Line 806: AuditLogDirector.log(logable, AuditLogType.CAN_DO_ACTION_FAILED, message); Line 807: log.warn(message); Line 808: } Line 809: } finally { -- To view, visit https://gerrit.ovirt.org/37991 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I42b8f70df9b840cfb71429fa006f250b0495e41e Gerrit-PatchSet: 2 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Ravi Nori <rn...@redhat.com> Gerrit-Reviewer: Eli Mesika <emes...@redhat.com> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Moti Asayag <masa...@redhat.com> Gerrit-Reviewer: Oved Ourfali <oourf...@redhat.com> Gerrit-Reviewer: Ravi Nori <rn...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
