Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix(target): handle creating short form custom target #1875

Closed
wants to merge 9 commits into from
Closed

fix(target): handle creating short form custom target #1875

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
c752209
handle short form custom target
aali309 Feb 12, 2024
5ee9314
convert shortform to full JMXServiceURL in TargetPostHandler
aali309 Feb 15, 2024
49ee548
mvn spotless:apply
aali309 Feb 15, 2024
ce61785
handle unit test failure and updated test
aali309 Feb 16, 2024
cbd36af
encode JMX service URL
aali309 Feb 21, 2024
9d814da
mvn spotless:apply
aali309 Feb 21, 2024
69c274f
resolve issues
aali309 Feb 21, 2024
831ee23
add .strip() to handle leading or trailing whitespaces
aali309 Feb 21, 2024
6e115c2
drop variable name
aali309 Feb 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions src/main/java/io/cryostat/net/web/http/api/v2/TargetsPostHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@

import javax.inject.Inject;

import org.openjdk.jmc.rjmx.ConnectionToolkit;

import io.cryostat.configuration.CredentialsManager;
import io.cryostat.core.log.Logger;
import io.cryostat.core.net.Credentials;
Expand Down Expand Up @@ -129,9 +131,20 @@ public IntermediateResponse<ServiceRef> handle(RequestParameters params) throws
try {
MultiMap attrs = params.getFormAttributes();
String connectUrl = attrs.get("connectUrl");

if (StringUtils.isBlank(connectUrl)) {
throw new ApiException(400, "\"connectUrl\" form parameter must be provided");
}
// check incase custom target has short form connection url (i.e `localhost:0`,
// etc)
connectUrl = connectUrl.strip();
boolean isShortForm = connectUrl.matches("^[^\\s/:]+:\\d+$");
if (isShortForm) {
String[] connectUrlParts = connectUrl.split(":");
String host = connectUrlParts[0];
int port = Integer.parseInt(connectUrlParts[1]);
connectUrl = ConnectionToolkit.createServiceURL(host, port).toString();
}
String alias = attrs.get("alias");
if (StringUtils.isBlank(alias)) {
throw new ApiException(400, "\"alias\" form parameter must be provided");
Expand Down
37 changes: 37 additions & 0 deletions src/test/java/io/cryostat/net/web/http/api/v2/TargetsPostHandlerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -253,6 +253,43 @@ void testRequestWithBadAlias(String alias) throws IOException {
MatcherAssert.assertThat(ex.getStatusCode(), Matchers.equalTo(400));
}

@Test
void testRequestWithShortFormCustomTarget() throws Exception {
MultiMap attrs = MultiMap.caseInsensitiveMultiMap();
RequestParameters params = Mockito.mock(RequestParameters.class);
Mockito.when(params.getFormAttributes()).thenReturn(attrs);
Mockito.when(params.getQueryParams()).thenReturn(MultiMap.caseInsensitiveMultiMap());
Mockito.when(customTargetPlatformClient.addTarget(Mockito.any())).thenReturn(true);
Mockito.when(storage.listDiscoverableServices()).thenReturn(List.of());
Mockito.when(
jvmIdHelper.getJvmId(
Mockito.anyString(),
Mockito.anyBoolean(),
Mockito.any(Optional.class)))
.thenReturn("id");

String connectUrl = "localhost:9099";
String alias = "TestAlias";
attrs.set("connectUrl", connectUrl);
attrs.set("alias", alias);

IntermediateResponse<ServiceRef> response = handler.handle(params);
MatcherAssert.assertThat(response.getStatusCode(), Matchers.equalTo(200));

ArgumentCaptor<ServiceRef> refCaptor = ArgumentCaptor.forClass(ServiceRef.class);
Mockito.verify(customTargetPlatformClient).addTarget(refCaptor.capture());
ServiceRef captured = refCaptor.getValue();
String expectedConnectUrl = "service:jmx:rmi:///jndi/rmi://localhost:9099/jmxrmi";
MatcherAssert.assertThat(
captured.getServiceUri(), Matchers.equalTo(new URI(expectedConnectUrl)));
MatcherAssert.assertThat(captured.getAlias(), Matchers.equalTo(Optional.of(alias)));
MatcherAssert.assertThat(captured.getPlatformAnnotations(), Matchers.equalTo(Map.of()));
MatcherAssert.assertThat(
captured.getCryostatAnnotations(),
Matchers.equalTo(Map.of(AnnotationKey.REALM, "Custom Targets")));
MatcherAssert.assertThat(response.getBody(), Matchers.equalTo(captured));
}

@Test
void testRequestWithDuplicateTarget() throws IOException {
MultiMap attrs = MultiMap.caseInsensitiveMultiMap();
Expand Down
29 changes: 22 additions & 7 deletions src/test/java/itest/CustomTargetsIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
*/
package itest;

import java.nio.charset.StandardCharsets;
import java.util.Collections;
import java.util.HashMap;
import java.util.Map;
import java.util.concurrent.CompletableFuture;
Expand All @@ -35,6 +37,7 @@
import itest.util.ITestCleanupFailedException;
import itest.util.http.JvmIdWebRequest;
import itest.util.http.StoredCredential;
import org.apache.http.client.utils.URLEncodedUtils;
import org.hamcrest.MatcherAssert;
import org.hamcrest.Matchers;
import org.junit.jupiter.api.AfterAll;
Expand Down Expand Up @@ -114,7 +117,9 @@ void shouldBeAbleToTestTargetConnection() throws InterruptedException, Execution
response.complete(ar.result().bodyAsJsonObject());
});
JsonObject body = response.get().getJsonObject("data").getJsonObject("result");
MatcherAssert.assertThat(body.getString("connectUrl"), Matchers.equalTo("localhost:0"));
MatcherAssert.assertThat(
body.getString("connectUrl"),
Matchers.equalTo("service:jmx:rmi:///jndi/rmi://localhost:0/jmxrmi"));
MatcherAssert.assertThat(body.getString("alias"), Matchers.equalTo("self"));
}

Expand Down Expand Up @@ -215,7 +220,9 @@ void shouldBeAbleToDefineTarget()
latch.await(30, TimeUnit.SECONDS);

JsonObject body = response.get().getJsonObject("data").getJsonObject("result");
MatcherAssert.assertThat(body.getString("connectUrl"), Matchers.equalTo("localhost:0"));
MatcherAssert.assertThat(
body.getString("connectUrl"),
Matchers.equalTo("service:jmx:rmi:///jndi/rmi://localhost:0/jmxrmi"));
MatcherAssert.assertThat(body.getString("alias"), Matchers.equalTo("self"));

JsonObject result1 = resultFuture1.get();
Expand All @@ -231,7 +238,9 @@ void shouldBeAbleToDefineTarget()
MatcherAssert.assertThat(storedCredential.id, Matchers.any(Integer.class));
MatcherAssert.assertThat(
storedCredential.matchExpression,
Matchers.equalTo("target.connectUrl == \"localhost:0\""));
Matchers.equalTo(
"target.connectUrl =="
+ " \"service:jmx:rmi:///jndi/rmi://localhost:0/jmxrmi\""));
MatcherAssert.assertThat(
storedCredential.numMatchingTargets, Matchers.equalTo(Integer.valueOf(1)));

Expand All @@ -240,7 +249,7 @@ void shouldBeAbleToDefineTarget()
MatcherAssert.assertThat(event.getString("kind"), Matchers.equalTo("FOUND"));
MatcherAssert.assertThat(
event.getJsonObject("serviceRef").getString("connectUrl"),
Matchers.equalTo("localhost:0"));
Matchers.equalTo("service:jmx:rmi:///jndi/rmi://localhost:0/jmxrmi"));
MatcherAssert.assertThat(
event.getJsonObject("serviceRef").getString("alias"), Matchers.equalTo("self"));
}
Expand Down Expand Up @@ -294,7 +303,7 @@ void targetShouldAppearInListing()
"alias",
"self",
"connectUrl",
"localhost:0",
"service:jmx:rmi:///jndi/rmi://localhost:0/jmxrmi",
"labels",
Map.of(),
"annotations",
Expand Down Expand Up @@ -328,7 +337,8 @@ void shouldBeAbleToDeleteTarget()
MatcherAssert.assertThat(
event.getJsonObject("serviceRef")
.getString("connectUrl"),
Matchers.equalTo("localhost:0"));
Matchers.equalTo(
"service:jmx:rmi:///jndi/rmi://localhost:0/jmxrmi"));
MatcherAssert.assertThat(
event.getJsonObject("serviceRef")
.getString("alias"),
Expand All @@ -341,9 +351,14 @@ void shouldBeAbleToDeleteTarget()
}
});

String encodedUrl =
URLEncodedUtils.formatSegments(
Collections.singletonList(
"service:jmx:rmi:///jndi/rmi://localhost:0/jmxrmi"),
StandardCharsets.UTF_8);
CompletableFuture<JsonObject> response = new CompletableFuture<>();
webClient
.delete("/api/v2/targets/localhost:0")
.delete("/api/v2/targets/" + encodedUrl.strip())
.send(
ar -> {
assertRequestStatus(ar, response);
Expand Down
Loading