Skip to content

🐛 remove setup-go requirement for Packaging with goreleaser #4673

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

Merged
merged 3 commits into from
Jun 30, 2025

Conversation

AdamKorcz
Copy link
Contributor

@AdamKorcz AdamKorcz commented Jun 26, 2025

What kind of change does this PR introduce?

(Is it a bug fix, feature, docs update, something else?)

What is the current behavior?

Currently, the Go JobMatcher requires both the setup-go and goreleaser actions to be used to consider the project to use and automated releaser. Some users have adopted other actions that install Go and still use the goreleaser action in which case Scorecard will not see the goreleaser action.

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

This PR removes the requirement that users must use the setup-go action to Scorecard recognizing goreleaser.

  • Tests for the changes have been added (for bug fixes/features)

Which issue(s) this PR fixes

Fixes #4617

Special notes for your reviewer

Does this PR introduce a user-facing change?

For user-facing changes, please add a concise, human-readable release note to
the release-note

(In particular, describe what changes users might need to make in their
application as a result of this pull request.)

Fix bug in parsing workflow files.

Signed-off-by: Adam Korczynski <[email protected]>
@AdamKorcz AdamKorcz requested a review from a team as a code owner June 26, 2025 18:07
@AdamKorcz AdamKorcz requested review from justaugustus and raghavkaul and removed request for a team June 26, 2025 18:07
@AdamKorcz AdamKorcz temporarily deployed to integration-test June 26, 2025 18:07 — with GitHub Actions Inactive
Copy link

codecov bot commented Jun 26, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 68.30%. Comparing base (353ed60) to head (1ebe5b6).
Report is 193 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4673      +/-   ##
==========================================
+ Coverage   66.80%   68.30%   +1.49%     
==========================================
  Files         230      249      +19     
  Lines       16602    18895    +2293     
==========================================
+ Hits        11091    12906    +1815     
- Misses       4808     5130     +322     
- Partials      703      859     +156     
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@spencerschrock
Copy link
Member

Currently, matches will return false if the first Step of the JobMatcher fails. This is a problem if there are multiple steps in the JobMatcher where the second one should return true.

This is by design, the matcher is looking for every step in the pattern to be in the job. "Each step in this field has a matching step in the job"

// JobMatcher is rule for matching a job.
type JobMatcher struct {
// The text to be logged when a job match is found.
LogText string
// Each step in this field has a matching step in the job.
Steps []*JobMatcherStep

As written, this PR would give credit to any repo which uses actions/setup-go regardless of if it uses goreleaser. e.g.
https://github.com/spencerschrock/actions-test/blob/f4f716a8b0df876e28868d879246863b2c4bcfa8/.github/workflows/disk.yml#L12

  "score": 10.0,
  "checks": [
    {
      "details": [
        "Info: Project packages its releases by way of GitHub Actions.: .github/workflows/disk.yml:8"
      ],
      "score": 10,
      "reason": "packaging workflow detected",
      "name": "Packaging",

I think the question we want to ask is: do we need actions/setup-go to give credit for a goreleaser release workflow.

// Go packages.
Steps: []*JobMatcherStep{
{
Uses: "actions/setup-go",
},
{
Uses: "goreleaser/goreleaser-action",
},
},

@AdamKorcz
Copy link
Contributor Author

Thanks @spencerschrock Essentially this means that we (in this case) only detect a goreleaser use if it is preceded by a step using actions/setup-go. Do you know what the logic behind this is? As in, why do we only want to detect goreleaser if it is preceded by setup-go?

@spencerschrock
Copy link
Member

only detect a goreleaser use if it is preceded by a step using actions/setup-go

as long as all steps are present, I don't think order matters to our current implementation.

Do you know what the logic behind this is?

This predates my time on the project. Detection for Go was first added in #800, but similar logic (expecting language setup) was added in #132 for npm and Python.

My guess is it was to cut down on false positives, as the entire workflow file was scanned with a regex, instead of the workflow parsing and step analysis that happens now.

@AdamKorcz
Copy link
Contributor Author

okay, it may be better to remove actions/setup-go from the JobMatcher Uses. It seems unnecessary that both actions need to exist for the goreleaser action to be valid.

@spencerschrock
Copy link
Member

okay, it may be better to remove actions/setup-go from the JobMatcher Uses. It seems unnecessary that both actions need to exist for the goreleaser action to be valid.

Seems reasonable to me.

Signed-off-by: Adam Korczynski <[email protected]>
@AdamKorcz
Copy link
Contributor Author

Updated the PR and the PR description.

@spencerschrock
Copy link
Member

/scdiff generate Packaging,Token-Permissions

Copy link

@spencerschrock spencerschrock deployed to integration-test June 30, 2025 17:44 — with GitHub Actions Active
@spencerschrock spencerschrock changed the title 🐛 fix bug in workflow parsing 🐛 remove setup-go requirement for Packaging with goreleaser Jun 30, 2025
@spencerschrock spencerschrock enabled auto-merge (squash) June 30, 2025 17:45
@spencerschrock spencerschrock merged commit bb9c347 into ossf:main Jun 30, 2025
41 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

BUG packaging not recognized
2 participants