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

go-require: lots of false positives when scanning the gateway-api repo #67

Closed
dprotaso opened this issue Feb 28, 2024 · 4 comments · Fixed by #70
Closed

go-require: lots of false positives when scanning the gateway-api repo #67

dprotaso opened this issue Feb 28, 2024 · 4 comments · Fixed by #70
Labels
false-positive Checker's false positive example

Comments

@dprotaso
Copy link

dprotaso commented Feb 28, 2024

git clone https://github.com/kubernetes-sigs/gateway-api.git
cd gateway-api
git checkout 4f71b2306e7df666a8528dda59ede6620ad0e4e2
go install github.com/Antonboom/testifylint@latest
testifylint ./...

You'll see tons of false positives - I've disabled the linter so it's important to checkout the repo at that SHA

/gateway-api/conformance/utils/kubernetes/certificate.go:45:2: empty: use require.NotEmpty
/gateway-api/conformance/utils/http/mirror.go:47:4: go-require: require must only be used in the goroutine running the test function
/gateway-api/conformance/tests/gateway-modify-listeners.go:152:4: len: use require.Lenf
/gateway-api/conformance/tests/gateway-static-addresses.go:81:3: go-require: require must only be used in the goroutine running the test function
/gateway-api/conformance/tests/gateway-static-addresses.go:82:3: go-require: require must only be used in the goroutine running the test function
/gateway-api/conformance/tests/gateway-static-addresses.go:98:3: go-require: require must only be used in the goroutine running the test function
/gateway-api/conformance/tests/gateway-static-addresses.go:103:3: go-require: require must only be used in the goroutine running the test function
/gateway-api/conformance/tests/gateway-static-addresses.go:119:3: go-require: require must only be used in the goroutine running the test function
/gateway-api/conformance/tests/gateway-static-addresses.go:124:3: go-require: require must only be used in the goroutine running the test function
/gateway-api/conformance/tests/gateway-static-addresses.go:136:3: go-require: require must only be used in the goroutine running the test function
/gateway-api/conformance/tests/gateway-static-addresses.go:137:3: go-require: require must only be used in the goroutine running the test function
/gateway-api/conformance/tests/gateway-static-addresses.go:138:3: go-require: require must only be used in the goroutine running the test function
/gateway-api/conformance/tests/gateway-static-addresses.go:139:3: go-require: require must only be used in the goroutine running the test function
/gateway-api/conformance/tests/httproute-reference-grant.go:72:3: go-require: require must only be used in the goroutine running the test function

There are no go-routines in /gateway-api/conformance/tests/gateway-static-addresses.go

See: https://github.com/kubernetes-sigs/gateway-api/blob/4f71b2306e7df666a8528dda59ede6620ad0e4e2/conformance/tests/gateway-static-addresses.go

@Antonboom
Copy link
Owner

Antonboom commented Mar 2, 2024

hi, @dprotaso!

thank you for request.

not false positive:

// kubernetes/certificate.go:45:2: empty: use require.NotEmpty
require.Greater(t, len(hosts), 0, "require a non-empty hosts for Subject Alternate Name values")

not false positive:

// gateway-modify-listeners.go:152:4: len: use require.Lenf
require.Equalf(t, 2, len(mutate.Spec.Listeners), "the gateway must have 2 listeners")

not false positive:

// utils/http/mirror.go:47:4: go-require: require must only be used in the goroutine running the test function
for _, mirrorPod := range mirrorPods {
	go func(mirrorPod BackendRef) {
		defer wg.Done()

		require.Eventually(t, func() bool {

About another go-require – I will investigate.


Anyway you can just disable one checker, not overall linter 👌

https://golangci-lint.run/usage/linters/#testifylint

@Antonboom Antonboom added the bug Something isn't working label Mar 2, 2024
@Antonboom
Copy link
Owner

@dprotaso, thanks for this tricky case

fixed!

/private/tmp/gateway-api 4f71b230
$ testifylint ./...        
/private/tmp/gateway-api/conformance/utils/kubernetes/certificate.go:45:2: empty: use require.NotEmpty
/private/tmp/gateway-api/conformance/utils/http/mirror.go:47:4: go-require: require must only be used in the goroutine running the test function
/private/tmp/gateway-api/conformance/tests/gateway-modify-listeners.go:152:4: len: use require.Lenf

@dprotaso
Copy link
Author

dprotaso commented Mar 2, 2024

awesome thanks @Antonboom - yeah some of them are not false positives.

What's the best way to pick up these changes? The project is using golangci-lint

@Antonboom
Copy link
Owner

Antonboom commented Mar 2, 2024

What's the best way to pick up these changes?

golangci-lint will pull the new version automatically and these changes will be available in the new golangci-lint's release

@Antonboom Antonboom changed the title Lots of false positives when scanning the gateway-api repo go-require: lots of false positives when scanning the gateway-api repo Jun 8, 2024
@Antonboom Antonboom added false-positive Checker's false positive example and removed bug Something isn't working labels Jun 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
false-positive Checker's false positive example
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants