ambari-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From lpus...@apache.org
Subject [53/71] [abbrv] ambari git commit: AMBARI-2130 ldap connections handled in thefacade. Code cleanup
Date Thu, 26 Oct 2017 09:32:07 GMT
AMBARI-2130 ldap connections handled in thefacade. Code cleanup


Project: http://git-wip-us.apache.org/repos/asf/ambari/repo
Commit: http://git-wip-us.apache.org/repos/asf/ambari/commit/35b3cb42
Tree: http://git-wip-us.apache.org/repos/asf/ambari/tree/35b3cb42
Diff: http://git-wip-us.apache.org/repos/asf/ambari/diff/35b3cb42

Branch: refs/heads/feature-branch-AMBARI-21307
Commit: 35b3cb4288d21c1f505e14bd989e404d3d277a5f
Parents: a08a681
Author: lpuskas <lpuskas@apache.org>
Authored: Tue Sep 12 15:38:25 2017 +0200
Committer: lpuskas <lpuskas@apache.org>
Committed: Thu Oct 26 11:28:49 2017 +0200

----------------------------------------------------------------------
 .../server/ldap/service/AmbariLdapFacade.java   | 51 +++++++++----
 .../ldap/service/LdapConnectionService.java     | 12 ++-
 .../ambari/server/ldap/service/LdapFacade.java  |  2 +-
 .../ads/DefaultLdapConfigurationService.java    | 77 ++++----------------
 .../ads/DefaultLdapConnectionService.java       | 41 ++++++++++-
 .../OccurranceAndWeightBasedDetector.java       |  2 +-
 6 files changed, 103 insertions(+), 82 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/ambari/blob/35b3cb42/ambari-server/src/main/java/org/apache/ambari/server/ldap/service/AmbariLdapFacade.java
----------------------------------------------------------------------
diff --git a/ambari-server/src/main/java/org/apache/ambari/server/ldap/service/AmbariLdapFacade.java
b/ambari-server/src/main/java/org/apache/ambari/server/ldap/service/AmbariLdapFacade.java
index f159418..d2bdef3 100644
--- a/ambari-server/src/main/java/org/apache/ambari/server/ldap/service/AmbariLdapFacade.java
+++ b/ambari-server/src/main/java/org/apache/ambari/server/ldap/service/AmbariLdapFacade.java
@@ -19,7 +19,6 @@ import java.util.Map;
 import java.util.Set;
 
 import javax.inject.Inject;
-import javax.inject.Provider;
 import javax.inject.Singleton;
 
 import org.apache.ambari.server.ldap.AmbariLdapConfiguration;
@@ -58,35 +57,59 @@ public class AmbariLdapFacade implements LdapFacade {
   @Inject
   private LdapAttributeDetectionService ldapAttributeDetectionService;
 
-  //todo remove this, added for testing purposes only
-  @Inject
-  private Provider<AmbariLdapConfiguration> ambariLdapConfigurationProvider;
-
   @Inject
   public AmbariLdapFacade() {
   }
 
   @Override
   public void checkConnection(AmbariLdapConfiguration ambariLdapConfiguration) throws AmbariLdapException
{
+    LdapConnection connection = null;
     try {
+
       LOGGER.info("Validating LDAP connection related configuration based on: {}", ambariLdapConfiguration);
-      LdapConnection connection = ldapConnectionService.createLdapConnection(ambariLdapConfiguration);
+      connection = ldapConnectionService.getBoundLdapConnection(ambariLdapConfiguration);
       ldapConfigurationService.checkConnection(connection, ambariLdapConfiguration);
-    } catch (AmbariLdapException e) {
+      LOGGER.info("Validating LDAP connection related configuration: SUCCESS");
+
+    } catch (Exception e) {
+
       LOGGER.error("Validating LDAP connection configuration failed", e);
-      throw e;
+      throw new AmbariLdapException(e);
+
+    } finally {
+      try {
+        connection.unBind();
+        connection.close();
+      } catch (Exception e) {
+        throw new AmbariLdapException(e);
+      }
     }
-    LOGGER.info("Validating LDAP connection related configuration: SUCCESS");
+
   }
 
 
   @Override
-  public AmbariLdapConfiguration detectAttributes(AmbariLdapConfiguration ambariLdapConfiguration)
{
+  public AmbariLdapConfiguration detectAttributes(AmbariLdapConfiguration ambariLdapConfiguration)
throws AmbariLdapException {
     LOGGER.info("Detecting LDAP configuration attributes ...");
 
-    LdapConnection connection = ldapConnectionService.createLdapConnection(ambariLdapConfiguration);
-    ambariLdapConfiguration = ldapAttributeDetectionService.detectLdapUserAttributes(connection,
ambariLdapConfiguration);
-    return ambariLdapConfiguration;
+    LdapConnection connection = ldapConnectionService.getBoundLdapConnection(ambariLdapConfiguration);
+    try {
+
+      ambariLdapConfiguration = ldapAttributeDetectionService.detectLdapUserAttributes(connection,
ambariLdapConfiguration);
+      ambariLdapConfiguration = ldapAttributeDetectionService.detectLdapGroupAttributes(connection,
ambariLdapConfiguration);
+      return ambariLdapConfiguration;
+
+    } catch (Exception e) {
+      LOGGER.error("Error during LDAP attribute detection", e);
+      throw new AmbariLdapException(e);
+    } finally {
+      try {
+        connection.unBind();
+        connection.close();
+      } catch (Exception e) {
+        throw new AmbariLdapException(e);
+      }
+    }
   }
 
   @Override
@@ -98,7 +121,7 @@ public class AmbariLdapFacade implements LdapFacade {
       throw new IllegalArgumentException("No test user available for testing LDAP attributes");
     }
 
-    LdapConnection ldapConnection = ldapConnectionService.createLdapConnection(ldapConfiguration);
+    LdapConnection ldapConnection = ldapConnectionService.getBoundLdapConnection(ldapConfiguration);
 
     LOGGER.info("Testing LDAP user attributes with test user: {}", userName);
     String userDn = ldapConfigurationService.checkUserAttributes(ldapConnection, userName,
testUserPass, ldapConfiguration);

http://git-wip-us.apache.org/repos/asf/ambari/blob/35b3cb42/ambari-server/src/main/java/org/apache/ambari/server/ldap/service/LdapConnectionService.java
----------------------------------------------------------------------
diff --git a/ambari-server/src/main/java/org/apache/ambari/server/ldap/service/LdapConnectionService.java
b/ambari-server/src/main/java/org/apache/ambari/server/ldap/service/LdapConnectionService.java
index 50ee8ed..b4daeaa 100644
--- a/ambari-server/src/main/java/org/apache/ambari/server/ldap/service/LdapConnectionService.java
+++ b/ambari-server/src/main/java/org/apache/ambari/server/ldap/service/LdapConnectionService.java
@@ -15,7 +15,7 @@
 package org.apache.ambari.server.ldap.service;
 
 import org.apache.ambari.server.ldap.AmbariLdapConfiguration;
-import org.apache.directory.ldap.client.api.LdapNetworkConnection;
+import org.apache.directory.ldap.client.api.LdapConnection;
 
 /**
  * Contract defining factory methods for creating LDAP connection instances.
@@ -29,7 +29,15 @@ public interface LdapConnectionService {
    * @param ambariLdapConfiguration configuration instance with information for creating
the connection instance
    * @return a set up LdapConnection instance
    */
-  LdapNetworkConnection createLdapConnection(AmbariLdapConfiguration ambariLdapConfiguration);
+  LdapConnection createLdapConnection(AmbariLdapConfiguration ambariLdapConfiguration);
+
+  /**
+   * Creates an LdapConnection instance and binds to the LDAP server based on the provided
configuration entries
+   *
+   * @param ambariLdapConfiguration ambari configuration instance
+   * @return
+   */
+  LdapConnection getBoundLdapConnection(AmbariLdapConfiguration ambariLdapConfiguration);
 
 
 }

http://git-wip-us.apache.org/repos/asf/ambari/blob/35b3cb42/ambari-server/src/main/java/org/apache/ambari/server/ldap/service/LdapFacade.java
----------------------------------------------------------------------
diff --git a/ambari-server/src/main/java/org/apache/ambari/server/ldap/service/LdapFacade.java
b/ambari-server/src/main/java/org/apache/ambari/server/ldap/service/LdapFacade.java
index 7cd25da..6060d7f 100644
--- a/ambari-server/src/main/java/org/apache/ambari/server/ldap/service/LdapFacade.java
+++ b/ambari-server/src/main/java/org/apache/ambari/server/ldap/service/LdapFacade.java
@@ -39,7 +39,7 @@ public interface LdapFacade {
    *
    * @param ambariLdapConfiguration
    */
-  AmbariLdapConfiguration detectAttributes(AmbariLdapConfiguration ambariLdapConfiguration);
+  AmbariLdapConfiguration detectAttributes(AmbariLdapConfiguration ambariLdapConfiguration)
throws AmbariLdapException;
 
   /**
    * Checks user and group related LDAP configuration attributes in the configuration object
with the help of the provided parameters

http://git-wip-us.apache.org/repos/asf/ambari/blob/35b3cb42/ambari-server/src/main/java/org/apache/ambari/server/ldap/service/ads/DefaultLdapConfigurationService.java
----------------------------------------------------------------------
diff --git a/ambari-server/src/main/java/org/apache/ambari/server/ldap/service/ads/DefaultLdapConfigurationService.java
b/ambari-server/src/main/java/org/apache/ambari/server/ldap/service/ads/DefaultLdapConfigurationService.java
index fa2e44b..5735d7d 100644
--- a/ambari-server/src/main/java/org/apache/ambari/server/ldap/service/ads/DefaultLdapConfigurationService.java
+++ b/ambari-server/src/main/java/org/apache/ambari/server/ldap/service/ads/DefaultLdapConfigurationService.java
@@ -14,24 +14,20 @@
 
 package org.apache.ambari.server.ldap.service.ads;
 
-import java.io.IOException;
 import java.util.List;
 import java.util.Set;
 
 import javax.inject.Inject;
 import javax.inject.Singleton;
 
-import org.apache.ambari.server.AmbariException;
 import org.apache.ambari.server.ldap.AmbariLdapConfiguration;
 import org.apache.ambari.server.ldap.LdapConfigurationService;
 import org.apache.ambari.server.ldap.service.AmbariLdapException;
-import org.apache.ambari.server.ldap.service.LdapConnectionService;
 import org.apache.directory.api.ldap.codec.decorators.SearchResultEntryDecorator;
 import org.apache.directory.api.ldap.model.constants.SchemaConstants;
 import org.apache.directory.api.ldap.model.cursor.EntryCursor;
 import org.apache.directory.api.ldap.model.cursor.SearchCursor;
 import org.apache.directory.api.ldap.model.entry.Entry;
-import org.apache.directory.api.ldap.model.exception.LdapException;
 import org.apache.directory.api.ldap.model.message.Response;
 import org.apache.directory.api.ldap.model.message.SearchRequest;
 import org.apache.directory.api.ldap.model.message.SearchRequestImpl;
@@ -53,9 +49,6 @@ public class DefaultLdapConfigurationService implements LdapConfigurationService
 
   private static final Logger LOGGER = LoggerFactory.getLogger(DefaultLdapConfigurationService.class);
 
-  @Inject
-  private LdapConnectionService ldapConnectionService;
-
   /**
    * Facilitating the instantiation
    */
@@ -65,12 +58,12 @@ public class DefaultLdapConfigurationService implements LdapConfigurationService
 
   @Override
   public void checkConnection(LdapConnection ldapConnection, AmbariLdapConfiguration ambariLdapConfiguration)
throws AmbariLdapException {
-    try {
-      bind(ambariLdapConfiguration, ldapConnection);
-    } catch (LdapException e) {
-      LOGGER.error("Could not connect to the LDAP server", e);
-      throw new AmbariLdapException(e);
+
+    if (!ldapConnection.isConnected()) {
+      LOGGER.error("Could not connect to the LDAP server");
+      throw new AmbariLdapException("Could not connect to the LDAP server. Configuration:
" + ambariLdapConfiguration);
     }
+
   }
 
 
@@ -80,22 +73,20 @@ public class DefaultLdapConfigurationService implements LdapConfigurationService
    *
    * Invalid attributes are signaled by throwing an exception.
    *
+   * @param ldapConnection          connection instance used to connect to the LDAP server
    * @param testUserName            the test username
    * @param testPassword            the test password
    * @param ambariLdapConfiguration configuration instance holding ldap configuration details
    * @return the DN of the test user
-   * @throws AmbariException if the attributes are not valid or any errors occurs
+   * @throws AmbariLdapException if an error occurs
    */
   @Override
   public String checkUserAttributes(LdapConnection ldapConnection, String testUserName, String
testPassword, AmbariLdapConfiguration ambariLdapConfiguration) throws AmbariLdapException
{
-    SearchCursor searchCursor = null;
     String userDn = null;
+    EntryCursor entryCursor = null;
     try {
       LOGGER.info("Checking user attributes for user {} r ...", testUserName);
 
-      // bind anonimously or with manager data
-      bind(ambariLdapConfiguration, ldapConnection);
-
       // set up a filter based on the provided attributes
       String filter = FilterBuilder.and(
         FilterBuilder.equal(SchemaConstants.OBJECT_CLASS_AT, ambariLdapConfiguration.userObjectClass()),
@@ -103,7 +94,7 @@ public class DefaultLdapConfigurationService implements LdapConfigurationService
         .toString();
 
       LOGGER.info("Searching for the user: {} using the search filter: {}", testUserName,
filter);
-      EntryCursor entryCursor = ldapConnection.search(new Dn(ambariLdapConfiguration.userSearchBase()),
filter, SearchScope.SUBTREE);
+      entryCursor = ldapConnection.search(new Dn(ambariLdapConfiguration.userSearchBase()),
filter, SearchScope.SUBTREE);
 
       // collecting search result entries
       List<Entry> users = Lists.newArrayList();
@@ -127,7 +118,9 @@ public class DefaultLdapConfigurationService implements LdapConfigurationService
       throw new AmbariLdapException(e.getMessage(), e);
 
     } finally {
-      closeResources(ldapConnection, searchCursor);
+      if (null != entryCursor) {
+        entryCursor.close();
+      }
     }
     return userDn;
   }
@@ -141,8 +134,6 @@ public class DefaultLdapConfigurationService implements LdapConfigurationService
     try {
       LOGGER.info("Checking group attributes for user dn {} ...", userDn);
 
-      bind(ambariLdapConfiguration, ldapConnection);
-
       // set up a filter based on the provided attributes
       String filter = FilterBuilder.and(
         FilterBuilder.equal(SchemaConstants.OBJECT_CLASS_AT, ambariLdapConfiguration.groupObjectClass()),
@@ -171,36 +162,14 @@ public class DefaultLdapConfigurationService implements LdapConfigurationService
       throw new AmbariLdapException(e.getMessage(), e);
 
     } finally {
-      closeResources(ldapConnection, searchCursor);
+      if (null != searchCursor) {
+        searchCursor.close();
+      }
     }
 
     return processGroupResults(groupResponses, ambariLdapConfiguration);
   }
 
-  /**
-   * Binds to the LDAP server (anonimously or wit manager credentials)
-   *
-   * @param ambariLdapConfiguration configuration instance
-   * @param connection              connection instance
-   * @throws LdapException if the bind operation fails
-   */
-  private void bind(AmbariLdapConfiguration ambariLdapConfiguration, LdapConnection connection)
throws LdapException {
-    LOGGER.info("Connecting to LDAP ....");
-    if (!ambariLdapConfiguration.anonymousBind()) {
-      LOGGER.debug("Anonimous binding not supported, binding with the manager detailas...");
-      connection.bind(ambariLdapConfiguration.bindDn(), ambariLdapConfiguration.bindPassword());
-    } else {
-      LOGGER.debug("Binding anonymously ...");
-      connection.bind();
-    }
-
-    if (!connection.isConnected()) {
-      LOGGER.error("Not connected to the LDAP server. Connection instance: {}", connection);
-      throw new IllegalStateException("The connection to the LDAP server is not alive");
-    }
-    LOGGER.info("Connected to LDAP.");
-  }
-
 
   /**
    * Extracts meaningful values from the search result.
@@ -220,22 +189,6 @@ public class DefaultLdapConfigurationService implements LdapConfigurationService
     return groupStrSet;
   }
 
-  private void closeResources(LdapConnection connection, SearchCursor searchCursor) {
-    LOGGER.debug("Housekeeping: closing the connection and the search cursor ...");
-
-    if (null != searchCursor) {
-      // this method is idempotent
-      searchCursor.close();
-    }
-
-    if (null != connection) {
-      try {
-        connection.close();
-      } catch (IOException e) {
-        LOGGER.error("Exception occurred while closing the connection", e);
-      }
-    }
-  }
 
 }
 

http://git-wip-us.apache.org/repos/asf/ambari/blob/35b3cb42/ambari-server/src/main/java/org/apache/ambari/server/ldap/service/ads/DefaultLdapConnectionService.java
----------------------------------------------------------------------
diff --git a/ambari-server/src/main/java/org/apache/ambari/server/ldap/service/ads/DefaultLdapConnectionService.java
b/ambari-server/src/main/java/org/apache/ambari/server/ldap/service/ads/DefaultLdapConnectionService.java
index f39df54..457e23e 100644
--- a/ambari-server/src/main/java/org/apache/ambari/server/ldap/service/ads/DefaultLdapConnectionService.java
+++ b/ambari-server/src/main/java/org/apache/ambari/server/ldap/service/ads/DefaultLdapConnectionService.java
@@ -32,6 +32,7 @@ import javax.inject.Singleton;
 
 import org.apache.ambari.server.ldap.AmbariLdapConfiguration;
 import org.apache.ambari.server.ldap.service.LdapConnectionService;
+import org.apache.directory.ldap.client.api.LdapConnection;
 import org.apache.directory.ldap.client.api.LdapConnectionConfig;
 import org.apache.directory.ldap.client.api.LdapNetworkConnection;
 import org.slf4j.Logger;
@@ -45,18 +46,54 @@ public class DefaultLdapConnectionService implements LdapConnectionService
{
   @Override
   public LdapNetworkConnection createLdapConnection(AmbariLdapConfiguration ambariLdapConfiguration)
{
     LOGGER.debug("Creating ldap connection instance from: {}", ambariLdapConfiguration);
+
     return new LdapNetworkConnection(getLdapConnectionConfig(ambariLdapConfiguration));
   }
 
+  @Override
+  public LdapConnection getBoundLdapConnection(AmbariLdapConfiguration ambariLdapConfiguration)
{
+    LOGGER.info("Creating LDAP connection instance and binding to LDAP server ...");
+
+    try {
+      LdapConnection connection = createLdapConnection(ambariLdapConfiguration);
+
+      if (!ambariLdapConfiguration.anonymousBind()) {
+
+        LOGGER.debug("Anonymous binding not supported, binding with the manager detailas...");
+        connection.bind(ambariLdapConfiguration.bindDn(), ambariLdapConfiguration.bindPassword());
+
+      } else {
+
+        LOGGER.debug("Binding anonymously ...");
+        connection.bind();
+
+      }
+
+      if (!connection.isConnected()) {
+
+        LOGGER.error("Not connected to the LDAP server. Connection instance: {}", connection);
+        throw new IllegalStateException("The connection to the LDAP server is not alive");
+
+      }
+
+      LOGGER.info("Connected / bound to LDAP server.");
+      return connection;
+
+    } catch (Exception e) {
+      LOGGER.error("Could not create or bind LdapConnection", e);
+      throw new IllegalArgumentException(e);
+    }
+
+  }
+
   private LdapConnectionConfig getLdapConnectionConfig(AmbariLdapConfiguration ambariAmbariLdapConfiguration)
{
-    LOGGER.debug("Creating a configuration instance based on the ambari configuration: {}",
ambariAmbariLdapConfiguration);
+    LOGGER.debug("Creating a LDAP connection config instance based on the ambari configuration:
{}", ambariAmbariLdapConfiguration);
 
     LdapConnectionConfig ldapConnectionConfig = new LdapConnectionConfig();
     ldapConnectionConfig.setLdapHost(ambariAmbariLdapConfiguration.serverHost());
     ldapConnectionConfig.setLdapPort(ambariAmbariLdapConfiguration.serverPort());
     ldapConnectionConfig.setUseSsl(ambariAmbariLdapConfiguration.useSSL());
 
-    // todo set the other values as required
     return ldapConnectionConfig;
   }
 

http://git-wip-us.apache.org/repos/asf/ambari/blob/35b3cb42/ambari-server/src/main/java/org/apache/ambari/server/ldap/service/ads/detectors/OccurranceAndWeightBasedDetector.java
----------------------------------------------------------------------
diff --git a/ambari-server/src/main/java/org/apache/ambari/server/ldap/service/ads/detectors/OccurranceAndWeightBasedDetector.java
b/ambari-server/src/main/java/org/apache/ambari/server/ldap/service/ads/detectors/OccurranceAndWeightBasedDetector.java
index 8aaf6c1..71dfb42 100644
--- a/ambari-server/src/main/java/org/apache/ambari/server/ldap/service/ads/detectors/OccurranceAndWeightBasedDetector.java
+++ b/ambari-server/src/main/java/org/apache/ambari/server/ldap/service/ads/detectors/OccurranceAndWeightBasedDetector.java
@@ -78,7 +78,7 @@ public abstract class OccurranceAndWeightBasedDetector implements AttributeDetec
 
   @Override
   public void collect(Entry entry) {
-    LOGGER.info("Collecting ldap attributes/values form entry with dn: [{]]", entry.getDn());
+    LOGGER.info("Collecting ldap attributes/values form entry with dn: [{}]", entry.getDn());
 
     for (String attributeValue : occurranceMap().keySet()) {
       if (applies(entry, attributeValue)) {


Mime
View raw message