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.

Sign up for GitHub

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

Jump to bottom

Adds generic sbt node snyk workflow #11

Merged
merged 7 commits into from Feb 9, 2022
Merged

Adds generic sbt node snyk workflow #11

Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
5ad3553
Adds generic sbt node snyk workflow
SHession Jan 19, 2022
2d42b44
Sets Java version to 11 and simplifies node version retrieval
SHession Jan 20, 2022
165178a
Adds java_version input to the snyk workflow
SHession Feb 4, 2022
8d51820
Make org required
jorgeazevedo Feb 3, 2022
1c7bd54
Simplify handling of ORG variable
jorgeazevedo Feb 3, 2022
9b44635
Handle org whitespace
jorgeazevedo Feb 3, 2022
7c34166
rearranges cli arguments
SHession Feb 4, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
41 changes: 41 additions & 0 deletions .github/workflows/sbt-node-snyk.yml
View file
Open in desktop
@@ -0,0 +1,41 @@
name: Simple Snyk monitor for SBT + Node

on:
workflow_call:
Copy link
Contributor

Choose a reason for hiding this comment

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

I see in https://github.com/guardian/workflow-frontend/pull/356/files we're setting this up to be called on push to any branch. I think the guidance in the past has been to run synk test for branches and synk monitor only on main, so that the continuously monitored application in snyk is main.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jfsoul, this is a good point. Our workflows diverged from that recommendation due to failing checks against PRs which were nothing to do with the content of the PR. As many of our projects have outstanding vulnerabilities which need resolving, the test function would always provide a failing result. This was unhelpful and indicated an issue to developers unfamiliar with this process. As it represented a problem with overall codebase and not the PR specifically we felt it wasn't relevant to have as a PR check.

We therefore thought it would be preferable to exclude the test check and only run monitor, the results of which can be reviewed in the Snyk dashboard. It would be interesting to know how other teams addressed these concerns.

Copy link
Contributor

Choose a reason for hiding this comment

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

After discussing offline I realise now that workflow-frontend will actually only be running this workflow on pushes to main - we've just commented that restriction out for testing. I do think there's value in snyk test for at least some teams, but I think this is an area that needs more thought before we can form a solid recommendation.

inputs:
DEBUG:
type: string
required: false
ORG:
type: string
required: true
JAVA_VERSION:
type: string
required: false
default: "11"
secrets:
SNYK_TOKEN:
required: true

jobs:
security:
runs-on: ubuntu-latest
steps:
- name: Checkout branch
uses: actions/checkout@v2

- uses: snyk/actions/setup@0.3.0
- uses: actions/setup-node@v2
with:
node-version-file: '.nvmrc'

- uses: actions/setup-java@v2
with:
java-version: ${{ inputs.JAVA_VERSION }}
distribution: "adopt"

- name: Snyk monitor
run: snyk monitor ${INPUT_DEBUG:+ -d} --all-projects --org="${{ inputs.ORG }}"
env:
SNYK_TOKEN: ${{ secrets.SNYK_TOKEN }}
INPUT_DEBUG: ${{ inputs.DEBUG }}