Skip to content

Commit

Permalink
Revert "[WOR-1560] Upgrade to TCL 1.1.1 for MDC-aware Stairway (#418)"
Browse files Browse the repository at this point in the history
This reverts commit 530a866.

Reason: TCL's upgrade to k8s client 20.0.1 broke its KubePodListener, which could affect the recovery of Stairway flights.
  • Loading branch information
okotsopoulos committed Mar 28, 2024
1 parent 5c2fd3d commit f89aa96
Show file tree
Hide file tree
Showing 10 changed files with 202 additions and 38 deletions.
18 changes: 0 additions & 18 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -76,23 +76,5 @@ Automatically fix linting issues:
./gradlew generateSwaggerCode
```

### Running SourceClear locally

[SourceClear](https://srcclr.github.io) is a static analysis tool that scans a project's Java
dependencies for known vulnerabilities. If you are working on addressing dependency vulnerabilities
in response to a SourceClear finding and want to run a scan locally, you need to get the API token
from Vault before running the Gradle task.

```sh
export SRCCLR_API_TOKEN=$(vault read -field=api_token secret/secops/ci/srcclr/gradle-agent)
./gradlew srcclr
```

High-level results are outputted to the terminal.
Full results are uploaded to [Veracode](https://sca.analysiscenter.veracode.com/workspaces/jppForw/projects/551485/issues)
(if running off of a feature branch, navigate to Project Details > Selected Branch > Change to select your feature branch).
You can request a Veracode account to view full results from #dsp-infosec-champions.


## Tech Stack
BPM adheres to the [Terra Tech Stack](https://docs.google.com/document/d/1JkTrtaci7EI0TnuR-68zYgTx_mRCNu-u2eV9XhexWTI/edit#heading=h.5z6knaqygr4a). See linked document for relevant technology choices and the rationale behind their inclusion in this service.
3 changes: 1 addition & 2 deletions buildSrc/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,8 @@ dependencies {
implementation 'de.undercouch.download:de.undercouch.download.gradle.plugin:5.6.0'
implementation 'io.spring.dependency-management:io.spring.dependency-management.gradle.plugin:1.1.4'
implementation 'org.hidetake.swagger.generator:org.hidetake.swagger.generator.gradle.plugin:2.19.2'
implementation 'org.springframework.boot:spring-boot-gradle-plugin:3.2.3'
implementation 'org.springframework.boot:spring-boot-gradle-plugin:3.1.2'
implementation 'bio.terra:terra-test-runner:0.2.0-SNAPSHOT'
implementation 'com.srcclr.gradle:com.srcclr.gradle.gradle.plugin:3.1.12'

// Transitive dependency constraints due to security vulnerabilities in prior versions.
// These are not directly included, they are just constrained if they are pulled in as
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ plugins {

id 'com.diffplug.spotless'
id 'org.hidetake.swagger.generator'
id 'com.srcclr.gradle'
}

boolean isCiServer = System.getenv().containsKey("CI")
Expand Down Expand Up @@ -40,10 +39,23 @@ repositories {
dependencies {
compileOnly "com.google.code.findbugs:annotations:3.0.1"
implementation 'io.swagger.core.v3:swagger-annotations:2.1.12'
implementation 'io.opentelemetry.instrumentation:opentelemetry-instrumentation-annotations:2.2.0'
swaggerCodegen 'io.swagger.codegen.v3:swagger-codegen-cli:3.0.47'

implementation 'org.slf4j:slf4j-api'

testImplementation 'ch.qos.logback:logback-classic'
testImplementation 'org.hamcrest:hamcrest:2.2'

var openTelemetryVersion = '1.31.0'
implementation "io.opentelemetry:opentelemetry-api:${openTelemetryVersion}"
implementation "io.opentelemetry:opentelemetry-sdk:${openTelemetryVersion}"
implementation "io.opentelemetry:opentelemetry-sdk-metrics:${openTelemetryVersion}"
implementation "io.opentelemetry:opentelemetry-exporter-logging:${openTelemetryVersion}"
implementation "io.opentelemetry.semconv:opentelemetry-semconv:1.21.0-alpha"
implementation "io.opentelemetry.instrumentation:opentelemetry-spring-webmvc-6.0:${openTelemetryVersion}-alpha"
implementation "io.opentelemetry.instrumentation:opentelemetry-instrumentation-annotations:${openTelemetryVersion}"
implementation "io.opentelemetry.instrumentation:opentelemetry-spring-boot:${openTelemetryVersion}-alpha"
implementation "com.google.cloud.opentelemetry:exporter-trace:0.25.2"
}

// Modify the standard :test task to only include unit-tagged tests.
Expand Down Expand Up @@ -94,7 +106,3 @@ jacocoTestReport {
xml.required = true
}
}

srcclr {
scope = "runtimeClasspath"
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,5 @@ plugins {
id 'org.springframework.boot'
}

// Spring Boot 3.2.3 pulls in opentelemetry-bom 1.31.0.
// It must have version >= 1.34.1 for compatibility with terra-common-lib 1.0.9:
ext['opentelemetry.version'] = '1.36.0'
// Spring Boot 3.1.2 pulls in postgresql 42.6.0, vulnerable to CVE-2024-1597
ext['postgresql.version'] = '42.7.2'
5 changes: 2 additions & 3 deletions service/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ apply from: 'publishing.gradle'
dependencies {
implementation 'io.sentry:sentry:7.6.0'

implementation 'bio.terra:terra-common-lib:1.1.1-SNAPSHOT'
implementation 'bio.terra:terra-common-lib:0.1.9-SNAPSHOT'

implementation 'org.apache.commons:commons-dbcp2'
implementation 'org.springframework.boot:spring-boot-starter-data-jdbc'
Expand All @@ -27,8 +27,7 @@ dependencies {
implementation group: "org.apache.commons", name: "commons-lang3"
implementation group: 'jakarta.ws.rs', name: 'jakarta.ws.rs-api', version: '3.1.0'
implementation group: 'com.nimbusds', name: 'nimbus-jose-jwt', version: '9.37.3'

implementation 'org.springframework.boot:spring-boot-starter-actuator'
implementation 'org.springframework.boot:spring-boot-starter-actuator:2.7.18'
implementation 'io.micrometer:micrometer-registry-prometheus'

// Google dependencies
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
package bio.terra.profile.app.common;

import bio.terra.common.exception.InternalServerErrorException;
import java.util.List;

public class MDCHandlingException extends InternalServerErrorException {

public MDCHandlingException(String message) {
super(message);
}

public MDCHandlingException(String message, Throwable cause) {
super(message, cause);
}

public MDCHandlingException(Throwable cause) {
super(cause);
}

public MDCHandlingException(String message, List<String> causes) {
super(message, causes);
}

public MDCHandlingException(String message, Throwable cause, List<String> causes) {
super(message, cause, causes);
}
}
136 changes: 136 additions & 0 deletions service/src/main/java/bio/terra/profile/app/common/MdcHook.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,136 @@
package bio.terra.profile.app.common;

import bio.terra.stairway.FlightContext;
import bio.terra.stairway.FlightMap;
import bio.terra.stairway.HookAction;
import bio.terra.stairway.StairwayHook;
import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.core.type.TypeReference;
import com.fasterxml.jackson.databind.ObjectMapper;
import jakarta.annotation.Nullable;
import java.util.Map;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.slf4j.MDC;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.stereotype.Component;

/**
* A {@link StairwayHook} for propagating MDC context across steps using the input {@link
* FlightMap}.
*
* <p>This allows steps to have the same MDC context as when their flight was created. Note that any
* modifications to the MDC context within a step are not propagated to other steps.
*/
@Component
public class MdcHook implements StairwayHook {
private static final Logger logger = LoggerFactory.getLogger(MdcHook.class);
private static final String FLIGHT_LOG_FORMAT = "Operation: {}, flightClass: {}, flightId: {}";
private static final String STEP_LOG_FORMAT =
"Operation: {}, flightClass: {}, flightId: {}, stepClass: {}, stepIndex: {}, direction: {}";

/** The key to use in {@link FlightMap} for storing the MDC context. */
public static final String MDC_FLIGHT_MAP_KEY = "mdcKey";

private static final TypeReference<Map<String, String>> mapType = new TypeReference<>() {};

private final ObjectMapper objectMapper;

@Autowired
public MdcHook(ObjectMapper objectMapper) {
this.objectMapper = objectMapper;
}

/**
* Returns a serialized version of the current MDC context.
*
* <p>This is meant to be used to set up the initial {@link FlightMap} when a Flight is being
* created to propagate the context of the flight creation to the steps.
*/
public Object getSerializedCurrentContext() {
return serializeMdc(MDC.getCopyOfContextMap());
}

@Override
public HookAction startStep(FlightContext flightContext) {
logger.info(
STEP_LOG_FORMAT,
"startStep",
flightContext.getFlightClassName(),
flightContext.getFlightId(),
flightContext.getStepClassName(),
flightContext.getStepIndex(),
flightContext.getDirection().name());
String serializedMdc = flightContext.getInputParameters().get(MDC_FLIGHT_MAP_KEY, String.class);
// Note that this destroys any previous context on this thread.
MDC.setContextMap(deserializeMdc(serializedMdc));
return HookAction.CONTINUE;
}

@Override
public HookAction endStep(FlightContext flightContext) {
logger.info(
STEP_LOG_FORMAT,
"endStep",
flightContext.getFlightClassName(),
flightContext.getFlightId(),
flightContext.getStepClassName(),
flightContext.getStepIndex(),
flightContext.getDirection().name());
MDC.clear();
return HookAction.CONTINUE;
}

@Override
public HookAction startFlight(FlightContext flightContext) {
logger.info(
FLIGHT_LOG_FORMAT,
"startFlight",
flightContext.getFlightClassName(),
flightContext.getFlightId());
return HookAction.CONTINUE;
}

@Override
public HookAction endFlight(FlightContext flightContext) {
logger.info(
FLIGHT_LOG_FORMAT,
"endFlight",
flightContext.getFlightClassName(),
flightContext.getFlightId());
return HookAction.CONTINUE;
}

private String serializeMdc(@Nullable Map<String, String> mdcMap) {
try {
return objectMapper.writeValueAsString(mdcMap);
} catch (JsonProcessingException e) {
throw new MDCHandlingException(
"Error serializing MDC map from string: "
+ (mdcMap == null ? "null" : mdcMap.toString()));
}
}

private Map<String, String> deserializeMdc(@Nullable String serializedMdc) {
if (serializedMdc == null) {
return Map.of();
}
Map<String, String> mdcContext;
try {
mdcContext = objectMapper.readValue(serializedMdc, mapType);
} catch (JsonProcessingException e) {
throw new MDCHandlingException("Error deserializing MDC map from string: " + serializedMdc);
}
if (mdcContext == null) {
return Map.of();
}
return mdcContext;
}

@Override
public HookAction stateTransition(FlightContext context) throws InterruptedException {
logger.info(
"Flight ID {} changed status to {}.", context.getFlightId(), context.getFlightStatus());
return HookAction.CONTINUE;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import bio.terra.common.iam.AuthenticatedUserRequest;
import bio.terra.common.stairway.MonitoringHook;
import bio.terra.common.stairway.StairwayComponent;
import bio.terra.profile.app.common.MdcHook;
import bio.terra.profile.service.job.exception.InvalidJobIdException;
import bio.terra.profile.service.job.exception.InvalidJobParameterException;
import bio.terra.stairway.Flight;
Expand All @@ -16,6 +17,7 @@
public class JobBuilder {
private final JobService jobService;
private final StairwayComponent stairwayComponent;
private final MdcHook mdcHook;
private final FlightMap jobParameterMap;
private final OpenTelemetry openTelemetry;
@Nullable private Class<? extends Flight> flightClass;
Expand All @@ -25,9 +27,13 @@ public class JobBuilder {
@Nullable private AuthenticatedUserRequest userRequest;

public JobBuilder(
JobService jobService, StairwayComponent stairwayComponent, OpenTelemetry openTelemetry) {
JobService jobService,
StairwayComponent stairwayComponent,
MdcHook mdcHook,
OpenTelemetry openTelemetry) {
this.jobService = jobService;
this.stairwayComponent = stairwayComponent;
this.mdcHook = mdcHook;
this.openTelemetry = openTelemetry;
this.jobParameterMap = new FlightMap();
}
Expand Down Expand Up @@ -103,7 +109,8 @@ private void populateInputParams() {
jobId = stairwayComponent.get().createFlightId();
}

// Always add the tracing span parameters
// Always add the MDC logging and tracing span parameters for the mdc hook
addParameter(MdcHook.MDC_FLIGHT_MAP_KEY, mdcHook.getSerializedCurrentContext());
addParameter(
MonitoringHook.SUBMISSION_SPAN_CONTEXT_MAP_KEY,
MonitoringHook.serializeCurrentTracingContext(openTelemetry));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@
import bio.terra.common.iam.AuthenticatedUserRequest;
import bio.terra.common.stairway.MonitoringHook;
import bio.terra.common.stairway.StairwayComponent;
import bio.terra.common.stairway.StairwayLoggingHook;
import bio.terra.profile.app.common.ErrorReportUtils;
import bio.terra.profile.app.common.MdcHook;
import bio.terra.profile.app.configuration.IngressConfiguration;
import bio.terra.profile.app.configuration.JobConfiguration;
import bio.terra.profile.app.configuration.StairwayDatabaseConfiguration;
Expand Down Expand Up @@ -58,6 +58,7 @@ public class JobService {
private final IngressConfiguration ingressConfig;
private final StairwayDatabaseConfiguration stairwayDatabaseConfiguration;
private final ScheduledExecutorService executor;
private final MdcHook mdcHook;
private final StairwayComponent stairwayComponent;
private final ApplicationContext context;
private final Logger logger = LoggerFactory.getLogger(JobService.class);
Expand All @@ -70,6 +71,7 @@ public JobService(
JobConfiguration jobConfig,
IngressConfiguration ingressConfig,
StairwayDatabaseConfiguration stairwayDatabaseConfiguration,
MdcHook mdcHook,
StairwayComponent stairwayComponent,
ApplicationContext context,
ObjectMapper objectMapper,
Expand All @@ -78,6 +80,7 @@ public JobService(
this.ingressConfig = ingressConfig;
this.stairwayDatabaseConfiguration = stairwayDatabaseConfiguration;
this.executor = Executors.newScheduledThreadPool(jobConfig.maxThreads());
this.mdcHook = mdcHook;
this.stairwayComponent = stairwayComponent;
this.context = context;
this.objectMapper = objectMapper;
Expand All @@ -86,7 +89,7 @@ public JobService(

// Fully fluent style of JobBuilder
public JobBuilder newJob() {
return new JobBuilder(this, stairwayComponent, openTelemetry);
return new JobBuilder(this, stairwayComponent, mdcHook, openTelemetry);
}

// submit a new job to stairway
Expand Down Expand Up @@ -162,7 +165,7 @@ public void initialize() {
.newStairwayOptionsBuilder()
.dataSource(stairwayDatabaseConfiguration.getDataSource())
.context(context)
.addHook(new StairwayLoggingHook())
.addHook(mdcHook)
.addHook(new MonitoringHook(openTelemetry))
.exceptionSerializer(new StairwayExceptionSerializer(objectMapper)));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import bio.terra.policy.model.TpsPaoGetResult;
import bio.terra.policy.model.TpsPolicyInput;
import bio.terra.policy.model.TpsPolicyInputs;
import bio.terra.profile.app.common.MdcHook;
import bio.terra.profile.common.BaseUnitTest;
import bio.terra.profile.common.ProfileFixtures;
import bio.terra.profile.db.ProfileDao;
Expand Down Expand Up @@ -543,7 +544,10 @@ void createProfileFlightSetup() {
when(billingCow.testIamPermissions(any())).thenReturn(iamPermissionsResponse);
StairwayComponent stairwayComponent = mock(StairwayComponent.class);

var builder = spy(new JobBuilder(jobService, stairwayComponent, OpenTelemetry.noop()));
var builder =
spy(
new JobBuilder(
jobService, stairwayComponent, mock(MdcHook.class), OpenTelemetry.noop()));
when(jobService.newJob()).thenReturn(builder);

var profileDescription = ProfileFixtures.createGcpBillingProfileDescription("ABCD1234");
Expand Down

0 comments on commit f89aa96

Please sign in to comment.