Skip to content

Commit

Permalink
Issue #103: Refactor worker and manager to consolidate checkCredentia…
Browse files Browse the repository at this point in the history
…ls and getHost and prevent losing history on invalid credentials.
  • Loading branch information
mk23 committed Apr 20, 2016
1 parent 55e4175 commit 1842dfb
Show file tree
Hide file tree
Showing 5 changed files with 130 additions and 104 deletions.
1 change: 0 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,6 @@ JMXProxy service has the following miscelleneous APIs for convinience and UI bui
Limitations
-----------

* Agents with authentication enabled, have their history purged when connecting from the Web UI.
* SSL agent connections are currently not supported. Remote JVM must be started with `-Dcom.sun.management.jmxremote.ssl=false`.


Expand Down
42 changes: 19 additions & 23 deletions src/main/java/com/github/mk23/jmxproxy/jmx/ConnectionManager.java
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,10 @@ public final AppConfig getConfiguration() {
*
* @return {@link Set} of {@link ConnectionWorker} name {@link String}s.
*/
public final synchronized Set<String> getHosts() {
return hosts.keySet();
public final Set<String> getHosts() {
synchronized (hosts) {
return hosts.keySet();
}
}

/**
Expand All @@ -95,15 +97,17 @@ public final synchronized Set<String> getHosts() {
*
* @throws WebApplicationException if key is not found in the map store.
*/
public final synchronized boolean delHost(final String host) throws WebApplicationException {
if (hosts.containsKey(host)) {
LOG.debug("purging " + host);
hosts.remove(host).shutdown();
} else {
throw new WebApplicationException(Response.Status.NOT_FOUND);
}
public final boolean delHost(final String host) throws WebApplicationException {
synchronized (hosts) {
if (hosts.containsKey(host)) {
LOG.debug("purging " + host);
hosts.remove(host).shutdown();

return true;
return true;
} else {
throw new WebApplicationException(Response.Status.NOT_FOUND);
}
}
}

/**
Expand Down Expand Up @@ -157,29 +161,21 @@ public final Host getHost(

try {
synchronized (hosts) {
if (hosts.containsKey(host) && !hosts.get(host).checkCredentials(auth)) {
LOG.info("resetting credentials for " + host);
hosts.remove(host).shutdown();
}

if (!hosts.containsKey(host)) {
LOG.info("creating new worker for " + host);
ConnectionWorker worker = new ConnectionWorker(
hosts.put(host, new ConnectionWorker(
host,
auth,
config.getCacheDuration().toMilliseconds(),
config.getHistorySize()
);
));

hosts.put(host, worker);
}
return hosts.get(host).getHost(auth);
}

return hosts.get(host).getHost();
} catch (SecurityException e) {
throw new WebApplicationException(Response.Status.UNAUTHORIZED);
} catch (MalformedURLException e) {
throw new WebApplicationException(Response.Status.BAD_REQUEST);
} catch (SecurityException e) {
throw new WebApplicationException(Response.Status.UNAUTHORIZED);
} catch (Exception e) {
throw new WebApplicationException(Response.Status.NOT_FOUND);
}
Expand Down
187 changes: 109 additions & 78 deletions src/main/java/com/github/mk23/jmxproxy/jmx/ConnectionWorker.java
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import java.util.Map;
import java.util.Set;

import java.util.concurrent.CountDownLatch;
import java.util.concurrent.Executors;
import java.util.concurrent.ScheduledExecutorService;
import java.util.concurrent.TimeUnit;
Expand Down Expand Up @@ -44,16 +45,21 @@
public class ConnectionWorker {
private static final Logger LOG = LoggerFactory.getLogger(ConnectionWorker.class);

private Host host = null;
private ConnectionCredentials auth = null;
private Host host;
private ConnectionCredentials authCreds;

private int historySize;
private boolean authValid;
private IOException connError;

private JMXServiceURL url;
private final Object fetchLock;
private final int historySize;
private final long cacheDuration;

private final JMXServiceURL url;

private long accessTime;

private ScheduledExecutorService fetch;
private ScheduledExecutorService pollerSvc;

/**
* <p>Default constructor.</p>
Expand All @@ -63,58 +69,75 @@ public class ConnectionWorker {
* connecting and fetching JMX objects.
*
* @param hostName host:port {@link String} JMX agent target.
* @param auth optional {@link ConnectionCredentials} for the provided JMX agent or null if none.
* @param cacheDuration period in milliseconds for how often to connect to the JMX agent.
* @param historySize number of {@link com.github.mk23.jmxproxy.core.Attribute}s to keep per
* {@link MBean} {@link History}.
*
* @throws MalformedURLException if hostName isn't a valid host:port {@link String}.
* @throws MalformedURLException if the specified host is not a valid host:port {@link String}.
*/
public ConnectionWorker(
final String hostName,
final ConnectionCredentials auth,
final long cacheDuration,
final int historySize
) throws MalformedURLException {
this.auth = auth;
this.historySize = historySize;
this.cacheDuration = cacheDuration;

fetchLock = new Object();
url = new JMXServiceURL("service:jmx:rmi:///jndi/rmi://" + hostName + "/jmxrmi");
fetch = Executors.newSingleThreadScheduledExecutor();

fetch.scheduleAtFixedRate(new Runnable() {
@Override
public void run() {
fetchJMXValues();
}
}, cacheDuration, cacheDuration, TimeUnit.MILLISECONDS);

fetchJMXValues();
}

/**
* <p>Getter for host.<p>
*
* Fetches the tracked {@link Host} object and resets the request access time.
*
* @return {@link Host} as populated by the most recent fetch operation.
*/
public final synchronized Host getHost() {
accessTime = System.currentTimeMillis();
return host;
}

/**
* <p>Verifies tracked credentials.</p>
*
* Checks if the tracked {@link ConnectionCredentials} have changed since worker instantiation.
* @param testCreds optional {@link ConnectionCredentials} for the provided JMX agent or null if none.
*
* @param peer new request {@link ConnectionCredentials} to compare against.
* @return {@link Host} as populated by the most recent fetch operation.
*
* @return true if {@link ConnectionCredentials} are the same between requests, false otherwise.
* @throws IOException if the specified host is unreachable.
* @throws SecurityException if the specified credentials to the host are incorrect.
*/
public final boolean checkCredentials(final ConnectionCredentials peer) {
return auth == peer || auth != null && auth.equals(peer) || peer != null && peer.equals(auth);
public final Host getHost(final ConnectionCredentials testCreds) throws IOException, SecurityException {
if (host == null) {
final CountDownLatch ready = new CountDownLatch(1);

shutdown();

host = new Host();
authCreds = (testCreds != null) ? testCreds : new ConnectionCredentials();

pollerSvc = Executors.newSingleThreadScheduledExecutor();
pollerSvc.scheduleAtFixedRate(new Runnable() {
@Override
public void run() {
try {
fetchJMXValues();
} catch (Exception e) {
e.printStackTrace();
}
ready.countDown();
}
}, 0, cacheDuration, TimeUnit.MILLISECONDS);

try {
ready.await();
} catch (InterruptedException e) {
LOG.error("unable to finish first run", e);
}
}

if (connError != null) {
throw connError;
} else if (!authCreds.equals(testCreds) && authValid || !authValid) {
throw new SecurityException();
} else {
synchronized (fetchLock) {
accessTime = System.currentTimeMillis();
return host;
}
}
}

/**
Expand All @@ -132,81 +155,89 @@ public final boolean isExpired(final long accessDuration) {
}

/**
* <p>Shuts down the scheduled fetcher.</p>
* <p>Stops the scheduled fetcher.</p>
*
* Signals the {@link ScheduledExecutorService} to shutdown for process termination cleanup.
*/
public final void shutdown() {
if (!fetch.isShutdown()) {
fetch.shutdown();
if (pollerSvc != null && !pollerSvc.isShutdown()) {
pollerSvc.shutdown();
}
}

private synchronized void fetchJMXValues() throws SecurityException {
private void fetchJMXValues() {
JMXConnector connection = null;
MBeanServerConnection server = null;

Map<String, Object> environment = new HashMap<String, Object>();
if (auth != null) {
environment.put(JMXConnector.CREDENTIALS, new String[]{auth.getUsername(), auth.getPassword()});
if (authCreds != null) {
environment.put(JMXConnector.CREDENTIALS, new String[]{authCreds.getUsername(), authCreds.getPassword()});
}

try {
LOG.debug("connecting to mbean server " + url);

connection = JMXConnectorFactory.connect(url, environment);
server = connection.getMBeanServerConnection();
synchronized (fetchLock) {
connection = JMXConnectorFactory.connect(url, environment);
server = connection.getMBeanServerConnection();

if (host == null) {
host = new Host();
}
authValid = true;
connError = null;

Set<String> freshMBeans = new HashSet<String>();
Set<String> freshMBeans = new HashSet<String>();

for (ObjectName mbeanName : server.queryNames(null, null)) {
LOG.debug("discovered mbean " + mbeanName);
freshMBeans.add(mbeanName.toString());
for (ObjectName mbeanName : server.queryNames(null, null)) {
LOG.debug("discovered mbean " + mbeanName);
freshMBeans.add(mbeanName.toString());

MBean mbean = host.addMBean(mbeanName.toString());
try {
for (MBeanAttributeInfo attributeObject : server.getMBeanInfo(mbeanName).getAttributes()) {
if (attributeObject.isReadable()) {
try {
History history = mbean.addHistory(attributeObject.getName(), historySize);
history.addAttributeValue(server.getAttribute(mbeanName, attributeObject.getName()));
} catch (java.lang.NullPointerException e) {
LOG.error("failed to add attribute " + attributeObject.toString() + ": " + e);
} catch (java.rmi.UnmarshalException e) {
LOG.error("failed to add attribute " + attributeObject.toString() + ": " + e);
} catch (javax.management.AttributeNotFoundException e) {
LOG.error("failed to add attribute " + attributeObject.toString() + ": " + e);
} catch (javax.management.MBeanException e) {
LOG.error("failed to add attribute " + attributeObject.toString() + ": " + e);
} catch (javax.management.RuntimeMBeanException e) {
if (!(e.getCause() instanceof UnsupportedOperationException)) {
MBean mbean = host.addMBean(mbeanName.toString());
try {
for (MBeanAttributeInfo attributeObject : server.getMBeanInfo(mbeanName).getAttributes()) {
if (attributeObject.isReadable()) {
try {
Object attribute = server.getAttribute(mbeanName, attributeObject.getName());

History history = mbean.addHistory(attributeObject.getName(), historySize);
history.addAttributeValue(attribute);
} catch (java.lang.NullPointerException e) {
LOG.error("failed to add attribute " + attributeObject.toString() + ": " + e);
} catch (java.rmi.UnmarshalException e) {
LOG.error("failed to add attribute " + attributeObject.toString() + ": " + e);
} catch (javax.management.AttributeNotFoundException e) {
LOG.error("failed to add attribute " + attributeObject.toString() + ": " + e);
} catch (javax.management.MBeanException e) {
LOG.error("failed to add attribute " + attributeObject.toString() + ": " + e);
} catch (javax.management.RuntimeMBeanException e) {
if (!(e.getCause() instanceof UnsupportedOperationException)) {
LOG.error("failed to add attribute " + attributeObject.toString() + ": " + e);
}
}
}
}
} catch (javax.management.InstanceNotFoundException e) {
LOG.error("failed to get mbean info for " + mbeanName, e);
} catch (javax.management.IntrospectionException e) {
LOG.error("failed to get mbean info for " + mbeanName, e);
} catch (javax.management.ReflectionException e) {
LOG.error("failed to get mbean info for " + mbeanName, e);
}
} catch (javax.management.InstanceNotFoundException e) {
LOG.error("failed to get mbean info for " + mbeanName, e);
} catch (javax.management.IntrospectionException e) {
LOG.error("failed to get mbean info for " + mbeanName, e);
} catch (javax.management.ReflectionException e) {
LOG.error("failed to get mbean info for " + mbeanName, e);
}
}

Set<String> staleMBeans = new HashSet<String>(host.getMBeans());
staleMBeans.removeAll(freshMBeans);
for (String mbeanName : staleMBeans) {
host.removeMBean(mbeanName);
LOG.debug("removed stale mbean " + mbeanName);
Set<String> staleMBeans = new HashSet<String>(host.getMBeans());
staleMBeans.removeAll(freshMBeans);
for (String mbeanName : staleMBeans) {
host.removeMBean(mbeanName);
LOG.debug("removed stale mbean " + mbeanName);
}
}
} catch (IOException e) {
host = null;
connError = e;
LOG.error("communication failure with " + url, e);
} catch (SecurityException e) {
host = null;
authValid = false;
LOG.error("invalid credentials for " + url, e);
} finally {
if (connection != null) {
try {
Expand Down
2 changes: 1 addition & 1 deletion src/main/resources/assets/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ <h3 class="modal-title">Endpoint Authentication</h3>
</div>
<div class="collapse navbar-collapse">
<ul id="endpoint-tabs" class="nav navbar-nav" role="tablist">
<li><a href="#overview" data-toggle="tab" role="tab"><i class="fa fa-dashboard"></i> Overview </a></li>
<li><a href="#overview" data-toggle="tab" role="tab"><i class="fa fa-dashboard"></i> Overview</a></li>
<li><a href="#memory" data-toggle="tab" role="tab"><i class="fa fa-link"></i> Memory</a></li>
<li><a href="#threads" data-toggle="tab" role="tab"><i class="fa fa-tasks"></i> Threads</a></li>
<li><a href="#classes" data-toggle="tab" role="tab"><i class="fa fa-cubes"></i> Classes</a></li>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ public void checkValidHost() throws Exception {
assertNotNull(manager.getHost(validHost, validAuth));
}

@Test
@Test(expected=WebApplicationException.class)
public void checkInvalidHost() throws Exception {
final ConnectionManager manager = new ConnectionManager(new AppConfig());

Expand Down

0 comments on commit 1842dfb

Please sign in to comment.