Conversation
Signed-off-by: Carlos Eduardo Arango Gutierrez <eduardoa@nvidia.com>
|
Inspired by NVIDIA/gpu-operator#721 that is proposing parallel runs |
cmd/action/ci/ci.go
Outdated
|
|
||
| import ( | ||
| "fmt" | ||
| "math/rand" //nolint:all |
There was a problem hiding this comment.
Do we want to add a comment as to why this is done?
| "math/rand" //nolint:all | |
| "math/rand" //nolint:gosec // We use randomization for appending unique suffixes and this need not be cryptographically secure. |
|
|
||
| b := make([]byte, 8) | ||
| for i := range b { | ||
| b[i] = charset[rand.Intn(len(charset))] |
There was a problem hiding this comment.
Question: Is the lint error reported here or at the import?
There was a problem hiding this comment.
It would be reported here. Tested in the device plugin:
✗ make check
golangci-lint run ./...
tests/e2e/framework/framework.go:149:53: G404: Use of weak random number generator (math/rand instead of crypto/rand) (gosec)
f.UniqueName = fmt.Sprintf("%s-%08x", f.BaseName, rand.Int31())
^
tests/e2e/framework/util.go:50:22: G404: Use of weak random number generator (math/rand instead of crypto/rand) (gosec)
return strconv.Itoa(rand.Intn(10000))
^
make: *** [Makefile:81: lint] Error 1
There was a problem hiding this comment.
I mean, do we want to add the nolint here and NOT at the import.
There was a problem hiding this comment.
I did a run without the comment on both
❯ golangci-lint run ./... |grep weakand golangci-lint didn't complained
There was a problem hiding this comment.
I have edited it to only have the lint comment on the code and not the import.
There was a problem hiding this comment.
This project doesn't seem to have a .golangci file. Could that be why we're not seeing the issue?
There was a problem hiding this comment.
I copied the one from device-plug repo thinking the same, and got no error
❯ golangci-lint --version
golangci-lint has version 1.59.0 built with go1.22.3 from 2059b18 on 2024-05-25T11:38:08Z1c72260 to
d77bad6
Compare
eb4e8a3 to
654f3d4
Compare
|
Updated as discussed @elezar |
When running parallel workflows we need to have UID for each instance to avoid them having the same name