Skip to content

Commit

Permalink
[WOR-1560] Upgrade to TCL 1.1.4 for MDC-aware Stairway (#427)
Browse files Browse the repository at this point in the history
* Revert "Revert "[WOR-1560] Upgrade to TCL 1.1.1 for MDC-aware Stairway (#418)" (#425)"

This reverts commit 9220e78.

Reason: we are going to upgrade TCL to version k8s client at 20.0.1-legacy rather than 20.0.1.  The rest of the commit that we originally reverted we'd like to apply.

* Upgrade TCL to 1.1.4

This upgrade versions our transitive dependency on k8s client at 20.0.1-legacy -- the latest legacy version which moves past a number of vulnerabilities, but does not introduce breaking changes.

* Do not useMavenLocal by default
  • Loading branch information
okotsopoulos authored Apr 1, 2024
1 parent 68b454c commit c2e221c
Show file tree
Hide file tree
Showing 10 changed files with 59 additions and 202 deletions.
35 changes: 35 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -76,5 +76,40 @@ Automatically fix linting issues:
./gradlew generateSwaggerCode
```


## SourceClear

[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, you may want to run a scan off of a feature branch and/or local code.

### Github Action

You can trigger BPM's SCA scan on demand via its
[Github Action](https://github.com/broadinstitute/dsp-appsec-sourceclear-github-actions/actions/workflows/z-manual-terra-billing-profile-manager.yml),
and optionally specify a Github ref (branch, tag, or SHA) to check out from the repo to scan. By default,
the scan is run off of BPM's `main` branch.

High-level results are outputted in the Github Actions run.

### Running Locally

You will need to get the API token from Vault before running the Gradle `srcclr` 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.

### Veracode

Full results including dependency graphs 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.
1 change: 1 addition & 0 deletions buildSrc/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ dependencies {
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 '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,6 +5,7 @@ plugins {

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

boolean isCiServer = System.getenv().containsKey("CI")
Expand All @@ -21,7 +22,12 @@ java {
}
}

// If true, search local repository (~/.m2/repository/) first for dependencies.
def useMavenLocal = false
repositories {
if (useMavenLocal) {
mavenLocal() // must be listed first to take effect
}
maven {
// Terra proxy for maven central
url 'https://broadinstitute.jfrog.io/broadinstitute/maven-central/'
Expand All @@ -33,29 +39,15 @@ repositories {
maven {
url 'https://broadinstitute.jfrog.io/broadinstitute/libs-snapshot-local/'
}
//mavenLocal()
}

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 @@ -106,3 +98,7 @@ jacocoTestReport {
xml.required = true
}
}

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

// Spring Boot 3.1.2 pulls in postgresql 42.6.0, vulnerable to CVE-2024-1597
ext['postgresql.version'] = '42.7.2'
// 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'
5 changes: 3 additions & 2 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:0.1.9-SNAPSHOT'
implementation 'bio.terra:terra-common-lib:1.1.4-SNAPSHOT'

implementation 'org.apache.commons:commons-dbcp2'
implementation 'org.springframework.boot:spring-boot-starter-data-jdbc'
Expand All @@ -27,7 +27,8 @@ 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:2.7.18'

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

// Google dependencies
Expand Down

This file was deleted.

136 changes: 0 additions & 136 deletions service/src/main/java/bio/terra/profile/app/common/MdcHook.java

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
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 @@ -17,7 +16,6 @@
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 @@ -27,13 +25,9 @@ public class JobBuilder {
@Nullable private AuthenticatedUserRequest userRequest;

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

// Always add the MDC logging and tracing span parameters for the mdc hook
addParameter(MdcHook.MDC_FLIGHT_MAP_KEY, mdcHook.getSerializedCurrentContext());
// Always add the tracing span parameters
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,7 +58,6 @@ 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 @@ -71,7 +70,6 @@ public JobService(
JobConfiguration jobConfig,
IngressConfiguration ingressConfig,
StairwayDatabaseConfiguration stairwayDatabaseConfiguration,
MdcHook mdcHook,
StairwayComponent stairwayComponent,
ApplicationContext context,
ObjectMapper objectMapper,
Expand All @@ -80,7 +78,6 @@ 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 @@ -89,7 +86,7 @@ public JobService(

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

// submit a new job to stairway
Expand Down Expand Up @@ -165,7 +162,7 @@ public void initialize() {
.newStairwayOptionsBuilder()
.dataSource(stairwayDatabaseConfiguration.getDataSource())
.context(context)
.addHook(mdcHook)
.addHook(new StairwayLoggingHook())
.addHook(new MonitoringHook(openTelemetry))
.exceptionSerializer(new StairwayExceptionSerializer(objectMapper)));
}
Expand Down
Loading

0 comments on commit c2e221c

Please sign in to comment.