Skip to content

Commit

Permalink
feat(plugin): send additional env payload to plugins on registration (#…
Browse files Browse the repository at this point in the history
…1520) (#1773)

(cherry picked from commit 5cfd2fc)

Co-authored-by: Andrew Azores <aazores@redhat.com>
  • Loading branch information
mergify[bot] and andrewazores authored Nov 9, 2023
1 parent 793db52 commit f5a1220
Show file tree
Hide file tree
Showing 4 changed files with 108 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import java.net.URISyntaxException;
import java.net.URL;
import java.util.EnumSet;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Objects;
Expand All @@ -34,6 +35,7 @@
import io.cryostat.MainModule;
import io.cryostat.configuration.CredentialsManager;
import io.cryostat.core.log.Logger;
import io.cryostat.core.sys.Environment;
import io.cryostat.discovery.DiscoveryStorage;
import io.cryostat.discovery.PluginInfo;
import io.cryostat.discovery.RegistrationException;
Expand All @@ -43,6 +45,8 @@
import io.cryostat.net.web.WebServer;
import io.cryostat.net.web.http.HttpMimeType;
import io.cryostat.net.web.http.api.ApiVersion;
import io.cryostat.platform.PlatformModule;
import io.cryostat.platform.internal.PlatformDetectionStrategy;
import io.cryostat.util.StringUtil;

import com.google.gson.Gson;
Expand All @@ -55,11 +59,13 @@
import io.vertx.core.json.JsonObject;
import org.apache.commons.lang3.StringUtils;

class DiscoveryRegistrationHandler extends AbstractV2RequestHandler<Map<String, String>> {
class DiscoveryRegistrationHandler extends AbstractV2RequestHandler<Map<String, Object>> {

static final String PATH = "discovery";
private final Environment env;
private final DiscoveryStorage storage;
private final Lazy<WebServer> webServer;
private final Set<PlatformDetectionStrategy<?>> selectedStrategies;
private final DiscoveryJwtHelper jwtFactory;
private final Function<String, UUID> uuidFromString;
private final Logger logger;
Expand All @@ -68,15 +74,20 @@ class DiscoveryRegistrationHandler extends AbstractV2RequestHandler<Map<String,
DiscoveryRegistrationHandler(
AuthManager auth,
CredentialsManager credentialsManager,
Environment env,
DiscoveryStorage storage,
Lazy<WebServer> webServer,
@Named(PlatformModule.SELECTED_PLATFORMS)
Set<PlatformDetectionStrategy<?>> selectedStrategies,
DiscoveryJwtHelper jwt,
@Named(MainModule.UUID_FROM_STRING) Function<String, UUID> uuidFromString,
Gson gson,
Logger logger) {
super(auth, credentialsManager, gson);
this.env = env;
this.storage = storage;
this.webServer = webServer;
this.selectedStrategies = selectedStrategies;
this.jwtFactory = jwt;
this.uuidFromString = uuidFromString;
this.logger = logger;
Expand Down Expand Up @@ -118,7 +129,7 @@ public boolean isAsync() {
}

@Override
public IntermediateResponse<Map<String, String>> handle(RequestParameters params)
public IntermediateResponse<Map<String, Object>> handle(RequestParameters params)
throws Exception {
String pluginId, realm, priorToken;
URI callbackUri;
Expand Down Expand Up @@ -175,9 +186,15 @@ public IntermediateResponse<Map<String, String>> handle(RequestParameters params
realm,
address,
AbstractDiscoveryJwtConsumingHandler.getResourceUri(hostUrl, pluginId));
return new IntermediateResponse<Map<String, String>>()
Map<String, String> mergedEnv = new HashMap<>();
// FIXME currently only the OpenShiftPlatformStrategy provides any entries for the env map,
// but in the future if any more strategies also provide entries then the order here may be
// undefined and the map entries may collide and be overwritten. There should be some
// prefixing scheme to prevent collisions.
selectedStrategies.forEach(s -> mergedEnv.putAll(s.environment(env)));
return new IntermediateResponse<Map<String, Object>>()
.statusCode(201)
.addHeader(HttpHeaders.LOCATION, String.format("%s/%s", path(), pluginId))
.body(Map.of("id", pluginId, "token", token));
.body(Map.of("id", pluginId, "token", token, "env", mergedEnv));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@
*/
package io.cryostat.platform.internal;

import java.util.HashMap;
import java.util.Map;

import io.cryostat.core.sys.Environment;
import io.cryostat.net.AuthManager;
import io.cryostat.platform.PlatformClient;

Expand All @@ -24,4 +28,12 @@ public interface PlatformDetectionStrategy<T extends PlatformClient> {
T getPlatformClient();

AuthManager getAuthManager();

default Map<String, String> environment(Environment env) {
Map<String, String> map = new HashMap<>();
if (env.hasEnv("INSIGHTS_PROXY")) {
map.put("INSIGHTS_SVC", env.getEnv("INSIGHTS_PROXY"));
}
return map;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,16 @@
import io.cryostat.MainModule;
import io.cryostat.configuration.CredentialsManager;
import io.cryostat.core.log.Logger;
import io.cryostat.core.sys.Environment;
import io.cryostat.discovery.DiscoveryStorage;
import io.cryostat.net.AuthManager;
import io.cryostat.net.security.ResourceAction;
import io.cryostat.net.security.jwt.DiscoveryJwtHelper;
import io.cryostat.net.web.WebServer;
import io.cryostat.net.web.http.HttpMimeType;
import io.cryostat.net.web.http.api.ApiVersion;
import io.cryostat.platform.PlatformClient;
import io.cryostat.platform.internal.PlatformDetectionStrategy;

import com.google.gson.Gson;
import io.vertx.core.MultiMap;
Expand All @@ -55,9 +58,10 @@

@ExtendWith(MockitoExtension.class)
class DiscoveryRegistrationHandlerTest {
AbstractV2RequestHandler<Map<String, String>> handler;
AbstractV2RequestHandler<Map<String, Object>> handler;
@Mock AuthManager auth;
@Mock CredentialsManager credentialsManager;
@Mock Environment env;
@Mock DiscoveryStorage storage;
@Mock WebServer webServer;
@Mock DiscoveryJwtHelper jwt;
Expand All @@ -70,8 +74,10 @@ void setup() {
new DiscoveryRegistrationHandler(
auth,
credentialsManager,
env,
storage,
() -> webServer,
Set.of(new AllEnvPlatformStrategy(), new FakePlatformStrategy()),
jwt,
UUID::fromString,
gson,
Expand Down Expand Up @@ -161,6 +167,8 @@ void shouldRegisterWithStorageAndSendResponse() throws Exception {
Mockito.when(storage.register(Mockito.anyString(), Mockito.any(URI.class)))
.thenReturn(id);

Mockito.when(env.getEnv()).thenReturn(Map.of("CRYOSTAT", "hello", "TEST", "true"));

Mockito.when(requestParams.getBody())
.thenReturn(
gson.toJson(
Expand All @@ -185,19 +193,77 @@ void shouldRegisterWithStorageAndSendResponse() throws Exception {
Mockito.any(URI.class)))
.thenReturn(token);

IntermediateResponse<Map<String, String>> resp = handler.handle(requestParams);
IntermediateResponse<Map<String, Object>> resp = handler.handle(requestParams);

MatcherAssert.assertThat(resp.getStatusCode(), Matchers.equalTo(201));
MatcherAssert.assertThat(
resp.getHeaders(),
Matchers.equalTo(Map.of(HttpHeaders.LOCATION, "/api/v2.2/discovery/" + id)));
MatcherAssert.assertThat(
resp.getBody(), Matchers.equalTo(Map.of("token", token, "id", id.toString())));
resp.getBody(),
Matchers.equalTo(
Map.of(
"token",
token,
"id",
id.toString(),
"env",
Map.of(
"CRYOSTAT",
"hello",
"TEST",
"true",
"HELLO",
"WORLD"))));

Mockito.verify(storage)
.register(
Mockito.eq("test-realm"),
Mockito.eq(URI.create("http://example.com/callback")));
}
}

static class AllEnvPlatformStrategy implements PlatformDetectionStrategy {
@Override
public Map<String, String> environment(Environment env) {
return env.getEnv();
}

@Override
public boolean isAvailable() {
return true;
}

@Override
public PlatformClient getPlatformClient() {
throw new UnsupportedOperationException("Unimplemented method 'getPlatformClient'");
}

@Override
public AuthManager getAuthManager() {
throw new UnsupportedOperationException("Unimplemented method 'getAuthManager'");
}
}

static class FakePlatformStrategy implements PlatformDetectionStrategy {
@Override
public Map<String, String> environment(Environment env) {
return Map.of("HELLO", "WORLD");
}

@Override
public boolean isAvailable() {
return true;
}

@Override
public PlatformClient getPlatformClient() {
throw new UnsupportedOperationException("Unimplemented method 'getPlatformClient'");
}

@Override
public AuthManager getAuthManager() {
throw new UnsupportedOperationException("Unimplemented method 'getAuthManager'");
}
}
}
6 changes: 6 additions & 0 deletions src/test/java/itest/DiscoveryPluginIT.java
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import java.util.UUID;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.ExecutionException;
import java.util.stream.Collectors;

import io.vertx.core.buffer.Buffer;
import io.vertx.core.json.JsonArray;
Expand All @@ -40,6 +41,7 @@ class DiscoveryPluginIT extends StandardSelfTest {
final URI callback = URI.create("http://localhost:8181/");
private static volatile String id;
private static volatile String token;
private static volatile Map<String, String> env;

@Test
@Order(1)
Expand All @@ -60,8 +62,12 @@ void shouldBeAbleToRegister() throws InterruptedException, ExecutionException {
JsonObject info = resp.getJsonObject("data").getJsonObject("result");
DiscoveryPluginIT.id = info.getString("id");
DiscoveryPluginIT.token = info.getString("token");
DiscoveryPluginIT.env =
info.getJsonObject("env").getMap().entrySet().stream()
.collect(Collectors.toMap(k -> k.toString(), v -> v.toString()));
MatcherAssert.assertThat(id, Matchers.not(Matchers.emptyOrNullString()));
MatcherAssert.assertThat(token, Matchers.not(Matchers.emptyOrNullString()));
MatcherAssert.assertThat(env, Matchers.is(Matchers.equalTo(Map.of())));
}

@Test
Expand Down

0 comments on commit f5a1220

Please sign in to comment.