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

Intermittent incorrect "xyz is in the denylist" errors #25

Closed
bmhatfield opened this issue Aug 10, 2022 · 11 comments
Closed

Intermittent incorrect "xyz is in the denylist" errors #25

bmhatfield opened this issue Aug 10, 2022 · 11 comments
Labels
bug Something isn't working
Milestone

Comments

@bmhatfield
Copy link

Hi there,

We're using depguard as part of golangci-lint (v1.47.3). We've been getting intermittent false positives - given the same code and same rules, re-running multiple times will produce different results. Each of the false positives do not match any of our actual rules (of which we have 3). The false positives themselves change - they don't seem to be centered around any particular package in our repository. True positives (ie, a rule that has legitimately been broken) seem reliable.

I haven't been able to track down if any particular condition seems to cause this false positive behavior. Clearing the go and golangci-lint caches doesn't seem to influence the behavior. It seems to have become more prevalent on more recent versions of Go, especially since 1.18 but increasing in 1.19. It occurs both locally (MacOS) and in CI (CircleCI, Alpine Linux container).

I'm painfully aware that this issue description is light on technical detail, because I haven't been able to diagnose this issue so far. I've been delaying filing it for some time because of this, but nobody else seems to have filed this as an issue, so I thought I would at least start the conversation. Is anyone else seeing this?

@bmhatfield
Copy link
Author

It looks like this issue mentions the itermittent denylisting in the body, however, we are not observing the NPEs discussed in that issue.

@ldez
Copy link
Contributor

ldez commented Aug 21, 2022

hello @bmhatfield, I'm interested in a reproducible use case, have you something to share?

@jerome-laforge
Copy link

Is anyone else seeing this?

I am facing with the same issue. And as @bmhatfield, this issue is more prevalent with Go 1.19 on my local machine (Linux) and in our CI (on-premise gitlab).

@bmhatfield
Copy link
Author

hello @bmhatfield, I'm interested in a reproducible use case, have you something to share?

Here's an example snippet of our config, though I imagine anything would do this. Because this is intermittent / random, I don't really have a reliable reproduce case beyond golangci-lint, go 1.19, and maybe linux-based containers.

linters-settings:
  depguard:
    list-type: denylist
    packages:
      - log
      - "github.com/gogo/protobuf"
    packages-with-error-message:
      - log: "Use shared/logger"
      - github.com/gogo/protobuf: "Use golang/protobuf"
    include-go-root: true

@jerome-laforge
Copy link

As workaround, we have disabled the depguard linter into our golangci-linter configuration.

@ldez
Copy link
Contributor

ldez commented Aug 26, 2022

Disabling a linter is not really a workaround for this linter 😉

You just stopped to use the linter.

@jerome-laforge
Copy link

You 're right ;)
It just allow us to have reliable CI ;)

@dixonwille
Copy link
Member

Thanks for feedback. I have a WIP V2 branch that is very early in development (past few hours). I think the intermittent issues comes with golangci-lint making concurrent calls with depguard's single configuration (which it was not designed to handle back in golangci-lint v1.4). I believe these concurrent issues are what you are seeing.

@dixonwille dixonwille added this to the V2 milestone Aug 30, 2022
@dixonwille
Copy link
Member

So I have a working beta v2.0.0-beta.2

go install github.com/OpenPeeDeeP/depguard/v2/cmd/depguard@v2.0.0-beta.2

I have tested on a few repositories. But would like others to try it out to.

LukeShu added a commit to emissary-ingress/emissary that referenced this issue Sep 21, 2022
Scarjit added a commit to united-manufacturing-hub/united-manufacturing-hub that referenced this issue Sep 26, 2022
Scarjit added a commit to united-manufacturing-hub/united-manufacturing-hub that referenced this issue Sep 29, 2022
* feat: fixed json errors and some forbidden imports

* feat: fixed base tests

* feat: added more checks

* feat: next batch of lints fixed

* feat: more passed checks

* feat: next batch of lints

* feat: working pre-push hook

* feat: fixed test crashes

* fix: changed requirements for CONTRIBUTING.md

* fix: flaky depguard

* fix: upgraded golangci-lint to preventing OpenPeeDeeP/depguard#25
@bmhatfield
Copy link
Author

It looks like #23 has been pulled in by golangci-lint, which appears to resolve this problem. We did not realize that the main branch had previously been updated and then pulled into golangci-lint. We'll test for a couple days and close this issue if it's working!

@dixonwille
Copy link
Member

I have just released version 2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
No open projects
Status: Done
Development

No branches or pull requests

4 participants