You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@atlas.apache.org by Nixon Rodrigues <ni...@freestoneinfotech.com> on 2020/09/22 10:59:21 UTC

Review Request 72894: ATLAS-3952 :- Authorize Super And SubTypes and depend entityType for type-read access while creating Classificationdef

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

Review request for atlas, chaitali, Jayendra Parab, Madhan Neethiraj, Nikhil Bonte, and Sarath Subramanian.


Bugs: ATLAS-3952
    https://issues.apache.org/jira/browse/ATLAS-3952


Repository: atlas


Description
-------

This patch consist two fixes.
1) Authorize Super And SubTypes and entityType for type-read access before creating Classificationdef.
2) Move create-type access check before duplicate "Given type _ already exists" validation.


Diffs
-----

  intg/src/main/java/org/apache/atlas/type/AtlasTypeRegistry.java 4a79b6fd5 
  repository/src/main/java/org/apache/atlas/repository/store/graph/v2/AtlasClassificationDefStoreV2.java 9ffede4e3 


Diff: https://reviews.apache.org/r/72894/diff/1/


Testing
-------

Tested creating classification while superTypes/ subTypes and entityType with valid/invalid read-Type access.
Testing duplicate classification while valid/invalid create-type access.


Thanks,

Nixon Rodrigues


Re: Review Request 72894: ATLAS-3952 :- Authorize Super And SubTypes and depend entityType for type-read access while creating Classificationdef

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




repository/src/main/java/org/apache/atlas/repository/store/graph/v2/AtlasClassificationDefStoreV2.java
Lines 75 (patched)
<https://reviews.apache.org/r/72894/#comment311008>

    This will add entityTypes to superTypes of the classification. I suggest to replace #74 - #80 with the following:
      verifyTypeReadAccess(classificationDef.getSuperTypes());
      verifyTypeReadAccess(classificationDef.getEntityTypes());
    
    And make sure to handle null as parameter value in verifyTypeReadAccess().


- Madhan Neethiraj


On Sept. 23, 2020, 6:42 p.m., Nixon Rodrigues wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72894/
> -----------------------------------------------------------
> 
> (Updated Sept. 23, 2020, 6:42 p.m.)
> 
> 
> Review request for atlas, chaitali, Jayendra Parab, Madhan Neethiraj, Nikhil Bonte, and Sarath Subramanian.
> 
> 
> Bugs: ATLAS-3952
>     https://issues.apache.org/jira/browse/ATLAS-3952
> 
> 
> Repository: atlas
> 
> 
> Description
> -------
> 
> This patch consist two fixes.
> 1) Authorize Super And SubTypes and entityType for type-read access before creating Classificationdef.
> 2) Move create-type access check before duplicate "Given type _ already exists" validation.
> 
> 
> Diffs
> -----
> 
>   intg/src/main/java/org/apache/atlas/type/AtlasTypeRegistry.java 4a79b6fd5 
>   repository/src/main/java/org/apache/atlas/repository/store/graph/v2/AtlasClassificationDefStoreV2.java 9ffede4e3 
> 
> 
> Diff: https://reviews.apache.org/r/72894/diff/3/
> 
> 
> Testing
> -------
> 
> Tested creating classification while superTypes/ subTypes and entityType with valid/invalid read-Type access.
> Testing duplicate classification while valid/invalid create-type access.
> 
> 
> Thanks,
> 
> Nixon Rodrigues
> 
>


Re: Review Request 72894: ATLAS-3952 :- Authorize Super And SubTypes and depend entityType for type-read access while creating Classificationdef

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


Ship it!




Ship It!

- Madhan Neethiraj


On Sept. 25, 2020, 8:53 a.m., Nixon Rodrigues wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72894/
> -----------------------------------------------------------
> 
> (Updated Sept. 25, 2020, 8:53 a.m.)
> 
> 
> Review request for atlas, chaitali, Jayendra Parab, Madhan Neethiraj, Nikhil Bonte, and Sarath Subramanian.
> 
> 
> Bugs: ATLAS-3952
>     https://issues.apache.org/jira/browse/ATLAS-3952
> 
> 
> Repository: atlas
> 
> 
> Description
> -------
> 
> This patch consist two fixes.
> 1) Authorize Super And SubTypes and entityType for type-read access before creating Classificationdef.
> 2) Move create-type access check before duplicate "Given type _ already exists" validation.
> 
> 
> Diffs
> -----
> 
>   intg/src/main/java/org/apache/atlas/type/AtlasTypeRegistry.java 4a79b6fd5 
>   repository/src/main/java/org/apache/atlas/repository/store/graph/v2/AtlasClassificationDefStoreV2.java 9ffede4e3 
> 
> 
> Diff: https://reviews.apache.org/r/72894/diff/4/
> 
> 
> Testing
> -------
> 
> Tested creating classification while superTypes/ subTypes and entityType with valid/invalid read-Type access.
> Testing duplicate classification while valid/invalid create-type access.
> 
> 
> Thanks,
> 
> Nixon Rodrigues
> 
>


Re: Review Request 72894: ATLAS-3952 :- Authorize Super And SubTypes and depend entityType for type-read access while creating Classificationdef

Posted by Nixon Rodrigues <ni...@freestoneinfotech.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72894/
-----------------------------------------------------------

(Updated Sept. 25, 2020, 8:53 a.m.)


Review request for atlas, chaitali, Jayendra Parab, Madhan Neethiraj, Nikhil Bonte, and Sarath Subramanian.


Changes
-------

updated patch for review comments from Madhan


Bugs: ATLAS-3952
    https://issues.apache.org/jira/browse/ATLAS-3952


Repository: atlas


Description
-------

This patch consist two fixes.
1) Authorize Super And SubTypes and entityType for type-read access before creating Classificationdef.
2) Move create-type access check before duplicate "Given type _ already exists" validation.


Diffs (updated)
-----

  intg/src/main/java/org/apache/atlas/type/AtlasTypeRegistry.java 4a79b6fd5 
  repository/src/main/java/org/apache/atlas/repository/store/graph/v2/AtlasClassificationDefStoreV2.java 9ffede4e3 


Diff: https://reviews.apache.org/r/72894/diff/4/

Changes: https://reviews.apache.org/r/72894/diff/3-4/


Testing
-------

Tested creating classification while superTypes/ subTypes and entityType with valid/invalid read-Type access.
Testing duplicate classification while valid/invalid create-type access.


Thanks,

Nixon Rodrigues


Re: Review Request 72894: ATLAS-3952 :- Authorize Super And SubTypes and depend entityType for type-read access while creating Classificationdef

Posted by Nixon Rodrigues <ni...@freestoneinfotech.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72894/
-----------------------------------------------------------

(Updated Sept. 23, 2020, 6:42 p.m.)


Review request for atlas, chaitali, Jayendra Parab, Madhan Neethiraj, Nikhil Bonte, and Sarath Subramanian.


Changes
-------

This patch addressed review comments from Madhan.


Bugs: ATLAS-3952
    https://issues.apache.org/jira/browse/ATLAS-3952


Repository: atlas


Description
-------

This patch consist two fixes.
1) Authorize Super And SubTypes and entityType for type-read access before creating Classificationdef.
2) Move create-type access check before duplicate "Given type _ already exists" validation.


Diffs (updated)
-----

  intg/src/main/java/org/apache/atlas/type/AtlasTypeRegistry.java 4a79b6fd5 
  repository/src/main/java/org/apache/atlas/repository/store/graph/v2/AtlasClassificationDefStoreV2.java 9ffede4e3 


Diff: https://reviews.apache.org/r/72894/diff/2/

Changes: https://reviews.apache.org/r/72894/diff/1-2/


Testing
-------

Tested creating classification while superTypes/ subTypes and entityType with valid/invalid read-Type access.
Testing duplicate classification while valid/invalid create-type access.


Thanks,

Nixon Rodrigues


Re: Review Request 72894: ATLAS-3952 :- Authorize Super And SubTypes and depend entityType for type-read access while creating Classificationdef

Posted by Nixon Rodrigues <ni...@freestoneinfotech.com>.

> On Sept. 22, 2020, 4 p.m., Madhan Neethiraj wrote:
> > intg/src/main/java/org/apache/atlas/type/AtlasTypeRegistry.java
> > Line 753 (original)
> > <https://reviews.apache.org/r/72894/diff/1/?file=2239801#file2239801line753>
> >
> >     I suggest to not remove this validation from here. BTW, has this validation been moved to else where?

validation is removed from here so that authorization can be done before the TYPE_ALREADY_EXISTS check and user who do not have read access do not know about this typedef.

TYPE_ALREADY_EXISTS check is also done in preCreate method of AtlasClassificationDefStoreV2 which handle duplicate check.
It also avaialble in precreate of AtlasEntityDefStoreV2 , AtlasEnumDefStoreV2 , AtlasRelationshipDefStoreV2, AtlasBusinessMetadataDefStoreV2, AtlasStructDefStoreV2


> On Sept. 22, 2020, 4 p.m., Madhan Neethiraj wrote:
> > repository/src/main/java/org/apache/atlas/repository/store/graph/v2/AtlasClassificationDefStoreV2.java
> > Lines 379 (patched)
> > <https://reviews.apache.org/r/72894/diff/1/?file=2239802#file2239802line381>
> >
> >     When multiple classifications are created in one REST call, typeRegistry.getTypeDefByName(typeName) will return null when typeName is one of the newly created type. For example:
> >     
> >       {
> >         "classificationDefs": [
> >           {
> >             "name": "tagBase"
> >           },
> >           {
> >             "name": "tagDerived",
> >             "superTypes": [ "typeBase" ]
> >           }
> >         ]
> >       }
> >     
> >     Please validate above.

{
  "classificationDefs": [
    {
      "name": "PII2_parent",
      "description": "PII2_parent",
      "superTypes": [],
      "attributeDefs": [],
      "entityTypes": [
        "hdfs_path",
        "hive_table"
      ],
      "category": "CLASSIFICATION"
    },
    {
      "name": "PII2_child",
      "description": "PII2_child",
      "superTypes": [
        "PII2_parent"
      ],
      "attributeDefs": [],
      "entityTypes": [
        "hdfs_path",
        "hive_table"
      ],
      "category": "CLASSIFICATION"
    }
  ],
  "entityDefs": [],
  "enumDefs": [],
  "structDefs": []
}

Tested with above case for creating classfications. 

typeRegistry.getTypeDefByName("PII2_parent") return AtlasBaseTypeDef object correctly. also I have added null check for def object.


- Nixon


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


On Sept. 23, 2020, 6:42 p.m., Nixon Rodrigues wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72894/
> -----------------------------------------------------------
> 
> (Updated Sept. 23, 2020, 6:42 p.m.)
> 
> 
> Review request for atlas, chaitali, Jayendra Parab, Madhan Neethiraj, Nikhil Bonte, and Sarath Subramanian.
> 
> 
> Bugs: ATLAS-3952
>     https://issues.apache.org/jira/browse/ATLAS-3952
> 
> 
> Repository: atlas
> 
> 
> Description
> -------
> 
> This patch consist two fixes.
> 1) Authorize Super And SubTypes and entityType for type-read access before creating Classificationdef.
> 2) Move create-type access check before duplicate "Given type _ already exists" validation.
> 
> 
> Diffs
> -----
> 
>   intg/src/main/java/org/apache/atlas/type/AtlasTypeRegistry.java 4a79b6fd5 
>   repository/src/main/java/org/apache/atlas/repository/store/graph/v2/AtlasClassificationDefStoreV2.java 9ffede4e3 
> 
> 
> Diff: https://reviews.apache.org/r/72894/diff/2/
> 
> 
> Testing
> -------
> 
> Tested creating classification while superTypes/ subTypes and entityType with valid/invalid read-Type access.
> Testing duplicate classification while valid/invalid create-type access.
> 
> 
> Thanks,
> 
> Nixon Rodrigues
> 
>


Re: Review Request 72894: ATLAS-3952 :- Authorize Super And SubTypes and depend entityType for type-read access while creating Classificationdef

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




intg/src/main/java/org/apache/atlas/type/AtlasTypeRegistry.java
Line 753 (original)
<https://reviews.apache.org/r/72894/#comment310974>

    I suggest to not remove this validation from here. BTW, has this validation been moved to else where?



repository/src/main/java/org/apache/atlas/repository/store/graph/v2/AtlasClassificationDefStoreV2.java
Lines 77 (patched)
<https://reviews.apache.org/r/72894/#comment310971>

    'subTypes' is a read-only/computed field. It is ignored during create/update operations. Please remove subTypes from 'typeNames'.



repository/src/main/java/org/apache/atlas/repository/store/graph/v2/AtlasClassificationDefStoreV2.java
Lines 80 (patched)
<https://reviews.apache.org/r/72894/#comment310972>

    verifyReadAccessForSuperAndSubTypes() => verifyTypeReadAccess()



repository/src/main/java/org/apache/atlas/repository/store/graph/v2/AtlasClassificationDefStoreV2.java
Lines 379 (patched)
<https://reviews.apache.org/r/72894/#comment310973>

    When multiple classifications are created in one REST call, typeRegistry.getTypeDefByName(typeName) will return null when typeName is one of the newly created type. For example:
    
      {
        "classificationDefs": [
          {
            "name": "tagBase"
          },
          {
            "name": "tagDerived",
            "superTypes": [ "typeBase" ]
          }
        ]
      }
    
    Please validate above.


- Madhan Neethiraj


On Sept. 22, 2020, 10:59 a.m., Nixon Rodrigues wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72894/
> -----------------------------------------------------------
> 
> (Updated Sept. 22, 2020, 10:59 a.m.)
> 
> 
> Review request for atlas, chaitali, Jayendra Parab, Madhan Neethiraj, Nikhil Bonte, and Sarath Subramanian.
> 
> 
> Bugs: ATLAS-3952
>     https://issues.apache.org/jira/browse/ATLAS-3952
> 
> 
> Repository: atlas
> 
> 
> Description
> -------
> 
> This patch consist two fixes.
> 1) Authorize Super And SubTypes and entityType for type-read access before creating Classificationdef.
> 2) Move create-type access check before duplicate "Given type _ already exists" validation.
> 
> 
> Diffs
> -----
> 
>   intg/src/main/java/org/apache/atlas/type/AtlasTypeRegistry.java 4a79b6fd5 
>   repository/src/main/java/org/apache/atlas/repository/store/graph/v2/AtlasClassificationDefStoreV2.java 9ffede4e3 
> 
> 
> Diff: https://reviews.apache.org/r/72894/diff/1/
> 
> 
> Testing
> -------
> 
> Tested creating classification while superTypes/ subTypes and entityType with valid/invalid read-Type access.
> Testing duplicate classification while valid/invalid create-type access.
> 
> 
> Thanks,
> 
> Nixon Rodrigues
> 
>