From daf2e4bf27a2aa0b394191c25b20db4d0de1049e Mon Sep 17 00:00:00 2001 From: Mickael Maison Date: Fri, 19 Jul 2024 17:57:20 +0200 Subject: [PATCH] Improve parsing of allowlist Signed-off-by: Mickael Maison --- .../PrometheusMetricsReporterConfig.java | 11 +++- .../PrometheusMetricsReporterConfigTest.java | 57 +++++++++++-------- 2 files changed, 42 insertions(+), 26 deletions(-) diff --git a/src/main/java/io/strimzi/kafka/metrics/PrometheusMetricsReporterConfig.java b/src/main/java/io/strimzi/kafka/metrics/PrometheusMetricsReporterConfig.java index 1a8c745..94066d5 100644 --- a/src/main/java/io/strimzi/kafka/metrics/PrometheusMetricsReporterConfig.java +++ b/src/main/java/io/strimzi/kafka/metrics/PrometheusMetricsReporterConfig.java @@ -20,6 +20,8 @@ import java.util.Optional; import java.util.regex.Matcher; import java.util.regex.Pattern; +import java.util.regex.PatternSyntaxException; +import java.util.stream.Collectors; /** * Configuration for the PrometheusMetricsReporter implementation. @@ -97,7 +99,14 @@ public boolean isAllowed(String name) { } private Pattern compileAllowlist(List allowlist) { - String joined = String.join("|", allowlist); + for (String entry : allowlist) { + try { + Pattern.compile(entry); + } catch (PatternSyntaxException pse) { + throw new ConfigException("Invalid regex pattern found in " + ALLOWLIST_CONFIG + ": " + entry); + } + } + String joined = allowlist.stream().map(v -> "(" + v + ")").collect(Collectors.joining("|")); return Pattern.compile(joined); } diff --git a/src/test/java/io/strimzi/kafka/metrics/PrometheusMetricsReporterConfigTest.java b/src/test/java/io/strimzi/kafka/metrics/PrometheusMetricsReporterConfigTest.java index a784e76..dd3b9b5 100644 --- a/src/test/java/io/strimzi/kafka/metrics/PrometheusMetricsReporterConfigTest.java +++ b/src/test/java/io/strimzi/kafka/metrics/PrometheusMetricsReporterConfigTest.java @@ -9,11 +9,15 @@ import org.apache.kafka.common.config.ConfigException; import org.junit.jupiter.api.Test; -import java.util.Collections; import java.util.HashMap; import java.util.Map; import java.util.Optional; +import static io.strimzi.kafka.metrics.PrometheusMetricsReporterConfig.ALLOWLIST_CONFIG; +import static io.strimzi.kafka.metrics.PrometheusMetricsReporterConfig.LISTENER_CONFIG; +import static io.strimzi.kafka.metrics.PrometheusMetricsReporterConfig.LISTENER_ENABLE_CONFIG; +import static java.util.Collections.emptyMap; +import static java.util.Collections.singletonMap; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertThrows; @@ -23,7 +27,7 @@ public class PrometheusMetricsReporterConfigTest { @Test public void testDefaults() { - PrometheusMetricsReporterConfig config = new PrometheusMetricsReporterConfig(Collections.emptyMap(), new PrometheusRegistry()); + PrometheusMetricsReporterConfig config = new PrometheusMetricsReporterConfig(emptyMap(), new PrometheusRegistry()); assertEquals(PrometheusMetricsReporterConfig.LISTENER_CONFIG_DEFAULT, config.listener()); assertTrue(config.isAllowed("random_name")); } @@ -31,8 +35,8 @@ public void testDefaults() { @Test public void testOverrides() { Map props = new HashMap<>(); - props.put(PrometheusMetricsReporterConfig.LISTENER_CONFIG, "http://:0"); - props.put(PrometheusMetricsReporterConfig.ALLOWLIST_CONFIG, "kafka_server.*"); + props.put(LISTENER_CONFIG, "http://:0"); + props.put(ALLOWLIST_CONFIG, "kafka_server.*"); PrometheusMetricsReporterConfig config = new PrometheusMetricsReporterConfig(props, new PrometheusRegistry()); assertEquals("http://:0", config.listener()); @@ -42,13 +46,17 @@ public void testOverrides() { @Test public void testAllowList() { - Map props = new HashMap<>(); - props.put(PrometheusMetricsReporterConfig.ALLOWLIST_CONFIG, "kafka_server.*,kafka_network.*"); + Map props = singletonMap(ALLOWLIST_CONFIG, "kafka_server.*,kafka_network.*"); PrometheusMetricsReporterConfig config = new PrometheusMetricsReporterConfig(props, new PrometheusRegistry()); assertFalse(config.isAllowed("random_name")); assertTrue(config.isAllowed("kafka_server_metric")); assertTrue(config.isAllowed("kafka_network_metric")); + + assertThrows(ConfigException.class, + () -> new PrometheusMetricsReporterConfig(singletonMap(ALLOWLIST_CONFIG, "hell[o,s]world"), null)); + assertThrows(ConfigException.class, + () -> new PrometheusMetricsReporterConfig(singletonMap(ALLOWLIST_CONFIG, "hello\\,world"), null)); } @Test @@ -73,28 +81,28 @@ public void testListenerParseListener() { @Test public void testValidator() { PrometheusMetricsReporterConfig.ListenerValidator validator = new PrometheusMetricsReporterConfig.ListenerValidator(); - validator.ensureValid(PrometheusMetricsReporterConfig.LISTENER_CONFIG, "http://:0"); - validator.ensureValid(PrometheusMetricsReporterConfig.LISTENER_CONFIG, "http://123:8080"); - validator.ensureValid(PrometheusMetricsReporterConfig.LISTENER_CONFIG, "http://::1:8080"); - validator.ensureValid(PrometheusMetricsReporterConfig.LISTENER_CONFIG, "http://[::1]:8080"); - validator.ensureValid(PrometheusMetricsReporterConfig.LISTENER_CONFIG, "http://random:8080"); - - assertThrows(ConfigException.class, () -> validator.ensureValid(PrometheusMetricsReporterConfig.LISTENER_CONFIG, "http")); - assertThrows(ConfigException.class, () -> validator.ensureValid(PrometheusMetricsReporterConfig.LISTENER_CONFIG, "http://")); - assertThrows(ConfigException.class, () -> validator.ensureValid(PrometheusMetricsReporterConfig.LISTENER_CONFIG, "http://random")); - assertThrows(ConfigException.class, () -> validator.ensureValid(PrometheusMetricsReporterConfig.LISTENER_CONFIG, "http://random:")); - assertThrows(ConfigException.class, () -> validator.ensureValid(PrometheusMetricsReporterConfig.LISTENER_CONFIG, "http://:-8080")); - assertThrows(ConfigException.class, () -> validator.ensureValid(PrometheusMetricsReporterConfig.LISTENER_CONFIG, "http://random:-8080")); - assertThrows(ConfigException.class, () -> validator.ensureValid(PrometheusMetricsReporterConfig.LISTENER_CONFIG, "http://:8080random")); - assertThrows(ConfigException.class, () -> validator.ensureValid(PrometheusMetricsReporterConfig.LISTENER_CONFIG, "randomhttp://:8080random")); - assertThrows(ConfigException.class, () -> validator.ensureValid(PrometheusMetricsReporterConfig.LISTENER_CONFIG, "randomhttp://:8080")); + validator.ensureValid(LISTENER_CONFIG, "http://:0"); + validator.ensureValid(LISTENER_CONFIG, "http://123:8080"); + validator.ensureValid(LISTENER_CONFIG, "http://::1:8080"); + validator.ensureValid(LISTENER_CONFIG, "http://[::1]:8080"); + validator.ensureValid(LISTENER_CONFIG, "http://random:8080"); + + assertThrows(ConfigException.class, () -> validator.ensureValid(LISTENER_CONFIG, "http")); + assertThrows(ConfigException.class, () -> validator.ensureValid(LISTENER_CONFIG, "http://")); + assertThrows(ConfigException.class, () -> validator.ensureValid(LISTENER_CONFIG, "http://random")); + assertThrows(ConfigException.class, () -> validator.ensureValid(LISTENER_CONFIG, "http://random:")); + assertThrows(ConfigException.class, () -> validator.ensureValid(LISTENER_CONFIG, "http://:-8080")); + assertThrows(ConfigException.class, () -> validator.ensureValid(LISTENER_CONFIG, "http://random:-8080")); + assertThrows(ConfigException.class, () -> validator.ensureValid(LISTENER_CONFIG, "http://:8080random")); + assertThrows(ConfigException.class, () -> validator.ensureValid(LISTENER_CONFIG, "randomhttp://:8080random")); + assertThrows(ConfigException.class, () -> validator.ensureValid(LISTENER_CONFIG, "randomhttp://:8080")); } @Test public void testIsListenerEnabled() { Map props = new HashMap<>(); - props.put(PrometheusMetricsReporterConfig.LISTENER_ENABLE_CONFIG, "true"); - props.put(PrometheusMetricsReporterConfig.LISTENER_CONFIG, "http://:0"); + props.put(LISTENER_ENABLE_CONFIG, "true"); + props.put(LISTENER_CONFIG, "http://:0"); PrometheusMetricsReporterConfig config = new PrometheusMetricsReporterConfig(props, new PrometheusRegistry()); Optional httpServerOptional = config.startHttpServer(); @@ -105,8 +113,7 @@ public void testIsListenerEnabled() { @Test public void testIsListenerDisabled() { - Map props = new HashMap<>(); - props.put(PrometheusMetricsReporterConfig.LISTENER_ENABLE_CONFIG, false); + Map props = singletonMap(LISTENER_ENABLE_CONFIG, false); PrometheusMetricsReporterConfig config = new PrometheusMetricsReporterConfig(props, new PrometheusRegistry()); Optional httpServerOptional = config.startHttpServer();