fix(threads): revert #1388 (#1467)

* Revert "fix(credentials): store JMX session credentials in ThreadLocal (#1388)" This reverts commit 39695d0. * compile fixes Signed-off-by: Andrew Azores <[email protected]> * fixup! Revert "fix(credentials): store JMX session credentials in ThreadLocal (#1388)" --------- Signed-off-by: Andrew Azores <[email protected]> (cherry picked from commit eaf801a)
cryostatio · Apr 25, 2023 · 88dc691 · 88dc691
1 parent 998c539
commit 88dc691
Show file tree

Hide file tree

Showing 61 changed files with 285 additions and 941 deletions.
diff --git a/src/main/java/io/cryostat/configuration/CredentialsManager.java b/src/main/java/io/cryostat/configuration/CredentialsManager.java
@@ -80,9 +80,6 @@ public class CredentialsManager
     private final Gson gson;
     private final Logger logger;
 
-    public static final ThreadLocal<Map<String, Credentials>> SESSION_CREDENTIALS =
-            ThreadLocal.withInitial(() -> new HashMap<>());
-
     CredentialsManager(
             Path credentialsDir,
             MatchExpressionValidator matchExpressionValidator,
@@ -184,23 +181,7 @@ public int removeCredentials(String matchExpression) throws MatchExpressionValid
         return -1;
     }
 
-    public void setSessionCredentials(String targetId, Credentials credentials) {
-        if (credentials == null) {
-            SESSION_CREDENTIALS.get().remove(targetId);
-        } else {
-            SESSION_CREDENTIALS.get().put(targetId, credentials);
-        }
-    }
-
-    public Credentials getSessionCredentials(String targetId) {
-        return SESSION_CREDENTIALS.get().get(targetId);
-    }
-
     public Credentials getCredentialsByTargetId(String targetId) throws ScriptException {
-        Credentials sessionCredentials = getSessionCredentials(targetId);
-        if (sessionCredentials != null) {
-            return sessionCredentials;
-        }
         for (ServiceRef service : this.platformClient.listDiscoverableServices()) {
             if (Objects.equals(targetId, service.getServiceUri().toString())) {
                 return getCredentials(service);
@@ -210,11 +191,6 @@ public Credentials getCredentialsByTargetId(String targetId) throws ScriptExcept
     }
 
     public Credentials getCredentials(ServiceRef serviceRef) throws ScriptException {
-        Credentials sessionCredentials =
-                getSessionCredentials(serviceRef.getServiceUri().toString());
-        if (sessionCredentials != null) {
-            return sessionCredentials;
-        }
         for (StoredCredentials sc : dao.getAll()) {
             if (matchExpressionEvaluator.get().applies(sc.getMatchExpression(), serviceRef)) {
                 return sc.getCredentials();

diff --git a/src/main/java/io/cryostat/net/ConnectionDescriptor.java b/src/main/java/io/cryostat/net/ConnectionDescriptor.java
@@ -39,7 +39,6 @@
 
 import java.util.Optional;
 
-import io.cryostat.configuration.CredentialsManager;
 import io.cryostat.core.net.Credentials;
 import io.cryostat.platform.ServiceRef;
 
@@ -55,19 +54,12 @@ public ConnectionDescriptor(ServiceRef serviceRef) {
         this(serviceRef.getServiceUri().toString());
     }
 
-    // TODO remove this constructor, all descriptors should explicitly specify any credentials
     public ConnectionDescriptor(String targetId) {
-        this(targetId, CredentialsManager.SESSION_CREDENTIALS.get().get(targetId));
+        this(targetId, null);
     }
 
     public ConnectionDescriptor(ServiceRef serviceRef, Credentials credentials) {
-        this(
-                serviceRef.getServiceUri().toString(),
-                Optional.ofNullable(
-                                CredentialsManager.SESSION_CREDENTIALS
-                                        .get()
-                                        .get(serviceRef.getServiceUri().toString()))
-                        .orElse(credentials));
+        this(serviceRef.getServiceUri().toString(), credentials);
     }
 
     public ConnectionDescriptor(String targetId, Credentials credentials) {

diff --git a/src/main/java/io/cryostat/net/NetworkModule.java b/src/main/java/io/cryostat/net/NetworkModule.java
@@ -41,7 +41,7 @@
 import java.net.UnknownHostException;
 import java.nio.file.Path;
 import java.time.Duration;
-import java.util.concurrent.ExecutorService;
+import java.util.concurrent.ForkJoinPool;
 
 import javax.inject.Named;
 import javax.inject.Singleton;
@@ -144,13 +144,12 @@ static TargetConnectionManager provideTargetConnectionManager(
             DiscoveryStorage storage,
             @Named(Variables.TARGET_CACHE_TTL) Duration maxTargetTtl,
             @Named(Variables.TARGET_MAX_CONCURRENT_CONNECTIONS) int maxTargetConnections,
-            @Named(WebModule.VERTX_EXECUTOR) ExecutorService executor,
             Logger logger) {
         return new TargetConnectionManager(
                 connectionToolkit,
                 agentConnectionFactory,
                 storage,
-                executor,
+                ForkJoinPool.commonPool(),
                 Scheduler.systemScheduler(),
                 maxTargetTtl,
                 maxTargetConnections,

diff --git a/src/main/java/io/cryostat/net/openshift/OpenShiftNetworkModule.java b/src/main/java/io/cryostat/net/openshift/OpenShiftNetworkModule.java
@@ -40,7 +40,7 @@
 import java.io.BufferedReader;
 import java.io.IOException;
 import java.nio.file.Paths;
-import java.util.concurrent.ExecutorService;
+import java.util.concurrent.ForkJoinPool;
 import java.util.function.Function;
 
 import javax.inject.Named;
@@ -50,7 +50,6 @@
 import io.cryostat.core.sys.Environment;
 import io.cryostat.core.sys.FileSystem;
 import io.cryostat.net.AuthManager;
-import io.cryostat.net.web.WebModule;
 import io.cryostat.util.resource.ClassPropertiesLoader;
 
 import com.github.benmanes.caffeine.cache.Scheduler;
@@ -123,7 +122,6 @@ static OpenShiftClient provideServiceAccountClient(
     @Singleton
     static OpenShiftAuthManager provideOpenShiftAuthManager(
             Environment env,
-            @Named(WebModule.VERTX_EXECUTOR) ExecutorService executor,
             @Named(OPENSHIFT_NAMESPACE) Lazy<String> namespace,
             Lazy<OpenShiftClient> serviceAccountClient,
             @Named(TOKENED_CLIENT) Function<String, OpenShiftClient> clientProvider,
@@ -137,7 +135,7 @@ static OpenShiftAuthManager provideOpenShiftAuthManager(
                 clientProvider,
                 classPropertiesLoader,
                 gson,
-                executor,
+                ForkJoinPool.commonPool(),
                 Scheduler.systemScheduler(),
                 logger);
     }

diff --git a/src/main/java/io/cryostat/net/web/Vertexecutor.java b/src/main/java/io/cryostat/net/web/Vertexecutor.java
diff --git a/src/main/java/io/cryostat/net/web/WebModule.java b/src/main/java/io/cryostat/net/web/WebModule.java
@@ -40,7 +40,6 @@
 import java.io.IOException;
 import java.nio.file.Path;
 import java.util.Set;
-import java.util.concurrent.ExecutorService;
 
 import javax.inject.Named;
 import javax.inject.Singleton;
@@ -57,12 +56,10 @@
 import com.google.gson.Gson;
 import dagger.Module;
 import dagger.Provides;
-import io.vertx.core.Vertx;
 
 @Module(includes = {HttpModule.class})
 public abstract class WebModule {
     public static final String WEBSERVER_TEMP_DIR_PATH = "WEBSERVER_TEMP_DIR_PATH";
-    public static final String VERTX_EXECUTOR = "VERTX_EXECUTOR";
 
     @Provides
     static WebServer provideWebServer(
@@ -93,11 +90,4 @@ static Path provideWebServerTempDirPath(FileSystem fs) {
             throw new RuntimeException(ioe);
         }
     }
-
-    @Provides
-    @Singleton
-    @Named(VERTX_EXECUTOR)
-    static ExecutorService provideVertexecutor(Vertx vertx, Logger logger) {
-        return new Vertexecutor(vertx, logger);
-    }
 }
diff --git a/src/main/java/io/cryostat/net/web/http/AbstractAuthenticatedRequestHandler.java b/src/main/java/io/cryostat/net/web/http/AbstractAuthenticatedRequestHandler.java
@@ -156,9 +156,6 @@ protected ConnectionDescriptor getConnectionDescriptorFromContext(RoutingContext
                             427, "Unrecognized " + JMX_AUTHORIZATION_HEADER + " credential format");
                 }
                 credentials = new Credentials(parts[0], parts[1]);
-                credentialsManager.setSessionCredentials(targetId, credentials);
-                ctx.addEndHandler(
-                        unused -> credentialsManager.setSessionCredentials(targetId, null));
             } else {
                 credentials = credentialsManager.getCredentialsByTargetId(targetId);
             }

diff --git a/src/main/java/io/cryostat/net/web/http/api/v2/AbstractV2RequestHandler.java b/src/main/java/io/cryostat/net/web/http/api/v2/AbstractV2RequestHandler.java
@@ -94,10 +94,6 @@ public abstract IntermediateResponse<T> handle(RequestParameters requestParams)
     @Override
     public final void handle(RoutingContext ctx) {
         RequestParameters requestParams = RequestParameters.from(ctx);
-        String targetId = requestParams.getPathParams().get("targetId");
-        if (targetId != null) {
-            ctx.addEndHandler(unused -> credentialsManager.setSessionCredentials(targetId, null));
-        }
         try {
             if (requiresAuthentication()) {
                 boolean permissionGranted =
@@ -168,7 +164,6 @@ protected ConnectionDescriptor getConnectionDescriptorFromParams(RequestParamete
                                             + " credential format");
                         }
                         credentials = new Credentials(parts[0], parts[1]);
-                        credentialsManager.setSessionCredentials(targetId, credentials);
                     }
                 }
             } else {

diff --git a/src/main/java/io/cryostat/net/web/http/api/v2/TargetsPostHandler.java b/src/main/java/io/cryostat/net/web/http/api/v2/TargetsPostHandler.java
@@ -180,28 +180,23 @@ public IntermediateResponse<ServiceRef> handle(RequestParameters params) throws
                             ? Optional.empty()
                             : Optional.of(new Credentials(username, password));
 
-            if (credentials.isPresent()) {
-                if (storeCredentials) {
-                    String matchExpression =
-                            CredentialsManager.targetIdToMatchExpression(connectUrl);
-                    int id = credentialsManager.addCredentials(matchExpression, credentials.get());
-                    notificationFactory
-                            .createBuilder()
-                            .metaCategory("CredentialsStored")
-                            .metaType(HttpMimeType.JSON)
-                            .message(
-                                    Map.of(
-                                            "id",
-                                            id,
-                                            "matchExpression",
-                                            matchExpression,
-                                            "numMatchingTargets",
-                                            1))
-                            .build()
-                            .send();
-                } else {
-                    credentialsManager.setSessionCredentials(uri.toString(), credentials.get());
-                }
+            if (storeCredentials && credentials.isPresent()) {
+                String matchExpression = CredentialsManager.targetIdToMatchExpression(connectUrl);
+                int id = credentialsManager.addCredentials(matchExpression, credentials.get());
+                notificationFactory
+                        .createBuilder()
+                        .metaCategory("CredentialsStored")
+                        .metaType(HttpMimeType.JSON)
+                        .message(
+                                Map.of(
+                                        "id",
+                                        id,
+                                        "matchExpression",
+                                        matchExpression,
+                                        "numMatchingTargets",
+                                        1))
+                        .build()
+                        .send();
             }
 
             String jvmId = jvmIdHelper.getJvmId(uri.toString(), false, credentials);