Skip to content

Commit

Permalink
reviews
Browse files Browse the repository at this point in the history
  • Loading branch information
@mwangggg
mwangggg committed Aug 7, 2024
1 parent e659972 commit 69dda53
Show file tree
Hide file tree
Showing 3 changed files with 93 additions and 67 deletions.
97 changes: 56 additions & 41 deletions src/main/java/io/cryostat/agent/ConfigModule.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@
import java.util.Optional;
import java.util.UUID;
import java.util.function.Predicate;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
import java.util.stream.Collectors;
import java.util.stream.StreamSupport;

Expand All @@ -43,7 +45,6 @@

import dagger.Module;
import dagger.Provides;

import org.eclipse.microprofile.config.Config;
import org.eclipse.microprofile.config.ConfigProvider;
import org.slf4j.Logger;
Expand Down Expand Up @@ -75,7 +76,7 @@ public abstract class ConfigModule {
"cryostat.agent.webclient.connect.timeout-ms";
public static final String CRYOSTAT_AGENT_WEBCLIENT_RESPONSE_TIMEOUT_MS =
"cryostat.agent.webclient.response.timeout-ms";
public static final String CRYOSTAT_AGENT_TRUSTSTORES = "truststore.cert";
public static final String CRYOSTAT_AGENT_TRUSTSTORES = "cryostat.agent.truststore.cert";

public static final String CRYOSTAT_AGENT_WEBSERVER_HOST = "cryostat.agent.webserver.host";
public static final String CRYOSTAT_AGENT_WEBSERVER_PORT = "cryostat.agent.webserver.port";
Expand Down Expand Up @@ -169,7 +170,7 @@ public static Config provideConfig() {
@Singleton
@Named(CRYOSTAT_AGENT_TRUSTSTORES)
public static List<TruststoreConfig> provideTruststoreConfigs(Config config) {
Map<Integer, TruststoreConfig> truststoreMap = new HashMap<>();
Map<Integer, TruststoreConfig.Builder> truststoreBuilders = new HashMap<>();

List<String> propertyNames =
StreamSupport.stream(config.getPropertyNames().spliterator(), false)
Expand All @@ -178,49 +179,63 @@ public static List<TruststoreConfig> provideTruststoreConfigs(Config config) {

for (String name : propertyNames) {

String[] parts = name.split("\\.");
if (parts.length < 3) {
log.error(
"Invalid truststore config property name format: {}. Rename to"
+ " 'truststore.cert[CERT_NUMBER].CERT_PROPERTY",
name);
continue;
}

int truststoreNumber = 0;
try {
truststoreNumber = Integer.parseInt(parts[1].substring(5, 6));
} catch (IllegalArgumentException e) {
log.error(
"Invalid truststore config property name format: {}. Make sure"
+ " the certificate number is valid",
name);
}
Pattern pattern =
Pattern.compile("^(cryostat\\.agent\\.truststore\\.cert)\\[(\\d+)\\]\\.(.*)$");
Matcher matcher = pattern.matcher(name);

TruststoreConfig truststore =
truststoreMap.computeIfAbsent(
truststoreNumber, k -> new TruststoreConfig(null, null, null));

String value = config.getOptionalValue(name, String.class).orElse(null);
switch (parts[2]) {
case "alias":
truststore.setAlias(value);
break;
case "path":
truststore.setPath(value);
break;
case "type":
truststore.setType(value);
break;
default:
if (matcher.matches()) {
if (matcher.groupCount() < 3) {
log.error(
"Truststore config only includes alias, path, and type. Rename this"
+ " config property: {}",
"Invalid truststore config property name format: {}. Rename to"
+ " 'cryostat.agent.truststore.cert[CERT_NUMBER].CERT_PROPERTY'",
name);
break;
continue;
}

int truststoreNumber = Integer.parseInt(matcher.group(2));
String configProp = matcher.group(3);

TruststoreConfig.Builder truststoreBuilder =
truststoreBuilders.computeIfAbsent(
truststoreNumber, k -> new TruststoreConfig.Builder());
;

String value = config.getValue(name, String.class);
switch (configProp) {
case "alias":
truststoreBuilder = truststoreBuilder.withAlias(value);
break;
case "path":
truststoreBuilder = truststoreBuilder.withPath(value);
break;
case "type":
truststoreBuilder = truststoreBuilder.withType(value);
break;
default:
log.error(
"Truststore config only includes alias, path, and type. Rename this"
+ " config property: {}",
name);
break;
}
} else {
throw new IllegalArgumentException(
String.format(
"Invalid truststore config property name format: %s. Make sure the"
+ " config property matches the following pattern: "
+ " 'cryostat.agent.truststore.cert[CERT_NUMBER].CERT_PROPERTY'",
name));
}
}
List<TruststoreConfig> truststoreConfigs = new ArrayList<>();
for (TruststoreConfig.Builder builder : truststoreBuilders.values()) {
try {
truststoreConfigs.add(builder.build());
} catch (IllegalStateException e) {
log.error("Error building truststore configs: {}", e.getMessage());
}
}
return new ArrayList<>(truststoreMap.values());
return truststoreConfigs;
}

@Provides
Expand Down
7 changes: 0 additions & 7 deletions src/main/java/io/cryostat/agent/MainModule.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,6 @@
import dagger.Lazy;
import dagger.Module;
import dagger.Provides;

import org.apache.commons.io.IOUtils;
import org.apache.commons.lang3.exception.ExceptionUtils;
import org.apache.http.Header;
Expand Down Expand Up @@ -161,12 +160,6 @@ public static SSLContext provideClientSslContext(
String certAlias = truststore.getAlias();
String certPath = truststore.getPath();

if (certType == null || certAlias == null || certPath == null) {
throw new IllegalArgumentException(
"The truststore config properties must include a type, alias, and"
+ " path for each certificate provided.");
}

// load truststore with certificatesCertificate
InputStream certFile = new FileInputStream(certPath);
CertificateFactory cf = CertificateFactory.getInstance(certType);
Expand Down
56 changes: 37 additions & 19 deletions src/main/java/io/cryostat/agent/TruststoreConfig.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,37 +16,55 @@
package io.cryostat.agent;

public class TruststoreConfig {
private String alias;
private String path;
private String type;

public TruststoreConfig(String alias, String path, String type) {
this.alias = alias;
this.path = path;
this.type = type;
}

public String getPath() {
return this.path;
}
private final String alias;
private final String path;
private final String type;

public void setPath(String path) {
this.path = path;
public TruststoreConfig(Builder builder) {
this.alias = builder.alias;
this.path = builder.path;
this.type = builder.type;
}

public String getAlias() {
return this.alias;
}

public void setAlias(String alias) {
this.alias = alias;
public String getPath() {
return this.path;
}

public String getType() {
return this.type;
}

public void setType(String type) {
this.type = type;
public static class Builder {
private String alias;
private String path;
private String type;

public Builder withAlias(String alias) {
this.alias = alias;
return this;
}

public Builder withPath(String path) {
this.path = path;
return this;
}

public Builder withType(String type) {
this.type = type;
return this;
}

public TruststoreConfig build() {
if (this.alias == null || this.path == null || this.type == null) {
throw new IllegalArgumentException(
"The truststore config properties must include a type, alias, and"
+ " path for each certificate provided");
}
return new TruststoreConfig(this);
}
}
}

0 comments on commit 69dda53

Please sign in to comment.