Re: Review Request 68287: ATLAS-2807: Re-evaluate classification propagation during entity delete

2018-08-10 Thread Sarath Subramanian 


> On Aug. 10, 2018, 4:01 a.m., David Radley wrote:
> > repository/src/main/java/org/apache/atlas/repository/store/graph/v1/HardDeleteHandlerV1.java
> > Line 56 (original)
> > 
> >
> > I assume this is implementing the reporting text "Hard Delete:
> > In cases of hard delete, the source entity (f1) is deleted from atlas, 
> > but its classification vertex
> > (PII) continue to exist and will continue propagating to process1 and 
> > t1." 
> > 
> > I am not sure about this - as if a source entity is PII and it is 
> > deleted - I would think any downstream entities should not be PII.

David, we are planning to add a global property to toggle this default behavior.


for e.g
consider the following example where we assign PII to medical_records_table and 
add security policies in ranger enforcing authorized users to access these 
sensitive table and view. If the source table is deleted for some reason and if 
we remove the propagated classification, this might possibly pose a security 
access issue. Sensitive data is now accesible to everyone. This JIRA solves 
this case. 

medical_records_table -> Process -> uk_medical_records_table
[PII] {PII}{PII}


- Sarath


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


On Aug. 9, 2018, 7:36 p.m., Sarath Subramanian wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68287/
> ---
> 
> (Updated Aug. 9, 2018, 7:36 p.m.)
> 
> 
> Review request for atlas, Apoorv Naik, Ashutosh Mestry, Madhan Neethiraj, and 
> Nixon Rodrigues.
> 
> 
> Bugs: ATLAS-2807
> https://issues.apache.org/jira/browse/ATLAS-2807
> 
> 
> Repository: atlas
> 
> 
> Description
> ---
> 
> The current behavior - when we delete an entity f1, all tags associate to f1 
> which got propagated
> 
> 
> to downstream entities are removed – PII tag propagated to process1 and t1 is 
> removed
> 
> 
> Proposed Change
> 
> 
> Soft Delete:
> 
> 
> When entity f1 is deleted, retain the propagated classification edges, so the 
> downstream entities
> 
> 
> – process1 and t1 continue to have PII classification associated to them
> 
> 
> Hard Delete:
> 
> 
> In cases of hard delete, the source entity (f1) is deleted from atlas, but 
> its classification vertex
> 
> 
> (PII) continue to exist and will continue propagating to process1 and t1.
> 
> 
> Diffs
> -
> 
>   common/src/main/java/org/apache/atlas/repository/Constants.java 7988d1d81 
>   intg/src/main/java/org/apache/atlas/AtlasErrorCode.java 328b76734 
>   intg/src/main/java/org/apache/atlas/model/instance/AtlasClassification.java 
> 3132379b9 
>   repository/src/main/java/org/apache/atlas/repository/graph/GraphHelper.java 
> d328873f1 
>   
> repository/src/main/java/org/apache/atlas/repository/store/graph/AtlasEntityStore.java
>  c6be4061b 
>   
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/DeleteHandlerV1.java
>  4a0924bbe 
>   
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/HardDeleteHandlerV1.java
>  a95e6898e 
>   
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/SoftDeleteHandlerV1.java
>  41e65d4a3 
>   
> repository/src/main/java/org/apache/atlas/repository/store/graph/v2/AtlasEntityStoreV2.java
>  28a26ab4f 
>   
> repository/src/main/java/org/apache/atlas/repository/store/graph/v2/EntityGraphMapper.java
>  f57ce99f7 
>   
> repository/src/main/java/org/apache/atlas/repository/store/graph/v2/EntityGraphRetriever.java
>  8f4faaff8 
>   
> repository/src/test/java/org/apache/atlas/repository/store/graph/v2/AtlasEntityStoreV2Test.java
>  58005c458 
>   
> repository/src/test/java/org/apache/atlas/repository/tagpropagation/ClassificationPropagationTest.java
>  8ceeedb01 
>   webapp/src/main/java/org/apache/atlas/web/resources/EntityResource.java 
> e01cba92d 
>   webapp/src/main/java/org/apache/atlas/web/rest/EntityREST.java fd331faa0 
>   webapp/src/test/java/org/apache/atlas/web/adapters/TestEntityREST.java 
> 78bd53ce7 
> 
> 
> Diff: https://reviews.apache.org/r/68287/diff/1/
> 
> 
> Testing
> ---
> 
> Validated basic cases using REST clirnt.
> 
> 
> PreCommit: In progress
> 
> 
> Thanks,
> 
> Sarath Subramanian
> 
>

Re: Review Request 68287: ATLAS-2807: Re-evaluate classification propagation during entity delete

2018-08-10 Thread Sarath Subramanian 


> On Aug. 10, 2018, 3:37 a.m., Graham Wallis wrote:
> > Please look at the question I posted to the JIRA - I am not sure that you 
> > always want the specified behaviour.

The behavior doesn't depend on the nature of the relationship, although the 
example showcases retaining propagated tags for Dataset -> Process -> Dataset 
relationship. This behavior is applicable to all relationship types. We are 
planning to add a global property to toggle the default behavior.

for e.g. if retainPropagatedClassificationsOnEntityDelete = true/false. This 
will be implemented in a follow up JIRA.


- Sarath


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


On Aug. 9, 2018, 7:36 p.m., Sarath Subramanian wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68287/
> ---
> 
> (Updated Aug. 9, 2018, 7:36 p.m.)
> 
> 
> Review request for atlas, Apoorv Naik, Ashutosh Mestry, Madhan Neethiraj, and 
> Nixon Rodrigues.
> 
> 
> Bugs: ATLAS-2807
> https://issues.apache.org/jira/browse/ATLAS-2807
> 
> 
> Repository: atlas
> 
> 
> Description
> ---
> 
> The current behavior - when we delete an entity f1, all tags associate to f1 
> which got propagated
> 
> 
> to downstream entities are removed – PII tag propagated to process1 and t1 is 
> removed
> 
> 
> Proposed Change
> 
> 
> Soft Delete:
> 
> 
> When entity f1 is deleted, retain the propagated classification edges, so the 
> downstream entities
> 
> 
> – process1 and t1 continue to have PII classification associated to them
> 
> 
> Hard Delete:
> 
> 
> In cases of hard delete, the source entity (f1) is deleted from atlas, but 
> its classification vertex
> 
> 
> (PII) continue to exist and will continue propagating to process1 and t1.
> 
> 
> Diffs
> -
> 
>   common/src/main/java/org/apache/atlas/repository/Constants.java 7988d1d81 
>   intg/src/main/java/org/apache/atlas/AtlasErrorCode.java 328b76734 
>   intg/src/main/java/org/apache/atlas/model/instance/AtlasClassification.java 
> 3132379b9 
>   repository/src/main/java/org/apache/atlas/repository/graph/GraphHelper.java 
> d328873f1 
>   
> repository/src/main/java/org/apache/atlas/repository/store/graph/AtlasEntityStore.java
>  c6be4061b 
>   
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/DeleteHandlerV1.java
>  4a0924bbe 
>   
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/HardDeleteHandlerV1.java
>  a95e6898e 
>   
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/SoftDeleteHandlerV1.java
>  41e65d4a3 
>   
> repository/src/main/java/org/apache/atlas/repository/store/graph/v2/AtlasEntityStoreV2.java
>  28a26ab4f 
>   
> repository/src/main/java/org/apache/atlas/repository/store/graph/v2/EntityGraphMapper.java
>  f57ce99f7 
>   
> repository/src/main/java/org/apache/atlas/repository/store/graph/v2/EntityGraphRetriever.java
>  8f4faaff8 
>   
> repository/src/test/java/org/apache/atlas/repository/store/graph/v2/AtlasEntityStoreV2Test.java
>  58005c458 
>   
> repository/src/test/java/org/apache/atlas/repository/tagpropagation/ClassificationPropagationTest.java
>  8ceeedb01 
>   webapp/src/main/java/org/apache/atlas/web/resources/EntityResource.java 
> e01cba92d 
>   webapp/src/main/java/org/apache/atlas/web/rest/EntityREST.java fd331faa0 
>   webapp/src/test/java/org/apache/atlas/web/adapters/TestEntityREST.java 
> 78bd53ce7 
> 
> 
> Diff: https://reviews.apache.org/r/68287/diff/1/
> 
> 
> Testing
> ---
> 
> Validated basic cases using REST clirnt.
> 
> 
> PreCommit: In progress
> 
> 
> Thanks,
> 
> Sarath Subramanian
> 
>

Re: Review Request 68287: ATLAS-2807: Re-evaluate classification propagation during entity delete

2018-08-10 Thread David Radley 

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




repository/src/main/java/org/apache/atlas/repository/store/graph/v1/HardDeleteHandlerV1.java
Line 56 (original)


I assume this is implementing the reporting text "Hard Delete:
In cases of hard delete, the source entity (f1) is deleted from atlas, but 
its classification vertex
(PII) continue to exist and will continue propagating to process1 and t1." 

I am not sure about this - as if a source entity is PII and it is deleted - 
I would think any downstream entities should not be PII.


- David Radley


On Aug. 10, 2018, 2:36 a.m., Sarath Subramanian wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68287/
> ---
> 
> (Updated Aug. 10, 2018, 2:36 a.m.)
> 
> 
> Review request for atlas, Apoorv Naik, Ashutosh Mestry, Madhan Neethiraj, and 
> Nixon Rodrigues.
> 
> 
> Bugs: ATLAS-2807
> https://issues.apache.org/jira/browse/ATLAS-2807
> 
> 
> Repository: atlas
> 
> 
> Description
> ---
> 
> The current behavior - when we delete an entity f1, all tags associate to f1 
> which got propagated
> 
> 
> to downstream entities are removed – PII tag propagated to process1 and t1 is 
> removed
> 
> 
> Proposed Change
> 
> 
> Soft Delete:
> 
> 
> When entity f1 is deleted, retain the propagated classification edges, so the 
> downstream entities
> 
> 
> – process1 and t1 continue to have PII classification associated to them
> 
> 
> Hard Delete:
> 
> 
> In cases of hard delete, the source entity (f1) is deleted from atlas, but 
> its classification vertex
> 
> 
> (PII) continue to exist and will continue propagating to process1 and t1.
> 
> 
> Diffs
> -
> 
>   common/src/main/java/org/apache/atlas/repository/Constants.java 7988d1d81 
>   intg/src/main/java/org/apache/atlas/AtlasErrorCode.java 328b76734 
>   intg/src/main/java/org/apache/atlas/model/instance/AtlasClassification.java 
> 3132379b9 
>   repository/src/main/java/org/apache/atlas/repository/graph/GraphHelper.java 
> d328873f1 
>   
> repository/src/main/java/org/apache/atlas/repository/store/graph/AtlasEntityStore.java
>  c6be4061b 
>   
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/DeleteHandlerV1.java
>  4a0924bbe 
>   
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/HardDeleteHandlerV1.java
>  a95e6898e 
>   
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/SoftDeleteHandlerV1.java
>  41e65d4a3 
>   
> repository/src/main/java/org/apache/atlas/repository/store/graph/v2/AtlasEntityStoreV2.java
>  28a26ab4f 
>   
> repository/src/main/java/org/apache/atlas/repository/store/graph/v2/EntityGraphMapper.java
>  f57ce99f7 
>   
> repository/src/main/java/org/apache/atlas/repository/store/graph/v2/EntityGraphRetriever.java
>  8f4faaff8 
>   
> repository/src/test/java/org/apache/atlas/repository/store/graph/v2/AtlasEntityStoreV2Test.java
>  58005c458 
>   
> repository/src/test/java/org/apache/atlas/repository/tagpropagation/ClassificationPropagationTest.java
>  8ceeedb01 
>   webapp/src/main/java/org/apache/atlas/web/resources/EntityResource.java 
> e01cba92d 
>   webapp/src/main/java/org/apache/atlas/web/rest/EntityREST.java fd331faa0 
>   webapp/src/test/java/org/apache/atlas/web/adapters/TestEntityREST.java 
> 78bd53ce7 
> 
> 
> Diff: https://reviews.apache.org/r/68287/diff/1/
> 
> 
> Testing
> ---
> 
> Validated basic cases using REST clirnt.
> 
> 
> PreCommit: In progress
> 
> 
> Thanks,
> 
> Sarath Subramanian
> 
>

Re: Review Request 68287: ATLAS-2807: Re-evaluate classification propagation during entity delete

2018-08-10 Thread Graham Wallis 

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



Please look at the question I posted to the JIRA - I am not sure that you 
always want the specified behaviour.

- Graham Wallis


On Aug. 10, 2018, 2:36 a.m., Sarath Subramanian wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68287/
> ---
> 
> (Updated Aug. 10, 2018, 2:36 a.m.)
> 
> 
> Review request for atlas, Apoorv Naik, Ashutosh Mestry, Madhan Neethiraj, and 
> Nixon Rodrigues.
> 
> 
> Bugs: ATLAS-2807
> https://issues.apache.org/jira/browse/ATLAS-2807
> 
> 
> Repository: atlas
> 
> 
> Description
> ---
> 
> The current behavior - when we delete an entity f1, all tags associate to f1 
> which got propagated
> 
> 
> to downstream entities are removed – PII tag propagated to process1 and t1 is 
> removed
> 
> 
> Proposed Change
> 
> 
> Soft Delete:
> 
> 
> When entity f1 is deleted, retain the propagated classification edges, so the 
> downstream entities
> 
> 
> – process1 and t1 continue to have PII classification associated to them
> 
> 
> Hard Delete:
> 
> 
> In cases of hard delete, the source entity (f1) is deleted from atlas, but 
> its classification vertex
> 
> 
> (PII) continue to exist and will continue propagating to process1 and t1.
> 
> 
> Diffs
> -
> 
>   common/src/main/java/org/apache/atlas/repository/Constants.java 7988d1d81 
>   intg/src/main/java/org/apache/atlas/AtlasErrorCode.java 328b76734 
>   intg/src/main/java/org/apache/atlas/model/instance/AtlasClassification.java 
> 3132379b9 
>   repository/src/main/java/org/apache/atlas/repository/graph/GraphHelper.java 
> d328873f1 
>   
> repository/src/main/java/org/apache/atlas/repository/store/graph/AtlasEntityStore.java
>  c6be4061b 
>   
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/DeleteHandlerV1.java
>  4a0924bbe 
>   
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/HardDeleteHandlerV1.java
>  a95e6898e 
>   
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/SoftDeleteHandlerV1.java
>  41e65d4a3 
>   
> repository/src/main/java/org/apache/atlas/repository/store/graph/v2/AtlasEntityStoreV2.java
>  28a26ab4f 
>   
> repository/src/main/java/org/apache/atlas/repository/store/graph/v2/EntityGraphMapper.java
>  f57ce99f7 
>   
> repository/src/main/java/org/apache/atlas/repository/store/graph/v2/EntityGraphRetriever.java
>  8f4faaff8 
>   
> repository/src/test/java/org/apache/atlas/repository/store/graph/v2/AtlasEntityStoreV2Test.java
>  58005c458 
>   
> repository/src/test/java/org/apache/atlas/repository/tagpropagation/ClassificationPropagationTest.java
>  8ceeedb01 
>   webapp/src/main/java/org/apache/atlas/web/resources/EntityResource.java 
> e01cba92d 
>   webapp/src/main/java/org/apache/atlas/web/rest/EntityREST.java fd331faa0 
>   webapp/src/test/java/org/apache/atlas/web/adapters/TestEntityREST.java 
> 78bd53ce7 
> 
> 
> Diff: https://reviews.apache.org/r/68287/diff/1/
> 
> 
> Testing
> ---
> 
> Validated basic cases using REST clirnt.
> 
> 
> PreCommit: In progress
> 
> 
> Thanks,
> 
> Sarath Subramanian
> 
>

Re: Review Request 68287: ATLAS-2807: Re-evaluate classification propagation during entity delete

2018-08-10 Thread Madhan Neethiraj 

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


Fix it, then Ship it!





intg/src/main/java/org/apache/atlas/AtlasErrorCode.java
Line 131 (original), 131 (patched)


"Propagated classification {0} - associatedEntityGuid {1} is not associated 
with entity {2}" ==>

"Propagated classification {0} is not associated with entity {2}, it is 
associated with entity {1}"



intg/src/main/java/org/apache/atlas/AtlasErrorCode.java
Lines 155 (patched)


"Propagated classification {0} with associatedEntityGuid {1} removal not 
supported since associated entity is ACTIVE" ==>

"Removal of classification {0}, which is propagated from entity {1}, is not 
supported"



repository/src/main/java/org/apache/atlas/repository/store/graph/v2/EntityGraphMapper.java
Lines 1420 (patched)


Are lines #1420 - #1424 necessary, since subsequent call to 
getPropagatedClassificationEdge() checks if the given classification is 
propagated from the given entity? If this block is not needed, please remove.



repository/src/main/java/org/apache/atlas/repository/store/graph/v2/EntityGraphMapper.java
Lines 1733 (patched)


Why copy to a set? Wouldn't 
existingClassifications.contains(suppliedClassificationName) do?


- Madhan Neethiraj


On Aug. 10, 2018, 2:36 a.m., Sarath Subramanian wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68287/
> ---
> 
> (Updated Aug. 10, 2018, 2:36 a.m.)
> 
> 
> Review request for atlas, Apoorv Naik, Ashutosh Mestry, Madhan Neethiraj, and 
> Nixon Rodrigues.
> 
> 
> Bugs: ATLAS-2807
> https://issues.apache.org/jira/browse/ATLAS-2807
> 
> 
> Repository: atlas
> 
> 
> Description
> ---
> 
> The current behavior - when we delete an entity f1, all tags associate to f1 
> which got propagated
> 
> 
> to downstream entities are removed – PII tag propagated to process1 and t1 is 
> removed
> 
> 
> Proposed Change
> 
> 
> Soft Delete:
> 
> 
> When entity f1 is deleted, retain the propagated classification edges, so the 
> downstream entities
> 
> 
> – process1 and t1 continue to have PII classification associated to them
> 
> 
> Hard Delete:
> 
> 
> In cases of hard delete, the source entity (f1) is deleted from atlas, but 
> its classification vertex
> 
> 
> (PII) continue to exist and will continue propagating to process1 and t1.
> 
> 
> Diffs
> -
> 
>   common/src/main/java/org/apache/atlas/repository/Constants.java 7988d1d81 
>   intg/src/main/java/org/apache/atlas/AtlasErrorCode.java 328b76734 
>   intg/src/main/java/org/apache/atlas/model/instance/AtlasClassification.java 
> 3132379b9 
>   repository/src/main/java/org/apache/atlas/repository/graph/GraphHelper.java 
> d328873f1 
>   
> repository/src/main/java/org/apache/atlas/repository/store/graph/AtlasEntityStore.java
>  c6be4061b 
>   
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/DeleteHandlerV1.java
>  4a0924bbe 
>   
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/HardDeleteHandlerV1.java
>  a95e6898e 
>   
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/SoftDeleteHandlerV1.java
>  41e65d4a3 
>   
> repository/src/main/java/org/apache/atlas/repository/store/graph/v2/AtlasEntityStoreV2.java
>  28a26ab4f 
>   
> repository/src/main/java/org/apache/atlas/repository/store/graph/v2/EntityGraphMapper.java
>  f57ce99f7 
>   
> repository/src/main/java/org/apache/atlas/repository/store/graph/v2/EntityGraphRetriever.java
>  8f4faaff8 
>   
> repository/src/test/java/org/apache/atlas/repository/store/graph/v2/AtlasEntityStoreV2Test.java
>  58005c458 
>   
> repository/src/test/java/org/apache/atlas/repository/tagpropagation/ClassificationPropagationTest.java
>  8ceeedb01 
>   webapp/src/main/java/org/apache/atlas/web/resources/EntityResource.java 
> e01cba92d 
>   webapp/src/main/java/org/apache/atlas/web/rest/EntityREST.java fd331faa0 
>   webapp/src/test/java/org/apache/atlas/web/adapters/TestEntityREST.java 
> 78bd53ce7 
> 
> 
> Diff: https://reviews.apache.org/r/68287/diff/1/
> 
> 
> Testing
> ---
> 
> Validated basic cases using REST clirnt.
> 
> 
> PreCommit: In progress
> 
> 
> Thanks,
> 
> Sarath Subramanian
> 
>