added prepare() into plugin APIs, and removed redundant logs

sunlzx · Feb 26, 2013 · 2123a17 · 2123a17
1 parent 8308863
commit 2123a17
Show file tree

Hide file tree

Showing 13 changed files with 61 additions and 37 deletions.
diff --git a/src/jvm/backtype/storm/security/auth/AuthUtils.java b/src/jvm/backtype/storm/security/auth/AuthUtils.java
@@ -55,7 +55,8 @@ public static ITransportPlugin GetTransportPlugin(Map storm_conf, Configuration
         try {
             String transport_plugin_klassName = (String) storm_conf.get(Config.STORM_THRIFT_TRANSPORT_PLUGIN);
             Class klass = Class.forName(transport_plugin_klassName);
-            transportPlugin = (ITransportPlugin)klass.getConstructor(Configuration.class).newInstance(login_conf);
+            transportPlugin = (ITransportPlugin)klass.newInstance();
+            transportPlugin.prepare(storm_conf, login_conf);
         } catch(Exception e) {
             throw new RuntimeException(e);
         } 
@@ -66,7 +67,6 @@ public static String get(Configuration configuration, String section, String key
         AppConfigurationEntry configurationEntries[] = configuration.getAppConfigurationEntry(section);
         if (configurationEntries == null) {
             String errorMessage = "Could not find a '"+ section + "' entry in this configuration.";
-            LOG.error(errorMessage);
             throw new IOException(errorMessage);
         }
 

diff --git a/src/jvm/backtype/storm/security/auth/IAuthorizer.java b/src/jvm/backtype/storm/security/auth/IAuthorizer.java
@@ -1,5 +1,7 @@
 package backtype.storm.security.auth;
 
+import java.util.Map;
+
 /**
  * Nimbus could be configured with an authorization plugin.
  * If not specified, all requests are authorized.
@@ -11,6 +13,12 @@
  *   nimbus.authorization.class: backtype.storm.security.auth.NoopAuthorizer
  */
 public interface IAuthorizer {
+    /**
+     * Invoked once immediately after construction
+     * @param conf Storm configuration 
+     */
+    void prepare(Map storm_conf);
+
     /**
      * permit() method is invoked for each incoming Thrift request.
      * @param contrext request context includes info about 

diff --git a/src/jvm/backtype/storm/security/auth/ITransportPlugin.java b/src/jvm/backtype/storm/security/auth/ITransportPlugin.java
@@ -1,18 +1,26 @@
 package backtype.storm.security.auth;
 
 import java.io.IOException;
+import java.util.Map;
+
+import javax.security.auth.login.Configuration;
+
 import org.apache.thrift7.TProcessor;
 import org.apache.thrift7.server.TServer;
 import org.apache.thrift7.transport.TTransport;
 import org.apache.thrift7.transport.TTransportException;
 
 /**
  * Interface for Thrift Transport plugin
- * 
- * Each plugin should have a constructor 
- *  Foo(Configuration login_conf)
  */
 public interface ITransportPlugin {
+    /**
+     * Invoked once immediately after construction
+     * @param storm_conf Storm configuration 
+     * @param login_conf login configuration
+     */
+    void prepare(Map storm_conf, Configuration login_conf);
+
     /**
      * Create a server associated with a given port and service handler
      * @param port listening port

diff --git a/src/jvm/backtype/storm/security/auth/SaslTransportPlugin.java b/src/jvm/backtype/storm/security/auth/SaslTransportPlugin.java
@@ -3,6 +3,8 @@
 import java.io.IOException;
 import java.net.Socket;
 import java.security.Principal;
+import java.util.Map;
+
 import javax.security.auth.Subject;
 import javax.security.auth.login.Configuration;
 import javax.security.sasl.SaslServer;
@@ -29,9 +31,11 @@ public abstract class SaslTransportPlugin implements ITransportPlugin {
     private static final Logger LOG = LoggerFactory.getLogger(SaslTransportPlugin.class);
 
     /**
-     * constructor
+     * Invoked once immediately after construction
+     * @param conf Storm configuration 
+     * @param login_conf login configuration
      */
-    public SaslTransportPlugin(Configuration login_conf) {
+    public void prepare(Map storm_conf, Configuration login_conf) {        
         this.login_conf = login_conf;
     }
 
@@ -92,7 +96,6 @@ public boolean process(final TProtocol inProt, final TProtocol outProt) throws T
             //remote subject 
             SaslServer saslServer = saslTrans.getSaslServer();
             String authId = saslServer.getAuthorizationID();
-            LOG.debug("AUTH ID ======>" + authId);
             Subject remoteUser = new Subject();
             remoteUser.getPrincipals().add(new User(authId));
             req_context.setSubject(remoteUser);

diff --git a/src/jvm/backtype/storm/security/auth/SimpleTransportPlugin.java b/src/jvm/backtype/storm/security/auth/SimpleTransportPlugin.java
@@ -4,22 +4,21 @@
 import java.net.InetAddress;
 import java.net.Socket;
 import java.net.UnknownHostException;
+import java.util.Map;
+
 import javax.security.auth.login.Configuration;
 import org.apache.thrift7.TException;
 import org.apache.thrift7.TProcessor;
 import org.apache.thrift7.protocol.TBinaryProtocol;
 import org.apache.thrift7.protocol.TProtocol;
 import org.apache.thrift7.server.THsHaServer;
 import org.apache.thrift7.server.TServer;
-import org.apache.thrift7.server.TThreadPoolServer;
 import org.apache.thrift7.transport.TFramedTransport;
 import org.apache.thrift7.transport.TMemoryInputTransport;
 import org.apache.thrift7.transport.TNonblockingServerSocket;
-import org.apache.thrift7.transport.TServerSocket;
 import org.apache.thrift7.transport.TSocket;
 import org.apache.thrift7.transport.TTransport;
 import org.apache.thrift7.transport.TTransportException;
-import org.apache.thrift7.transport.TTransportFactory;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -33,9 +32,11 @@ public class SimpleTransportPlugin implements ITransportPlugin {
     private static final Logger LOG = LoggerFactory.getLogger(SimpleTransportPlugin.class);
 
     /**
-     * constructor
+     * Invoked once immediately after construction
+     * @param conf Storm configuration 
+     * @param login_conf login configuration
      */
-    public SimpleTransportPlugin(Configuration login_conf) {
+    public void prepare(Map storm_conf, Configuration login_conf) {        
         this.login_conf = login_conf;
     }
 
@@ -100,14 +101,7 @@ public boolean process(final TProtocol inProt, final TProtocol outProt) throws T
             req_context.setSubject(null);
 
             //invoke service handler
-            try {
-                return wrapped.process(inProt, outProt);
-            } catch (RuntimeException ex) {
-                LOG.info(ex.getMessage());
-                return false;
-            }
+            return wrapped.process(inProt, outProt);
         }
     } 
-
-
 }
diff --git a/src/jvm/backtype/storm/security/auth/ThriftClient.java b/src/jvm/backtype/storm/security/auth/ThriftClient.java
@@ -45,7 +45,6 @@ public ThriftClient(Map storm_conf, String host, int port, Integer timeout) thro
             //establish client-server transport via plugin
             _transport =  transportPlugin.connect(underlyingTransport, host); 
         } catch (IOException ex) {
-            LOG.info(ex.getMessage(), ex);
             throw new RuntimeException(ex);
         }
         _protocol = null;

diff --git a/src/jvm/backtype/storm/security/auth/ThriftServer.java b/src/jvm/backtype/storm/security/auth/ThriftServer.java
@@ -47,7 +47,7 @@ public void serve()  {
         } catch (Exception ex) {
             LOG.error("ThriftServer is being stopped due to: " + ex, ex);
             if (_server != null) _server.stop();
-            System.exit(1); //shutdown server process since we could not handle Thrift requests any more
+            Runtime.getRuntime().halt(1); //shutdown server process since we could not handle Thrift requests any more
         }
     }
 }
diff --git a/src/jvm/backtype/storm/security/auth/authorizer/DenyAuthorizer.java b/src/jvm/backtype/storm/security/auth/authorizer/DenyAuthorizer.java
@@ -1,5 +1,7 @@
 package backtype.storm.security.auth.authorizer;
 
+import java.util.Map;
+
 import backtype.storm.Config;
 import backtype.storm.security.auth.IAuthorizer;
 import backtype.storm.security.auth.ReqContext;
@@ -12,6 +14,13 @@
  */
 public class DenyAuthorizer implements IAuthorizer {
     private static final Logger LOG = LoggerFactory.getLogger(DenyAuthorizer.class);
+
+    /**
+     * Invoked once immediately after construction
+     * @param conf Stom configuration 
+     */
+    public void prepare(Map conf) {        
+    }
 
     /**
      * permit() method is invoked for each incoming Thrift request

diff --git a/src/jvm/backtype/storm/security/auth/authorizer/NoopAuthorizer.java b/src/jvm/backtype/storm/security/auth/authorizer/NoopAuthorizer.java
@@ -1,5 +1,7 @@
 package backtype.storm.security.auth.authorizer;
 
+import java.util.Map;
+
 import backtype.storm.Config;
 import backtype.storm.security.auth.IAuthorizer;
 import backtype.storm.security.auth.ReqContext;
@@ -13,6 +15,13 @@
 public class NoopAuthorizer implements IAuthorizer {
     private static final Logger LOG = LoggerFactory.getLogger(NoopAuthorizer.class);
 
+    /**
+     * Invoked once immediately after construction
+     * @param conf Stom configuration 
+     */
+    public void prepare(Map conf) {        
+    }
+
     /**
      * permit() method is invoked for each incoming Thrift request
      * @param contrext request context includes info about

diff --git a/src/jvm/backtype/storm/security/auth/digest/ClientCallbackHandler.java b/src/jvm/backtype/storm/security/auth/digest/ClientCallbackHandler.java
@@ -39,7 +39,6 @@ public ClientCallbackHandler(Configuration configuration) throws IOException {
         if (configurationEntries == null) {
             String errorMessage = "Could not find a '"+AuthUtils.LOGIN_CONTEXT_CLIENT
                     + "' entry in this configuration: Client cannot start.";
-            LOG.error(errorMessage);
             throw new IOException(errorMessage);
         }
 

diff --git a/src/jvm/backtype/storm/security/auth/digest/DigestSaslTransportPlugin.java b/src/jvm/backtype/storm/security/auth/digest/DigestSaslTransportPlugin.java
@@ -1,6 +1,8 @@
 package backtype.storm.security.auth.digest;
 
 import java.io.IOException;
+import java.util.Map;
+
 import javax.security.auth.callback.CallbackHandler;
 import javax.security.auth.login.Configuration;
 
@@ -19,13 +21,6 @@ public class DigestSaslTransportPlugin extends SaslTransportPlugin {
     public static final String DIGEST = "DIGEST-MD5";
     private static final Logger LOG = LoggerFactory.getLogger(DigestSaslTransportPlugin.class);
 
-    /**
-     * constructor
-     */
-    public DigestSaslTransportPlugin(Configuration login_conf) {
-        super(login_conf);
-    }
-
     protected TTransportFactory getServerTransportFactory() throws IOException {        
         //create an authentication callback handler
         CallbackHandler serer_callback_handler = new ServerCallbackHandler(login_conf);

diff --git a/src/jvm/backtype/storm/security/auth/digest/ServerCallbackHandler.java b/src/jvm/backtype/storm/security/auth/digest/ServerCallbackHandler.java
@@ -34,7 +34,6 @@ public ServerCallbackHandler(Configuration configuration) throws IOException {
         AppConfigurationEntry configurationEntries[] = configuration.getAppConfigurationEntry(AuthUtils.LOGIN_CONTEXT_SERVER);
         if (configurationEntries == null) {
             String errorMessage = "Could not find a '"+AuthUtils.LOGIN_CONTEXT_SERVER+"' entry in this configuration: Server cannot start.";
-            LOG.error(errorMessage);
             throw new IOException(errorMessage);
         }
         credentials.clear();

diff --git a/test/clj/backtype/storm/security/auth/auth_test.clj b/test/clj/backtype/storm/security/auth/auth_test.clj
@@ -16,21 +16,22 @@
 
 (def nimbus-timeout (Integer. 30))
 
-(defn mk-authorization-handler [conf]
-  (let [klassname (conf NIMBUS-AUTHORIZER) 
+(defn mk-authorization-handler [storm-conf]
+  (let [klassname (storm-conf NIMBUS-AUTHORIZER) 
         aznClass (if klassname (Class/forName klassname))
         aznHandler (if aznClass (.newInstance aznClass))] 
+    (if aznHandler (.prepare aznHandler storm-conf))
     (log-debug "authorization class name:" klassname
                " class:" aznClass
                " handler:" aznHandler)
     aznHandler
     ))
 
-(defn nimbus-data [conf inimbus]
+(defn nimbus-data [storm-conf inimbus]
   (let [forced-scheduler (.getForcedScheduler inimbus)]
-    {:conf conf
+    {:conf storm-conf
      :inimbus inimbus
-     :authorization-handler (mk-authorization-handler conf)
+     :authorization-handler (mk-authorization-handler storm-conf)
      :submitted-count (atom 0)
      :storm-cluster-state nil
      :submit-lock (Object.)