You should likely use Java 11rather than 8

Happy to make that change. Out of interest, what affect would you expect this to have on our results from Snyk results / SBT dependencies?

I am not certain why we need to setup java in the first place for snyk monitor, however if we do we need to set it up, it seems we should use the LTS java version we have moved to.

It's a good point, I've made that change now. In regards to the setup, I'll add some more context to the description but essentially using the Snyk setup action, we are responsible for setting up the environment in which our runner performs the Snyk CLI checks. So in this case we are setting up a Node and Java environment so we can check the dependencies of SBT and Node projects.

if there's a quick way to make this configurable (with a default of 11) that would be nice

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.

@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.

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.

I suppose this isn't just Node, so we might want to rename. Although I think at least in terms of clarity it might be preferable to separate the 2 actions?

What would you recommend we rename it to? My intention was to create a generic Snyk workflow which would match the general setup of our projects i.e. SBT with Node. If a project has one language then the existing Snyk actions could be used, unless we also wanted to create our own versions for reusability.

Sorry, I misread this initially so my comment isn't really valid.

Maybe something like Simple Snyk monitor for SBT + Node?

I haven't used --all-projects before, I wonder whether it'll be a bit prescriptive for more complex projects that might want to be specific about which manifests to monitor.

Relatedly, there's potentially other CLI flags we'd want to consider supporting like --org --target-reference etc.

all-projects has been an interesting one. As you mention it's not applicable in all cases such as flexible-content. My aim was to make a simple and generic case which would be useable across most projects. In cases where they differ, a manual workflow could be created as an alternative. My assumption is, you are trying to conceive of a way to cover the cases this wouldn't be able to cope with? If so, I think this is a good idea, but would consider this to still be a sensible starting point.