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

runtime-v2: mask workDir value in logs by default #1052

Merged
merged 3 commits into from
Dec 19, 2024
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
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ public class AgentConfiguration {

private final Path logDir;
private final long logMaxDelay;
private final boolean workDirMasking;

private final int workersCount;
private final long pollInterval;
Expand Down Expand Up @@ -81,6 +82,7 @@ public AgentConfiguration(Config cfg) {

this.logDir = getOrCreatePath(cfg, "logDir");
this.logMaxDelay = cfg.getDuration("logMaxDelay", TimeUnit.MILLISECONDS);
this.workDirMasking = cfg.getBoolean("workDirMasking");

this.workersCount = cfg.getInt("workersCount");
this.maintenanceModeListenerHost = cfg.getString("maintenanceModeListenerHost");
Expand Down Expand Up @@ -136,6 +138,10 @@ public long getLogMaxDelay() {
return logMaxDelay;
}

public boolean isWorkDirMaskings() {
return workDirMasking;
}

public int getWorkersCount() {
return workersCount;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,7 @@ public JobExecutor create(JobRequest.Type jobType) {
.exposeDockerDaemon(dockerCfg.exposeDockerDaemon())
.maxHeartbeatInterval(serverCfg.getMaxNoHeartbeatInterval())
.segmentedLogs(segmentedLogs)
.workDirMasking(agentCfg.isWorkDirMaskings())
.persistentWorkDir(runnerCfg.getPersistentWorkDir())
.preforkEnabled(preForkCfg.isEnabled())
.cleanRunnerDescendants(runnerCfg.getCleanRunnerDescendants())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,7 @@ private static RunnerConfiguration createRunnerConfiguration(RunnerJobExecutorCo
.logging(LoggingConfiguration.builder()
.sendSystemOutAndErrToSLF4J(true)
.segmentedLogs(execCfg.segmentedLogs())
.workDirMasking(execCfg.workDirMasking())
.build())
.build();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -759,6 +759,8 @@ public interface RunnerJobExecutorConfiguration {

boolean segmentedLogs();

boolean workDirMasking();

@Value.Default
default List<String> extraDockerVolumes() {
return Collections.emptyList();
Expand Down
3 changes: 3 additions & 0 deletions agent/src/main/resources/concord-agent.conf
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,9 @@ concord-agent {
# determines how ofter the logs are send back to the server
logMaxDelay = "2 seconds"

# replace the current process' workDir in logs with literal "$WORK_DIR"
workDirMasking = true

# maximum number of concurrent processes
workersCount = 3
workersCount = ${?WORKERS_COUNT}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
<filter class="com.walmartlabs.concord.runtime.v2.runner.logging.LogLevelFilter" />

<encoder class="com.walmartlabs.concord.runtime.v2.runner.logging.ConcordLogEncoder">
<layout class="com.walmartlabs.concord.runtime.v2.runner.logging.MaskingSensitiveDataLayout">
<layout class="com.walmartlabs.concord.runtime.v2.runner.logging.CustomLayout">
<!-- the UI expects log timestamps in a specific format to be able to convert it to the local time -->
<pattern>%date{yyyy-MM-dd'T'HH:mm:ss.SSSZ, UTC} [%-5level] %msg%n%rEx{full, com.sun, sun}</pattern>
</layout>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,15 @@ default boolean sendSystemOutAndErrToSLF4J() {
return true;
}

/**
* If {@code true}, any ${workDir} value will be replaced with literal
* "$WORK_DIR" string. Reduces noise in the logs.
*/
@Value.Default
default boolean workDirMasking() {
return true;
}

static ImmutableLoggingConfiguration.Builder builder() {
return ImmutableLoggingConfiguration.builder();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,8 @@
import com.walmartlabs.concord.runtime.v2.runner.vm.ParallelCommand;
import com.walmartlabs.concord.runtime.v2.sdk.*;
import com.walmartlabs.concord.sdk.Constants;
import com.walmartlabs.concord.svm.*;
import com.walmartlabs.concord.svm.Runtime;
import com.walmartlabs.concord.svm.*;
import org.junit.jupiter.api.extension.AfterEachCallback;
import org.junit.jupiter.api.extension.BeforeEachCallback;
import org.junit.jupiter.api.extension.ExtensionContext;
Expand All @@ -70,7 +70,7 @@
import java.util.regex.Matcher;
import java.util.regex.Pattern;

import static org.junit.jupiter.api.Assertions.*;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.junit.jupiter.api.Assertions.fail;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.spy;
Expand Down Expand Up @@ -196,7 +196,10 @@ public byte[] run(RunnerConfiguration baseCfg) throws Exception {
}

ImmutableRunnerConfiguration.Builder runnerCfg = RunnerConfiguration.builder()
.logging(LoggingConfiguration.builder().segmentedLogs(false).build());
.logging(LoggingConfiguration.builder()
.segmentedLogs(false)
.workDirMasking(false)
.build());

if (baseCfg != null) {
runnerCfg.from(baseCfg);
Expand Down Expand Up @@ -377,7 +380,7 @@ protected void configure() {
taskCallListeners.addBinding().to(TaskResultListener.class);

Multibinder<ExecutionListener> executionListeners = Multibinder.newSetBinder(binder(), ExecutionListener.class);
executionListeners.addBinding().toInstance(new ExecutionListener(){
executionListeners.addBinding().toInstance(new ExecutionListener() {
@Override
public void beforeProcessStart(Runtime runtime, State state) {
SensitiveDataHolder.getInstance().get().clear();
Expand All @@ -390,7 +393,7 @@ public void beforeProcessStart(Runtime runtime, State state) {
@Override
public Result afterCommand(Runtime runtime, VM vm, State state, ThreadId threadId, Command cmd) {
if (cmd instanceof BlockCommand
|| cmd instanceof ParallelCommand) {
|| cmd instanceof ParallelCommand) {
return ExecutionListener.super.afterCommand(runtime, vm, state, threadId, cmd);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -625,6 +625,7 @@ public void testSegmentedLogging() throws Exception {
RunnerConfiguration runnerCfg = RunnerConfiguration.builder()
.logging(LoggingConfiguration.builder()
.sendSystemOutAndErrToSLF4J(false)
.workDirMasking(false)
.build())
.build();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
import com.walmartlabs.concord.runtime.v2.runner.guice.CurrentClasspathModule;
import com.walmartlabs.concord.runtime.v2.runner.guice.DefaultRunnerModule;
import com.walmartlabs.concord.runtime.v2.runner.guice.ProcessDependenciesModule;
import com.walmartlabs.concord.runtime.v2.runner.logging.CustomLayout;
import com.walmartlabs.concord.runtime.v2.runner.tasks.V2;
import com.walmartlabs.concord.runtime.v2.sdk.ProcessConfiguration;
import com.walmartlabs.concord.runtime.v2.sdk.Task;
Expand Down Expand Up @@ -123,6 +124,10 @@ private ConfigurationModule(WorkingDirectory workDir,
@Override
protected void configure() {
bind(WorkingDirectory.class).toInstance(workDir);
if (runnerCfg.logging().workDirMasking()) {
CustomLayout.enableWorkingDirectoryMasking(workDir);
}

bind(RunnerConfiguration.class).toInstance(runnerCfg);
bind(ProcessConfiguration.class).toProvider(processCfgProvider);
bind(InstanceId.class).toProvider(InstanceIdProvider.class);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,17 +23,31 @@
import ch.qos.logback.classic.PatternLayout;
import ch.qos.logback.classic.spi.ILoggingEvent;
import com.walmartlabs.concord.runtime.v2.runner.SensitiveDataHolder;
import com.walmartlabs.concord.runtime.v2.sdk.WorkingDirectory;

import java.util.Collection;
import static java.util.Objects.requireNonNull;

public class MaskingSensitiveDataLayout extends PatternLayout {
public class CustomLayout extends PatternLayout {

private static volatile String workDirToReplace;

/**
* Enables masking of ${workDir} values in logs. Such values often add noise to logs.
*/
public static void enableWorkingDirectoryMasking(WorkingDirectory workDir) {
requireNonNull(workDir);
CustomLayout.workDirToReplace = workDir.getValue().toString();
}

@Override
public String doLayout(ILoggingEvent event) {
Collection<String> sensitiveData = SensitiveDataHolder.getInstance().get();
String msg = super.doLayout(event);
for (String d : sensitiveData) {
msg = msg.replace(d, "******");
var sensitiveData = SensitiveDataHolder.getInstance().get();
var msg = super.doLayout(event);
for (var sensitiveString : sensitiveData) {
msg = msg.replace(sensitiveString, "******");
}
if (CustomLayout.workDirToReplace != null) {
msg = msg.replace(workDirToReplace, "$WORK_DIR");
}
return msg;
}
Expand Down
4 changes: 2 additions & 2 deletions runtime/v2/runner/src/main/resources/logback.xml
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
<filter class="com.walmartlabs.concord.runtime.v2.runner.logging.LogLevelFilter" />

<encoder class="com.walmartlabs.concord.runtime.v2.runner.logging.ConcordLogEncoder">
<layout class="com.walmartlabs.concord.runtime.v2.runner.logging.MaskingSensitiveDataLayout">
<layout class="com.walmartlabs.concord.runtime.v2.runner.logging.CustomLayout">
<!-- the UI expects log timestamps in a specific format to be able to convert it to the local time -->
<pattern>%date{yyyy-MM-dd'T'HH:mm:ss.SSSZ, UTC} [%-5level] %msg%n%rEx{full, com.sun, sun}</pattern>
</layout>
Expand All @@ -14,7 +14,7 @@
<filter class="com.walmartlabs.concord.runtime.v2.runner.logging.LogLevelFilter" />

<encoder class="com.walmartlabs.concord.runtime.v2.runner.logging.ConcordLogEncoder">
<layout class="com.walmartlabs.concord.runtime.v2.runner.logging.MaskingSensitiveDataLayout">
<layout class="com.walmartlabs.concord.runtime.v2.runner.logging.CustomLayout">
<!-- the UI expects log timestamps in a specific format to be able to convert it to the local time -->
<pattern>%date{yyyy-MM-dd'T'HH:mm:ss.SSSZ, UTC} %msg%n</pattern>
</layout>
Expand Down
Loading