-
Notifications
You must be signed in to change notification settings - Fork 3.8k
[release/1.6 backport] assorted linting, and golang update-related changes #8229
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
Merged
estesp
merged 23 commits into
containerd:release/1.6
from
thaJeztah:1.6_backport_golangci_lint_updates
Mar 8, 2023
Merged
[release/1.6 backport] assorted linting, and golang update-related changes #8229
estesp
merged 23 commits into
containerd:release/1.6
from
thaJeztah:1.6_backport_golangci_lint_updates
Mar 8, 2023
Conversation
This file contains hidden or 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
Signed-off-by: ningmingxiao <[email protected]> (cherry picked from commit 54e95e6) Signed-off-by: Sebastiaan van Stijn <[email protected]>
There's no specific need mentioned at the points it was added, and it makes the Windows-hosted test run setup slightly weird. Signed-off-by: Paul "TBBle" Hampson <[email protected]> (cherry picked from commit 5b78a9a) Signed-off-by: Sebastiaan van Stijn <[email protected]>
The directory created by `T.TempDir` is automatically removed when the test and all its subtests complete. Reference: https://pkg.go.dev/testing#T.TempDir Signed-off-by: Eng Zer Jun <[email protected]> (cherry picked from commit 18ec276) Signed-off-by: Sebastiaan van Stijn <[email protected]>
Signed-off-by: Eng Zer Jun <[email protected]> (cherry picked from commit 52d307a) Signed-off-by: Sebastiaan van Stijn <[email protected]>
`gosec` linter is able to identify issues described in containerd#6584 e.g. $ git revert 54e95e6 [gosec dfc8ca1ec] Revert "fix Implicit memory aliasing in for loop" 2 files changed, 2 deletions(-) $ make check + proto-fmt + check GOGC=75 golangci-lint run containerstore.go:192:54: G601: Implicit memory aliasing in for loop. (gosec) containers = append(containers, containerFromProto(&container)) ^ image_store.go:132:42: G601: Implicit memory aliasing in for loop. (gosec) images = append(images, imageFromProto(&image)) ^ make: *** [check] Error 1 I also disabled following two settings which prevent the linter to show a complete list of issues. * max-issues-per-linter (default 50) * max-same-issues (default 3) Furthermore enabling gosec revealed many other issues. For now I blacklisted the ones except G601. Will create separate tasks to address them one by one moving next. Signed-off-by: Henry Wang <[email protected]> (cherry picked from commit b8bf504) Signed-off-by: Sebastiaan van Stijn <[email protected]>
This change disables Windows Defender real-time monitoring on the test workers, and increases the test timeout to 20 minutes (default is 10). The Windows Defender real time monitoring feature scans any newly created files for malitious contents. This takes up a lot of CPU when expanding image archives, which contain lots of files. The CI has been timing out due to the fact that tests take longer than 10 minutes. This change should address that issue. Signed-off-by: Gabriel Adrian Samfira <[email protected]> (cherry picked from commit c7bdcdf) Signed-off-by: Sebastiaan van Stijn <[email protected]>
Signed-off-by: Maksym Pavlenko <[email protected]> (cherry picked from commit 2d59a39) Signed-off-by: Sebastiaan van Stijn <[email protected]>
Signed-off-by: Maksym Pavlenko <[email protected]> (cherry picked from commit 62c846b) Signed-off-by: Sebastiaan van Stijn <[email protected]>
Signed-off-by: Kang.Zhang <[email protected]> (cherry picked from commit fceab7f) Signed-off-by: Sebastiaan van Stijn <[email protected]>
Signed-off-by: Maksym Pavlenko <[email protected]> (cherry picked from commit ea66130) Signed-off-by: Sebastiaan van Stijn <[email protected]>
Signed-off-by: Maksym Pavlenko <[email protected]> (cherry picked from commit ca3b9b5) Signed-off-by: Sebastiaan van Stijn <[email protected]>
Signed-off-by: Maksym Pavlenko <[email protected]> (cherry picked from commit 68bae25) Signed-off-by: Sebastiaan van Stijn <[email protected]>
It is follow-up of containerd#7254. This commit will increase ReadHeaderTimeout from 3s to 30m, which prevent from unexpected timeout when the node is running with high-load. 30 Minutes is longer enough to get close to before what containerd#7254 changes. And ideally, we should allow user to configure the streaming server if the users want this feature. Signed-off-by: Wei Fu <[email protected]> (cherry picked from commit 460b053) Signed-off-by: Sebastiaan van Stijn <[email protected]>
Signed-off-by: Abirdcfly Fu <[email protected]> (cherry picked from commit dcfaa30) Signed-off-by: Sebastiaan van Stijn <[email protected]>
GOGC=75 golangci-lint run
services/server/server.go:320:27: G114: Use of net/http serve function that has no support for setting timeouts (gosec)
return trapClosedConnErr(http.Serve(l, m))
^
services/server/server.go:340:27: G114: Use of net/http serve function that has no support for setting timeouts (gosec)
return trapClosedConnErr(http.Serve(l, m))
^
cmd/containerd-stress/main.go:238:13: G114: Use of net/http serve function that has no support for setting timeouts (gosec)
if err := http.ListenAndServe(c.Metrics, metrics.Handler()); err != nil {
^
Signed-off-by: Sebastiaan van Stijn <[email protected]>
(cherry picked from commit 3ebeb6d)
Signed-off-by: Sebastiaan van Stijn <[email protected]>
THis makes it easier to find which linters are enabled when going through the list. Signed-off-by: Sebastiaan van Stijn <[email protected]> (cherry picked from commit 0eaace3) Signed-off-by: Sebastiaan van Stijn <[email protected]>
…fy nolint This `//nolint` was added in containerd/cri@f5c7ac9 to suppress warnings about the `NameToCertificate` function being deprecated: // Deprecated: NameToCertificate only allows associating a single certificate // with a given name. Leave that field nil to let the library select the first // compatible chain from Certificates. Looking at that, it was deprecated in Go 1.14 through golang/go@eb93c68 (https://go-review.googlesource.com/c/go/+/205059), which describes: crypto/tls: select only compatible chains from Certificates Now that we have a full implementation of the logic to check certificate compatibility, we can let applications just list multiple chains in Certificates (for example, an RSA and an ECDSA one) and choose the most appropriate automatically. NameToCertificate only maps each name to one chain, so simply deprecate it, and while at it simplify its implementation by not stripping trailing dots from the SNI (which is specified not to have any, see RFC 6066, Section 3) and by not supporting multi-level wildcards, which are not a thing in the WebPKI (and in crypto/x509). We should at least have a comment describing why we are ignoring this, but preferably review whether we should still use it. Signed-off-by: Sebastiaan van Stijn <[email protected]> (cherry picked from commit d215725) Signed-off-by: Sebastiaan van Stijn <[email protected]>
- fix "nolint" comments to be in the correct format (`//nolint:<linters>[,<linter>` no leading space, required colon (`:`) and linters. - remove "nolint" comments for errcheck, which is disabled in our config. - remove "nolint" comments that were no longer needed (nolintlint). - where known, add a comment describing why a "nolint" was applied. Signed-off-by: Sebastiaan van Stijn <[email protected]> (cherry picked from commit 29c7fc9) Signed-off-by: Sebastiaan van Stijn <[email protected]>
Some changes to account for 91800c4 and 894af07 runtime/v2/shim/shim.go:178:18: directive `//nolint:staticcheck // Ignore SA4023 as some platforms always return error` is unused for linter "staticcheck" (nolintlint) if err != nil { //nolint:staticcheck // Ignore SA4023 as some platforms always return error ^ runtime/v2/shim/shim.go:264:18: directive `//nolint:staticcheck // Ignore SA4023 as some platforms always return error` is unused for linter "staticcheck" (nolintlint) if err != nil { //nolint:staticcheck // Ignore SA4023 as some platforms always return error ^ runtime/v2/shim/shim.go:269:39: directive `//nolint:staticcheck // Ignore SA4023 as some platforms always return error` is unused for linter "staticcheck" (nolintlint) if err := subreaper(); err != nil { //nolint:staticcheck // Ignore SA4023 as some platforms always return error ^ runtime/v2/shim/shim.go:448:67: directive `//nolint:staticcheck // Ignore SA4023 as some platforms always return error` is unused for linter "staticcheck" (nolintlint) if err := serve(ctx, server, signals, sd.Shutdown); err != nil { //nolint:staticcheck // Ignore SA4023 as some platforms always return error ^ runtime/v2/shim/shim.go:480:18: directive `//nolint:staticcheck // Ignore SA4023 as some platforms always return error` is unused for linter "staticcheck" (nolintlint) if err != nil { //nolint:staticcheck // Ignore SA4023 as some platforms always return error ^ integration/main_test.go:446:31: directive `//nolint:unused` is unused for linter "unused" (nolintlint) func KillPid(pid int) error { //nolint:unused Signed-off-by: Sebastiaan van Stijn <[email protected]>
Remove nolint-comments that weren't hit by linters, and remove the "structcheck"
and "varcheck" linters, as they have been deprecated:
WARN [runner] The linter 'structcheck' is deprecated (since v1.49.0) due to: The owner seems to have abandoned the linter. Replaced by unused.
WARN [runner] The linter 'varcheck' is deprecated (since v1.49.0) due to: The owner seems to have abandoned the linter. Replaced by unused.
WARN [linters context] structcheck is disabled because of generics. You can track the evolution of the generics support by following the golangci/golangci-lint#2649.
Signed-off-by: Sebastiaan van Stijn <[email protected]>
(cherry picked from commit f9c80be)
Signed-off-by: Sebastiaan van Stijn <[email protected]>
Also remove "nolint" comments for deadcode, which is deprecated, and removed from the defaults. Signed-off-by: Sebastiaan van Stijn <[email protected]> (cherry picked from commit 8b5df7d) Signed-off-by: Sebastiaan van Stijn <[email protected]>
|
Skipping CI for Draft Pull Request. |
This comment was marked as resolved.
This comment was marked as resolved.
Signed-off-by: Luca Comellini <[email protected]> (cherry picked from commit c5fff10) Signed-off-by: Sebastiaan van Stijn <[email protected]>
Signed-off-by: Maksym Pavlenko <[email protected]> (cherry picked from commit 06bfcd6) Signed-off-by: Sebastiaan van Stijn <[email protected]>
e354c56 to
9cbea6f
Compare
Member
Author
|
Linters are happy; Windows failure looks unrelated (but other CI is still running) |
Member
Author
|
Perhaps someone's able to give Windows CI a kick to run it again 😓 |
AkihiroSuda
approved these changes
Mar 8, 2023
Member
Author
estesp
approved these changes
Mar 8, 2023
Mengkzhaoyun
pushed a commit
to open-beagle/containerd
that referenced
this pull request
Apr 10, 2023
containerd 1.6.20 Welcome to the v1.6.20 release of containerd! The twentieth patch release for containerd 1.6 contains various fixes and updates. * **Disable looking up usernames and groupnames on host** ([#8230](containerd/containerd#8230)) * **Add support for Windows ArgsEscaped images** ([#8273](containerd/containerd#8273)) * **Update hcsshim to v0.9.8** ([#8274](containerd/containerd#8274)) * **Fix debug flag in shim** ([#8288](containerd/containerd#8288)) * **Add `WithReadonlyTempMount` to support readonly temporary mounts** ([#8299](containerd/containerd#8299)) * **Update ttrpc to fix file descriptor leak** ([#8308](containerd/containerd#8308)) * **Update runc binary to v1.1.5** ([#8324](containerd/containerd#8324)) * **Update image config to support ArgsEscaped** ([#8306](containerd/containerd#8306)) See the changelog for complete list of changes Please try out the release binaries and report any issues at https://github.com/containerd/containerd/issues. * Sebastiaan van Stijn * Derek McGowan * Maksym Pavlenko * Akihiro Suda * Phil Estes * Eng Zer Jun * Justin Terry * Kazuyoshi Kato * Wei Fu * Abirdcfly * Gabriel Adrian Samfira * Henry Wang * Kang.Zhang * Kirtana Ashok * Laura Brehm * Luca Comellini * Paul "TBBle" Hampson * liyuxuan.darfux * ningmingxiao * wanglei <details><summary>48 commits</summary> <p> * [release/1.6] Prepare release notes for v1.6.20 ([#8310](containerd/containerd#8310)) * [`a039a2b9c`](containerd/containerd@a039a2b) Prepare release notes for v1.6.20 * [release/1.6]Updates oci image config to support upstream ArgsEscaped ([#8306](containerd/containerd#8306)) * [`5dd94a7e6`](containerd/containerd@5dd94a7) Updates oci image config to support upstream ArgsEscaped * [release/1.6] update runc binary to v1.1.5 ([#8324](containerd/containerd#8324)) * [`59fa6b191`](containerd/containerd@59fa6b1) update runc binary to v1.1.5 * [`0c0aad93e`](containerd/containerd@0c0aad9) go.mod: github.com/opencontainers/runc v1.1.5 * [release/1.6] Update ttrpc to v1.1.1 ([#8308](containerd/containerd#8308)) * [`50a6be0b4`](containerd/containerd@50a6be0) Update ttrpc to v1.1.1 * [release/1.6 backport] Add `WithReadonlyTempMount` to create readonly temporary mounts ([#8299](containerd/containerd#8299)) * [`8cead6594`](containerd/containerd@8cead65) Add `WithReadonlyTempMount` to create readonly temporary mounts * [release/1.6] Adds support for Windows ArgsEscaped images ([#8273](containerd/containerd#8273)) * [`f0dc0297d`](containerd/containerd@f0dc029) Adds support for Windows ArgsEscaped images * [release/1.6]go.mod: Bump hcsshim tag to v0.9.8 ([#8274](containerd/containerd#8274)) * [`5981a24e2`](containerd/containerd@5981a24) Update hcsshim tag to v0.9.8 * [1.6] shim: fix debug flag not working ([#8288](containerd/containerd#8288)) * [`28f1e32e3`](containerd/containerd@28f1e32) shim: fix debug flag not working * [release/1.6] cherry-pick: Update go-restful to v3 ([#8271](containerd/containerd#8271)) * [`5a8ea75df`](containerd/containerd@5a8ea75) Update go-restful to v3 * [`59bdc1d5a`](containerd/containerd@59bdc1d) go.mod: update to github.com/emicklei/go-restful/v3 v3.7.3 * [release/1.6] Go 1.19.7 ([#8238](containerd/containerd#8238)) * [`86e0bd9e3`](containerd/containerd@86e0bd9) Go 1.19.7 * [release/1.6 backport] archive: disable looking up usernames and groupnames on the host ([#8230](containerd/containerd#8230)) * [`063ad2f19`](containerd/containerd@063ad2f) archive: disable looking up usernames and groupnames on the host * [release/1.6 backport] assorted linting, and golang update-related changes ([#8229](containerd/containerd#8229)) * [`9cbea6fe7`](containerd/containerd@9cbea6f) Enable dupword linter * [`c73f1abff`](containerd/containerd@c73f1ab) Bump golangci-lint to v1.50.1 * [`f198f7724`](containerd/containerd@f198f77) update golangci-lint to v1.49.0 * [`e6179af1e`](containerd/containerd@e6179af) remove unneeded nolint-comments (nolintlint), disable deprecated linters * [`77160e6b5`](containerd/containerd@77160e6) [release/1.6] adjust some nolint comments * [`95655f4ce`](containerd/containerd@95655f4) clean-up "nolint" comments, remove unused ones * [`9f0617ecc`](containerd/containerd@9f0617e) pkg/cri/(server|sbserver): criService.getTLSConfig() add TODO to verify nolint * [`e66397d83`](containerd/containerd@e66397d) golangci-lint: sort linters in config file * [`682a567e9`](containerd/containerd@682a567) linting: address gosec G112/G114 * [`627f563e6`](containerd/containerd@627f563) chore: remove duplicate word in comments * [`efb88a8bb`](containerd/containerd@efb88a8) pkg/cri/streaming: increase ReadHeaderTimeout * [`45f055df6`](containerd/containerd@45f055d) Update protobuf definitions * [`584707524`](containerd/containerd@5847075) Run gofmt 1.19 * [`f33e38572`](containerd/containerd@f33e385) Switch to Go 1.19 * [`fc10cd23a`](containerd/containerd@fc10cd2) remove duplicate * [`7cbb9e746`](containerd/containerd@7cbb9e7) Update linters to use t.Setenv * [`4347a3265`](containerd/containerd@4347a32) Use t.Setenv instead of os.Setenv * [`10357eab5`](containerd/containerd@10357ea) Address some timeout issues in the Windows CI * [`977ce8ef5`](containerd/containerd@977ce8e) Enable gosec linter for golangci-lint * [`c23945c5f`](containerd/containerd@c23945c) test: remove redundant `mountPoint` * [`588ed91d3`](containerd/containerd@588ed91) test: use `T.TempDir` to create temporary test directory * [`c2ed63c86`](containerd/containerd@c2ed63c) Remove hardcoded /tmp in tempfile paths * [`7e382c516`](containerd/containerd@7e382c5) fix Implicit memory aliasing in for loop </p> </details> <details><summary>2 commits</summary> <p> * [release/1.1] server: Fix connection leak when receiving ECONNRESET ([#136](containerd/ttrpc#136)) * [`8977f59`](containerd/ttrpc@8977f59) server: Fix connection leak when receiving ECONNRESET </p> </details> * **github.com/Microsoft/hcsshim** v0.9.7 -> v0.9.8 * **github.com/containerd/ttrpc** v1.1.0 -> v1.1.1 * **github.com/emicklei/go-restful/v3** v3.7.3 **_new_** * **github.com/opencontainers/image-spec** c5a74bcca799 -> 3a7f492d3f1b * **github.com/opencontainers/runc** v1.1.2 -> v1.1.5 Previous release can be found at [v1.6.19](https://github.com/containerd/containerd/releases/tag/v1.6.19)
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.
backporting various changes where main started to diverge
backports of:
T.TempDirto create temporary test directory #6681TestAdditionalGids()(which was modified in containerd v1.6.18 for the security fix)Remove hacks around contrib/fuzz #7087Copy fuzzers from github.com/cncf/cncf-fuzzing #7123