----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68855/#review209110 -----------------------------------------------------------
plugin-kafka/src/main/java/org/apache/ranger/authorization/kafka/authorizer/RangerKafkaAuthorizer.java Line 187 (original), 177 (patched) <https://reviews.apache.org/r/68855/#comment293373> - in #178, create 'rangerResource' with: rangerResource = new RangerAccessResourceImpl() - move #177 to end of 'if' blocks - after #193; and initialize it with the created resource. plugin-kafka/src/main/java/org/apache/ranger/authorization/kafka/authorizer/RangerKafkaAuthorizer.java Lines 207 (patched) <https://reviews.apache.org/r/68855/#comment293372> Consider passing 'rangerResourceClusterLevel' as argument to createRequest(), instead of updating the value returned by this method. RangerAccessResourceImpl clusterResource = new RangerAccessResourceImpl(); clusterResource.setValue(KEY_TOPIC, null); clusterResource.setValue(KEY_CLUSTER, "kafka-cluster"); RangerAccessRequestImpl clusterAccessRequest = createRequest(session, operation, clusterResource, resource, accessType); rangerAccessRequests.add(clusterAccessRequest); plugin-kafka/src/main/java/org/apache/ranger/authorization/kafka/authorizer/RangerKafkaAuthorizer.java Lines 209 (patched) <https://reviews.apache.org/r/68855/#comment293374> Hardcoding of the resource name, 'kafka-cluster', doesn't look right. plugin-kafka/src/main/java/org/apache/ranger/authorization/kafka/authorizer/RangerKafkaAuthorizer.java Lines 218 (patched) <https://reviews.apache.org/r/68855/#comment293375> It seems the authorizer is looking for at least one 'allow' from the given requests. This doesn't look right. What if access was explicitly denied for one of the requests in the list. Also, the approach of using multiple requests to determine access to one operation doesn't look right. security-admin/src/main/java/org/apache/ranger/patch/PatchForKafkaServiceDefUpdate_J10018.java Lines 145 (patched) <https://reviews.apache.org/r/68855/#comment293376> updateHiveServiceDef() ==> updateKafkaServiceDef() - Madhan Neethiraj On Sept. 28, 2018, 8:35 p.m., Ramesh Mani wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/68855/ > ----------------------------------------------------------- > > (Updated Sept. 28, 2018, 8:35 p.m.) > > > Review request for ranger, Abhay Kulkarni, Madhan Neethiraj, and Velmurugan > Periasamy. > > > Bugs: RANGER-2222 > https://issues.apache.org/jira/browse/RANGER-2222 > > > Repository: ranger > > > Description > ------- > > RANGER-2222: Apache RangerKafkaPlugin support to handle Kafka Cluster as a > new resource > > > Diffs > ----- > > agents-common/src/main/resources/service-defs/ranger-servicedef-kafka.json > ca3e0fe > > plugin-kafka/src/main/java/org/apache/ranger/authorization/kafka/authorizer/RangerKafkaAuthorizer.java > eab869a > > plugin-kafka/src/test/java/org/apache/ranger/authorization/kafka/authorizer/KafkaRangerAuthorizerGSSTest.java > c1386fe > > plugin-kafka/src/test/java/org/apache/ranger/authorization/kafka/authorizer/KafkaRangerTopicCreationTest.java > PRE-CREATION > plugin-kafka/src/test/resources/kafka-policies.json 0c07604 > plugin-kafka/src/test/resources/kafka_kerberos.jaas 1de804b > security-admin/db/mysql/optimized/current/ranger_core_db_mysql.sql 3f23b00 > security-admin/db/oracle/optimized/current/ranger_core_db_oracle.sql > bafdb96 > security-admin/db/postgres/optimized/current/ranger_core_db_postgres.sql > 2bc58ac > > security-admin/db/sqlanywhere/optimized/current/ranger_core_db_sqlanywhere.sql > 1b64eea > security-admin/db/sqlserver/optimized/current/ranger_core_db_sqlserver.sql > 4a216fe > > security-admin/src/main/java/org/apache/ranger/patch/PatchForKafkaServiceDefUpdate_J10018.java > PRE-CREATION > src/main/assembly/plugin-kafka.xml 97ff8ad > > > Diff: https://reviews.apache.org/r/68855/diff/3/ > > > Testing > ------- > > - This patch addresses "Cluster" and "DelegationToken" as resource in Ranger > plugin. > - Tested in local vm and added unit test for TopicCreation. > - Upgrade patch tested for default policy creation for cluster and delegation > token as resource. > > > Thanks, > > Ramesh Mani > >
