You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sentry.apache.org by Sergio Pena via Review Board <no...@reviews.apache.org> on 2018/08/08 14:07:10 UTC
Review Request 68268: SENTRY-2157: Update audit log to grant/revoke
owner privileges
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68268/
-----------------------------------------------------------
Review request for sentry, Arjun Mishra, kalyan kumar kalvagadda, and Na Li.
Bugs: sentry-2157
https://issues.apache.org/jira/browse/sentry-2157
Repository: sentry
Description
-------
This patch logs owner privileges grants and revokes.
Diffs
-----
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyStoreProcessor.java 61f9168b1970144dbf0b7a7378f2d25e70f1761d
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/audit/SentryAuditLogger.java PRE-CREATION
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/log/entity/JsonLogEntityFactory.java 61becceac881443b02182e6ab1012add4c046499
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/log/util/CommandUtil.java 6479a6055e8c7087f0e484080ec9d46a9c146212
sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/log/entity/TestJsonLogEntityFactory.java 307f38eadb65bf12dc6225cfe43a5d590657d055
Diff: https://reviews.apache.org/r/68268/diff/1/
Testing
-------
I run the patch in a cluster and the audit logs is displaying the correct messages.
Thanks,
Sergio Pena
Re: Review Request 68268: SENTRY-2157: Update audit log to
grant/revoke owner privileges
Posted by Sergio Pena via Review Board <no...@reviews.apache.org>.
> On Aug. 8, 2018, 9:12 p.m., Na Li wrote:
> > sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/log/entity/JsonLogEntityFactory.java
> > Lines 169 (patched)
> > <https://reviews.apache.org/r/68268/diff/1/?file=2070544#file2070544line169>
> >
> > This function calls createCmdForImplicitGrantOwnerPrivilege without checking anything specific to owner privilege. Should this function's name reflect the fact that it is specific for owner privilege? Otherwise, it is easy to introduce bug that some other developer could call this function for privilege other than owner privilege. Then audit log is wrong
I modify the flags to avoid confusions.
- Sergio
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68268/#review206999
-----------------------------------------------------------
On Aug. 8, 2018, 2:29 p.m., Sergio Pena wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68268/
> -----------------------------------------------------------
>
> (Updated Aug. 8, 2018, 2:29 p.m.)
>
>
> Review request for sentry, Arjun Mishra, kalyan kumar kalvagadda, and Na Li.
>
>
> Bugs: sentry-2157
> https://issues.apache.org/jira/browse/sentry-2157
>
>
> Repository: sentry
>
>
> Description
> -------
>
> This patch logs owner privileges grants and revokes.
>
>
> Diffs
> -----
>
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyStoreProcessor.java 61f9168b1970144dbf0b7a7378f2d25e70f1761d
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/audit/SentryAuditLogger.java PRE-CREATION
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/log/entity/JsonLogEntityFactory.java 61becceac881443b02182e6ab1012add4c046499
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/log/util/CommandUtil.java 6479a6055e8c7087f0e484080ec9d46a9c146212
> sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/log/entity/TestJsonLogEntityFactory.java 307f38eadb65bf12dc6225cfe43a5d590657d055
>
>
> Diff: https://reviews.apache.org/r/68268/diff/1/
>
>
> Testing
> -------
>
> I run the patch in a cluster and the audit logs is displaying the correct messages.
>
> {"serviceName":"Sentry-Service","userName":"ubuntu","impersonator":"","ipAddress":"/127.0.0.1","operation":"GRANT_PRIVILEGE","eventTime":"1533738512795","operationText":"OWNER privilege granted to USER: sergio","allowed":"true","databaseName":"default","tableName":"t2","column":null,"resourcePath":null,"objectType":"PRINCIPAL"}
>
>
> Thanks,
>
> Sergio Pena
>
>
Re: Review Request 68268: SENTRY-2157: Update audit log to
grant/revoke owner privileges
Posted by Na Li via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68268/#review206999
-----------------------------------------------------------
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/log/entity/JsonLogEntityFactory.java
Lines 169 (patched)
<https://reviews.apache.org/r/68268/#comment290163>
This function calls createCmdForImplicitGrantOwnerPrivilege without checking anything specific to owner privilege. Should this function's name reflect the fact that it is specific for owner privilege? Otherwise, it is easy to introduce bug that some other developer could call this function for privilege other than owner privilege. Then audit log is wrong
- Na Li
On Aug. 8, 2018, 2:29 p.m., Sergio Pena wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68268/
> -----------------------------------------------------------
>
> (Updated Aug. 8, 2018, 2:29 p.m.)
>
>
> Review request for sentry, Arjun Mishra, kalyan kumar kalvagadda, and Na Li.
>
>
> Bugs: sentry-2157
> https://issues.apache.org/jira/browse/sentry-2157
>
>
> Repository: sentry
>
>
> Description
> -------
>
> This patch logs owner privileges grants and revokes.
>
>
> Diffs
> -----
>
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyStoreProcessor.java 61f9168b1970144dbf0b7a7378f2d25e70f1761d
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/audit/SentryAuditLogger.java PRE-CREATION
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/log/entity/JsonLogEntityFactory.java 61becceac881443b02182e6ab1012add4c046499
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/log/util/CommandUtil.java 6479a6055e8c7087f0e484080ec9d46a9c146212
> sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/log/entity/TestJsonLogEntityFactory.java 307f38eadb65bf12dc6225cfe43a5d590657d055
>
>
> Diff: https://reviews.apache.org/r/68268/diff/1/
>
>
> Testing
> -------
>
> I run the patch in a cluster and the audit logs is displaying the correct messages.
>
> {"serviceName":"Sentry-Service","userName":"ubuntu","impersonator":"","ipAddress":"/127.0.0.1","operation":"GRANT_PRIVILEGE","eventTime":"1533738512795","operationText":"OWNER privilege granted to USER: sergio","allowed":"true","databaseName":"default","tableName":"t2","column":null,"resourcePath":null,"objectType":"PRINCIPAL"}
>
>
> Thanks,
>
> Sergio Pena
>
>
Re: Review Request 68268: SENTRY-2157: Update audit log to
grant/revoke owner privileges
Posted by Sergio Pena via Review Board <no...@reviews.apache.org>.
> On Aug. 8, 2018, 3:46 p.m., Arjun Mishra wrote:
> > sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyStoreProcessor.java
> > Lines 1503 (patched)
> > <https://reviews.apache.org/r/68268/diff/1/?file=2070542#file2070542line1566>
> >
> > Can we maintain the same consistency with other audit methods?Maybe just pass in the request object?
> >
> > Also, Can you explain why onGrantOwnerPrivileges needs the Status parameter to be passed in?
I cannot keep the consistency unless I create a thrift object for user privileges. Because the owner privilege is granted implicitly by Sentry, then we don't need the Thrift object, so I had to have this method with those parameters.
The Status is used by the createJsonLogEntity() method to append the status.
> On Aug. 8, 2018, 3:46 p.m., Arjun Mishra wrote:
> > sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyStoreProcessor.java
> > Line 1619 (original), 1571 (patched)
> > <https://reviews.apache.org/r/68268/diff/1/?file=2070542#file2070542line1635>
> >
> > Why don't we have an audit log for revoking owner privileges?
Because there is not information about what is being revoked. The revoke happens by the revokeOwnerPrivileges() call.
I initially though on breaking the code to allow auditing the revoke, but something special of the OWNER privilege is that only one role or user can have it and it cannot be revoked. So, if you grant the OWNER to a user or role, it immediatly means nobody else has that privilege, so in this case, it does not make sense to log that as the OWNER privilege is assigned to one user or role.
> On Aug. 8, 2018, 3:46 p.m., Arjun Mishra wrote:
> > sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/log/util/CommandUtil.java
> > Lines 215 (patched)
> > <https://reviews.apache.org/r/68268/diff/1/?file=2070545#file2070545line216>
> >
> > I like this. Was curious since this should return the command that the user ran.
> > Is there some way to display that?
I thought that too, but how to know what happend was executed? We know it could be a create database or table, or alter database or table; but that's a command from Hive. This audit log is for Sentry (no Hive). The only way I thought so is to write that message.
- Sergio
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68268/#review206980
-----------------------------------------------------------
On Aug. 8, 2018, 2:29 p.m., Sergio Pena wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68268/
> -----------------------------------------------------------
>
> (Updated Aug. 8, 2018, 2:29 p.m.)
>
>
> Review request for sentry, Arjun Mishra, kalyan kumar kalvagadda, and Na Li.
>
>
> Bugs: sentry-2157
> https://issues.apache.org/jira/browse/sentry-2157
>
>
> Repository: sentry
>
>
> Description
> -------
>
> This patch logs owner privileges grants and revokes.
>
>
> Diffs
> -----
>
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyStoreProcessor.java 61f9168b1970144dbf0b7a7378f2d25e70f1761d
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/audit/SentryAuditLogger.java PRE-CREATION
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/log/entity/JsonLogEntityFactory.java 61becceac881443b02182e6ab1012add4c046499
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/log/util/CommandUtil.java 6479a6055e8c7087f0e484080ec9d46a9c146212
> sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/log/entity/TestJsonLogEntityFactory.java 307f38eadb65bf12dc6225cfe43a5d590657d055
>
>
> Diff: https://reviews.apache.org/r/68268/diff/1/
>
>
> Testing
> -------
>
> I run the patch in a cluster and the audit logs is displaying the correct messages.
>
> {"serviceName":"Sentry-Service","userName":"ubuntu","impersonator":"","ipAddress":"/127.0.0.1","operation":"GRANT_PRIVILEGE","eventTime":"1533738512795","operationText":"OWNER privilege granted to USER: sergio","allowed":"true","databaseName":"default","tableName":"t2","column":null,"resourcePath":null,"objectType":"PRINCIPAL"}
>
>
> Thanks,
>
> Sergio Pena
>
>
Re: Review Request 68268: SENTRY-2157: Update audit log to
grant/revoke owner privileges
Posted by Sergio Pena via Review Board <no...@reviews.apache.org>.
> On Aug. 8, 2018, 3:46 p.m., Arjun Mishra wrote:
> > sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyStoreProcessor.java
> > Line 1619 (original), 1571 (patched)
> > <https://reviews.apache.org/r/68268/diff/1/?file=2070542#file2070542line1635>
> >
> > Why don't we have an audit log for revoking owner privileges?
>
> Sergio Pena wrote:
> Because there is not information about what is being revoked. The revoke happens by the revokeOwnerPrivileges() call.
>
> I initially though on breaking the code to allow auditing the revoke, but something special of the OWNER privilege is that only one role or user can have it and it cannot be revoked. So, if you grant the OWNER to a user or role, it immediatly means nobody else has that privilege, so in this case, it does not make sense to log that as the OWNER privilege is assigned to one user or role.
>
> Arjun Mishra wrote:
> So are you saying we can never revoke owner privileges? If yes then I am ok with the change. I would think the owner privilege would get revoked by deleting the hive object. And I know Sentry gets notified of it
The owner and any other privilege is deleted by Sentry when an object is deleted on HMS. That's (for some reason) hasn't been audited. We might want to audit that too.
And yes, the OWNER privilege cannot be revoked, it can only be transferred to another user.
- Sergio
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68268/#review206980
-----------------------------------------------------------
On Aug. 8, 2018, 2:29 p.m., Sergio Pena wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68268/
> -----------------------------------------------------------
>
> (Updated Aug. 8, 2018, 2:29 p.m.)
>
>
> Review request for sentry, Arjun Mishra, kalyan kumar kalvagadda, and Na Li.
>
>
> Bugs: sentry-2157
> https://issues.apache.org/jira/browse/sentry-2157
>
>
> Repository: sentry
>
>
> Description
> -------
>
> This patch logs owner privileges grants and revokes.
>
>
> Diffs
> -----
>
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyStoreProcessor.java 61f9168b1970144dbf0b7a7378f2d25e70f1761d
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/audit/SentryAuditLogger.java PRE-CREATION
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/log/entity/JsonLogEntityFactory.java 61becceac881443b02182e6ab1012add4c046499
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/log/util/CommandUtil.java 6479a6055e8c7087f0e484080ec9d46a9c146212
> sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/log/entity/TestJsonLogEntityFactory.java 307f38eadb65bf12dc6225cfe43a5d590657d055
>
>
> Diff: https://reviews.apache.org/r/68268/diff/1/
>
>
> Testing
> -------
>
> I run the patch in a cluster and the audit logs is displaying the correct messages.
>
> {"serviceName":"Sentry-Service","userName":"ubuntu","impersonator":"","ipAddress":"/127.0.0.1","operation":"GRANT_PRIVILEGE","eventTime":"1533738512795","operationText":"OWNER privilege granted to USER: sergio","allowed":"true","databaseName":"default","tableName":"t2","column":null,"resourcePath":null,"objectType":"PRINCIPAL"}
>
>
> Thanks,
>
> Sergio Pena
>
>
Re: Review Request 68268: SENTRY-2157: Update audit log to
grant/revoke owner privileges
Posted by Arjun Mishra via Review Board <no...@reviews.apache.org>.
> On Aug. 8, 2018, 3:46 p.m., Arjun Mishra wrote:
> > sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyStoreProcessor.java
> > Line 1619 (original), 1571 (patched)
> > <https://reviews.apache.org/r/68268/diff/1/?file=2070542#file2070542line1635>
> >
> > Why don't we have an audit log for revoking owner privileges?
>
> Sergio Pena wrote:
> Because there is not information about what is being revoked. The revoke happens by the revokeOwnerPrivileges() call.
>
> I initially though on breaking the code to allow auditing the revoke, but something special of the OWNER privilege is that only one role or user can have it and it cannot be revoked. So, if you grant the OWNER to a user or role, it immediatly means nobody else has that privilege, so in this case, it does not make sense to log that as the OWNER privilege is assigned to one user or role.
So are you saying we can never revoke owner privileges? If yes then I am ok with the change. I would think the owner privilege would get revoked by deleting the hive object. And I know Sentry gets notified of it
- Arjun
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68268/#review206980
-----------------------------------------------------------
On Aug. 8, 2018, 2:29 p.m., Sergio Pena wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68268/
> -----------------------------------------------------------
>
> (Updated Aug. 8, 2018, 2:29 p.m.)
>
>
> Review request for sentry, Arjun Mishra, kalyan kumar kalvagadda, and Na Li.
>
>
> Bugs: sentry-2157
> https://issues.apache.org/jira/browse/sentry-2157
>
>
> Repository: sentry
>
>
> Description
> -------
>
> This patch logs owner privileges grants and revokes.
>
>
> Diffs
> -----
>
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyStoreProcessor.java 61f9168b1970144dbf0b7a7378f2d25e70f1761d
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/audit/SentryAuditLogger.java PRE-CREATION
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/log/entity/JsonLogEntityFactory.java 61becceac881443b02182e6ab1012add4c046499
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/log/util/CommandUtil.java 6479a6055e8c7087f0e484080ec9d46a9c146212
> sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/log/entity/TestJsonLogEntityFactory.java 307f38eadb65bf12dc6225cfe43a5d590657d055
>
>
> Diff: https://reviews.apache.org/r/68268/diff/1/
>
>
> Testing
> -------
>
> I run the patch in a cluster and the audit logs is displaying the correct messages.
>
> {"serviceName":"Sentry-Service","userName":"ubuntu","impersonator":"","ipAddress":"/127.0.0.1","operation":"GRANT_PRIVILEGE","eventTime":"1533738512795","operationText":"OWNER privilege granted to USER: sergio","allowed":"true","databaseName":"default","tableName":"t2","column":null,"resourcePath":null,"objectType":"PRINCIPAL"}
>
>
> Thanks,
>
> Sergio Pena
>
>
Re: Review Request 68268: SENTRY-2157: Update audit log to
grant/revoke owner privileges
Posted by Arjun Mishra via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68268/#review206980
-----------------------------------------------------------
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyStoreProcessor.java
Lines 1503 (patched)
<https://reviews.apache.org/r/68268/#comment290142>
Can we maintain the same consistency with other audit methods?Maybe just pass in the request object?
Also, Can you explain why onGrantOwnerPrivileges needs the Status parameter to be passed in?
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyStoreProcessor.java
Line 1619 (original), 1571 (patched)
<https://reviews.apache.org/r/68268/#comment290141>
Why don't we have an audit log for revoking owner privileges?
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/log/util/CommandUtil.java
Lines 215 (patched)
<https://reviews.apache.org/r/68268/#comment290143>
I like this. Was curious since this should return the command that the user ran.
Is there some way to display that?
- Arjun Mishra
On Aug. 8, 2018, 2:29 p.m., Sergio Pena wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68268/
> -----------------------------------------------------------
>
> (Updated Aug. 8, 2018, 2:29 p.m.)
>
>
> Review request for sentry, Arjun Mishra, kalyan kumar kalvagadda, and Na Li.
>
>
> Bugs: sentry-2157
> https://issues.apache.org/jira/browse/sentry-2157
>
>
> Repository: sentry
>
>
> Description
> -------
>
> This patch logs owner privileges grants and revokes.
>
>
> Diffs
> -----
>
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyStoreProcessor.java 61f9168b1970144dbf0b7a7378f2d25e70f1761d
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/audit/SentryAuditLogger.java PRE-CREATION
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/log/entity/JsonLogEntityFactory.java 61becceac881443b02182e6ab1012add4c046499
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/log/util/CommandUtil.java 6479a6055e8c7087f0e484080ec9d46a9c146212
> sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/log/entity/TestJsonLogEntityFactory.java 307f38eadb65bf12dc6225cfe43a5d590657d055
>
>
> Diff: https://reviews.apache.org/r/68268/diff/1/
>
>
> Testing
> -------
>
> I run the patch in a cluster and the audit logs is displaying the correct messages.
>
> {"serviceName":"Sentry-Service","userName":"ubuntu","impersonator":"","ipAddress":"/127.0.0.1","operation":"GRANT_PRIVILEGE","eventTime":"1533738512795","operationText":"OWNER privilege granted to USER: sergio","allowed":"true","databaseName":"default","tableName":"t2","column":null,"resourcePath":null,"objectType":"PRINCIPAL"}
>
>
> Thanks,
>
> Sergio Pena
>
>
Re: Review Request 68268: SENTRY-2157: Update audit log to
grant/revoke owner privileges
Posted by Arjun Mishra via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68268/#review206989
-----------------------------------------------------------
Ship it!
Ship It!
- Arjun Mishra
On Aug. 8, 2018, 2:29 p.m., Sergio Pena wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68268/
> -----------------------------------------------------------
>
> (Updated Aug. 8, 2018, 2:29 p.m.)
>
>
> Review request for sentry, Arjun Mishra, kalyan kumar kalvagadda, and Na Li.
>
>
> Bugs: sentry-2157
> https://issues.apache.org/jira/browse/sentry-2157
>
>
> Repository: sentry
>
>
> Description
> -------
>
> This patch logs owner privileges grants and revokes.
>
>
> Diffs
> -----
>
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyStoreProcessor.java 61f9168b1970144dbf0b7a7378f2d25e70f1761d
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/audit/SentryAuditLogger.java PRE-CREATION
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/log/entity/JsonLogEntityFactory.java 61becceac881443b02182e6ab1012add4c046499
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/log/util/CommandUtil.java 6479a6055e8c7087f0e484080ec9d46a9c146212
> sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/log/entity/TestJsonLogEntityFactory.java 307f38eadb65bf12dc6225cfe43a5d590657d055
>
>
> Diff: https://reviews.apache.org/r/68268/diff/1/
>
>
> Testing
> -------
>
> I run the patch in a cluster and the audit logs is displaying the correct messages.
>
> {"serviceName":"Sentry-Service","userName":"ubuntu","impersonator":"","ipAddress":"/127.0.0.1","operation":"GRANT_PRIVILEGE","eventTime":"1533738512795","operationText":"OWNER privilege granted to USER: sergio","allowed":"true","databaseName":"default","tableName":"t2","column":null,"resourcePath":null,"objectType":"PRINCIPAL"}
>
>
> Thanks,
>
> Sergio Pena
>
>
Re: Review Request 68268: SENTRY-2157: Update audit log to
grant/revoke owner privileges
Posted by Arjun Mishra via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68268/#review206981
-----------------------------------------------------------
Ship it!
Overall looks good. I had a question about audit message for revoking owner privleges and a few clarifications.
- Arjun Mishra
On Aug. 8, 2018, 2:29 p.m., Sergio Pena wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68268/
> -----------------------------------------------------------
>
> (Updated Aug. 8, 2018, 2:29 p.m.)
>
>
> Review request for sentry, Arjun Mishra, kalyan kumar kalvagadda, and Na Li.
>
>
> Bugs: sentry-2157
> https://issues.apache.org/jira/browse/sentry-2157
>
>
> Repository: sentry
>
>
> Description
> -------
>
> This patch logs owner privileges grants and revokes.
>
>
> Diffs
> -----
>
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyStoreProcessor.java 61f9168b1970144dbf0b7a7378f2d25e70f1761d
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/audit/SentryAuditLogger.java PRE-CREATION
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/log/entity/JsonLogEntityFactory.java 61becceac881443b02182e6ab1012add4c046499
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/log/util/CommandUtil.java 6479a6055e8c7087f0e484080ec9d46a9c146212
> sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/log/entity/TestJsonLogEntityFactory.java 307f38eadb65bf12dc6225cfe43a5d590657d055
>
>
> Diff: https://reviews.apache.org/r/68268/diff/1/
>
>
> Testing
> -------
>
> I run the patch in a cluster and the audit logs is displaying the correct messages.
>
> {"serviceName":"Sentry-Service","userName":"ubuntu","impersonator":"","ipAddress":"/127.0.0.1","operation":"GRANT_PRIVILEGE","eventTime":"1533738512795","operationText":"OWNER privilege granted to USER: sergio","allowed":"true","databaseName":"default","tableName":"t2","column":null,"resourcePath":null,"objectType":"PRINCIPAL"}
>
>
> Thanks,
>
> Sergio Pena
>
>
Re: Review Request 68268: SENTRY-2157: Update audit log to
grant/revoke owner privileges
Posted by Sergio Pena via Review Board <no...@reviews.apache.org>.
> On Aug. 8, 2018, 7:25 p.m., Na Li wrote:
> > sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyStoreProcessor.java
> > Lines 1577 (patched)
> > <https://reviews.apache.org/r/68268/diff/1/?file=2070542#file2070542line1641>
> >
> > should this be audit.onUpdateOwnerPrivilege()?
>
> Sergio Pena wrote:
> Update implies a grant, so I'd rather use grant to avoid having two methods that do the same.
>
> Na Li wrote:
> update implies a grant and also removing. It is alter, not just grant. It is better to be accurate, so we can differentiate different scenarios. In this way, it is easier to debug when something goes wrong. To do this, you need to add more functions for update message such as "OWNER privilege is transferred to USER" OR "OWNER privilege is transferred to ROLE" base on the new owner type, not "OWNER privilege granted to USER"
Aaa true. I changed it to audit.onTransferOwnerPrivilege()
- Sergio
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68268/#review206991
-----------------------------------------------------------
On Aug. 8, 2018, 2:29 p.m., Sergio Pena wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68268/
> -----------------------------------------------------------
>
> (Updated Aug. 8, 2018, 2:29 p.m.)
>
>
> Review request for sentry, Arjun Mishra, kalyan kumar kalvagadda, and Na Li.
>
>
> Bugs: sentry-2157
> https://issues.apache.org/jira/browse/sentry-2157
>
>
> Repository: sentry
>
>
> Description
> -------
>
> This patch logs owner privileges grants and revokes.
>
>
> Diffs
> -----
>
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyStoreProcessor.java 61f9168b1970144dbf0b7a7378f2d25e70f1761d
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/audit/SentryAuditLogger.java PRE-CREATION
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/log/entity/JsonLogEntityFactory.java 61becceac881443b02182e6ab1012add4c046499
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/log/util/CommandUtil.java 6479a6055e8c7087f0e484080ec9d46a9c146212
> sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/log/entity/TestJsonLogEntityFactory.java 307f38eadb65bf12dc6225cfe43a5d590657d055
>
>
> Diff: https://reviews.apache.org/r/68268/diff/1/
>
>
> Testing
> -------
>
> I run the patch in a cluster and the audit logs is displaying the correct messages.
>
> {"serviceName":"Sentry-Service","userName":"ubuntu","impersonator":"","ipAddress":"/127.0.0.1","operation":"GRANT_PRIVILEGE","eventTime":"1533738512795","operationText":"OWNER privilege granted to USER: sergio","allowed":"true","databaseName":"default","tableName":"t2","column":null,"resourcePath":null,"objectType":"PRINCIPAL"}
>
>
> Thanks,
>
> Sergio Pena
>
>
Re: Review Request 68268: SENTRY-2157: Update audit log to
grant/revoke owner privileges
Posted by Na Li via Review Board <no...@reviews.apache.org>.
> On Aug. 8, 2018, 7:25 p.m., Na Li wrote:
> > sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyStoreProcessor.java
> > Lines 1577 (patched)
> > <https://reviews.apache.org/r/68268/diff/1/?file=2070542#file2070542line1641>
> >
> > should this be audit.onUpdateOwnerPrivilege()?
>
> Sergio Pena wrote:
> Update implies a grant, so I'd rather use grant to avoid having two methods that do the same.
update implies a grant and also removing. It is alter, not just grant. It is better to be accurate, so we can differentiate different scenarios. In this way, it is easier to debug when something goes wrong. To do this, you need to add more functions for update message such as "OWNER privilege is transferred to USER" OR "OWNER privilege is transferred to ROLE" base on the new owner type, not "OWNER privilege granted to USER"
- Na
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68268/#review206991
-----------------------------------------------------------
On Aug. 8, 2018, 2:29 p.m., Sergio Pena wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68268/
> -----------------------------------------------------------
>
> (Updated Aug. 8, 2018, 2:29 p.m.)
>
>
> Review request for sentry, Arjun Mishra, kalyan kumar kalvagadda, and Na Li.
>
>
> Bugs: sentry-2157
> https://issues.apache.org/jira/browse/sentry-2157
>
>
> Repository: sentry
>
>
> Description
> -------
>
> This patch logs owner privileges grants and revokes.
>
>
> Diffs
> -----
>
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyStoreProcessor.java 61f9168b1970144dbf0b7a7378f2d25e70f1761d
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/audit/SentryAuditLogger.java PRE-CREATION
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/log/entity/JsonLogEntityFactory.java 61becceac881443b02182e6ab1012add4c046499
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/log/util/CommandUtil.java 6479a6055e8c7087f0e484080ec9d46a9c146212
> sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/log/entity/TestJsonLogEntityFactory.java 307f38eadb65bf12dc6225cfe43a5d590657d055
>
>
> Diff: https://reviews.apache.org/r/68268/diff/1/
>
>
> Testing
> -------
>
> I run the patch in a cluster and the audit logs is displaying the correct messages.
>
> {"serviceName":"Sentry-Service","userName":"ubuntu","impersonator":"","ipAddress":"/127.0.0.1","operation":"GRANT_PRIVILEGE","eventTime":"1533738512795","operationText":"OWNER privilege granted to USER: sergio","allowed":"true","databaseName":"default","tableName":"t2","column":null,"resourcePath":null,"objectType":"PRINCIPAL"}
>
>
> Thanks,
>
> Sergio Pena
>
>
Re: Review Request 68268: SENTRY-2157: Update audit log to
grant/revoke owner privileges
Posted by Sergio Pena via Review Board <no...@reviews.apache.org>.
> On Aug. 8, 2018, 7:25 p.m., Na Li wrote:
> > sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyStoreProcessor.java
> > Line 1396 (original), 1336 (patched)
> > <https://reviews.apache.org/r/68268/diff/1/?file=2070542#file2070542line1399>
> >
> > do we generate audit message when owner privilege is removed in notification processor?
There is not audit process in the NotificationProcessor. That would require different parameters in the Audit loger and how the JSON are generated as well.
Currently all privileges are cleaned-up when a DROP command is found and are not audited. We should consider investigat that issue in another patch as cleaning an unexistent object of privileges is different from revoking privileges.
> On Aug. 8, 2018, 7:25 p.m., Na Li wrote:
> > sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyStoreProcessor.java
> > Lines 1577 (patched)
> > <https://reviews.apache.org/r/68268/diff/1/?file=2070542#file2070542line1641>
> >
> > should this be audit.onUpdateOwnerPrivilege()?
Update implies a grant, so I'd rather use grant to avoid having two methods that do the same.
> On Aug. 8, 2018, 7:25 p.m., Na Li wrote:
> > sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyStoreProcessor.java
> > Lines 1582 (patched)
> > <https://reviews.apache.org/r/68268/diff/1/?file=2070542#file2070542line1646>
> >
> > should this be audit.onUpdateOwnerPrivilege()?
Update implies a grant, so I'd rather use grant to avoid having two methods that do the same.
- Sergio
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68268/#review206991
-----------------------------------------------------------
On Aug. 8, 2018, 2:29 p.m., Sergio Pena wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68268/
> -----------------------------------------------------------
>
> (Updated Aug. 8, 2018, 2:29 p.m.)
>
>
> Review request for sentry, Arjun Mishra, kalyan kumar kalvagadda, and Na Li.
>
>
> Bugs: sentry-2157
> https://issues.apache.org/jira/browse/sentry-2157
>
>
> Repository: sentry
>
>
> Description
> -------
>
> This patch logs owner privileges grants and revokes.
>
>
> Diffs
> -----
>
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyStoreProcessor.java 61f9168b1970144dbf0b7a7378f2d25e70f1761d
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/audit/SentryAuditLogger.java PRE-CREATION
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/log/entity/JsonLogEntityFactory.java 61becceac881443b02182e6ab1012add4c046499
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/log/util/CommandUtil.java 6479a6055e8c7087f0e484080ec9d46a9c146212
> sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/log/entity/TestJsonLogEntityFactory.java 307f38eadb65bf12dc6225cfe43a5d590657d055
>
>
> Diff: https://reviews.apache.org/r/68268/diff/1/
>
>
> Testing
> -------
>
> I run the patch in a cluster and the audit logs is displaying the correct messages.
>
> {"serviceName":"Sentry-Service","userName":"ubuntu","impersonator":"","ipAddress":"/127.0.0.1","operation":"GRANT_PRIVILEGE","eventTime":"1533738512795","operationText":"OWNER privilege granted to USER: sergio","allowed":"true","databaseName":"default","tableName":"t2","column":null,"resourcePath":null,"objectType":"PRINCIPAL"}
>
>
> Thanks,
>
> Sergio Pena
>
>
Re: Review Request 68268: SENTRY-2157: Update audit log to
grant/revoke owner privileges
Posted by Na Li via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68268/#review206991
-----------------------------------------------------------
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyStoreProcessor.java
Line 1396 (original), 1336 (patched)
<https://reviews.apache.org/r/68268/#comment290151>
do we generate audit message when owner privilege is removed in notification processor?
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyStoreProcessor.java
Lines 1577 (patched)
<https://reviews.apache.org/r/68268/#comment290152>
should this be audit.onUpdateOwnerPrivilege()?
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyStoreProcessor.java
Lines 1582 (patched)
<https://reviews.apache.org/r/68268/#comment290153>
should this be audit.onUpdateOwnerPrivilege()?
- Na Li
On Aug. 8, 2018, 2:29 p.m., Sergio Pena wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68268/
> -----------------------------------------------------------
>
> (Updated Aug. 8, 2018, 2:29 p.m.)
>
>
> Review request for sentry, Arjun Mishra, kalyan kumar kalvagadda, and Na Li.
>
>
> Bugs: sentry-2157
> https://issues.apache.org/jira/browse/sentry-2157
>
>
> Repository: sentry
>
>
> Description
> -------
>
> This patch logs owner privileges grants and revokes.
>
>
> Diffs
> -----
>
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyStoreProcessor.java 61f9168b1970144dbf0b7a7378f2d25e70f1761d
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/audit/SentryAuditLogger.java PRE-CREATION
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/log/entity/JsonLogEntityFactory.java 61becceac881443b02182e6ab1012add4c046499
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/log/util/CommandUtil.java 6479a6055e8c7087f0e484080ec9d46a9c146212
> sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/log/entity/TestJsonLogEntityFactory.java 307f38eadb65bf12dc6225cfe43a5d590657d055
>
>
> Diff: https://reviews.apache.org/r/68268/diff/1/
>
>
> Testing
> -------
>
> I run the patch in a cluster and the audit logs is displaying the correct messages.
>
> {"serviceName":"Sentry-Service","userName":"ubuntu","impersonator":"","ipAddress":"/127.0.0.1","operation":"GRANT_PRIVILEGE","eventTime":"1533738512795","operationText":"OWNER privilege granted to USER: sergio","allowed":"true","databaseName":"default","tableName":"t2","column":null,"resourcePath":null,"objectType":"PRINCIPAL"}
>
>
> Thanks,
>
> Sergio Pena
>
>
Re: Review Request 68268: SENTRY-2157: Update audit log to
grant/revoke owner privileges
Posted by Na Li via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68268/#review207006
-----------------------------------------------------------
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/log/entity/JsonLogEntityFactory.java
Lines 181 (patched)
<https://reviews.apache.org/r/68268/#comment290168>
It would be better to throw exception for other value, so people calls this function with other value can add meaningful operation text.
- Na Li
On Aug. 8, 2018, 10:36 p.m., Sergio Pena wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68268/
> -----------------------------------------------------------
>
> (Updated Aug. 8, 2018, 10:36 p.m.)
>
>
> Review request for sentry, Arjun Mishra, kalyan kumar kalvagadda, and Na Li.
>
>
> Bugs: sentry-2157
> https://issues.apache.org/jira/browse/sentry-2157
>
>
> Repository: sentry
>
>
> Description
> -------
>
> This patch logs owner privileges grants and revokes.
>
>
> Diffs
> -----
>
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyStoreProcessor.java 61f9168b1970144dbf0b7a7378f2d25e70f1761d
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/audit/SentryAuditLogger.java PRE-CREATION
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/log/entity/JsonLogEntityFactory.java 61becceac881443b02182e6ab1012add4c046499
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/log/util/CommandUtil.java 6479a6055e8c7087f0e484080ec9d46a9c146212
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/log/util/Constants.java 6e91f8b9ead009629d6bccd206122b2071e8fd64
> sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/log/entity/TestJsonLogEntityFactory.java 307f38eadb65bf12dc6225cfe43a5d590657d055
>
>
> Diff: https://reviews.apache.org/r/68268/diff/3/
>
>
> Testing
> -------
>
> I run the patch in a cluster and the audit logs is displaying the correct messages.
>
> {"serviceName":"Sentry-Service","userName":"ubuntu","impersonator":"","ipAddress":"/127.0.0.1","operation":"GRANT_PRIVILEGE","eventTime":"1533738512795","operationText":"OWNER privilege granted to USER: sergio","allowed":"true","databaseName":"default","tableName":"t2","column":null,"resourcePath":null,"objectType":"PRINCIPAL"}
>
>
> Thanks,
>
> Sergio Pena
>
>
Re: Review Request 68268: SENTRY-2157: Update audit log to
grant/revoke owner privileges
Posted by Na Li via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68268/#review207005
-----------------------------------------------------------
Ship it!
Ship It!
- Na Li
On Aug. 8, 2018, 10:36 p.m., Sergio Pena wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68268/
> -----------------------------------------------------------
>
> (Updated Aug. 8, 2018, 10:36 p.m.)
>
>
> Review request for sentry, Arjun Mishra, kalyan kumar kalvagadda, and Na Li.
>
>
> Bugs: sentry-2157
> https://issues.apache.org/jira/browse/sentry-2157
>
>
> Repository: sentry
>
>
> Description
> -------
>
> This patch logs owner privileges grants and revokes.
>
>
> Diffs
> -----
>
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyStoreProcessor.java 61f9168b1970144dbf0b7a7378f2d25e70f1761d
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/audit/SentryAuditLogger.java PRE-CREATION
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/log/entity/JsonLogEntityFactory.java 61becceac881443b02182e6ab1012add4c046499
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/log/util/CommandUtil.java 6479a6055e8c7087f0e484080ec9d46a9c146212
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/log/util/Constants.java 6e91f8b9ead009629d6bccd206122b2071e8fd64
> sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/log/entity/TestJsonLogEntityFactory.java 307f38eadb65bf12dc6225cfe43a5d590657d055
>
>
> Diff: https://reviews.apache.org/r/68268/diff/3/
>
>
> Testing
> -------
>
> I run the patch in a cluster and the audit logs is displaying the correct messages.
>
> {"serviceName":"Sentry-Service","userName":"ubuntu","impersonator":"","ipAddress":"/127.0.0.1","operation":"GRANT_PRIVILEGE","eventTime":"1533738512795","operationText":"OWNER privilege granted to USER: sergio","allowed":"true","databaseName":"default","tableName":"t2","column":null,"resourcePath":null,"objectType":"PRINCIPAL"}
>
>
> Thanks,
>
> Sergio Pena
>
>
Re: Review Request 68268: SENTRY-2157: Update audit log to
grant/revoke owner privileges
Posted by Na Li via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68268/#review207004
-----------------------------------------------------------
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/log/entity/JsonLogEntityFactory.java
Line 179 (original), 179 (patched)
<https://reviews.apache.org/r/68268/#comment290167>
should you throw exception for other value? So if someone calls this function for other reason, they can update this function to add meaningful operation text
- Na Li
On Aug. 8, 2018, 10:36 p.m., Sergio Pena wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68268/
> -----------------------------------------------------------
>
> (Updated Aug. 8, 2018, 10:36 p.m.)
>
>
> Review request for sentry, Arjun Mishra, kalyan kumar kalvagadda, and Na Li.
>
>
> Bugs: sentry-2157
> https://issues.apache.org/jira/browse/sentry-2157
>
>
> Repository: sentry
>
>
> Description
> -------
>
> This patch logs owner privileges grants and revokes.
>
>
> Diffs
> -----
>
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyStoreProcessor.java 61f9168b1970144dbf0b7a7378f2d25e70f1761d
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/audit/SentryAuditLogger.java PRE-CREATION
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/log/entity/JsonLogEntityFactory.java 61becceac881443b02182e6ab1012add4c046499
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/log/util/CommandUtil.java 6479a6055e8c7087f0e484080ec9d46a9c146212
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/log/util/Constants.java 6e91f8b9ead009629d6bccd206122b2071e8fd64
> sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/log/entity/TestJsonLogEntityFactory.java 307f38eadb65bf12dc6225cfe43a5d590657d055
>
>
> Diff: https://reviews.apache.org/r/68268/diff/3/
>
>
> Testing
> -------
>
> I run the patch in a cluster and the audit logs is displaying the correct messages.
>
> {"serviceName":"Sentry-Service","userName":"ubuntu","impersonator":"","ipAddress":"/127.0.0.1","operation":"GRANT_PRIVILEGE","eventTime":"1533738512795","operationText":"OWNER privilege granted to USER: sergio","allowed":"true","databaseName":"default","tableName":"t2","column":null,"resourcePath":null,"objectType":"PRINCIPAL"}
>
>
> Thanks,
>
> Sergio Pena
>
>
Re: Review Request 68268: SENTRY-2157: Update audit log to
grant/revoke owner privileges
Posted by Sergio Pena via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68268/
-----------------------------------------------------------
(Updated Aug. 8, 2018, 10:36 p.m.)
Review request for sentry, Arjun Mishra, kalyan kumar kalvagadda, and Na Li.
Bugs: sentry-2157
https://issues.apache.org/jira/browse/sentry-2157
Repository: sentry
Description
-------
This patch logs owner privileges grants and revokes.
Diffs (updated)
-----
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyStoreProcessor.java 61f9168b1970144dbf0b7a7378f2d25e70f1761d
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/audit/SentryAuditLogger.java PRE-CREATION
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/log/entity/JsonLogEntityFactory.java 61becceac881443b02182e6ab1012add4c046499
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/log/util/CommandUtil.java 6479a6055e8c7087f0e484080ec9d46a9c146212
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/log/util/Constants.java 6e91f8b9ead009629d6bccd206122b2071e8fd64
sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/log/entity/TestJsonLogEntityFactory.java 307f38eadb65bf12dc6225cfe43a5d590657d055
Diff: https://reviews.apache.org/r/68268/diff/3/
Changes: https://reviews.apache.org/r/68268/diff/2-3/
Testing
-------
I run the patch in a cluster and the audit logs is displaying the correct messages.
{"serviceName":"Sentry-Service","userName":"ubuntu","impersonator":"","ipAddress":"/127.0.0.1","operation":"GRANT_PRIVILEGE","eventTime":"1533738512795","operationText":"OWNER privilege granted to USER: sergio","allowed":"true","databaseName":"default","tableName":"t2","column":null,"resourcePath":null,"objectType":"PRINCIPAL"}
Thanks,
Sergio Pena
Re: Review Request 68268: SENTRY-2157: Update audit log to
grant/revoke owner privileges
Posted by Sergio Pena via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68268/
-----------------------------------------------------------
(Updated Aug. 8, 2018, 10:10 p.m.)
Review request for sentry, Arjun Mishra, kalyan kumar kalvagadda, and Na Li.
Changes
-------
The new output shows:
{"serviceName":"Sentry-Service","userName":"ubuntu","impersonator":"","ipAddress":"/127.0.0.1","operation":"GRANT_OWNER_PRIVILEGE","eventTime":"1533765994895","operationText":"OWNER privilege on table 'db2.t1'
is granted to USER: sergio","allowed":"true","databaseName":"db2","tableName":"t1","column":null,"resourcePath":null,"objectType":"PRINCIPAL"}
If a transferred happens, then the message will display 'OWNER PRIVILEGE on table db2.t1 is transffered to USER: sergio'
Bugs: sentry-2157
https://issues.apache.org/jira/browse/sentry-2157
Repository: sentry
Description
-------
This patch logs owner privileges grants and revokes.
Diffs (updated)
-----
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/SentryHmsEvent.java ef63a34df1c90a08fc397a22454b6be176bae6cf
sentry-binding/sentry-binding-hive/src/test/java/org/apache/sentry/binding/metastore/TestSentrySyncHMSNotificationsPostEventListener.java e261fc6f66e29174c13940c70bdcf0caaa5290fa
sentry-core/sentry-core-common/src/main/java/org/apache/sentry/SentryOwnerInfo.java ee4fce56ec18092dca49036263e7f2590ba9fb66
sentry-core/sentry-core-common/src/main/java/org/apache/sentry/service/common/ServiceConstants.java c1beaed0a6c76ae6cc60bde241b1aa2deae030a8
sentry-hdfs/sentry-hdfs-common/src/gen/thrift/gen-javabean/org/apache/sentry/hdfs/service/thrift/TPrivilegeChanges.java abcf3ca5d813c8ae04413111d3decdbf587c693d
sentry-hdfs/sentry-hdfs-common/src/gen/thrift/gen-javabean/org/apache/sentry/hdfs/service/thrift/TPrivilegeEntity.java 85f81475192c58535700a0ca078bfbe25ac1fac4
sentry-hdfs/sentry-hdfs-common/src/gen/thrift/gen-javabean/org/apache/sentry/hdfs/service/thrift/TPrivilegeEntityType.java ac44c1f91396f96d6dec52912f3fade089eeb845
sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/PermissionsUpdate.java 5691933fe85688f4f064c8c25e181c2857b1e6f3
sentry-hdfs/sentry-hdfs-common/src/main/resources/sentry_hdfs_service.thrift 61582cd0eb9a8c096d84fa97e0d7639605f6265e
sentry-hdfs/sentry-hdfs-common/src/test/java/org/apache/sentry/hdfs/TestPermissionUpdate.java 8bd9d439bcca7bdb6b94375a21b98388e2dcdb99
sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryPermissions.java c162ec19cbc40fc2a8dc6192f5af2db82e90336e
sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/UpdateableAuthzPermissions.java 761c760f2f982e2ce580a1e0af0295d735d9fb5b
sentry-hdfs/sentry-hdfs-namenode-plugin/src/test/java/org/apache/sentry/hdfs/TestSentryPermissions.java f0ca787bb0c98c0ecbfebe4f8bba1a0fb499eec5
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/DBUpdateForwarder.java d47b9f73ee92de49bd785e9587ce47df5b2d3a80
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/PermImageRetriever.java 2a675041811ece7147fdd3c1bef767ac27113a2c
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryPlugin.java 6cb787bb7f61f0ee3e536d073757ba8758ae8fa6
sentry-hdfs/sentry-hdfs-service/src/test/java/org/apache/sentry/hdfs/TestDeltaRetriever.java d7bc748345638c49f341404b356d8a1944c2612f
sentry-hdfs/sentry-hdfs-service/src/test/java/org/apache/sentry/hdfs/TestImageRetriever.java b86136d6fe857aa1a87214550276f7b6b0800de9
sentry-hdfs/sentry-hdfs-service/src/test/java/org/apache/sentry/hdfs/TestSentryHDFSServiceProcessor.java 845c1377d3756fe2df794d79a5fcb3d0e9f2eb31
sentry-service/sentry-service-api/src/gen/thrift/gen-javabean/org/apache/sentry/api/service/thrift/TListSentryPrivilegesRequest.java 01e52300e2eba72aa4f7fa3e9ecab77aa0737d4b
sentry-service/sentry-service-api/src/gen/thrift/gen-javabean/org/apache/sentry/api/service/thrift/TSentryHmsEventNotification.java fe891252eec81b28651106ea73a9480d42d5a0b7
sentry-service/sentry-service-api/src/gen/thrift/gen-javabean/org/apache/sentry/api/service/thrift/TSentryObjectOwnerType.java 6b540b88241abf4cb87246082b481f2b4c6fdca2
sentry-service/sentry-service-api/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyServiceClient.java 1a8034b10f381395ca93c81cd432505d5fdcb5c8
sentry-service/sentry-service-api/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyServiceClientDefaultImpl.java 07da2ba3caf9f5e5f1191ab7d3843ab4fd5dde33
sentry-service/sentry-service-api/src/main/resources/sentry_policy_service.thrift f238748ccc5c1f1b6157ec04a0b1d3211f997ccf
sentry-service/sentry-service-api/src/test/java/org/apache/sentry/api/service/thrift/TestSentryPolicyServiceClientDefaultImpl.java c7145848a9e280d9267fbd5f890739853f5956f7
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyStoreProcessor.java 2efc8cf9c1380a063c54d6bf4ef83e9d0fa8ebc9
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/audit/SentryAuditLogger.java PRE-CREATION
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/log/entity/JsonLogEntityFactory.java 61becceac881443b02182e6ab1012add4c046499
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/log/util/CommandUtil.java 6479a6055e8c7087f0e484080ec9d46a9c146212
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/log/util/Constants.java 6e91f8b9ead009629d6bccd206122b2071e8fd64
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPrivilege.java e5eb4c403bbb278c43ec0ccd77f663676c319040
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/model/MSentryRole.java 74213afe2f30ad6db67598ff17e74364e05a05f4
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/model/MSentryUser.java 6e44c79df6de4cdc2ab83045fb2f9d54705fcc4c
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/NotificationProcessor.java 01899bf39f05c598c3bf76398fbc3ddf6e45cd3a
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/PermissionsImage.java 4a02db2a6074f086e8676093a2cb9348cc6b0f7a
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/PrivilegeEntity.java 3f7ba9783f364e84bde0ace2a5666d570866908c
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java 4f7bdbde1270580faab13eeadc636c34c2bf267c
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStoreInterface.java 44431486d639c00a860d9cafd60aef3f4c9e5d35
sentry-service/sentry-service-server/src/test/java/org/apache/sentry/api/service/thrift/TestSentryPolicyStoreProcessor.java ee9442b7f198c57ac3167e4a4f31613a327b45a1
sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/generic/service/persistent/TestSentryRole.java 3b58de37d0949b15a6986931ddc98a7f98a59c7a
sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/log/entity/TestJsonLogEntityFactory.java 307f38eadb65bf12dc6225cfe43a5d590657d055
sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestHMSFollowerSentryStoreIntegration.java 12efb2d9515c994a35c53186c2617f6f3a32a425
sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 07a5069240608b17586678fe85441abcd57703a6
Diff: https://reviews.apache.org/r/68268/diff/2/
Changes: https://reviews.apache.org/r/68268/diff/1-2/
Testing
-------
I run the patch in a cluster and the audit logs is displaying the correct messages.
{"serviceName":"Sentry-Service","userName":"ubuntu","impersonator":"","ipAddress":"/127.0.0.1","operation":"GRANT_PRIVILEGE","eventTime":"1533738512795","operationText":"OWNER privilege granted to USER: sergio","allowed":"true","databaseName":"default","tableName":"t2","column":null,"resourcePath":null,"objectType":"PRINCIPAL"}
Thanks,
Sergio Pena
Re: Review Request 68268: SENTRY-2157: Update audit log to
grant/revoke owner privileges
Posted by Sergio Pena via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68268/
-----------------------------------------------------------
(Updated Aug. 8, 2018, 2:29 p.m.)
Review request for sentry, Arjun Mishra, kalyan kumar kalvagadda, and Na Li.
Bugs: sentry-2157
https://issues.apache.org/jira/browse/sentry-2157
Repository: sentry
Description
-------
This patch logs owner privileges grants and revokes.
Diffs
-----
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyStoreProcessor.java 61f9168b1970144dbf0b7a7378f2d25e70f1761d
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/audit/SentryAuditLogger.java PRE-CREATION
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/log/entity/JsonLogEntityFactory.java 61becceac881443b02182e6ab1012add4c046499
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/log/util/CommandUtil.java 6479a6055e8c7087f0e484080ec9d46a9c146212
sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/log/entity/TestJsonLogEntityFactory.java 307f38eadb65bf12dc6225cfe43a5d590657d055
Diff: https://reviews.apache.org/r/68268/diff/1/
Testing (updated)
-------
I run the patch in a cluster and the audit logs is displaying the correct messages.
{"serviceName":"Sentry-Service","userName":"ubuntu","impersonator":"","ipAddress":"/127.0.0.1","operation":"GRANT_PRIVILEGE","eventTime":"1533738512795","operationText":"OWNER privilege granted to USER: sergio","allowed":"true","databaseName":"default","tableName":"t2","column":null,"resourcePath":null,"objectType":"PRINCIPAL"}
Thanks,
Sergio Pena