You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ranger.apache.org by Abhay Kulkarni <ak...@hortonworks.com> on 2016/04/15 01:45:38 UTC

Review Request 46236: x_service table columns that track policy/tag updates to be moved to a new table

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46236/
-----------------------------------------------------------

Review request for ranger and Madhan Neethiraj.


Bugs: RANGER-901
    https://issues.apache.org/jira/browse/RANGER-901


Repository: ranger


Description
-------

x_service table columns that maintain version-numbers of contained policies and tag-updates to related service-resource objects are moved to a separate database table.


Diffs
-----

  security-admin/db/mysql/patches/022-split-service-table.sql PRE-CREATION 
  security-admin/db/postgres/patches/022-split-service-table.sql PRE-CREATION 
  security-admin/src/main/java/org/apache/ranger/biz/ServiceDBStore.java 9d2e4c6 
  security-admin/src/main/java/org/apache/ranger/biz/TagDBStore.java e11dad6 
  security-admin/src/main/java/org/apache/ranger/common/AppConstants.java 3851069 
  security-admin/src/main/java/org/apache/ranger/db/RangerDaoManagerBase.java 6559850 
  security-admin/src/main/java/org/apache/ranger/db/XXServiceDao.java 54c48dc 
  security-admin/src/main/java/org/apache/ranger/db/XXServiceVersionInfoDao.java PRE-CREATION 
  security-admin/src/main/java/org/apache/ranger/entity/XXServiceVersionInfo.java PRE-CREATION 
  security-admin/src/main/java/org/apache/ranger/patch/PatchForServiceVersionInfo_J10004.java PRE-CREATION 
  security-admin/src/main/java/org/apache/ranger/service/RangerServiceResourceService.java 807ad61 
  security-admin/src/main/java/org/apache/ranger/service/RangerServiceService.java a8f54f6 
  security-admin/src/main/java/org/apache/ranger/service/RangerServiceServiceBase.java 594d091 
  security-admin/src/main/java/org/apache/ranger/service/RangerTagDefService.java beb6295 
  security-admin/src/main/java/org/apache/ranger/service/RangerTagResourceMapService.java 257821d 
  security-admin/src/main/java/org/apache/ranger/service/RangerTagService.java 34ed0ad 
  security-admin/src/main/resources/META-INF/jpa_named_queries.xml 0f65243 
  security-admin/src/test/java/org/apache/ranger/biz/TestServiceDBStore.java 61e13da 
  security-admin/src/test/java/org/apache/ranger/service/TestRangerServiceService.java 215506e 
  security-admin/src/test/java/org/apache/ranger/service/TestRangerServiceServiceBase.java 5dff936 

Diff: https://reviews.apache.org/r/46236/diff/


Testing
-------

Tested Service creation/deletion and update scenarios.
Tested ServiceResource create/delete and update scenarios.
Tested Policy create/delete/update scenario.
Tested upgrade scenario.


Thanks,

Abhay Kulkarni


Re: Review Request 46236: x_service table columns that track policy/tag updates to be moved to a new table

Posted by Madhan Neethiraj <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46236/#review129943
-----------------------------------------------------------


Ship it!




Ship It!

- Madhan Neethiraj


On April 20, 2016, 8:27 p.m., Abhay Kulkarni wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46236/
> -----------------------------------------------------------
> 
> (Updated April 20, 2016, 8:27 p.m.)
> 
> 
> Review request for ranger and Madhan Neethiraj.
> 
> 
> Bugs: RANGER-901
>     https://issues.apache.org/jira/browse/RANGER-901
> 
> 
> Repository: ranger
> 
> 
> Description
> -------
> 
> x_service table columns that maintain version-numbers of contained policies and tag-updates to related service-resource objects are moved to a separate database table.
> 
> 
> Diffs
> -----
> 
>   agents-common/src/main/java/org/apache/ranger/plugin/contextenricher/RangerTagEnricher.java 9d3d201 
>   security-admin/db/mysql/patches/022-split-service-table.sql PRE-CREATION 
>   security-admin/db/oracle/patches/022-split-service-table.sql PRE-CREATION 
>   security-admin/db/postgres/patches/022-split-service-table.sql PRE-CREATION 
>   security-admin/db/sqlanywhere/patches/022-split-service-table.sql PRE-CREATION 
>   security-admin/db/sqlserver/patches/022-split-service-table.sql PRE-CREATION 
>   security-admin/src/main/java/org/apache/ranger/biz/ServiceDBStore.java 21ed686 
>   security-admin/src/main/java/org/apache/ranger/biz/TagDBStore.java e11dad6 
>   security-admin/src/main/java/org/apache/ranger/common/AppConstants.java 3851069 
>   security-admin/src/main/java/org/apache/ranger/db/RangerDaoManagerBase.java 6559850 
>   security-admin/src/main/java/org/apache/ranger/db/XXServiceDao.java 54c48dc 
>   security-admin/src/main/java/org/apache/ranger/db/XXServiceVersionInfoDao.java PRE-CREATION 
>   security-admin/src/main/java/org/apache/ranger/entity/XXServiceVersionInfo.java PRE-CREATION 
>   security-admin/src/main/java/org/apache/ranger/patch/PatchForServiceVersionInfo_J10004.java PRE-CREATION 
>   security-admin/src/main/java/org/apache/ranger/service/RangerServiceResourceService.java 807ad61 
>   security-admin/src/main/java/org/apache/ranger/service/RangerServiceService.java a8f54f6 
>   security-admin/src/main/java/org/apache/ranger/service/RangerServiceServiceBase.java 594d091 
>   security-admin/src/main/java/org/apache/ranger/service/RangerTagDefService.java beb6295 
>   security-admin/src/main/java/org/apache/ranger/service/RangerTagResourceMapService.java 257821d 
>   security-admin/src/main/java/org/apache/ranger/service/RangerTagService.java 34ed0ad 
>   security-admin/src/main/resources/META-INF/jpa_named_queries.xml 0f65243 
>   security-admin/src/test/java/org/apache/ranger/biz/TestServiceDBStore.java 61e13da 
>   security-admin/src/test/java/org/apache/ranger/service/TestRangerServiceService.java 215506e 
>   security-admin/src/test/java/org/apache/ranger/service/TestRangerServiceServiceBase.java 5dff936 
> 
> Diff: https://reviews.apache.org/r/46236/diff/
> 
> 
> Testing
> -------
> 
> Tested Service creation/deletion and update scenarios.
> Tested ServiceResource create/delete and update scenarios.
> Tested Policy create/delete/update scenario.
> Tested upgrade scenario.
> 
> 
> Thanks,
> 
> Abhay Kulkarni
> 
>


Re: Review Request 46236: x_service table columns that track policy/tag updates to be moved to a new table

Posted by Abhay Kulkarni <ak...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46236/
-----------------------------------------------------------

(Updated April 20, 2016, 8:27 p.m.)


Review request for ranger and Madhan Neethiraj.


Changes
-------

Addressed review comments


Bugs: RANGER-901
    https://issues.apache.org/jira/browse/RANGER-901


Repository: ranger


Description
-------

x_service table columns that maintain version-numbers of contained policies and tag-updates to related service-resource objects are moved to a separate database table.


Diffs (updated)
-----

  agents-common/src/main/java/org/apache/ranger/plugin/contextenricher/RangerTagEnricher.java 9d3d201 
  security-admin/db/mysql/patches/022-split-service-table.sql PRE-CREATION 
  security-admin/db/oracle/patches/022-split-service-table.sql PRE-CREATION 
  security-admin/db/postgres/patches/022-split-service-table.sql PRE-CREATION 
  security-admin/db/sqlanywhere/patches/022-split-service-table.sql PRE-CREATION 
  security-admin/db/sqlserver/patches/022-split-service-table.sql PRE-CREATION 
  security-admin/src/main/java/org/apache/ranger/biz/ServiceDBStore.java 21ed686 
  security-admin/src/main/java/org/apache/ranger/biz/TagDBStore.java e11dad6 
  security-admin/src/main/java/org/apache/ranger/common/AppConstants.java 3851069 
  security-admin/src/main/java/org/apache/ranger/db/RangerDaoManagerBase.java 6559850 
  security-admin/src/main/java/org/apache/ranger/db/XXServiceDao.java 54c48dc 
  security-admin/src/main/java/org/apache/ranger/db/XXServiceVersionInfoDao.java PRE-CREATION 
  security-admin/src/main/java/org/apache/ranger/entity/XXServiceVersionInfo.java PRE-CREATION 
  security-admin/src/main/java/org/apache/ranger/patch/PatchForServiceVersionInfo_J10004.java PRE-CREATION 
  security-admin/src/main/java/org/apache/ranger/service/RangerServiceResourceService.java 807ad61 
  security-admin/src/main/java/org/apache/ranger/service/RangerServiceService.java a8f54f6 
  security-admin/src/main/java/org/apache/ranger/service/RangerServiceServiceBase.java 594d091 
  security-admin/src/main/java/org/apache/ranger/service/RangerTagDefService.java beb6295 
  security-admin/src/main/java/org/apache/ranger/service/RangerTagResourceMapService.java 257821d 
  security-admin/src/main/java/org/apache/ranger/service/RangerTagService.java 34ed0ad 
  security-admin/src/main/resources/META-INF/jpa_named_queries.xml 0f65243 
  security-admin/src/test/java/org/apache/ranger/biz/TestServiceDBStore.java 61e13da 
  security-admin/src/test/java/org/apache/ranger/service/TestRangerServiceService.java 215506e 
  security-admin/src/test/java/org/apache/ranger/service/TestRangerServiceServiceBase.java 5dff936 

Diff: https://reviews.apache.org/r/46236/diff/


Testing
-------

Tested Service creation/deletion and update scenarios.
Tested ServiceResource create/delete and update scenarios.
Tested Policy create/delete/update scenario.
Tested upgrade scenario.


Thanks,

Abhay Kulkarni


Re: Review Request 46236: x_service table columns that track policy/tag updates to be moved to a new table

Posted by Madhan Neethiraj <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46236/#review129266
-----------------------------------------------------------




security-admin/src/main/java/org/apache/ranger/biz/ServiceDBStore.java (line 2045)
<https://reviews.apache.org/r/46236/#comment192725>

    It will be safe to treate this case as "serviceDbObj.getPolicyVersion() == null" in the earlier revision.



security-admin/src/main/java/org/apache/ranger/biz/ServiceDBStore.java (line 2094)
<https://reviews.apache.org/r/46236/#comment192726>

    Consider not treating this as an error condition; instead simply set policyVersion and policyUpdateTime to null.



security-admin/src/main/java/org/apache/ranger/biz/ServiceDBStore.java (line 2119)
<https://reviews.apache.org/r/46236/#comment192727>

    Consider not treating this as an error condition; instead simply set policyVersion and policyUpdateTime to null.



security-admin/src/main/java/org/apache/ranger/biz/ServiceDBStore.java (line 2382)
<https://reviews.apache.org/r/46236/#comment192728>

    Consider creating a XXServiceVersionInfo in the database, instead of bailing out.



security-admin/src/main/java/org/apache/ranger/biz/ServiceDBStore.java (line 2402)
<https://reviews.apache.org/r/46236/#comment192729>

    Consider creating a XXServiceVersionInfo in the database when serviceVersionInfoDbObj==null.



security-admin/src/main/java/org/apache/ranger/biz/ServiceDBStore.java (line 2751)
<https://reviews.apache.org/r/46236/#comment192730>

    Consider creating a XXServiceVersionInfo in the database when serviceVersionInfo==null.



security-admin/src/main/java/org/apache/ranger/biz/ServiceDBStore.java (line 2764)
<https://reviews.apache.org/r/46236/#comment192731>

    Consider creating a XXServiceVersionInfo in the database when serviceVersionInfo==null.



security-admin/src/main/java/org/apache/ranger/biz/TagDBStore.java (line 916)
<https://reviews.apache.org/r/46236/#comment192732>

    Consider treating this as "xxService.getTagVersion == null" in the earlier version, instead of an error condition.



security-admin/src/main/java/org/apache/ranger/biz/TagDBStore.java (line 965)
<https://reviews.apache.org/r/46236/#comment192733>

    Instead of bailing out in this case, simply set tagVersion and tagUpdateTime fields to null in the return value.



security-admin/src/main/java/org/apache/ranger/db/XXServiceVersionInfoDao.java (line 62)
<https://reviews.apache.org/r/46236/#comment192740>

    Is this method used? Please review and remove if not used.



security-admin/src/main/java/org/apache/ranger/entity/XXServiceVersionInfo.java (line 157)
<https://reviews.apache.org/r/46236/#comment192734>

    - ensure that obj is of type XXServiceVersionInfo
    - ensure that obj is not null



security-admin/src/main/java/org/apache/ranger/patch/PatchForServiceVersionInfo_J10004.java (line 81)
<https://reviews.apache.org/r/46236/#comment192735>

    Before creating, check if an entry already exists. If it exists, simply update the fields.



security-admin/src/main/java/org/apache/ranger/patch/PatchForServiceVersionInfo_J10004.java (line 84)
<https://reviews.apache.org/r/46236/#comment192736>

    Please add a comment on the need for updating policy/tag version fields in XService object.



security-admin/src/main/java/org/apache/ranger/patch/PatchForServiceVersionInfo_J10004.java (line 85)
<https://reviews.apache.org/r/46236/#comment192737>

    wouldn't getTagVersion() be null? If yes, this needs to be handled similar to the previous line.



security-admin/src/main/java/org/apache/ranger/service/RangerServiceServiceBase.java (line 115)
<https://reviews.apache.org/r/46236/#comment192738>

    This "TO DO" can now be removed, as this condition is handled below.



security-admin/src/main/resources/META-INF/jpa_named_queries.xml (line 413)
<https://reviews.apache.org/r/46236/#comment192739>

    XXService.findByTagId and few other queries may not be used anymore, as they they are moved to XXServiceVersionInfo. Please review and remove such queries.


- Madhan Neethiraj


On April 15, 2016, 6:30 p.m., Abhay Kulkarni wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46236/
> -----------------------------------------------------------
> 
> (Updated April 15, 2016, 6:30 p.m.)
> 
> 
> Review request for ranger and Madhan Neethiraj.
> 
> 
> Bugs: RANGER-901
>     https://issues.apache.org/jira/browse/RANGER-901
> 
> 
> Repository: ranger
> 
> 
> Description
> -------
> 
> x_service table columns that maintain version-numbers of contained policies and tag-updates to related service-resource objects are moved to a separate database table.
> 
> 
> Diffs
> -----
> 
>   security-admin/db/mysql/patches/022-split-service-table.sql PRE-CREATION 
>   security-admin/db/oracle/patches/022-split-service-table.sql PRE-CREATION 
>   security-admin/db/postgres/patches/022-split-service-table.sql PRE-CREATION 
>   security-admin/db/sqlanywhere/patches/022-split-service-table.sql PRE-CREATION 
>   security-admin/db/sqlserver/patches/022-split-service-table.sql PRE-CREATION 
>   security-admin/src/main/java/org/apache/ranger/biz/ServiceDBStore.java 21ed686 
>   security-admin/src/main/java/org/apache/ranger/biz/TagDBStore.java e11dad6 
>   security-admin/src/main/java/org/apache/ranger/common/AppConstants.java 3851069 
>   security-admin/src/main/java/org/apache/ranger/db/RangerDaoManagerBase.java 6559850 
>   security-admin/src/main/java/org/apache/ranger/db/XXServiceDao.java 54c48dc 
>   security-admin/src/main/java/org/apache/ranger/db/XXServiceVersionInfoDao.java PRE-CREATION 
>   security-admin/src/main/java/org/apache/ranger/entity/XXServiceVersionInfo.java PRE-CREATION 
>   security-admin/src/main/java/org/apache/ranger/patch/PatchForServiceVersionInfo_J10004.java PRE-CREATION 
>   security-admin/src/main/java/org/apache/ranger/service/RangerServiceResourceService.java 807ad61 
>   security-admin/src/main/java/org/apache/ranger/service/RangerServiceService.java a8f54f6 
>   security-admin/src/main/java/org/apache/ranger/service/RangerServiceServiceBase.java 594d091 
>   security-admin/src/main/java/org/apache/ranger/service/RangerTagDefService.java beb6295 
>   security-admin/src/main/java/org/apache/ranger/service/RangerTagResourceMapService.java 257821d 
>   security-admin/src/main/java/org/apache/ranger/service/RangerTagService.java 34ed0ad 
>   security-admin/src/main/resources/META-INF/jpa_named_queries.xml 0f65243 
>   security-admin/src/test/java/org/apache/ranger/biz/TestServiceDBStore.java 61e13da 
>   security-admin/src/test/java/org/apache/ranger/service/TestRangerServiceService.java 215506e 
>   security-admin/src/test/java/org/apache/ranger/service/TestRangerServiceServiceBase.java 5dff936 
> 
> Diff: https://reviews.apache.org/r/46236/diff/
> 
> 
> Testing
> -------
> 
> Tested Service creation/deletion and update scenarios.
> Tested ServiceResource create/delete and update scenarios.
> Tested Policy create/delete/update scenario.
> Tested upgrade scenario.
> 
> 
> Thanks,
> 
> Abhay Kulkarni
> 
>


Re: Review Request 46236: x_service table columns that track policy/tag updates to be moved to a new table

Posted by Abhay Kulkarni <ak...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46236/
-----------------------------------------------------------

(Updated April 15, 2016, 6:30 p.m.)


Review request for ranger and Madhan Neethiraj.


Changes
-------

Added database table creation scripts for Oracle, SqlAnywhere and SQLServer.


Bugs: RANGER-901
    https://issues.apache.org/jira/browse/RANGER-901


Repository: ranger


Description
-------

x_service table columns that maintain version-numbers of contained policies and tag-updates to related service-resource objects are moved to a separate database table.


Diffs (updated)
-----

  security-admin/db/mysql/patches/022-split-service-table.sql PRE-CREATION 
  security-admin/db/oracle/patches/022-split-service-table.sql PRE-CREATION 
  security-admin/db/postgres/patches/022-split-service-table.sql PRE-CREATION 
  security-admin/db/sqlanywhere/patches/022-split-service-table.sql PRE-CREATION 
  security-admin/db/sqlserver/patches/022-split-service-table.sql PRE-CREATION 
  security-admin/src/main/java/org/apache/ranger/biz/ServiceDBStore.java 21ed686 
  security-admin/src/main/java/org/apache/ranger/biz/TagDBStore.java e11dad6 
  security-admin/src/main/java/org/apache/ranger/common/AppConstants.java 3851069 
  security-admin/src/main/java/org/apache/ranger/db/RangerDaoManagerBase.java 6559850 
  security-admin/src/main/java/org/apache/ranger/db/XXServiceDao.java 54c48dc 
  security-admin/src/main/java/org/apache/ranger/db/XXServiceVersionInfoDao.java PRE-CREATION 
  security-admin/src/main/java/org/apache/ranger/entity/XXServiceVersionInfo.java PRE-CREATION 
  security-admin/src/main/java/org/apache/ranger/patch/PatchForServiceVersionInfo_J10004.java PRE-CREATION 
  security-admin/src/main/java/org/apache/ranger/service/RangerServiceResourceService.java 807ad61 
  security-admin/src/main/java/org/apache/ranger/service/RangerServiceService.java a8f54f6 
  security-admin/src/main/java/org/apache/ranger/service/RangerServiceServiceBase.java 594d091 
  security-admin/src/main/java/org/apache/ranger/service/RangerTagDefService.java beb6295 
  security-admin/src/main/java/org/apache/ranger/service/RangerTagResourceMapService.java 257821d 
  security-admin/src/main/java/org/apache/ranger/service/RangerTagService.java 34ed0ad 
  security-admin/src/main/resources/META-INF/jpa_named_queries.xml 0f65243 
  security-admin/src/test/java/org/apache/ranger/biz/TestServiceDBStore.java 61e13da 
  security-admin/src/test/java/org/apache/ranger/service/TestRangerServiceService.java 215506e 
  security-admin/src/test/java/org/apache/ranger/service/TestRangerServiceServiceBase.java 5dff936 

Diff: https://reviews.apache.org/r/46236/diff/


Testing
-------

Tested Service creation/deletion and update scenarios.
Tested ServiceResource create/delete and update scenarios.
Tested Policy create/delete/update scenario.
Tested upgrade scenario.


Thanks,

Abhay Kulkarni


Re: Review Request 46236: x_service table columns that track policy/tag updates to be moved to a new table

Posted by Velmurugan Periasamy <vp...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46236/#review129032
-----------------------------------------------------------



There are only patches for mysql and postgres. How are you planning to cover oracle, sqlanywhere, sqlserver?

- Velmurugan Periasamy


On April 14, 2016, 11:45 p.m., Abhay Kulkarni wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46236/
> -----------------------------------------------------------
> 
> (Updated April 14, 2016, 11:45 p.m.)
> 
> 
> Review request for ranger and Madhan Neethiraj.
> 
> 
> Bugs: RANGER-901
>     https://issues.apache.org/jira/browse/RANGER-901
> 
> 
> Repository: ranger
> 
> 
> Description
> -------
> 
> x_service table columns that maintain version-numbers of contained policies and tag-updates to related service-resource objects are moved to a separate database table.
> 
> 
> Diffs
> -----
> 
>   security-admin/db/mysql/patches/022-split-service-table.sql PRE-CREATION 
>   security-admin/db/postgres/patches/022-split-service-table.sql PRE-CREATION 
>   security-admin/src/main/java/org/apache/ranger/biz/ServiceDBStore.java 9d2e4c6 
>   security-admin/src/main/java/org/apache/ranger/biz/TagDBStore.java e11dad6 
>   security-admin/src/main/java/org/apache/ranger/common/AppConstants.java 3851069 
>   security-admin/src/main/java/org/apache/ranger/db/RangerDaoManagerBase.java 6559850 
>   security-admin/src/main/java/org/apache/ranger/db/XXServiceDao.java 54c48dc 
>   security-admin/src/main/java/org/apache/ranger/db/XXServiceVersionInfoDao.java PRE-CREATION 
>   security-admin/src/main/java/org/apache/ranger/entity/XXServiceVersionInfo.java PRE-CREATION 
>   security-admin/src/main/java/org/apache/ranger/patch/PatchForServiceVersionInfo_J10004.java PRE-CREATION 
>   security-admin/src/main/java/org/apache/ranger/service/RangerServiceResourceService.java 807ad61 
>   security-admin/src/main/java/org/apache/ranger/service/RangerServiceService.java a8f54f6 
>   security-admin/src/main/java/org/apache/ranger/service/RangerServiceServiceBase.java 594d091 
>   security-admin/src/main/java/org/apache/ranger/service/RangerTagDefService.java beb6295 
>   security-admin/src/main/java/org/apache/ranger/service/RangerTagResourceMapService.java 257821d 
>   security-admin/src/main/java/org/apache/ranger/service/RangerTagService.java 34ed0ad 
>   security-admin/src/main/resources/META-INF/jpa_named_queries.xml 0f65243 
>   security-admin/src/test/java/org/apache/ranger/biz/TestServiceDBStore.java 61e13da 
>   security-admin/src/test/java/org/apache/ranger/service/TestRangerServiceService.java 215506e 
>   security-admin/src/test/java/org/apache/ranger/service/TestRangerServiceServiceBase.java 5dff936 
> 
> Diff: https://reviews.apache.org/r/46236/diff/
> 
> 
> Testing
> -------
> 
> Tested Service creation/deletion and update scenarios.
> Tested ServiceResource create/delete and update scenarios.
> Tested Policy create/delete/update scenario.
> Tested upgrade scenario.
> 
> 
> Thanks,
> 
> Abhay Kulkarni
> 
>