Skip to content

Commit d6d5755

Browse files
authored
Prevent pipe sink task id from logging secrets (#17981)
* Prevent pipe sink task id from logging secrets * Fix pipe sink subtask compatibility
1 parent d11770d commit d6d5755

13 files changed

Lines changed: 140 additions & 40 deletions

File tree

iotdb-api/pipe-api/src/main/java/org/apache/iotdb/pipe/api/customizer/parameter/PipeParameters.java

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
import java.util.HashMap;
3030
import java.util.HashSet;
3131
import java.util.List;
32+
import java.util.Locale;
3233
import java.util.Map;
3334
import java.util.Map.Entry;
3435
import java.util.Objects;
@@ -434,13 +435,14 @@ public static class ValueHider {
434435
}
435436

436437
static String hide(final String key, final String value) {
437-
if (Objects.isNull(key)) {
438-
return value;
439-
}
440-
if (KEYS.contains(KeyReducer.reduce(key))) {
438+
if (isHiddenKey(key)) {
441439
return PLACEHOLDER;
442440
}
443441
return value;
444442
}
443+
444+
public static boolean isHiddenKey(final String key) {
445+
return Objects.nonNull(key) && KEYS.contains(KeyReducer.reduce(key).toLowerCase(Locale.ROOT));
446+
}
445447
}
446448
}

iotdb-core/datanode/src/main/java/org/apache/iotdb/db/pipe/agent/task/subtask/processor/PipeProcessorSubtask.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -260,7 +260,7 @@ protected boolean executeOnce() throws Exception {
260260
throw new PipeException(
261261
String.format(
262262
"Exception in pipe process, subtask: %s, last event: %s, root cause: %s",
263-
taskID,
263+
getDisplayTaskID(),
264264
lastEvent instanceof EnrichedEvent
265265
? ((EnrichedEvent) lastEvent).coreReportMessage()
266266
: lastEvent,
@@ -300,7 +300,7 @@ public void close() {
300300
} catch (final Exception e) {
301301
LOGGER.info(
302302
DataNodePipeMessages.EXCEPTION_OCCURRED_WHEN_CLOSING_PIPE_PROCESSOR_SUBTASK,
303-
taskID,
303+
getDisplayTaskID(),
304304
ErrorHandlingCommonUtils.getRootCause(e).getMessage(),
305305
e);
306306
} finally {

iotdb-core/datanode/src/main/java/org/apache/iotdb/db/pipe/agent/task/subtask/sink/PipeSinkSubtask.java

Lines changed: 33 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@ public class PipeSinkSubtask extends PipeAbstractSinkSubtask {
6060

6161
// Record these variables to provide corresponding value to tag key of monitoring metrics
6262
private final String attributeSortedString;
63+
private final String attributeDisplayString;
6364
private final int sinkIndex;
6465

6566
// Now parallel connectors run the same time, thus the heartbeat events are not sure
@@ -75,8 +76,27 @@ public PipeSinkSubtask(
7576
final int sinkIndex,
7677
final UnboundedBlockingPendingQueue<Event> inputPendingQueue,
7778
final PipeConnector outputPipeConnector) {
79+
this(
80+
taskID,
81+
creationTime,
82+
attributeSortedString,
83+
attributeSortedString,
84+
sinkIndex,
85+
inputPendingQueue,
86+
outputPipeConnector);
87+
}
88+
89+
public PipeSinkSubtask(
90+
final String taskID,
91+
final long creationTime,
92+
final String attributeSortedString,
93+
final String attributeDisplayString,
94+
final int sinkIndex,
95+
final UnboundedBlockingPendingQueue<Event> inputPendingQueue,
96+
final PipeConnector outputPipeConnector) {
7897
super(taskID, creationTime, outputPipeConnector);
7998
this.attributeSortedString = attributeSortedString;
99+
this.attributeDisplayString = attributeDisplayString;
80100
this.sinkIndex = sinkIndex;
81101
this.inputPendingQueue = inputPendingQueue;
82102

@@ -156,7 +176,7 @@ private void transferHeartbeatEvent(final PipeHeartbeatEvent event) {
156176
DataNodePipeMessages.PIPECONNECTOR
157177
+ outputPipeSink.getClass().getName()
158178
+ "(id: "
159-
+ taskID
179+
+ getDisplayTaskID()
160180
+ ")"
161181
+ " heartbeat failed, or encountered failure when transferring generic event. Failure: "
162182
+ e.getMessage(),
@@ -181,13 +201,13 @@ public void close() {
181201
outputPipeSink.close();
182202
LOGGER.info(
183203
DataNodePipeMessages.PIPE_CONNECTOR_SUBTASK_WAS_CLOSED_WITHIN_MS,
184-
taskID,
204+
getDisplayTaskID(),
185205
outputPipeSink,
186206
System.currentTimeMillis() - startTime);
187207
} catch (final Exception e) {
188208
LOGGER.info(
189209
DataNodePipeMessages.EXCEPTION_OCCURRED_WHEN_CLOSING_PIPE_CONNECTOR_SUBTASK,
190-
taskID,
210+
getDisplayTaskID(),
191211
ErrorHandlingCommonUtils.getRootCause(e).getMessage(),
192212
e);
193213
} finally {
@@ -366,4 +386,14 @@ protected void report(final EnrichedEvent event, final PipeRuntimeException exce
366386
lastExceptionTime = Long.MAX_VALUE;
367387
PipeDataNodeAgent.runtime().report(event, exception);
368388
}
389+
390+
@Override
391+
public String getDisplayTaskID() {
392+
return generateDisplayTaskID(attributeDisplayString, creationTime, sinkIndex);
393+
}
394+
395+
static String generateDisplayTaskID(
396+
final String attributeDisplayString, final long creationTime, final int sinkIndex) {
397+
return String.format("%s_%s_%s", attributeDisplayString, creationTime, sinkIndex);
398+
}
369399
}

iotdb-core/datanode/src/main/java/org/apache/iotdb/db/pipe/agent/task/subtask/sink/PipeSinkSubtaskLifeCycle.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ public synchronized void register() {
7575
registeredTaskCount++;
7676
LOGGER.info(
7777
DataNodePipeMessages.REGISTER_SUBTASK_RUNNINGTASKCOUNT_REGISTEREDTASKCOUNT,
78-
subtask,
78+
subtask.getDisplayTaskID(),
7979
runningTaskCount,
8080
registeredTaskCount);
8181
}
@@ -112,7 +112,7 @@ public synchronized boolean deregister(final CommitterKey committerKey) {
112112
registeredTaskCount--;
113113
LOGGER.info(
114114
DataNodePipeMessages.DEREGISTER_SUBTASK_RUNNINGTASKCOUNT_REGISTEREDTASKCOUNT,
115-
subtask,
115+
subtask.getDisplayTaskID(),
116116
runningTaskCount,
117117
registeredTaskCount);
118118
}
@@ -135,7 +135,7 @@ public synchronized void start() {
135135
runningTaskCount++;
136136
LOGGER.info(
137137
DataNodePipeMessages.START_SUBTASK_RUNNINGTASKCOUNT_REGISTEREDTASKCOUNT,
138-
subtask,
138+
subtask.getDisplayTaskID(),
139139
runningTaskCount,
140140
registeredTaskCount);
141141
}
@@ -152,7 +152,7 @@ public synchronized void stop() {
152152
runningTaskCount--;
153153
LOGGER.info(
154154
DataNodePipeMessages.STOP_SUBTASK_RUNNINGTASKCOUNT_REGISTEREDTASKCOUNT,
155-
subtask,
155+
subtask.getDisplayTaskID(),
156156
runningTaskCount,
157157
registeredTaskCount);
158158
}

iotdb-core/datanode/src/main/java/org/apache/iotdb/db/pipe/agent/task/subtask/sink/PipeSinkSubtaskManager.java

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -148,8 +148,7 @@ public synchronized String register(
148148
for (int sinkIndex = 0; sinkIndex < sinkNum; sinkIndex++) {
149149
final String taskID =
150150
String.format(
151-
"%s_%s_%s",
152-
attributeDisplayStringWithPrefix, environment.getCreationTime(), sinkIndex);
151+
"%s_%s_%s", attributeSortedString, environment.getCreationTime(), sinkIndex);
153152
environment.setSinkTaskId(taskID);
154153

155154
final PipeConnector pipeSink =
@@ -184,6 +183,7 @@ public synchronized String register(
184183
new PipeSinkSubtask(
185184
taskID,
186185
environment.getCreationTime(),
186+
attributeSortedString,
187187
attributeDisplayStringWithPrefix,
188188
sinkIndex,
189189
pendingQueue,
@@ -293,13 +293,15 @@ private static String generateAttributeSortedString(
293293
return sortedStringSourceMap.toString();
294294
}
295295

296-
/** Masked attribute string for logs, metrics and exception messages. */
297-
private static String generateAttributeDisplayString(
298-
final PipeParameters pipeConnectorParameters) {
296+
/**
297+
* Attribute string for logs, metrics and exception messages with sensitive attributes removed.
298+
*/
299+
static String generateAttributeDisplayString(final PipeParameters pipeConnectorParameters) {
299300
final TreeMap<String, String> filteredAttributes =
300301
new TreeMap<>(pipeConnectorParameters.getAttribute());
301302
filteredAttributes.remove(SystemConstant.RESTART_OR_NEWLY_ADDED_KEY);
302-
return new PipeParameters(filteredAttributes).toString();
303+
filteredAttributes.keySet().removeIf(PipeParameters.ValueHider::isHiddenKey);
304+
return filteredAttributes.toString();
303305
}
304306

305307
private void throwNoSuchSubtaskException(final String attributeSortedString) {

iotdb-core/datanode/src/main/java/org/apache/iotdb/db/pipe/metric/schema/PipeSchemaRegionSinkMetrics.java

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,9 @@ public void register(final PipeSinkSubtask pipeSinkSubtask) {
110110

111111
public void deregister(final String taskID) {
112112
if (!connectorMap.containsKey(taskID)) {
113-
LOGGER.warn(DataNodePipeMessages.FAILED_TO_DEREGISTER_PIPE_SCHEMA_REGION_CONNECTOR, taskID);
113+
LOGGER.warn(
114+
DataNodePipeMessages.FAILED_TO_DEREGISTER_PIPE_SCHEMA_REGION_CONNECTOR,
115+
getDisplayTaskID(taskID));
114116
return;
115117
}
116118
if (Objects.nonNull(metricService)) {
@@ -125,12 +127,18 @@ public void markSchemaEvent(final String taskID) {
125127
}
126128
final Rate rate = schemaRateMap.get(taskID);
127129
if (rate == null) {
128-
LOGGER.info(DataNodePipeMessages.FAILED_TO_MARK_PIPE_SCHEMA_REGION_WRITE, taskID);
130+
LOGGER.info(
131+
DataNodePipeMessages.FAILED_TO_MARK_PIPE_SCHEMA_REGION_WRITE, getDisplayTaskID(taskID));
129132
return;
130133
}
131134
rate.mark();
132135
}
133136

137+
private String getDisplayTaskID(final String taskID) {
138+
final PipeSinkSubtask connector = connectorMap.get(taskID);
139+
return Objects.nonNull(connector) ? connector.getDisplayTaskID() : "unknown";
140+
}
141+
134142
//////////////////////////// singleton ////////////////////////////
135143

136144
private static class PipeSchemaRegionSinkMetricsHolder {

iotdb-core/datanode/src/main/java/org/apache/iotdb/db/pipe/metric/sink/PipeDataRegionSinkMetrics.java

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -447,7 +447,9 @@ public void register(final PipeSinkSubtask pipeSinkSubtask) {
447447

448448
public void deregister(final String taskID) {
449449
if (!sinkMap.containsKey(taskID)) {
450-
LOGGER.warn(DataNodePipeMessages.FAILED_TO_DEREGISTER_PIPE_DATA_REGION_SINK, taskID);
450+
LOGGER.warn(
451+
DataNodePipeMessages.FAILED_TO_DEREGISTER_PIPE_DATA_REGION_SINK,
452+
getDisplayTaskID(taskID));
451453
return;
452454
}
453455
if (Objects.nonNull(metricService)) {
@@ -462,7 +464,8 @@ public void markTabletEvent(final String taskID) {
462464
}
463465
final Rate rate = tabletRateMap.get(taskID);
464466
if (rate == null) {
465-
LOGGER.info(DataNodePipeMessages.FAILED_TO_MARK_PIPE_DATA_REGION_SINK, taskID);
467+
LOGGER.info(
468+
DataNodePipeMessages.FAILED_TO_MARK_PIPE_DATA_REGION_SINK, getDisplayTaskID(taskID));
466469
return;
467470
}
468471
rate.mark();
@@ -474,12 +477,18 @@ public void markTsFileEvent(final String taskID) {
474477
}
475478
final Rate rate = tsFileRateMap.get(taskID);
476479
if (rate == null) {
477-
LOGGER.info(DataNodePipeMessages.FAILED_TO_MARK_PIPE_DATA_REGION_SINK_1, taskID);
480+
LOGGER.info(
481+
DataNodePipeMessages.FAILED_TO_MARK_PIPE_DATA_REGION_SINK_1, getDisplayTaskID(taskID));
478482
return;
479483
}
480484
rate.mark();
481485
}
482486

487+
private String getDisplayTaskID(final String taskID) {
488+
final PipeSinkSubtask sink = sinkMap.get(taskID);
489+
return Objects.nonNull(sink) ? sink.getDisplayTaskID() : "unknown";
490+
}
491+
483492
public void markPipeHeartbeatEvent(final String taskID) {
484493
if (Objects.isNull(metricService)) {
485494
return;

iotdb-core/datanode/src/test/java/org/apache/iotdb/db/pipe/agent/task/PipeSinkSubtaskExecutorTest.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ public void setUp() throws Exception {
4141
"PipeConnectorSubtaskExecutorTest",
4242
System.currentTimeMillis(),
4343
"TestAttributeSortedString",
44+
"TestAttributeSortedString",
4445
0,
4546
mock(UnboundedBlockingPendingQueue.class),
4647
mock(PipeConnector.class)));

iotdb-core/datanode/src/test/java/org/apache/iotdb/db/pipe/agent/task/subtask/sink/PipeSinkSubtaskTest.java

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,12 +23,18 @@
2323
import org.apache.iotdb.commons.pipe.agent.task.progress.CommitterKey;
2424
import org.apache.iotdb.commons.pipe.sink.protocol.PipeConnectorWithEventDiscard;
2525
import org.apache.iotdb.pipe.api.PipeConnector;
26+
import org.apache.iotdb.pipe.api.event.Event;
27+
import org.apache.iotdb.pipe.api.exception.PipeException;
2628

29+
import org.junit.Assert;
2730
import org.junit.Test;
2831
import org.mockito.Mockito;
2932

33+
import static org.mockito.ArgumentMatchers.any;
34+
import static org.mockito.Mockito.doThrow;
3035
import static org.mockito.Mockito.mock;
3136
import static org.mockito.Mockito.verify;
37+
import static org.mockito.Mockito.when;
3238
import static org.mockito.Mockito.withSettings;
3339

3440
public class PipeSinkSubtaskTest {
@@ -47,6 +53,7 @@ public void testDiscardEventsOfPipeDelegatesToConnector() {
4753
"PipeSinkSubtaskTest",
4854
System.currentTimeMillis(),
4955
"data_test",
56+
"data_test",
5057
0,
5158
(UnboundedBlockingPendingQueue) pendingQueue,
5259
connector));
@@ -60,4 +67,41 @@ public void testDiscardEventsOfPipeDelegatesToConnector() {
6067
subtask.close();
6168
}
6269
}
70+
71+
@Test
72+
public void testTransferExceptionUsesDisplayTaskID() throws Exception {
73+
final PipeConnector connector = mock(PipeConnector.class);
74+
final UnboundedBlockingPendingQueue<Event> pendingQueue =
75+
mock(UnboundedBlockingPendingQueue.class);
76+
final Event event = mock(Event.class);
77+
78+
when(pendingQueue.waitedPoll()).thenReturn(event);
79+
doThrow(new RuntimeException("No more authentication methods available"))
80+
.when(connector)
81+
.transfer(any(Event.class));
82+
83+
final PipeSinkSubtask subtask =
84+
new PipeSinkSubtask(
85+
"data_{sink=TSFILE_REMOTE_SINK, sink.scp.password=Iotdb@2026}_1701687309493_0",
86+
1701687309493L,
87+
"data_{sink=TSFILE_REMOTE_SINK, sink.scp.password=Iotdb@2026}",
88+
"data_{sink=TSFILE_REMOTE_SINK, sink.scp.host=172.20.70.119}",
89+
0,
90+
pendingQueue,
91+
connector);
92+
93+
try {
94+
subtask.executeOnce();
95+
Assert.fail();
96+
} catch (final PipeException e) {
97+
Assert.assertTrue(e.getMessage().contains("Exception in pipe transfer, subtask: data_{"));
98+
Assert.assertTrue(e.getMessage().contains("sink=TSFILE_REMOTE_SINK"));
99+
Assert.assertTrue(e.getMessage().contains("sink.scp.host=172.20.70.119"));
100+
Assert.assertTrue(e.getMessage().contains("No more authentication methods available"));
101+
Assert.assertFalse(e.getMessage().contains("sink.scp.password"));
102+
Assert.assertFalse(e.getMessage().contains("Iotdb@2026"));
103+
} finally {
104+
subtask.close();
105+
}
106+
}
63107
}

iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/pipe/agent/task/execution/PipeSubtaskExecutor.java

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ protected PipeSubtaskExecutor(
9999

100100
public final synchronized void register(final PipeSubtask subtask) {
101101
if (registeredIdSubtaskMapper.containsKey(subtask.getTaskID())) {
102-
LOGGER.warn(PipeMessages.SUBTASK_ALREADY_REGISTERED, getSafeSubtaskStr(subtask.getTaskID()));
102+
LOGGER.warn(PipeMessages.SUBTASK_ALREADY_REGISTERED, subtask.getDisplayTaskID());
103103
return;
104104
}
105105

@@ -125,13 +125,13 @@ public final synchronized void start(final String subTaskID) {
125125
final PipeSubtask subtask = registeredIdSubtaskMapper.get(subTaskID);
126126
if (subtask.isSubmittingSelf()) {
127127
if (LOGGER.isDebugEnabled()) {
128-
LOGGER.debug(PipeMessages.SUBTASK_ALREADY_RUNNING, getSafeSubtaskStr(subTaskID));
128+
LOGGER.debug(PipeMessages.SUBTASK_ALREADY_RUNNING, subtask.getDisplayTaskID());
129129
}
130130
} else {
131131
subtask.allowSubmittingSelf();
132132
subtask.submitSelf();
133133
++runningSubtaskNumber;
134-
LOGGER.info(PipeMessages.SUBTASK_STARTED, getSafeSubtaskStr(subTaskID));
134+
LOGGER.info(PipeMessages.SUBTASK_STARTED, subtask.getDisplayTaskID());
135135
}
136136
}
137137

@@ -154,9 +154,9 @@ public final synchronized void deregister(final String subTaskID) {
154154
if (subtask != null) {
155155
try {
156156
subtask.close();
157-
LOGGER.info(PipeMessages.SUBTASK_CLOSED, getSafeSubtaskStr(subTaskID));
157+
LOGGER.info(PipeMessages.SUBTASK_CLOSED, subtask.getDisplayTaskID());
158158
} catch (final Exception e) {
159-
LOGGER.error(PipeMessages.SUBTASK_CLOSE_FAILED, getSafeSubtaskStr(subTaskID), e);
159+
LOGGER.error(PipeMessages.SUBTASK_CLOSE_FAILED, subtask.getDisplayTaskID(), e);
160160
}
161161
}
162162
}

0 commit comments

Comments
 (0)