Skip to content

Commit

Permalink
fix(credentials): store credentials with unique matchExpression (#156)
Browse files Browse the repository at this point in the history
* fix(credentials): store credentials with expr matching callback and jvmId

* generate UUID for instance, include in alias/realm as well as platform annotation

* do not try to delete credentials if they have not yet been submitted

* document new config

* fix incorrect doc description
  • Loading branch information
andrewazores authored Jun 21, 2023
1 parent 34cdd78 commit 09987e0
Show file tree
Hide file tree
Showing 6 changed files with 42 additions and 5 deletions.
3 changes: 2 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,9 @@ 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.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 includes the `cryostat.agent.app.name` and a random UUID.
- [ ] `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`.
- [ ] `cryostat.agent.authorization` [`String`]: Authorization header value to include with API requests to the Cryostat server, ex. `Bearer abcd1234`. Default `None`.
- [ ] `cryostat.agent.webclient.ssl.trust-all` [`boolean`]: Control whether the agent trusts all certificates presented by the Cryostat server. Default `false`. This should only be overridden for development and testing purposes, never in production.
- [ ] `cryostat.agent.webclient.ssl.verify-hostname` [`boolean`]: Control whether the agent verifies hostnames on certificates presented by the Cryostat server. Default `true`. This should only be overridden for development and testing purposes, never in production.
Expand Down
10 changes: 10 additions & 0 deletions src/main/java/io/cryostat/agent/ConfigModule.java
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
import java.net.UnknownHostException;
import java.util.Arrays;
import java.util.List;
import java.util.UUID;

import javax.inject.Named;
import javax.inject.Singleton;
Expand All @@ -58,6 +59,7 @@ public abstract class ConfigModule {

private static final Logger log = LoggerFactory.getLogger(ConfigModule.class);

public static final String CRYOSTAT_AGENT_INSTANCE_ID = "cryostat.agent.instance-id";
public static final String CRYOSTAT_AGENT_BASEURI = "cryostat.agent.baseuri";
public static final String CRYOSTAT_AGENT_CALLBACK = "cryostat.agent.callback";
public static final String CRYOSTAT_AGENT_REALM = "cryostat.agent.realm";
Expand Down Expand Up @@ -182,6 +184,14 @@ public static int provideCryostatAgentWebserverPort(SmallRyeConfig config) {
return config.getValue(CRYOSTAT_AGENT_WEBSERVER_PORT, int.class);
}

@Provides
@Singleton
@Named(CRYOSTAT_AGENT_INSTANCE_ID)
public static String provideCryostatAgentInstanceId(SmallRyeConfig config) {
return config.getOptionalValue(CRYOSTAT_AGENT_INSTANCE_ID, String.class)
.orElseGet(() -> UUID.randomUUID().toString());
}

@Provides
@Singleton
@Named(CRYOSTAT_AGENT_APP_NAME)
Expand Down
11 changes: 10 additions & 1 deletion src/main/java/io/cryostat/agent/CryostatClient.java
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ public class CryostatClient {
private final HttpClient http;

private final String appName;
private final String instanceId;
private final String jvmId;
private final URI baseUri;
private final String realm;
Expand All @@ -102,13 +103,15 @@ public class CryostatClient {
Executor executor,
ObjectMapper mapper,
HttpClient http,
String instanceId,
String jvmId,
String appName,
URI baseUri,
String realm) {
this.executor = executor;
this.mapper = mapper;
this.http = http;
this.instanceId = instanceId;
this.jvmId = jvmId;
this.appName = appName;
this.baseUri = baseUri;
Expand Down Expand Up @@ -224,6 +227,9 @@ private CompletableFuture<Integer> submitCredentials(Credentials credentials, UR
}

public CompletableFuture<Void> deleteCredentials(int id) {
if (id < 0) {
return CompletableFuture.completedFuture(null);
}
HttpDelete req = new HttpDelete(baseUri.resolve(CREDENTIALS_API_PATH + "/" + id));
log.info("{}", req);
return supply(req, (res) -> logResponse(req, res))
Expand Down Expand Up @@ -354,7 +360,10 @@ private CountingInputStream getRecordingInputStream(Path filePath) throws IOExce
}

private String selfMatchExpression(URI callback) {
return String.format("target.connectUrl == \"%s\"", callback);
return String.format(
"target.connectUrl == \"%s\" && target.jvmId == \"%s\" &&"
+ " target.annotations.platform[\"INSTANCE_ID\"] == \"%s\"",
callback, jvmId, instanceId);
}

private boolean isOkStatus(HttpResponse res) {
Expand Down
6 changes: 5 additions & 1 deletion src/main/java/io/cryostat/agent/MainModule.java
Original file line number Diff line number Diff line change
Expand Up @@ -201,12 +201,14 @@ public static CryostatClient provideCryostatClient(
ScheduledExecutorService executor,
ObjectMapper objectMapper,
HttpClient http,
@Named(ConfigModule.CRYOSTAT_AGENT_INSTANCE_ID) String instanceId,
@Named(JVM_ID) String jvmId,
@Named(ConfigModule.CRYOSTAT_AGENT_APP_NAME) String appName,
@Named(ConfigModule.CRYOSTAT_AGENT_BASEURI) URI baseUri,
@Named(ConfigModule.CRYOSTAT_AGENT_REALM) String realm,
@Named(ConfigModule.CRYOSTAT_AGENT_AUTHORIZATION) String authorization) {
return new CryostatClient(executor, objectMapper, http, jvmId, appName, baseUri, realm);
return new CryostatClient(
executor, objectMapper, http, instanceId, jvmId, appName, baseUri, realm);
}

@Provides
Expand All @@ -216,6 +218,7 @@ public static Registration provideRegistration(
CryostatClient cryostat,
@Named(ConfigModule.CRYOSTAT_AGENT_CALLBACK) URI callback,
WebServer webServer,
@Named(ConfigModule.CRYOSTAT_AGENT_INSTANCE_ID) String instanceId,
@Named(JVM_ID) String jvmId,
@Named(ConfigModule.CRYOSTAT_AGENT_APP_NAME) String appName,
@Named(ConfigModule.CRYOSTAT_AGENT_REALM) String realm,
Expand All @@ -229,6 +232,7 @@ public static Registration provideRegistration(
cryostat,
callback,
webServer,
instanceId,
jvmId,
appName,
realm,
Expand Down
14 changes: 13 additions & 1 deletion src/main/java/io/cryostat/agent/Registration.java
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ class Registration {
private final CryostatClient cryostat;
private final URI callback;
private final WebServer webServer;
private final String instanceId;
private final String jvmId;
private final String appName;
private final String realm;
Expand All @@ -89,6 +90,7 @@ class Registration {
CryostatClient cryostat,
URI callback,
WebServer webServer,
String instanceId,
String jvmId,
String appName,
String realm,
Expand All @@ -101,6 +103,7 @@ class Registration {
this.cryostat = cryostat;
this.callback = callback;
this.webServer = webServer;
this.instanceId = instanceId;
this.jvmId = jvmId;
this.appName = appName;
this.realm = realm;
Expand Down Expand Up @@ -302,7 +305,16 @@ private DiscoveryNode defineSelf() throws UnknownHostException, URISyntaxExcepti
}
DiscoveryNode.Target target =
new DiscoveryNode.Target(
realm, uri, appName, jvmId, pid, hostname, port, javaMain, startTime);
realm,
uri,
appName,
instanceId,
jvmId,
pid,
hostname,
port,
javaMain,
startTime);

DiscoveryNode selfNode =
new DiscoveryNode(appName + "-" + pluginInfo.getId(), NODE_TYPE, target);
Expand Down
3 changes: 2 additions & 1 deletion src/main/java/io/cryostat/agent/model/DiscoveryNode.java
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ public Target(
String realm,
URI connectUrl,
String alias,
String instanceId,
String jvmId,
long pid,
String hostname,
Expand All @@ -109,7 +110,7 @@ public Target(
this.connectUrl = connectUrl;
this.alias = alias;
this.annotations = new Annotations();
annotations.setPlatform(Map.of());
annotations.setPlatform(Map.of("INSTANCE_ID", instanceId));
annotations.setCryostat(
Map.of(
"REALM",
Expand Down

0 comments on commit 09987e0

Please sign in to comment.