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

feat: support yarn workspaces projects in --all-projects #2341

Merged
merged 3 commits into from
Jan 4, 2022

Conversation

lili2311
Copy link
Contributor

@lili2311 lili2311 commented Nov 3, 2021

What does this PR do?

Support detecting & scanning Yarn workspace projects --all-projects.
Process all projects as yarn workspaces first since many files are related to each other and could belong to a workspace, then process the rest as usual.

Where should the reviewer start?

https://github.com/snyk/snyk/pull/2341/files#diff-2194ffc6b9f5b3533db3a8c636c6fcde3d7f163e92de6d915a5dc542a576fb82R48

How should this be manually tested?

Run snyk test --all-projects in a yarn workspaces projects

What are the relevant tickets?

https://github.com/snyk/snyk/issues/1561

Screenshots

Scenario 1:

snyk test --all-projects detects all the Yarn workspace projects + the non-workspace project.
CleanShot 2021-11-15 at 16 44 40

Scenario 2:

snyk test --all-projects --json has the same output all the Yarn workspace projects via --yarn-workspaces with the addition of the non-workspace project.

CleanShot 2021-11-15 at 16 47 29

Scenario 3:

Out of sync error during --yarn-workspaces scan is resulting in the same error code and behaviour as an error in another project type during --all-projects

Note: --all-projects by default does not log the errors and instead shows them in debug mode. So the difference in the error seen is expected.

CleanShot 2021-11-19 at 10 18 42

No supported manifests error during --yarn-workspaces scan is resulting in the same error code and message as during --all-projects

Scenario 4:

CleanShot 2021-11-19 at 14 49 47

Scenario 5:

snyk-dev test --all-projects --strict-out-of-sync=false and snyk-dev test --yarn-workspaces --strict-out-of-sync=false both detect and scan all the projects in a yarn workspace monorepo

CleanShot 2021-11-19 at 15 00 17

Scenario 6 ⚠️ :

--all-projects does not fail if 1 project fails, it logs it and keeps going. So scanning a out of sync workspace now follows the same login when scanned with --all-projects to ensure backwards compatible & expected behaviour. The same behaviour as with --yarn-workspaces can be achieved by adding --fail-fast flag.

CleanShot 2021-11-19 at 15 54 29

Scenario 7:

Exit codes match when 100% of the projects fail to scan via --all-projects and --yarn-workspaces

CleanShot 2021-11-19 at 16 12 45

Scenario 8:

--all-projects --dev --json matches the data from --yarn-workspaces --dev --json + finds an extra non workspace projects.

⚠️ need to check why the patched date has a slight diff.

CleanShot 2021-11-19 at 16 17 17

Scenario 9:

snyk monitor --all-projects matches the same projects from snyk monitor --yarn-workspaces + applies all the Snyk org ignores

CleanShot 2021-11-22 at 11 12 11

@github-actions
Copy link
Contributor

github-actions bot commented Nov 3, 2021

Warnings
⚠️

Since the CLI is unifying on a standard and improved tooling, we're starting to migrate old-style imports and exports to ES6 ones.
A file you've modified is using either module.exports or require(). If you can, please update them to ES6 import syntax and export syntax.
Files found:

  • src/lib/plugins/get-multi-plugin-result.ts
  • src/lib/plugins/nodejs-plugin/yarn-workspaces-parser.ts
  • src/lib/snyk-test/run-test.ts

Generated by 🚫 dangerJS against 2e5effe

Copy link
Contributor

@admons admons left a comment

Choose a reason for hiding this comment

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

Please see my comments

src/lib/plugins/get-multi-plugin-result.ts Outdated Show resolved Hide resolved
src/lib/plugins/get-multi-plugin-result.ts Outdated Show resolved Hide resolved
@lili2311 lili2311 force-pushed the feat/yarn-workspaces-all-projects branch 6 times, most recently from 60afda4 to e031c82 Compare November 9, 2021 11:12
@lili2311 lili2311 changed the title feat: support yarn wokspaces projects in --all-projects feat: support yarn workspaces projects in --all-projects Nov 9, 2021
@lili2311 lili2311 force-pushed the feat/yarn-workspaces-all-projects branch from e031c82 to 8a6346e Compare November 9, 2021 11:14
@lili2311 lili2311 self-assigned this Nov 10, 2021
@lili2311 lili2311 force-pushed the feat/yarn-workspaces-all-projects branch 3 times, most recently from c27ef30 to fe81b83 Compare November 11, 2021 15:38
@lili2311 lili2311 marked this pull request as ready for review November 12, 2021 14:09
@lili2311 lili2311 requested review from a team as code owners November 12, 2021 14:09
@lili2311
Copy link
Contributor Author

Ready for review, still have some manual verification left to do before it can be merged

@lili2311 lili2311 force-pushed the feat/yarn-workspaces-all-projects branch 6 times, most recently from 4612af1 to 39312ef Compare November 19, 2021 16:37
@lili2311
Copy link
Contributor Author

QA completed, please see the PR description for tested scenarios & their results

By default include Yarn workspace projects for duscovery during
--all-projects as well. Filter out any files that were scanned
as part of a workspace before forewarding on the rest of the detected
manifests to be scanned by the individual plugins.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants