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

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

Closed
3 tasks done
grantstephens opened this issue May 20, 2024 · 21 comments
Closed
3 tasks done

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

grantstephens opened this issue May 20, 2024 · 21 comments
Labels
question Further information is requested

Comments

@grantstephens
Copy link

Welcome

  • Yes, I understand that the GitHub action repository is not the repository of golangci-lint itself.
  • Yes, I've searched similar issues on GitHub and didn't find any.
  • Yes, I've included all information below (version, config, etc).

Description of the problem

👋 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

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://[email protected]/".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
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
Copy link
Member

ldez commented May 20, 2024

Hello,

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

I think it's a duplicate of #996

@ldez ldez added the question Further information is requested label May 20, 2024
@grantstephens
Copy link
Author

grantstephens commented May 20, 2024

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
Copy link
Member

ldez commented May 20, 2024

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
Copy link
Member

ldez commented May 20, 2024

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
Copy link
Author

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
Copy link
Author

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
Copy link
Member

ldez commented May 22, 2024

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
Copy link
Author

grantstephens commented May 22, 2024

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
Copy link
Member

ldez commented May 22, 2024

Can you check if you have something like:

  • can't read from patch file
  • Can't process result by
  • can't prepare diff by revgrep

@ldez
Copy link
Member

ldez commented May 22, 2024

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

@grantstephens
Copy link
Author

Don't have patch file, result by or revgrep

@ldez
Copy link
Member

ldez commented May 22, 2024

Maybe it's related to the content of the patch 🤔

But the call is really simple:

const patchResp = await octokit.rest.pulls.get({
owner: ctx.repo.owner,
repo: ctx.repo.repo,
[`pull_number`]: pr.number,
mediaType: {
format: `diff`,
},
})

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
Copy link
Author

grantstephens commented May 22, 2024

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
Copy link
Member

ldez commented May 23, 2024

I can't find a way to reproduce the problem 🤔

ldez/redesignedwaffle#2

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

@grantstephens
Copy link
Author

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
Copy link
Member

ldez commented May 24, 2024

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

ldez/redesignedwaffle#3

@ldez
Copy link
Member

ldez commented May 24, 2024

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
Copy link
Author

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
Copy link
Member

ldez commented May 24, 2024

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
Copy link
Member

ldez commented May 29, 2024

any news?

@grantstephens
Copy link
Author

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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants