RANGER-397 Applied review feedback

Project: http://git-wip-us.apache.org/repos/asf/incubator-ranger/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-ranger/commit/94ba6beb
Tree: http://git-wip-us.apache.org/repos/asf/incubator-ranger/tree/94ba6beb
Diff: http://git-wip-us.apache.org/repos/asf/incubator-ranger/diff/94ba6beb

Branch: refs/heads/ranger-0.5
Commit: 94ba6beb3841f094d5800619275d80296a8b54b6
Parents: a2de245
Author: Don Bosco Durai <[email protected]>
Authored: Sat May 30 12:14:19 2015 -0700
Committer: Don Bosco Durai <[email protected]>
Committed: Sat May 30 12:14:19 2015 -0700

----------------------------------------------------------------------
 .../audit/destination/DBAuditDestination.java   | 24 +++++++++++-------
 .../ranger/audit/queue/AuditAsyncQueue.java     | 25 +++++++++----------
 .../ranger/audit/queue/AuditBatchQueue.java     | 21 ++++++----------
 .../apache/ranger/audit/queue/AuditQueue.java   |  7 ++++++
 .../ranger/audit/queue/AuditSummaryQueue.java   | 26 +++++++++-----------
 .../kafka/client/ServiceKafkaClient.java        |  5 ++--
 .../services/solr/client/ServiceSolrClient.java |  5 ++--
 .../org/apache/ranger/common/ServiceUtil.java   | 13 +++++++---
 src/main/assembly/plugin-kafka.xml              |  2 ++
 9 files changed, 66 insertions(+), 62 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-ranger/blob/94ba6beb/agents-audit/src/main/java/org/apache/ranger/audit/destination/DBAuditDestination.java
----------------------------------------------------------------------
diff --git 
a/agents-audit/src/main/java/org/apache/ranger/audit/destination/DBAuditDestination.java
 
b/agents-audit/src/main/java/org/apache/ranger/audit/destination/DBAuditDestination.java
index c58748e..8cece4e 100644
--- 
a/agents-audit/src/main/java/org/apache/ranger/audit/destination/DBAuditDestination.java
+++ 
b/agents-audit/src/main/java/org/apache/ranger/audit/destination/DBAuditDestination.java
@@ -119,8 +119,8 @@ public class DBAuditDestination extends AuditDestination {
                                        + PROP_DB_JDBC_URL);
                        dbUser = MiscUtil.getStringProperty(props, propPrefix + 
"."
                                        + PROP_DB_USER);
-                       String dbPassword = MiscUtil.getStringProperty(props, 
propPrefix
-                                       + "." + PROP_DB_PASSWORD);
+                       String dbPasswordFromProp = 
MiscUtil.getStringProperty(props,
+                                       propPrefix + "." + PROP_DB_PASSWORD);
                        String tmpAlias = MiscUtil.getStringProperty(props, 
propPrefix
                                        + "." + PROP_DB_PASSWORD_ALIAS);
                        dbPasswordAlias = tmpAlias != null ? tmpAlias : 
dbPasswordAlias;
@@ -142,16 +142,22 @@ public class DBAuditDestination extends AuditDestination {
                                                + propPrefix + "." + 
PROP_DB_USER);
                                return;
                        }
+                       String dbPassword = 
MiscUtil.getCredentialString(credFile,
+                                       dbPasswordAlias);
+
                        if (dbPassword == null || dbPassword.isEmpty()) {
-                               logger.warn("DB password not provided. Will 
assume empty for now. Set property name "
-                                               + propPrefix + "." + 
PROP_DB_PASSWORD);
-                       } else {
-                               dbPassword = 
MiscUtil.getCredentialString(credFile,
-                                               dbPasswordAlias);
+                               // If password is not in credential store, 
let's try password
+                               // from property
+                               dbPassword = dbPasswordFromProp;
+                       }
+
+                       if (dbPassword == null || dbPassword.isEmpty()) {
+                               logger.warn("DB password not provided. Will 
assume it is empty and continue");
                        }
                        logger.info("JDBC Driver=" + jdbcDriver + ", JDBC URL=" 
+ jdbcURL
                                        + ", dbUser=" + dbUser + ", 
passwordAlias="
-                                       + dbPasswordAlias + ", credFile=" + 
credFile);
+                                       + dbPasswordAlias + ", credFile=" + 
credFile
+                                       + ", usingPassword=" + (dbPassword == 
null ? "no" : "yes"));
 
                        Map<String, String> dbProperties = new HashMap<String, 
String>();
                        dbProperties.put("javax.persistence.jdbc.driver", 
jdbcDriver);
@@ -170,7 +176,7 @@ public class DBAuditDestination extends AuditDestination {
                        
daoManager.setEntityManagerFactory(entityManagerFactory);
 
                        // this forces the connection to be made to DB
-                       if (daoManager.getEntityManager() != null) {
+                       if (daoManager.getEntityManager() == null) {
                                logger.error("Error connecting audit database. 
EntityManager is null. dbURL="
                                                + jdbcURL + ", dbUser=" + 
dbUser);
                        }

http://git-wip-us.apache.org/repos/asf/incubator-ranger/blob/94ba6beb/agents-audit/src/main/java/org/apache/ranger/audit/queue/AuditAsyncQueue.java
----------------------------------------------------------------------
diff --git 
a/agents-audit/src/main/java/org/apache/ranger/audit/queue/AuditAsyncQueue.java 
b/agents-audit/src/main/java/org/apache/ranger/audit/queue/AuditAsyncQueue.java
index de5941a..47480da 100644
--- 
a/agents-audit/src/main/java/org/apache/ranger/audit/queue/AuditAsyncQueue.java
+++ 
b/agents-audit/src/main/java/org/apache/ranger/audit/queue/AuditAsyncQueue.java
@@ -103,9 +103,6 @@ public class AuditAsyncQueue extends AuditQueue implements 
Runnable {
        @Override
        public void stop() {
                logger.info("Stop called. name=" + getName());
-               if (stopTime != 0) {
-                       stopTime = System.currentTimeMillis();
-               }
                setDrain(true);
                try {
                        if (consumerThread != null) {
@@ -145,21 +142,21 @@ public class AuditAsyncQueue extends AuditQueue 
implements Runnable {
                                }
                        } catch (InterruptedException e) {
                                logger.info(
-                                               "Caught exception in consumer 
thread. Mostly server is shutting down.",
+                                               "Caught exception in consumer 
thread. Shutdown might be in progress",
                                                e);
                        } catch (Throwable t) {
                                logger.error("Caught error during processing 
request.", t);
                        }
-                       if (isDrain() && queue.isEmpty()) {
-                               break;
-                       }
-                       if (isDrain()
-                                       && (stopTime - 
System.currentTimeMillis()) > AUDIT_CONSUMER_THREAD_WAIT_MS) {
-                               logger.warn("Exiting polling loop to max time 
allowed. name="
-                                               + getName() + ", waited for "
-                                               + (stopTime - 
System.currentTimeMillis()) + " ms");
-
-                               break;
+                       if (isDrain()) {
+                               if (queue.isEmpty()) {
+                                       break;
+                               }
+                               if (isDrainMaxTimeElapsed()) {
+                                       logger.warn("Exiting polling loop 
because max time allowed reached. name="
+                                                       + getName()
+                                                       + ", waited for "
+                                                       + (stopTime - 
System.currentTimeMillis()) + " ms");
+                               }
                        }
                }
                logger.info("Exiting polling loop. name=" + getName());

http://git-wip-us.apache.org/repos/asf/incubator-ranger/blob/94ba6beb/agents-audit/src/main/java/org/apache/ranger/audit/queue/AuditBatchQueue.java
----------------------------------------------------------------------
diff --git 
a/agents-audit/src/main/java/org/apache/ranger/audit/queue/AuditBatchQueue.java 
b/agents-audit/src/main/java/org/apache/ranger/audit/queue/AuditBatchQueue.java
index 645483b..80d7853 100644
--- 
a/agents-audit/src/main/java/org/apache/ranger/audit/queue/AuditBatchQueue.java
+++ 
b/agents-audit/src/main/java/org/apache/ranger/audit/queue/AuditBatchQueue.java
@@ -120,10 +120,6 @@ public class AuditBatchQueue extends AuditQueue implements 
Runnable {
        @Override
        public void stop() {
                logger.info("Stop called. name=" + getName());
-               if (stopTime != 0) {
-                       stopTime = System.currentTimeMillis();
-               }
-
                setDrain(true);
                flush();
                try {
@@ -266,7 +262,7 @@ public class AuditBatchQueue extends AuditQueue implements 
Runnable {
                                }
                        } catch (InterruptedException e) {
                                logger.info(
-                                               "Caught exception in consumer 
thread. Mostly server is shutting down.",
+                                               "Caught exception in consumer 
thread. Shutdown might be in progress",
                                                e);
                                setDrain(true);
                        } catch (Throwable t) {
@@ -319,16 +315,13 @@ public class AuditBatchQueue extends AuditQueue 
implements Runnable {
                                } else {
                                        break;
                                }
+                               if (isDrainMaxTimeElapsed()) {
+                                       logger.warn("Exiting polling loop 
because max time allowed reached. name="
+                                                       + getName()
+                                                       + ", waited for "
+                                                       + (stopTime - 
System.currentTimeMillis()) + " ms");
+                               }
                        }
-                       if (isDrain()
-                                       && (stopTime - 
System.currentTimeMillis()) > AUDIT_CONSUMER_THREAD_WAIT_MS) {
-                               logger.warn("Exiting polling loop to max time 
allowed. name="
-                                               + getName() + ", waited for "
-                                               + (stopTime - 
System.currentTimeMillis()) + " ms");
-
-                               break;
-                       }
-
                }
 
                logger.info("Exiting consumerThread. Queue=" + getName() + ", 
dest="

http://git-wip-us.apache.org/repos/asf/incubator-ranger/blob/94ba6beb/agents-audit/src/main/java/org/apache/ranger/audit/queue/AuditQueue.java
----------------------------------------------------------------------
diff --git 
a/agents-audit/src/main/java/org/apache/ranger/audit/queue/AuditQueue.java 
b/agents-audit/src/main/java/org/apache/ranger/audit/queue/AuditQueue.java
index 039dc6d..e873459 100644
--- a/agents-audit/src/main/java/org/apache/ranger/audit/queue/AuditQueue.java
+++ b/agents-audit/src/main/java/org/apache/ranger/audit/queue/AuditQueue.java
@@ -114,11 +114,18 @@ public abstract class AuditQueue extends BaseAuditHandler 
{
                return consumer;
        }
 
+       public boolean isDrainMaxTimeElapsed() {
+               return (stopTime - System.currentTimeMillis()) > 
AUDIT_CONSUMER_THREAD_WAIT_MS;
+       }
+       
        public boolean isDrain() {
                return isDrain;
        }
 
        public void setDrain(boolean isDrain) {
+               if (isDrain && stopTime != 0) {
+                       stopTime = System.currentTimeMillis();
+               }
                this.isDrain = isDrain;
        }
 

http://git-wip-us.apache.org/repos/asf/incubator-ranger/blob/94ba6beb/agents-audit/src/main/java/org/apache/ranger/audit/queue/AuditSummaryQueue.java
----------------------------------------------------------------------
diff --git 
a/agents-audit/src/main/java/org/apache/ranger/audit/queue/AuditSummaryQueue.java
 
b/agents-audit/src/main/java/org/apache/ranger/audit/queue/AuditSummaryQueue.java
index 1e5b500..f1ce799 100644
--- 
a/agents-audit/src/main/java/org/apache/ranger/audit/queue/AuditSummaryQueue.java
+++ 
b/agents-audit/src/main/java/org/apache/ranger/audit/queue/AuditSummaryQueue.java
@@ -123,10 +123,6 @@ public class AuditSummaryQueue extends AuditQueue 
implements Runnable {
        @Override
        public void stop() {
                logger.info("Stop called. name=" + getName());
-               if (stopTime != 0) {
-                       stopTime = System.currentTimeMillis();
-               }
-
                setDrain(true);
                try {
                        if (consumerThread != null) {
@@ -179,7 +175,7 @@ public class AuditSummaryQueue extends AuditQueue 
implements Runnable {
                                }
                        } catch (InterruptedException e) {
                                logger.info(
-                                               "Caught exception in consumer 
thread. Mostly server is shutting down.",
+                                               "Caught exception in consumer 
thread. Shutdown might be in progress",
                                                e);
                        } catch (Throwable t) {
                                logger.error("Caught error during processing 
request.", t);
@@ -223,16 +219,16 @@ public class AuditSummaryQueue extends AuditQueue 
implements Runnable {
                                summaryMap.clear();
                        }
 
-                       if (isDrain() && summaryMap.isEmpty() && 
queue.isEmpty()) {
-                               break;
-                       }
-                       if (isDrain()
-                                       && (stopTime - 
System.currentTimeMillis()) > AUDIT_CONSUMER_THREAD_WAIT_MS) {
-                               logger.warn("Exiting polling loop to max time 
allowed. name="
-                                               + getName() + ", waited for "
-                                               + (stopTime - 
System.currentTimeMillis()) + " ms");
-
-                               break;
+                       if (isDrain()) {
+                               if (summaryMap.isEmpty() && queue.isEmpty()) {
+                                       break;
+                               }
+                               if (isDrainMaxTimeElapsed()) {
+                                       logger.warn("Exiting polling loop 
because max time allowed reached. name="
+                                                       + getName()
+                                                       + ", waited for "
+                                                       + (stopTime - 
System.currentTimeMillis()) + " ms");
+                               }
                        }
 
                }

http://git-wip-us.apache.org/repos/asf/incubator-ranger/blob/94ba6beb/plugin-kafka/src/main/java/org/apache/ranger/services/kafka/client/ServiceKafkaClient.java
----------------------------------------------------------------------
diff --git 
a/plugin-kafka/src/main/java/org/apache/ranger/services/kafka/client/ServiceKafkaClient.java
 
b/plugin-kafka/src/main/java/org/apache/ranger/services/kafka/client/ServiceKafkaClient.java
index 5cca619..0698bf6 100644
--- 
a/plugin-kafka/src/main/java/org/apache/ranger/services/kafka/client/ServiceKafkaClient.java
+++ 
b/plugin-kafka/src/main/java/org/apache/ranger/services/kafka/client/ServiceKafkaClient.java
@@ -61,20 +61,19 @@ public class ServiceKafkaClient {
 
        public HashMap<String, Object> testConnection() throws Exception {
                String errMsg = errMessage;
-               boolean connectivityStatus = false;
                HashMap<String, Object> responseData = new HashMap<String, 
Object>();
                try {
                        getTopicList(null);
                        // If it doesn't throw exception, then assume the 
instance is
                        // reachable
                        String successMsg = "TestConnection Successful";
-                       BaseClient.generateResponseDataMap(connectivityStatus, 
successMsg,
+                       BaseClient.generateResponseDataMap(true, successMsg,
                                        successMsg, null, null, responseData);
                } catch (IOException e) {
                        LOG.error("Error connecting to Kafka. kafkaClient=" + 
this, e);
                        String failureMsg = "Unable to connect to Kafka 
instance."
                                        + e.getMessage();
-                       BaseClient.generateResponseDataMap(connectivityStatus, 
failureMsg,
+                       BaseClient.generateResponseDataMap(false, failureMsg,
                                        failureMsg + errMsg, null, null, 
responseData);
                }
                return responseData;

http://git-wip-us.apache.org/repos/asf/incubator-ranger/blob/94ba6beb/plugin-solr/src/main/java/org/apache/ranger/services/solr/client/ServiceSolrClient.java
----------------------------------------------------------------------
diff --git 
a/plugin-solr/src/main/java/org/apache/ranger/services/solr/client/ServiceSolrClient.java
 
b/plugin-solr/src/main/java/org/apache/ranger/services/solr/client/ServiceSolrClient.java
index 6a192f4..801578b 100644
--- 
a/plugin-solr/src/main/java/org/apache/ranger/services/solr/client/ServiceSolrClient.java
+++ 
b/plugin-solr/src/main/java/org/apache/ranger/services/solr/client/ServiceSolrClient.java
@@ -72,7 +72,6 @@ public class ServiceSolrClient {
 
        public HashMap<String, Object> testConnection() throws Exception {
                String errMsg = errMessage;
-               boolean connectivityStatus = false;
                HashMap<String, Object> responseData = new HashMap<String, 
Object>();
 
                try {
@@ -80,13 +79,13 @@ public class ServiceSolrClient {
                        // If it doesn't throw exception, then assume the 
instance is
                        // reachable
                        String successMsg = "TestConnection Successful";
-                       BaseClient.generateResponseDataMap(connectivityStatus, 
successMsg,
+                       BaseClient.generateResponseDataMap(true, successMsg,
                                        successMsg, null, null, responseData);
                } catch (IOException e) {
                        LOG.error("Error connecting to Solr. solrClient=" + 
solrClient, e);
                        String failureMsg = "Unable to connect to Solr 
instance."
                                        + e.getMessage();
-                       BaseClient.generateResponseDataMap(connectivityStatus, 
failureMsg,
+                       BaseClient.generateResponseDataMap(false, failureMsg,
                                        failureMsg + errMsg, null, null, 
responseData);
 
                }

http://git-wip-us.apache.org/repos/asf/incubator-ranger/blob/94ba6beb/security-admin/src/main/java/org/apache/ranger/common/ServiceUtil.java
----------------------------------------------------------------------
diff --git 
a/security-admin/src/main/java/org/apache/ranger/common/ServiceUtil.java 
b/security-admin/src/main/java/org/apache/ranger/common/ServiceUtil.java
index 09759c3..b7a923b 100644
--- a/security-admin/src/main/java/org/apache/ranger/common/ServiceUtil.java
+++ b/security-admin/src/main/java/org/apache/ranger/common/ServiceUtil.java
@@ -1305,27 +1305,29 @@ public class ServiceUtil {
                try {
                        service = svcStore.getServiceByName(serviceName);
                } catch (Exception e) {
-                       LOG.error("Requested Service not found");
+                       LOG.error("Requested Service not found. serviceName=" + 
serviceName);
                        throw restErrorUtil.createRESTException("Serivce:" + 
serviceName + " not found",  
                                        MessageEnums.DATA_NOT_FOUND);
                }
                if(service==null){
-                       LOG.error("Requested Service not found");
+                       LOG.error("Requested Service not found. Service name is 
null.");
                        throw restErrorUtil.createRESTException("No Data 
Found.",
                                        MessageEnums.DATA_NOT_FOUND);
                }
                if(!service.getIsEnabled()){
-                       LOG.error("Requested Service is disabled");
+                       LOG.error("Requested Service is disabled. serviceName=" 
+ serviceName);
                        throw restErrorUtil.createRESTException("Unauthorized 
access.",
                                        
MessageEnums.OPER_NOT_ALLOWED_FOR_STATE);
                }               
                if (!httpEnabled) {
                        if (!isSecure) {
+                               LOG.error("Unauthorized access. Only https is 
allowed. serviceName=" + serviceName);
                                throw 
restErrorUtil.createRESTException("Unauthorized access -"
                                                + " only https allowed",
                                                
MessageEnums.OPER_NOT_ALLOWED_FOR_ENTITY);
                        }
                        if (certchain == null || certchain.length == 0) {
+                               LOG.error("Unauthorized access. Unable to get 
client certificate. serviceName=" + serviceName);
                                throw 
restErrorUtil.createRESTException("Unauthorized access -"
                                                + " unable to get client 
certificate",
                                                
MessageEnums.OPER_NOT_ALLOWED_FOR_ENTITY);
@@ -1344,13 +1346,14 @@ public class ServiceUtil {
                                        }
                                }
                                if (commonName == null) {
+                                       LOG.error("Unauthorized access. CName 
is null. serviceName=" + serviceName);
                                        throw restErrorUtil.createRESTException(
                                                        "Unauthorized access - 
Unable to find Common Name from ["
                                                                        + dn + 
"]",
                                                        
MessageEnums.OPER_NOT_ALLOWED_FOR_ENTITY);
                                }
                        } catch (InvalidNameException e) {
-                               LOG.error("Invalid Common Name.", e);
+                               LOG.error("Invalid Common Name. CName=" + 
commonName + ", serviceName=" + serviceName, e);
                                throw restErrorUtil.createRESTException(
                                                "Unauthorized access - Invalid 
Common Name",
                                                
MessageEnums.OPER_NOT_ALLOWED_FOR_ENTITY);
@@ -1362,6 +1365,8 @@ public class ServiceUtil {
                        String cnFromConfig = 
configMap.get("commonNameForCertificate");
                        if (cnFromConfig == null
                                        || 
!commonName.equalsIgnoreCase(cnFromConfig)) {
+                               LOG.error("Unauthorized access. expected [" + 
cnFromConfig + "], found [" 
+                                       + commonName + "], serviceName=" + 
serviceName);
                                throw restErrorUtil.createRESTException(
                                                "Unauthorized access. expected 
[" + cnFromConfig
                                                                + "], found [" 
+ commonName + "]",

http://git-wip-us.apache.org/repos/asf/incubator-ranger/blob/94ba6beb/src/main/assembly/plugin-kafka.xml
----------------------------------------------------------------------
diff --git a/src/main/assembly/plugin-kafka.xml 
b/src/main/assembly/plugin-kafka.xml
index 77c4e65..67e8489 100644
--- a/src/main/assembly/plugin-kafka.xml
+++ b/src/main/assembly/plugin-kafka.xml
@@ -36,6 +36,8 @@
                                                        </include>
                                                        
<include>org.apache.hadoop:hadoop-common-plus:jar:${hadoop-common.version}
                                                        </include>
+                                                       
<include>org.apache.hadoop:hadoop-auth:jar:${hadoop-common.version}
+                                                       </include>
                                                        
<include>com.google.code.gson:gson</include>
                                                        
<include>org.eclipse.persistence:eclipselink</include>
                                                        
<include>org.eclipse.persistence:javax.persistence</include>

Reply via email to