-
Notifications
You must be signed in to change notification settings - Fork 173
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
Run themis on deps #817
Run themis on deps #817
Conversation
bf95558
to
2ad5bc5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I still have to go through uploading logic, and testing. Leaving some optional nits, and other comments for now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One required change, specifically the extractXZippedBinary
comment.
src/App/Fossa/EmbeddedBinary.hs
Outdated
compressedThemisIndex <- extractEmbeddedBinary ThemisIndex | ||
let decompressedThemisIndex = | ||
BinaryPaths | ||
{ binaryPathContainer = binaryPathContainer compressedThemisIndex | ||
, binaryFilePath = $(mkRelFile "index.gob") | ||
} | ||
sendIO $ extractLzma (toPath compressedThemisIndex) (toPath decompressedThemisIndex) | ||
pure $ ThemisBins themisActual $ applyTag @ThemisIndex decompressedThemisIndex |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have the entire binary file in memory, it seems a little weird to dump it out to a file and then unzip it on-disk. I think it's best if we create some extractXZippedBinary
function which does similar work to extractEmbeddedBinary
, but runs in-memory lzma
unzipping on the embedded bytestring and dumps that directly to the file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like Lzma.decompress
is what we're looking for.
src/App/Fossa/FossaAPIV1.hs
Outdated
(Has (Lift IO) sig m, Has Diagnostics sig m) => | ||
ApiOpts -> | ||
ArchiveComponents -> | ||
m (Maybe C.ByteString) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is the return type a bytestring? Generally, we maintain that the API function should handle all data transformation and parsing from the server before returning that value to the caller.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see that the old archive upload function did that, let's leave a TODO to fix this later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This endpoint just returns a 201 if there's a success, and the only place that we call this discards the return value.
Should the return type be m ()
or something like that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, but let's make it a todo to keep this ticket smaller.
9a6cdb8
to
05eab7a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Drive-by, I should probably go though this more thoroughly if you want my full review on it.
Co-authored-by: Wesley Van Melle <wes@fossa.com>
Superseded by #847, couldn't rebase on latest master due to merge commits in the history. |
Overview
Closes ANE-25 — Use themis to license scan vendored dependencies
Closes ANE-26 — Upload cli-side license data to S3 and trigger the build
Run
themis-cli
on the list of vendored dependencies infossa-deps.yml
, upload the resulting scan to S3 and then trigger a build on Core.Things this PR does not do:
--output
flag (https://fossa.atlassian.net/browse/ANE-97)There are also no tests. I think that this code is similar to other code-paths that are not tested, but please let me know if there are areas that are reasonable to test.
Acceptance criteria
This PR changes the behavior for vendored dependencies found in
fossa-deps.yml
.archiveOrCLI
inApp.Fossa.ManualDeps
is set toArchiveUpload
, then there should be no change: we should run an archive upload on any vendored dependenciesarchiveOrCLI
is set toCLILicenseScan
, then we should run a license scan on any vendored dependenciesRunning a license scan means:
CLILicenseScanBuild
job for each vendored dependency.archiveOrCLI
must be set toArchiveUpload
before this is merged into masterTesting plan
Core setup
Check out the CLI license scan endpoint PR. The branch name is
cli-license-scan-endpoint
Do whatever you normally do to get core running on your system. You'll need to have minio, the agent and a faktory worker running, so you might also need to do:
You'll want to watch these logs to make sure that the right job is getting queued:
test directory setup
Make a simple yarn project with a single vendored dependency:
Put this in
fossa-deps.yml
:Put this in
yarn-package/package.json
:Now do the same for a directory with multiple archives:
Put this in
fossa-deps.yml
:in first-yarn-package/package.json:
in second-yarn-package/package.json:
Run the CLI
Without any changes, run the CLI:
This should successfully go through the previously existing archive upload workflow.
Now edit
src/App/Fossa/ManualDeps.hs
. On line 92 changeArchiveUpload
toCLILicenseScan
Edit
fossa-deps.yml
and change the version of the dependency from1.0.1
to1.0.2
.run the scan again:
This time it should run a license scan on the
yarn-package
directory and upload the results to your local minio.There should be no difference in the UI. The license scan result from a cli-side license scan should be the same as a license scan done on the servers.
However, what happens on the back end is very different.
The archive upload workflow uploads the contents of the vendored dependency to
archive_components/archive+<org ID>/<locator>
.So, if your ORG ID is 1, you'll see a file uploaded to
archive_components/archive+1/yarn-package$1.0.1
.Download that file and gunzip it. Check that you see the contents of the yarn-package directory.
The CLI license scan workflow scans the contents of the vendored dependency and uploads the results of that license scan to
archive_license_data/archive+<org ID>/locator
.So, if your ORG ID is 1, you'll see a file uploaded to
archive_license_data/archive+1/yarn-package$1.0.2
.Download that file and gunzip it. Check that you see a JSON file containing the results of a license scan. It should look like this:
The contents of the file are just the JSON output.
In both cases, the UI should show an MIT license found for the
yarn-package
dependency.Do the same thing for the project in
~/fossa/license-scan-dirs/archive-upload-with-multple-targets
:cd ~/fossa/license-scan-dirs/archive-upload-with-multple-targets
You should see two new files in minio in
archive_components/archive+<org ID>
.You should also see two
CliLicenseScannedBuild
tasks run in your faktory logs.You should see an MIT license for
first-package
and an Apache-2.0 license forsecond-package
.Risks
References
Checklist
docs/
.Changelog.md
if this change is externally facing. If this PR did not mark a release, I added my changes into an# Unreleased
section at the top.*schema.json
if I have made changes for.fossa.yml
,fossa-deps.{json, yaml, yml}
. You may also need to update these if you have added/removed new dependency (e.g. pip) or analysis target type (e.g. poetry).