fossas / fossa-cli

Fast, portable and reliable dependency analysis for any codebase. Supports license & vulnerability scanning for large monoliths. Language-agnostic; integrates with 20+ build systems.
https://fossa.com
Other
1.29k stars 173 forks source link

Do not error on rocky linux referenced deps #1473

Closed gilfaizon closed 2 months ago

gilfaizon commented 2 months ago

Overview

If using rocky as an os in the fossa-deps.yml, fossa will fail to read the fossa-deps file with the following error:

[ERROR] An issue occurred

  *** Relevant Errors ***

      Error: parsing file: path/to/fossa-deps.yml
        Aeson exception:
        Error in $['referenced-dependencies'][1]: Provided os: rocky is not supported! Please provide oneOf: alpine, centos, debian, redhat, ubuntu, oraclelinux, busybox, sles, fedora
      Support: If you believe this to be a defect, please report a bug to FOSSA support at https://support.fossa.com, with a copy of: /path/to/fossa-deps.yml

This aims to fix that, and allow the use of rocky for rpm-generic deps. It also updates the fossa-deps schema.

Acceptance criteria

FOSSA CLI analysis does not fail when using rocky as an os value.

Testing plan

I created a fossa-deps.yml with the follwing:

referenced-dependencies:
- name: binutils
  type: rpm-generic
  arch: x86_64
  os: rocky
  osVersion: '8.6'
  version: 0:2.30-113.el8

This will fail on fossa-cli versions outside of this branch.

I tested by downloading the built MacOS arm build from this run:

My scan was successful, and the dependency was included in my FOSSA project.

Risks

I looked at this very quickly; I may have missed a spot where I need to update the OS, or provide additional info.

References

N/A

Checklist

spatten commented 2 months ago

@csasarak does your work on Core affect this ticket at all?

https://github.com/fossas/FOSSA/pull/13898

csasarak commented 2 months ago

I don't think it does directly because in this case we're validating the OS against a list internal to the CLI. We actually do support Rocky Linux in our backend so I think it's OK to merge this PR.

However, the future "correct" solution I think is to either not validate or fetch a list of valid distributions at some point during analysis and validate against that. In the future if we're able to more easily add support for other Linux distributions we don't want to be forced to coordinate new CLI releases with the addition of that support. So I think I'd also be good with a version of this that either warns or doesn't do the check at all.

@spatten If you want to we can open up a discussion internally but I think the above is what we ought to do.

csasarak commented 2 months ago

I guess one possible reason to do the validation here is to potentially lessen the load on Core. I think if Core can't handle that we should figure out a way to fix it rather than maintaining this logic IMO.

spatten commented 2 months ago

I guess one possible reason to do the validation here is to potentially lessen the load on Core. I think if Core can't handle that we should figure out a way to fix it rather than maintaining this logic IMO.

However, the future "correct" solution I think is to either not validate or fetch a list of valid distributions at some point during analysis and validate against that. In the future if we're able to more easily add support for other Linux distributions we don't want to be forced to coordinate new CLI releases with the addition of that support. So I think I'd also be good with a version of this that either warns or doesn't do the check at all.

I agree with that. This PR is still worth merging, but in the future we should skip validation or move the validation to Core