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

Setup elasticsearch dependency monitoring with Snyk for production code #88036

Merged
merged 17 commits into from
Jun 29, 2022

Conversation

breskeby
Copy link
Contributor

@breskeby breskeby commented Jun 24, 2022

This setups the generation and upload logic of gradle dependency graphs to snyk as raised in #87620

We directly implemented a rest api based snyk plugin as:

  • the existing snyk gradle plugin delegates to the snyk command line tool
  • the command line tool uses custom gradle logic by injecting a init file that is
    a) using deprecated build logic which we definitely want to avoid
    b) uses gradle api we avoid like eager task creation.
  • shipping this as a internal gradle plugin gives us the most flexibility

As we only want to monitor production code for now we apply this plugin as part of the elasticsearch.build plugin, that usage has been for now the de-facto indicator if a project is considered a "production" project that ends up in our distribution or public maven repositories. This isnt yet ideal and we will revisit the distinction between production and non production code / projects in a separate effort.

The snyk upload can be triggered from command line via:

>./gradlew uploadSnykDependencyGraph -PsnykToken=XXX 

where XXX is the api token that can be configured at https://app.snyk.io/account

The outcome of this can be found at https://app.snyk.io/org/elasticsearch/projects and https://app.snyk.io/org/elasticsearch/reports/

Setting up build jobs for regular updating the dependency monitoring in snyk will be covered in a separate PR.

As part of this effort we added the elasticsearch.build plugin to more projects that actually end up in the distribution. To unblock us on this we for now disabled a few check tasks that started failing by applying elasticsearch.build. We'll address this in separate PR at its out of scope of this story to fix.

@breskeby breskeby self-assigned this Jun 24, 2022
@breskeby breskeby added :Delivery/Build Build or test infrastructure >enhancement Team:Delivery Meta label for Delivery team labels Jun 24, 2022
@breskeby breskeby changed the title WIP snyk Setup elasticsearch dependency monitoring with Snyk for production code Jun 24, 2022
@@ -168,7 +168,6 @@ private static String readFirstLine(final Path path) throws IOException {

/** Find the reponame. */
public String urlFromOrigin() {
String oritin = getOrigin();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed typo on the way

Class<? extends Plugin> pluginClassUnderTest = SnykDependencyMonitoringGradlePlugin.class

def setup() {
configurationCacheCompatible = false // configuration is not cc compliant
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For creating the dependency graph we need to work on the Configuration object that is not compliant with the gradle configuration-cache at the moment. This should not affect our developers as this task should never be triggered in day to day life from dev environments

}

dependencies {
implementation 'org.apache.lucene:lucene-monitor:9.2.0'
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For now, I don't want to go down the route of creating a maven test fixture to reproduce dependency graphs from scratch. Therefore we test against real dependencies from real repositories.

String content = FileUtils.readFileToString(inputFile.getAsFile().get());
HttpPut putRequest = new HttpPut(endpoint);
putRequest.addHeader("Authorization", "token " + token.get());
putRequest.addHeader("Content-Type", "application/json");
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For now we send the plain json in text format to the endpoint. Snyk supports gzipped data here but I didn't see a major performance issue here at all that would make the effort to support gz worth it for now.

@@ -17,8 +17,7 @@ tasks.named("assemble").configure { enabled = true }
archivesBaseName = 'client-benchmarks'
mainClassName = 'org.elasticsearch.client.benchmark.BenchmarkMain'

// never try to invoke tests on the benchmark project - there aren't any
tasks.named("test").configure {enabled = false }
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed those explicit set tests to not execute. Gradle will shortcut that anyhow as no source is detected. It also makes the outcome of that task more clear IMO as it will tell NO_SOURCE instead of SKIPPED which can have different reasons.

@@ -20,5 +20,4 @@ esplugin {
tasks.named("assemble").configure { enabled = false }
tasks.named("dependencyLicenses").configure { enabled = false }
tasks.named("dependenciesInfo").configure { enabled = false }
// no unit tests
tasks.named("test").configure { enabled = false }
tasks.named("uploadSnykDependencyGraph").configure { enabled = false }
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this plugin applies elasticsearch.build but I don't consider it production code

@@ -17,8 +17,7 @@ tasks.named("assemble").configure { enabled = true }
archivesBaseName = 'client-benchmarks'
mainClassName = 'org.elasticsearch.client.benchmark.BenchmarkMain'

// never try to invoke tests on the benchmark project - there aren't any
tasks.named("test").configure {enabled = false }
tasks.named("uploadSnykDependencyGraph").configure {enabled = false }
Copy link
Contributor Author

@breskeby breskeby Jun 24, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this project applies elasticsearch.build but I don't consider it production code

@@ -7,3 +7,7 @@ dependencies {

testImplementation project(":test:framework")
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

plenty of forbidden api failures. TODO: raise ticket

@@ -8,3 +8,9 @@ dependencies {
tasks.named('shadowJar').configure {
exclude 'org/apache/hadoop/util/ShutdownHookManager$*.class'
}

['jarHell', 'thirdPartyAudit', 'forbiddenApisMain', 'splitPackagesAudit'].each {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All kinds of problems here that have been not discovered before.
TODO: raise ticket

@@ -47,7 +47,6 @@ tasks.named('forbiddenApisMain').configure {
// TODO: should we have licenses for our test deps?
tasks.named("dependencyLicenses").configure { enabled = false }
tasks.named("dependenciesInfo").configure { enabled = false }
tasks.named("dependenciesGraph").configure { enabled = false }
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if this should be part of the snyk check or not. It seems we upload this somewhere so I would consider it kind of production code

@@ -30,7 +30,6 @@ tasks.named('forbiddenApisMain').configure {
// TODO: should we have licenses for our test deps?
tasks.named("dependencyLicenses").configure { enabled = false }
tasks.named("dependenciesInfo").configure { enabled = false }
tasks.named("dependenciesGraph").configure { enabled = false }
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as above

@@ -291,6 +291,7 @@ dependencies {
exclude module: "groovy"
}
testImplementation buildLibs.spock.junit4
testImplementation buildLibs.json.assert
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

allows asserting generated json without hassle.

@breskeby breskeby requested review from mark-vieira, rjernst and jkakavas and removed request for mark-vieira and rjernst June 24, 2022 21:49
@breskeby breskeby marked this pull request as ready for review June 24, 2022 21:52
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-delivery (Team:Delivery)

@breskeby breskeby force-pushed the snyk-dependency-monitoring branch from 100a2ce to e08f2b8 Compare June 27, 2022 12:26
@jkakavas
Copy link
Member

  • Does this allow for a dry run where you'd print out the dependency list instead of pushing to Snyk ? I have found that helpful to have
  • This has probably nothing to do with this PR, but since we're talking dependencies, are there any current plans to change the way we declare dependencies ? The explicit way we do so now, which leads us to be reporting a flattened dependency list instead of a graph makes it complicated to figure out if dependency X is direct or transitive ( and which direct dependency brings that in ).

@breskeby
Copy link
Contributor Author

@jkakavas

about 1) The way this works is split in 2 tasks. If you just run "generateSnykDependencyGraph" you can generate the file containing actually the json as we push it to snyk.
about 2) Indeed we have plans to move to transitive dependency handling at one point. The logic in this PR on how the graph is created takes this already into account. The hierarchy is not flat as it was with the former DependenciesGraphPlugin. So we will see which how a dependency was brought in in the first place.

@breskeby breskeby added the auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) label Jun 28, 2022
@breskeby breskeby force-pushed the snyk-dependency-monitoring branch from f923ead to 47206c4 Compare June 29, 2022 07:30
@breskeby breskeby merged commit 8ccae4d into elastic:master Jun 29, 2022
@breskeby breskeby deleted the snyk-dependency-monitoring branch June 29, 2022 11:29
breskeby added a commit to breskeby/elasticsearch that referenced this pull request Jun 30, 2022
…de (elastic#88036)

This adds the generation and upload logic of Gradle dependency graphs to snyk

We directly implemented a rest api based snyk plugin as:

the existing snyk gradle plugin delegates to the snyk command line tool the command line tool
uses custom gradle logic by injecting a init file that is

a) using deprecated build logic which we definitely want to avoid
b) uses gradle api we avoid like eager task creation.

Shipping this as a internal gradle plugin gives us the most flexibility as we only want to monitor
production code for now we apply this plugin as part of the elasticsearch.build plugin,
that usage has been for now the de-facto indicator if a project is considered a "production" project
that ends up in our distribution or public maven repositories. This isnt yet ideal and we will revisit
the distinction between production and non production code / projects in a separate effort.

As part of this effort we added the elasticsearch.build plugin to more projects that actually end up
in the distribution. To unblock us on this we for now disabled a few check tasks that started failing by applying elasticsearch.build.

Addresses  elastic#87620
# Conflicts:
#	build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/BuildPlugin.java
#	client/benchmark/build.gradle
#	client/test/build.gradle
#	distribution/tools/cli-launcher/build.gradle
#	distribution/tools/java-version-checker/build.gradle
#	distribution/tools/windows-service-cli/build.gradle
#	rest-api-spec/build.gradle
#	test/framework/build.gradle
breskeby added a commit that referenced this pull request Jul 4, 2022
…tion code (#88036) (#88223)

Backports the following commits to 7.17:
 - Setup elasticsearch dependency monitoring with Snyk for production code (#88036)
breskeby added a commit to breskeby/elasticsearch that referenced this pull request Jul 15, 2022
…de (elastic#88036)

This adds the generation and upload logic of Gradle dependency graphs to snyk

We directly implemented a rest api based snyk plugin as:

the existing snyk gradle plugin delegates to the snyk command line tool the command line tool
uses custom gradle logic by injecting a init file that is

a) using deprecated build logic which we definitely want to avoid
b) uses gradle api we avoid like eager task creation.

Shipping this as a internal gradle plugin gives us the most flexibility as we only want to monitor
production code for now we apply this plugin as part of the elasticsearch.build plugin,
that usage has been for now the de-facto indicator if a project is considered a "production" project
that ends up in our distribution or public maven repositories. This isnt yet ideal and we will revisit
the distinction between production and non production code / projects in a separate effort.

As part of this effort we added the elasticsearch.build plugin to more projects that actually end up
in the distribution. To unblock us on this we for now disabled a few check tasks that started failing by applying elasticsearch.build.

Addresses  elastic#87620
# Conflicts:
#	test/framework/build.gradle
breskeby added a commit that referenced this pull request Jul 15, 2022
…ion code (#88036) (#88566)

* Setup elasticsearch dependency monitoring with Snyk for production code (#88036)

This adds the generation and upload logic of Gradle dependency graphs to snyk

We directly implemented a rest api based snyk plugin as:

the existing snyk gradle plugin delegates to the snyk command line tool the command line tool
uses custom gradle logic by injecting a init file that is

a) using deprecated build logic which we definitely want to avoid
b) uses gradle api we avoid like eager task creation.

Shipping this as a internal gradle plugin gives us the most flexibility as we only want to monitor
production code for now we apply this plugin as part of the elasticsearch.build plugin,
that usage has been for now the de-facto indicator if a project is considered a "production" project
that ends up in our distribution or public maven repositories. This isnt yet ideal and we will revisit
the distinction between production and non production code / projects in a separate effort.

As part of this effort we added the elasticsearch.build plugin to more projects that actually end up
in the distribution. To unblock us on this we for now disabled a few check tasks that started failing by applying elasticsearch.build.

Addresses  #87620
# Conflicts:
#	test/framework/build.gradle

* Fix license report when backporting
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) :Delivery/Build Build or test infrastructure >enhancement Team:Delivery Meta label for Delivery team v7.17.6 v8.4.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants