ANE 966: first party license scans [1/4] #1187

spatten · 2023-05-05T00:46:35Z

Overview

ANE-966 - First party license scans

If requested, the CLI will license scan the full directory, merge the license scan data with the standard dependency data and upload the results to S3. Core will analyze the uploaded data so that licenses found in the project are shown as "Directly in Code" in the FOSSA UI.

The core side of this PR is https://github.com/fossas/FOSSA/pull/10219

Three more things are coming in future PRs:

ANE-969 will skip license scanning vendored dependencies as part of the first-party scan
ANE-991 will add user facing documentation for this feature
ANE-981 will delete first-party scan data from S3 or minio 30 days after it is uploaded, just like we do for archive uploads and CLI-side license scans.

I'll add the Changelog as part of the user facing documentation.

Acceptance criteria

The project should be license scanned if the "--experimental-force-first-party-scans" flag is present
The project should be license scanned if the org has a "orgDefaultsToFirstPartyScans" flag set to true and the "--experimental-force-no-first-party-scans" flag is not present
It should be an error to have both the "--experimental-force-first-party-scans" and "--experimental-force-no-first-party-scans" flags
It should just run a normal scan if the "--experimental-force-first-party-scans" is provided but the version of Core you are pointing at does not support first-part scans
The first-party license scan should respect the path filters for vendored dependencies
The full output of the dependency scan and the license scan should be uploaded to S3
The CLI should post to /api/builds/custom_with_first_party when the scan is complete and uploaded to S3

Note: the first-party scan results should be included when you use the --output flag, but I missed that in this PR. It's fixed in #1198

Testing plan

The first step is to check out this branch on both Core and the CLI and get core running.

Scan a project that has licenses and dependencies in it. I scanned broker, but it has ~300 dependencies so you might want to try something with fewer deps in it.

make install-dev
cd ~/fossa/broker
fossa-dev analyze -e http://localhost:9578 --experimental-force-first-party-scans --debug ~/fossa/broker

The debug logs should tell you that it is running a first-party scan:

[DEBUG] Running a first-party license scan on the code in this repository. Licenses found in this repository will show up as 'Directly in code' in the FOSSA UI
[DEBUG] License Scanning 'first-party' at '.'

The result of that build will have a few licenses that are labelled as "directly in code". If you look, you'll see that most of them are in the target directory.

So, let's filter that out by adding some vendored dependency excludes to broker's .fossa.yml:

version: 3

targets:
  exclude:
    # Node isn't used for this actual project, but we have a node test fixture
    # to make tests run faster.
    - type: npm
vendoredDependencies:
  licenseScanPathFilters:
    exclude:
      - "./target/**"
      - "./target/*"

Run again. You should now only see one license that is "directly in code"

Turn on the "default to first party scans" feature flag in local core:

Run an analysis without the "--experimental-force-first-party-scans" flag. It should still run a first-party scan:

fossa-dev analyze -e http://localhost:9578 --debug ~/fossa/broker

Turn the feature flag off and run fossa-dev analyze again. It should now skip the first-party scan.

Turn the first-party scans feature flag back on, and run with and without the "Full file uploads for cli-license scans" feature flag on.

It should work both ways. With it on, you should see the full file in the UI like this:

With the feature flag off, you should only see the part of the file that was a license match:

Run a scan with --experimental-force-first-party-scans against a version of Core that does not support first party scans:

FOSSA_API_KEY=$FOSSA_PRODUCTION_API_KEY fossa-dev analyze --debug --experimental-force-first-party-scans ~/fossa/broker

This should just run a normal scan.

Risks

References

https://fossa.atlassian.net/browse/ANE-966

Checklist

I added tests for this PR's change (or explained in the PR description why tests don't make sense).
If this PR introduced a user-visible change, I added documentation into docs/.
If this change is externally visible, I updated Changelog.md. If this PR did not mark a release, I added my changes into an # Unreleased section at the top.
If I made changes to .fossa.yml or fossa-deps.{json.yml}, I updated docs/references/files/*.schema.json. You may also need to update these if you have added/removed new dependency type (e.g. pip) or analysis target type (e.g. poetry).

zlav

Overall looks good! I had a number of questions and small suggestions so I'm requesting changes until those are addressed.

src/App/Fossa/Analyze.hs

src/App/Fossa/Analyze/Upload.hs

zlav · 2023-05-24T00:11:44Z

src/App/Fossa/Analyze/Upload.hs

+uploadFirstPartyAnalysisToS3AndCore ::
+  ( Has Diagnostics sig m
+  , Has FossaApiClient sig m
+  , Has StickyLogger sig m
+  ) =>
+  ProjectRevision ->
+  ProjectMetadata ->
+  NE.NonEmpty FullSourceUnit ->
+  FullFileUploads ->
+  m UploadResponse
+uploadFirstPartyAnalysisToS3AndCore revision metadata mergedUnits fullFileUploads = do
+  _ <- uploadFirstPartyAnalysisToS3 revision mergedUnits
+  uploadFirstPartyAnalysis revision metadata fullFileUploads
+
+uploadFirstPartyAnalysisToS3 ::
+  ( Has Diagnostics sig m
+  , Has FossaApiClient sig m
+  , Has StickyLogger sig m
+  ) =>
+  ProjectRevision ->
+  NE.NonEmpty FullSourceUnit ->
+  m ()
+uploadFirstPartyAnalysisToS3 revision mergedUnits = do
+  signedURL <- getSignedFirstPartyScanUrl $ PackageRevision{packageVersion = projectRevision revision, packageName = projectName revision}
+  logSticky $ "Uploading '" <> projectName revision <> "' to secure S3 bucket"
+  -- TODO: copy/paste/modify of uploadLicenseScanResult


I think these functions should be in the src/Control/Carrier/FossaApiClient/Internal directory where uploadAnalyis is located. Is there a circular dependency or something else preventing them from being in that API directory?

I don't think these fit into the src/Control/Carrier/FossaApiClient/Internal directory. Those functions are all readers that have already had runFossaApiClient apiOpts run on them and just talk directly to the API.

These functions are calling out to the API (with getSignedFirstPartyScanUrl, uploadFirstPartyScanResult and uploadFirstPartyAnalysis) and logging things.

The uploadFirstPartyAnalysis and uploadFirstPartyScanResult functions are the equivalent of uploadAnalysis, and they live in the src/Control/Carrier/FossaApiClient/Internal directory

src/App/Fossa/Config/Analyze.hs

src/Fossa/API/Types.hs

zlav · 2023-05-24T01:29:55Z

src/App/Fossa/FirstPartyScan.hs

+firstPartyScanWithOrgInfo ::
+  ( Has Diagnostics sig m
+  , Has (Lift IO) sig m
+  , Has StickyLogger sig m
+  , Has Logger sig m
+  , Has Exec sig m
+  , Has ReadFS sig m
+  , Has FossaApiClient sig m
+  ) =>
+  Path Abs Dir ->
+  StandardAnalyzeConfig ->
+  m (Maybe LicenseSourceUnit)
+firstPartyScanWithOrgInfo root cfg = do
+  org <- getOrganization
+  firstPartyScanMain root cfg org


Does this function need to exist or could we have a new do block and grab org info right after we get Just apiOpts in the block above? This could also exist as a where function in the block above. Is this done to satisfy the FossaApiClient effect?

I think we need this so that we can run runFossaApiClient apiOpts and satisfy the FossaApiClient effect.

There may be a way to do it in a where block, but I played around and this felt like the best solution to me

src/App/Fossa/FirstPartyScan.hs

zlav · 2023-05-24T01:48:20Z

src/Control/Carrier/FossaApiClient/Internal/FossaAPIV1.hs

@@ -615,6 +623,31 @@ uploadAnalysis apiOpts ProjectRevision{..} metadata sourceUnits = fossaReq $ do
  resp <- req POST (uploadUrl baseUrl) (ReqBodyJson $ NE.toList sourceUnits) jsonResponse (baseOpts <> opts)
  pure (responseBody resp)

+uploadFirstPartyAnalysis ::


Suggested change

uploadFirstPartyAnalysis ::

uploadFirstPartyLicenseAnalysis ::

I would add something to clarify that this is a license scan. FirstPartyAnalysis sounds the same to me as Analysis outside of the context of this PR.

I see what you're saying, but I think it obscures the fact that the upload contains both license and non-license analysis results.

It's a merger of results from the first-party scan and the normal analysis.

I'm going to go with uploadAnalysisWithFirstPartyLicenses. Does that work for you?

Yea, I think thats better! As much as I don't love how long the function name is.

src/Control/Carrier/FossaApiClient/Internal/FossaAPIV1.hs

zlav · 2023-05-24T19:22:38Z

test/App/Fossa/Analyze/UploadSpec.hs

+              Fixtures.sourceUnits
+              Nothing
+          locator `shouldBe'` expectedLocator
+      it' "aborts when uploading to a monorepo"


I think you could ignore this test if Jess's monorepo PR merges before this PR.

I'll leave it for now so that things compile (it's mostly a whitespace change, but I also had to add another parameter to uploadSuccessfulAnalysis to make things work

I'll delete the test once I merge Jess's PR - I'm assuming that I'll get a merge conflict if I don't

…urceUnit into FullSourceUnit

…ntion to it

… flag but the FOSSA server does not support first-party scans

…and --experimental-force-no-first-party-scans flags

… on the org

…st party analysis

…ts to the right spot

…scans

…imental-block-first-party-scans"

…ith the --output flag

zlav

LGTM!

spatten force-pushed the ane-966-first-party-scans branch 2 times, most recently from 31cdc28 to 0db91b4 Compare May 18, 2023 19:06

spatten changed the title ~~ANE 966: first party scans~~ ANE 966: first party license scans May 18, 2023

spatten marked this pull request as ready for review May 18, 2023 21:13

spatten requested a review from a team as a code owner May 18, 2023 21:13

spatten requested a review from zlav May 18, 2023 21:13

zlav requested changes May 24, 2023

View reviewed changes

spatten added 23 commits May 24, 2023 16:23

add the first-party force on and off flags

d5a2f85

scaffolding for the FirstPartyScan module

91985a9

run the scans if the flags say to

ee82fb2

get rid of one level of functions in FirstPartyScan

8d5fb07

merge and upload

ed6a1da

add a FullSourceUnit type and functions to convert LicenseUnit and So…

d2c842f

…urceUnit into FullSourceUnit

merge the license and build units

48c1f96

upload first-party license scan to S3

b3fb1cb

upload first-party license scan results to S3

aa15cc3

use NonEmpty instead of [FullSourceUnit]

b7a3905

upload results to /api/builds/custom_with_first_party

18fb1f4

add the supportsFirstPartyScans flag to the organization and pay atte…

2e2df0b

…ntion to it

fatal error if you provide the --experimental-force-first-party-scans…

e61ae37

… flag but the FOSSA server does not support first-party scans

error if you provide both the --experimental-force-first-party-scans …

d361bad

…and --experimental-force-no-first-party-scans flags

unit specs for FirstPartyScan

ed93210

formatting

54e8fba

more formatting

ee9c477

even more formatting

d403e24

first party scans should pay attention to the fullFileUploads setting…

b5a080d

… on the org

temporarily force cliLicenseScanType to match_data when uploading fir…

206258f

…st party analysis

add getSignedFirstPartyScanUrl and use it to upload first-party resul…

4e76248

…ts to the right spot

send the fullFileUploads flag up to core when finalizing first-party …

d38c8c4

…scans

pass the themis path filter flags through properly for first party scans

42a7be7

spatten added 17 commits May 24, 2023 16:25

get the tests running again

b442f4d

tests for uploadSuccessfulAnalysis with first-party scans

8b88ad2

add a test for mergeSourceAndLicenseUnits

ad43b2a

make Fixtures.absDir work on Windows

1e3616c

does this force it to be a Path Abs Dir?

082ea04

fix formatting

c7c88c0

maybe this fixes it?

d78441f

format by hand

66cb9a1

log that the first-party license scan is happening

4f28aec

formatting

95b8754

mention that flag that forced off the scan

a78f1e2

get rid of no-longer-relevant TODO

8b461b5

rename the flag from experimental-force-no-first-party-scans to exper…

2ce724f

…imental-block-first-party-scans"

explain why we might not have an org when running first-party scans w…

84dbef6

…ith the --output flag

uploadFirstPartyAnalysis -> uploadAnalysisWithFirstPartyLicenses

d4d388b

rename signedArcUI -> signedUploadURI and arcURL -> uploadURL

4db4f3b

format

5c89913

spatten force-pushed the ane-966-first-party-scans branch from 2af8acc to 5c89913 Compare May 24, 2023 23:25

spatten added 5 commits May 24, 2023 16:38

remove a merge conflict

5a45790

fix an improt

e9a760b

fix some bad merge commits

f19dba9

remove an import

ae1d65a

formatting

cb21e60

spatten changed the title ~~ANE 966: first party license scans~~ ANE 966: first party license scans [1/4] May 25, 2023

spatten requested a review from zlav May 25, 2023 17:43

zlav approved these changes May 26, 2023

View reviewed changes

spatten added 2 commits May 30, 2023 21:37

ANE 969: first party scans avoid scanning vendored deps [2/4] (#1198)

c21f2eb

Merge branch 'master' into ane-966-first-party-scans

9bd3fd9

spatten merged commit 13cd70b into master May 31, 2023
17 checks passed

spatten deleted the ane-966-first-party-scans branch May 31, 2023 05:36

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ANE 966: first party license scans [1/4] #1187

ANE 966: first party license scans [1/4] #1187

spatten commented May 5, 2023 •

edited

zlav left a comment

zlav May 24, 2023

spatten May 24, 2023

zlav May 24, 2023

spatten May 24, 2023

zlav May 24, 2023

spatten May 24, 2023

zlav May 24, 2023

zlav May 24, 2023

spatten May 24, 2023

zlav left a comment

	uploadFirstPartyAnalysis ::
	uploadFirstPartyLicenseAnalysis ::

ANE 966: first party license scans [1/4] #1187

ANE 966: first party license scans [1/4] #1187

Conversation

spatten commented May 5, 2023 • edited

Overview

Acceptance criteria

Testing plan

Risks

References

Checklist

zlav left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zlav left a comment

Choose a reason for hiding this comment

spatten commented May 5, 2023 •

edited