From 7b774d9de404868348190b15de63637aa46303bf Mon Sep 17 00:00:00 2001 From: Andrew Azores Date: Thu, 27 Jul 2023 16:10:08 -0400 Subject: [PATCH 01/45] feat(api): enable dynamic JFR start (#165) --- README.md | 1 + .../java/io/cryostat/agent/ConfigModule.java | 3 + .../java/io/cryostat/agent/MainModule.java | 73 +++++- .../java/io/cryostat/agent/WebServer.java | 63 +++-- .../agent/remote/EventTemplatesContext.java | 48 ++-- .../agent/remote/EventTypesContext.java | 35 +-- .../cryostat/agent/remote/MBeanContext.java | 38 +-- .../agent/remote/MutatingRemoteContext.java | 60 +++++ .../agent/remote/RecordingsContext.java | 217 +++++++++++++----- .../cryostat/agent/remote/RemoteContext.java | 4 + .../META-INF/microprofile-config.properties | 2 + 11 files changed, 401 insertions(+), 143 deletions(-) create mode 100644 src/main/java/io/cryostat/agent/remote/MutatingRemoteContext.java diff --git a/README.md b/README.md index 483f886f..97348324 100644 --- a/README.md +++ b/README.md @@ -34,6 +34,7 @@ and how it advertises itself to a Cryostat server instance. Required properties - [x] `cryostat.agent.baseuri` [`java.net.URI`]: the URL location of the Cryostat server backend that this agent advertises itself to. - [x] `cryostat.agent.callback` [`java.net.URI`]: a URL pointing back to this agent, ex. `"https://12.34.56.78:1234/"`. Cryostat will use this URL to perform health checks and request updates from the agent. This reflects the externally-visible IP address/hostname and port where this application and agent can be found. +- [ ] `cryostat.agent.api.writes-enabled` [`boolean`]: Control whether the agent accepts "write" or mutating operations on its HTTP API. Requests for remote operations such as dynamically starting Flight Recordings will be rejected unless this is set. Default `false`. - [ ] `cryostat.agent.instance-id` [`String`]: a unique ID for this agent instance. This will be used to uniquely identify the agent in the Cryostat discovery database, as well as to unambiguously match its encrypted stored credentials. The default is a random UUID string. It is not recommended to override this value. - [ ] `cryostat.agent.hostname` [`String`]: the hostname for this application instance. This will be used for the published JMX connection URL. If not provided then the default is to attempt to resolve the localhost hostname. - [ ] `cryostat.agent.realm` [`String`]: the Cryostat Discovery API "realm" that this agent belongs to. This should be unique per agent instance. The default is the value of `cryostat.agent.app.name`. diff --git a/src/main/java/io/cryostat/agent/ConfigModule.java b/src/main/java/io/cryostat/agent/ConfigModule.java index c1b1c9e3..a464439b 100644 --- a/src/main/java/io/cryostat/agent/ConfigModule.java +++ b/src/main/java/io/cryostat/agent/ConfigModule.java @@ -85,6 +85,9 @@ public abstract class ConfigModule { public static final String CRYOSTAT_AGENT_HARVESTER_MAX_SIZE_B = "cryostat.agent.harvester.max-size-b"; + public static final String CRYOSTAT_AGENT_API_WRITES_ENABLED = + "cryostat.agent.api.writes-enabled"; + @Provides @Singleton public static SmallRyeConfig provideConfig() { diff --git a/src/main/java/io/cryostat/agent/MainModule.java b/src/main/java/io/cryostat/agent/MainModule.java index e963ab66..d66e1280 100644 --- a/src/main/java/io/cryostat/agent/MainModule.java +++ b/src/main/java/io/cryostat/agent/MainModule.java @@ -15,7 +15,9 @@ */ package io.cryostat.agent; +import java.io.IOException; import java.net.URI; +import java.nio.file.Path; import java.security.KeyManagementException; import java.security.NoSuchAlgorithmException; import java.security.SecureRandom; @@ -39,6 +41,7 @@ import io.cryostat.core.net.JFRConnectionToolkit; import io.cryostat.core.sys.Environment; import io.cryostat.core.sys.FileSystem; +import io.cryostat.core.templates.LocalStorageTemplateService; import io.cryostat.core.tui.ClientWriter; import com.fasterxml.jackson.databind.ObjectMapper; @@ -63,6 +66,7 @@ public abstract class MainModule { // one for outbound HTTP requests, one for incoming HTTP requests, and one as a general worker private static final int NUM_WORKER_THREADS = 3; private static final String JVM_ID = "JVM_ID"; + private static final String TEMPLATES_PATH = "TEMPLATES_PATH"; @Provides @Singleton @@ -258,21 +262,61 @@ public static Harvester provideHarvester( registration); } + @Provides + @Singleton + public static FileSystem provideFileSystem() { + return new FileSystem(); + } + + @Provides + @Singleton + @Named(TEMPLATES_PATH) + public static Path provideTemplatesTmpPath(FileSystem fs) { + try { + return fs.createTempDirectory(null); + } catch (IOException e) { + throw new RuntimeException(e); + } + } + + @Provides + @Singleton + public static Environment provideEnvironment(@Named(TEMPLATES_PATH) Path templatesTmp) { + return new Environment() { + @Override + public String getEnv(String key) { + if (LocalStorageTemplateService.TEMPLATE_PATH.equals(key)) { + return templatesTmp.toString(); + } + return super.getEnv(key); + } + }; + } + + @Provides + @Singleton + public static ClientWriter provideClientWriter() { + Logger log = LoggerFactory.getLogger(JFRConnectionToolkit.class); + return new ClientWriter() { + @Override + public void print(String msg) { + log.info(msg); + } + }; + } + + @Provides + @Singleton + public static JFRConnectionToolkit provideJfrConnectionToolkit( + ClientWriter cw, FileSystem fs, Environment env) { + return new JFRConnectionToolkit(cw, fs, env); + } + @Provides @Singleton @Named(JVM_ID) - public static String provideJvmId() { + public static String provideJvmId(JFRConnectionToolkit tk) { Logger log = LoggerFactory.getLogger(JFRConnectionToolkit.class); - JFRConnectionToolkit tk = - new JFRConnectionToolkit( - new ClientWriter() { - @Override - public void print(String msg) { - log.warn(msg); - } - }, - new FileSystem(), - new Environment()); try { try (JFRConnection connection = tk.connect(tk.createServiceURL("localhost", 0))) { String id = connection.getJvmId(); @@ -283,4 +327,11 @@ public void print(String msg) { throw new RuntimeException(e); } } + + @Provides + @Singleton + public static LocalStorageTemplateService provideLocalStorageTemplateService( + FileSystem fs, Environment env) { + return new LocalStorageTemplateService(fs, env); + } } diff --git a/src/main/java/io/cryostat/agent/WebServer.java b/src/main/java/io/cryostat/agent/WebServer.java index 7c349f4d..1d2f78c6 100644 --- a/src/main/java/io/cryostat/agent/WebServer.java +++ b/src/main/java/io/cryostat/agent/WebServer.java @@ -40,6 +40,7 @@ import com.sun.net.httpserver.Filter; import com.sun.net.httpserver.HttpContext; import com.sun.net.httpserver.HttpExchange; +import com.sun.net.httpserver.HttpHandler; import com.sun.net.httpserver.HttpServer; import dagger.Lazy; import org.apache.http.HttpStatus; @@ -100,13 +101,15 @@ void start() throws IOException, NoSuchAlgorithmException { Set mergedContexts = new HashSet<>(remoteContexts.get()); mergedContexts.add(new PingContext()); - mergedContexts.forEach( - rc -> { - HttpContext ctx = this.http.createContext(rc.path(), rc::handle); - ctx.setAuthenticator(agentAuthenticator); - ctx.getFilters().add(requestLoggingFilter); - ctx.getFilters().add(compressionFilter); - }); + mergedContexts.stream() + .filter(RemoteContext::available) + .forEach( + rc -> { + HttpContext ctx = this.http.createContext(rc.path(), wrap(rc::handle)); + ctx.setAuthenticator(agentAuthenticator); + ctx.getFilters().add(requestLoggingFilter); + ctx.getFilters().add(compressionFilter); + }); this.http.start(); } @@ -154,6 +157,18 @@ CompletableFuture generateCredentials() throws NoSuchAlgorithmException { } } + private HttpHandler wrap(HttpHandler handler) { + return x -> { + try { + handler.handle(x); + } catch (Exception e) { + log.error("Unhandled exception", e); + x.sendResponseHeaders(HttpStatus.SC_INTERNAL_SERVER_ERROR, 0); + x.close(); + } + }; + } + private class PingContext implements RemoteContext { @Override @@ -163,23 +178,25 @@ public String path() { @Override public void handle(HttpExchange exchange) throws IOException { - String mtd = exchange.getRequestMethod(); - switch (mtd) { - case "POST": - synchronized (WebServer.this.credentials) { - executor.execute(registration.get()::tryRegister); + try { + String mtd = exchange.getRequestMethod(); + switch (mtd) { + case "POST": + synchronized (WebServer.this.credentials) { + executor.execute(registration.get()::tryRegister); + exchange.sendResponseHeaders(HttpStatus.SC_NO_CONTENT, -1); + } + break; + case "GET": exchange.sendResponseHeaders(HttpStatus.SC_NO_CONTENT, -1); - exchange.close(); - } - break; - case "GET": - exchange.sendResponseHeaders(HttpStatus.SC_NO_CONTENT, -1); - exchange.close(); - break; - default: - exchange.sendResponseHeaders(HttpStatus.SC_NOT_FOUND, -1); - exchange.close(); - break; + break; + default: + log.warn("Unknown request method {}", mtd); + exchange.sendResponseHeaders(HttpStatus.SC_METHOD_NOT_ALLOWED, -1); + break; + } + } finally { + exchange.close(); } } } diff --git a/src/main/java/io/cryostat/agent/remote/EventTemplatesContext.java b/src/main/java/io/cryostat/agent/remote/EventTemplatesContext.java index 03c0770d..fb5fca55 100644 --- a/src/main/java/io/cryostat/agent/remote/EventTemplatesContext.java +++ b/src/main/java/io/cryostat/agent/remote/EventTemplatesContext.java @@ -48,30 +48,32 @@ public String path() { @Override public void handle(HttpExchange exchange) throws IOException { - String mtd = exchange.getRequestMethod(); - switch (mtd) { - case "GET": - try { - exchange.sendResponseHeaders(HttpStatus.SC_OK, 0); - try (OutputStream response = exchange.getResponseBody()) { - FlightRecorderMXBean bean = - ManagementFactory.getPlatformMXBean(FlightRecorderMXBean.class); - List xmlTexts = - bean.getConfigurations().stream() - .map(ConfigurationInfo::getContents) - .collect(Collectors.toList()); - mapper.writeValue(response, xmlTexts); + try { + String mtd = exchange.getRequestMethod(); + switch (mtd) { + case "GET": + try { + exchange.sendResponseHeaders(HttpStatus.SC_OK, 0); + try (OutputStream response = exchange.getResponseBody()) { + FlightRecorderMXBean bean = + ManagementFactory.getPlatformMXBean(FlightRecorderMXBean.class); + List xmlTexts = + bean.getConfigurations().stream() + .map(ConfigurationInfo::getContents) + .collect(Collectors.toList()); + mapper.writeValue(response, xmlTexts); + } + } catch (Exception e) { + log.error("events serialization failure", e); } - } catch (Exception e) { - log.error("events serialization failure", e); - } finally { - exchange.close(); - } - break; - default: - exchange.sendResponseHeaders(HttpStatus.SC_NOT_FOUND, -1); - exchange.close(); - break; + break; + default: + log.warn("Unknown request method {}", mtd); + exchange.sendResponseHeaders(HttpStatus.SC_METHOD_NOT_ALLOWED, -1); + break; + } + } finally { + exchange.close(); } } } diff --git a/src/main/java/io/cryostat/agent/remote/EventTypesContext.java b/src/main/java/io/cryostat/agent/remote/EventTypesContext.java index 8efe03fb..0fc79763 100644 --- a/src/main/java/io/cryostat/agent/remote/EventTypesContext.java +++ b/src/main/java/io/cryostat/agent/remote/EventTypesContext.java @@ -50,25 +50,30 @@ public String path() { @Override public void handle(HttpExchange exchange) throws IOException { - String mtd = exchange.getRequestMethod(); - switch (mtd) { - case "GET": - try { - List events = getEventTypes(); + try { + String mtd = exchange.getRequestMethod(); + switch (mtd) { + case "GET": + List events = new ArrayList<>(); + try { + events.addAll(getEventTypes()); + } catch (Exception e) { + log.error("events serialization failure", e); + exchange.sendResponseHeaders(HttpStatus.SC_INTERNAL_SERVER_ERROR, 0); + break; + } exchange.sendResponseHeaders(HttpStatus.SC_OK, 0); try (OutputStream response = exchange.getResponseBody()) { mapper.writeValue(response, events); } - } catch (Exception e) { - log.error("events serialization failure", e); - } finally { - exchange.close(); - } - break; - default: - exchange.sendResponseHeaders(HttpStatus.SC_NOT_FOUND, -1); - exchange.close(); - break; + break; + default: + log.warn("Unknown request method {}", mtd); + exchange.sendResponseHeaders(HttpStatus.SC_METHOD_NOT_ALLOWED, -1); + break; + } + } finally { + exchange.close(); } } diff --git a/src/main/java/io/cryostat/agent/remote/MBeanContext.java b/src/main/java/io/cryostat/agent/remote/MBeanContext.java index 3b7bd1bd..c085df90 100644 --- a/src/main/java/io/cryostat/agent/remote/MBeanContext.java +++ b/src/main/java/io/cryostat/agent/remote/MBeanContext.java @@ -64,25 +64,27 @@ public String path() { @Override public void handle(HttpExchange exchange) throws IOException { - String mtd = exchange.getRequestMethod(); - switch (mtd) { - case "GET": - try { - MBeanMetrics metrics = getMBeanMetrics(); - exchange.sendResponseHeaders(HttpStatus.SC_OK, 0); - try (OutputStream response = exchange.getResponseBody()) { - mapper.writeValue(response, metrics); + try { + String mtd = exchange.getRequestMethod(); + switch (mtd) { + case "GET": + try { + MBeanMetrics metrics = getMBeanMetrics(); + exchange.sendResponseHeaders(HttpStatus.SC_OK, 0); + try (OutputStream response = exchange.getResponseBody()) { + mapper.writeValue(response, metrics); + } + } catch (Exception e) { + log.error("mbean serialization failure", e); } - } catch (Exception e) { - log.error("mbean serialization failure", e); - } finally { - exchange.close(); - } - break; - default: - exchange.sendResponseHeaders(HttpStatus.SC_NOT_FOUND, -1); - exchange.close(); - break; + break; + default: + log.warn("Unknown request method {}", mtd); + exchange.sendResponseHeaders(HttpStatus.SC_METHOD_NOT_ALLOWED, -1); + break; + } + } finally { + exchange.close(); } } diff --git a/src/main/java/io/cryostat/agent/remote/MutatingRemoteContext.java b/src/main/java/io/cryostat/agent/remote/MutatingRemoteContext.java new file mode 100644 index 00000000..89f65c67 --- /dev/null +++ b/src/main/java/io/cryostat/agent/remote/MutatingRemoteContext.java @@ -0,0 +1,60 @@ +/* + * Copyright The Cryostat Authors + * + * The Universal Permissive License (UPL), Version 1.0 + * + * Subject to the condition set forth below, permission is hereby granted to any + * person obtaining a copy of this software, associated documentation and/or data + * (collectively the "Software"), free of charge and under any and all copyright + * rights in the Software, and any and all patent rights owned or freely + * licensable by each licensor hereunder covering either (i) the unmodified + * Software as contributed to or provided by such licensor, or (ii) the Larger + * Works (as defined below), to deal in both + * + * (a) the Software, and + * (b) any piece of software and/or hardware listed in the lrgrwrks.txt file if + * one is included with the Software (each a "Larger Work" to which the Software + * is contributed by such licensors), + * + * without restriction, including without limitation the rights to copy, create + * derivative works of, display, perform, and distribute the Software and make, + * use, sell, offer for sale, import, export, have made, and have sold the + * Software and the Larger Work(s), and to sublicense the foregoing rights on + * either these or other terms. + * + * This license is subject to the following condition: + * The above copyright notice and either this complete permission notice or at + * a minimum a reference to the UPL must be included in all copies or + * substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + */ +package io.cryostat.agent.remote; + +import io.cryostat.agent.ConfigModule; + +import io.smallrye.config.SmallRyeConfig; + +public abstract class MutatingRemoteContext implements RemoteContext { + + protected final SmallRyeConfig config; + + protected MutatingRemoteContext(SmallRyeConfig config) { + this.config = config; + } + + @Override + public boolean available() { + return apiWritesEnabled(config); + } + + public static boolean apiWritesEnabled(SmallRyeConfig config) { + return config.getValue(ConfigModule.CRYOSTAT_AGENT_API_WRITES_ENABLED, boolean.class); + } +} diff --git a/src/main/java/io/cryostat/agent/remote/RecordingsContext.java b/src/main/java/io/cryostat/agent/remote/RecordingsContext.java index 0a605cbd..d5c9cc96 100644 --- a/src/main/java/io/cryostat/agent/remote/RecordingsContext.java +++ b/src/main/java/io/cryostat/agent/remote/RecordingsContext.java @@ -15,19 +15,41 @@ */ package io.cryostat.agent.remote; +import java.io.ByteArrayInputStream; import java.io.IOException; +import java.io.InputStream; import java.io.OutputStream; +import java.nio.charset.StandardCharsets; import java.util.List; -import java.util.Map; +import java.util.Set; import java.util.stream.Collectors; import javax.inject.Inject; +import org.openjdk.jmc.common.unit.IConstrainedMap; +import org.openjdk.jmc.common.unit.QuantityConversionException; +import org.openjdk.jmc.common.unit.UnitLookup; +import org.openjdk.jmc.flightrecorder.configuration.events.EventOptionID; +import org.openjdk.jmc.flightrecorder.configuration.recording.RecordingOptionsBuilder; +import org.openjdk.jmc.rjmx.ServiceNotAvailableException; +import org.openjdk.jmc.rjmx.services.jfr.IFlightRecorderService; + +import io.cryostat.agent.StringUtils; +import io.cryostat.core.FlightRecorderException; +import io.cryostat.core.net.JFRConnection; +import io.cryostat.core.net.JFRConnectionToolkit; +import io.cryostat.core.serialization.SerializableRecordingDescriptor; +import io.cryostat.core.templates.LocalStorageTemplateService; +import io.cryostat.core.templates.MutableTemplateService.InvalidEventTemplateException; +import io.cryostat.core.templates.MutableTemplateService.InvalidXmlException; +import io.cryostat.core.templates.RemoteTemplateService; +import io.cryostat.core.templates.Template; +import io.cryostat.core.templates.TemplateType; + import com.fasterxml.jackson.databind.ObjectMapper; import com.sun.net.httpserver.HttpExchange; -import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; +import io.smallrye.config.SmallRyeConfig; import jdk.jfr.FlightRecorder; -import jdk.jfr.Recording; import org.apache.http.HttpStatus; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -35,11 +57,21 @@ class RecordingsContext implements RemoteContext { private final Logger log = LoggerFactory.getLogger(getClass()); + private final SmallRyeConfig config; private final ObjectMapper mapper; + private final JFRConnectionToolkit jfrConnectionToolkit; + private final LocalStorageTemplateService localStorageTemplateService; @Inject - RecordingsContext(ObjectMapper mapper) { + RecordingsContext( + SmallRyeConfig config, + ObjectMapper mapper, + JFRConnectionToolkit jfrConnectionToolkit, + LocalStorageTemplateService localStorageTemplateService) { + this.config = config; this.mapper = mapper; + this.jfrConnectionToolkit = jfrConnectionToolkit; + this.localStorageTemplateService = localStorageTemplateService; } @Override @@ -49,67 +81,146 @@ public String path() { @Override public void handle(HttpExchange exchange) throws IOException { - String mtd = exchange.getRequestMethod(); - switch (mtd) { - case "GET": - try { - List recordings = getRecordings(); - exchange.sendResponseHeaders(HttpStatus.SC_OK, 0); + try { + String mtd = exchange.getRequestMethod(); + if (!ensureMethodAccepted(exchange)) { + return; + } + switch (mtd) { + case "GET": try (OutputStream response = exchange.getResponseBody()) { + List recordings = getRecordings(); + exchange.sendResponseHeaders(HttpStatus.SC_OK, 0); mapper.writeValue(response, recordings); + } catch (Exception e) { + log.error("recordings serialization failure", e); + } + break; + case "POST": + try (InputStream body = exchange.getRequestBody()) { + StartRecordingRequest req = + mapper.readValue(body, StartRecordingRequest.class); + if (!req.isValid()) { + exchange.sendResponseHeaders(HttpStatus.SC_BAD_REQUEST, -1); + return; + } + SerializableRecordingDescriptor recording = startRecording(req); + exchange.sendResponseHeaders(HttpStatus.SC_CREATED, 0); + try (OutputStream response = exchange.getResponseBody()) { + mapper.writeValue(response, recording); + } + } catch (QuantityConversionException + | ServiceNotAvailableException + | FlightRecorderException + | org.openjdk.jmc.rjmx.services.jfr.FlightRecorderException + | InvalidEventTemplateException + | InvalidXmlException + | IOException e) { + log.error("Failed to start recording", e); + exchange.sendResponseHeaders(HttpStatus.SC_INTERNAL_SERVER_ERROR, -1); } - } catch (Exception e) { - log.error("recordings serialization failure", e); - } finally { - exchange.close(); - } - break; - default: - exchange.sendResponseHeaders(HttpStatus.SC_NOT_FOUND, -1); - exchange.close(); - break; + break; + default: + log.warn("Unknown request method {}", mtd); + exchange.sendResponseHeaders(HttpStatus.SC_METHOD_NOT_ALLOWED, -1); + break; + } + } finally { + exchange.close(); + } + } + + private boolean ensureMethodAccepted(HttpExchange exchange) throws IOException { + Set blocked = Set.of("POST"); + String mtd = exchange.getRequestMethod(); + boolean restricted = blocked.contains(mtd); + if (!restricted) { + return true; + } + boolean passed = restricted && MutatingRemoteContext.apiWritesEnabled(config); + if (!passed) { + exchange.sendResponseHeaders(HttpStatus.SC_FORBIDDEN, -1); } + return passed; } - private List getRecordings() { + private List getRecordings() { return FlightRecorder.getFlightRecorder().getRecordings().stream() - .map(RecordingInfo::new) + .map(SerializableRecordingDescriptor::new) .collect(Collectors.toList()); } - @SuppressFBWarnings(value = "URF_UNREAD_FIELD") - private static class RecordingInfo { - - public final long id; - public final String name; - public final String state; - public final Map options; - public final long startTime; - public final long duration; - public final boolean isContinuous; - public final boolean toDisk; - public final long maxSize; - public final long maxAge; - - RecordingInfo(Recording rec) { - this.id = rec.getId(); - this.name = rec.getName(); - this.state = rec.getState().name(); - this.options = rec.getSettings(); - if (rec.getStartTime() != null) { - this.startTime = rec.getStartTime().toEpochMilli(); + private SerializableRecordingDescriptor startRecording(StartRecordingRequest req) + throws QuantityConversionException, ServiceNotAvailableException, + FlightRecorderException, + org.openjdk.jmc.rjmx.services.jfr.FlightRecorderException, + InvalidEventTemplateException, InvalidXmlException, IOException { + Runnable cleanup = () -> {}; + try { + JFRConnection conn = + jfrConnectionToolkit.connect( + jfrConnectionToolkit.createServiceURL("localhost", 0)); + IConstrainedMap events; + if (req.requestsCustomTemplate()) { + Template template = + localStorageTemplateService.addTemplate( + new ByteArrayInputStream( + req.template.getBytes(StandardCharsets.UTF_8))); + events = localStorageTemplateService.getEvents(template).orElseThrow(); + cleanup = + () -> { + try { + localStorageTemplateService.deleteTemplate(template); + } catch (InvalidEventTemplateException | IOException e) { + log.error("Failed to clean up template " + template.getName(), e); + } + }; } else { - this.startTime = 0; - } - this.isContinuous = rec.getDuration() == null; - this.duration = this.isContinuous ? 0 : rec.getDuration().toMillis(); - this.toDisk = rec.isToDisk(); - this.maxSize = rec.getMaxSize(); - if (rec.getMaxAge() != null) { - this.maxAge = rec.getMaxAge().toMillis(); - } else { - this.maxAge = 0; + events = + new RemoteTemplateService(conn) + .getEvents(req.localTemplateName, TemplateType.TARGET).stream() + .findFirst() + .orElseThrow(); } + IFlightRecorderService svc = conn.getService(); + return new SerializableRecordingDescriptor( + svc.start( + new RecordingOptionsBuilder(conn.getService()) + .name(req.name) + .duration(UnitLookup.MILLISECOND.quantity(req.duration)) + .maxSize(UnitLookup.BYTE.quantity(req.maxSize)) + .maxAge(UnitLookup.MILLISECOND.quantity(req.maxAge)) + .toDisk(true) + .build(), + events)); + } finally { + cleanup.run(); + } + } + + static class StartRecordingRequest { + + public String name; + public String localTemplateName; + public String template; + public long duration; + public long maxSize; + public long maxAge; + + boolean requestsCustomTemplate() { + return !StringUtils.isBlank(template); + } + + boolean requestsBundledTemplate() { + return !StringUtils.isBlank(localTemplateName); + } + + boolean isValid() { + boolean requestsCustomTemplate = requestsCustomTemplate(); + boolean requestsBundledTemplate = requestsBundledTemplate(); + boolean requestsEither = requestsCustomTemplate || requestsBundledTemplate; + boolean requestsBoth = requestsCustomTemplate && requestsBundledTemplate; + return requestsEither && !requestsBoth; } } } diff --git a/src/main/java/io/cryostat/agent/remote/RemoteContext.java b/src/main/java/io/cryostat/agent/remote/RemoteContext.java index 5290cd39..3b5a2271 100644 --- a/src/main/java/io/cryostat/agent/remote/RemoteContext.java +++ b/src/main/java/io/cryostat/agent/remote/RemoteContext.java @@ -19,4 +19,8 @@ public interface RemoteContext extends HttpHandler { String path(); + + default boolean available() { + return true; + } } diff --git a/src/main/resources/META-INF/microprofile-config.properties b/src/main/resources/META-INF/microprofile-config.properties index c1eac25e..3af7416f 100644 --- a/src/main/resources/META-INF/microprofile-config.properties +++ b/src/main/resources/META-INF/microprofile-config.properties @@ -1,6 +1,8 @@ cryostat.agent.app.name=cryostat-agent cryostat.agent.baseuri= +cryostat.agent.api.writes-enabled=false + cryostat.agent.webclient.ssl.trust-all=false cryostat.agent.webclient.ssl.verify-hostname=true cryostat.agent.webclient.connect.timeout-ms=1000 From 08a200fcac0ef4f9ffcd554f17acad77ea8d1863 Mon Sep 17 00:00:00 2001 From: Andrew Azores Date: Fri, 4 Aug 2023 15:08:33 -0400 Subject: [PATCH 02/45] fix license header --- .../agent/remote/MutatingRemoteContext.java | 42 +++++-------------- 1 file changed, 10 insertions(+), 32 deletions(-) diff --git a/src/main/java/io/cryostat/agent/remote/MutatingRemoteContext.java b/src/main/java/io/cryostat/agent/remote/MutatingRemoteContext.java index 89f65c67..753a2f52 100644 --- a/src/main/java/io/cryostat/agent/remote/MutatingRemoteContext.java +++ b/src/main/java/io/cryostat/agent/remote/MutatingRemoteContext.java @@ -1,39 +1,17 @@ /* - * Copyright The Cryostat Authors + * Copyright The Cryostat Authors. * - * The Universal Permissive License (UPL), Version 1.0 + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at * - * Subject to the condition set forth below, permission is hereby granted to any - * person obtaining a copy of this software, associated documentation and/or data - * (collectively the "Software"), free of charge and under any and all copyright - * rights in the Software, and any and all patent rights owned or freely - * licensable by each licensor hereunder covering either (i) the unmodified - * Software as contributed to or provided by such licensor, or (ii) the Larger - * Works (as defined below), to deal in both + * http://www.apache.org/licenses/LICENSE-2.0 * - * (a) the Software, and - * (b) any piece of software and/or hardware listed in the lrgrwrks.txt file if - * one is included with the Software (each a "Larger Work" to which the Software - * is contributed by such licensors), - * - * without restriction, including without limitation the rights to copy, create - * derivative works of, display, perform, and distribute the Software and make, - * use, sell, offer for sale, import, export, have made, and have sold the - * Software and the Larger Work(s), and to sublicense the foregoing rights on - * either these or other terms. - * - * This license is subject to the following condition: - * The above copyright notice and either this complete permission notice or at - * a minimum a reference to the UPL must be included in all copies or - * substantial portions of the Software. - * - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, - * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE - * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER - * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, - * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE - * SOFTWARE. + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. */ package io.cryostat.agent.remote; From 1a2acaa460875270843b6daf4537ad1f1492bf8d Mon Sep 17 00:00:00 2001 From: Andrew Azores Date: Mon, 31 Jul 2023 10:41:15 -0400 Subject: [PATCH 03/45] end paths with / so as to not match by prefix --- .../java/io/cryostat/agent/remote/EventTemplatesContext.java | 2 +- src/main/java/io/cryostat/agent/remote/EventTypesContext.java | 2 +- src/main/java/io/cryostat/agent/remote/MBeanContext.java | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/main/java/io/cryostat/agent/remote/EventTemplatesContext.java b/src/main/java/io/cryostat/agent/remote/EventTemplatesContext.java index fb5fca55..bf12b1e9 100644 --- a/src/main/java/io/cryostat/agent/remote/EventTemplatesContext.java +++ b/src/main/java/io/cryostat/agent/remote/EventTemplatesContext.java @@ -43,7 +43,7 @@ class EventTemplatesContext implements RemoteContext { @Override public String path() { - return "/event-templates"; + return "/event-templates/"; } @Override diff --git a/src/main/java/io/cryostat/agent/remote/EventTypesContext.java b/src/main/java/io/cryostat/agent/remote/EventTypesContext.java index 0fc79763..fb1f90cb 100644 --- a/src/main/java/io/cryostat/agent/remote/EventTypesContext.java +++ b/src/main/java/io/cryostat/agent/remote/EventTypesContext.java @@ -45,7 +45,7 @@ class EventTypesContext implements RemoteContext { @Override public String path() { - return "/event-types"; + return "/event-types/"; } @Override diff --git a/src/main/java/io/cryostat/agent/remote/MBeanContext.java b/src/main/java/io/cryostat/agent/remote/MBeanContext.java index c085df90..7ee21456 100644 --- a/src/main/java/io/cryostat/agent/remote/MBeanContext.java +++ b/src/main/java/io/cryostat/agent/remote/MBeanContext.java @@ -59,7 +59,7 @@ class MBeanContext implements RemoteContext { @Override public String path() { - return "/mbean-metrics"; + return "/mbean-metrics/"; } @Override From 18f3742a5c353022f952010306afa6e79ea37683 Mon Sep 17 00:00:00 2001 From: Andrew Azores Date: Mon, 31 Jul 2023 10:41:31 -0400 Subject: [PATCH 04/45] refactor: extract methods --- .../agent/remote/RecordingsContext.java | 65 ++++++++++--------- 1 file changed, 36 insertions(+), 29 deletions(-) diff --git a/src/main/java/io/cryostat/agent/remote/RecordingsContext.java b/src/main/java/io/cryostat/agent/remote/RecordingsContext.java index d5c9cc96..9955409d 100644 --- a/src/main/java/io/cryostat/agent/remote/RecordingsContext.java +++ b/src/main/java/io/cryostat/agent/remote/RecordingsContext.java @@ -88,37 +88,10 @@ public void handle(HttpExchange exchange) throws IOException { } switch (mtd) { case "GET": - try (OutputStream response = exchange.getResponseBody()) { - List recordings = getRecordings(); - exchange.sendResponseHeaders(HttpStatus.SC_OK, 0); - mapper.writeValue(response, recordings); - } catch (Exception e) { - log.error("recordings serialization failure", e); - } + handleGetList(exchange); break; case "POST": - try (InputStream body = exchange.getRequestBody()) { - StartRecordingRequest req = - mapper.readValue(body, StartRecordingRequest.class); - if (!req.isValid()) { - exchange.sendResponseHeaders(HttpStatus.SC_BAD_REQUEST, -1); - return; - } - SerializableRecordingDescriptor recording = startRecording(req); - exchange.sendResponseHeaders(HttpStatus.SC_CREATED, 0); - try (OutputStream response = exchange.getResponseBody()) { - mapper.writeValue(response, recording); - } - } catch (QuantityConversionException - | ServiceNotAvailableException - | FlightRecorderException - | org.openjdk.jmc.rjmx.services.jfr.FlightRecorderException - | InvalidEventTemplateException - | InvalidXmlException - | IOException e) { - log.error("Failed to start recording", e); - exchange.sendResponseHeaders(HttpStatus.SC_INTERNAL_SERVER_ERROR, -1); - } + handleStart(exchange); break; default: log.warn("Unknown request method {}", mtd); @@ -130,6 +103,40 @@ public void handle(HttpExchange exchange) throws IOException { } } + private void handleGetList(HttpExchange exchange) { + try (OutputStream response = exchange.getResponseBody()) { + List recordings = getRecordings(); + exchange.sendResponseHeaders(HttpStatus.SC_OK, 0); + mapper.writeValue(response, recordings); + } catch (Exception e) { + log.error("recordings serialization failure", e); + } + } + + private void handleStart(HttpExchange exchange) throws IOException { + try (InputStream body = exchange.getRequestBody()) { + StartRecordingRequest req = mapper.readValue(body, StartRecordingRequest.class); + if (!req.isValid()) { + exchange.sendResponseHeaders(HttpStatus.SC_BAD_REQUEST, -1); + return; + } + SerializableRecordingDescriptor recording = startRecording(req); + exchange.sendResponseHeaders(HttpStatus.SC_CREATED, 0); + try (OutputStream response = exchange.getResponseBody()) { + mapper.writeValue(response, recording); + } + } catch (QuantityConversionException + | ServiceNotAvailableException + | FlightRecorderException + | org.openjdk.jmc.rjmx.services.jfr.FlightRecorderException + | InvalidEventTemplateException + | InvalidXmlException + | IOException e) { + log.error("Failed to start recording", e); + exchange.sendResponseHeaders(HttpStatus.SC_INTERNAL_SERVER_ERROR, -1); + } + } + private boolean ensureMethodAccepted(HttpExchange exchange) throws IOException { Set blocked = Set.of("POST"); String mtd = exchange.getRequestMethod(); From 65777347af76430e4dc13efe8341abac242a2fe5 Mon Sep 17 00:00:00 2001 From: Andrew Azores Date: Mon, 31 Jul 2023 10:54:23 -0400 Subject: [PATCH 05/45] add utility for extracting ID from path --- .../agent/remote/RecordingsContext.java | 23 +++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/src/main/java/io/cryostat/agent/remote/RecordingsContext.java b/src/main/java/io/cryostat/agent/remote/RecordingsContext.java index 9955409d..d0f38260 100644 --- a/src/main/java/io/cryostat/agent/remote/RecordingsContext.java +++ b/src/main/java/io/cryostat/agent/remote/RecordingsContext.java @@ -22,6 +22,8 @@ import java.nio.charset.StandardCharsets; import java.util.List; import java.util.Set; +import java.util.regex.Matcher; +import java.util.regex.Pattern; import java.util.stream.Collectors; import javax.inject.Inject; @@ -56,6 +58,10 @@ class RecordingsContext implements RemoteContext { + private static final String PATH = "/recordings"; + private static final Pattern PATH_ID_PATTERN = + Pattern.compile("^" + PATH + "/(\\d+)$", Pattern.MULTILINE); + private final Logger log = LoggerFactory.getLogger(getClass()); private final SmallRyeConfig config; private final ObjectMapper mapper; @@ -76,7 +82,7 @@ class RecordingsContext implements RemoteContext { @Override public String path() { - return "/recordings"; + return PATH; } @Override @@ -88,7 +94,12 @@ public void handle(HttpExchange exchange) throws IOException { } switch (mtd) { case "GET": - handleGetList(exchange); + int id = extractId(exchange); + if (id < 0) { + handleGetList(exchange); + } else { + exchange.sendResponseHeaders(HttpStatus.SC_NOT_IMPLEMENTED, -1); + } break; case "POST": handleStart(exchange); @@ -103,6 +114,14 @@ public void handle(HttpExchange exchange) throws IOException { } } + private static int extractId(HttpExchange exchange) throws IOException { + Matcher m = PATH_ID_PATTERN.matcher(exchange.getRequestURI().getPath()); + if (!m.find()) { + return Integer.MIN_VALUE; + } + return Integer.parseInt(m.group(1)); + } + private void handleGetList(HttpExchange exchange) { try (OutputStream response = exchange.getResponseBody()) { List recordings = getRecordings(); From 8789208adad37f56482268cab7c6c1add23faa3c Mon Sep 17 00:00:00 2001 From: Andrew Azores Date: Mon, 31 Jul 2023 15:00:49 -0400 Subject: [PATCH 06/45] fixup! add utility for extracting ID from path --- src/main/java/io/cryostat/agent/remote/RecordingsContext.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/io/cryostat/agent/remote/RecordingsContext.java b/src/main/java/io/cryostat/agent/remote/RecordingsContext.java index d0f38260..d4c4e1fa 100644 --- a/src/main/java/io/cryostat/agent/remote/RecordingsContext.java +++ b/src/main/java/io/cryostat/agent/remote/RecordingsContext.java @@ -95,7 +95,7 @@ public void handle(HttpExchange exchange) throws IOException { switch (mtd) { case "GET": int id = extractId(exchange); - if (id < 0) { + if (id == Integer.MIN_VALUE) { handleGetList(exchange); } else { exchange.sendResponseHeaders(HttpStatus.SC_NOT_IMPLEMENTED, -1); From 7b6275398c60da4011ce336f93c56ccbe069206f Mon Sep 17 00:00:00 2001 From: Andrew Azores Date: Mon, 31 Jul 2023 11:33:49 -0400 Subject: [PATCH 07/45] add handling for stopping recording --- .../agent/remote/RecordingsContext.java | 40 ++++++++++++++++++- 1 file changed, 39 insertions(+), 1 deletion(-) diff --git a/src/main/java/io/cryostat/agent/remote/RecordingsContext.java b/src/main/java/io/cryostat/agent/remote/RecordingsContext.java index d4c4e1fa..9e2f801f 100644 --- a/src/main/java/io/cryostat/agent/remote/RecordingsContext.java +++ b/src/main/java/io/cryostat/agent/remote/RecordingsContext.java @@ -92,9 +92,10 @@ public void handle(HttpExchange exchange) throws IOException { if (!ensureMethodAccepted(exchange)) { return; } + int id = Integer.MIN_VALUE; switch (mtd) { case "GET": - int id = extractId(exchange); + id = extractId(exchange); if (id == Integer.MIN_VALUE) { handleGetList(exchange); } else { @@ -104,6 +105,13 @@ public void handle(HttpExchange exchange) throws IOException { case "POST": handleStart(exchange); break; + case "PATCH": + id = extractId(exchange); + if (id < 0) { + handleStop(exchange, id); + } else { + exchange.sendResponseHeaders(HttpStatus.SC_BAD_REQUEST, -1); + } default: log.warn("Unknown request method {}", mtd); exchange.sendResponseHeaders(HttpStatus.SC_METHOD_NOT_ALLOWED, -1); @@ -156,6 +164,36 @@ private void handleStart(HttpExchange exchange) throws IOException { } } + private void handleStop(HttpExchange exchange, int id) throws IOException { + FlightRecorder.getFlightRecorder().getRecordings().stream() + .filter(r -> r.getId() == id) + .findFirst() + .ifPresentOrElse( + r -> { + try { + try { + boolean stopped = r.stop(); + if (!stopped) { + exchange.sendResponseHeaders(HttpStatus.SC_BAD_REQUEST, -1); + } else { + exchange.sendResponseHeaders(HttpStatus.SC_NO_CONTENT, -1); + } + } catch (IllegalStateException e) { + exchange.sendResponseHeaders(HttpStatus.SC_CONFLICT, -1); + } + } catch (IOException ioe) { + throw new IllegalStateException(ioe); + } + }, + () -> { + try { + exchange.sendResponseHeaders(HttpStatus.SC_NOT_FOUND, -1); + } catch (IOException e) { + throw new IllegalStateException(e); + } + }); + } + private boolean ensureMethodAccepted(HttpExchange exchange) throws IOException { Set blocked = Set.of("POST"); String mtd = exchange.getRequestMethod(); From b8d10b8f229c961609f12e7d4f616f1aae482b84 Mon Sep 17 00:00:00 2001 From: Andrew Azores Date: Mon, 31 Jul 2023 11:43:27 -0400 Subject: [PATCH 08/45] add handling for closing (deleting) recording --- .../agent/remote/RecordingsContext.java | 69 ++++++++++++------- 1 file changed, 46 insertions(+), 23 deletions(-) diff --git a/src/main/java/io/cryostat/agent/remote/RecordingsContext.java b/src/main/java/io/cryostat/agent/remote/RecordingsContext.java index 9e2f801f..45a65f65 100644 --- a/src/main/java/io/cryostat/agent/remote/RecordingsContext.java +++ b/src/main/java/io/cryostat/agent/remote/RecordingsContext.java @@ -22,6 +22,7 @@ import java.nio.charset.StandardCharsets; import java.util.List; import java.util.Set; +import java.util.function.Consumer; import java.util.regex.Matcher; import java.util.regex.Pattern; import java.util.stream.Collectors; @@ -52,6 +53,7 @@ import com.sun.net.httpserver.HttpExchange; import io.smallrye.config.SmallRyeConfig; import jdk.jfr.FlightRecorder; +import jdk.jfr.Recording; import org.apache.http.HttpStatus; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -112,6 +114,13 @@ public void handle(HttpExchange exchange) throws IOException { } else { exchange.sendResponseHeaders(HttpStatus.SC_BAD_REQUEST, -1); } + case "DELETE": + id = extractId(exchange); + if (id < 0) { + handleDelete(exchange, id); + } else { + exchange.sendResponseHeaders(HttpStatus.SC_BAD_REQUEST, -1); + } default: log.warn("Unknown request method {}", mtd); exchange.sendResponseHeaders(HttpStatus.SC_METHOD_NOT_ALLOWED, -1); @@ -165,33 +174,47 @@ private void handleStart(HttpExchange exchange) throws IOException { } private void handleStop(HttpExchange exchange, int id) throws IOException { + invokeOnRecording( + exchange, + id, + r -> { + try { + boolean stopped = r.stop(); + if (!stopped) { + sendHeader(exchange, HttpStatus.SC_BAD_REQUEST); + } else { + sendHeader(exchange, HttpStatus.SC_NO_CONTENT); + } + } catch (IllegalStateException e) { + sendHeader(exchange, HttpStatus.SC_CONFLICT); + } + }); + } + + private void handleDelete(HttpExchange exchange, int id) throws IOException { + invokeOnRecording( + exchange, + id, + r -> { + r.close(); + sendHeader(exchange, HttpStatus.SC_NO_CONTENT); + }); + } + + private void invokeOnRecording(HttpExchange exchange, long id, Consumer consumer) { FlightRecorder.getFlightRecorder().getRecordings().stream() .filter(r -> r.getId() == id) .findFirst() .ifPresentOrElse( - r -> { - try { - try { - boolean stopped = r.stop(); - if (!stopped) { - exchange.sendResponseHeaders(HttpStatus.SC_BAD_REQUEST, -1); - } else { - exchange.sendResponseHeaders(HttpStatus.SC_NO_CONTENT, -1); - } - } catch (IllegalStateException e) { - exchange.sendResponseHeaders(HttpStatus.SC_CONFLICT, -1); - } - } catch (IOException ioe) { - throw new IllegalStateException(ioe); - } - }, - () -> { - try { - exchange.sendResponseHeaders(HttpStatus.SC_NOT_FOUND, -1); - } catch (IOException e) { - throw new IllegalStateException(e); - } - }); + consumer::accept, () -> sendHeader(exchange, HttpStatus.SC_NOT_FOUND)); + } + + private void sendHeader(HttpExchange exchange, int status) { + try { + exchange.sendResponseHeaders(status, -1); + } catch (IOException e) { + throw new IllegalStateException(e); + } } private boolean ensureMethodAccepted(HttpExchange exchange) throws IOException { From 15195e76e20f3ef0614268f1017c6639c5e8b6c0 Mon Sep 17 00:00:00 2001 From: Andrew Azores Date: Mon, 31 Jul 2023 15:01:10 -0400 Subject: [PATCH 09/45] fixup! add handling for closing (deleting) recording --- src/main/java/io/cryostat/agent/remote/RecordingsContext.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/main/java/io/cryostat/agent/remote/RecordingsContext.java b/src/main/java/io/cryostat/agent/remote/RecordingsContext.java index 45a65f65..3fa14041 100644 --- a/src/main/java/io/cryostat/agent/remote/RecordingsContext.java +++ b/src/main/java/io/cryostat/agent/remote/RecordingsContext.java @@ -116,11 +116,12 @@ public void handle(HttpExchange exchange) throws IOException { } case "DELETE": id = extractId(exchange); - if (id < 0) { + if (id >= 0) { handleDelete(exchange, id); } else { exchange.sendResponseHeaders(HttpStatus.SC_BAD_REQUEST, -1); } + break; default: log.warn("Unknown request method {}", mtd); exchange.sendResponseHeaders(HttpStatus.SC_METHOD_NOT_ALLOWED, -1); From 8cc9a879196de5d63d4885ae3abb59c91ab777c8 Mon Sep 17 00:00:00 2001 From: Andrew Azores Date: Mon, 31 Jul 2023 15:01:19 -0400 Subject: [PATCH 10/45] fixup! add handling for stopping recording --- src/main/java/io/cryostat/agent/remote/RecordingsContext.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/main/java/io/cryostat/agent/remote/RecordingsContext.java b/src/main/java/io/cryostat/agent/remote/RecordingsContext.java index 3fa14041..cb1cbe3d 100644 --- a/src/main/java/io/cryostat/agent/remote/RecordingsContext.java +++ b/src/main/java/io/cryostat/agent/remote/RecordingsContext.java @@ -109,11 +109,12 @@ public void handle(HttpExchange exchange) throws IOException { break; case "PATCH": id = extractId(exchange); - if (id < 0) { + if (id >= 0) { handleStop(exchange, id); } else { exchange.sendResponseHeaders(HttpStatus.SC_BAD_REQUEST, -1); } + break; case "DELETE": id = extractId(exchange); if (id >= 0) { From e4df5266dc95cb1d3d1ed76fbca4e27f8a9b20dc Mon Sep 17 00:00:00 2001 From: Andrew Azores Date: Mon, 31 Jul 2023 15:07:33 -0400 Subject: [PATCH 11/45] only allow GET requests if write-operations are not enabled --- src/main/java/io/cryostat/agent/remote/RecordingsContext.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/io/cryostat/agent/remote/RecordingsContext.java b/src/main/java/io/cryostat/agent/remote/RecordingsContext.java index cb1cbe3d..b855302f 100644 --- a/src/main/java/io/cryostat/agent/remote/RecordingsContext.java +++ b/src/main/java/io/cryostat/agent/remote/RecordingsContext.java @@ -220,9 +220,9 @@ private void sendHeader(HttpExchange exchange, int status) { } private boolean ensureMethodAccepted(HttpExchange exchange) throws IOException { - Set blocked = Set.of("POST"); + Set alwaysAllowed = Set.of("GET"); String mtd = exchange.getRequestMethod(); - boolean restricted = blocked.contains(mtd); + boolean restricted = !alwaysAllowed.contains(mtd); if (!restricted) { return true; } From 0bcaf25ec30a18a8f16c2eafae3802912a07b0d5 Mon Sep 17 00:00:00 2001 From: Andrew Azores Date: Wed, 9 Aug 2023 09:41:40 -0400 Subject: [PATCH 12/45] extract HTTP response body length constants --- .../java/io/cryostat/agent/WebServer.java | 12 ++++++---- .../agent/remote/EventTemplatesContext.java | 5 +++-- .../agent/remote/EventTypesContext.java | 8 ++++--- .../cryostat/agent/remote/MBeanContext.java | 6 +++-- .../agent/remote/RecordingsContext.java | 22 ++++++++++--------- .../cryostat/agent/remote/RemoteContext.java | 4 ++++ 6 files changed, 36 insertions(+), 21 deletions(-) diff --git a/src/main/java/io/cryostat/agent/WebServer.java b/src/main/java/io/cryostat/agent/WebServer.java index 1d2f78c6..bf48374c 100644 --- a/src/main/java/io/cryostat/agent/WebServer.java +++ b/src/main/java/io/cryostat/agent/WebServer.java @@ -163,7 +163,8 @@ private HttpHandler wrap(HttpHandler handler) { handler.handle(x); } catch (Exception e) { log.error("Unhandled exception", e); - x.sendResponseHeaders(HttpStatus.SC_INTERNAL_SERVER_ERROR, 0); + x.sendResponseHeaders( + HttpStatus.SC_INTERNAL_SERVER_ERROR, RemoteContext.BODY_LENGTH_NONE); x.close(); } }; @@ -184,15 +185,18 @@ public void handle(HttpExchange exchange) throws IOException { case "POST": synchronized (WebServer.this.credentials) { executor.execute(registration.get()::tryRegister); - exchange.sendResponseHeaders(HttpStatus.SC_NO_CONTENT, -1); + exchange.sendResponseHeaders( + HttpStatus.SC_NO_CONTENT, RemoteContext.BODY_LENGTH_NONE); } break; case "GET": - exchange.sendResponseHeaders(HttpStatus.SC_NO_CONTENT, -1); + exchange.sendResponseHeaders( + HttpStatus.SC_NO_CONTENT, RemoteContext.BODY_LENGTH_NONE); break; default: log.warn("Unknown request method {}", mtd); - exchange.sendResponseHeaders(HttpStatus.SC_METHOD_NOT_ALLOWED, -1); + exchange.sendResponseHeaders( + HttpStatus.SC_METHOD_NOT_ALLOWED, RemoteContext.BODY_LENGTH_NONE); break; } } finally { diff --git a/src/main/java/io/cryostat/agent/remote/EventTemplatesContext.java b/src/main/java/io/cryostat/agent/remote/EventTemplatesContext.java index bf12b1e9..53aa3bed 100644 --- a/src/main/java/io/cryostat/agent/remote/EventTemplatesContext.java +++ b/src/main/java/io/cryostat/agent/remote/EventTemplatesContext.java @@ -53,7 +53,7 @@ public void handle(HttpExchange exchange) throws IOException { switch (mtd) { case "GET": try { - exchange.sendResponseHeaders(HttpStatus.SC_OK, 0); + exchange.sendResponseHeaders(HttpStatus.SC_OK, BODY_LENGTH_UNKNOWN); try (OutputStream response = exchange.getResponseBody()) { FlightRecorderMXBean bean = ManagementFactory.getPlatformMXBean(FlightRecorderMXBean.class); @@ -69,7 +69,8 @@ public void handle(HttpExchange exchange) throws IOException { break; default: log.warn("Unknown request method {}", mtd); - exchange.sendResponseHeaders(HttpStatus.SC_METHOD_NOT_ALLOWED, -1); + exchange.sendResponseHeaders( + HttpStatus.SC_METHOD_NOT_ALLOWED, BODY_LENGTH_NONE); break; } } finally { diff --git a/src/main/java/io/cryostat/agent/remote/EventTypesContext.java b/src/main/java/io/cryostat/agent/remote/EventTypesContext.java index fb1f90cb..620f5315 100644 --- a/src/main/java/io/cryostat/agent/remote/EventTypesContext.java +++ b/src/main/java/io/cryostat/agent/remote/EventTypesContext.java @@ -59,17 +59,19 @@ public void handle(HttpExchange exchange) throws IOException { events.addAll(getEventTypes()); } catch (Exception e) { log.error("events serialization failure", e); - exchange.sendResponseHeaders(HttpStatus.SC_INTERNAL_SERVER_ERROR, 0); + exchange.sendResponseHeaders( + HttpStatus.SC_INTERNAL_SERVER_ERROR, BODY_LENGTH_NONE); break; } - exchange.sendResponseHeaders(HttpStatus.SC_OK, 0); + exchange.sendResponseHeaders(HttpStatus.SC_OK, BODY_LENGTH_UNKNOWN); try (OutputStream response = exchange.getResponseBody()) { mapper.writeValue(response, events); } break; default: log.warn("Unknown request method {}", mtd); - exchange.sendResponseHeaders(HttpStatus.SC_METHOD_NOT_ALLOWED, -1); + exchange.sendResponseHeaders( + HttpStatus.SC_METHOD_NOT_ALLOWED, BODY_LENGTH_NONE); break; } } finally { diff --git a/src/main/java/io/cryostat/agent/remote/MBeanContext.java b/src/main/java/io/cryostat/agent/remote/MBeanContext.java index 7ee21456..b8af5143 100644 --- a/src/main/java/io/cryostat/agent/remote/MBeanContext.java +++ b/src/main/java/io/cryostat/agent/remote/MBeanContext.java @@ -70,7 +70,8 @@ public void handle(HttpExchange exchange) throws IOException { case "GET": try { MBeanMetrics metrics = getMBeanMetrics(); - exchange.sendResponseHeaders(HttpStatus.SC_OK, 0); + exchange.sendResponseHeaders( + HttpStatus.SC_OK, RemoteContext.BODY_LENGTH_UNKNOWN); try (OutputStream response = exchange.getResponseBody()) { mapper.writeValue(response, metrics); } @@ -80,7 +81,8 @@ public void handle(HttpExchange exchange) throws IOException { break; default: log.warn("Unknown request method {}", mtd); - exchange.sendResponseHeaders(HttpStatus.SC_METHOD_NOT_ALLOWED, -1); + exchange.sendResponseHeaders( + HttpStatus.SC_METHOD_NOT_ALLOWED, RemoteContext.BODY_LENGTH_NONE); break; } } finally { diff --git a/src/main/java/io/cryostat/agent/remote/RecordingsContext.java b/src/main/java/io/cryostat/agent/remote/RecordingsContext.java index b855302f..a59f2060 100644 --- a/src/main/java/io/cryostat/agent/remote/RecordingsContext.java +++ b/src/main/java/io/cryostat/agent/remote/RecordingsContext.java @@ -101,7 +101,8 @@ public void handle(HttpExchange exchange) throws IOException { if (id == Integer.MIN_VALUE) { handleGetList(exchange); } else { - exchange.sendResponseHeaders(HttpStatus.SC_NOT_IMPLEMENTED, -1); + exchange.sendResponseHeaders( + HttpStatus.SC_NOT_IMPLEMENTED, BODY_LENGTH_NONE); } break; case "POST": @@ -112,7 +113,7 @@ public void handle(HttpExchange exchange) throws IOException { if (id >= 0) { handleStop(exchange, id); } else { - exchange.sendResponseHeaders(HttpStatus.SC_BAD_REQUEST, -1); + exchange.sendResponseHeaders(HttpStatus.SC_BAD_REQUEST, BODY_LENGTH_NONE); } break; case "DELETE": @@ -120,12 +121,13 @@ public void handle(HttpExchange exchange) throws IOException { if (id >= 0) { handleDelete(exchange, id); } else { - exchange.sendResponseHeaders(HttpStatus.SC_BAD_REQUEST, -1); + exchange.sendResponseHeaders(HttpStatus.SC_BAD_REQUEST, BODY_LENGTH_NONE); } break; default: log.warn("Unknown request method {}", mtd); - exchange.sendResponseHeaders(HttpStatus.SC_METHOD_NOT_ALLOWED, -1); + exchange.sendResponseHeaders( + HttpStatus.SC_METHOD_NOT_ALLOWED, BODY_LENGTH_NONE); break; } } finally { @@ -144,7 +146,7 @@ private static int extractId(HttpExchange exchange) throws IOException { private void handleGetList(HttpExchange exchange) { try (OutputStream response = exchange.getResponseBody()) { List recordings = getRecordings(); - exchange.sendResponseHeaders(HttpStatus.SC_OK, 0); + exchange.sendResponseHeaders(HttpStatus.SC_OK, BODY_LENGTH_UNKNOWN); mapper.writeValue(response, recordings); } catch (Exception e) { log.error("recordings serialization failure", e); @@ -155,11 +157,11 @@ private void handleStart(HttpExchange exchange) throws IOException { try (InputStream body = exchange.getRequestBody()) { StartRecordingRequest req = mapper.readValue(body, StartRecordingRequest.class); if (!req.isValid()) { - exchange.sendResponseHeaders(HttpStatus.SC_BAD_REQUEST, -1); + exchange.sendResponseHeaders(HttpStatus.SC_BAD_REQUEST, BODY_LENGTH_NONE); return; } SerializableRecordingDescriptor recording = startRecording(req); - exchange.sendResponseHeaders(HttpStatus.SC_CREATED, 0); + exchange.sendResponseHeaders(HttpStatus.SC_CREATED, BODY_LENGTH_UNKNOWN); try (OutputStream response = exchange.getResponseBody()) { mapper.writeValue(response, recording); } @@ -171,7 +173,7 @@ private void handleStart(HttpExchange exchange) throws IOException { | InvalidXmlException | IOException e) { log.error("Failed to start recording", e); - exchange.sendResponseHeaders(HttpStatus.SC_INTERNAL_SERVER_ERROR, -1); + exchange.sendResponseHeaders(HttpStatus.SC_INTERNAL_SERVER_ERROR, BODY_LENGTH_NONE); } } @@ -213,7 +215,7 @@ private void invokeOnRecording(HttpExchange exchange, long id, Consumer Date: Mon, 14 Aug 2023 11:27:21 -0400 Subject: [PATCH 13/45] cleanup --- src/main/java/io/cryostat/agent/remote/MBeanContext.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/main/java/io/cryostat/agent/remote/MBeanContext.java b/src/main/java/io/cryostat/agent/remote/MBeanContext.java index b8af5143..26d466e9 100644 --- a/src/main/java/io/cryostat/agent/remote/MBeanContext.java +++ b/src/main/java/io/cryostat/agent/remote/MBeanContext.java @@ -70,8 +70,7 @@ public void handle(HttpExchange exchange) throws IOException { case "GET": try { MBeanMetrics metrics = getMBeanMetrics(); - exchange.sendResponseHeaders( - HttpStatus.SC_OK, RemoteContext.BODY_LENGTH_UNKNOWN); + exchange.sendResponseHeaders(HttpStatus.SC_OK, BODY_LENGTH_UNKNOWN); try (OutputStream response = exchange.getResponseBody()) { mapper.writeValue(response, metrics); } @@ -82,7 +81,7 @@ public void handle(HttpExchange exchange) throws IOException { default: log.warn("Unknown request method {}", mtd); exchange.sendResponseHeaders( - HttpStatus.SC_METHOD_NOT_ALLOWED, RemoteContext.BODY_LENGTH_NONE); + HttpStatus.SC_METHOD_NOT_ALLOWED, BODY_LENGTH_NONE); break; } } finally { From 6154aa807b677f1a1f0a47d6a443d0419aa5a490 Mon Sep 17 00:00:00 2001 From: Andrew Azores Date: Mon, 14 Aug 2023 11:35:24 -0400 Subject: [PATCH 14/45] feat(api): enable dynamic JFR stop, delete (#176) * end paths with / so as to not match by prefix * refactor: extract methods * add utility for extracting ID from path * add handling for stopping recording * add handling for closing (deleting) recording * only allow GET requests if write-operations are not enabled * extract HTTP response body length constants --- .../java/io/cryostat/agent/WebServer.java | 12 +- .../agent/remote/EventTemplatesContext.java | 7 +- .../agent/remote/EventTypesContext.java | 10 +- .../cryostat/agent/remote/MBeanContext.java | 7 +- .../agent/remote/RecordingsContext.java | 155 ++++++++++++++---- .../cryostat/agent/remote/RemoteContext.java | 4 + 6 files changed, 149 insertions(+), 46 deletions(-) diff --git a/src/main/java/io/cryostat/agent/WebServer.java b/src/main/java/io/cryostat/agent/WebServer.java index 1d2f78c6..bf48374c 100644 --- a/src/main/java/io/cryostat/agent/WebServer.java +++ b/src/main/java/io/cryostat/agent/WebServer.java @@ -163,7 +163,8 @@ private HttpHandler wrap(HttpHandler handler) { handler.handle(x); } catch (Exception e) { log.error("Unhandled exception", e); - x.sendResponseHeaders(HttpStatus.SC_INTERNAL_SERVER_ERROR, 0); + x.sendResponseHeaders( + HttpStatus.SC_INTERNAL_SERVER_ERROR, RemoteContext.BODY_LENGTH_NONE); x.close(); } }; @@ -184,15 +185,18 @@ public void handle(HttpExchange exchange) throws IOException { case "POST": synchronized (WebServer.this.credentials) { executor.execute(registration.get()::tryRegister); - exchange.sendResponseHeaders(HttpStatus.SC_NO_CONTENT, -1); + exchange.sendResponseHeaders( + HttpStatus.SC_NO_CONTENT, RemoteContext.BODY_LENGTH_NONE); } break; case "GET": - exchange.sendResponseHeaders(HttpStatus.SC_NO_CONTENT, -1); + exchange.sendResponseHeaders( + HttpStatus.SC_NO_CONTENT, RemoteContext.BODY_LENGTH_NONE); break; default: log.warn("Unknown request method {}", mtd); - exchange.sendResponseHeaders(HttpStatus.SC_METHOD_NOT_ALLOWED, -1); + exchange.sendResponseHeaders( + HttpStatus.SC_METHOD_NOT_ALLOWED, RemoteContext.BODY_LENGTH_NONE); break; } } finally { diff --git a/src/main/java/io/cryostat/agent/remote/EventTemplatesContext.java b/src/main/java/io/cryostat/agent/remote/EventTemplatesContext.java index fb5fca55..53aa3bed 100644 --- a/src/main/java/io/cryostat/agent/remote/EventTemplatesContext.java +++ b/src/main/java/io/cryostat/agent/remote/EventTemplatesContext.java @@ -43,7 +43,7 @@ class EventTemplatesContext implements RemoteContext { @Override public String path() { - return "/event-templates"; + return "/event-templates/"; } @Override @@ -53,7 +53,7 @@ public void handle(HttpExchange exchange) throws IOException { switch (mtd) { case "GET": try { - exchange.sendResponseHeaders(HttpStatus.SC_OK, 0); + exchange.sendResponseHeaders(HttpStatus.SC_OK, BODY_LENGTH_UNKNOWN); try (OutputStream response = exchange.getResponseBody()) { FlightRecorderMXBean bean = ManagementFactory.getPlatformMXBean(FlightRecorderMXBean.class); @@ -69,7 +69,8 @@ public void handle(HttpExchange exchange) throws IOException { break; default: log.warn("Unknown request method {}", mtd); - exchange.sendResponseHeaders(HttpStatus.SC_METHOD_NOT_ALLOWED, -1); + exchange.sendResponseHeaders( + HttpStatus.SC_METHOD_NOT_ALLOWED, BODY_LENGTH_NONE); break; } } finally { diff --git a/src/main/java/io/cryostat/agent/remote/EventTypesContext.java b/src/main/java/io/cryostat/agent/remote/EventTypesContext.java index 0fc79763..620f5315 100644 --- a/src/main/java/io/cryostat/agent/remote/EventTypesContext.java +++ b/src/main/java/io/cryostat/agent/remote/EventTypesContext.java @@ -45,7 +45,7 @@ class EventTypesContext implements RemoteContext { @Override public String path() { - return "/event-types"; + return "/event-types/"; } @Override @@ -59,17 +59,19 @@ public void handle(HttpExchange exchange) throws IOException { events.addAll(getEventTypes()); } catch (Exception e) { log.error("events serialization failure", e); - exchange.sendResponseHeaders(HttpStatus.SC_INTERNAL_SERVER_ERROR, 0); + exchange.sendResponseHeaders( + HttpStatus.SC_INTERNAL_SERVER_ERROR, BODY_LENGTH_NONE); break; } - exchange.sendResponseHeaders(HttpStatus.SC_OK, 0); + exchange.sendResponseHeaders(HttpStatus.SC_OK, BODY_LENGTH_UNKNOWN); try (OutputStream response = exchange.getResponseBody()) { mapper.writeValue(response, events); } break; default: log.warn("Unknown request method {}", mtd); - exchange.sendResponseHeaders(HttpStatus.SC_METHOD_NOT_ALLOWED, -1); + exchange.sendResponseHeaders( + HttpStatus.SC_METHOD_NOT_ALLOWED, BODY_LENGTH_NONE); break; } } finally { diff --git a/src/main/java/io/cryostat/agent/remote/MBeanContext.java b/src/main/java/io/cryostat/agent/remote/MBeanContext.java index c085df90..26d466e9 100644 --- a/src/main/java/io/cryostat/agent/remote/MBeanContext.java +++ b/src/main/java/io/cryostat/agent/remote/MBeanContext.java @@ -59,7 +59,7 @@ class MBeanContext implements RemoteContext { @Override public String path() { - return "/mbean-metrics"; + return "/mbean-metrics/"; } @Override @@ -70,7 +70,7 @@ public void handle(HttpExchange exchange) throws IOException { case "GET": try { MBeanMetrics metrics = getMBeanMetrics(); - exchange.sendResponseHeaders(HttpStatus.SC_OK, 0); + exchange.sendResponseHeaders(HttpStatus.SC_OK, BODY_LENGTH_UNKNOWN); try (OutputStream response = exchange.getResponseBody()) { mapper.writeValue(response, metrics); } @@ -80,7 +80,8 @@ public void handle(HttpExchange exchange) throws IOException { break; default: log.warn("Unknown request method {}", mtd); - exchange.sendResponseHeaders(HttpStatus.SC_METHOD_NOT_ALLOWED, -1); + exchange.sendResponseHeaders( + HttpStatus.SC_METHOD_NOT_ALLOWED, BODY_LENGTH_NONE); break; } } finally { diff --git a/src/main/java/io/cryostat/agent/remote/RecordingsContext.java b/src/main/java/io/cryostat/agent/remote/RecordingsContext.java index d5c9cc96..a59f2060 100644 --- a/src/main/java/io/cryostat/agent/remote/RecordingsContext.java +++ b/src/main/java/io/cryostat/agent/remote/RecordingsContext.java @@ -22,6 +22,9 @@ import java.nio.charset.StandardCharsets; import java.util.List; import java.util.Set; +import java.util.function.Consumer; +import java.util.regex.Matcher; +import java.util.regex.Pattern; import java.util.stream.Collectors; import javax.inject.Inject; @@ -50,12 +53,17 @@ import com.sun.net.httpserver.HttpExchange; import io.smallrye.config.SmallRyeConfig; import jdk.jfr.FlightRecorder; +import jdk.jfr.Recording; import org.apache.http.HttpStatus; import org.slf4j.Logger; import org.slf4j.LoggerFactory; class RecordingsContext implements RemoteContext { + private static final String PATH = "/recordings"; + private static final Pattern PATH_ID_PATTERN = + Pattern.compile("^" + PATH + "/(\\d+)$", Pattern.MULTILINE); + private final Logger log = LoggerFactory.getLogger(getClass()); private final SmallRyeConfig config; private final ObjectMapper mapper; @@ -76,7 +84,7 @@ class RecordingsContext implements RemoteContext { @Override public String path() { - return "/recordings"; + return PATH; } @Override @@ -86,43 +94,40 @@ public void handle(HttpExchange exchange) throws IOException { if (!ensureMethodAccepted(exchange)) { return; } + int id = Integer.MIN_VALUE; switch (mtd) { case "GET": - try (OutputStream response = exchange.getResponseBody()) { - List recordings = getRecordings(); - exchange.sendResponseHeaders(HttpStatus.SC_OK, 0); - mapper.writeValue(response, recordings); - } catch (Exception e) { - log.error("recordings serialization failure", e); + id = extractId(exchange); + if (id == Integer.MIN_VALUE) { + handleGetList(exchange); + } else { + exchange.sendResponseHeaders( + HttpStatus.SC_NOT_IMPLEMENTED, BODY_LENGTH_NONE); } break; case "POST": - try (InputStream body = exchange.getRequestBody()) { - StartRecordingRequest req = - mapper.readValue(body, StartRecordingRequest.class); - if (!req.isValid()) { - exchange.sendResponseHeaders(HttpStatus.SC_BAD_REQUEST, -1); - return; - } - SerializableRecordingDescriptor recording = startRecording(req); - exchange.sendResponseHeaders(HttpStatus.SC_CREATED, 0); - try (OutputStream response = exchange.getResponseBody()) { - mapper.writeValue(response, recording); - } - } catch (QuantityConversionException - | ServiceNotAvailableException - | FlightRecorderException - | org.openjdk.jmc.rjmx.services.jfr.FlightRecorderException - | InvalidEventTemplateException - | InvalidXmlException - | IOException e) { - log.error("Failed to start recording", e); - exchange.sendResponseHeaders(HttpStatus.SC_INTERNAL_SERVER_ERROR, -1); + handleStart(exchange); + break; + case "PATCH": + id = extractId(exchange); + if (id >= 0) { + handleStop(exchange, id); + } else { + exchange.sendResponseHeaders(HttpStatus.SC_BAD_REQUEST, BODY_LENGTH_NONE); + } + break; + case "DELETE": + id = extractId(exchange); + if (id >= 0) { + handleDelete(exchange, id); + } else { + exchange.sendResponseHeaders(HttpStatus.SC_BAD_REQUEST, BODY_LENGTH_NONE); } break; default: log.warn("Unknown request method {}", mtd); - exchange.sendResponseHeaders(HttpStatus.SC_METHOD_NOT_ALLOWED, -1); + exchange.sendResponseHeaders( + HttpStatus.SC_METHOD_NOT_ALLOWED, BODY_LENGTH_NONE); break; } } finally { @@ -130,16 +135,102 @@ public void handle(HttpExchange exchange) throws IOException { } } + private static int extractId(HttpExchange exchange) throws IOException { + Matcher m = PATH_ID_PATTERN.matcher(exchange.getRequestURI().getPath()); + if (!m.find()) { + return Integer.MIN_VALUE; + } + return Integer.parseInt(m.group(1)); + } + + private void handleGetList(HttpExchange exchange) { + try (OutputStream response = exchange.getResponseBody()) { + List recordings = getRecordings(); + exchange.sendResponseHeaders(HttpStatus.SC_OK, BODY_LENGTH_UNKNOWN); + mapper.writeValue(response, recordings); + } catch (Exception e) { + log.error("recordings serialization failure", e); + } + } + + private void handleStart(HttpExchange exchange) throws IOException { + try (InputStream body = exchange.getRequestBody()) { + StartRecordingRequest req = mapper.readValue(body, StartRecordingRequest.class); + if (!req.isValid()) { + exchange.sendResponseHeaders(HttpStatus.SC_BAD_REQUEST, BODY_LENGTH_NONE); + return; + } + SerializableRecordingDescriptor recording = startRecording(req); + exchange.sendResponseHeaders(HttpStatus.SC_CREATED, BODY_LENGTH_UNKNOWN); + try (OutputStream response = exchange.getResponseBody()) { + mapper.writeValue(response, recording); + } + } catch (QuantityConversionException + | ServiceNotAvailableException + | FlightRecorderException + | org.openjdk.jmc.rjmx.services.jfr.FlightRecorderException + | InvalidEventTemplateException + | InvalidXmlException + | IOException e) { + log.error("Failed to start recording", e); + exchange.sendResponseHeaders(HttpStatus.SC_INTERNAL_SERVER_ERROR, BODY_LENGTH_NONE); + } + } + + private void handleStop(HttpExchange exchange, int id) throws IOException { + invokeOnRecording( + exchange, + id, + r -> { + try { + boolean stopped = r.stop(); + if (!stopped) { + sendHeader(exchange, HttpStatus.SC_BAD_REQUEST); + } else { + sendHeader(exchange, HttpStatus.SC_NO_CONTENT); + } + } catch (IllegalStateException e) { + sendHeader(exchange, HttpStatus.SC_CONFLICT); + } + }); + } + + private void handleDelete(HttpExchange exchange, int id) throws IOException { + invokeOnRecording( + exchange, + id, + r -> { + r.close(); + sendHeader(exchange, HttpStatus.SC_NO_CONTENT); + }); + } + + private void invokeOnRecording(HttpExchange exchange, long id, Consumer consumer) { + FlightRecorder.getFlightRecorder().getRecordings().stream() + .filter(r -> r.getId() == id) + .findFirst() + .ifPresentOrElse( + consumer::accept, () -> sendHeader(exchange, HttpStatus.SC_NOT_FOUND)); + } + + private void sendHeader(HttpExchange exchange, int status) { + try { + exchange.sendResponseHeaders(status, BODY_LENGTH_NONE); + } catch (IOException e) { + throw new IllegalStateException(e); + } + } + private boolean ensureMethodAccepted(HttpExchange exchange) throws IOException { - Set blocked = Set.of("POST"); + Set alwaysAllowed = Set.of("GET"); String mtd = exchange.getRequestMethod(); - boolean restricted = blocked.contains(mtd); + boolean restricted = !alwaysAllowed.contains(mtd); if (!restricted) { return true; } boolean passed = restricted && MutatingRemoteContext.apiWritesEnabled(config); if (!passed) { - exchange.sendResponseHeaders(HttpStatus.SC_FORBIDDEN, -1); + exchange.sendResponseHeaders(HttpStatus.SC_FORBIDDEN, BODY_LENGTH_NONE); } return passed; } diff --git a/src/main/java/io/cryostat/agent/remote/RemoteContext.java b/src/main/java/io/cryostat/agent/remote/RemoteContext.java index 3b5a2271..2bcdd8f9 100644 --- a/src/main/java/io/cryostat/agent/remote/RemoteContext.java +++ b/src/main/java/io/cryostat/agent/remote/RemoteContext.java @@ -18,6 +18,10 @@ import com.sun.net.httpserver.HttpHandler; public interface RemoteContext extends HttpHandler { + + public static final int BODY_LENGTH_NONE = -1; + public static final int BODY_LENGTH_UNKNOWN = 0; + String path(); default boolean available() { From 86a2c02180a6671adeb88eae8e06ef8674dc5768 Mon Sep 17 00:00:00 2001 From: Andrew Azores Date: Fri, 4 Aug 2023 14:09:04 -0400 Subject: [PATCH 15/45] use longs for IDs --- .../io/cryostat/agent/remote/RecordingsContext.java | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/main/java/io/cryostat/agent/remote/RecordingsContext.java b/src/main/java/io/cryostat/agent/remote/RecordingsContext.java index a59f2060..532c8954 100644 --- a/src/main/java/io/cryostat/agent/remote/RecordingsContext.java +++ b/src/main/java/io/cryostat/agent/remote/RecordingsContext.java @@ -94,7 +94,7 @@ public void handle(HttpExchange exchange) throws IOException { if (!ensureMethodAccepted(exchange)) { return; } - int id = Integer.MIN_VALUE; + long id = Long.MIN_VALUE; switch (mtd) { case "GET": id = extractId(exchange); @@ -135,12 +135,12 @@ public void handle(HttpExchange exchange) throws IOException { } } - private static int extractId(HttpExchange exchange) throws IOException { + private static long extractId(HttpExchange exchange) throws IOException { Matcher m = PATH_ID_PATTERN.matcher(exchange.getRequestURI().getPath()); if (!m.find()) { - return Integer.MIN_VALUE; + return Long.MIN_VALUE; } - return Integer.parseInt(m.group(1)); + return Long.parseLong(m.group(1)); } private void handleGetList(HttpExchange exchange) { @@ -177,7 +177,7 @@ private void handleStart(HttpExchange exchange) throws IOException { } } - private void handleStop(HttpExchange exchange, int id) throws IOException { + private void handleStop(HttpExchange exchange, long id) throws IOException { invokeOnRecording( exchange, id, @@ -195,7 +195,7 @@ private void handleStop(HttpExchange exchange, int id) throws IOException { }); } - private void handleDelete(HttpExchange exchange, int id) throws IOException { + private void handleDelete(HttpExchange exchange, long id) throws IOException { invokeOnRecording( exchange, id, From abcb2b9ef683f081b6d85a6218cbc3099d22a052 Mon Sep 17 00:00:00 2001 From: Andrew Azores Date: Fri, 4 Aug 2023 15:01:59 -0400 Subject: [PATCH 16/45] feat(api): implement GET /recordings/:id for streaming files --- .../agent/remote/RecordingsContext.java | 47 +++++++++++++++++-- 1 file changed, 42 insertions(+), 5 deletions(-) diff --git a/src/main/java/io/cryostat/agent/remote/RecordingsContext.java b/src/main/java/io/cryostat/agent/remote/RecordingsContext.java index 532c8954..9127bbcc 100644 --- a/src/main/java/io/cryostat/agent/remote/RecordingsContext.java +++ b/src/main/java/io/cryostat/agent/remote/RecordingsContext.java @@ -15,6 +15,7 @@ */ package io.cryostat.agent.remote; +import java.io.BufferedInputStream; import java.io.ByteArrayInputStream; import java.io.IOException; import java.io.InputStream; @@ -60,9 +61,9 @@ class RecordingsContext implements RemoteContext { - private static final String PATH = "/recordings"; + private static final String PATH = "/recordings/"; private static final Pattern PATH_ID_PATTERN = - Pattern.compile("^" + PATH + "/(\\d+)$", Pattern.MULTILINE); + Pattern.compile("^" + PATH + "(\\d+)$", Pattern.MULTILINE); private final Logger log = LoggerFactory.getLogger(getClass()); private final SmallRyeConfig config; @@ -98,11 +99,10 @@ public void handle(HttpExchange exchange) throws IOException { switch (mtd) { case "GET": id = extractId(exchange); - if (id == Integer.MIN_VALUE) { + if (id == Long.MIN_VALUE) { handleGetList(exchange); } else { - exchange.sendResponseHeaders( - HttpStatus.SC_NOT_IMPLEMENTED, BODY_LENGTH_NONE); + handleGetRecording(exchange, id); } break; case "POST": @@ -153,6 +153,43 @@ private void handleGetList(HttpExchange exchange) { } } + private void handleGetRecording(HttpExchange exchange, long id) { + FlightRecorder.getFlightRecorder().getRecordings().stream() + .filter(r -> r.getId() == id) + .findFirst() + .ifPresentOrElse( + r -> { + Recording copy = r.copy(true); + try (InputStream stream = copy.getStream(null, null); + BufferedInputStream bis = new BufferedInputStream(stream); + OutputStream response = exchange.getResponseBody()) { + if (stream == null) { + exchange.sendResponseHeaders(HttpStatus.SC_NO_CONTENT, -1); + } else { + exchange.sendResponseHeaders(HttpStatus.SC_OK, 0); + bis.transferTo(response); + } + } catch (IOException ioe) { + log.error("I/O error", ioe); + try { + exchange.sendResponseHeaders( + HttpStatus.SC_INTERNAL_SERVER_ERROR, -1); + } catch (IOException ioe2) { + log.error("Failed to write response", ioe2); + } + } finally { + copy.close(); + } + }, + () -> { + try { + exchange.sendResponseHeaders(HttpStatus.SC_NOT_FOUND, -1); + } catch (IOException e) { + log.error("Failed to write response", e); + } + }); + } + private void handleStart(HttpExchange exchange) throws IOException { try (InputStream body = exchange.getRequestBody()) { StartRecordingRequest req = mapper.readValue(body, StartRecordingRequest.class); From 42ecb618e0031b756002b6106e8d51aee0143abb Mon Sep 17 00:00:00 2001 From: Andrew Azores Date: Mon, 14 Aug 2023 12:14:44 -0400 Subject: [PATCH 17/45] refactor to use constants --- .../io/cryostat/agent/remote/RecordingsContext.java | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/main/java/io/cryostat/agent/remote/RecordingsContext.java b/src/main/java/io/cryostat/agent/remote/RecordingsContext.java index 9127bbcc..fbd7b0ca 100644 --- a/src/main/java/io/cryostat/agent/remote/RecordingsContext.java +++ b/src/main/java/io/cryostat/agent/remote/RecordingsContext.java @@ -164,16 +164,18 @@ private void handleGetRecording(HttpExchange exchange, long id) { BufferedInputStream bis = new BufferedInputStream(stream); OutputStream response = exchange.getResponseBody()) { if (stream == null) { - exchange.sendResponseHeaders(HttpStatus.SC_NO_CONTENT, -1); + exchange.sendResponseHeaders( + HttpStatus.SC_NO_CONTENT, BODY_LENGTH_NONE); } else { - exchange.sendResponseHeaders(HttpStatus.SC_OK, 0); + exchange.sendResponseHeaders( + HttpStatus.SC_OK, BODY_LENGTH_UNKNOWN); bis.transferTo(response); } } catch (IOException ioe) { log.error("I/O error", ioe); try { exchange.sendResponseHeaders( - HttpStatus.SC_INTERNAL_SERVER_ERROR, -1); + HttpStatus.SC_INTERNAL_SERVER_ERROR, BODY_LENGTH_NONE); } catch (IOException ioe2) { log.error("Failed to write response", ioe2); } @@ -183,7 +185,8 @@ private void handleGetRecording(HttpExchange exchange, long id) { }, () -> { try { - exchange.sendResponseHeaders(HttpStatus.SC_NOT_FOUND, -1); + exchange.sendResponseHeaders( + HttpStatus.SC_NOT_FOUND, BODY_LENGTH_NONE); } catch (IOException e) { log.error("Failed to write response", e); } From 06a8cc10f24feba30c9db3ae31f1b31f29f1b428 Mon Sep 17 00:00:00 2001 From: Andrew Azores Date: Thu, 27 Jul 2023 16:10:08 -0400 Subject: [PATCH 18/45] feat(api): enable dynamic JFR start (#165) --- README.md | 1 + .../java/io/cryostat/agent/ConfigModule.java | 3 + .../java/io/cryostat/agent/MainModule.java | 73 +++++- .../java/io/cryostat/agent/WebServer.java | 63 +++-- .../agent/remote/EventTemplatesContext.java | 48 ++-- .../agent/remote/EventTypesContext.java | 35 +-- .../cryostat/agent/remote/MBeanContext.java | 38 +-- .../agent/remote/MutatingRemoteContext.java | 60 +++++ .../agent/remote/RecordingsContext.java | 217 +++++++++++++----- .../cryostat/agent/remote/RemoteContext.java | 4 + .../META-INF/microprofile-config.properties | 2 + 11 files changed, 401 insertions(+), 143 deletions(-) create mode 100644 src/main/java/io/cryostat/agent/remote/MutatingRemoteContext.java diff --git a/README.md b/README.md index 483f886f..97348324 100644 --- a/README.md +++ b/README.md @@ -34,6 +34,7 @@ and how it advertises itself to a Cryostat server instance. Required properties - [x] `cryostat.agent.baseuri` [`java.net.URI`]: the URL location of the Cryostat server backend that this agent advertises itself to. - [x] `cryostat.agent.callback` [`java.net.URI`]: a URL pointing back to this agent, ex. `"https://12.34.56.78:1234/"`. Cryostat will use this URL to perform health checks and request updates from the agent. This reflects the externally-visible IP address/hostname and port where this application and agent can be found. +- [ ] `cryostat.agent.api.writes-enabled` [`boolean`]: Control whether the agent accepts "write" or mutating operations on its HTTP API. Requests for remote operations such as dynamically starting Flight Recordings will be rejected unless this is set. Default `false`. - [ ] `cryostat.agent.instance-id` [`String`]: a unique ID for this agent instance. This will be used to uniquely identify the agent in the Cryostat discovery database, as well as to unambiguously match its encrypted stored credentials. The default is a random UUID string. It is not recommended to override this value. - [ ] `cryostat.agent.hostname` [`String`]: the hostname for this application instance. This will be used for the published JMX connection URL. If not provided then the default is to attempt to resolve the localhost hostname. - [ ] `cryostat.agent.realm` [`String`]: the Cryostat Discovery API "realm" that this agent belongs to. This should be unique per agent instance. The default is the value of `cryostat.agent.app.name`. diff --git a/src/main/java/io/cryostat/agent/ConfigModule.java b/src/main/java/io/cryostat/agent/ConfigModule.java index c1b1c9e3..a464439b 100644 --- a/src/main/java/io/cryostat/agent/ConfigModule.java +++ b/src/main/java/io/cryostat/agent/ConfigModule.java @@ -85,6 +85,9 @@ public abstract class ConfigModule { public static final String CRYOSTAT_AGENT_HARVESTER_MAX_SIZE_B = "cryostat.agent.harvester.max-size-b"; + public static final String CRYOSTAT_AGENT_API_WRITES_ENABLED = + "cryostat.agent.api.writes-enabled"; + @Provides @Singleton public static SmallRyeConfig provideConfig() { diff --git a/src/main/java/io/cryostat/agent/MainModule.java b/src/main/java/io/cryostat/agent/MainModule.java index e963ab66..d66e1280 100644 --- a/src/main/java/io/cryostat/agent/MainModule.java +++ b/src/main/java/io/cryostat/agent/MainModule.java @@ -15,7 +15,9 @@ */ package io.cryostat.agent; +import java.io.IOException; import java.net.URI; +import java.nio.file.Path; import java.security.KeyManagementException; import java.security.NoSuchAlgorithmException; import java.security.SecureRandom; @@ -39,6 +41,7 @@ import io.cryostat.core.net.JFRConnectionToolkit; import io.cryostat.core.sys.Environment; import io.cryostat.core.sys.FileSystem; +import io.cryostat.core.templates.LocalStorageTemplateService; import io.cryostat.core.tui.ClientWriter; import com.fasterxml.jackson.databind.ObjectMapper; @@ -63,6 +66,7 @@ public abstract class MainModule { // one for outbound HTTP requests, one for incoming HTTP requests, and one as a general worker private static final int NUM_WORKER_THREADS = 3; private static final String JVM_ID = "JVM_ID"; + private static final String TEMPLATES_PATH = "TEMPLATES_PATH"; @Provides @Singleton @@ -258,21 +262,61 @@ public static Harvester provideHarvester( registration); } + @Provides + @Singleton + public static FileSystem provideFileSystem() { + return new FileSystem(); + } + + @Provides + @Singleton + @Named(TEMPLATES_PATH) + public static Path provideTemplatesTmpPath(FileSystem fs) { + try { + return fs.createTempDirectory(null); + } catch (IOException e) { + throw new RuntimeException(e); + } + } + + @Provides + @Singleton + public static Environment provideEnvironment(@Named(TEMPLATES_PATH) Path templatesTmp) { + return new Environment() { + @Override + public String getEnv(String key) { + if (LocalStorageTemplateService.TEMPLATE_PATH.equals(key)) { + return templatesTmp.toString(); + } + return super.getEnv(key); + } + }; + } + + @Provides + @Singleton + public static ClientWriter provideClientWriter() { + Logger log = LoggerFactory.getLogger(JFRConnectionToolkit.class); + return new ClientWriter() { + @Override + public void print(String msg) { + log.info(msg); + } + }; + } + + @Provides + @Singleton + public static JFRConnectionToolkit provideJfrConnectionToolkit( + ClientWriter cw, FileSystem fs, Environment env) { + return new JFRConnectionToolkit(cw, fs, env); + } + @Provides @Singleton @Named(JVM_ID) - public static String provideJvmId() { + public static String provideJvmId(JFRConnectionToolkit tk) { Logger log = LoggerFactory.getLogger(JFRConnectionToolkit.class); - JFRConnectionToolkit tk = - new JFRConnectionToolkit( - new ClientWriter() { - @Override - public void print(String msg) { - log.warn(msg); - } - }, - new FileSystem(), - new Environment()); try { try (JFRConnection connection = tk.connect(tk.createServiceURL("localhost", 0))) { String id = connection.getJvmId(); @@ -283,4 +327,11 @@ public void print(String msg) { throw new RuntimeException(e); } } + + @Provides + @Singleton + public static LocalStorageTemplateService provideLocalStorageTemplateService( + FileSystem fs, Environment env) { + return new LocalStorageTemplateService(fs, env); + } } diff --git a/src/main/java/io/cryostat/agent/WebServer.java b/src/main/java/io/cryostat/agent/WebServer.java index 7c349f4d..1d2f78c6 100644 --- a/src/main/java/io/cryostat/agent/WebServer.java +++ b/src/main/java/io/cryostat/agent/WebServer.java @@ -40,6 +40,7 @@ import com.sun.net.httpserver.Filter; import com.sun.net.httpserver.HttpContext; import com.sun.net.httpserver.HttpExchange; +import com.sun.net.httpserver.HttpHandler; import com.sun.net.httpserver.HttpServer; import dagger.Lazy; import org.apache.http.HttpStatus; @@ -100,13 +101,15 @@ void start() throws IOException, NoSuchAlgorithmException { Set mergedContexts = new HashSet<>(remoteContexts.get()); mergedContexts.add(new PingContext()); - mergedContexts.forEach( - rc -> { - HttpContext ctx = this.http.createContext(rc.path(), rc::handle); - ctx.setAuthenticator(agentAuthenticator); - ctx.getFilters().add(requestLoggingFilter); - ctx.getFilters().add(compressionFilter); - }); + mergedContexts.stream() + .filter(RemoteContext::available) + .forEach( + rc -> { + HttpContext ctx = this.http.createContext(rc.path(), wrap(rc::handle)); + ctx.setAuthenticator(agentAuthenticator); + ctx.getFilters().add(requestLoggingFilter); + ctx.getFilters().add(compressionFilter); + }); this.http.start(); } @@ -154,6 +157,18 @@ CompletableFuture generateCredentials() throws NoSuchAlgorithmException { } } + private HttpHandler wrap(HttpHandler handler) { + return x -> { + try { + handler.handle(x); + } catch (Exception e) { + log.error("Unhandled exception", e); + x.sendResponseHeaders(HttpStatus.SC_INTERNAL_SERVER_ERROR, 0); + x.close(); + } + }; + } + private class PingContext implements RemoteContext { @Override @@ -163,23 +178,25 @@ public String path() { @Override public void handle(HttpExchange exchange) throws IOException { - String mtd = exchange.getRequestMethod(); - switch (mtd) { - case "POST": - synchronized (WebServer.this.credentials) { - executor.execute(registration.get()::tryRegister); + try { + String mtd = exchange.getRequestMethod(); + switch (mtd) { + case "POST": + synchronized (WebServer.this.credentials) { + executor.execute(registration.get()::tryRegister); + exchange.sendResponseHeaders(HttpStatus.SC_NO_CONTENT, -1); + } + break; + case "GET": exchange.sendResponseHeaders(HttpStatus.SC_NO_CONTENT, -1); - exchange.close(); - } - break; - case "GET": - exchange.sendResponseHeaders(HttpStatus.SC_NO_CONTENT, -1); - exchange.close(); - break; - default: - exchange.sendResponseHeaders(HttpStatus.SC_NOT_FOUND, -1); - exchange.close(); - break; + break; + default: + log.warn("Unknown request method {}", mtd); + exchange.sendResponseHeaders(HttpStatus.SC_METHOD_NOT_ALLOWED, -1); + break; + } + } finally { + exchange.close(); } } } diff --git a/src/main/java/io/cryostat/agent/remote/EventTemplatesContext.java b/src/main/java/io/cryostat/agent/remote/EventTemplatesContext.java index 03c0770d..fb5fca55 100644 --- a/src/main/java/io/cryostat/agent/remote/EventTemplatesContext.java +++ b/src/main/java/io/cryostat/agent/remote/EventTemplatesContext.java @@ -48,30 +48,32 @@ public String path() { @Override public void handle(HttpExchange exchange) throws IOException { - String mtd = exchange.getRequestMethod(); - switch (mtd) { - case "GET": - try { - exchange.sendResponseHeaders(HttpStatus.SC_OK, 0); - try (OutputStream response = exchange.getResponseBody()) { - FlightRecorderMXBean bean = - ManagementFactory.getPlatformMXBean(FlightRecorderMXBean.class); - List xmlTexts = - bean.getConfigurations().stream() - .map(ConfigurationInfo::getContents) - .collect(Collectors.toList()); - mapper.writeValue(response, xmlTexts); + try { + String mtd = exchange.getRequestMethod(); + switch (mtd) { + case "GET": + try { + exchange.sendResponseHeaders(HttpStatus.SC_OK, 0); + try (OutputStream response = exchange.getResponseBody()) { + FlightRecorderMXBean bean = + ManagementFactory.getPlatformMXBean(FlightRecorderMXBean.class); + List xmlTexts = + bean.getConfigurations().stream() + .map(ConfigurationInfo::getContents) + .collect(Collectors.toList()); + mapper.writeValue(response, xmlTexts); + } + } catch (Exception e) { + log.error("events serialization failure", e); } - } catch (Exception e) { - log.error("events serialization failure", e); - } finally { - exchange.close(); - } - break; - default: - exchange.sendResponseHeaders(HttpStatus.SC_NOT_FOUND, -1); - exchange.close(); - break; + break; + default: + log.warn("Unknown request method {}", mtd); + exchange.sendResponseHeaders(HttpStatus.SC_METHOD_NOT_ALLOWED, -1); + break; + } + } finally { + exchange.close(); } } } diff --git a/src/main/java/io/cryostat/agent/remote/EventTypesContext.java b/src/main/java/io/cryostat/agent/remote/EventTypesContext.java index 8efe03fb..0fc79763 100644 --- a/src/main/java/io/cryostat/agent/remote/EventTypesContext.java +++ b/src/main/java/io/cryostat/agent/remote/EventTypesContext.java @@ -50,25 +50,30 @@ public String path() { @Override public void handle(HttpExchange exchange) throws IOException { - String mtd = exchange.getRequestMethod(); - switch (mtd) { - case "GET": - try { - List events = getEventTypes(); + try { + String mtd = exchange.getRequestMethod(); + switch (mtd) { + case "GET": + List events = new ArrayList<>(); + try { + events.addAll(getEventTypes()); + } catch (Exception e) { + log.error("events serialization failure", e); + exchange.sendResponseHeaders(HttpStatus.SC_INTERNAL_SERVER_ERROR, 0); + break; + } exchange.sendResponseHeaders(HttpStatus.SC_OK, 0); try (OutputStream response = exchange.getResponseBody()) { mapper.writeValue(response, events); } - } catch (Exception e) { - log.error("events serialization failure", e); - } finally { - exchange.close(); - } - break; - default: - exchange.sendResponseHeaders(HttpStatus.SC_NOT_FOUND, -1); - exchange.close(); - break; + break; + default: + log.warn("Unknown request method {}", mtd); + exchange.sendResponseHeaders(HttpStatus.SC_METHOD_NOT_ALLOWED, -1); + break; + } + } finally { + exchange.close(); } } diff --git a/src/main/java/io/cryostat/agent/remote/MBeanContext.java b/src/main/java/io/cryostat/agent/remote/MBeanContext.java index 3b7bd1bd..c085df90 100644 --- a/src/main/java/io/cryostat/agent/remote/MBeanContext.java +++ b/src/main/java/io/cryostat/agent/remote/MBeanContext.java @@ -64,25 +64,27 @@ public String path() { @Override public void handle(HttpExchange exchange) throws IOException { - String mtd = exchange.getRequestMethod(); - switch (mtd) { - case "GET": - try { - MBeanMetrics metrics = getMBeanMetrics(); - exchange.sendResponseHeaders(HttpStatus.SC_OK, 0); - try (OutputStream response = exchange.getResponseBody()) { - mapper.writeValue(response, metrics); + try { + String mtd = exchange.getRequestMethod(); + switch (mtd) { + case "GET": + try { + MBeanMetrics metrics = getMBeanMetrics(); + exchange.sendResponseHeaders(HttpStatus.SC_OK, 0); + try (OutputStream response = exchange.getResponseBody()) { + mapper.writeValue(response, metrics); + } + } catch (Exception e) { + log.error("mbean serialization failure", e); } - } catch (Exception e) { - log.error("mbean serialization failure", e); - } finally { - exchange.close(); - } - break; - default: - exchange.sendResponseHeaders(HttpStatus.SC_NOT_FOUND, -1); - exchange.close(); - break; + break; + default: + log.warn("Unknown request method {}", mtd); + exchange.sendResponseHeaders(HttpStatus.SC_METHOD_NOT_ALLOWED, -1); + break; + } + } finally { + exchange.close(); } } diff --git a/src/main/java/io/cryostat/agent/remote/MutatingRemoteContext.java b/src/main/java/io/cryostat/agent/remote/MutatingRemoteContext.java new file mode 100644 index 00000000..89f65c67 --- /dev/null +++ b/src/main/java/io/cryostat/agent/remote/MutatingRemoteContext.java @@ -0,0 +1,60 @@ +/* + * Copyright The Cryostat Authors + * + * The Universal Permissive License (UPL), Version 1.0 + * + * Subject to the condition set forth below, permission is hereby granted to any + * person obtaining a copy of this software, associated documentation and/or data + * (collectively the "Software"), free of charge and under any and all copyright + * rights in the Software, and any and all patent rights owned or freely + * licensable by each licensor hereunder covering either (i) the unmodified + * Software as contributed to or provided by such licensor, or (ii) the Larger + * Works (as defined below), to deal in both + * + * (a) the Software, and + * (b) any piece of software and/or hardware listed in the lrgrwrks.txt file if + * one is included with the Software (each a "Larger Work" to which the Software + * is contributed by such licensors), + * + * without restriction, including without limitation the rights to copy, create + * derivative works of, display, perform, and distribute the Software and make, + * use, sell, offer for sale, import, export, have made, and have sold the + * Software and the Larger Work(s), and to sublicense the foregoing rights on + * either these or other terms. + * + * This license is subject to the following condition: + * The above copyright notice and either this complete permission notice or at + * a minimum a reference to the UPL must be included in all copies or + * substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + */ +package io.cryostat.agent.remote; + +import io.cryostat.agent.ConfigModule; + +import io.smallrye.config.SmallRyeConfig; + +public abstract class MutatingRemoteContext implements RemoteContext { + + protected final SmallRyeConfig config; + + protected MutatingRemoteContext(SmallRyeConfig config) { + this.config = config; + } + + @Override + public boolean available() { + return apiWritesEnabled(config); + } + + public static boolean apiWritesEnabled(SmallRyeConfig config) { + return config.getValue(ConfigModule.CRYOSTAT_AGENT_API_WRITES_ENABLED, boolean.class); + } +} diff --git a/src/main/java/io/cryostat/agent/remote/RecordingsContext.java b/src/main/java/io/cryostat/agent/remote/RecordingsContext.java index 0a605cbd..d5c9cc96 100644 --- a/src/main/java/io/cryostat/agent/remote/RecordingsContext.java +++ b/src/main/java/io/cryostat/agent/remote/RecordingsContext.java @@ -15,19 +15,41 @@ */ package io.cryostat.agent.remote; +import java.io.ByteArrayInputStream; import java.io.IOException; +import java.io.InputStream; import java.io.OutputStream; +import java.nio.charset.StandardCharsets; import java.util.List; -import java.util.Map; +import java.util.Set; import java.util.stream.Collectors; import javax.inject.Inject; +import org.openjdk.jmc.common.unit.IConstrainedMap; +import org.openjdk.jmc.common.unit.QuantityConversionException; +import org.openjdk.jmc.common.unit.UnitLookup; +import org.openjdk.jmc.flightrecorder.configuration.events.EventOptionID; +import org.openjdk.jmc.flightrecorder.configuration.recording.RecordingOptionsBuilder; +import org.openjdk.jmc.rjmx.ServiceNotAvailableException; +import org.openjdk.jmc.rjmx.services.jfr.IFlightRecorderService; + +import io.cryostat.agent.StringUtils; +import io.cryostat.core.FlightRecorderException; +import io.cryostat.core.net.JFRConnection; +import io.cryostat.core.net.JFRConnectionToolkit; +import io.cryostat.core.serialization.SerializableRecordingDescriptor; +import io.cryostat.core.templates.LocalStorageTemplateService; +import io.cryostat.core.templates.MutableTemplateService.InvalidEventTemplateException; +import io.cryostat.core.templates.MutableTemplateService.InvalidXmlException; +import io.cryostat.core.templates.RemoteTemplateService; +import io.cryostat.core.templates.Template; +import io.cryostat.core.templates.TemplateType; + import com.fasterxml.jackson.databind.ObjectMapper; import com.sun.net.httpserver.HttpExchange; -import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; +import io.smallrye.config.SmallRyeConfig; import jdk.jfr.FlightRecorder; -import jdk.jfr.Recording; import org.apache.http.HttpStatus; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -35,11 +57,21 @@ class RecordingsContext implements RemoteContext { private final Logger log = LoggerFactory.getLogger(getClass()); + private final SmallRyeConfig config; private final ObjectMapper mapper; + private final JFRConnectionToolkit jfrConnectionToolkit; + private final LocalStorageTemplateService localStorageTemplateService; @Inject - RecordingsContext(ObjectMapper mapper) { + RecordingsContext( + SmallRyeConfig config, + ObjectMapper mapper, + JFRConnectionToolkit jfrConnectionToolkit, + LocalStorageTemplateService localStorageTemplateService) { + this.config = config; this.mapper = mapper; + this.jfrConnectionToolkit = jfrConnectionToolkit; + this.localStorageTemplateService = localStorageTemplateService; } @Override @@ -49,67 +81,146 @@ public String path() { @Override public void handle(HttpExchange exchange) throws IOException { - String mtd = exchange.getRequestMethod(); - switch (mtd) { - case "GET": - try { - List recordings = getRecordings(); - exchange.sendResponseHeaders(HttpStatus.SC_OK, 0); + try { + String mtd = exchange.getRequestMethod(); + if (!ensureMethodAccepted(exchange)) { + return; + } + switch (mtd) { + case "GET": try (OutputStream response = exchange.getResponseBody()) { + List recordings = getRecordings(); + exchange.sendResponseHeaders(HttpStatus.SC_OK, 0); mapper.writeValue(response, recordings); + } catch (Exception e) { + log.error("recordings serialization failure", e); + } + break; + case "POST": + try (InputStream body = exchange.getRequestBody()) { + StartRecordingRequest req = + mapper.readValue(body, StartRecordingRequest.class); + if (!req.isValid()) { + exchange.sendResponseHeaders(HttpStatus.SC_BAD_REQUEST, -1); + return; + } + SerializableRecordingDescriptor recording = startRecording(req); + exchange.sendResponseHeaders(HttpStatus.SC_CREATED, 0); + try (OutputStream response = exchange.getResponseBody()) { + mapper.writeValue(response, recording); + } + } catch (QuantityConversionException + | ServiceNotAvailableException + | FlightRecorderException + | org.openjdk.jmc.rjmx.services.jfr.FlightRecorderException + | InvalidEventTemplateException + | InvalidXmlException + | IOException e) { + log.error("Failed to start recording", e); + exchange.sendResponseHeaders(HttpStatus.SC_INTERNAL_SERVER_ERROR, -1); } - } catch (Exception e) { - log.error("recordings serialization failure", e); - } finally { - exchange.close(); - } - break; - default: - exchange.sendResponseHeaders(HttpStatus.SC_NOT_FOUND, -1); - exchange.close(); - break; + break; + default: + log.warn("Unknown request method {}", mtd); + exchange.sendResponseHeaders(HttpStatus.SC_METHOD_NOT_ALLOWED, -1); + break; + } + } finally { + exchange.close(); + } + } + + private boolean ensureMethodAccepted(HttpExchange exchange) throws IOException { + Set blocked = Set.of("POST"); + String mtd = exchange.getRequestMethod(); + boolean restricted = blocked.contains(mtd); + if (!restricted) { + return true; + } + boolean passed = restricted && MutatingRemoteContext.apiWritesEnabled(config); + if (!passed) { + exchange.sendResponseHeaders(HttpStatus.SC_FORBIDDEN, -1); } + return passed; } - private List getRecordings() { + private List getRecordings() { return FlightRecorder.getFlightRecorder().getRecordings().stream() - .map(RecordingInfo::new) + .map(SerializableRecordingDescriptor::new) .collect(Collectors.toList()); } - @SuppressFBWarnings(value = "URF_UNREAD_FIELD") - private static class RecordingInfo { - - public final long id; - public final String name; - public final String state; - public final Map options; - public final long startTime; - public final long duration; - public final boolean isContinuous; - public final boolean toDisk; - public final long maxSize; - public final long maxAge; - - RecordingInfo(Recording rec) { - this.id = rec.getId(); - this.name = rec.getName(); - this.state = rec.getState().name(); - this.options = rec.getSettings(); - if (rec.getStartTime() != null) { - this.startTime = rec.getStartTime().toEpochMilli(); + private SerializableRecordingDescriptor startRecording(StartRecordingRequest req) + throws QuantityConversionException, ServiceNotAvailableException, + FlightRecorderException, + org.openjdk.jmc.rjmx.services.jfr.FlightRecorderException, + InvalidEventTemplateException, InvalidXmlException, IOException { + Runnable cleanup = () -> {}; + try { + JFRConnection conn = + jfrConnectionToolkit.connect( + jfrConnectionToolkit.createServiceURL("localhost", 0)); + IConstrainedMap events; + if (req.requestsCustomTemplate()) { + Template template = + localStorageTemplateService.addTemplate( + new ByteArrayInputStream( + req.template.getBytes(StandardCharsets.UTF_8))); + events = localStorageTemplateService.getEvents(template).orElseThrow(); + cleanup = + () -> { + try { + localStorageTemplateService.deleteTemplate(template); + } catch (InvalidEventTemplateException | IOException e) { + log.error("Failed to clean up template " + template.getName(), e); + } + }; } else { - this.startTime = 0; - } - this.isContinuous = rec.getDuration() == null; - this.duration = this.isContinuous ? 0 : rec.getDuration().toMillis(); - this.toDisk = rec.isToDisk(); - this.maxSize = rec.getMaxSize(); - if (rec.getMaxAge() != null) { - this.maxAge = rec.getMaxAge().toMillis(); - } else { - this.maxAge = 0; + events = + new RemoteTemplateService(conn) + .getEvents(req.localTemplateName, TemplateType.TARGET).stream() + .findFirst() + .orElseThrow(); } + IFlightRecorderService svc = conn.getService(); + return new SerializableRecordingDescriptor( + svc.start( + new RecordingOptionsBuilder(conn.getService()) + .name(req.name) + .duration(UnitLookup.MILLISECOND.quantity(req.duration)) + .maxSize(UnitLookup.BYTE.quantity(req.maxSize)) + .maxAge(UnitLookup.MILLISECOND.quantity(req.maxAge)) + .toDisk(true) + .build(), + events)); + } finally { + cleanup.run(); + } + } + + static class StartRecordingRequest { + + public String name; + public String localTemplateName; + public String template; + public long duration; + public long maxSize; + public long maxAge; + + boolean requestsCustomTemplate() { + return !StringUtils.isBlank(template); + } + + boolean requestsBundledTemplate() { + return !StringUtils.isBlank(localTemplateName); + } + + boolean isValid() { + boolean requestsCustomTemplate = requestsCustomTemplate(); + boolean requestsBundledTemplate = requestsBundledTemplate(); + boolean requestsEither = requestsCustomTemplate || requestsBundledTemplate; + boolean requestsBoth = requestsCustomTemplate && requestsBundledTemplate; + return requestsEither && !requestsBoth; } } } diff --git a/src/main/java/io/cryostat/agent/remote/RemoteContext.java b/src/main/java/io/cryostat/agent/remote/RemoteContext.java index 5290cd39..3b5a2271 100644 --- a/src/main/java/io/cryostat/agent/remote/RemoteContext.java +++ b/src/main/java/io/cryostat/agent/remote/RemoteContext.java @@ -19,4 +19,8 @@ public interface RemoteContext extends HttpHandler { String path(); + + default boolean available() { + return true; + } } diff --git a/src/main/resources/META-INF/microprofile-config.properties b/src/main/resources/META-INF/microprofile-config.properties index c1eac25e..3af7416f 100644 --- a/src/main/resources/META-INF/microprofile-config.properties +++ b/src/main/resources/META-INF/microprofile-config.properties @@ -1,6 +1,8 @@ cryostat.agent.app.name=cryostat-agent cryostat.agent.baseuri= +cryostat.agent.api.writes-enabled=false + cryostat.agent.webclient.ssl.trust-all=false cryostat.agent.webclient.ssl.verify-hostname=true cryostat.agent.webclient.connect.timeout-ms=1000 From 80399070d355dab51b7c5c69ea5c38363d85afbc Mon Sep 17 00:00:00 2001 From: Andrew Azores Date: Fri, 4 Aug 2023 15:08:33 -0400 Subject: [PATCH 19/45] fix license header --- .../agent/remote/MutatingRemoteContext.java | 42 +++++-------------- 1 file changed, 10 insertions(+), 32 deletions(-) diff --git a/src/main/java/io/cryostat/agent/remote/MutatingRemoteContext.java b/src/main/java/io/cryostat/agent/remote/MutatingRemoteContext.java index 89f65c67..753a2f52 100644 --- a/src/main/java/io/cryostat/agent/remote/MutatingRemoteContext.java +++ b/src/main/java/io/cryostat/agent/remote/MutatingRemoteContext.java @@ -1,39 +1,17 @@ /* - * Copyright The Cryostat Authors + * Copyright The Cryostat Authors. * - * The Universal Permissive License (UPL), Version 1.0 + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at * - * Subject to the condition set forth below, permission is hereby granted to any - * person obtaining a copy of this software, associated documentation and/or data - * (collectively the "Software"), free of charge and under any and all copyright - * rights in the Software, and any and all patent rights owned or freely - * licensable by each licensor hereunder covering either (i) the unmodified - * Software as contributed to or provided by such licensor, or (ii) the Larger - * Works (as defined below), to deal in both + * http://www.apache.org/licenses/LICENSE-2.0 * - * (a) the Software, and - * (b) any piece of software and/or hardware listed in the lrgrwrks.txt file if - * one is included with the Software (each a "Larger Work" to which the Software - * is contributed by such licensors), - * - * without restriction, including without limitation the rights to copy, create - * derivative works of, display, perform, and distribute the Software and make, - * use, sell, offer for sale, import, export, have made, and have sold the - * Software and the Larger Work(s), and to sublicense the foregoing rights on - * either these or other terms. - * - * This license is subject to the following condition: - * The above copyright notice and either this complete permission notice or at - * a minimum a reference to the UPL must be included in all copies or - * substantial portions of the Software. - * - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, - * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE - * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER - * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, - * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE - * SOFTWARE. + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. */ package io.cryostat.agent.remote; From 82002e87639228838418f4d2e9ba9af462417fe3 Mon Sep 17 00:00:00 2001 From: Andrew Azores Date: Mon, 14 Aug 2023 11:35:24 -0400 Subject: [PATCH 20/45] feat(api): enable dynamic JFR stop, delete (#176) * end paths with / so as to not match by prefix * refactor: extract methods * add utility for extracting ID from path * add handling for stopping recording * add handling for closing (deleting) recording * only allow GET requests if write-operations are not enabled * extract HTTP response body length constants --- .../java/io/cryostat/agent/WebServer.java | 12 +- .../agent/remote/EventTemplatesContext.java | 7 +- .../agent/remote/EventTypesContext.java | 10 +- .../cryostat/agent/remote/MBeanContext.java | 7 +- .../agent/remote/RecordingsContext.java | 155 ++++++++++++++---- .../cryostat/agent/remote/RemoteContext.java | 4 + 6 files changed, 149 insertions(+), 46 deletions(-) diff --git a/src/main/java/io/cryostat/agent/WebServer.java b/src/main/java/io/cryostat/agent/WebServer.java index 1d2f78c6..bf48374c 100644 --- a/src/main/java/io/cryostat/agent/WebServer.java +++ b/src/main/java/io/cryostat/agent/WebServer.java @@ -163,7 +163,8 @@ private HttpHandler wrap(HttpHandler handler) { handler.handle(x); } catch (Exception e) { log.error("Unhandled exception", e); - x.sendResponseHeaders(HttpStatus.SC_INTERNAL_SERVER_ERROR, 0); + x.sendResponseHeaders( + HttpStatus.SC_INTERNAL_SERVER_ERROR, RemoteContext.BODY_LENGTH_NONE); x.close(); } }; @@ -184,15 +185,18 @@ public void handle(HttpExchange exchange) throws IOException { case "POST": synchronized (WebServer.this.credentials) { executor.execute(registration.get()::tryRegister); - exchange.sendResponseHeaders(HttpStatus.SC_NO_CONTENT, -1); + exchange.sendResponseHeaders( + HttpStatus.SC_NO_CONTENT, RemoteContext.BODY_LENGTH_NONE); } break; case "GET": - exchange.sendResponseHeaders(HttpStatus.SC_NO_CONTENT, -1); + exchange.sendResponseHeaders( + HttpStatus.SC_NO_CONTENT, RemoteContext.BODY_LENGTH_NONE); break; default: log.warn("Unknown request method {}", mtd); - exchange.sendResponseHeaders(HttpStatus.SC_METHOD_NOT_ALLOWED, -1); + exchange.sendResponseHeaders( + HttpStatus.SC_METHOD_NOT_ALLOWED, RemoteContext.BODY_LENGTH_NONE); break; } } finally { diff --git a/src/main/java/io/cryostat/agent/remote/EventTemplatesContext.java b/src/main/java/io/cryostat/agent/remote/EventTemplatesContext.java index fb5fca55..53aa3bed 100644 --- a/src/main/java/io/cryostat/agent/remote/EventTemplatesContext.java +++ b/src/main/java/io/cryostat/agent/remote/EventTemplatesContext.java @@ -43,7 +43,7 @@ class EventTemplatesContext implements RemoteContext { @Override public String path() { - return "/event-templates"; + return "/event-templates/"; } @Override @@ -53,7 +53,7 @@ public void handle(HttpExchange exchange) throws IOException { switch (mtd) { case "GET": try { - exchange.sendResponseHeaders(HttpStatus.SC_OK, 0); + exchange.sendResponseHeaders(HttpStatus.SC_OK, BODY_LENGTH_UNKNOWN); try (OutputStream response = exchange.getResponseBody()) { FlightRecorderMXBean bean = ManagementFactory.getPlatformMXBean(FlightRecorderMXBean.class); @@ -69,7 +69,8 @@ public void handle(HttpExchange exchange) throws IOException { break; default: log.warn("Unknown request method {}", mtd); - exchange.sendResponseHeaders(HttpStatus.SC_METHOD_NOT_ALLOWED, -1); + exchange.sendResponseHeaders( + HttpStatus.SC_METHOD_NOT_ALLOWED, BODY_LENGTH_NONE); break; } } finally { diff --git a/src/main/java/io/cryostat/agent/remote/EventTypesContext.java b/src/main/java/io/cryostat/agent/remote/EventTypesContext.java index 0fc79763..620f5315 100644 --- a/src/main/java/io/cryostat/agent/remote/EventTypesContext.java +++ b/src/main/java/io/cryostat/agent/remote/EventTypesContext.java @@ -45,7 +45,7 @@ class EventTypesContext implements RemoteContext { @Override public String path() { - return "/event-types"; + return "/event-types/"; } @Override @@ -59,17 +59,19 @@ public void handle(HttpExchange exchange) throws IOException { events.addAll(getEventTypes()); } catch (Exception e) { log.error("events serialization failure", e); - exchange.sendResponseHeaders(HttpStatus.SC_INTERNAL_SERVER_ERROR, 0); + exchange.sendResponseHeaders( + HttpStatus.SC_INTERNAL_SERVER_ERROR, BODY_LENGTH_NONE); break; } - exchange.sendResponseHeaders(HttpStatus.SC_OK, 0); + exchange.sendResponseHeaders(HttpStatus.SC_OK, BODY_LENGTH_UNKNOWN); try (OutputStream response = exchange.getResponseBody()) { mapper.writeValue(response, events); } break; default: log.warn("Unknown request method {}", mtd); - exchange.sendResponseHeaders(HttpStatus.SC_METHOD_NOT_ALLOWED, -1); + exchange.sendResponseHeaders( + HttpStatus.SC_METHOD_NOT_ALLOWED, BODY_LENGTH_NONE); break; } } finally { diff --git a/src/main/java/io/cryostat/agent/remote/MBeanContext.java b/src/main/java/io/cryostat/agent/remote/MBeanContext.java index c085df90..26d466e9 100644 --- a/src/main/java/io/cryostat/agent/remote/MBeanContext.java +++ b/src/main/java/io/cryostat/agent/remote/MBeanContext.java @@ -59,7 +59,7 @@ class MBeanContext implements RemoteContext { @Override public String path() { - return "/mbean-metrics"; + return "/mbean-metrics/"; } @Override @@ -70,7 +70,7 @@ public void handle(HttpExchange exchange) throws IOException { case "GET": try { MBeanMetrics metrics = getMBeanMetrics(); - exchange.sendResponseHeaders(HttpStatus.SC_OK, 0); + exchange.sendResponseHeaders(HttpStatus.SC_OK, BODY_LENGTH_UNKNOWN); try (OutputStream response = exchange.getResponseBody()) { mapper.writeValue(response, metrics); } @@ -80,7 +80,8 @@ public void handle(HttpExchange exchange) throws IOException { break; default: log.warn("Unknown request method {}", mtd); - exchange.sendResponseHeaders(HttpStatus.SC_METHOD_NOT_ALLOWED, -1); + exchange.sendResponseHeaders( + HttpStatus.SC_METHOD_NOT_ALLOWED, BODY_LENGTH_NONE); break; } } finally { diff --git a/src/main/java/io/cryostat/agent/remote/RecordingsContext.java b/src/main/java/io/cryostat/agent/remote/RecordingsContext.java index d5c9cc96..a59f2060 100644 --- a/src/main/java/io/cryostat/agent/remote/RecordingsContext.java +++ b/src/main/java/io/cryostat/agent/remote/RecordingsContext.java @@ -22,6 +22,9 @@ import java.nio.charset.StandardCharsets; import java.util.List; import java.util.Set; +import java.util.function.Consumer; +import java.util.regex.Matcher; +import java.util.regex.Pattern; import java.util.stream.Collectors; import javax.inject.Inject; @@ -50,12 +53,17 @@ import com.sun.net.httpserver.HttpExchange; import io.smallrye.config.SmallRyeConfig; import jdk.jfr.FlightRecorder; +import jdk.jfr.Recording; import org.apache.http.HttpStatus; import org.slf4j.Logger; import org.slf4j.LoggerFactory; class RecordingsContext implements RemoteContext { + private static final String PATH = "/recordings"; + private static final Pattern PATH_ID_PATTERN = + Pattern.compile("^" + PATH + "/(\\d+)$", Pattern.MULTILINE); + private final Logger log = LoggerFactory.getLogger(getClass()); private final SmallRyeConfig config; private final ObjectMapper mapper; @@ -76,7 +84,7 @@ class RecordingsContext implements RemoteContext { @Override public String path() { - return "/recordings"; + return PATH; } @Override @@ -86,43 +94,40 @@ public void handle(HttpExchange exchange) throws IOException { if (!ensureMethodAccepted(exchange)) { return; } + int id = Integer.MIN_VALUE; switch (mtd) { case "GET": - try (OutputStream response = exchange.getResponseBody()) { - List recordings = getRecordings(); - exchange.sendResponseHeaders(HttpStatus.SC_OK, 0); - mapper.writeValue(response, recordings); - } catch (Exception e) { - log.error("recordings serialization failure", e); + id = extractId(exchange); + if (id == Integer.MIN_VALUE) { + handleGetList(exchange); + } else { + exchange.sendResponseHeaders( + HttpStatus.SC_NOT_IMPLEMENTED, BODY_LENGTH_NONE); } break; case "POST": - try (InputStream body = exchange.getRequestBody()) { - StartRecordingRequest req = - mapper.readValue(body, StartRecordingRequest.class); - if (!req.isValid()) { - exchange.sendResponseHeaders(HttpStatus.SC_BAD_REQUEST, -1); - return; - } - SerializableRecordingDescriptor recording = startRecording(req); - exchange.sendResponseHeaders(HttpStatus.SC_CREATED, 0); - try (OutputStream response = exchange.getResponseBody()) { - mapper.writeValue(response, recording); - } - } catch (QuantityConversionException - | ServiceNotAvailableException - | FlightRecorderException - | org.openjdk.jmc.rjmx.services.jfr.FlightRecorderException - | InvalidEventTemplateException - | InvalidXmlException - | IOException e) { - log.error("Failed to start recording", e); - exchange.sendResponseHeaders(HttpStatus.SC_INTERNAL_SERVER_ERROR, -1); + handleStart(exchange); + break; + case "PATCH": + id = extractId(exchange); + if (id >= 0) { + handleStop(exchange, id); + } else { + exchange.sendResponseHeaders(HttpStatus.SC_BAD_REQUEST, BODY_LENGTH_NONE); + } + break; + case "DELETE": + id = extractId(exchange); + if (id >= 0) { + handleDelete(exchange, id); + } else { + exchange.sendResponseHeaders(HttpStatus.SC_BAD_REQUEST, BODY_LENGTH_NONE); } break; default: log.warn("Unknown request method {}", mtd); - exchange.sendResponseHeaders(HttpStatus.SC_METHOD_NOT_ALLOWED, -1); + exchange.sendResponseHeaders( + HttpStatus.SC_METHOD_NOT_ALLOWED, BODY_LENGTH_NONE); break; } } finally { @@ -130,16 +135,102 @@ public void handle(HttpExchange exchange) throws IOException { } } + private static int extractId(HttpExchange exchange) throws IOException { + Matcher m = PATH_ID_PATTERN.matcher(exchange.getRequestURI().getPath()); + if (!m.find()) { + return Integer.MIN_VALUE; + } + return Integer.parseInt(m.group(1)); + } + + private void handleGetList(HttpExchange exchange) { + try (OutputStream response = exchange.getResponseBody()) { + List recordings = getRecordings(); + exchange.sendResponseHeaders(HttpStatus.SC_OK, BODY_LENGTH_UNKNOWN); + mapper.writeValue(response, recordings); + } catch (Exception e) { + log.error("recordings serialization failure", e); + } + } + + private void handleStart(HttpExchange exchange) throws IOException { + try (InputStream body = exchange.getRequestBody()) { + StartRecordingRequest req = mapper.readValue(body, StartRecordingRequest.class); + if (!req.isValid()) { + exchange.sendResponseHeaders(HttpStatus.SC_BAD_REQUEST, BODY_LENGTH_NONE); + return; + } + SerializableRecordingDescriptor recording = startRecording(req); + exchange.sendResponseHeaders(HttpStatus.SC_CREATED, BODY_LENGTH_UNKNOWN); + try (OutputStream response = exchange.getResponseBody()) { + mapper.writeValue(response, recording); + } + } catch (QuantityConversionException + | ServiceNotAvailableException + | FlightRecorderException + | org.openjdk.jmc.rjmx.services.jfr.FlightRecorderException + | InvalidEventTemplateException + | InvalidXmlException + | IOException e) { + log.error("Failed to start recording", e); + exchange.sendResponseHeaders(HttpStatus.SC_INTERNAL_SERVER_ERROR, BODY_LENGTH_NONE); + } + } + + private void handleStop(HttpExchange exchange, int id) throws IOException { + invokeOnRecording( + exchange, + id, + r -> { + try { + boolean stopped = r.stop(); + if (!stopped) { + sendHeader(exchange, HttpStatus.SC_BAD_REQUEST); + } else { + sendHeader(exchange, HttpStatus.SC_NO_CONTENT); + } + } catch (IllegalStateException e) { + sendHeader(exchange, HttpStatus.SC_CONFLICT); + } + }); + } + + private void handleDelete(HttpExchange exchange, int id) throws IOException { + invokeOnRecording( + exchange, + id, + r -> { + r.close(); + sendHeader(exchange, HttpStatus.SC_NO_CONTENT); + }); + } + + private void invokeOnRecording(HttpExchange exchange, long id, Consumer consumer) { + FlightRecorder.getFlightRecorder().getRecordings().stream() + .filter(r -> r.getId() == id) + .findFirst() + .ifPresentOrElse( + consumer::accept, () -> sendHeader(exchange, HttpStatus.SC_NOT_FOUND)); + } + + private void sendHeader(HttpExchange exchange, int status) { + try { + exchange.sendResponseHeaders(status, BODY_LENGTH_NONE); + } catch (IOException e) { + throw new IllegalStateException(e); + } + } + private boolean ensureMethodAccepted(HttpExchange exchange) throws IOException { - Set blocked = Set.of("POST"); + Set alwaysAllowed = Set.of("GET"); String mtd = exchange.getRequestMethod(); - boolean restricted = blocked.contains(mtd); + boolean restricted = !alwaysAllowed.contains(mtd); if (!restricted) { return true; } boolean passed = restricted && MutatingRemoteContext.apiWritesEnabled(config); if (!passed) { - exchange.sendResponseHeaders(HttpStatus.SC_FORBIDDEN, -1); + exchange.sendResponseHeaders(HttpStatus.SC_FORBIDDEN, BODY_LENGTH_NONE); } return passed; } diff --git a/src/main/java/io/cryostat/agent/remote/RemoteContext.java b/src/main/java/io/cryostat/agent/remote/RemoteContext.java index 3b5a2271..2bcdd8f9 100644 --- a/src/main/java/io/cryostat/agent/remote/RemoteContext.java +++ b/src/main/java/io/cryostat/agent/remote/RemoteContext.java @@ -18,6 +18,10 @@ import com.sun.net.httpserver.HttpHandler; public interface RemoteContext extends HttpHandler { + + public static final int BODY_LENGTH_NONE = -1; + public static final int BODY_LENGTH_UNKNOWN = 0; + String path(); default boolean available() { From 05ead99d99f6f940977d8c434476c1a0a3d15cc0 Mon Sep 17 00:00:00 2001 From: Andrew Azores Date: Mon, 14 Aug 2023 15:27:00 -0400 Subject: [PATCH 21/45] feat(api): implement GET /recordings/:id for streaming files --- .../agent/remote/RecordingsContext.java | 62 +++++++++++++++---- 1 file changed, 51 insertions(+), 11 deletions(-) diff --git a/src/main/java/io/cryostat/agent/remote/RecordingsContext.java b/src/main/java/io/cryostat/agent/remote/RecordingsContext.java index a59f2060..fbd7b0ca 100644 --- a/src/main/java/io/cryostat/agent/remote/RecordingsContext.java +++ b/src/main/java/io/cryostat/agent/remote/RecordingsContext.java @@ -15,6 +15,7 @@ */ package io.cryostat.agent.remote; +import java.io.BufferedInputStream; import java.io.ByteArrayInputStream; import java.io.IOException; import java.io.InputStream; @@ -60,9 +61,9 @@ class RecordingsContext implements RemoteContext { - private static final String PATH = "/recordings"; + private static final String PATH = "/recordings/"; private static final Pattern PATH_ID_PATTERN = - Pattern.compile("^" + PATH + "/(\\d+)$", Pattern.MULTILINE); + Pattern.compile("^" + PATH + "(\\d+)$", Pattern.MULTILINE); private final Logger log = LoggerFactory.getLogger(getClass()); private final SmallRyeConfig config; @@ -94,15 +95,14 @@ public void handle(HttpExchange exchange) throws IOException { if (!ensureMethodAccepted(exchange)) { return; } - int id = Integer.MIN_VALUE; + long id = Long.MIN_VALUE; switch (mtd) { case "GET": id = extractId(exchange); - if (id == Integer.MIN_VALUE) { + if (id == Long.MIN_VALUE) { handleGetList(exchange); } else { - exchange.sendResponseHeaders( - HttpStatus.SC_NOT_IMPLEMENTED, BODY_LENGTH_NONE); + handleGetRecording(exchange, id); } break; case "POST": @@ -135,12 +135,12 @@ public void handle(HttpExchange exchange) throws IOException { } } - private static int extractId(HttpExchange exchange) throws IOException { + private static long extractId(HttpExchange exchange) throws IOException { Matcher m = PATH_ID_PATTERN.matcher(exchange.getRequestURI().getPath()); if (!m.find()) { - return Integer.MIN_VALUE; + return Long.MIN_VALUE; } - return Integer.parseInt(m.group(1)); + return Long.parseLong(m.group(1)); } private void handleGetList(HttpExchange exchange) { @@ -153,6 +153,46 @@ private void handleGetList(HttpExchange exchange) { } } + private void handleGetRecording(HttpExchange exchange, long id) { + FlightRecorder.getFlightRecorder().getRecordings().stream() + .filter(r -> r.getId() == id) + .findFirst() + .ifPresentOrElse( + r -> { + Recording copy = r.copy(true); + try (InputStream stream = copy.getStream(null, null); + BufferedInputStream bis = new BufferedInputStream(stream); + OutputStream response = exchange.getResponseBody()) { + if (stream == null) { + exchange.sendResponseHeaders( + HttpStatus.SC_NO_CONTENT, BODY_LENGTH_NONE); + } else { + exchange.sendResponseHeaders( + HttpStatus.SC_OK, BODY_LENGTH_UNKNOWN); + bis.transferTo(response); + } + } catch (IOException ioe) { + log.error("I/O error", ioe); + try { + exchange.sendResponseHeaders( + HttpStatus.SC_INTERNAL_SERVER_ERROR, BODY_LENGTH_NONE); + } catch (IOException ioe2) { + log.error("Failed to write response", ioe2); + } + } finally { + copy.close(); + } + }, + () -> { + try { + exchange.sendResponseHeaders( + HttpStatus.SC_NOT_FOUND, BODY_LENGTH_NONE); + } catch (IOException e) { + log.error("Failed to write response", e); + } + }); + } + private void handleStart(HttpExchange exchange) throws IOException { try (InputStream body = exchange.getRequestBody()) { StartRecordingRequest req = mapper.readValue(body, StartRecordingRequest.class); @@ -177,7 +217,7 @@ private void handleStart(HttpExchange exchange) throws IOException { } } - private void handleStop(HttpExchange exchange, int id) throws IOException { + private void handleStop(HttpExchange exchange, long id) throws IOException { invokeOnRecording( exchange, id, @@ -195,7 +235,7 @@ private void handleStop(HttpExchange exchange, int id) throws IOException { }); } - private void handleDelete(HttpExchange exchange, int id) throws IOException { + private void handleDelete(HttpExchange exchange, long id) throws IOException { invokeOnRecording( exchange, id, From d360a51ce105f1568292b1a7ea07d2aca9156a07 Mon Sep 17 00:00:00 2001 From: Ming Wang Date: Wed, 16 Aug 2023 11:46:06 -0400 Subject: [PATCH 22/45] accept snapshot --- .../agent/remote/RecordingsContext.java | 86 +++++++++++++------ 1 file changed, 58 insertions(+), 28 deletions(-) diff --git a/src/main/java/io/cryostat/agent/remote/RecordingsContext.java b/src/main/java/io/cryostat/agent/remote/RecordingsContext.java index fbd7b0ca..6c481417 100644 --- a/src/main/java/io/cryostat/agent/remote/RecordingsContext.java +++ b/src/main/java/io/cryostat/agent/remote/RecordingsContext.java @@ -62,8 +62,7 @@ class RecordingsContext implements RemoteContext { private static final String PATH = "/recordings/"; - private static final Pattern PATH_ID_PATTERN = - Pattern.compile("^" + PATH + "(\\d+)$", Pattern.MULTILINE); + private static final Pattern PATH_ID_PATTERN = Pattern.compile("^" + PATH + "(\\d+)$", Pattern.MULTILINE); private final Logger log = LoggerFactory.getLogger(getClass()); private final SmallRyeConfig config; @@ -106,7 +105,7 @@ public void handle(HttpExchange exchange) throws IOException { } break; case "POST": - handleStart(exchange); + handleStartRecordingOrSnapshot(exchange); break; case "PATCH": id = extractId(exchange); @@ -193,13 +192,26 @@ private void handleGetRecording(HttpExchange exchange, long id) { }); } - private void handleStart(HttpExchange exchange) throws IOException { + private void handleStartRecordingOrSnapshot(HttpExchange exchange) throws IOException { try (InputStream body = exchange.getRequestBody()) { StartRecordingRequest req = mapper.readValue(body, StartRecordingRequest.class); if (!req.isValid()) { exchange.sendResponseHeaders(HttpStatus.SC_BAD_REQUEST, BODY_LENGTH_NONE); return; } + if (req.requestSnapshot()) { + try { + SerializableRecordingDescriptor snapshot = startSnapshot(req, exchange); + exchange.sendResponseHeaders(HttpStatus.SC_CREATED, BODY_LENGTH_UNKNOWN); + try (OutputStream response = exchange.getResponseBody()) { + mapper.writeValue(response, snapshot); + } + return; + } catch (IOException e) { + log.error("Failed to start snapshot", e); + exchange.sendResponseHeaders(HttpStatus.SC_SERVICE_UNAVAILABLE, BODY_LENGTH_NONE); + } + } SerializableRecordingDescriptor recording = startRecording(req); exchange.sendResponseHeaders(HttpStatus.SC_CREATED, BODY_LENGTH_UNKNOWN); try (OutputStream response = exchange.getResponseBody()) { @@ -283,35 +295,31 @@ private List getRecordings() { private SerializableRecordingDescriptor startRecording(StartRecordingRequest req) throws QuantityConversionException, ServiceNotAvailableException, - FlightRecorderException, - org.openjdk.jmc.rjmx.services.jfr.FlightRecorderException, - InvalidEventTemplateException, InvalidXmlException, IOException { + FlightRecorderException, + org.openjdk.jmc.rjmx.services.jfr.FlightRecorderException, + InvalidEventTemplateException, InvalidXmlException, IOException { Runnable cleanup = () -> {}; try { - JFRConnection conn = - jfrConnectionToolkit.connect( - jfrConnectionToolkit.createServiceURL("localhost", 0)); + JFRConnection conn = jfrConnectionToolkit.connect( + jfrConnectionToolkit.createServiceURL("localhost", 0)); IConstrainedMap events; if (req.requestsCustomTemplate()) { - Template template = - localStorageTemplateService.addTemplate( - new ByteArrayInputStream( - req.template.getBytes(StandardCharsets.UTF_8))); + Template template = localStorageTemplateService.addTemplate( + new ByteArrayInputStream( + req.template.getBytes(StandardCharsets.UTF_8))); events = localStorageTemplateService.getEvents(template).orElseThrow(); - cleanup = - () -> { - try { - localStorageTemplateService.deleteTemplate(template); - } catch (InvalidEventTemplateException | IOException e) { - log.error("Failed to clean up template " + template.getName(), e); - } - }; + cleanup = () -> { + try { + localStorageTemplateService.deleteTemplate(template); + } catch (InvalidEventTemplateException | IOException e) { + log.error("Failed to clean up template " + template.getName(), e); + } + }; } else { - events = - new RemoteTemplateService(conn) - .getEvents(req.localTemplateName, TemplateType.TARGET).stream() - .findFirst() - .orElseThrow(); + events = new RemoteTemplateService(conn) + .getEvents(req.localTemplateName, TemplateType.TARGET).stream() + .findFirst() + .orElseThrow(); } IFlightRecorderService svc = conn.getService(); return new SerializableRecordingDescriptor( @@ -329,6 +337,21 @@ private SerializableRecordingDescriptor startRecording(StartRecordingRequest req } } + private SerializableRecordingDescriptor startSnapshot(StartRecordingRequest req, HttpExchange exchange) + throws IOException { + Runnable cleanup = () -> {}; + try { + Recording snapshot = FlightRecorder.getFlightRecorder().takeSnapshot(); + if (snapshot.getSize() == 0) { + log.warn("No active recordings"); + exchange.sendResponseHeaders(HttpStatus.SC_SERVICE_UNAVAILABLE, BODY_LENGTH_NONE); + } + return new SerializableRecordingDescriptor(snapshot); + } finally { + cleanup.run(); + } + } + static class StartRecordingRequest { public String name; @@ -346,12 +369,19 @@ boolean requestsBundledTemplate() { return !StringUtils.isBlank(localTemplateName); } + boolean requestSnapshot() { + boolean snapshotName = name.equals("snapshot"); + boolean snapshotTemplate = StringUtils.isBlank(template) && StringUtils.isBlank(localTemplateName); + boolean snapshotFeatures = duration == 0 && maxSize == 0 && maxAge == 0; + return snapshotName && snapshotTemplate && snapshotFeatures; + } + boolean isValid() { boolean requestsCustomTemplate = requestsCustomTemplate(); boolean requestsBundledTemplate = requestsBundledTemplate(); boolean requestsEither = requestsCustomTemplate || requestsBundledTemplate; boolean requestsBoth = requestsCustomTemplate && requestsBundledTemplate; - return requestsEither && !requestsBoth; + return requestsEither && !requestsBoth && !requestSnapshot(); } } } From f80cb466ced29f328e0e16a2e3042ba6ddbe32cd Mon Sep 17 00:00:00 2001 From: Ming Wang Date: Wed, 16 Aug 2023 13:33:52 -0400 Subject: [PATCH 23/45] update --- src/main/java/io/cryostat/agent/remote/RecordingsContext.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/io/cryostat/agent/remote/RecordingsContext.java b/src/main/java/io/cryostat/agent/remote/RecordingsContext.java index 6c481417..dcecc4b6 100644 --- a/src/main/java/io/cryostat/agent/remote/RecordingsContext.java +++ b/src/main/java/io/cryostat/agent/remote/RecordingsContext.java @@ -206,11 +206,11 @@ private void handleStartRecordingOrSnapshot(HttpExchange exchange) throws IOExce try (OutputStream response = exchange.getResponseBody()) { mapper.writeValue(response, snapshot); } - return; } catch (IOException e) { log.error("Failed to start snapshot", e); exchange.sendResponseHeaders(HttpStatus.SC_SERVICE_UNAVAILABLE, BODY_LENGTH_NONE); } + return; } SerializableRecordingDescriptor recording = startRecording(req); exchange.sendResponseHeaders(HttpStatus.SC_CREATED, BODY_LENGTH_UNKNOWN); From 5e42c4cedea760245a69c39d6be222708cc7ec9f Mon Sep 17 00:00:00 2001 From: Ming Wang Date: Thu, 24 Aug 2023 10:17:03 -0400 Subject: [PATCH 24/45] clean up --- .../agent/remote/RecordingsContext.java | 55 +++++++++++-------- 1 file changed, 31 insertions(+), 24 deletions(-) diff --git a/src/main/java/io/cryostat/agent/remote/RecordingsContext.java b/src/main/java/io/cryostat/agent/remote/RecordingsContext.java index dcecc4b6..bb3324f9 100644 --- a/src/main/java/io/cryostat/agent/remote/RecordingsContext.java +++ b/src/main/java/io/cryostat/agent/remote/RecordingsContext.java @@ -62,7 +62,8 @@ class RecordingsContext implements RemoteContext { private static final String PATH = "/recordings/"; - private static final Pattern PATH_ID_PATTERN = Pattern.compile("^" + PATH + "(\\d+)$", Pattern.MULTILINE); + private static final Pattern PATH_ID_PATTERN = + Pattern.compile("^" + PATH + "(\\d+)$", Pattern.MULTILINE); private final Logger log = LoggerFactory.getLogger(getClass()); private final SmallRyeConfig config; @@ -208,7 +209,8 @@ private void handleStartRecordingOrSnapshot(HttpExchange exchange) throws IOExce } } catch (IOException e) { log.error("Failed to start snapshot", e); - exchange.sendResponseHeaders(HttpStatus.SC_SERVICE_UNAVAILABLE, BODY_LENGTH_NONE); + exchange.sendResponseHeaders( + HttpStatus.SC_SERVICE_UNAVAILABLE, BODY_LENGTH_NONE); } return; } @@ -295,31 +297,35 @@ private List getRecordings() { private SerializableRecordingDescriptor startRecording(StartRecordingRequest req) throws QuantityConversionException, ServiceNotAvailableException, - FlightRecorderException, - org.openjdk.jmc.rjmx.services.jfr.FlightRecorderException, - InvalidEventTemplateException, InvalidXmlException, IOException { + FlightRecorderException, + org.openjdk.jmc.rjmx.services.jfr.FlightRecorderException, + InvalidEventTemplateException, InvalidXmlException, IOException { Runnable cleanup = () -> {}; try { - JFRConnection conn = jfrConnectionToolkit.connect( - jfrConnectionToolkit.createServiceURL("localhost", 0)); + JFRConnection conn = + jfrConnectionToolkit.connect( + jfrConnectionToolkit.createServiceURL("localhost", 0)); IConstrainedMap events; if (req.requestsCustomTemplate()) { - Template template = localStorageTemplateService.addTemplate( - new ByteArrayInputStream( - req.template.getBytes(StandardCharsets.UTF_8))); + Template template = + localStorageTemplateService.addTemplate( + new ByteArrayInputStream( + req.template.getBytes(StandardCharsets.UTF_8))); events = localStorageTemplateService.getEvents(template).orElseThrow(); - cleanup = () -> { - try { - localStorageTemplateService.deleteTemplate(template); - } catch (InvalidEventTemplateException | IOException e) { - log.error("Failed to clean up template " + template.getName(), e); - } - }; + cleanup = + () -> { + try { + localStorageTemplateService.deleteTemplate(template); + } catch (InvalidEventTemplateException | IOException e) { + log.error("Failed to clean up template " + template.getName(), e); + } + }; } else { - events = new RemoteTemplateService(conn) - .getEvents(req.localTemplateName, TemplateType.TARGET).stream() - .findFirst() - .orElseThrow(); + events = + new RemoteTemplateService(conn) + .getEvents(req.localTemplateName, TemplateType.TARGET).stream() + .findFirst() + .orElseThrow(); } IFlightRecorderService svc = conn.getService(); return new SerializableRecordingDescriptor( @@ -337,8 +343,8 @@ private SerializableRecordingDescriptor startRecording(StartRecordingRequest req } } - private SerializableRecordingDescriptor startSnapshot(StartRecordingRequest req, HttpExchange exchange) - throws IOException { + private SerializableRecordingDescriptor startSnapshot( + StartRecordingRequest req, HttpExchange exchange) throws IOException { Runnable cleanup = () -> {}; try { Recording snapshot = FlightRecorder.getFlightRecorder().takeSnapshot(); @@ -371,7 +377,8 @@ boolean requestsBundledTemplate() { boolean requestSnapshot() { boolean snapshotName = name.equals("snapshot"); - boolean snapshotTemplate = StringUtils.isBlank(template) && StringUtils.isBlank(localTemplateName); + boolean snapshotTemplate = + StringUtils.isBlank(template) && StringUtils.isBlank(localTemplateName); boolean snapshotFeatures = duration == 0 && maxSize == 0 && maxAge == 0; return snapshotName && snapshotTemplate && snapshotFeatures; } From 39fcfb657fb66f1350b557015df414ce130ebd73 Mon Sep 17 00:00:00 2001 From: Ming Wang Date: Thu, 24 Aug 2023 10:41:01 -0400 Subject: [PATCH 25/45] check recording is valid --- .../agent/remote/RecordingsContext.java | 69 ++++++++++++++++--- 1 file changed, 61 insertions(+), 8 deletions(-) diff --git a/src/main/java/io/cryostat/agent/remote/RecordingsContext.java b/src/main/java/io/cryostat/agent/remote/RecordingsContext.java index bb3324f9..220229f9 100644 --- a/src/main/java/io/cryostat/agent/remote/RecordingsContext.java +++ b/src/main/java/io/cryostat/agent/remote/RecordingsContext.java @@ -21,6 +21,7 @@ import java.io.InputStream; import java.io.OutputStream; import java.nio.charset.StandardCharsets; +import java.time.Duration; import java.util.List; import java.util.Set; import java.util.function.Consumer; @@ -31,6 +32,7 @@ import javax.inject.Inject; import org.openjdk.jmc.common.unit.IConstrainedMap; +import org.openjdk.jmc.common.unit.ITypedQuantity; import org.openjdk.jmc.common.unit.QuantityConversionException; import org.openjdk.jmc.common.unit.UnitLookup; import org.openjdk.jmc.flightrecorder.configuration.events.EventOptionID; @@ -50,6 +52,7 @@ import io.cryostat.core.templates.Template; import io.cryostat.core.templates.TemplateType; +import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; import com.sun.net.httpserver.HttpExchange; import io.smallrye.config.SmallRyeConfig; @@ -110,8 +113,19 @@ public void handle(HttpExchange exchange) throws IOException { break; case "PATCH": id = extractId(exchange); - if (id >= 0) { - handleStop(exchange, id); + String request = requestStopOrUpdate(exchange); + if (request == "STOPPED") { + if (id >= 0) { + handleStop(exchange, id); + } else { + exchange.sendResponseHeaders(HttpStatus.SC_BAD_REQUEST, BODY_LENGTH_NONE); + } + } else if (request == "UPDATE") { + if (id >= 0) { + handleRecordingUpdate(exchange, id); + } else { + exchange.sendResponseHeaders(HttpStatus.SC_BAD_REQUEST, BODY_LENGTH_NONE); + } } else { exchange.sendResponseHeaders(HttpStatus.SC_BAD_REQUEST, BODY_LENGTH_NONE); } @@ -135,6 +149,14 @@ public void handle(HttpExchange exchange) throws IOException { } } + private static String requestStopOrUpdate(HttpExchange exchange) throws IOException { + try (InputStream body = exchange.getRequestBody()) { + ObjectMapper mapper = new ObjectMapper(); + JsonNode jsonMap = mapper.readTree(body); + return jsonMap.get("state").toString(); + } + } + private static long extractId(HttpExchange exchange) throws IOException { Matcher m = PATH_ID_PATTERN.matcher(exchange.getRequestURI().getPath()); if (!m.find()) { @@ -196,10 +218,6 @@ private void handleGetRecording(HttpExchange exchange, long id) { private void handleStartRecordingOrSnapshot(HttpExchange exchange) throws IOException { try (InputStream body = exchange.getRequestBody()) { StartRecordingRequest req = mapper.readValue(body, StartRecordingRequest.class); - if (!req.isValid()) { - exchange.sendResponseHeaders(HttpStatus.SC_BAD_REQUEST, BODY_LENGTH_NONE); - return; - } if (req.requestSnapshot()) { try { SerializableRecordingDescriptor snapshot = startSnapshot(req, exchange); @@ -214,6 +232,10 @@ private void handleStartRecordingOrSnapshot(HttpExchange exchange) throws IOExce } return; } + if (!req.isValid()) { + exchange.sendResponseHeaders(HttpStatus.SC_BAD_REQUEST, BODY_LENGTH_NONE); + return; + } SerializableRecordingDescriptor recording = startRecording(req); exchange.sendResponseHeaders(HttpStatus.SC_CREATED, BODY_LENGTH_UNKNOWN); try (OutputStream response = exchange.getResponseBody()) { @@ -231,6 +253,37 @@ private void handleStartRecordingOrSnapshot(HttpExchange exchange) throws IOExce } } + private void handleRecordingUpdate(HttpExchange exchange, long id) throws IOException { + try (InputStream body = exchange.getRequestBody()) { + ObjectMapper mapper = new ObjectMapper(); + JsonNode jsonMap = mapper.readTree(body); + invokeOnRecording( + exchange, + id, + r -> { + try { + if (jsonMap.has("name")) { + r.setName(jsonMap.get("name").toString()); + } + if (jsonMap.has("toDisk")) { + r.setToDisk(jsonMap.get("toDisk").asBoolean()); + } + if (jsonMap.has("duration")) { + r.setDuration(Duration.ofMillis(jsonMap.get("duration").asLong())); + } + if (jsonMap.has("maxSize")) { + r.setMaxSize(jsonMap.get("name").asLong()); + } + if (jsonMap.has("maxAge")) { + r.setMaxAge(Duration.ofMillis(jsonMap.get("name").asLong())); + } + } catch (IllegalStateException e) { + sendHeader(exchange, HttpStatus.SC_CONFLICT); + } + }); + } + } + private void handleStop(HttpExchange exchange, long id) throws IOException { invokeOnRecording( exchange, @@ -378,7 +431,7 @@ boolean requestsBundledTemplate() { boolean requestSnapshot() { boolean snapshotName = name.equals("snapshot"); boolean snapshotTemplate = - StringUtils.isBlank(template) && StringUtils.isBlank(localTemplateName); + StringUtils.isBlank(template) && StringUtils.isBlank(localTemplateName); boolean snapshotFeatures = duration == 0 && maxSize == 0 && maxAge == 0; return snapshotName && snapshotTemplate && snapshotFeatures; } @@ -388,7 +441,7 @@ boolean isValid() { boolean requestsBundledTemplate = requestsBundledTemplate(); boolean requestsEither = requestsCustomTemplate || requestsBundledTemplate; boolean requestsBoth = requestsCustomTemplate && requestsBundledTemplate; - return requestsEither && !requestsBoth && !requestSnapshot(); + return (requestsEither && !requestsBoth) || requestSnapshot(); } } } From 2cc7720b646d19318af68c6542bfab006a956103 Mon Sep 17 00:00:00 2001 From: Ming Wang Date: Thu, 31 Aug 2023 15:00:18 -0400 Subject: [PATCH 26/45] update or stop --- .../agent/remote/RecordingsContext.java | 138 ++++++++++-------- 1 file changed, 76 insertions(+), 62 deletions(-) diff --git a/src/main/java/io/cryostat/agent/remote/RecordingsContext.java b/src/main/java/io/cryostat/agent/remote/RecordingsContext.java index 220229f9..e171d338 100644 --- a/src/main/java/io/cryostat/agent/remote/RecordingsContext.java +++ b/src/main/java/io/cryostat/agent/remote/RecordingsContext.java @@ -20,10 +20,14 @@ import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; +import java.net.MalformedURLException; import java.nio.charset.StandardCharsets; import java.time.Duration; +import java.util.Iterator; import java.util.List; +import java.util.Map; import java.util.Set; +import java.util.Map.Entry; import java.util.function.Consumer; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -32,13 +36,14 @@ import javax.inject.Inject; import org.openjdk.jmc.common.unit.IConstrainedMap; -import org.openjdk.jmc.common.unit.ITypedQuantity; import org.openjdk.jmc.common.unit.QuantityConversionException; import org.openjdk.jmc.common.unit.UnitLookup; import org.openjdk.jmc.flightrecorder.configuration.events.EventOptionID; import org.openjdk.jmc.flightrecorder.configuration.recording.RecordingOptionsBuilder; +import org.openjdk.jmc.rjmx.ConnectionException; import org.openjdk.jmc.rjmx.ServiceNotAvailableException; import org.openjdk.jmc.rjmx.services.jfr.IFlightRecorderService; +import org.openjdk.jmc.rjmx.services.jfr.IRecordingDescriptor; import io.cryostat.agent.StringUtils; import io.cryostat.core.FlightRecorderException; @@ -113,19 +118,8 @@ public void handle(HttpExchange exchange) throws IOException { break; case "PATCH": id = extractId(exchange); - String request = requestStopOrUpdate(exchange); - if (request == "STOPPED") { - if (id >= 0) { - handleStop(exchange, id); - } else { - exchange.sendResponseHeaders(HttpStatus.SC_BAD_REQUEST, BODY_LENGTH_NONE); - } - } else if (request == "UPDATE") { - if (id >= 0) { - handleRecordingUpdate(exchange, id); - } else { - exchange.sendResponseHeaders(HttpStatus.SC_BAD_REQUEST, BODY_LENGTH_NONE); - } + if (id >= 0) { + handleStopOrUpdate(exchange, id); } else { exchange.sendResponseHeaders(HttpStatus.SC_BAD_REQUEST, BODY_LENGTH_NONE); } @@ -149,14 +143,6 @@ public void handle(HttpExchange exchange) throws IOException { } } - private static String requestStopOrUpdate(HttpExchange exchange) throws IOException { - try (InputStream body = exchange.getRequestBody()) { - ObjectMapper mapper = new ObjectMapper(); - JsonNode jsonMap = mapper.readTree(body); - return jsonMap.get("state").toString(); - } - } - private static long extractId(HttpExchange exchange) throws IOException { Matcher m = PATH_ID_PATTERN.matcher(exchange.getRequestURI().getPath()); if (!m.find()) { @@ -253,53 +239,81 @@ private void handleStartRecordingOrSnapshot(HttpExchange exchange) throws IOExce } } - private void handleRecordingUpdate(HttpExchange exchange, long id) throws IOException { - try (InputStream body = exchange.getRequestBody()) { - ObjectMapper mapper = new ObjectMapper(); + private void handleStopOrUpdate(HttpExchange exchange, long id) throws IOException { + try { + JFRConnection conn = + jfrConnectionToolkit.connect( + jfrConnectionToolkit.createServiceURL("localhost", 0)); + IFlightRecorderService svc = conn.getService(); + IRecordingDescriptor dsc = svc.getAvailableRecordings().stream().filter(r -> r.getId() == id).findFirst().get(); + RecordingOptionsBuilder builder = new RecordingOptionsBuilder(conn.getService()); + + InputStream body = exchange.getRequestBody(); JsonNode jsonMap = mapper.readTree(body); - invokeOnRecording( - exchange, - id, - r -> { - try { - if (jsonMap.has("name")) { - r.setName(jsonMap.get("name").toString()); - } - if (jsonMap.has("toDisk")) { - r.setToDisk(jsonMap.get("toDisk").asBoolean()); - } - if (jsonMap.has("duration")) { - r.setDuration(Duration.ofMillis(jsonMap.get("duration").asLong())); - } - if (jsonMap.has("maxSize")) { - r.setMaxSize(jsonMap.get("name").asLong()); - } - if (jsonMap.has("maxAge")) { - r.setMaxAge(Duration.ofMillis(jsonMap.get("name").asLong())); - } - } catch (IllegalStateException e) { - sendHeader(exchange, HttpStatus.SC_CONFLICT); + Iterator> fields = jsonMap.fields(); + + while (fields.hasNext()) { + Map.Entry field = fields.next(); + + switch(field.getKey()){ + case "state": + if ("STOPPED".equals(field.getValue().toString())) { + handleStop(exchange, id); + break; + } else if (!"UPDATE".equals(field.getValue().toString())) { + exchange.sendResponseHeaders(HttpStatus.SC_BAD_REQUEST, BODY_LENGTH_NONE); } - }); + break; + case "name": + builder = builder.name(field.getValue().toString()); + break; + case "duration": + builder = builder.duration(field.getValue().asLong()); + break; + case "maxSize": + builder = builder.maxSize(field.getValue().asLong()); + break; + case "maxAge": + builder = builder.maxAge(field.getValue().asLong()); + break; + case "toDisk": + builder = builder.toDisk(field.getValue().asBoolean()); + break; + default: + log.warn("Unknown recording option {}", field.getKey()); + exchange.sendResponseHeaders( + HttpStatus.SC_METHOD_NOT_ALLOWED, BODY_LENGTH_NONE); + break; + } + } + svc.updateRecordingOptions(dsc, builder.build()); + exchange.sendResponseHeaders(HttpStatus.SC_CREATED, BODY_LENGTH_UNKNOWN); + } catch ( ServiceNotAvailableException + | org.openjdk.jmc.rjmx.services.jfr.FlightRecorderException + | QuantityConversionException e){ + log.error("Failed to update recording", e); + exchange.sendResponseHeaders(HttpStatus.SC_INTERNAL_SERVER_ERROR, BODY_LENGTH_NONE); + } finally { + exchange.close(); } } private void handleStop(HttpExchange exchange, long id) throws IOException { invokeOnRecording( - exchange, - id, - r -> { - try { - boolean stopped = r.stop(); - if (!stopped) { - sendHeader(exchange, HttpStatus.SC_BAD_REQUEST); - } else { - sendHeader(exchange, HttpStatus.SC_NO_CONTENT); - } - } catch (IllegalStateException e) { - sendHeader(exchange, HttpStatus.SC_CONFLICT); + exchange, + id, + r -> { + try { + boolean stopped = r.stop(); + if (!stopped) { + sendHeader(exchange, HttpStatus.SC_BAD_REQUEST); + } else { + sendHeader(exchange, HttpStatus.SC_NO_CONTENT); } - }); + } catch (IllegalStateException e) { + sendHeader(exchange, HttpStatus.SC_CONFLICT); + } + }); } private void handleDelete(HttpExchange exchange, long id) throws IOException { @@ -352,7 +366,7 @@ private SerializableRecordingDescriptor startRecording(StartRecordingRequest req throws QuantityConversionException, ServiceNotAvailableException, FlightRecorderException, org.openjdk.jmc.rjmx.services.jfr.FlightRecorderException, - InvalidEventTemplateException, InvalidXmlException, IOException { + InvalidEventTemplateException, InvalidXmlException, IOException, FlightRecorderException { Runnable cleanup = () -> {}; try { JFRConnection conn = From 103361d2a90f4d347fa3fa56b4b4f25a293e8881 Mon Sep 17 00:00:00 2001 From: Ming Wang Date: Fri, 1 Sep 2023 12:16:34 -0400 Subject: [PATCH 27/45] cleanup --- src/main/java/io/cryostat/agent/remote/RecordingsContext.java | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/main/java/io/cryostat/agent/remote/RecordingsContext.java b/src/main/java/io/cryostat/agent/remote/RecordingsContext.java index e171d338..0ada1829 100644 --- a/src/main/java/io/cryostat/agent/remote/RecordingsContext.java +++ b/src/main/java/io/cryostat/agent/remote/RecordingsContext.java @@ -20,9 +20,7 @@ import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; -import java.net.MalformedURLException; import java.nio.charset.StandardCharsets; -import java.time.Duration; import java.util.Iterator; import java.util.List; import java.util.Map; @@ -40,7 +38,6 @@ import org.openjdk.jmc.common.unit.UnitLookup; import org.openjdk.jmc.flightrecorder.configuration.events.EventOptionID; import org.openjdk.jmc.flightrecorder.configuration.recording.RecordingOptionsBuilder; -import org.openjdk.jmc.rjmx.ConnectionException; import org.openjdk.jmc.rjmx.ServiceNotAvailableException; import org.openjdk.jmc.rjmx.services.jfr.IFlightRecorderService; import org.openjdk.jmc.rjmx.services.jfr.IRecordingDescriptor; From 1a8f8da8d342ddfa84113329a952b5de9cb4e7ac Mon Sep 17 00:00:00 2001 From: Ming Wang Date: Thu, 7 Sep 2023 11:02:23 -0400 Subject: [PATCH 28/45] updates --- .../agent/remote/RecordingsContext.java | 37 ++++++++++++++++--- 1 file changed, 31 insertions(+), 6 deletions(-) diff --git a/src/main/java/io/cryostat/agent/remote/RecordingsContext.java b/src/main/java/io/cryostat/agent/remote/RecordingsContext.java index 0ada1829..b7278448 100644 --- a/src/main/java/io/cryostat/agent/remote/RecordingsContext.java +++ b/src/main/java/io/cryostat/agent/remote/RecordingsContext.java @@ -257,7 +257,7 @@ private void handleStopOrUpdate(HttpExchange exchange, long id) throws IOExcepti if ("STOPPED".equals(field.getValue().toString())) { handleStop(exchange, id); break; - } else if (!"UPDATE".equals(field.getValue().toString())) { + } else { exchange.sendResponseHeaders(HttpStatus.SC_BAD_REQUEST, BODY_LENGTH_NONE); } break; @@ -265,16 +265,32 @@ private void handleStopOrUpdate(HttpExchange exchange, long id) throws IOExcepti builder = builder.name(field.getValue().toString()); break; case "duration": - builder = builder.duration(field.getValue().asLong()); + if (field.getValue().canConvertToLong()) { + builder = builder.duration(field.getValue().asLong()); + break; + } + exchange.sendResponseHeaders(HttpStatus.SC_BAD_REQUEST, BODY_LENGTH_NONE); break; case "maxSize": - builder = builder.maxSize(field.getValue().asLong()); + if (field.getValue().canConvertToLong()) { + builder = builder.maxSize(field.getValue().asLong()); + break; + } + exchange.sendResponseHeaders(HttpStatus.SC_BAD_REQUEST, BODY_LENGTH_NONE); break; case "maxAge": - builder = builder.maxAge(field.getValue().asLong()); + if (field.getValue().canConvertToLong()) { + builder = builder.maxAge(field.getValue().asLong()); + break; + } + exchange.sendResponseHeaders(HttpStatus.SC_BAD_REQUEST, BODY_LENGTH_NONE); break; case "toDisk": - builder = builder.toDisk(field.getValue().asBoolean()); + if (field.getValue().isBoolean()) { + builder = builder.toDisk(field.getValue().asBoolean()); + break; + } + exchange.sendResponseHeaders(HttpStatus.SC_BAD_REQUEST, BODY_LENGTH_NONE); break; default: log.warn("Unknown recording option {}", field.getKey()); @@ -284,7 +300,15 @@ private void handleStopOrUpdate(HttpExchange exchange, long id) throws IOExcepti } } svc.updateRecordingOptions(dsc, builder.build()); - exchange.sendResponseHeaders(HttpStatus.SC_CREATED, BODY_LENGTH_UNKNOWN); + exchange.sendResponseHeaders(HttpStatus.SC_OK, BODY_LENGTH_UNKNOWN); + + try (OutputStream response = exchange.getResponseBody()) { + if (response == null) { + exchange.sendResponseHeaders(HttpStatus.SC_NO_CONTENT, BODY_LENGTH_NONE); + } else { + mapper.writeValue(response, dsc); + } + } } catch ( ServiceNotAvailableException | org.openjdk.jmc.rjmx.services.jfr.FlightRecorderException | QuantityConversionException e){ @@ -293,6 +317,7 @@ private void handleStopOrUpdate(HttpExchange exchange, long id) throws IOExcepti } finally { exchange.close(); } + } private void handleStop(HttpExchange exchange, long id) throws IOException { From 95a9430af7dd917e91e31d6040b5fb47935c6b1a Mon Sep 17 00:00:00 2001 From: Andrew Azores Date: Tue, 12 Sep 2023 11:42:13 -0400 Subject: [PATCH 29/45] mvn spotless:apply --- .../agent/remote/RecordingsContext.java | 51 ++++++++++--------- 1 file changed, 28 insertions(+), 23 deletions(-) diff --git a/src/main/java/io/cryostat/agent/remote/RecordingsContext.java b/src/main/java/io/cryostat/agent/remote/RecordingsContext.java index b7278448..0b6c0b32 100644 --- a/src/main/java/io/cryostat/agent/remote/RecordingsContext.java +++ b/src/main/java/io/cryostat/agent/remote/RecordingsContext.java @@ -24,8 +24,8 @@ import java.util.Iterator; import java.util.List; import java.util.Map; -import java.util.Set; import java.util.Map.Entry; +import java.util.Set; import java.util.function.Consumer; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -242,9 +242,13 @@ private void handleStopOrUpdate(HttpExchange exchange, long id) throws IOExcepti jfrConnectionToolkit.connect( jfrConnectionToolkit.createServiceURL("localhost", 0)); IFlightRecorderService svc = conn.getService(); - IRecordingDescriptor dsc = svc.getAvailableRecordings().stream().filter(r -> r.getId() == id).findFirst().get(); + IRecordingDescriptor dsc = + svc.getAvailableRecordings().stream() + .filter(r -> r.getId() == id) + .findFirst() + .get(); RecordingOptionsBuilder builder = new RecordingOptionsBuilder(conn.getService()); - + InputStream body = exchange.getRequestBody(); JsonNode jsonMap = mapper.readTree(body); Iterator> fields = jsonMap.fields(); @@ -252,13 +256,14 @@ private void handleStopOrUpdate(HttpExchange exchange, long id) throws IOExcepti while (fields.hasNext()) { Map.Entry field = fields.next(); - switch(field.getKey()){ + switch (field.getKey()) { case "state": if ("STOPPED".equals(field.getValue().toString())) { handleStop(exchange, id); break; } else { - exchange.sendResponseHeaders(HttpStatus.SC_BAD_REQUEST, BODY_LENGTH_NONE); + exchange.sendResponseHeaders( + HttpStatus.SC_BAD_REQUEST, BODY_LENGTH_NONE); } break; case "name": @@ -309,33 +314,32 @@ private void handleStopOrUpdate(HttpExchange exchange, long id) throws IOExcepti mapper.writeValue(response, dsc); } } - } catch ( ServiceNotAvailableException + } catch (ServiceNotAvailableException | org.openjdk.jmc.rjmx.services.jfr.FlightRecorderException - | QuantityConversionException e){ + | QuantityConversionException e) { log.error("Failed to update recording", e); exchange.sendResponseHeaders(HttpStatus.SC_INTERNAL_SERVER_ERROR, BODY_LENGTH_NONE); } finally { exchange.close(); } - } private void handleStop(HttpExchange exchange, long id) throws IOException { invokeOnRecording( - exchange, - id, - r -> { - try { - boolean stopped = r.stop(); - if (!stopped) { - sendHeader(exchange, HttpStatus.SC_BAD_REQUEST); - } else { - sendHeader(exchange, HttpStatus.SC_NO_CONTENT); + exchange, + id, + r -> { + try { + boolean stopped = r.stop(); + if (!stopped) { + sendHeader(exchange, HttpStatus.SC_BAD_REQUEST); + } else { + sendHeader(exchange, HttpStatus.SC_NO_CONTENT); + } + } catch (IllegalStateException e) { + sendHeader(exchange, HttpStatus.SC_CONFLICT); } - } catch (IllegalStateException e) { - sendHeader(exchange, HttpStatus.SC_CONFLICT); - } - }); + }); } private void handleDelete(HttpExchange exchange, long id) throws IOException { @@ -388,7 +392,8 @@ private SerializableRecordingDescriptor startRecording(StartRecordingRequest req throws QuantityConversionException, ServiceNotAvailableException, FlightRecorderException, org.openjdk.jmc.rjmx.services.jfr.FlightRecorderException, - InvalidEventTemplateException, InvalidXmlException, IOException, FlightRecorderException { + InvalidEventTemplateException, InvalidXmlException, IOException, + FlightRecorderException { Runnable cleanup = () -> {}; try { JFRConnection conn = @@ -467,7 +472,7 @@ boolean requestsBundledTemplate() { boolean requestSnapshot() { boolean snapshotName = name.equals("snapshot"); boolean snapshotTemplate = - StringUtils.isBlank(template) && StringUtils.isBlank(localTemplateName); + StringUtils.isBlank(template) && StringUtils.isBlank(localTemplateName); boolean snapshotFeatures = duration == 0 && maxSize == 0 && maxAge == 0; return snapshotName && snapshotTemplate && snapshotFeatures; } From 8fe5372b31a972947b13001e68983101a04f01f3 Mon Sep 17 00:00:00 2001 From: Andrew Azores Date: Tue, 12 Sep 2023 12:03:23 -0400 Subject: [PATCH 30/45] use textValue() instead of toString() --- src/main/java/io/cryostat/agent/remote/RecordingsContext.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/io/cryostat/agent/remote/RecordingsContext.java b/src/main/java/io/cryostat/agent/remote/RecordingsContext.java index 0b6c0b32..bfb45037 100644 --- a/src/main/java/io/cryostat/agent/remote/RecordingsContext.java +++ b/src/main/java/io/cryostat/agent/remote/RecordingsContext.java @@ -258,7 +258,7 @@ private void handleStopOrUpdate(HttpExchange exchange, long id) throws IOExcepti switch (field.getKey()) { case "state": - if ("STOPPED".equals(field.getValue().toString())) { + if ("STOPPED".equals(field.getValue().textValue())) { handleStop(exchange, id); break; } else { @@ -267,7 +267,7 @@ private void handleStopOrUpdate(HttpExchange exchange, long id) throws IOExcepti } break; case "name": - builder = builder.name(field.getValue().toString()); + builder = builder.name(field.getValue().textValue()); break; case "duration": if (field.getValue().canConvertToLong()) { From 1a0d3b0f7a6ff69ff1b9b1533fda2f22499f579b Mon Sep 17 00:00:00 2001 From: Andrew Azores Date: Thu, 27 Jul 2023 16:10:08 -0400 Subject: [PATCH 31/45] feat(api): enable dynamic JFR start (#165) --- README.md | 1 + .../java/io/cryostat/agent/ConfigModule.java | 3 + .../java/io/cryostat/agent/MainModule.java | 73 +++++- .../java/io/cryostat/agent/WebServer.java | 66 ++++-- .../agent/remote/EventTemplatesContext.java | 48 ++-- .../agent/remote/EventTypesContext.java | 35 +-- .../cryostat/agent/remote/MBeanContext.java | 38 +-- .../agent/remote/MutatingRemoteContext.java | 60 +++++ .../agent/remote/RecordingsContext.java | 217 +++++++++++++----- .../cryostat/agent/remote/RemoteContext.java | 4 + .../META-INF/microprofile-config.properties | 2 + 11 files changed, 402 insertions(+), 145 deletions(-) create mode 100644 src/main/java/io/cryostat/agent/remote/MutatingRemoteContext.java diff --git a/README.md b/README.md index 483f886f..97348324 100644 --- a/README.md +++ b/README.md @@ -34,6 +34,7 @@ and how it advertises itself to a Cryostat server instance. Required properties - [x] `cryostat.agent.baseuri` [`java.net.URI`]: the URL location of the Cryostat server backend that this agent advertises itself to. - [x] `cryostat.agent.callback` [`java.net.URI`]: a URL pointing back to this agent, ex. `"https://12.34.56.78:1234/"`. Cryostat will use this URL to perform health checks and request updates from the agent. This reflects the externally-visible IP address/hostname and port where this application and agent can be found. +- [ ] `cryostat.agent.api.writes-enabled` [`boolean`]: Control whether the agent accepts "write" or mutating operations on its HTTP API. Requests for remote operations such as dynamically starting Flight Recordings will be rejected unless this is set. Default `false`. - [ ] `cryostat.agent.instance-id` [`String`]: a unique ID for this agent instance. This will be used to uniquely identify the agent in the Cryostat discovery database, as well as to unambiguously match its encrypted stored credentials. The default is a random UUID string. It is not recommended to override this value. - [ ] `cryostat.agent.hostname` [`String`]: the hostname for this application instance. This will be used for the published JMX connection URL. If not provided then the default is to attempt to resolve the localhost hostname. - [ ] `cryostat.agent.realm` [`String`]: the Cryostat Discovery API "realm" that this agent belongs to. This should be unique per agent instance. The default is the value of `cryostat.agent.app.name`. diff --git a/src/main/java/io/cryostat/agent/ConfigModule.java b/src/main/java/io/cryostat/agent/ConfigModule.java index c1b1c9e3..a464439b 100644 --- a/src/main/java/io/cryostat/agent/ConfigModule.java +++ b/src/main/java/io/cryostat/agent/ConfigModule.java @@ -85,6 +85,9 @@ public abstract class ConfigModule { public static final String CRYOSTAT_AGENT_HARVESTER_MAX_SIZE_B = "cryostat.agent.harvester.max-size-b"; + public static final String CRYOSTAT_AGENT_API_WRITES_ENABLED = + "cryostat.agent.api.writes-enabled"; + @Provides @Singleton public static SmallRyeConfig provideConfig() { diff --git a/src/main/java/io/cryostat/agent/MainModule.java b/src/main/java/io/cryostat/agent/MainModule.java index 549996cb..3a69388b 100644 --- a/src/main/java/io/cryostat/agent/MainModule.java +++ b/src/main/java/io/cryostat/agent/MainModule.java @@ -15,7 +15,9 @@ */ package io.cryostat.agent; +import java.io.IOException; import java.net.URI; +import java.nio.file.Path; import java.security.KeyManagementException; import java.security.NoSuchAlgorithmException; import java.security.SecureRandom; @@ -39,6 +41,7 @@ import io.cryostat.core.net.JFRConnectionToolkit; import io.cryostat.core.sys.Environment; import io.cryostat.core.sys.FileSystem; +import io.cryostat.core.templates.LocalStorageTemplateService; import io.cryostat.core.tui.ClientWriter; import com.fasterxml.jackson.databind.DeserializationFeature; @@ -65,6 +68,7 @@ public abstract class MainModule { // one for outbound HTTP requests, one for incoming HTTP requests, and one as a general worker private static final int NUM_WORKER_THREADS = 3; private static final String JVM_ID = "JVM_ID"; + private static final String TEMPLATES_PATH = "TEMPLATES_PATH"; @Provides @Singleton @@ -268,21 +272,61 @@ public static Harvester provideHarvester( registration); } + @Provides + @Singleton + public static FileSystem provideFileSystem() { + return new FileSystem(); + } + + @Provides + @Singleton + @Named(TEMPLATES_PATH) + public static Path provideTemplatesTmpPath(FileSystem fs) { + try { + return fs.createTempDirectory(null); + } catch (IOException e) { + throw new RuntimeException(e); + } + } + + @Provides + @Singleton + public static Environment provideEnvironment(@Named(TEMPLATES_PATH) Path templatesTmp) { + return new Environment() { + @Override + public String getEnv(String key) { + if (LocalStorageTemplateService.TEMPLATE_PATH.equals(key)) { + return templatesTmp.toString(); + } + return super.getEnv(key); + } + }; + } + + @Provides + @Singleton + public static ClientWriter provideClientWriter() { + Logger log = LoggerFactory.getLogger(JFRConnectionToolkit.class); + return new ClientWriter() { + @Override + public void print(String msg) { + log.info(msg); + } + }; + } + + @Provides + @Singleton + public static JFRConnectionToolkit provideJfrConnectionToolkit( + ClientWriter cw, FileSystem fs, Environment env) { + return new JFRConnectionToolkit(cw, fs, env); + } + @Provides @Singleton @Named(JVM_ID) - public static String provideJvmId() { + public static String provideJvmId(JFRConnectionToolkit tk) { Logger log = LoggerFactory.getLogger(JFRConnectionToolkit.class); - JFRConnectionToolkit tk = - new JFRConnectionToolkit( - new ClientWriter() { - @Override - public void print(String msg) { - log.warn(msg); - } - }, - new FileSystem(), - new Environment()); try { try (JFRConnection connection = tk.connect(tk.createServiceURL("localhost", 0))) { String id = connection.getJvmId(); @@ -293,4 +337,11 @@ public void print(String msg) { throw new RuntimeException(e); } } + + @Provides + @Singleton + public static LocalStorageTemplateService provideLocalStorageTemplateService( + FileSystem fs, Environment env) { + return new LocalStorageTemplateService(fs, env); + } } diff --git a/src/main/java/io/cryostat/agent/WebServer.java b/src/main/java/io/cryostat/agent/WebServer.java index b352557f..f3bdbfa6 100644 --- a/src/main/java/io/cryostat/agent/WebServer.java +++ b/src/main/java/io/cryostat/agent/WebServer.java @@ -40,6 +40,7 @@ import com.sun.net.httpserver.Filter; import com.sun.net.httpserver.HttpContext; import com.sun.net.httpserver.HttpExchange; +import com.sun.net.httpserver.HttpHandler; import com.sun.net.httpserver.HttpServer; import dagger.Lazy; import org.apache.http.HttpStatus; @@ -97,13 +98,15 @@ void start() throws IOException, NoSuchAlgorithmException { Set mergedContexts = new HashSet<>(remoteContexts.get()); mergedContexts.add(new PingContext(registration)); - mergedContexts.forEach( - rc -> { - HttpContext ctx = this.http.createContext(rc.path(), rc::handle); - ctx.setAuthenticator(agentAuthenticator); - ctx.getFilters().add(requestLoggingFilter); - ctx.getFilters().add(compressionFilter); - }); + mergedContexts.stream() + .filter(RemoteContext::available) + .forEach( + rc -> { + HttpContext ctx = this.http.createContext(rc.path(), wrap(rc::handle)); + ctx.setAuthenticator(agentAuthenticator); + ctx.getFilters().add(requestLoggingFilter); + ctx.getFilters().add(compressionFilter); + }); this.http.start(); } @@ -145,6 +148,18 @@ CompletableFuture generateCredentials() throws NoSuchAlgorithmException { }); } + private HttpHandler wrap(HttpHandler handler) { + return x -> { + try { + handler.handle(x); + } catch (Exception e) { + log.error("Unhandled exception", e); + x.sendResponseHeaders(HttpStatus.SC_INTERNAL_SERVER_ERROR, 0); + x.close(); + } + }; + } + private class PingContext implements RemoteContext { private final Lazy registration; @@ -160,25 +175,26 @@ public String path() { @Override public void handle(HttpExchange exchange) throws IOException { - String mtd = exchange.getRequestMethod(); - switch (mtd) { - case "POST": - synchronized (WebServer.this.credentials) { + try { + String mtd = exchange.getRequestMethod(); + switch (mtd) { + case "POST": + synchronized (WebServer.this.credentials) { + exchange.sendResponseHeaders(HttpStatus.SC_NO_CONTENT, -1); + this.registration + .get() + .notify(Registration.RegistrationEvent.State.REFRESHING); + } + break; + case "GET": exchange.sendResponseHeaders(HttpStatus.SC_NO_CONTENT, -1); - exchange.close(); - this.registration - .get() - .notify(Registration.RegistrationEvent.State.REFRESHING); - } - break; - case "GET": - exchange.sendResponseHeaders(HttpStatus.SC_NO_CONTENT, -1); - exchange.close(); - break; - default: - exchange.sendResponseHeaders(HttpStatus.SC_NOT_FOUND, -1); - exchange.close(); - break; + break; + default: + exchange.sendResponseHeaders(HttpStatus.SC_NOT_FOUND, -1); + break; + } + } finally { + exchange.close(); } } } diff --git a/src/main/java/io/cryostat/agent/remote/EventTemplatesContext.java b/src/main/java/io/cryostat/agent/remote/EventTemplatesContext.java index 03c0770d..fb5fca55 100644 --- a/src/main/java/io/cryostat/agent/remote/EventTemplatesContext.java +++ b/src/main/java/io/cryostat/agent/remote/EventTemplatesContext.java @@ -48,30 +48,32 @@ public String path() { @Override public void handle(HttpExchange exchange) throws IOException { - String mtd = exchange.getRequestMethod(); - switch (mtd) { - case "GET": - try { - exchange.sendResponseHeaders(HttpStatus.SC_OK, 0); - try (OutputStream response = exchange.getResponseBody()) { - FlightRecorderMXBean bean = - ManagementFactory.getPlatformMXBean(FlightRecorderMXBean.class); - List xmlTexts = - bean.getConfigurations().stream() - .map(ConfigurationInfo::getContents) - .collect(Collectors.toList()); - mapper.writeValue(response, xmlTexts); + try { + String mtd = exchange.getRequestMethod(); + switch (mtd) { + case "GET": + try { + exchange.sendResponseHeaders(HttpStatus.SC_OK, 0); + try (OutputStream response = exchange.getResponseBody()) { + FlightRecorderMXBean bean = + ManagementFactory.getPlatformMXBean(FlightRecorderMXBean.class); + List xmlTexts = + bean.getConfigurations().stream() + .map(ConfigurationInfo::getContents) + .collect(Collectors.toList()); + mapper.writeValue(response, xmlTexts); + } + } catch (Exception e) { + log.error("events serialization failure", e); } - } catch (Exception e) { - log.error("events serialization failure", e); - } finally { - exchange.close(); - } - break; - default: - exchange.sendResponseHeaders(HttpStatus.SC_NOT_FOUND, -1); - exchange.close(); - break; + break; + default: + log.warn("Unknown request method {}", mtd); + exchange.sendResponseHeaders(HttpStatus.SC_METHOD_NOT_ALLOWED, -1); + break; + } + } finally { + exchange.close(); } } } diff --git a/src/main/java/io/cryostat/agent/remote/EventTypesContext.java b/src/main/java/io/cryostat/agent/remote/EventTypesContext.java index 8efe03fb..0fc79763 100644 --- a/src/main/java/io/cryostat/agent/remote/EventTypesContext.java +++ b/src/main/java/io/cryostat/agent/remote/EventTypesContext.java @@ -50,25 +50,30 @@ public String path() { @Override public void handle(HttpExchange exchange) throws IOException { - String mtd = exchange.getRequestMethod(); - switch (mtd) { - case "GET": - try { - List events = getEventTypes(); + try { + String mtd = exchange.getRequestMethod(); + switch (mtd) { + case "GET": + List events = new ArrayList<>(); + try { + events.addAll(getEventTypes()); + } catch (Exception e) { + log.error("events serialization failure", e); + exchange.sendResponseHeaders(HttpStatus.SC_INTERNAL_SERVER_ERROR, 0); + break; + } exchange.sendResponseHeaders(HttpStatus.SC_OK, 0); try (OutputStream response = exchange.getResponseBody()) { mapper.writeValue(response, events); } - } catch (Exception e) { - log.error("events serialization failure", e); - } finally { - exchange.close(); - } - break; - default: - exchange.sendResponseHeaders(HttpStatus.SC_NOT_FOUND, -1); - exchange.close(); - break; + break; + default: + log.warn("Unknown request method {}", mtd); + exchange.sendResponseHeaders(HttpStatus.SC_METHOD_NOT_ALLOWED, -1); + break; + } + } finally { + exchange.close(); } } diff --git a/src/main/java/io/cryostat/agent/remote/MBeanContext.java b/src/main/java/io/cryostat/agent/remote/MBeanContext.java index 3b7bd1bd..c085df90 100644 --- a/src/main/java/io/cryostat/agent/remote/MBeanContext.java +++ b/src/main/java/io/cryostat/agent/remote/MBeanContext.java @@ -64,25 +64,27 @@ public String path() { @Override public void handle(HttpExchange exchange) throws IOException { - String mtd = exchange.getRequestMethod(); - switch (mtd) { - case "GET": - try { - MBeanMetrics metrics = getMBeanMetrics(); - exchange.sendResponseHeaders(HttpStatus.SC_OK, 0); - try (OutputStream response = exchange.getResponseBody()) { - mapper.writeValue(response, metrics); + try { + String mtd = exchange.getRequestMethod(); + switch (mtd) { + case "GET": + try { + MBeanMetrics metrics = getMBeanMetrics(); + exchange.sendResponseHeaders(HttpStatus.SC_OK, 0); + try (OutputStream response = exchange.getResponseBody()) { + mapper.writeValue(response, metrics); + } + } catch (Exception e) { + log.error("mbean serialization failure", e); } - } catch (Exception e) { - log.error("mbean serialization failure", e); - } finally { - exchange.close(); - } - break; - default: - exchange.sendResponseHeaders(HttpStatus.SC_NOT_FOUND, -1); - exchange.close(); - break; + break; + default: + log.warn("Unknown request method {}", mtd); + exchange.sendResponseHeaders(HttpStatus.SC_METHOD_NOT_ALLOWED, -1); + break; + } + } finally { + exchange.close(); } } diff --git a/src/main/java/io/cryostat/agent/remote/MutatingRemoteContext.java b/src/main/java/io/cryostat/agent/remote/MutatingRemoteContext.java new file mode 100644 index 00000000..89f65c67 --- /dev/null +++ b/src/main/java/io/cryostat/agent/remote/MutatingRemoteContext.java @@ -0,0 +1,60 @@ +/* + * Copyright The Cryostat Authors + * + * The Universal Permissive License (UPL), Version 1.0 + * + * Subject to the condition set forth below, permission is hereby granted to any + * person obtaining a copy of this software, associated documentation and/or data + * (collectively the "Software"), free of charge and under any and all copyright + * rights in the Software, and any and all patent rights owned or freely + * licensable by each licensor hereunder covering either (i) the unmodified + * Software as contributed to or provided by such licensor, or (ii) the Larger + * Works (as defined below), to deal in both + * + * (a) the Software, and + * (b) any piece of software and/or hardware listed in the lrgrwrks.txt file if + * one is included with the Software (each a "Larger Work" to which the Software + * is contributed by such licensors), + * + * without restriction, including without limitation the rights to copy, create + * derivative works of, display, perform, and distribute the Software and make, + * use, sell, offer for sale, import, export, have made, and have sold the + * Software and the Larger Work(s), and to sublicense the foregoing rights on + * either these or other terms. + * + * This license is subject to the following condition: + * The above copyright notice and either this complete permission notice or at + * a minimum a reference to the UPL must be included in all copies or + * substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + */ +package io.cryostat.agent.remote; + +import io.cryostat.agent.ConfigModule; + +import io.smallrye.config.SmallRyeConfig; + +public abstract class MutatingRemoteContext implements RemoteContext { + + protected final SmallRyeConfig config; + + protected MutatingRemoteContext(SmallRyeConfig config) { + this.config = config; + } + + @Override + public boolean available() { + return apiWritesEnabled(config); + } + + public static boolean apiWritesEnabled(SmallRyeConfig config) { + return config.getValue(ConfigModule.CRYOSTAT_AGENT_API_WRITES_ENABLED, boolean.class); + } +} diff --git a/src/main/java/io/cryostat/agent/remote/RecordingsContext.java b/src/main/java/io/cryostat/agent/remote/RecordingsContext.java index 0a605cbd..d5c9cc96 100644 --- a/src/main/java/io/cryostat/agent/remote/RecordingsContext.java +++ b/src/main/java/io/cryostat/agent/remote/RecordingsContext.java @@ -15,19 +15,41 @@ */ package io.cryostat.agent.remote; +import java.io.ByteArrayInputStream; import java.io.IOException; +import java.io.InputStream; import java.io.OutputStream; +import java.nio.charset.StandardCharsets; import java.util.List; -import java.util.Map; +import java.util.Set; import java.util.stream.Collectors; import javax.inject.Inject; +import org.openjdk.jmc.common.unit.IConstrainedMap; +import org.openjdk.jmc.common.unit.QuantityConversionException; +import org.openjdk.jmc.common.unit.UnitLookup; +import org.openjdk.jmc.flightrecorder.configuration.events.EventOptionID; +import org.openjdk.jmc.flightrecorder.configuration.recording.RecordingOptionsBuilder; +import org.openjdk.jmc.rjmx.ServiceNotAvailableException; +import org.openjdk.jmc.rjmx.services.jfr.IFlightRecorderService; + +import io.cryostat.agent.StringUtils; +import io.cryostat.core.FlightRecorderException; +import io.cryostat.core.net.JFRConnection; +import io.cryostat.core.net.JFRConnectionToolkit; +import io.cryostat.core.serialization.SerializableRecordingDescriptor; +import io.cryostat.core.templates.LocalStorageTemplateService; +import io.cryostat.core.templates.MutableTemplateService.InvalidEventTemplateException; +import io.cryostat.core.templates.MutableTemplateService.InvalidXmlException; +import io.cryostat.core.templates.RemoteTemplateService; +import io.cryostat.core.templates.Template; +import io.cryostat.core.templates.TemplateType; + import com.fasterxml.jackson.databind.ObjectMapper; import com.sun.net.httpserver.HttpExchange; -import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; +import io.smallrye.config.SmallRyeConfig; import jdk.jfr.FlightRecorder; -import jdk.jfr.Recording; import org.apache.http.HttpStatus; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -35,11 +57,21 @@ class RecordingsContext implements RemoteContext { private final Logger log = LoggerFactory.getLogger(getClass()); + private final SmallRyeConfig config; private final ObjectMapper mapper; + private final JFRConnectionToolkit jfrConnectionToolkit; + private final LocalStorageTemplateService localStorageTemplateService; @Inject - RecordingsContext(ObjectMapper mapper) { + RecordingsContext( + SmallRyeConfig config, + ObjectMapper mapper, + JFRConnectionToolkit jfrConnectionToolkit, + LocalStorageTemplateService localStorageTemplateService) { + this.config = config; this.mapper = mapper; + this.jfrConnectionToolkit = jfrConnectionToolkit; + this.localStorageTemplateService = localStorageTemplateService; } @Override @@ -49,67 +81,146 @@ public String path() { @Override public void handle(HttpExchange exchange) throws IOException { - String mtd = exchange.getRequestMethod(); - switch (mtd) { - case "GET": - try { - List recordings = getRecordings(); - exchange.sendResponseHeaders(HttpStatus.SC_OK, 0); + try { + String mtd = exchange.getRequestMethod(); + if (!ensureMethodAccepted(exchange)) { + return; + } + switch (mtd) { + case "GET": try (OutputStream response = exchange.getResponseBody()) { + List recordings = getRecordings(); + exchange.sendResponseHeaders(HttpStatus.SC_OK, 0); mapper.writeValue(response, recordings); + } catch (Exception e) { + log.error("recordings serialization failure", e); + } + break; + case "POST": + try (InputStream body = exchange.getRequestBody()) { + StartRecordingRequest req = + mapper.readValue(body, StartRecordingRequest.class); + if (!req.isValid()) { + exchange.sendResponseHeaders(HttpStatus.SC_BAD_REQUEST, -1); + return; + } + SerializableRecordingDescriptor recording = startRecording(req); + exchange.sendResponseHeaders(HttpStatus.SC_CREATED, 0); + try (OutputStream response = exchange.getResponseBody()) { + mapper.writeValue(response, recording); + } + } catch (QuantityConversionException + | ServiceNotAvailableException + | FlightRecorderException + | org.openjdk.jmc.rjmx.services.jfr.FlightRecorderException + | InvalidEventTemplateException + | InvalidXmlException + | IOException e) { + log.error("Failed to start recording", e); + exchange.sendResponseHeaders(HttpStatus.SC_INTERNAL_SERVER_ERROR, -1); } - } catch (Exception e) { - log.error("recordings serialization failure", e); - } finally { - exchange.close(); - } - break; - default: - exchange.sendResponseHeaders(HttpStatus.SC_NOT_FOUND, -1); - exchange.close(); - break; + break; + default: + log.warn("Unknown request method {}", mtd); + exchange.sendResponseHeaders(HttpStatus.SC_METHOD_NOT_ALLOWED, -1); + break; + } + } finally { + exchange.close(); + } + } + + private boolean ensureMethodAccepted(HttpExchange exchange) throws IOException { + Set blocked = Set.of("POST"); + String mtd = exchange.getRequestMethod(); + boolean restricted = blocked.contains(mtd); + if (!restricted) { + return true; + } + boolean passed = restricted && MutatingRemoteContext.apiWritesEnabled(config); + if (!passed) { + exchange.sendResponseHeaders(HttpStatus.SC_FORBIDDEN, -1); } + return passed; } - private List getRecordings() { + private List getRecordings() { return FlightRecorder.getFlightRecorder().getRecordings().stream() - .map(RecordingInfo::new) + .map(SerializableRecordingDescriptor::new) .collect(Collectors.toList()); } - @SuppressFBWarnings(value = "URF_UNREAD_FIELD") - private static class RecordingInfo { - - public final long id; - public final String name; - public final String state; - public final Map options; - public final long startTime; - public final long duration; - public final boolean isContinuous; - public final boolean toDisk; - public final long maxSize; - public final long maxAge; - - RecordingInfo(Recording rec) { - this.id = rec.getId(); - this.name = rec.getName(); - this.state = rec.getState().name(); - this.options = rec.getSettings(); - if (rec.getStartTime() != null) { - this.startTime = rec.getStartTime().toEpochMilli(); + private SerializableRecordingDescriptor startRecording(StartRecordingRequest req) + throws QuantityConversionException, ServiceNotAvailableException, + FlightRecorderException, + org.openjdk.jmc.rjmx.services.jfr.FlightRecorderException, + InvalidEventTemplateException, InvalidXmlException, IOException { + Runnable cleanup = () -> {}; + try { + JFRConnection conn = + jfrConnectionToolkit.connect( + jfrConnectionToolkit.createServiceURL("localhost", 0)); + IConstrainedMap events; + if (req.requestsCustomTemplate()) { + Template template = + localStorageTemplateService.addTemplate( + new ByteArrayInputStream( + req.template.getBytes(StandardCharsets.UTF_8))); + events = localStorageTemplateService.getEvents(template).orElseThrow(); + cleanup = + () -> { + try { + localStorageTemplateService.deleteTemplate(template); + } catch (InvalidEventTemplateException | IOException e) { + log.error("Failed to clean up template " + template.getName(), e); + } + }; } else { - this.startTime = 0; - } - this.isContinuous = rec.getDuration() == null; - this.duration = this.isContinuous ? 0 : rec.getDuration().toMillis(); - this.toDisk = rec.isToDisk(); - this.maxSize = rec.getMaxSize(); - if (rec.getMaxAge() != null) { - this.maxAge = rec.getMaxAge().toMillis(); - } else { - this.maxAge = 0; + events = + new RemoteTemplateService(conn) + .getEvents(req.localTemplateName, TemplateType.TARGET).stream() + .findFirst() + .orElseThrow(); } + IFlightRecorderService svc = conn.getService(); + return new SerializableRecordingDescriptor( + svc.start( + new RecordingOptionsBuilder(conn.getService()) + .name(req.name) + .duration(UnitLookup.MILLISECOND.quantity(req.duration)) + .maxSize(UnitLookup.BYTE.quantity(req.maxSize)) + .maxAge(UnitLookup.MILLISECOND.quantity(req.maxAge)) + .toDisk(true) + .build(), + events)); + } finally { + cleanup.run(); + } + } + + static class StartRecordingRequest { + + public String name; + public String localTemplateName; + public String template; + public long duration; + public long maxSize; + public long maxAge; + + boolean requestsCustomTemplate() { + return !StringUtils.isBlank(template); + } + + boolean requestsBundledTemplate() { + return !StringUtils.isBlank(localTemplateName); + } + + boolean isValid() { + boolean requestsCustomTemplate = requestsCustomTemplate(); + boolean requestsBundledTemplate = requestsBundledTemplate(); + boolean requestsEither = requestsCustomTemplate || requestsBundledTemplate; + boolean requestsBoth = requestsCustomTemplate && requestsBundledTemplate; + return requestsEither && !requestsBoth; } } } diff --git a/src/main/java/io/cryostat/agent/remote/RemoteContext.java b/src/main/java/io/cryostat/agent/remote/RemoteContext.java index 5290cd39..3b5a2271 100644 --- a/src/main/java/io/cryostat/agent/remote/RemoteContext.java +++ b/src/main/java/io/cryostat/agent/remote/RemoteContext.java @@ -19,4 +19,8 @@ public interface RemoteContext extends HttpHandler { String path(); + + default boolean available() { + return true; + } } diff --git a/src/main/resources/META-INF/microprofile-config.properties b/src/main/resources/META-INF/microprofile-config.properties index c1eac25e..3af7416f 100644 --- a/src/main/resources/META-INF/microprofile-config.properties +++ b/src/main/resources/META-INF/microprofile-config.properties @@ -1,6 +1,8 @@ cryostat.agent.app.name=cryostat-agent cryostat.agent.baseuri= +cryostat.agent.api.writes-enabled=false + cryostat.agent.webclient.ssl.trust-all=false cryostat.agent.webclient.ssl.verify-hostname=true cryostat.agent.webclient.connect.timeout-ms=1000 From 9dd66f19d5c1e159334acf1b17213acdfd093534 Mon Sep 17 00:00:00 2001 From: Andrew Azores Date: Wed, 13 Sep 2023 12:44:02 -0400 Subject: [PATCH 32/45] update license header --- .../agent/remote/MutatingRemoteContext.java | 42 +++++-------------- 1 file changed, 10 insertions(+), 32 deletions(-) diff --git a/src/main/java/io/cryostat/agent/remote/MutatingRemoteContext.java b/src/main/java/io/cryostat/agent/remote/MutatingRemoteContext.java index 89f65c67..753a2f52 100644 --- a/src/main/java/io/cryostat/agent/remote/MutatingRemoteContext.java +++ b/src/main/java/io/cryostat/agent/remote/MutatingRemoteContext.java @@ -1,39 +1,17 @@ /* - * Copyright The Cryostat Authors + * Copyright The Cryostat Authors. * - * The Universal Permissive License (UPL), Version 1.0 + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at * - * Subject to the condition set forth below, permission is hereby granted to any - * person obtaining a copy of this software, associated documentation and/or data - * (collectively the "Software"), free of charge and under any and all copyright - * rights in the Software, and any and all patent rights owned or freely - * licensable by each licensor hereunder covering either (i) the unmodified - * Software as contributed to or provided by such licensor, or (ii) the Larger - * Works (as defined below), to deal in both + * http://www.apache.org/licenses/LICENSE-2.0 * - * (a) the Software, and - * (b) any piece of software and/or hardware listed in the lrgrwrks.txt file if - * one is included with the Software (each a "Larger Work" to which the Software - * is contributed by such licensors), - * - * without restriction, including without limitation the rights to copy, create - * derivative works of, display, perform, and distribute the Software and make, - * use, sell, offer for sale, import, export, have made, and have sold the - * Software and the Larger Work(s), and to sublicense the foregoing rights on - * either these or other terms. - * - * This license is subject to the following condition: - * The above copyright notice and either this complete permission notice or at - * a minimum a reference to the UPL must be included in all copies or - * substantial portions of the Software. - * - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, - * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE - * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER - * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, - * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE - * SOFTWARE. + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. */ package io.cryostat.agent.remote; From df6b1f52435e8a5379c2886b18cec516784d40d5 Mon Sep 17 00:00:00 2001 From: Andrew Azores Date: Wed, 13 Sep 2023 13:49:00 -0400 Subject: [PATCH 33/45] remove redundant condition --- src/main/java/io/cryostat/agent/remote/RecordingsContext.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/io/cryostat/agent/remote/RecordingsContext.java b/src/main/java/io/cryostat/agent/remote/RecordingsContext.java index d5c9cc96..0d608ef3 100644 --- a/src/main/java/io/cryostat/agent/remote/RecordingsContext.java +++ b/src/main/java/io/cryostat/agent/remote/RecordingsContext.java @@ -137,7 +137,7 @@ private boolean ensureMethodAccepted(HttpExchange exchange) throws IOException { if (!restricted) { return true; } - boolean passed = restricted && MutatingRemoteContext.apiWritesEnabled(config); + boolean passed = MutatingRemoteContext.apiWritesEnabled(config); if (!passed) { exchange.sendResponseHeaders(HttpStatus.SC_FORBIDDEN, -1); } From b857ff420a9d9a5d823f4fedd6750f09c4d743ae Mon Sep 17 00:00:00 2001 From: Andrew Azores Date: Wed, 13 Sep 2023 14:48:24 -0400 Subject: [PATCH 34/45] fixup --- .../java/io/cryostat/agent/WebServer.java | 26 ------------------- 1 file changed, 26 deletions(-) diff --git a/src/main/java/io/cryostat/agent/WebServer.java b/src/main/java/io/cryostat/agent/WebServer.java index b1969459..b8f41da6 100644 --- a/src/main/java/io/cryostat/agent/WebServer.java +++ b/src/main/java/io/cryostat/agent/WebServer.java @@ -161,32 +161,6 @@ private HttpHandler wrap(HttpHandler handler) { }; } - private HttpHandler wrap(HttpHandler handler) { - return x -> { - try { - handler.handle(x); - } catch (Exception e) { - log.error("Unhandled exception", e); - x.sendResponseHeaders( - HttpStatus.SC_INTERNAL_SERVER_ERROR, RemoteContext.BODY_LENGTH_NONE); - x.close(); - } - }; - } - - private HttpHandler wrap(HttpHandler handler) { - return x -> { - try { - handler.handle(x); - } catch (Exception e) { - log.error("Unhandled exception", e); - x.sendResponseHeaders( - HttpStatus.SC_INTERNAL_SERVER_ERROR, RemoteContext.BODY_LENGTH_NONE); - x.close(); - } - }; - } - private class PingContext implements RemoteContext { private final Lazy registration; From baa3e3754b3410aaf1eb6940e831e9d6e7b72936 Mon Sep 17 00:00:00 2001 From: Andrew Azores Date: Wed, 13 Sep 2023 15:05:25 -0400 Subject: [PATCH 35/45] attempt to open all resources before sending response header --- .../cryostat/agent/remote/EventTemplatesContext.java | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/main/java/io/cryostat/agent/remote/EventTemplatesContext.java b/src/main/java/io/cryostat/agent/remote/EventTemplatesContext.java index 53aa3bed..ffb35182 100644 --- a/src/main/java/io/cryostat/agent/remote/EventTemplatesContext.java +++ b/src/main/java/io/cryostat/agent/remote/EventTemplatesContext.java @@ -53,14 +53,14 @@ public void handle(HttpExchange exchange) throws IOException { switch (mtd) { case "GET": try { + FlightRecorderMXBean bean = + ManagementFactory.getPlatformMXBean(FlightRecorderMXBean.class); + List xmlTexts = + bean.getConfigurations().stream() + .map(ConfigurationInfo::getContents) + .collect(Collectors.toList()); exchange.sendResponseHeaders(HttpStatus.SC_OK, BODY_LENGTH_UNKNOWN); try (OutputStream response = exchange.getResponseBody()) { - FlightRecorderMXBean bean = - ManagementFactory.getPlatformMXBean(FlightRecorderMXBean.class); - List xmlTexts = - bean.getConfigurations().stream() - .map(ConfigurationInfo::getContents) - .collect(Collectors.toList()); mapper.writeValue(response, xmlTexts); } } catch (Exception e) { From 5268e587cb7c11db5c4c23dbd974ccdd69163ae6 Mon Sep 17 00:00:00 2001 From: Andrew Azores Date: Wed, 13 Sep 2023 15:07:54 -0400 Subject: [PATCH 36/45] remove redundant condition --- src/main/java/io/cryostat/agent/remote/RecordingsContext.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/io/cryostat/agent/remote/RecordingsContext.java b/src/main/java/io/cryostat/agent/remote/RecordingsContext.java index bfb45037..ef733f10 100644 --- a/src/main/java/io/cryostat/agent/remote/RecordingsContext.java +++ b/src/main/java/io/cryostat/agent/remote/RecordingsContext.java @@ -375,7 +375,7 @@ private boolean ensureMethodAccepted(HttpExchange exchange) throws IOException { if (!restricted) { return true; } - boolean passed = restricted && MutatingRemoteContext.apiWritesEnabled(config); + boolean passed = MutatingRemoteContext.apiWritesEnabled(config); if (!passed) { exchange.sendResponseHeaders(HttpStatus.SC_FORBIDDEN, BODY_LENGTH_NONE); } From 30c2ab183fe9980a291183c16abb910bcdf17dbe Mon Sep 17 00:00:00 2001 From: Andrew Azores Date: Thu, 14 Sep 2023 11:22:24 -0400 Subject: [PATCH 37/45] cleanup --- .../agent/remote/RecordingsContext.java | 21 ++++++++++--------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/src/main/java/io/cryostat/agent/remote/RecordingsContext.java b/src/main/java/io/cryostat/agent/remote/RecordingsContext.java index ef733f10..3db4dda1 100644 --- a/src/main/java/io/cryostat/agent/remote/RecordingsContext.java +++ b/src/main/java/io/cryostat/agent/remote/RecordingsContext.java @@ -204,6 +204,11 @@ private void handleStartRecordingOrSnapshot(HttpExchange exchange) throws IOExce if (req.requestSnapshot()) { try { SerializableRecordingDescriptor snapshot = startSnapshot(req, exchange); + if (snapshot == null) { + exchange.sendResponseHeaders( + HttpStatus.SC_SERVICE_UNAVAILABLE, BODY_LENGTH_NONE); + return; + } exchange.sendResponseHeaders(HttpStatus.SC_CREATED, BODY_LENGTH_UNKNOWN); try (OutputStream response = exchange.getResponseBody()) { mapper.writeValue(response, snapshot); @@ -439,17 +444,13 @@ private SerializableRecordingDescriptor startRecording(StartRecordingRequest req private SerializableRecordingDescriptor startSnapshot( StartRecordingRequest req, HttpExchange exchange) throws IOException { - Runnable cleanup = () -> {}; - try { - Recording snapshot = FlightRecorder.getFlightRecorder().takeSnapshot(); - if (snapshot.getSize() == 0) { - log.warn("No active recordings"); - exchange.sendResponseHeaders(HttpStatus.SC_SERVICE_UNAVAILABLE, BODY_LENGTH_NONE); - } - return new SerializableRecordingDescriptor(snapshot); - } finally { - cleanup.run(); + Recording snapshot = FlightRecorder.getFlightRecorder().takeSnapshot(); + if (snapshot.getSize() == 0) { + log.warn("No active recordings"); + snapshot.close(); + return null; } + return new SerializableRecordingDescriptor(snapshot); } static class StartRecordingRequest { From 271e659a2a45c41ac6833769aa7fcd753f9f9279 Mon Sep 17 00:00:00 2001 From: Andrew Azores Date: Thu, 14 Sep 2023 15:53:55 -0400 Subject: [PATCH 38/45] do not process blank name updates --- .../java/io/cryostat/agent/remote/RecordingsContext.java | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/main/java/io/cryostat/agent/remote/RecordingsContext.java b/src/main/java/io/cryostat/agent/remote/RecordingsContext.java index 3db4dda1..80e28bad 100644 --- a/src/main/java/io/cryostat/agent/remote/RecordingsContext.java +++ b/src/main/java/io/cryostat/agent/remote/RecordingsContext.java @@ -265,14 +265,17 @@ private void handleStopOrUpdate(HttpExchange exchange, long id) throws IOExcepti case "state": if ("STOPPED".equals(field.getValue().textValue())) { handleStop(exchange, id); - break; } else { exchange.sendResponseHeaders( HttpStatus.SC_BAD_REQUEST, BODY_LENGTH_NONE); } break; case "name": - builder = builder.name(field.getValue().textValue()); + if (!StringUtils.isBlank(field.getValue().textValue())) { + builder = builder.name(field.getValue().textValue()); + break; + } + exchange.sendResponseHeaders(HttpStatus.SC_BAD_REQUEST, BODY_LENGTH_NONE); break; case "duration": if (field.getValue().canConvertToLong()) { From f5e9b1c92824f59c2c536edd29c82c8a2ee1320c Mon Sep 17 00:00:00 2001 From: Andrew Azores Date: Thu, 14 Sep 2023 15:54:12 -0400 Subject: [PATCH 39/45] correct response status code when unknown PATCH body key is processed --- src/main/java/io/cryostat/agent/remote/RecordingsContext.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/io/cryostat/agent/remote/RecordingsContext.java b/src/main/java/io/cryostat/agent/remote/RecordingsContext.java index 80e28bad..cd950d30 100644 --- a/src/main/java/io/cryostat/agent/remote/RecordingsContext.java +++ b/src/main/java/io/cryostat/agent/remote/RecordingsContext.java @@ -308,7 +308,7 @@ private void handleStopOrUpdate(HttpExchange exchange, long id) throws IOExcepti default: log.warn("Unknown recording option {}", field.getKey()); exchange.sendResponseHeaders( - HttpStatus.SC_METHOD_NOT_ALLOWED, BODY_LENGTH_NONE); + HttpStatus.SC_BAD_REQUEST, BODY_LENGTH_NONE); break; } } From 77aa92e077d3642fbce4c0758d2ba6fd2a02e6fa Mon Sep 17 00:00:00 2001 From: Andrew Azores Date: Thu, 14 Sep 2023 15:55:40 -0400 Subject: [PATCH 40/45] do not continue processing and attempting to send response headers after first PATCH body validation failure --- .../cryostat/agent/remote/RecordingsContext.java | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/main/java/io/cryostat/agent/remote/RecordingsContext.java b/src/main/java/io/cryostat/agent/remote/RecordingsContext.java index cd950d30..d6bcec1f 100644 --- a/src/main/java/io/cryostat/agent/remote/RecordingsContext.java +++ b/src/main/java/io/cryostat/agent/remote/RecordingsContext.java @@ -268,6 +268,7 @@ private void handleStopOrUpdate(HttpExchange exchange, long id) throws IOExcepti } else { exchange.sendResponseHeaders( HttpStatus.SC_BAD_REQUEST, BODY_LENGTH_NONE); + return; } break; case "name": @@ -276,40 +277,39 @@ private void handleStopOrUpdate(HttpExchange exchange, long id) throws IOExcepti break; } exchange.sendResponseHeaders(HttpStatus.SC_BAD_REQUEST, BODY_LENGTH_NONE); - break; + return; case "duration": if (field.getValue().canConvertToLong()) { builder = builder.duration(field.getValue().asLong()); break; } exchange.sendResponseHeaders(HttpStatus.SC_BAD_REQUEST, BODY_LENGTH_NONE); - break; + return; case "maxSize": if (field.getValue().canConvertToLong()) { builder = builder.maxSize(field.getValue().asLong()); break; } exchange.sendResponseHeaders(HttpStatus.SC_BAD_REQUEST, BODY_LENGTH_NONE); - break; + return; case "maxAge": if (field.getValue().canConvertToLong()) { builder = builder.maxAge(field.getValue().asLong()); break; } exchange.sendResponseHeaders(HttpStatus.SC_BAD_REQUEST, BODY_LENGTH_NONE); - break; + return; case "toDisk": if (field.getValue().isBoolean()) { builder = builder.toDisk(field.getValue().asBoolean()); break; } exchange.sendResponseHeaders(HttpStatus.SC_BAD_REQUEST, BODY_LENGTH_NONE); - break; + return; default: log.warn("Unknown recording option {}", field.getKey()); - exchange.sendResponseHeaders( - HttpStatus.SC_BAD_REQUEST, BODY_LENGTH_NONE); - break; + exchange.sendResponseHeaders(HttpStatus.SC_BAD_REQUEST, BODY_LENGTH_NONE); + return; } } svc.updateRecordingOptions(dsc, builder.build()); From 196ee75dbe635c7c30382c0597bc8ef7993c933c Mon Sep 17 00:00:00 2001 From: Andrew Azores Date: Thu, 14 Sep 2023 16:03:58 -0400 Subject: [PATCH 41/45] preserve recording's original settings before updating --- .../java/io/cryostat/agent/remote/RecordingsContext.java | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/main/java/io/cryostat/agent/remote/RecordingsContext.java b/src/main/java/io/cryostat/agent/remote/RecordingsContext.java index d6bcec1f..29d716d5 100644 --- a/src/main/java/io/cryostat/agent/remote/RecordingsContext.java +++ b/src/main/java/io/cryostat/agent/remote/RecordingsContext.java @@ -252,7 +252,13 @@ private void handleStopOrUpdate(HttpExchange exchange, long id) throws IOExcepti .filter(r -> r.getId() == id) .findFirst() .get(); - RecordingOptionsBuilder builder = new RecordingOptionsBuilder(conn.getService()); + RecordingOptionsBuilder builder = + new RecordingOptionsBuilder(conn.getService()) + .name(dsc.getName()) + .duration(dsc.getDuration()) + .maxSize(dsc.getMaxSize()) + .maxAge(dsc.getMaxAge()) + .toDisk(dsc.getToDisk()); InputStream body = exchange.getRequestBody(); JsonNode jsonMap = mapper.readTree(body); From c3ce2d570f9163d63c3739059a930fed2186ee56 Mon Sep 17 00:00:00 2001 From: Andrew Azores Date: Thu, 14 Sep 2023 16:33:24 -0400 Subject: [PATCH 42/45] apply state changes (recording stop) last after other updates --- .../agent/remote/RecordingsContext.java | 71 ++++++++++--------- 1 file changed, 37 insertions(+), 34 deletions(-) diff --git a/src/main/java/io/cryostat/agent/remote/RecordingsContext.java b/src/main/java/io/cryostat/agent/remote/RecordingsContext.java index 29d716d5..08ead8ac 100644 --- a/src/main/java/io/cryostat/agent/remote/RecordingsContext.java +++ b/src/main/java/io/cryostat/agent/remote/RecordingsContext.java @@ -25,6 +25,7 @@ import java.util.List; import java.util.Map; import java.util.Map.Entry; +import java.util.Optional; import java.util.Set; import java.util.function.Consumer; import java.util.regex.Matcher; @@ -260,25 +261,27 @@ private void handleStopOrUpdate(HttpExchange exchange, long id) throws IOExcepti .maxAge(dsc.getMaxAge()) .toDisk(dsc.getToDisk()); + boolean shouldStop = false; InputStream body = exchange.getRequestBody(); JsonNode jsonMap = mapper.readTree(body); Iterator> fields = jsonMap.fields(); while (fields.hasNext()) { Map.Entry field = fields.next(); + log.info("Processing {0}={1}", field.getKey(), field.getValue().toPrettyString()); switch (field.getKey()) { case "state": if ("STOPPED".equals(field.getValue().textValue())) { - handleStop(exchange, id); - } else { - exchange.sendResponseHeaders( - HttpStatus.SC_BAD_REQUEST, BODY_LENGTH_NONE); - return; + log.info("shouldStop = true"); + shouldStop = true; + break; } - break; + exchange.sendResponseHeaders(HttpStatus.SC_BAD_REQUEST, BODY_LENGTH_NONE); + return; case "name": if (!StringUtils.isBlank(field.getValue().textValue())) { + log.info("name = {0}", field.getValue().textValue()); builder = builder.name(field.getValue().textValue()); break; } @@ -286,28 +289,32 @@ private void handleStopOrUpdate(HttpExchange exchange, long id) throws IOExcepti return; case "duration": if (field.getValue().canConvertToLong()) { - builder = builder.duration(field.getValue().asLong()); + log.info("duration = {0}", field.getValue().longValue()); + builder = builder.duration(field.getValue().longValue()); break; } exchange.sendResponseHeaders(HttpStatus.SC_BAD_REQUEST, BODY_LENGTH_NONE); return; case "maxSize": if (field.getValue().canConvertToLong()) { - builder = builder.maxSize(field.getValue().asLong()); + log.info("maxSize = {0}", field.getValue().longValue()); + builder = builder.maxSize(field.getValue().longValue()); break; } exchange.sendResponseHeaders(HttpStatus.SC_BAD_REQUEST, BODY_LENGTH_NONE); return; case "maxAge": if (field.getValue().canConvertToLong()) { - builder = builder.maxAge(field.getValue().asLong()); + log.info("maxAge = {0}", field.getValue().longValue()); + builder = builder.maxAge(field.getValue().longValue()); break; } exchange.sendResponseHeaders(HttpStatus.SC_BAD_REQUEST, BODY_LENGTH_NONE); return; case "toDisk": if (field.getValue().isBoolean()) { - builder = builder.toDisk(field.getValue().asBoolean()); + log.info("toDisk = {0}", field.getValue().booleanValue()); + builder = builder.toDisk(field.getValue().booleanValue()); break; } exchange.sendResponseHeaders(HttpStatus.SC_BAD_REQUEST, BODY_LENGTH_NONE); @@ -319,12 +326,15 @@ private void handleStopOrUpdate(HttpExchange exchange, long id) throws IOExcepti } } svc.updateRecordingOptions(dsc, builder.build()); - exchange.sendResponseHeaders(HttpStatus.SC_OK, BODY_LENGTH_UNKNOWN); + if (shouldStop) { + svc.stop(dsc); + } try (OutputStream response = exchange.getResponseBody()) { if (response == null) { exchange.sendResponseHeaders(HttpStatus.SC_NO_CONTENT, BODY_LENGTH_NONE); } else { + exchange.sendResponseHeaders(HttpStatus.SC_OK, BODY_LENGTH_UNKNOWN); mapper.writeValue(response, dsc); } } @@ -338,24 +348,6 @@ private void handleStopOrUpdate(HttpExchange exchange, long id) throws IOExcepti } } - private void handleStop(HttpExchange exchange, long id) throws IOException { - invokeOnRecording( - exchange, - id, - r -> { - try { - boolean stopped = r.stop(); - if (!stopped) { - sendHeader(exchange, HttpStatus.SC_BAD_REQUEST); - } else { - sendHeader(exchange, HttpStatus.SC_NO_CONTENT); - } - } catch (IllegalStateException e) { - sendHeader(exchange, HttpStatus.SC_CONFLICT); - } - }); - } - private void handleDelete(HttpExchange exchange, long id) throws IOException { invokeOnRecording( exchange, @@ -366,12 +358,23 @@ private void handleDelete(HttpExchange exchange, long id) throws IOException { }); } - private void invokeOnRecording(HttpExchange exchange, long id, Consumer consumer) { - FlightRecorder.getFlightRecorder().getRecordings().stream() + private Optional getRecordingById(long id) { + return FlightRecorder.getFlightRecorder().getRecordings().stream() .filter(r -> r.getId() == id) - .findFirst() - .ifPresentOrElse( - consumer::accept, () -> sendHeader(exchange, HttpStatus.SC_NOT_FOUND)); + .findFirst(); + } + + private void invokeOnRecording(HttpExchange exchange, long id, Consumer consumer) { + Optional opt = getRecordingById(id); + if (!opt.isPresent()) { + sendHeader(exchange, HttpStatus.SC_NOT_FOUND); + } + try { + consumer.accept(opt.get()); + } catch (Exception e) { + log.error("Operation failed", e); + sendHeader(exchange, HttpStatus.SC_INTERNAL_SERVER_ERROR); + } } private void sendHeader(HttpExchange exchange, int status) { From 98b54c67a5c1f8cb288fa328c56887e0a91e963b Mon Sep 17 00:00:00 2001 From: Andrew Azores Date: Thu, 14 Sep 2023 17:08:35 -0400 Subject: [PATCH 43/45] only re-set duration setting if the recording is not already stopped --- .../java/io/cryostat/agent/remote/RecordingsContext.java | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/main/java/io/cryostat/agent/remote/RecordingsContext.java b/src/main/java/io/cryostat/agent/remote/RecordingsContext.java index 08ead8ac..31258037 100644 --- a/src/main/java/io/cryostat/agent/remote/RecordingsContext.java +++ b/src/main/java/io/cryostat/agent/remote/RecordingsContext.java @@ -42,6 +42,7 @@ import org.openjdk.jmc.rjmx.ServiceNotAvailableException; import org.openjdk.jmc.rjmx.services.jfr.IFlightRecorderService; import org.openjdk.jmc.rjmx.services.jfr.IRecordingDescriptor; +import org.openjdk.jmc.rjmx.services.jfr.IRecordingDescriptor.RecordingState; import io.cryostat.agent.StringUtils; import io.cryostat.core.FlightRecorderException; @@ -256,10 +257,15 @@ private void handleStopOrUpdate(HttpExchange exchange, long id) throws IOExcepti RecordingOptionsBuilder builder = new RecordingOptionsBuilder(conn.getService()) .name(dsc.getName()) - .duration(dsc.getDuration()) .maxSize(dsc.getMaxSize()) .maxAge(dsc.getMaxAge()) .toDisk(dsc.getToDisk()); + if (!RecordingState.STOPPED.equals(dsc.getState())) { + // we should apply this default, but if this key is present when the recording is + // stopped then updating the recording options will throw, even if we are re-setting + // the same duration + builder = builder.duration(dsc.getDuration()); + } boolean shouldStop = false; InputStream body = exchange.getRequestBody(); From 9317c9b172a15d61ad55eba7786a80c02e95be07 Mon Sep 17 00:00:00 2001 From: Andrew Azores Date: Thu, 14 Sep 2023 17:09:20 -0400 Subject: [PATCH 44/45] remove troubleshooting logging --- .../java/io/cryostat/agent/remote/RecordingsContext.java | 7 ------- 1 file changed, 7 deletions(-) diff --git a/src/main/java/io/cryostat/agent/remote/RecordingsContext.java b/src/main/java/io/cryostat/agent/remote/RecordingsContext.java index 31258037..7333e8e4 100644 --- a/src/main/java/io/cryostat/agent/remote/RecordingsContext.java +++ b/src/main/java/io/cryostat/agent/remote/RecordingsContext.java @@ -274,12 +274,10 @@ private void handleStopOrUpdate(HttpExchange exchange, long id) throws IOExcepti while (fields.hasNext()) { Map.Entry field = fields.next(); - log.info("Processing {0}={1}", field.getKey(), field.getValue().toPrettyString()); switch (field.getKey()) { case "state": if ("STOPPED".equals(field.getValue().textValue())) { - log.info("shouldStop = true"); shouldStop = true; break; } @@ -287,7 +285,6 @@ private void handleStopOrUpdate(HttpExchange exchange, long id) throws IOExcepti return; case "name": if (!StringUtils.isBlank(field.getValue().textValue())) { - log.info("name = {0}", field.getValue().textValue()); builder = builder.name(field.getValue().textValue()); break; } @@ -295,7 +292,6 @@ private void handleStopOrUpdate(HttpExchange exchange, long id) throws IOExcepti return; case "duration": if (field.getValue().canConvertToLong()) { - log.info("duration = {0}", field.getValue().longValue()); builder = builder.duration(field.getValue().longValue()); break; } @@ -303,7 +299,6 @@ private void handleStopOrUpdate(HttpExchange exchange, long id) throws IOExcepti return; case "maxSize": if (field.getValue().canConvertToLong()) { - log.info("maxSize = {0}", field.getValue().longValue()); builder = builder.maxSize(field.getValue().longValue()); break; } @@ -311,7 +306,6 @@ private void handleStopOrUpdate(HttpExchange exchange, long id) throws IOExcepti return; case "maxAge": if (field.getValue().canConvertToLong()) { - log.info("maxAge = {0}", field.getValue().longValue()); builder = builder.maxAge(field.getValue().longValue()); break; } @@ -319,7 +313,6 @@ private void handleStopOrUpdate(HttpExchange exchange, long id) throws IOExcepti return; case "toDisk": if (field.getValue().isBoolean()) { - log.info("toDisk = {0}", field.getValue().booleanValue()); builder = builder.toDisk(field.getValue().booleanValue()); break; } From 53339645b2b1dfc1db49191ae4b986b3ed1b1589 Mon Sep 17 00:00:00 2001 From: Andrew Azores Date: Thu, 14 Sep 2023 17:09:32 -0400 Subject: [PATCH 45/45] handle STOPPED state request case-insensitively --- src/main/java/io/cryostat/agent/remote/RecordingsContext.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/io/cryostat/agent/remote/RecordingsContext.java b/src/main/java/io/cryostat/agent/remote/RecordingsContext.java index 7333e8e4..23830b87 100644 --- a/src/main/java/io/cryostat/agent/remote/RecordingsContext.java +++ b/src/main/java/io/cryostat/agent/remote/RecordingsContext.java @@ -277,7 +277,7 @@ private void handleStopOrUpdate(HttpExchange exchange, long id) throws IOExcepti switch (field.getKey()) { case "state": - if ("STOPPED".equals(field.getValue().textValue())) { + if ("stopped".equalsIgnoreCase(field.getValue().textValue())) { shouldStop = true; break; }