[Improvement-16999] Fix SensitiveDataConverter cannot convert empty d…

…ata (#17000)
apache · Feb 8, 2025 · 934da2e · 934da2e
1 parent 806f051
commit 934da2e
Show file tree

Hide file tree

Showing 14 changed files with 49 additions and 130 deletions.
diff --git a/docs/docs/en/architecture/design.md b/docs/docs/en/architecture/design.md
@@ -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">

diff --git a/docs/docs/zh/architecture/design.md b/docs/docs/zh/architecture/design.md
@@ -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">

diff --git a/...r-common/src/main/java/org/apache/dolphinscheduler/common/log/SensitiveDataConverter.java b/...r-common/src/main/java/org/apache/dolphinscheduler/common/log/SensitiveDataConverter.java
diff --git a/...ava/org/apache/dolphinscheduler/plugin/datasource/api/datasource/DataSourceProcessor.java b/...ava/org/apache/dolphinscheduler/plugin/datasource/api/datasource/DataSourceProcessor.java
@@ -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);
 

diff --git a/...rc/main/java/org/apache/dolphinscheduler/plugin/datasource/api/utils/DataSourceUtils.java b/...rc/main/java/org/apache/dolphinscheduler/plugin/datasource/api/utils/DataSourceUtils.java
@@ -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) {

diff --git a/dolphinscheduler-master/src/main/resources/logback-spring.xml b/dolphinscheduler-master/src/main/resources/logback-spring.xml
@@ -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">

diff --git a/dolphinscheduler-master/src/test/resources/logback.xml b/dolphinscheduler-master/src/test/resources/logback.xml
@@ -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">

diff --git a/dolphinscheduler-standalone-server/src/main/resources/logback-spring.xml b/dolphinscheduler-standalone-server/src/main/resources/logback-spring.xml
@@ -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">

diff --git a/...ler-task-api/src/main/java/org/apache/dolphinscheduler/plugin/task/api/TaskConstants.java b/...ler-task-api/src/main/java/org/apache/dolphinscheduler/plugin/task/api/TaskConstants.java
@@ -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 = "******";
 }
diff --git a/...src/main/java/org/apache/dolphinscheduler/plugin/task/api/log/SensitiveDataConverter.java b/...src/main/java/org/apache/dolphinscheduler/plugin/task/api/log/SensitiveDataConverter.java
@@ -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) {
@@ -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);
 

diff --git a/...ommon/log/SensitiveDataConverterTest.java → ...k/api/log/SensitiveDataConverterTest.java b/...ommon/log/SensitiveDataConverterTest.java → ...k/api/log/SensitiveDataConverterTest.java
@@ -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\","
@@ -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" +
@@ -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" +
@@ -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" +
@@ -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);
     }
 }
diff --git a/...task-sqoop/src/test/java/org/apache/dolphinscheduler/plugin/task/sqoop/SqoopTaskTest.java b/...task-sqoop/src/test/java/org/apache/dolphinscheduler/plugin/task/sqoop/SqoopTaskTest.java
@@ -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));

diff --git a/dolphinscheduler-worker/src/main/resources/logback-spring.xml b/dolphinscheduler-worker/src/main/resources/logback-spring.xml
@@ -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">

diff --git a/dolphinscheduler-worker/src/test/resources/logback.xml b/dolphinscheduler-worker/src/test/resources/logback.xml
@@ -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">