Skip to content
This repository has been archived by the owner on Apr 1, 2022. It is now read-only.

Discovered binaries reports origin path #396

Merged
merged 9 commits into from
Oct 18, 2021

Conversation

jssblck
Copy link
Member

@jssblck jssblck commented Oct 15, 2021

Overview

Reports an origin for uploaded SourceUserDefDeps.

The background here is that a customer requested a feature which we call "binary discovery". The TL;DR of this feature is that we, as a strategy in Spectrometer, identify files with binary content in the project and report them as unlicensed user-defined dependencies in Core.

This led to a follow up feature request: "please display the path at which the binary is located in the report under the File Path(s) column". This PR makes that happen.

"Origin", in this context, means the path at which the user defined dependency was identified.
This then is used to provide the actual point of the PR, which is hydrating File Path(s) in the report for the project when using binary discovery.

Acceptance criteria

I booted a core instance that supports this information and ran a scan.
I then verified that:

  • Origin paths are displayed in the UI (when they're clicked they just say "this file wasn't uploaded", but that's fine).
  • The report correctly hydrates the File Path(s) column with this origin.

Testing plan

  1. Boot core with the branch discovered-binary-origins. Make sure the "File paths in reports" feature flag is enabled.
  2. Create a project with a binary file inside (or add a binary file to a project you already have).
  3. Run a scan with Spectrometer on this branch with the flag --experimental-enable-binary-discovery.
  4. Run a report, selecting to display the file paths. Validate that the file path of the binary appears in that report.

Screenshots

Showing this working:

Screen Shot 2021-10-15 at 2 15 43 PM

Screen Shot 2021-10-15 at 2 16 03 PM

And showing that user defined deps without origins are still displayed properly:

Screen Shot 2021-10-15 at 2 17 33 PM

Risks

Not particularly risky.

Checklist

  • I added tests for this PR's change (or confirmed tests are not viable).
  • I updated 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.

Copy link
Contributor

@skilly-lily skilly-lily left a comment

Choose a reason for hiding this comment

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

Looks good, left one comment

Comment on lines 25 to 26
renderSomeBase :: SomeBase f -> Text
renderSomeBase path = case path of
Copy link
Contributor

Choose a reason for hiding this comment

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

[optional nit]: I think, for this, I would prefer if SomeBase had instances for ToString and ToText in Data.String.Conversion. Then we could just use toString or toText everywhere. But this isn't necessary to change in this PR.

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh, yeah, that's obviously the correct way to go. Will make that change!

@jssblck jssblck merged commit 0bab669 into master Oct 18, 2021
@jssblck jssblck deleted the discovered-binaries-origin-path branch October 18, 2021 19:41
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants