fix: policies to be correctly picked up from nested scans #1224
Conversation
ghost
requested review from
lili2311
changed the title
lili2311
force-pushed
the
fix/policies-all-projects
branch
2 times, most recently
from
1bfbb65
to
523bbc5
Compare
lili2311
force-pushed
the
fix/policies-all-projects
branch
from
523bbc5
to
cb64cd2
Compare
|
|
||
| // Forcing options.path to be a string as pathUtil requires is to be stringified | ||
| const targetFileRelativePath = targetFile | ||
| ? pathUtil.join(pathUtil.resolve(`${options.path || root}`), targetFile) |
There was a problem hiding this comment.
there is no options.path when coming via snyk protect flow
lili2311
force-pushed
the
fix/policies-all-projects
branch
6 times, most recently
from
8f389af
to
a498b29
Compare
lili2311
force-pushed
the
fix/policies-all-projects
branch
from
a498b29
to
ed93009
Compare
test/acceptance/workspaces/gradle-multi-project/.gradle/buildOutputCleanup/cache.properties
Outdated
Show resolved
Hide resolved
| // TODO: this should return 1 policy when fixed | ||
| // uncomment then | ||
| // t.match( | ||
| // req.body.policy, |
There was a problem hiding this comment.
What is current missing to make this work? Something that we already discussed about on snyk-gradle-plugin? Thanks!
There was a problem hiding this comment.
yes we must fix how targetFile is sent there to not just be build.gradle but relative path from root. currently all the scans look like they are from the same folder and only the projectName indicates otherwise.
There was a problem hiding this comment.
Please see the thread in Slack
Also fix some formatting
lili2311
force-pushed
the
fix/policies-all-projects
branch
from
ed93009
to
9f50581
Compare
|
Expected release notes (by @lili2311) fixes: others (will not be included in Semantic-Release notes):
|
|
🎉 This PR is included in version 1.348.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
|
Eh, this change constitutes in fact a breaking change, which has caused us quite some time loss trying to figure out why Snyk suddenly ignored the I guess we were unknowingly relying on a bug, but that is no excuse in my opinion. Changing which locations are accepted as a location for config files is a breaking change, so rather than releasing as a bugfix release, v1.138.1 should've been v2.0.0, a major release. Also, why are the release notes so sparse? It's hardly possible to figure out what has actually changed, and |
|
👋 @erwinpleo Apologies for this inconvenience, you are right the notes could be better I have gone ahead and updated these with more information. I have written up a detailed response here: https://github.com/snyk/snyk/issues/1237#issuecomment-651378812 I hope this helps to clarify the reason for the change and provides some advice. If you have further comments It may be beneficial to keep it on that thread in one place. |
|
@erwinpleo just realised that perhaps you can achieve exactly what you had if you use |
What does this PR do?
Show that when triggering a test with
--fileor--all-projectthe policy is not discovered as we expect to find it in therootwhich is where the command is run and it should instead be where the manifest was discovered.Any background context you want to provide?
While working on yarn workspaces feature test noticed the policy from the workspaces were not being used or sent.
https://github.com/snyk/snyk/issues/1014