Skip to content

Commit

Permalink
Improved ServerSet health check reliability
Browse files Browse the repository at this point in the history
Updated the LDAP SDK's ServerSet implementations so that they can
perform bind and post-connect processing on the connections that
they create.  Previously, server sets were only intended to
establish connections, but not to authenticate them, and not to
perform any other post-processing on them (like using the StartTLS
extended operation to convert an insecure connection to a secure
one).  It was up to the caller (which was often a connection pool)
to perform that processing once it got the connection back from the
server set.

However, when asked to create a connection, a server set can be
given an LDAPConnectionPoolHealthCheck to use to check the validity
of the connection that it has created.  If such a health check was
provided, then it would have always been invoked on an
unauthenticated connection, which could cause problems against
servers that do not permit unauthenticated requests.  Further, if
some post-connect processing (for example, the StartTLS operation)
is needed on those connections, then the health checking would
have been performed before that processing had been completed, and
that could also lead to erroneous behavior.

With this update, server sets can now be created so that they
themselves perform authentication and post-connect processing on
connections before they make those connections available to the
caller.  More importantly, if provided with a health check, then any
appropriate authentication and post-connect processing will have
been performed on the connection before the health check is invoked.
This makes the health check more reliable because the connection
can be in a more useful state.  When a server set configured with
support for bind and post-connect processing is used in conjunction
with a connection pool, the pool will now delegate that processing
to the server set.  If the associated server set is not configured
to perform authentication and post-connect processing, then the
connection pool will still perform those tasks.

This update also fixes a potential problem that could lead to the
connection pool being unable to obtain a connection for processing
an operation.  Most server set implementations can be used in
conjunction with multiple servers, so that if it fails to establish
a usable connection to one server, it can try to obtain a
connection from a different server and shield the caller from the
connection problem.  However, this only applies to the portion of
the processing that is performed within the server set itself.
When the authentication and post-connect processing were left up to
the caller of the server set, the caller was more likely to
encounter a problem that it could not work around.  Now that the
additional processing is performed by the server set, its
resiliency can be extended to this authentication and post-connect
processing.
  • Loading branch information
dirmgr committed Jan 22, 2018
1 parent feba120 commit fb304c5
Show file tree
Hide file tree
Showing 13 changed files with 1,106 additions and 181 deletions.
10 changes: 10 additions & 0 deletions docs/release-notes.html
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,16 @@ <h3>Version 4.0.4</h3>
<br><br>
</li>

<li>
Updated the LDAP SDK's <tt>ServerSet</tt> implementations so that they can
perform authentication and post-connect processing. This can improve resiliency
against certain types of certain types of processing failures, and can allow
health checks against newly established connections to succeed in certain cases
in which they might have previously failed (for example, if the server is
configured to reject requests from unauthenticated clients).
<br><br>
</li>

<li>
Updated the <tt>GetEntryLDAPConnectionPoolHealthCheck</tt> class to provide
support for invoking the health check after a pooled connection has been
Expand Down
10 changes: 9 additions & 1 deletion src/com/unboundid/ldap/sdk/BindResult.java
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,15 @@ public BindResult(final LDAPException exception)
{
super(exception.toLDAPResult());

serverSASLCredentials = null;
if (exception instanceof LDAPBindException)
{
serverSASLCredentials =
((LDAPBindException) exception).getServerSASLCredentials();
}
else
{
serverSASLCredentials = null;
}
}


Expand Down
123 changes: 97 additions & 26 deletions src/com/unboundid/ldap/sdk/DNSSRVRecordServerSet.java
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,10 @@ public final class DNSSRVRecordServerSet



// The bind request to use to authenticate connections created by this
// server set.
private final BindRequest bindRequest;

// The properties that will be used to initialize the JNDI context.
private final Hashtable<String,String> jndiProperties;

Expand All @@ -140,6 +144,10 @@ public final class DNSSRVRecordServerSet
// information should be considered valid.
private final long ttlMillis;

// The post-connect processor to invoke against connections created by this
// server set.
private final PostConnectProcessor postConnectProcessor;

// The socket factory that should be used to create connections.
private final SocketFactory socketFactory;

Expand Down Expand Up @@ -247,8 +255,65 @@ public DNSSRVRecordServerSet(final String recordName,
final SocketFactory socketFactory,
final LDAPConnectionOptions connectionOptions)
{
this.socketFactory = socketFactory;
this(recordName, providerURL, jndiProperties, ttlMillis, socketFactory,
connectionOptions, null, null);
}



/**
* Creates a new instance of this server set that will use the provided
* settings.
*
* @param recordName The name of the DNS SRV record to retrieve.
* If this is {@code null}, then a default
* record name of "_ldap._tcp" will be used.
* @param providerURL The JNDI provider URL that may be used to
* specify the DNS server(s) to use. If this is
* not specified, then a default URL of
* "dns:" will be used, which will attempt to
* determine the appropriate servers from the
* underlying system configuration.
* @param jndiProperties A set of JNDI-related properties that should
* be be used when initializing the context for
* interacting with the DNS server via JNDI.
* If this is {@code null}, then a default set
* of properties will be used.
* @param ttlMillis Specifies the maximum length of time in
* milliseconds that DNS information should be
* cached before it needs to be retrieved
* again. A value less than or equal to zero
* will use the default TTL of one hour.
* @param socketFactory The socket factory that will be used when
* creating connections. It may be
* {@code null} if the JVM-default socket
* factory should be used.
* @param connectionOptions The set of connection options that should be
* used for the connections that are created.
* It may be {@code null} if the default
* connection options should be used.
* @param bindRequest The bind request that should be used to
* authenticate newly-established connections.
* It may be {@code null} if this server set
* should not perform any authentication.
* @param postConnectProcessor The post-connect processor that should be
* invoked on newly-established connections. It
* may be {@code null} if this server set should
* not perform any post-connect processing.
*/
public DNSSRVRecordServerSet(final String recordName,
final String providerURL,
final Properties jndiProperties,
final long ttlMillis,
final SocketFactory socketFactory,
final LDAPConnectionOptions connectionOptions,
final BindRequest bindRequest,
final PostConnectProcessor postConnectProcessor)
{
this.socketFactory = socketFactory;
this.connectionOptions = connectionOptions;
this.bindRequest = bindRequest;
this.postConnectProcessor = postConnectProcessor;

recordSet = null;

Expand Down Expand Up @@ -388,6 +453,28 @@ public LDAPConnectionOptions getConnectionOptions()



/**
* {@inheritDoc}
*/
@Override()
public boolean includesAuthentication()
{
return (bindRequest != null);
}



/**
* {@inheritDoc}
*/
@Override()
public boolean includesPostConnectProcessing()
{
return (postConnectProcessor != null);
}



/**
* {@inheritDoc}
*/
Expand Down Expand Up @@ -439,11 +526,13 @@ public LDAPConnection getConnection(
LDAPException firstException = null;
for (final SRVRecord r : recordSet.getOrderedRecords())
{
final LDAPConnection conn;
try
{
conn = new LDAPConnection(socketFactory, connectionOptions,
r.getAddress(), r.getPort());
final LDAPConnection connection = new LDAPConnection(socketFactory,
connectionOptions, r.getAddress(), r.getPort());
doBindPostConnectAndHealthCheckProcessing(connection, bindRequest,
postConnectProcessor, healthCheck);
return connection;
}
catch (final LDAPException le)
{
Expand All @@ -452,29 +541,7 @@ public LDAPConnection getConnection(
{
firstException = le;
}

continue;
}

if (healthCheck != null)
{
try
{
healthCheck.ensureNewConnectionValid(conn);
}
catch (final LDAPException le)
{
Debug.debugException(le);
if (firstException == null)
{
firstException = le;
}

continue;
}
}

return conn;
}

// If we've gotten here, then we couldn't connect to any of the servers.
Expand Down Expand Up @@ -510,6 +577,10 @@ public void toString(final StringBuilder buffer)
connectionOptions.toString(buffer);
}

buffer.append(", includesAuthentication=");
buffer.append(bindRequest != null);
buffer.append(", includesPostConnectProcessing=");
buffer.append(postConnectProcessor != null);
buffer.append(')');
}
}
131 changes: 120 additions & 11 deletions src/com/unboundid/ldap/sdk/FailoverServerSet.java
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import com.unboundid.util.ThreadSafetyLevel;

import static com.unboundid.util.Debug.*;
import static com.unboundid.util.StaticUtils.*;
import static com.unboundid.util.Validator.*;


Expand Down Expand Up @@ -253,6 +254,46 @@ public FailoverServerSet(final String[] addresses, final int[] ports,
public FailoverServerSet(final String[] addresses, final int[] ports,
final SocketFactory socketFactory,
final LDAPConnectionOptions connectionOptions)
{
this(addresses, ports, socketFactory, connectionOptions, null, null);
}



/**
* Creates a new failover server set with the specified set of directory
* server addresses and port numbers. It will use the provided socket factory
* to create the underlying sockets.
*
* @param addresses The addresses of the directory servers to
* which the connections should be established.
* It must not be {@code null} or empty.
* @param ports The ports of the directory servers to which
* the connections should be established. It
* must not be {@code null}, and it must have
* the same number of elements as the
* {@code addresses} array. The order of
* elements in the {@code addresses} array must
* correspond to the order of elements in the
* {@code ports} array.
* @param socketFactory The socket factory to use to create the
* underlying connections.
* @param connectionOptions The set of connection options to use for the
* underlying connections.
* @param bindRequest The bind request that should be used to
* authenticate newly-established connections.
* It may be {@code null} if this server set
* should not perform any authentication.
* @param postConnectProcessor The post-connect processor that should be
* invoked on newly-established connections. It
* may be {@code null} if this server set should
* not perform any post-connect processing.
*/
public FailoverServerSet(final String[] addresses, final int[] ports,
final SocketFactory socketFactory,
final LDAPConnectionOptions connectionOptions,
final BindRequest bindRequest,
final PostConnectProcessor postConnectProcessor)
{
ensureNotNull(addresses, ports);
ensureTrue(addresses.length > 0,
Expand Down Expand Up @@ -286,7 +327,8 @@ public FailoverServerSet(final String[] addresses, final int[] ports,
serverSets = new ServerSet[addresses.length];
for (int i=0; i < serverSets.length; i++)
{
serverSets[i] = new SingleServerSet(addresses[i], ports[i], sf, co);
serverSets[i] = new SingleServerSet(addresses[i], ports[i], sf, co,
bindRequest, postConnectProcessor);
}
}

Expand All @@ -297,18 +339,16 @@ public FailoverServerSet(final String[] addresses, final int[] ports,
* server sets.
*
* @param serverSets The server sets between which failover should occur.
* It must not be {@code null} or empty.
* It must not be {@code null} or empty. All of the
* provided sets must have the same return value for their
* {@link #includesAuthentication()} method, and all of
* the provided sets must have the same return value for
* their {@link #includesPostConnectProcessing()}
* method.
*/
public FailoverServerSet(final ServerSet... serverSets)
{
ensureNotNull(serverSets);
ensureFalse(serverSets.length == 0,
"FailoverServerSet.serverSets must not be empty.");

this.serverSets = serverSets;

reOrderOnFailover = new AtomicBoolean(false);
maxFailoverConnectionAge = null;
this(toList(serverSets));
}


Expand All @@ -318,7 +358,12 @@ public FailoverServerSet(final ServerSet... serverSets)
* server sets.
*
* @param serverSets The server sets between which failover should occur.
* It must not be {@code null} or empty.
* It must not be {@code null} or empty. All of the
* provided sets must have the same return value for their
* {@link #includesAuthentication()} method, and all of
* the provided sets must have the same return value for
* their {@link #includesPostConnectProcessing()}
* method.
*/
public FailoverServerSet(final List<ServerSet> serverSets)
{
Expand All @@ -329,6 +374,48 @@ public FailoverServerSet(final List<ServerSet> serverSets)
this.serverSets = new ServerSet[serverSets.size()];
serverSets.toArray(this.serverSets);

boolean anySupportsAuthentication = false;
boolean allSupportAuthentication = true;
boolean anySupportsPostConnectProcessing = false;
boolean allSupportPostConnectProcessing = true;
for (final ServerSet serverSet : this.serverSets)
{
if (serverSet.includesAuthentication())
{
anySupportsAuthentication = true;
}
else
{
allSupportAuthentication = false;
}

if (serverSet.includesPostConnectProcessing())
{
anySupportsPostConnectProcessing = true;
}
else
{
allSupportPostConnectProcessing = false;
}
}

if (anySupportsAuthentication)
{
ensureTrue(allSupportAuthentication,
"When creating a FailoverServerSet from a collection of server " +
"sets, either all of those sets must include authentication, " +
"or none of those sets may include authentication.");
}

if (anySupportsPostConnectProcessing)
{
ensureTrue(allSupportPostConnectProcessing,
"When creating a FailoverServerSet from a collection of server " +
"sets, either all of those sets must include post-connect " +
"processing, or none of those sets may include post-connect " +
"processing.");
}

reOrderOnFailover = new AtomicBoolean(false);
maxFailoverConnectionAge = null;
}
Expand Down Expand Up @@ -449,6 +536,28 @@ else if (maxFailoverConnectionAge > 0L)



/**
* {@inheritDoc}
*/
@Override()
public boolean includesAuthentication()
{
return serverSets[0].includesAuthentication();
}



/**
* {@inheritDoc}
*/
@Override()
public boolean includesPostConnectProcessing()
{
return serverSets[0].includesPostConnectProcessing();
}



/**
* {@inheritDoc}
*/
Expand Down
Loading

0 comments on commit fb304c5

Please sign in to comment.