golangci / golangci-lint-action

Official GitHub Action for golangci-lint from its authors
https://github.com/marketplace/actions/golangci-lint
MIT License
1.1k stars 152 forks source link

Dependabot PRs fail CI because it ignore only new issues #1045

Closed grantstephens closed 6 months ago

grantstephens commented 6 months ago

Welcome

Description of the problem

:wave: Hi. Running into a strange problem and I think I've narrowed it down to the action. Whenever Dependabot makes a new PR then golangci-lint fails. It fails because it seems to ignore the only new issues flag. In other words the line that runs golangci-lint looks like:

/home/runner/golangci-lint-1.58.2-linux-amd64/golangci-lint run --new-from-patch=/tmp/tmp-1633-UlWI7xtfg9sA/pull.patch --new=false --new-from-rev= --timeout 5m0s

But all the issues in the repo are found, i.e. not just new ones. In theory there shouldn't be any new issues for a dependabot PR, but that's neither here nor there. The action works perfectly and only flags new issues if somebody other than dependabot makes a PR.

Version of golangci-lint

1.58.2

Version of the GitHub Action

v6

Workflow file

```yaml name: golangci-lint on: push: branches: - main - master pull_request: permissions: contents: read pull-requests: read jobs: golangci: name: lint runs-on: ubuntu-latest env: GOPRIVATE: github.com/xxxx GH_ACCESS_TOKEN: xxxx steps: - name: Checkout uses: actions/checkout@v4 with: ref: "refs/pull/${{ github.event.number }}/head" - run: git config --global url."https://$GH_ACCESS_TOKEN@github.com/".insteadOf "https://github.com/" - name: Setup Go uses: actions/setup-go@v5 with: go-version-file: 'go.mod' - name: GolangCI-Lint uses: golangci/golangci-lint-action@v6 with: version: latest only-new-issues: true args: --timeout 5m0s ```

Go version

1.21.1

Code example or link to a public repository

FWIW here is the dependabot file ```yaml version: 2 updates: - package-ecosystem: "gomod" directory: "/" schedule: interval: "weekly" groups: gomod: applies-to: version-updates patterns: - "*" update-types: - "minor" - "patch" - package-ecosystem: "github-actions" directory: "/" schedule: interval: "weekly" ```
ldez commented 6 months ago

Hello,

Can you provide the logs? or a link to a public repository?

I think it's a duplicate of #996

grantstephens commented 6 months ago

I had a look at #996, but I don't have line

failed to fetch pull request patch: RequestError [HttpError]: The diff could not be processed because too many files changed

It may be related though. I ran it in debug mode and there are more than 5Mb of logs. I don't see any errors in there. Could you narrow it down as to which sections you'd want to see as I'd prefer not to post the whole thing in case there are any secrets in there. Unfortunately it is a private repo, so can't share it.\ Here are some snippets of the logs which may be useful

``` 2024-05-20T09:12:23.0767758Z ##[group]Run actions/checkout@v4 2024-05-20T09:12:23.0768203Z with: 2024-05-20T09:12:23.0768481Z ref: refs/pull/1625/head 2024-05-20T09:12:23.0768864Z repository: fastly/repo 2024-05-20T09:12:23.0769436Z token: *** 2024-05-20T09:12:23.0769728Z ssh-strict: true 2024-05-20T09:12:23.0770034Z ssh-user: git 2024-05-20T09:12:23.0770340Z persist-credentials: true 2024-05-20T09:12:23.0770708Z clean: true 2024-05-20T09:12:23.0771020Z sparse-checkout-cone-mode: true 2024-05-20T09:12:23.0771427Z fetch-depth: 1 2024-05-20T09:12:23.0771712Z fetch-tags: false 2024-05-20T09:12:23.0772111Z show-progress: true 2024-05-20T09:12:23.0772427Z lfs: false 2024-05-20T09:12:23.0772686Z submodules: false 2024-05-20T09:12:23.0773003Z set-safe-directory: true ```
``` 2024-05-20T09:12:41.5601084Z ##[group]Run golangci/golangci-lint-action@v6 2024-05-20T09:12:41.5601661Z with: 2024-05-20T09:12:41.5602121Z version: latest 2024-05-20T09:12:41.5602512Z only-new-issues: true 2024-05-20T09:12:41.5603086Z args: --timeout 5m0s 2024-05-20T09:12:41.5603666Z install-mode: binary 2024-05-20T09:12:41.5604284Z github-token: *** 2024-05-20T09:12:41.5604680Z skip-cache: false 2024-05-20T09:12:41.5605084Z skip-save-cache: false 2024-05-20T09:12:41.5605700Z problem-matchers: false 2024-05-20T09:12:41.5606192Z cache-invalidation-interval: 7 ```
ldez commented 6 months ago

Can you try to remove this line:

    steps:
      - name: Checkout
        uses: actions/checkout@v4
        with:
          ref: "refs/pull/${{ github.event.number }}/head" ## <----

dependabot creates PRs from branches and not a fork, so the event should be a push and not pull_request

only-new-issues uses GitHub API so you don't need to check out extra commits.

https://github.com/golangci/golangci-lint-action?tab=readme-ov-file#only-new-issues

ldez commented 6 months ago

In debug mode, you should have a log only new issues on ... with the event's name.

You should also have a log failed to ... if the access to the GitHub API fails.

grantstephens commented 6 months ago

Ok, I've got 2024-05-20T09:12:43.8001361Z only new issues on pull_request: /tmp/tmp-1633-UlvI7xTFx9sA/pull.patch but no failed to line. Will test removing the ref line today.

grantstephens commented 6 months ago

Ok, I've run it without the ref on the checkout and now get these: The run line looks like 2024-05-21T16:17:35.6170250Z Running [/home/runner/golangci-lint-1.58.2-linux-amd64/golangci-lint run --new-from-patch=/tmp/tmp-1709-RKS39Iu8s10T/pull.patch --new=false --new-from-rev= --timeout 5m0s] in [/home/runner/work/bulleit/bulleit] ...

``` 2024-05-21T16:17:13.6321446Z ##[group]Run actions/checkout@v4 2024-05-21T16:17:13.6321787Z with: 2024-05-21T16:17:13.6322009Z repository: fastly/myrepo 2024-05-21T16:17:13.6322465Z token: *** 2024-05-21T16:17:13.6322695Z ssh-strict: true 2024-05-21T16:17:13.6322925Z ssh-user: git 2024-05-21T16:17:13.6323167Z persist-credentials: true 2024-05-21T16:17:13.6323444Z clean: true 2024-05-21T16:17:13.6323679Z sparse-checkout-cone-mode: true 2024-05-21T16:17:13.6323988Z fetch-depth: 1 2024-05-21T16:17:13.6324227Z fetch-tags: false 2024-05-21T16:17:13.6324484Z show-progress: true 2024-05-21T16:17:13.6324729Z lfs: false 2024-05-21T16:17:13.6324947Z submodules: false 2024-05-21T16:17:13.6325191Z set-safe-directory: true 2024-05-21T16:17:13.6325455Z env: 2024-05-21T16:17:13.6325938Z GOPRIVATE: github.com/fastly 2024-05-21T16:17:13.6326218Z GH_ACCESS_TOKEN: ```

I don't see any other errors in the logs. Could it be that --new=false?

ldez commented 6 months ago

can you provide your golangci-lint configuration?

Could it be that --new=false?

No, this option is for a specific use case: unstaged changes or untracked files

https://golangci-lint.run/usage/configuration/#issues-configuration

grantstephens commented 6 months ago

Sure:

2024-05-21T16:17:33.1074355Z ##[group]Run golangci/golangci-lint-action@v6
2024-05-21T16:17:33.1074945Z with:
2024-05-21T16:17:33.1075269Z   version: latest
2024-05-21T16:17:33.1075659Z   only-new-issues: true
2024-05-21T16:17:33.1076078Z   args: --timeout 5m0s
2024-05-21T16:17:33.1076488Z   install-mode: binary
2024-05-21T16:17:33.1077139Z   github-token: ***
2024-05-21T16:17:33.1077535Z   skip-cache: false
2024-05-21T16:17:33.1077936Z   skip-save-cache: false
2024-05-21T16:17:33.1078381Z   problem-matchers: false
2024-05-21T16:17:33.1078859Z   cache-invalidation-interval: 7
2024-05-21T16:17:33.1079329Z env:
2024-05-21T16:17:33.1079668Z   GOPRIVATE: github.com/fastly
2024-05-21T16:17:33.1080135Z   GH_ACCESS_TOKEN: 

FWIW this is the .golangci-lint.yaml in the root of the repo:

---
linters-settings:
  govet:
    enable:
      - nilness
  goimports:
    local-prefixes: github.com/fastly

linters:
  enable:
    - gofumpt
    - goimports
    - gosec
    - ineffassign
ldez commented 6 months ago

Can you check if you have something like:

ldez commented 6 months ago

Can you also give me an overview (a few lines) of the linting issues?

grantstephens commented 6 months ago

Don't have patch file, result by or revgrep

ldez commented 6 months ago

Maybe it's related to the content of the patch :thinking:

But the call is really simple:

https://github.com/golangci/golangci-lint-action/blob/182ce0be585fd9240cb6524aeb7e0c4d1d707656/src/run.ts#L62-L69

The same thing in Go:

package main

import (
    "context"
    "fmt"
    "log"

    "github.com/google/go-github/v62/github"
    "golang.org/x/oauth2"
)

func main() {
    ctx := context.Background()

    ts := oauth2.StaticTokenSource(&oauth2.Token{AccessToken: "token"})
    client := github.NewClient(oauth2.NewClient(ctx, ts))

    org := "foo"
    repo := "bar"
    number := 8

    raw, _, err := client.PullRequests.GetRaw(ctx, org, repo, number, github.RawOptions{Type: github.Diff})
    if err != nil {
        log.Fatal(err)
    }

    fmt.Println(raw)
}

Can you try it?

grantstephens commented 6 months ago

Hmm, so because it is a dependabot PR, the patch is just the diff of the go.mod and go.sum. I've included an excerpt:

diff --git a/go.mod b/go.mod
index 0c2f7f1d4..82ff186bd 100644
--- a/go.mod
+++ b/go.mod
@@ -34,31 +34,31 @@ require (
    go.uber.org/atomic v1.11.0
    go.uber.org/zap v1.27.0
    golang.org/x/crypto v0.23.0
-   golang.org/x/exp v0.0.0-20231006140011-7918f672742d
+   golang.org/x/exp v0.0.0-20231110203233-9a3e6036ecaa
    golang.org/x/oauth2 v0.20.0
    golang.org/x/sys v0.20.0
 )
...
diff --git a/go.sum b/go.sum
index d16dd3273..8e4e44ba3 100644
--- a/go.sum
+++ b/go.sum
@@ -20,10 +20,10 @@ cloud.google.com/go v0.79.0/go.mod h1:3bzgcEeQlzbuEAYu4mrWhKqWjmpprinYgKJLgKHnbb
 cloud.google.com/go v0.81.0/go.mod h1:mk/AM35KwGk/Nm2YSeZbxXdrNK3KZOYHmLkOqC2V6E0=
 cloud.google.com/go v0.112.2 h1:ZaGT6LiG7dBzi6zNOvVZwacaXlmf3lRqnC4DQzqyRQw=
 cloud.google.com/go v0.112.2/go.mod h1:iEqjp//KquGIJV/m+Pk3xecgKNhV+ry+vVTsy4TbDms=
-cloud.google.com/go/auth v0.2.2 h1:gmxNJs4YZYcw6YvKRtVBaF2fyUE6UrWPyzU8jHvYfmI=
-cloud.google.com/go/auth v0.2.2/go.mod h1:2bDNJWtWziDT3Pu1URxHHbkHE/BbOCuyUiKIGcNvafo=
-cloud.google.com/go/auth/oauth2adapt v0.2.1 h1:VSPmMmUlT8CkIZ2PzD9AlLN+R3+D1clXMWHHa6vG/Ag=
ldez commented 6 months ago

I can't find a way to reproduce the problem :thinking:

https://github.com/ldez/redesignedwaffle/pull/2

The project contains issues, I used only-new-issues, the PR is from a branch and contains only updated dependencies.

grantstephens commented 6 months ago

Hmm, I'm wondering if it is something specific to dependabot PRs- could I make PR to add dependabot to that repo and see if the PR the dependabot creates fails?

ldez commented 6 months ago

Even a PR from dependabot doesn't produce the problem:

https://github.com/ldez/redesignedwaffle/pull/3

ldez commented 6 months ago

It looks like there is something specific to your repository: maybe it's a problem with a token, a repository setting, the dependabot configuration, etc.

Can you give me an overview (a few lines) of the linting issues reported inside this PR? Is it real linter reports or typecheck errors?

grantstephens commented 6 months ago

Ah, that is interesting, they all seem to be type check errors:

invalid operation: cannot index s (variable of type sortedSources) (typecheck)

I hadn't noticed that before.

Something else interesting- if there are changes in the main branch and I merge those back into the pr branch, then it succeeds. This makes me think it is a token or something that dependabot doesn't have, but once I've made a commit on the branch then it works.

ldez commented 6 months ago

typecheck errors are mainly compilation errors.

https://golangci-lint.run/welcome/faq/#why-do-you-have-typecheck-errors

In your context, I think it's missing access to private dependencies.

ldez commented 6 months ago

any news?

grantstephens commented 6 months ago

Hi @ldez

I'm busy working on it with our security team. I think we can close it as it looks like a permissions issue, but doesn't look like it from the outside.

Thank you very much for your help!