Skip to content

Commit

Permalink
Revert "fix(credentials): store JMX session credentials in ThreadLocal (
Browse files Browse the repository at this point in the history
cryostatio#1388)"

This reverts commit 39695d0.
  • Loading branch information
andrewazores committed Apr 22, 2023
1 parent 5176071 commit 0fa0564
Show file tree
Hide file tree
Showing 60 changed files with 272 additions and 921 deletions.
8 changes: 4 additions & 4 deletions repeated-integration-tests.bash
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ fi

getPomProperty() {
if command -v xpath > /dev/null 2>&1 ; then
xpath -q -e "project/$1/text()" pom.xml
xpath -q -e "project/properties/$1/text()" pom.xml
elif command -v mvnd > /dev/null 2>&1 ; then
mvnd help:evaluate -o -B -q -DforceStdout -Dexpression="$1"
else
Expand All @@ -26,15 +26,15 @@ getPomProperty() {
}

if [ -z "${POD_NAME}" ]; then
POD_NAME="$(getPomProperty properties/cryostat.itest.podName)"
POD_NAME="$(getPomProperty cryostat.itest.podName)"
fi

if [ -z "${CONTAINER_NAME}" ]; then
CONTAINER_NAME="$(getPomProperty properties/cryostat.itest.containerName)"
CONTAINER_NAME="$(getPomProperty cryostat.itest.containerName)"
fi

if [ -z "${ITEST_IMG_VERSION}" ]; then
ITEST_IMG_VERSION="$(getPomProperty version)"
ITEST_IMG_VERSION="$(getPomProperty project.version)"
ITEST_IMG_VERSION="${ITEST_IMG_VERSION,,}" # lowercase
fi

Expand Down
24 changes: 0 additions & 24 deletions src/main/java/io/cryostat/configuration/CredentialsManager.java
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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);
Expand All @@ -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();
Expand Down
12 changes: 2 additions & 10 deletions src/main/java/io/cryostat/net/ConnectionDescriptor.java
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@

import java.util.Optional;

import io.cryostat.configuration.CredentialsManager;
import io.cryostat.core.net.Credentials;
import io.cryostat.platform.ServiceRef;

Expand All @@ -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) {
Expand Down
5 changes: 2 additions & 3 deletions src/main/java/io/cryostat/net/NetworkModule.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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,
Expand All @@ -137,7 +135,7 @@ static OpenShiftAuthManager provideOpenShiftAuthManager(
clientProvider,
classPropertiesLoader,
gson,
executor,
ForkJoinPool.commonPool(),
Scheduler.systemScheduler(),
logger);
}
Expand Down
99 changes: 0 additions & 99 deletions src/main/java/io/cryostat/net/web/Vertexecutor.java

This file was deleted.

10 changes: 0 additions & 10 deletions src/main/java/io/cryostat/net/web/WebModule.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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(
Expand Down Expand Up @@ -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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand Down Expand Up @@ -168,7 +164,6 @@ protected ConnectionDescriptor getConnectionDescriptorFromParams(RequestParamete
+ " credential format");
}
credentials = new Credentials(parts[0], parts[1]);
credentialsManager.setSessionCredentials(targetId, credentials);
}
}
} else {
Expand Down
Loading

0 comments on commit 0fa0564

Please sign in to comment.