> On June 6, 2024, 9:17 p.m., Abhay Kulkarni wrote: > > security-admin/src/main/java/org/apache/ranger/biz/RangerBizUtil.java > > Lines 1172 (patched) > > <https://reviews.apache.org/r/75012/diff/4/?file=2288327#file2288327line1195> > > > > Why would xTrxLog be null? Please review. Also, is it expected that > > only one transaction will be created for each user action? If so, does this > > API need to take a list of transaction logs?
xTrxLog will not be null here. Updated to address this. There are scenarios where a single action from an user can result multiple records; for example, changing resources in a data-share will require recording the impact on all associated datasets. We should revisit though, to find if there are better ways to capture such scenario. I suggest to retain current signature till then. > On June 6, 2024, 9:17 p.m., Abhay Kulkarni wrote: > > security-admin/src/main/java/org/apache/ranger/service/AbstractAuditedResourceService.java > > Line 106 (original), 121 (patched) > > <https://reviews.apache.org/r/75012/diff/4/?file=2288342#file2288342line122> > > > > If only one XXTrxLogV2 object is created by every caller, then consider > > changing the return type to XXTrxLogV2 (rather than a list of XXTrxLogV2). > > > > Please review other APIs that accept (or return) list of XXTrxLogV2 > > objects to see they are necessary here and in RangerAuditedModelService > > class. Multiple trxLog entries might be created for a given change (see the comment in RangerBizUtil.java:1172). > On June 6, 2024, 9:17 p.m., Abhay Kulkarni wrote: > > security-admin/src/main/java/org/apache/ranger/service/RangerTrxLogV2Service.java > > Lines 108 (patched) > > <https://reviews.apache.org/r/75012/diff/4/?file=2288344#file2288344line108> > > > > Please see if uidNameCache needs to be created here. There may be > > always only one VXTrxLogV2 object for a given transactionId. There could be multiple trxLog entries for a given transaction ID (see the comment in RangerBizUtil.java:1172). Hence caching userId=>name mapping will help avoid excessive calls to the database. - Madhan ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/75012/#review226519 ----------------------------------------------------------- On June 6, 2024, 11:15 p.m., Madhan Neethiraj wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/75012/ > ----------------------------------------------------------- > > (Updated June 6, 2024, 11:15 p.m.) > > > Review request for ranger, Abhishek Kumar, Asit Vadhavkar, Fateh Singh, > Abhay Kulkarni, Mehul Parikh, Ramesh Mani, Siddhesh Phatak, Sailaja > Polavarapu, and Subhrat Chaudhary. > > > Bugs: RANGER-4803 > https://issues.apache.org/jira/browse/RANGER-4803 > > > Repository: ranger > > > Description > ------- > > - introduced table x_trx_log_v2 to store object change details, including all > changed attributes > - updated to create a single x_trx_log_v2 row instead of multiple rows in > x_trx_log > - updated APIs used by UI to retrieve admin audit logs to read from > x_trx_log_v2 > -- /assets/report > -- /assets/report/{transactionId} > - removed classes that reference x_trx_log table: XTrxLogService, > XTrxLogServiceBase, XXTrxLogDao, XXTrxLogUpdateUtil > > > Diffs > ----- > > security-admin/db/mysql/optimized/current/ranger_core_db_mysql.sql > fa72ef2ea > security-admin/db/oracle/optimized/current/ranger_core_db_oracle.sql > 772514610 > security-admin/db/postgres/optimized/current/ranger_core_db_postgres.sql > e177d44d7 > > security-admin/db/sqlanywhere/optimized/current/ranger_core_db_sqlanywhere.sql > 32cfed7ae > security-admin/db/sqlserver/optimized/current/ranger_core_db_sqlserver.sql > 831aad66d > security-admin/src/main/java/org/apache/ranger/biz/AssetMgr.java 6f1bcc40e > security-admin/src/main/java/org/apache/ranger/biz/RangerBizUtil.java > 5534c8056 > security-admin/src/main/java/org/apache/ranger/biz/ServiceDBStore.java > 7ac16a9ee > security-admin/src/main/java/org/apache/ranger/biz/UserMgr.java 6bcc19c0c > security-admin/src/main/java/org/apache/ranger/biz/XAuditMgrBase.java > c90296cf6 > security-admin/src/main/java/org/apache/ranger/biz/XUserMgr.java 3a1cb17f2 > > security-admin/src/main/java/org/apache/ranger/common/db/RangerTransactionSynchronizationAdapter.java > a9ec94e1c > security-admin/src/main/java/org/apache/ranger/db/RangerDaoManagerBase.java > 08bcfd57b > security-admin/src/main/java/org/apache/ranger/db/XXTrxLogDao.java > 13372ab4a > security-admin/src/main/java/org/apache/ranger/db/XXTrxLogV2Dao.java > PRE-CREATION > security-admin/src/main/java/org/apache/ranger/entity/XXTrxLogV2.java > PRE-CREATION > > security-admin/src/main/java/org/apache/ranger/patch/cliutil/XXTrxLogUpdateUtil.java > cc30fde9b > security-admin/src/main/java/org/apache/ranger/rest/AssetREST.java > be077e789 > security-admin/src/main/java/org/apache/ranger/rest/RoleREST.java b35b8af65 > security-admin/src/main/java/org/apache/ranger/rest/ServiceREST.java > cc762aefa > security-admin/src/main/java/org/apache/ranger/rest/XAuditREST.java > a7047e897 > > security-admin/src/main/java/org/apache/ranger/service/AbstractAuditedResourceService.java > b57979805 > > security-admin/src/main/java/org/apache/ranger/service/RangerAuditedModelService.java > b8c4fb31c > > security-admin/src/main/java/org/apache/ranger/service/RangerTrxLogV2Service.java > PRE-CREATION > > security-admin/src/main/java/org/apache/ranger/service/XPortalUserService.java > 7856671a2 > security-admin/src/main/java/org/apache/ranger/service/XTrxLogService.java > 676552e6e > > security-admin/src/main/java/org/apache/ranger/service/XTrxLogServiceBase.java > d6f23db40 > security-admin/src/main/java/org/apache/ranger/view/VXTrxLogV2.java > PRE-CREATION > security-admin/src/main/java/org/apache/ranger/view/VXTrxLogV2List.java > PRE-CREATION > security-admin/src/main/resources/META-INF/jpa_named_queries.xml bb93947b5 > security-admin/src/test/java/org/apache/ranger/rest/TestAssetREST.java > 23fbb8494 > security-admin/src/test/java/org/apache/ranger/rest/TestXAuditREST.java > 5cf21d5b0 > security-admin/src/test/java/org/apache/ranger/rest/TestXKeyREST.java > c0a858d81 > > > Diff: https://reviews.apache.org/r/75012/diff/5/ > > > Testing > ------- > > - verified that admin audits are stored in x_trx_log_v2, one entry for each > object > - Admin audits UI continues to work with data in x_trx_log_v2 table > - verified that all existing tests pass successfully > > > Thanks, > > Madhan Neethiraj > >
