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

Compatibility script: fossa report attribution --json #397

Merged
merged 19 commits into from
Oct 21, 2021
Merged

Conversation

jssblck
Copy link
Member

@jssblck jssblck commented Oct 19, 2021

Overview

Adds a compatibility script for fossa attribution --json.

Acceptance criteria

  • The compatibility layer transforms the report to use the same key structure and content.

The sole exception to this is that FOSSAv1 sometimes outputs Notes keys (which are arrays of strings) with a single zero-value string, but sometimes not. The FOSSAv2 version of the report consistently reports an empty array in these cases, so we don't really have a way to reproduce this exactly.

Example:

Screen Shot 2021-10-18 at 5 09 40 PM

Testing plan

  • Run a scan on a project.
  • Run fossa report attribution --json on that same project using both FOSSA v1 and v2. Save their outputs to disk and note the differences.
  • Run fossa report attribution --json | go build scripts/compat-attribution/main.go using FOSSA v2. Save its output to disk.
  • Note that the difference between the compat version of the output and the v1 output is within AC.

Risks

Not very risky.

Potentially Contentious Decisions

I set up a Go project at the root of Spectrometer to make this happen.

I originally spent a fair amount of time trying to hack this with something like jq in pure bash because I wanted to avoid this, but it quickly became a mess. Rather than go down that route I decided that this was the more reasonable option. Happy to discuss it with the team if there are strong objections though.

This is also why I moved the vendor/ folder to vendor-bins/: the existence of a vendor/ folder made Go very sad indeed.

Checklist

  • I added tests for this PR's change (or confirmed tests are not viable).
  • If this PR introduced a user-visible change, I added documentation into docs/.
  • 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.
  • I linked this PR to any referenced GitHub issues, if they exist.

Also I didn't include any tests because:

  • The operation of this script is extremely straightforward and mechanical.
  • I don't see a good way for us to integration test against an actual FOSSA instance, but if I'm missing something I'd love to be shown how I can do it.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
@jssblck jssblck requested review from a team and meghfossa and removed request for a team October 19, 2021 00:15
@jssblck jssblck removed the request for review from meghfossa October 19, 2021 18:41
@jssblck
Copy link
Member Author

jssblck commented Oct 19, 2021

@meghfossa removed your request for review for now because these build scripts are causing me grief. Once I have it figured out I'll re-request from you!


echo "Vendored binaries are ready for use"
ls -lh vendor/
echo "Vendor-binsed binaries are ready for use"
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm guessing a find-and-replace caught this one.

Copy link
Member Author

Choose a reason for hiding this comment

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

woops! 😅

@jssblck jssblck requested a review from meghfossa October 19, 2021 19:44
@meghfossa
Copy link
Contributor

@kitified Does attribution require specific permission or FF enabled for the org? (I'm getting FOSSA API: no permission) error - this is with my free seat account on app.fossa.com.

@jssblck
Copy link
Member Author

jssblck commented Oct 20, 2021

@meghfossa you have to run it with a full token, otherwise I'm not aware of anything special.

Copy link
Contributor

@meghfossa meghfossa left a comment

Choose a reason for hiding this comment

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

Please look at comments on Github Build workflow before merging!

Other than that LGTM.

  • Can you also add issue in backlog to remove this mode post April 2022?
  • It would nice if you can modify Makefile to build compat-attribution in this PR as well.

.gitignore Show resolved Hide resolved
Changelog.md Outdated
@@ -1,5 +1,9 @@
# Spectrometer Changelog

## Unreleased
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's have a dedicated release since, we are adding new executable in distribution

Copy link
Member Author

Choose a reason for hiding this comment

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

Agreed - I always make a final commit right before merging which sets the appropriate release number.

.github/workflows/build.yml Outdated Show resolved Hide resolved
.github/workflows/build.yml Outdated Show resolved Hide resolved
@jssblck
Copy link
Member Author

jssblck commented Oct 21, 2021

It would nice if you can modify Makefile to build compat-attribution in this PR as well.

@meghfossa is there a specific reason you want this? I personally would rather we didn't, as we don't build other targets in the makefile already (for example, pathfinder) and this script isn't something we intend to be a common workflow.

@meghfossa
Copy link
Contributor

It would nice if you can modify Makefile to build compat-attribution in this PR as well.

@meghfossa is there a specific reason you want this? I personally would rather we didn't, as we don't build other targets in the makefile already (for example, pathfinder) and this script isn't something we intend to be a common workflow.

That's good catch on pathfinder not existing on makefile - I take this back, feel free to ignore this comment.

@jssblck jssblck enabled auto-merge (squash) October 21, 2021 19:14
@jssblck jssblck merged commit 3a37d8c into master Oct 21, 2021
@jssblck jssblck deleted the reports-compat branch October 21, 2021 19:29
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.

3 participants