This is an automated email from the ASF dual-hosted git repository.
anmolnar pushed a commit to branch branch-3.9
in repository https://gitbox.apache.org/repos/asf/zookeeper.git
The following commit(s) were added to refs/heads/branch-3.9 by this push:
new 91805141e ZOOKEEPER-5049: Redact passwords from
PrometheusMetricsProvider configuration logging
91805141e is described below
commit 91805141e3decaa21d6165249af35fc1b957fa2f
Author: Dávid Paksy <[email protected]>
AuthorDate: Thu Jun 18 15:54:37 2026 +0200
ZOOKEEPER-5049: Redact passwords from PrometheusMetricsProvider
configuration logging
Reviewers: meszibalu, anmolnar
Author: PDavid
Closes #2392 from
PDavid/ZOOKEEPER-5049-PrometheusMetricsProvider-log-redact-branch-3.9
---
.../zookeeper-prometheus-metrics/pom.xml | 5 ++
.../prometheus/PrometheusMetricsProvider.java | 3 +-
.../PrometheusMetricsProviderConfigTest.java | 39 +++++++-
.../org/apache/zookeeper/common/LogRedactor.java | 62 +++++++++++++
.../java/org/apache/zookeeper/common/ZKConfig.java | 21 +----
.../apache/zookeeper/common/LogRedactorTest.java | 100 +++++++++++++++++++++
6 files changed, 211 insertions(+), 19 deletions(-)
diff --git a/zookeeper-metrics-providers/zookeeper-prometheus-metrics/pom.xml
b/zookeeper-metrics-providers/zookeeper-prometheus-metrics/pom.xml
index 0d183fd7b..03d21fc75 100755
--- a/zookeeper-metrics-providers/zookeeper-prometheus-metrics/pom.xml
+++ b/zookeeper-metrics-providers/zookeeper-prometheus-metrics/pom.xml
@@ -84,6 +84,11 @@
<artifactId>junit-platform-runner</artifactId>
<scope>test</scope>
</dependency>
+ <dependency>
+ <groupId>ch.qos.logback</groupId>
+ <artifactId>logback-classic</artifactId>
+ <scope>test</scope>
+ </dependency>
</dependencies>
<build>
diff --git
a/zookeeper-metrics-providers/zookeeper-prometheus-metrics/src/main/java/org/apache/zookeeper/metrics/prometheus/PrometheusMetricsProvider.java
b/zookeeper-metrics-providers/zookeeper-prometheus-metrics/src/main/java/org/apache/zookeeper/metrics/prometheus/PrometheusMetricsProvider.java
index af4bff9d2..89d4a3113 100644
---
a/zookeeper-metrics-providers/zookeeper-prometheus-metrics/src/main/java/org/apache/zookeeper/metrics/prometheus/PrometheusMetricsProvider.java
+++
b/zookeeper-metrics-providers/zookeeper-prometheus-metrics/src/main/java/org/apache/zookeeper/metrics/prometheus/PrometheusMetricsProvider.java
@@ -18,6 +18,7 @@
package org.apache.zookeeper.metrics.prometheus;
+import static org.apache.zookeeper.common.LogRedactor.redactSensitiveValues;
import io.prometheus.client.Collector;
import io.prometheus.client.CollectorRegistry;
import io.prometheus.client.exporter.MetricsServlet;
@@ -113,7 +114,7 @@ public class PrometheusMetricsProvider implements
MetricsProvider {
@Override
public void configure(Properties configuration) throws
MetricsProviderLifeCycleException {
- LOG.info("Initializing metrics, configuration: {}", configuration);
+ LOG.info("Initializing metrics, configuration: {}",
redactSensitiveValues(configuration));
this.host = configuration.getProperty("httpHost", "0.0.0.0");
this.port = Integer.parseInt(configuration.getProperty("httpPort",
"7000"));
this.exportJvmInfo =
Boolean.parseBoolean(configuration.getProperty("exportJvmInfo", "true"));
diff --git
a/zookeeper-metrics-providers/zookeeper-prometheus-metrics/src/test/java/org/apache/zookeeper/metrics/prometheus/PrometheusMetricsProviderConfigTest.java
b/zookeeper-metrics-providers/zookeeper-prometheus-metrics/src/test/java/org/apache/zookeeper/metrics/prometheus/PrometheusMetricsProviderConfigTest.java
index 54c320d69..44c466c9e 100644
---
a/zookeeper-metrics-providers/zookeeper-prometheus-metrics/src/test/java/org/apache/zookeeper/metrics/prometheus/PrometheusMetricsProviderConfigTest.java
+++
b/zookeeper-metrics-providers/zookeeper-prometheus-metrics/src/test/java/org/apache/zookeeper/metrics/prometheus/PrometheusMetricsProviderConfigTest.java
@@ -18,12 +18,17 @@
package org.apache.zookeeper.metrics.prometheus;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import ch.qos.logback.classic.Logger;
+import ch.qos.logback.classic.spi.ILoggingEvent;
+import ch.qos.logback.core.read.ListAppender;
import io.prometheus.client.CollectorRegistry;
import java.util.Properties;
import org.apache.zookeeper.metrics.MetricsProviderLifeCycleException;
import org.junit.Assert;
import org.junit.Test;
-
+import org.slf4j.LoggerFactory;
public class PrometheusMetricsProviderConfigTest {
@@ -63,4 +68,36 @@ public void testValidConfig() throws
MetricsProviderLifeCycleException {
provider.start();
}
+ @Test
+ public void testLogRedactorRedactsPasswords() throws Exception {
+ Logger logger = (Logger)
LoggerFactory.getLogger(PrometheusMetricsProvider.class);
+ ListAppender<ILoggingEvent> listAppender = new ListAppender<>();
+ listAppender.start();
+ logger.addAppender(listAppender);
+
+ try {
+ PrometheusMetricsProvider provider = new
PrometheusMetricsProvider();
+ Properties configuration = new Properties();
+ configuration.setProperty("httpHost", "0.0.0.0");
+ configuration.setProperty("httpPort", "65536");
+ configuration.setProperty("some.password", "SuperSecret123!");
+ configuration.setProperty("some-other.password",
"AnotherSecret456!");
+ provider.configure(configuration);
+
+ String logOutput = listAppender.list.stream()
+ .map(ILoggingEvent::getFormattedMessage)
+ .filter(msg -> msg.contains("configuration"))
+ .findFirst()
+ .orElse("");
+
+ assertFalse(logOutput.contains("SuperSecret123!"),
+ "Logs should not contain some password");
+ assertFalse(logOutput.contains("AnotherSecret456!"),
+ "Logs should not contain other password");
+ assertTrue(logOutput.contains("0.0.0.0"),
+ "Logs should still contain non-sensitive config like
host");
+ } finally {
+ logger.detachAppender(listAppender);
+ }
+ }
}
diff --git
a/zookeeper-server/src/main/java/org/apache/zookeeper/common/LogRedactor.java
b/zookeeper-server/src/main/java/org/apache/zookeeper/common/LogRedactor.java
new file mode 100644
index 000000000..7a9cf1a85
--- /dev/null
+++
b/zookeeper-server/src/main/java/org/apache/zookeeper/common/LogRedactor.java
@@ -0,0 +1,62 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you 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
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * 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 org.apache.zookeeper.common;
+
+import java.util.Locale;
+import java.util.Map;
+import java.util.Properties;
+
+public class LogRedactor {
+
+ private LogRedactor() {}
+
+ /**
+ * Redacts all values which ends with "password" from the given properties
/ map.
+ * Other values are not changed.
+ *
+ * @param properties the properties in which all passwords should be
redacted.
+ * @return new Properties object containing all values, passwords redacted.
+ */
+ public static Properties redactSensitiveValues(Map<?, ?> properties) {
+ Properties redactedConfig = new Properties();
+ properties.forEach((k, v) -> {
+ if (k != null && v != null) {
+ redactedConfig.put(k, redactValue((String) k, (String) v));
+ }
+ });
+ return redactedConfig;
+ }
+
+ /**
+ * Returns redacted value when the key ends with "password".
+ * Otherwise, just returns the value.
+ *
+ * @param key the key to check if it ends with "password".
+ * @return redacted value when the key ends with "password". Otherwise,
the value.
+ */
+ public static String redactValue(String key, String value) {
+ if (key == null) {
+ return value;
+ }
+ if (key.toLowerCase(Locale.ROOT).endsWith("password")) {
+ return "***";
+ }
+ return value;
+ }
+}
diff --git
a/zookeeper-server/src/main/java/org/apache/zookeeper/common/ZKConfig.java
b/zookeeper-server/src/main/java/org/apache/zookeeper/common/ZKConfig.java
index 4a3805d26..efc4689a5 100644
--- a/zookeeper-server/src/main/java/org/apache/zookeeper/common/ZKConfig.java
+++ b/zookeeper-server/src/main/java/org/apache/zookeeper/common/ZKConfig.java
@@ -18,12 +18,13 @@
package org.apache.zookeeper.common;
+import static org.apache.zookeeper.common.LogRedactor.redactSensitiveValues;
+import static org.apache.zookeeper.common.LogRedactor.redactValue;
import java.io.File;
import java.io.FileInputStream;
import java.io.IOException;
import java.nio.file.Path;
import java.util.HashMap;
-import java.util.Locale;
import java.util.Map;
import java.util.Map.Entry;
import java.util.Properties;
@@ -94,11 +95,7 @@ public ZKConfig(String configPath) throws
QuorumPeerConfig.ConfigException {
public ZKConfig(File configFile) throws QuorumPeerConfig.ConfigException {
this();
addConfiguration(configFile);
- Map<String, String> p = new HashMap<>();
- for (Entry<String, String> entry : properties.entrySet()) {
- p.put(entry.getKey(), logRedactor(entry.getKey(),
entry.getValue()));
- }
- LOG.info("ZK Config {}", p);
+ LOG.info("ZK Config {}", redactSensitiveValues(properties));
}
/**
@@ -211,7 +208,7 @@ public void setProperty(String key, String value) {
}
String oldValue = properties.put(key, value);
if (null != oldValue && !oldValue.equals(value)) {
- LOG.debug("key {}'s value {} is replaced with new value {}", key,
logRedactor(key, oldValue), logRedactor(key, value));
+ LOG.debug("key {}'s value {} is replaced with new value {}", key,
redactValue(key, oldValue), redactValue(key, value));
}
}
@@ -334,14 +331,4 @@ public int getInt(String key, int defaultValue) {
}
return defaultValue;
}
-
- private String logRedactor(String key, String value) {
- if (key == null) {
- return value;
- }
- if (key.toLowerCase(Locale.ROOT).endsWith("password")) {
- return "***";
- }
- return value;
- }
}
diff --git
a/zookeeper-server/src/test/java/org/apache/zookeeper/common/LogRedactorTest.java
b/zookeeper-server/src/test/java/org/apache/zookeeper/common/LogRedactorTest.java
new file mode 100644
index 000000000..a15696f02
--- /dev/null
+++
b/zookeeper-server/src/test/java/org/apache/zookeeper/common/LogRedactorTest.java
@@ -0,0 +1,100 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you 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
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * 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 org.apache.zookeeper.common;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Properties;
+import org.junit.jupiter.api.Test;
+
+class LogRedactorTest {
+
+ @Test
+ void testRedactValueWithPasswordKey() {
+ assertEquals("***", LogRedactor.redactValue("ssl.keyStore.password",
"secret"));
+ assertEquals("***", LogRedactor.redactValue("ssl.trustStore.password",
"secret"));
+ assertEquals("***", LogRedactor.redactValue("somePassword", "secret"));
+ }
+
+ @Test
+ void testRedactValueCaseInsensitive() {
+ assertEquals("***", LogRedactor.redactValue("ssl.keyStore.PASSWORD",
"secret"));
+ assertEquals("***", LogRedactor.redactValue("ssl.keyStore.Password",
"secret"));
+ assertEquals("***", LogRedactor.redactValue("ssl.keyStore.passWORD",
"secret"));
+ }
+
+ @Test
+ void testRedactValueNonSensitiveKey() {
+ assertEquals("localhost", LogRedactor.redactValue("server.host",
"localhost"));
+ assertEquals("8080", LogRedactor.redactValue("httpPort", "8080"));
+ assertEquals("/path/to/keystore",
LogRedactor.redactValue("ssl.keyStore.location", "/path/to/keystore"));
+ }
+
+ @Test
+ void testRedactValueNullKey() {
+ assertEquals("someValue", LogRedactor.redactValue(null, "someValue"));
+ }
+
+ @Test
+ void testRedactSensitiveValuesWithStringMap() {
+ Map<String, String> config = new HashMap<>();
+ config.put("ssl.keyStore.location", "/path/to/keystore.jks");
+ config.put("ssl.keyStore.password", "SuperSecret");
+ config.put("httpPort", "9141");
+
+ Properties redacted = LogRedactor.redactSensitiveValues(config);
+
+ assertEquals("/path/to/keystore.jks",
redacted.get("ssl.keyStore.location"));
+ assertEquals("***", redacted.get("ssl.keyStore.password"));
+ assertEquals("9141", redacted.get("httpPort"));
+ }
+
+ @Test
+ void testRedactSensitiveValuesWithProperties() {
+ Properties config = new Properties();
+ config.setProperty("ssl.trustStore.password", "TrustSecret");
+ config.setProperty("ssl.trustStore.location",
"/path/to/truststore.jks");
+
+ Properties redacted = LogRedactor.redactSensitiveValues(config);
+
+ assertEquals("***", redacted.get("ssl.trustStore.password"));
+ assertEquals("/path/to/truststore.jks",
redacted.get("ssl.trustStore.location"));
+ }
+
+ @Test
+ void testRedactSensitiveValuesSkipsNullValues() {
+ Map<String, String> config = new HashMap<>();
+ config.put("ssl.keyStore.location", null);
+ config.put("httpPort", "9141");
+
+ Properties redacted = LogRedactor.redactSensitiveValues(config);
+
+ assertFalse(redacted.containsKey("ssl.keyStore.location"));
+ assertEquals("9141", redacted.get("httpPort"));
+ }
+
+ @Test
+ void testRedactSensitiveValuesEmptyMap() {
+ Properties redacted = LogRedactor.redactSensitiveValues(new
HashMap<>());
+ assertTrue(redacted.isEmpty());
+ }
+}
