Skip to content

Commit f3551e1

Browse files
committed
[SPARK-57950] Support spark.redaction.regex and redact sensitive values in operator logs
### What changes were proposed in this pull request? This PR aims to redact sensitive configuration values in operator logs. - Add a new config option, `spark.redaction.regex`, reusing Apache Spark's redaction key and default pattern (`(?i)secret|password|token|access[.]?key`). - Add `Utils.redactSensitiveInfo` which replaces a value with `*********(redacted)` when its key or value matches the pattern. - Apply redaction to the configuration logging on startup (`spark.logConf`). - Escape pipes in `DocTable` cells and regenerate `docs/config_properties.md`. ### Why are the changes needed? When `spark.logConf` is enabled, the operator prints all configuration properties without filtering, exposing sensitive values like `*.password` or `spark.hadoop.fs.s3a.access.key` in plain text. ### Does this PR introduce _any_ user-facing change? Yes, sensitive configuration values are now printed as `*********(redacted)`, and the pattern is customizable via `spark.redaction.regex`. ### How was this patch tested? Pass the CIs with the newly added test cases. ### Was this patch authored or co-authored using generative AI tooling? Generated-by: Claude Fable 5 Closes #746 from dongjoon-hyun/SPARK-57950. Authored-by: Dongjoon Hyun <dongjoon@apache.org> Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>
1 parent dc4dea9 commit f3551e1

7 files changed

Lines changed: 270 additions & 2 deletions

File tree

build-tools/docs-utils/src/main/java/org/apache/spark/k8s/operator/utils/DocTable.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,8 @@ public void flush(PrintWriter writer) {
6363
private String joinRow(List<String> elements) {
6464
StringBuilder stringBuilder = new StringBuilder(ROW_SEPARATOR);
6565
for (String element : elements) {
66-
stringBuilder.append(element).append(ROW_SEPARATOR);
66+
// Escape pipes in cell content so that they do not break the Markdown table layout
67+
stringBuilder.append(element.replace("|", "\\|")).append(ROW_SEPARATOR);
6768
}
6869
if (elements.size() < columns) {
6970
// Append empty cells to end if needed

docs/config_properties.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,4 +51,5 @@
5151
| spark.kubernetes.operator.reconciler.trimStateTransitionHistoryEnabled | Boolean | true | true | When enabled, operator would trim state transition history when a new attempt starts, keeping previous attempt summary only. |
5252
| spark.kubernetes.operator.watchedNamespaces | String | default | true | Comma-separated list of namespaces that the operator would be watching for Spark resources. If set to '*', operator would watch all namespaces. |
5353
| spark.logConf | Boolean | false | true | When enabled, operator will print configurations |
54+
| spark.redaction.regex | String | (?i)secret\|password\|token\|access[.]?key | true | Regex to decide which parts of configuration properties contain sensitive information, whose values would be redacted in operator logs. This reuses Spark's redaction configuration key and default pattern. |
5455

spark-operator/src/main/java/org/apache/spark/k8s/operator/SparkOperator.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,8 @@ public SparkOperator() {
106106
this.registeredOperators = new ArrayList<>();
107107
this.registeredOperators.add(registerSparkOperator());
108108
if (SparkOperatorConf.LOG_CONF.getValue()) {
109-
for (var entry : SparkOperatorConfManager.INSTANCE.getAll().entrySet()) {
109+
for (var entry :
110+
Utils.redactSensitiveInfo(SparkOperatorConfManager.INSTANCE.getAll()).entrySet()) {
110111
log.info("{} = {}", entry.getKey(), entry.getValue());
111112
}
112113
}

spark-operator/src/main/java/org/apache/spark/k8s/operator/config/SparkOperatorConf.java

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,22 @@ public final class SparkOperatorConf {
4040
.defaultValue(false)
4141
.build();
4242

43+
/**
44+
* Regex to decide which parts of configuration properties contain sensitive information, whose
45+
* values are redacted in operator logs. Reuses Spark's {@code spark.redaction.regex}
46+
* configuration key and default pattern.
47+
*/
48+
public static final ConfigOption<String> REDACTION_REGEX =
49+
ConfigOption.<String>builder()
50+
.key("spark.redaction.regex")
51+
.description(
52+
"Regex to decide which parts of configuration properties contain sensitive "
53+
+ "information, whose values would be redacted in operator logs. This reuses "
54+
+ "Spark's redaction configuration key and default pattern.")
55+
.typeParameterClass(String.class)
56+
.defaultValue("(?i)secret|password|token|access[.]?key")
57+
.build();
58+
4359
/**
4460
* Interval (in seconds) between periodic System.gc() invocations. Set to 0 or a negative value
4561
* to disable.

spark-operator/src/main/java/org/apache/spark/k8s/operator/utils/Utils.java

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
import java.util.List;
3636
import java.util.Map;
3737
import java.util.Set;
38+
import java.util.regex.Pattern;
3839
import java.util.stream.Collectors;
3940

4041
import io.fabric8.kubernetes.api.model.HasMetadata;
@@ -51,6 +52,9 @@
5152
/** Utility class for common operations. */
5253
public final class Utils {
5354

55+
/** Replacement text for redacted sensitive values, same as Spark's redaction utility. */
56+
public static final String REDACTION_REPLACEMENT_TEXT = "*********(redacted)";
57+
5458
private Utils() {}
5559

5660
/**
@@ -232,6 +236,36 @@ public static String commonResourceLabelsStr() {
232236
return commonLabels;
233237
}
234238

239+
/**
240+
* Redacts sensitive values (e.g. secrets, passwords, tokens) in the given properties for safe
241+
* logging, following the semantics of Spark's {@code spark.redaction.regex} based redaction. The
242+
* redaction pattern is resolved from the {@code spark.redaction.regex} entry of the given
243+
* properties, falling back to the operator configuration ({@link
244+
* SparkOperatorConf#REDACTION_REGEX}). An entry is redacted when its key or value matches the
245+
* pattern.
246+
*
247+
* @param props The properties to redact.
248+
* @return A Map containing the given entries with sensitive values redacted.
249+
*/
250+
public static Map<String, String> redactSensitiveInfo(Map<?, ?> props) {
251+
Map<String, String> conf = new HashMap<>();
252+
props.forEach((k, v) -> conf.put(String.valueOf(k), String.valueOf(v)));
253+
String regex =
254+
conf.getOrDefault(
255+
SparkOperatorConf.REDACTION_REGEX.getKey(),
256+
SparkOperatorConf.REDACTION_REGEX.getValue());
257+
Pattern pattern = Pattern.compile(regex);
258+
Map<String, String> redacted = new HashMap<>();
259+
for (Map.Entry<String, String> entry : conf.entrySet()) {
260+
if (pattern.matcher(entry.getKey()).find() || pattern.matcher(entry.getValue()).find()) {
261+
redacted.put(entry.getKey(), REDACTION_REPLACEMENT_TEXT);
262+
} else {
263+
redacted.put(entry.getKey(), entry.getValue());
264+
}
265+
}
266+
return redacted;
267+
}
268+
235269
/**
236270
* Creates a SecondaryToPrimaryMapper that maps secondary resources to their primary resources
237271
* based on a common label.

spark-operator/src/test/java/org/apache/spark/k8s/operator/SparkOperatorTest.java

Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,12 +29,24 @@
2929
import static org.mockito.Mockito.verifyNoMoreInteractions;
3030
import static org.mockito.Mockito.when;
3131

32+
import java.util.List;
33+
import java.util.Map;
3234
import java.util.Set;
35+
import java.util.concurrent.CopyOnWriteArrayList;
3336
import java.util.function.Consumer;
3437

3538
import io.fabric8.kubernetes.client.KubernetesClient;
3639
import io.javaoperatorsdk.operator.Operator;
3740
import io.javaoperatorsdk.operator.RegisteredController;
41+
import org.apache.logging.log4j.Level;
42+
import org.apache.logging.log4j.LogManager;
43+
import org.apache.logging.log4j.core.Filter;
44+
import org.apache.logging.log4j.core.LogEvent;
45+
import org.apache.logging.log4j.core.LoggerContext;
46+
import org.apache.logging.log4j.core.appender.AbstractAppender;
47+
import org.apache.logging.log4j.core.config.Configuration;
48+
import org.apache.logging.log4j.core.config.Property;
49+
import org.apache.logging.log4j.core.layout.PatternLayout;
3850
import org.junit.jupiter.api.Assertions;
3951
import org.junit.jupiter.api.Test;
4052
import org.mockito.MockedConstruction;
@@ -43,6 +55,7 @@
4355
import org.apache.spark.k8s.operator.client.KubernetesClientFactory;
4456
import org.apache.spark.k8s.operator.config.DynamicConfigMonitor;
4557
import org.apache.spark.k8s.operator.config.SparkOperatorConf;
58+
import org.apache.spark.k8s.operator.config.SparkOperatorConfManager;
4659
import org.apache.spark.k8s.operator.metrics.MetricsService;
4760
import org.apache.spark.k8s.operator.metrics.MetricsSystem;
4861
import org.apache.spark.k8s.operator.metrics.MetricsSystemFactory;
@@ -252,4 +265,98 @@ void testUpdateWatchedNamespacesWithDynamicConfigEnabled() {
252265
setConfigKey(SparkOperatorConf.DYNAMIC_CONFIG_SOURCE, dynamicConfigSource);
253266
}
254267
}
268+
269+
@Test
270+
void testLogConfRedactsSensitiveValues() {
271+
MetricsSystem mockMetricsSystem = mock(MetricsSystem.class);
272+
KubernetesClient mockClient = mock(KubernetesClient.class);
273+
TestLogAppender appender = installAppender();
274+
try (MockedStatic<MetricsSystemFactory> mockMetricsSystemFactory =
275+
mockStatic(MetricsSystemFactory.class);
276+
MockedStatic<KubernetesClientFactory> mockKubernetesClientFactory =
277+
mockStatic(KubernetesClientFactory.class);
278+
MockedConstruction<Operator> operatorConstruction = mockConstruction(Operator.class);
279+
MockedConstruction<SparkAppReconciler> sparkAppReconcilerConstruction =
280+
mockConstruction(SparkAppReconciler.class);
281+
MockedConstruction<ProbeService> probeServiceConstruction =
282+
mockConstruction(ProbeService.class);
283+
MockedConstruction<MetricsService> metricsServiceConstruction =
284+
mockConstruction(MetricsService.class);
285+
MockedConstruction<KubernetesMetricsInterceptor> interceptorMockedConstruction =
286+
mockConstruction(KubernetesMetricsInterceptor.class)) {
287+
setConfigKey(SparkOperatorConf.LOG_CONF, true);
288+
SparkOperatorConfManager.INSTANCE.refresh(
289+
Map.of("spark.dummy.db.password", "super-sensitive-value"));
290+
mockMetricsSystemFactory
291+
.when(MetricsSystemFactory::createMetricsSystem)
292+
.thenReturn(mockMetricsSystem);
293+
mockKubernetesClientFactory
294+
.when(() -> KubernetesClientFactory.buildKubernetesClient(any()))
295+
.thenReturn(mockClient);
296+
297+
SparkOperator sparkOperator = new SparkOperator();
298+
Assertions.assertNotNull(sparkOperator);
299+
Assertions.assertEquals(1, operatorConstruction.constructed().size());
300+
Assertions.assertEquals(1, sparkAppReconcilerConstruction.constructed().size());
301+
Assertions.assertEquals(1, probeServiceConstruction.constructed().size());
302+
Assertions.assertEquals(1, metricsServiceConstruction.constructed().size());
303+
Assertions.assertEquals(1, interceptorMockedConstruction.constructed().size());
304+
305+
String redactedText = org.apache.spark.util.Utils$.MODULE$.REDACTION_REPLACEMENT_TEXT();
306+
Assertions.assertTrue(
307+
appender.messages.stream()
308+
.anyMatch(m -> m.equals("spark.dummy.db.password = " + redactedText)),
309+
"Expected redacted log for the sensitive key, got: " + appender.messages);
310+
Assertions.assertTrue(
311+
appender.messages.stream().noneMatch(m -> m.contains("super-sensitive-value")),
312+
"Sensitive value must not appear in operator logs");
313+
} finally {
314+
setConfigKey(SparkOperatorConf.LOG_CONF, false);
315+
SparkOperatorConfManager.INSTANCE.refresh(Map.of());
316+
removeAppender(appender);
317+
}
318+
}
319+
320+
private static TestLogAppender installAppender() {
321+
TestLogAppender appender = new TestLogAppender("SparkOperatorTestLogAppender");
322+
appender.start();
323+
LoggerContext ctx = (LoggerContext) LogManager.getContext(false);
324+
Configuration config = ctx.getConfiguration();
325+
// The test log4j2 configuration has a global WARN threshold filter; detach it so that the
326+
// INFO level configuration logging can reach the appender.
327+
appender.removedGlobalFilter = config.getFilter();
328+
if (appender.removedGlobalFilter != null) {
329+
config.removeFilter(appender.removedGlobalFilter);
330+
}
331+
config.getRootLogger().addAppender(appender, Level.ALL, null);
332+
ctx.updateLoggers();
333+
return appender;
334+
}
335+
336+
private static void removeAppender(TestLogAppender appender) {
337+
LoggerContext ctx = (LoggerContext) LogManager.getContext(false);
338+
Configuration config = ctx.getConfiguration();
339+
config.getRootLogger().removeAppender(appender.getName());
340+
if (appender.removedGlobalFilter != null) {
341+
config.addFilter(appender.removedGlobalFilter);
342+
}
343+
ctx.updateLoggers();
344+
appender.stop();
345+
}
346+
347+
private static final class TestLogAppender extends AbstractAppender {
348+
private final List<String> messages = new CopyOnWriteArrayList<>();
349+
private Filter removedGlobalFilter;
350+
351+
private TestLogAppender(String name) {
352+
super(name, null, PatternLayout.createDefaultLayout(), false, Property.EMPTY_ARRAY);
353+
}
354+
355+
@Override
356+
public void append(LogEvent event) {
357+
if (SparkOperator.class.getName().equals(event.getLoggerName())) {
358+
messages.add(event.getMessage().getFormattedMessage());
359+
}
360+
}
361+
}
255362
}
Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
20+
package org.apache.spark.k8s.operator.utils;
21+
22+
import static org.junit.jupiter.api.Assertions.assertEquals;
23+
24+
import java.util.Map;
25+
import java.util.Properties;
26+
27+
import org.junit.jupiter.api.AfterEach;
28+
import org.junit.jupiter.api.Test;
29+
30+
import org.apache.spark.k8s.operator.config.SparkOperatorConf;
31+
import org.apache.spark.k8s.operator.config.SparkOperatorConfManager;
32+
33+
class UtilsTest {
34+
private static final String REDACTED_TEXT =
35+
org.apache.spark.util.Utils$.MODULE$.REDACTION_REPLACEMENT_TEXT();
36+
private static final String REDACTION_REGEX_KEY = SparkOperatorConf.REDACTION_REGEX.getKey();
37+
38+
@Test
39+
void redactionRegexConfigReusesSparkRedactionConfig() {
40+
assertEquals(
41+
org.apache.spark.internal.config.package$.MODULE$.SECRET_REDACTION_PATTERN().key(),
42+
SparkOperatorConf.REDACTION_REGEX.getKey());
43+
assertEquals(
44+
org.apache.spark.internal.config.package$
45+
.MODULE$
46+
.SECRET_REDACTION_PATTERN()
47+
.defaultValueString(),
48+
SparkOperatorConf.REDACTION_REGEX.getDefaultValue());
49+
}
50+
51+
@AfterEach
52+
void resetConfigOverrides() {
53+
SparkOperatorConfManager.INSTANCE.refresh(Map.of());
54+
}
55+
56+
@Test
57+
void redactSensitiveInfoMasksSensitiveKeysWithSparkDefaultPattern() {
58+
Map<String, String> props =
59+
Map.of(
60+
"spark.hadoop.fs.s3a.access.key", "sensitive-access-key",
61+
"spark.ssl.keyStorePassword", "sensitive-keystore-value",
62+
"my.service.token", "sensitive-service-value",
63+
"custom.api.secret", "sensitive-api-value",
64+
"spark.kubernetes.operator.name", "spark-kubernetes-operator");
65+
Map<String, String> redacted = Utils.redactSensitiveInfo(props);
66+
assertEquals(REDACTED_TEXT, redacted.get("spark.hadoop.fs.s3a.access.key"));
67+
assertEquals(REDACTED_TEXT, redacted.get("spark.ssl.keyStorePassword"));
68+
assertEquals(REDACTED_TEXT, redacted.get("my.service.token"));
69+
assertEquals(REDACTED_TEXT, redacted.get("custom.api.secret"));
70+
assertEquals("spark-kubernetes-operator", redacted.get("spark.kubernetes.operator.name"));
71+
assertEquals(props.keySet(), redacted.keySet());
72+
}
73+
74+
@Test
75+
void redactSensitiveInfoAcceptsProperties() {
76+
Properties props = new Properties();
77+
props.setProperty("my.db.password", "sensitive-db-value");
78+
props.setProperty("spark.kubernetes.operator.namespace", "default");
79+
Map<String, String> redacted = Utils.redactSensitiveInfo(props);
80+
assertEquals(REDACTED_TEXT, redacted.get("my.db.password"));
81+
assertEquals("default", redacted.get("spark.kubernetes.operator.namespace"));
82+
}
83+
84+
@Test
85+
void redactSensitiveInfoHonorsRedactionRegexFromGivenProperties() {
86+
Map<String, String> props =
87+
Map.of(
88+
REDACTION_REGEX_KEY, "(?i)confidential",
89+
"custom.confidential.conf", "custom-value",
90+
"spark.kubernetes.operator.namespace", "default");
91+
Map<String, String> redacted = Utils.redactSensitiveInfo(props);
92+
assertEquals(REDACTED_TEXT, redacted.get("custom.confidential.conf"));
93+
assertEquals("default", redacted.get("spark.kubernetes.operator.namespace"));
94+
}
95+
96+
@Test
97+
void redactSensitiveInfoHonorsRedactionRegexFromOperatorConfig() {
98+
SparkOperatorConfManager.INSTANCE.refresh(Map.of(REDACTION_REGEX_KEY, "(?i)confidential"));
99+
Map<String, String> props =
100+
Map.of(
101+
"custom.confidential.conf", "custom-value",
102+
"spark.kubernetes.operator.namespace", "default");
103+
Map<String, String> redacted = Utils.redactSensitiveInfo(props);
104+
assertEquals(REDACTED_TEXT, redacted.get("custom.confidential.conf"));
105+
assertEquals("default", redacted.get("spark.kubernetes.operator.namespace"));
106+
assertEquals(props.keySet(), redacted.keySet());
107+
}
108+
}

0 commit comments

Comments
 (0)