Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Improvement-16999] Fix SensitiveDataConverter cannot convert empty data #17000

Merged
merged 1 commit into from
Feb 8, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/docs/en/architecture/design.md
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ In the early schedule design, if there is no priority design and use the fair sc
- For details, please refer to the logback configuration of Master and Worker, as shown in the following example:

```xml
<conversionRule conversionWord="message" converterClass="org.apache.dolphinscheduler.common.log.SensitiveDataConverter"/>
<conversionRule conversionWord="message" converterClass="org.apache.dolphinscheduler.plugin.task.api.log.SensitiveDataConverter"/>
<appender name="TASKLOGFILE" class="ch.qos.logback.classic.sift.SiftingAppender">
<filter class="org.apache.dolphinscheduler.plugin.task.api.log.TaskLogFilter"/>
<Discriminator class="org.apache.dolphinscheduler.plugin.task.api.log.TaskLogDiscriminator">
Expand Down
2 changes: 1 addition & 1 deletion docs/docs/zh/architecture/design.md
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@
- 详情可参考Master和Worker的logback配置,如下示例:

```xml
<conversionRule conversionWord="message" converterClass="org.apache.dolphinscheduler.common.log.SensitiveDataConverter"/>
<conversionRule conversionWord="message" converterClass="org.apache.dolphinscheduler.plugin.task.api.log.SensitiveDataConverter"/>
<appender name="TASKLOGFILE" class="ch.qos.logback.classic.sift.SiftingAppender">
<filter class="org.apache.dolphinscheduler.plugin.task.api.log.TaskLogFilter"/>
<Discriminator class="org.apache.dolphinscheduler.plugin.task.api.log.TaskLogDiscriminator">
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,8 @@ public interface DataSourceProcessor {
BaseDataSourceParamDTO castDatasourceParamDTO(String paramJson);

/**
* check datasource param is valid
* check datasource param is valid.
* @throws IllegalArgumentException if invalid
*/
void checkDatasourceParam(BaseDataSourceParamDTO datasourceParam);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,16 +46,8 @@ public static void checkDatasourceParam(BaseDataSourceParamDTO baseDataSourcePar
getDatasourceProcessor(baseDataSourceParamDTO.getType()).checkDatasourceParam(baseDataSourceParamDTO);
}

/**
* build connection url
*
* @param baseDataSourceParamDTO datasourceParam
*/
public static ConnectionParam buildConnectionParams(BaseDataSourceParamDTO baseDataSourceParamDTO) {
ConnectionParam connectionParams = getDatasourceProcessor(baseDataSourceParamDTO.getType())
.createConnectionParams(baseDataSourceParamDTO);
log.info("Parameters map:{}", connectionParams);
return connectionParams;
return getDatasourceProcessor(baseDataSourceParamDTO.getType()).createConnectionParams(baseDataSourceParamDTO);
}

public static ConnectionParam buildConnectionParams(DbType dbType, String connectionJson) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
</appender>

<conversionRule conversionWord="message"
converterClass="org.apache.dolphinscheduler.common.log.SensitiveDataConverter"/>
converterClass="org.apache.dolphinscheduler.plugin.task.api.log.SensitiveDataConverter"/>
<appender name="TASKLOGFILE" class="ch.qos.logback.classic.sift.SiftingAppender">
<filter class="org.apache.dolphinscheduler.plugin.task.api.log.TaskLogFilter"/>
<Discriminator class="org.apache.dolphinscheduler.plugin.task.api.log.TaskLogDiscriminator">
Expand Down
2 changes: 1 addition & 1 deletion dolphinscheduler-master/src/test/resources/logback.xml
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
</appender>

<conversionRule conversionWord="message"
converterClass="org.apache.dolphinscheduler.common.log.SensitiveDataConverter"/>
converterClass="org.apache.dolphinscheduler.plugin.task.api.log.SensitiveDataConverter"/>
<appender name="TASKLOGFILE" class="ch.qos.logback.classic.sift.SiftingAppender">
<filter class="org.apache.dolphinscheduler.plugin.task.api.log.TaskLogFilter"/>
<Discriminator class="org.apache.dolphinscheduler.plugin.task.api.log.TaskLogDiscriminator">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@
<logger name="org.apache.hadoop" level="WARN"/>

<conversionRule conversionWord="message"
converterClass="org.apache.dolphinscheduler.common.log.SensitiveDataConverter"/>
converterClass="org.apache.dolphinscheduler.plugin.task.api.log.SensitiveDataConverter"/>
<appender name="TASKLOGFILE" class="ch.qos.logback.classic.sift.SiftingAppender">
<filter class="org.apache.dolphinscheduler.plugin.task.api.log.TaskLogFilter"/>
<Discriminator class="org.apache.dolphinscheduler.plugin.task.api.log.TaskLogDiscriminator">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -387,4 +387,5 @@ private TaskConstants() {
public static final String TASK_INSTANCE_ID_MDC_KEY = "taskInstanceId";

public static final String STAR = "*";
public static final String SENSITIVE_DATA_MASK = "******";
}
Original file line number Diff line number Diff line change
Expand Up @@ -21,24 +21,25 @@

import org.apache.commons.lang3.StringUtils;

import java.util.Collections;
import java.util.HashSet;
import java.util.Set;
import java.util.regex.Matcher;
import java.util.regex.Pattern;

import ch.qos.logback.classic.pattern.MessageConverter;
import ch.qos.logback.classic.spi.ILoggingEvent;

import com.google.common.base.Strings;

/**
* sensitive data log converter
*/
public class SensitiveDataConverter extends MessageConverter {

private static Pattern multilinePattern;
private static HashSet<String> maskPatterns =
new HashSet<>(Collections.singletonList(TaskConstants.DATASOURCE_PASSWORD_REGEX));
private static final Set<String> maskPatterns = new HashSet<>();

static {
addMaskPattern(TaskConstants.DATASOURCE_PASSWORD_REGEX);
}

@Override
public String convert(ILoggingEvent event) {
Expand All @@ -50,23 +51,24 @@ public String convert(ILoggingEvent event) {
return maskSensitiveData(requestLogMsg);
}

public static void addMaskPattern(String maskPattern) {
public static synchronized void addMaskPattern(final String maskPattern) {
if (maskPatterns.contains(maskPattern)) {
return;
}
maskPatterns.add(maskPattern);
multilinePattern = Pattern.compile(String.join("|", maskPatterns), Pattern.MULTILINE);
}

public static String maskSensitiveData(final String logMsg) {
if (StringUtils.isEmpty(logMsg)) {
return logMsg;
}
multilinePattern = Pattern.compile(String.join("|", maskPatterns), Pattern.MULTILINE);

StringBuffer sb = new StringBuffer(logMsg.length());
Matcher matcher = multilinePattern.matcher(logMsg);
final StringBuffer sb = new StringBuffer(logMsg.length());
final Matcher matcher = multilinePattern.matcher(logMsg);

while (matcher.find()) {
String password = matcher.group();
String maskPassword = Strings.repeat(TaskConstants.STAR, password.length());
matcher.appendReplacement(sb, maskPassword);
matcher.appendReplacement(sb, TaskConstants.SENSITIVE_DATA_MASK);
}
matcher.appendTail(sb);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,26 +15,22 @@
* limitations under the License.
*/

package org.apache.dolphinscheduler.common.log;
package org.apache.dolphinscheduler.plugin.task.api.log;

import static org.apache.dolphinscheduler.common.constants.Constants.K8S_CONFIG_REGEX;
import static org.junit.jupiter.api.Assertions.assertEquals;

import java.util.HashMap;

import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

public class SensitiveDataConverterTest {

private final Logger logger = LoggerFactory.getLogger(SensitiveDataConverterTest.class);
class SensitiveDataConverterTest {

/**
* mask sensitive logMsg - sql task datasource password
*/
@Test
public void testPwdLogMsgConverter() {
void testPwdLogMsgConverter() {
HashMap<String, String> tcs = new HashMap<>();
tcs.put("{\"address\":\"jdbc:mysql://192.168.xx.xx:3306\","
+ "\"database\":\"carbond\","
Expand All @@ -46,7 +42,7 @@ public void testPwdLogMsgConverter() {
+ "\"database\":\"carbond\","
+ "\"jdbcUrl\":\"jdbc:mysql://192.168.xx.xx:3306/ods\","
+ "\"user\":\"view\","
+ "\"password\":\"*****\"}");
+ "\"password\":\"******\"}");

tcs.put("End initialize task {\n" +
" \"resourceParametersHelper\" : {\n" +
Expand All @@ -70,7 +66,7 @@ public void testPwdLogMsgConverter() {
" \"1\" : {\n" +
" \"resourceType\" : \"DATASOURCE\",\n" +
" \"type\" : \"ORACLE\",\n" +
" \"connectionParams\" : \"{\\\"user\\\":\\\"user\\\",\\\"password\\\":\\\"*****\\\"}\",\n"
" \"connectionParams\" : \"{\\\"user\\\":\\\"user\\\",\\\"password\\\":\\\"******\\\"}\",\n"
+
" \"DATASOURCE\" : null\n" +
" }\n" +
Expand All @@ -81,24 +77,32 @@ public void testPwdLogMsgConverter() {

for (String logMsg : tcs.keySet()) {
String maskedLog = SensitiveDataConverter.maskSensitiveData(logMsg);
logger.info("original parameter : {}", logMsg);
logger.info("masked parameter : {}", maskedLog);
Assertions.assertEquals(tcs.get(logMsg), maskedLog);
assertEquals(tcs.get(logMsg), maskedLog);
}
}

@Test
public void testPostJdbcInfoLogMsgConverter() {
void testPostJdbcInfoLogMsgConverter() {
String POST_JDBC_INFO_REGEX = "(?<=(post jdbc info:)).*(?=)";
SensitiveDataConverter.addMaskPattern(POST_JDBC_INFO_REGEX);
String postJdbcInfoLogMsg = "post jdbc info:clickhouse,jdbc:clickhouse://127.0.0.1:8123/td_cdp,admin,123%@@56";
final String maskedLog = SensitiveDataConverter.maskSensitiveData(postJdbcInfoLogMsg);
String expectedMsg = "post jdbc info:*****************************************************************";
Assertions.assertEquals(expectedMsg, maskedLog);
String expectedMsg = "post jdbc info:******";
assertEquals(expectedMsg, maskedLog);
}

@Test
public void testK8SLogMsgConverter() {
void testMaskSensitiveDataWithEmptyPassword() {
String postJdbcInfoLogMsg =
"MySQLConnectionParam{user='admin', password='', address='jdbc:mysql://localhost:3306', database='aa', jdbcUrl='jdbc:mysql://localhost:3306/aa', driverLocation='null', driverClassName='com.mysql.cj.jdbc.Driver', validationQuery='select 1', other='null'}";
final String maskedLog = SensitiveDataConverter.maskSensitiveData(postJdbcInfoLogMsg);
final String expectedMsg =
"MySQLConnectionParam{user='admin', password='******', address='jdbc:mysql://localhost:3306', database='aa', jdbcUrl='jdbc:mysql://localhost:3306/aa', driverLocation='null', driverClassName='com.mysql.cj.jdbc.Driver', validationQuery='select 1', other='null'}";
assertEquals(expectedMsg, maskedLog);
}

@Test
void testK8SLogMsgConverter() {
String msg = "End initialize task {\n" +
" \"taskName\" : \"echo\",\n" +
" \"k8sTaskExecutionContext\" : {\n" +
Expand All @@ -110,18 +114,14 @@ public void testK8SLogMsgConverter() {
String maskMsg = "End initialize task {\n" +
" \"taskName\" : \"echo\",\n" +
" \"k8sTaskExecutionContext\" : {\n" +
" \"configYaml\" : \"**************************************\",\n" +
" \"configYaml\" : \"******\",\n" +
" \"namespace\" : \"abc\"\n" +
" },\n" +
" \"logBufferEnable\" : false\n" +
"}";
SensitiveDataConverter.addMaskPattern(K8S_CONFIG_REGEX);
final String maskedLog = SensitiveDataConverter.maskSensitiveData(msg);

logger.info("original parameter : {}", msg);
logger.info("masked parameter : {}", maskedLog);

Assertions.assertEquals(maskMsg, maskedLog);

assertEquals(maskMsg, maskedLog);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ public void testSqoopPasswordMask() {
"sqoop import -D mapred.job.name=sqoop_task -m 1 --connect \"jdbc:mysql://localhost:3306/defuault\" --username root --password \"mypassword\" --table student --target-dir /sqoop_test --as-textfile";

final String maskScript =
"sqoop import -D mapred.job.name=sqoop_task -m 1 --connect \"jdbc:mysql://localhost:3306/defuault\" --username root --password \"**********\" --table student --target-dir /sqoop_test --as-textfile";
"sqoop import -D mapred.job.name=sqoop_task -m 1 --connect \"jdbc:mysql://localhost:3306/defuault\" --username root --password \"******\" --table student --target-dir /sqoop_test --as-textfile";

SensitiveDataConverter.addMaskPattern(SqoopConstants.SQOOP_PASSWORD_REGEX);
Assertions.assertEquals(maskScript, SensitiveDataConverter.maskSensitiveData(originalScript));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
</appender>

<conversionRule conversionWord="message"
converterClass="org.apache.dolphinscheduler.common.log.SensitiveDataConverter"/>
converterClass="org.apache.dolphinscheduler.plugin.task.api.log.SensitiveDataConverter"/>
<appender name="TASKLOGFILE" class="ch.qos.logback.classic.sift.SiftingAppender">
<filter class="org.apache.dolphinscheduler.plugin.task.api.log.TaskLogFilter"/>
<Discriminator class="org.apache.dolphinscheduler.plugin.task.api.log.TaskLogDiscriminator">
Expand Down
2 changes: 1 addition & 1 deletion dolphinscheduler-worker/src/test/resources/logback.xml
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
</appender>

<conversionRule conversionWord="message"
converterClass="org.apache.dolphinscheduler.common.log.SensitiveDataConverter"/>
converterClass="org.apache.dolphinscheduler.plugin.task.api.log.SensitiveDataConverter"/>
<appender name="TASKLOGFILE" class="ch.qos.logback.classic.sift.SiftingAppender">
<filter class="org.apache.dolphinscheduler.plugin.task.api.log.TaskLogFilter"/>
<Discriminator class="org.apache.dolphinscheduler.plugin.task.api.log.TaskLogDiscriminator">
Expand Down
Loading