golangci-lint: update to latest version & related tweaks #5737
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The latest golangci-lint version comes with two new detected issues, both of which we choose to silence here: 1) staticcheck: Deprecation of (crypto/x509).CertPool.Subjects(): This is still useful to us in the tests and there seems to be no good alternative (we use it merely to count the number of certs). 2) gosec: Potential integer overflow: This is a potentially useful check, but I didn't want to address this as part of this PR. https://github.com/ccoVeille/go-safecast might be useful for this (in fact it looks like it was inspired by the recent addition of this gosec rule). Additionally, change the Makefile to tell golangci-lint to emit all lints, as opposed to imposing its default limits. Those limits could actually be misleading, as it could e.g. lead to someone disabling lints without having seen all the reported issues. Signed-off-by: Carlo Teubner <[email protected]>
c4rlo
requested review from
evan2645,
amartinezfayo,
azdagron,
MarcosDY and
rturner3
as code owners
c4rlo
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The latest golangci-lint version comes with two new detected issues, both of which we choose to silence here:
staticcheck: Deprecation of
(crypto/x509).CertPool.Subjects():This is still useful to us in the tests and there seems to be no good alternative (we use it merely to count the number of certs).
gosec: Potential integer overflow:
This is a potentially useful check, but I didn't want to address this as part of this PR. https://github.com/ccoVeille/go-safecast might be useful for this (in fact it looks like it was inspired by the recent addition of this gosec rule).
Additionally, change the Makefile to tell golangci-lint to emit all lints, as opposed to imposing its default limits. Those limits could actually be misleading, as it could e.g. lead to someone disabling lints without having seen all the reported issues.
For completeness – full list of staticcheck CertPool.Subjects() reports:
And full list of gosec integer overflow reports:
cmd/spire-server/cli/federation/create.go:104:14: G115: integer overflow conversion int32 -> uint32 (gosec) codes.Code(r.Status.Code), ^ cmd/spire-server/cli/federation/update.go:100:14: G115: integer overflow conversion int32 -> uint32 (gosec) codes.Code(r.Status.Code), ^ pkg/server/datastore/sqlstore/migration.go:394:76: G115: integer overflow conversion uint64 -> int64 (gosec) if dbCodeVersion.Major != thisCodeVersion.Major || (math.Abs(float64(int64(dbCodeVersion.Minor)-int64(thisCodeVersion.Minor))) > 1) { ^ pkg/server/datastore/sqlstore/sqlstore.go:1310:14: G115: integer overflow conversion int -> int32 (gosec) return int32(count), nil ^ pkg/server/datastore/sqlstore/sqlstore.go:1596:14: G115: integer overflow conversion int -> int32 (gosec) return int32(count), nil ^ pkg/server/datastore/sqlstore/sqlstore.go:1695:15: G115: integer overflow conversion int -> int32 (gosec) val += int32(len(resp.Nodes)) ^ pkg/server/datastore/sqlstore/sqlstore.go:3334:15: G115: integer overflow conversion int -> int32 (gosec) val += int32(len(resp.Entries)) ^ pkg/server/datastore/sqlstore/sqlstore.go:3859:28: G115: integer overflow conversion int64 -> int32 (gosec) entry.X509SvidTtl = int32(r.RegTTL.Int64) ^ pkg/server/datastore/sqlstore/sqlstore.go:3862:27: G115: integer overflow conversion int64 -> int32 (gosec) entry.JwtSvidTtl = int32(r.RegJwtSvidTTL.Int64) ^ pkg/server/datastore/sqlstore/sqlstore_test.go:1578:30: G115: integer overflow conversion int -> uint (gosec) greaterThanEventID: uint(len(expectedEvents) / 2), ^ pkg/server/datastore/sqlstore/sqlstore_test.go:1579:30: G115: integer overflow conversion int -> uint (gosec) expectedFirstEventID: uint(len(expectedEvents)/2) + 1, ^ pkg/server/datastore/sqlstore/sqlstore_test.go:1585:30: G115: integer overflow conversion int -> uint (gosec) lessThanEventID: uint(len(expectedEvents) / 2), ^ pkg/server/datastore/sqlstore/sqlstore_test.go:1587:30: G115: integer overflow conversion int -> uint (gosec) expectedLastEventID: uint(len(expectedEvents)/2) - 1, ^ pkg/server/datastore/sqlstore/sqlstore_test.go:4063:30: G115: integer overflow conversion int -> uint (gosec) greaterThanEventID: uint(len(expectedEvents) / 2), ^ pkg/server/datastore/sqlstore/sqlstore_test.go:4064:30: G115: integer overflow conversion int -> uint (gosec) expectedFirstEventID: uint(len(expectedEvents)/2) + 1, ^ pkg/server/datastore/sqlstore/sqlstore_test.go:4070:30: G115: integer overflow conversion int -> uint (gosec) lessThanEventID: uint(len(expectedEvents) / 2), ^ pkg/server/datastore/sqlstore/sqlstore_test.go:4072:30: G115: integer overflow conversion int -> uint (gosec) expectedLastEventID: uint(len(expectedEvents)/2) - 1, ^ pkg/server/datastore/sqlstore/sqlstore_test.go:5286:17: G115: integer overflow conversion int -> uint (gosec) EventID: uint(len(expectedEvents) + 1), ^ pkg/server/plugin/upstreamauthority/v1.go:38:22: G115: integer overflow conversion int64 -> int32 (gosec) PreferredTtl: int32(preferredTTL / time.Second), ^ cmd/spire-server/cli/agent/agent_test.go:837:15: G115: integer overflow conversion int -> int32 (gosec) Count: int32(len(s.agents)), ^ cmd/spire-server/cli/token/generate.go:51:17: G115: integer overflow conversion int -> int32 (gosec) Ttl: int32(g.TTL), ^ test/clock/clock.go:93:36: G115: integer overflow conversion int -> int32 (gosec) if m.tickerCount.Load() >= int32(count) { ^ test/clock/clock.go:94:33: G115: integer overflow conversion int -> int32 (gosec) m.tickerCount.Add(-1 * int32(count)) ^ pkg/common/peertracker/peertracker_test.go:77:24: G115: integer overflow conversion int -> int32 (gosec) require.Equal(t, int32(os.Getpid()), conn.Info.Caller.PID) ^ pkg/common/peertracker/peertracker_test.go:81:24: G115: integer overflow conversion int -> int32 (gosec) require.Equal(t, int32(os.Getpid()), conn.Info.Watcher.PID()) ^ pkg/agent/endpoints/peertracker_test.go:66:48: G115: integer overflow conversion int -> int32 (gosec) func (w FakeWatcher) PID() int32 { return int32(os.Getpid()) } ^ pkg/server/plugin/keymanager/v1.go:121:35: G115: integer overflow conversion uint -> int32 (gosec) return keymanagerv1.HashAlgorithm(h) ^ pkg/server/plugin/keymanager/v1.go:158:25: G115: integer overflow conversion int -> int32 (gosec) SaltLength: int32(opts.SaltLength), ^ cmd/spire-server/cli/x509/mint.go:171:14: G115: integer overflow conversion int64 -> int32 (gosec) return int32((ttl + time.Second - 1) / time.Second) ^ cmd/spire-server/cli/x509/mint_test.go:336:27: G115: integer overflow conversion int64 -> int32 (gosec) assert.Equal(t, int32(tt.ttl/time.Second), req.Ttl) ^ pkg/server/api/svid/v1/service_test.go:551:15: G115: integer overflow conversion int64 -> int32 (gosec) Ttl: int32(tt.ttl / time.Second), ^ pkg/server/api/svid/v1/service_test.go:802:20: G115: integer overflow conversion int64 -> int32 (gosec) Ttl: int32(tt.ttl / time.Second), ^ pkg/server/api/status.go:16:17: G115: integer overflow conversion uint32 -> int32 (gosec) Code: int32(code), ^ pkg/server/api/status.go:24:17: G115: integer overflow conversion uint32 -> int32 (gosec) Code: int32(code), ^ cmd/spire-server/cli/bundle/common_test.go:283:15: G115: integer overflow conversion int -> int32 (gosec) Count: int32(len(f.bundles)), ^ pkg/common/containerinfo/extract.go:126:42: G115: integer overflow conversion int -> int32 (gosec) cgroups, err := cgroups.GetCgroups(int32(pid), dirFS(e.RootDir)) ^ cmd/spire-server/cli/jwt/mint.go:136:14: G115: integer overflow conversion int64 -> int32 (gosec) return int32((ttl + time.Second - 1) / time.Second) ^ cmd/spire-server/cli/jwt/mint_test.go:357:27: G115: integer overflow conversion int64 -> int32 (gosec) assert.Equal(t, int32(tt.ttl/time.Second), req.Ttl) ^ pkg/server/plugin/keymanager/base/keymanagerbase.go:173:27: G115: integer overflow conversion int32 -> uint (gosec) signerOpts = crypto.Hash(opts.HashAlgorithm) ^ pkg/server/plugin/keymanager/base/keymanagerbase.go:183:27: G115: integer overflow conversion int32 -> uint (gosec) Hash: crypto.Hash(opts.PssOptions.HashAlgorithm), ^ pkg/agent/plugin/keymanager/v1.go:121:35: G115: integer overflow conversion uint -> int32 (gosec) return keymanagerv1.HashAlgorithm(h) ^ pkg/agent/plugin/keymanager/v1.go:158:25: G115: integer overflow conversion int -> int32 (gosec) SaltLength: int32(opts.SaltLength), ^ pkg/server/api/bundle/v1/service_test.go:2482:23: G115: integer overflow conversion int32 -> uint32 (gosec) switch codes.Code(result.Status.Code) { ^ pkg/server/api/audit/audit.go:64:32: G115: integer overflow conversion int32 -> uint32 (gosec) err := status.Error(codes.Code(s.Code), s.Message) ^ pkg/common/selector/set_utils.go:72:44: G115: integer overflow conversion int -> uint64 (gosec) binaryString := strconv.FormatUint(uint64(i), 2) ^ test/tpmsimulator/simulator.go:274:9: G115: integer overflow conversion int -> uint16 (gosec) uint16(len(ekCert))) ^ pkg/agent/plugin/workloadattestor/v1.go:18:13: G115: integer overflow conversion int -> int32 (gosec) Pid: int32(pid), ^ cmd/spire-server/cli/entry/create.go:186:21: G115: integer overflow conversion int -> int32 (gosec) X509SvidTtl: int32(c.x509SVIDTTL), ^ cmd/spire-server/cli/entry/create.go:187:21: G115: integer overflow conversion int -> int32 (gosec) JwtSvidTtl: int32(c.jwtSVIDTTL), ^ cmd/spire-server/cli/entry/create.go:257:14: G115: integer overflow conversion int32 -> uint32 (gosec) codes.Code(r.Status.Code), ^ cmd/spire-server/cli/entry/delete.go:138:14: G115: integer overflow conversion int32 -> uint32 (gosec) codes.Code(result.Status.Code), ^ cmd/spire-server/cli/entry/update.go:179:21: G115: integer overflow conversion int -> int32 (gosec) X509SvidTtl: int32(c.x509SvidTTL), ^ cmd/spire-server/cli/entry/update.go:180:21: G115: integer overflow conversion int -> int32 (gosec) JwtSvidTtl: int32(c.jwtSvidTTL), ^ cmd/spire-server/cli/entry/update.go:243:14: G115: integer overflow conversion int32 -> uint32 (gosec) codes.Code(r.Status.Code), ^ pkg/agent/plugin/keymanager/base/keymanagerbase.go:173:27: G115: integer overflow conversion int32 -> uint (gosec) signerOpts = crypto.Hash(opts.HashAlgorithm) ^ pkg/agent/plugin/keymanager/base/keymanagerbase.go:183:27: G115: integer overflow conversion int32 -> uint (gosec) Hash: crypto.Hash(opts.PssOptions.HashAlgorithm), ^ pkg/server/endpoints/authorized_entryfetcher_attested_nodes.go:242:31: G115: integer overflow conversion uint -> int (gosec) if a.skippedNodeEvents != int(a.eventTracker.EventCount()) { ^ pkg/server/endpoints/authorized_entryfetcher_attested_nodes.go:243:28: G115: integer overflow conversion uint -> int (gosec) a.skippedNodeEvents = int(a.eventTracker.EventCount()) ^ pkg/server/endpoints/authorized_entryfetcher_registration_entries.go:254:32: G115: integer overflow conversion uint -> int (gosec) if a.skippedEntryEvents != int(a.eventTracker.EventCount()) { ^ pkg/server/endpoints/authorized_entryfetcher_registration_entries.go:255:29: G115: integer overflow conversion uint -> int (gosec) a.skippedEntryEvents = int(a.eventTracker.EventCount()) ^ pkg/server/endpoints/eventTracker.go:23:13: G115: integer overflow conversion int64 -> uint (gosec) return uint(1 + (trackTime-1)/pollTime) ^ pkg/agent/plugin/workloadattestor/systemd/systemd_posix.go:58:42: G115: integer overflow conversion int32 -> uint (gosec) uInfo, err := p.getUnitInfo(ctx, p, uint(req.Pid)) ^ pkg/agent/api/debug/v1/service.go:100:40: G115: integer overflow conversion int -> int32 (gosec) SvidsCount: int32(s.m.CountX509SVIDs()), ^ pkg/agent/api/debug/v1/service.go:101:40: G115: integer overflow conversion int -> int32 (gosec) CachedX509SvidsCount: int32(s.m.CountX509SVIDs()), ^ pkg/agent/api/debug/v1/service.go:102:40: G115: integer overflow conversion int -> int32 (gosec) CachedJwtSvidsCount: int32(s.m.CountJWTSVIDs()), ^ pkg/agent/api/debug/v1/service.go:103:40: G115: integer overflow conversion int -> int32 (gosec) CachedSvidstoreX509SvidsCount: int32(s.m.CountSVIDStoreX509SVIDs()), ^