Skip to content
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

🌱 Add probes to main call #3688

Merged
merged 15 commits into from
Dec 12, 2023

Conversation

AdamKorcz
Copy link
Contributor

What kind of change does this PR introduce?

Feature

What is the current behavior?

What is the new behavior (if this is a feature change)?**

This is WIP to enable invoking probes via Scorecards main function.

Usage:
go run main.go --repo=github.com/ossf/scorecard --probes=notArchived,hasLicenseFile,hasOSVVulnerabilities --format=probe

Output (Manually pretty'ed):

{
  "date": "2023-11-20",
  "repo": {
    "name": "github.com/ossf/scorecard",
    "commit": "0276a7cd7284e077f22b50208345201485029fbd"
  },
  "scorecard": {
    "version": "",
    "commit": "unknown"
  },
  "findings": [
    {
      "probe": "notArchived",
      "outcome": 12,
      "message": "Repository is not archived."
    },
    {
      "probe": "hasLicenseFile",
      "outcome": 0,
      "message": "project does not have a license file",
      "remediation": {
        "text": "For Github projects, follow [this guide](https://docs.github.com/en/repositories/managing-your-repositorys-settings-and-features/customizing-your-repository/licensing-a-repository) to determine which license to apply to your project and establish a license file for your project.\nFor Gitlab projects, create the license in a .adoc, .asc, .docx, .doc, .ext, .html, .markdown, .md, .rst, .txt, or .xml, named LICENSE, COPYRIGHT, or COPYING, and place it in the top-level directory. To identify a specific license, use an SPDX license identifier in the filename. Examples include LICENSE.md, Apache-2.0-LICENSE.md or LICENSE-Apache-2.0.\nAlternately, create a LICENSE directory and add a license file(s) with a name that matches your SPDX license identifier. such as LICENSES/Apache-2.0.txt.",
        "markdown": "",
        "effort": 1
      }
    },
    {
      "probe": "hasOSVVulnerabilities",
      "outcome": 12,
      "message": "Project does not contain OSV vulnerabilities"
    }
  ]
}
  • Tests for the changes have been added (for bug fixes/features)

Which issue(s) this PR fixes

NONE

Special notes for your reviewer

Tests and formatting has not been done yet.

Does this PR introduce a user-facing change?

Yes

Users can run probes via the command line.

Copy link

codecov bot commented Nov 20, 2023

Codecov Report

Merging #3688 (41f2839) into main (db7b6e7) will decrease coverage by 5.73%.
The diff coverage is 43.58%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3688      +/-   ##
==========================================
- Coverage   76.41%   70.68%   -5.73%     
==========================================
  Files         214      214              
  Lines       14628    14803     +175     
==========================================
- Hits        11178    10464     -714     
- Misses       2779     3715     +936     
+ Partials      671      624      -47     

@AdamKorcz AdamKorcz force-pushed the add-probes-to-main-call branch from 7ed7abb to 0281085 Compare November 22, 2023 11:40
@AdamKorcz AdamKorcz force-pushed the add-probes-to-main-call branch from 0281085 to b64e134 Compare November 22, 2023 12:10
@AdamKorcz AdamKorcz force-pushed the add-probes-to-main-call branch from b64e134 to d050bf6 Compare November 22, 2023 12:14
@AdamKorcz AdamKorcz force-pushed the add-probes-to-main-call branch from 7cc4bd4 to f9a3568 Compare November 22, 2023 13:42
@AdamKorcz AdamKorcz force-pushed the add-probes-to-main-call branch from f9a3568 to bf30105 Compare November 22, 2023 13:48
@AdamKorcz AdamKorcz force-pushed the add-probes-to-main-call branch from bf30105 to 301c562 Compare November 22, 2023 14:00
@AdamKorcz AdamKorcz force-pushed the add-probes-to-main-call branch from 301c562 to bdfd990 Compare November 22, 2023 14:04
@AdamKorcz
Copy link
Contributor Author

AdamKorcz commented Nov 22, 2023

Most of the missing coverage is APIs in the ./cmd dir which do not have tests already.

Copy link
Member

@spencerschrock spencerschrock left a comment

Choose a reason for hiding this comment

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

Overall, I think this works. The CLI flag is what we want --probes, we don't break the RunScorecard function signature, and the behavior seems to be what we want.

But man is this hacky and fragile. A lot of the reasons are due to probe implementation, so fixing it in a followup is probably needed, and I've left some notes for me to file issues.

For now, I'm just checking on a backward compatibility question, relating to renaming package import aliases.

cmd/root.go Show resolved Hide resolved
probes/entries.go Outdated Show resolved Hide resolved
pkg/scorecard.go Outdated Show resolved Hide resolved
pkg/scorecard.go Outdated Show resolved Hide resolved
probes/entries.go Show resolved Hide resolved
probes/entries.go Show resolved Hide resolved
probes/entries.go Outdated Show resolved Hide resolved
Signed-off-by: AdamKorcz <[email protected]>
Signed-off-by: AdamKorcz <[email protected]>
Signed-off-by: AdamKorcz <[email protected]>
Signed-off-by: Adam Korczynski <[email protected]>
Signed-off-by: Adam Korczynski <[email protected]>
Signed-off-by: Adam Korczynski <[email protected]>
Signed-off-by: Adam Korczynski <[email protected]>
Signed-off-by: Adam Korczynski <[email protected]>
Signed-off-by: Adam Korczynski <[email protected]>
Signed-off-by: Adam Korczynski <[email protected]>
Signed-off-by: Adam Korczynski <[email protected]>
Signed-off-by: Adam Korczynski <[email protected]>
Copy link
Member

@spencerschrock spencerschrock left a comment

Choose a reason for hiding this comment

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

For now, I'm just checking on a backward compatibility question, relating to renaming package import aliases.

As expected, renaming imports is purely cosmetic, won't break anything.

@spencerschrock spencerschrock merged commit 3ce1daa into ossf:main Dec 12, 2023
37 of 38 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants