Skip to content

Conversation

@corhere
Copy link
Contributor

@corhere corhere commented Feb 27, 2023

- What I did

Prerequisite work to unblock upgrading to Go 1.19

- How I did it

Backported:

- How to verify it

- Description for the changelog

- A picture of a cute animal (not mandatory but encouraged)

Signed-off-by: Sebastiaan van Stijn <[email protected]>
(cherry picked from commit 8dc5334)
Signed-off-by: Cory Snider <[email protected]>
Signed-off-by: Sebastiaan van Stijn <[email protected]>
(cherry picked from commit bc1790c)
Signed-off-by: Cory Snider <[email protected]>
Signed-off-by: Sebastiaan van Stijn <[email protected]>
(cherry picked from commit e946bf0)
Signed-off-by: Cory Snider <[email protected]>
Signed-off-by: Sebastiaan van Stijn <[email protected]>
(cherry picked from commit 76b4735)
Signed-off-by: Cory Snider <[email protected]>
Signed-off-by: Sebastiaan van Stijn <[email protected]>
(cherry picked from commit d14b5bf)
Signed-off-by: Cory Snider <[email protected]>
Signed-off-by: Sebastiaan van Stijn <[email protected]>
(cherry picked from commit 43795ec)
Signed-off-by: Cory Snider <[email protected]>
Signed-off-by: Sebastiaan van Stijn <[email protected]>
(cherry picked from commit f28c063)
Signed-off-by: Cory Snider <[email protected]>
Signed-off-by: Sebastiaan van Stijn <[email protected]>
(cherry picked from commit 9bdeb09)
Signed-off-by: Cory Snider <[email protected]>
Signed-off-by: Sebastiaan van Stijn <[email protected]>
(cherry picked from commit f61aab5)
Signed-off-by: Cory Snider <[email protected]>
Signed-off-by: Sebastiaan van Stijn <[email protected]>
(cherry picked from commit 3b3a0b8)
Signed-off-by: Cory Snider <[email protected]>
Signed-off-by: Sebastiaan van Stijn <[email protected]>
(cherry picked from commit c558df7)
Signed-off-by: Cory Snider <[email protected]>
Signed-off-by: Sebastiaan van Stijn <[email protected]>
(cherry picked from commit d1f26de)
Signed-off-by: Cory Snider <[email protected]>
Signed-off-by: Sebastiaan van Stijn <[email protected]>
(cherry picked from commit d59330f)
Signed-off-by: Cory Snider <[email protected]>
Signed-off-by: Sebastiaan van Stijn <[email protected]>
(cherry picked from commit 78cb61c)
Signed-off-by: Cory Snider <[email protected]>
Signed-off-by: Sebastiaan van Stijn <[email protected]>
(cherry picked from commit e0299ff)
Signed-off-by: Cory Snider <[email protected]>
Signed-off-by: Sebastiaan van Stijn <[email protected]>
(cherry picked from commit b5dce3c)
Signed-off-by: Cory Snider <[email protected]>
Signed-off-by: Sebastiaan van Stijn <[email protected]>
(cherry picked from commit cca73bf)
Signed-off-by: Cory Snider <[email protected]>
Signed-off-by: Sebastiaan van Stijn <[email protected]>
(cherry picked from commit 3f7e7bf)
Signed-off-by: Cory Snider <[email protected]>
Signed-off-by: Sebastiaan van Stijn <[email protected]>
(cherry picked from commit b9f0340)
Signed-off-by: Cory Snider <[email protected]>
Signed-off-by: Sebastiaan van Stijn <[email protected]>
(cherry picked from commit 71575ab)
Signed-off-by: Cory Snider <[email protected]>
Signed-off-by: Sebastiaan van Stijn <[email protected]>
(cherry picked from commit cca80cd)
Signed-off-by: Cory Snider <[email protected]>
Signed-off-by: Sebastiaan van Stijn <[email protected]>
(cherry picked from commit 58cf16d)
Signed-off-by: Cory Snider <[email protected]>
Signed-off-by: Sebastiaan van Stijn <[email protected]>
(cherry picked from commit 1e54bca)
Signed-off-by: Cory Snider <[email protected]>
Signed-off-by: Sebastiaan van Stijn <[email protected]>
(cherry picked from commit 86db51e)
Signed-off-by: Cory Snider <[email protected]>
Signed-off-by: Sebastiaan van Stijn <[email protected]>
(cherry picked from commit 0e3197e)
Signed-off-by: Cory Snider <[email protected]>
Signed-off-by: Sebastiaan van Stijn <[email protected]>
(cherry picked from commit 39ace68)
Signed-off-by: Cory Snider <[email protected]>
Signed-off-by: Sebastiaan van Stijn <[email protected]>
(cherry picked from commit e89af84)
Signed-off-by: Cory Snider <[email protected]>
Signed-off-by: Sebastiaan van Stijn <[email protected]>
(cherry picked from commit 7491c5a)
Signed-off-by: Cory Snider <[email protected]>
Signed-off-by: Sebastiaan van Stijn <[email protected]>
(cherry picked from commit 85754c9)
Signed-off-by: Cory Snider <[email protected]>
Signed-off-by: Sebastiaan van Stijn <[email protected]>
(cherry picked from commit 38e6257)
Signed-off-by: Cory Snider <[email protected]>
Signed-off-by: Sebastiaan van Stijn <[email protected]>
(cherry picked from commit 6c06950)
Signed-off-by: Cory Snider <[email protected]>
Older versions of Go do not format these comments, so we can already
reformat them ahead of time to prevent gofmt linting failing once
we update to Go 1.19 or up.

Result of:

    gofmt -s -w $(find . -type f -name '*.go' | grep -v "/vendor/")

With some manual adjusting.

Signed-off-by: Sebastiaan van Stijn <[email protected]>
(cherry picked from commit 82427d1)
Signed-off-by: Cory Snider <[email protected]>
    cli/command/image/build/context.go:238:23: "400" can be replaced by http.StatusBadRequest (usestdlibvars)
        if resp.StatusCode < 400 {
                             ^
    cli/trust/trust.go:139:30: "GET" can be replaced by http.MethodGet (usestdlibvars)
        req, err := http.NewRequest("GET", endpointStr, nil)
                                    ^

Signed-off-by: Sebastiaan van Stijn <[email protected]>
(cherry picked from commit d3d9301)
Signed-off-by: Cory Snider <[email protected]>
    cli-plugins/manager/plugin.go:37:1: directive `//nolint:gocyclo` is unused for linter "gocyclo" (nolintlint)
    //nolint:gocyclo
    ^
    cli/command/image/formatter_history_test.go:189:2: directive `//nolint:lll` is unused for linter "lll" (nolintlint)
        //nolint:lll
        ^
    cli/command/service/list.go:113:1: directive `//nolint:gocyclo` is unused for linter "gocyclo" (nolintlint)
    //nolint:gocyclo
    ^
    cli/command/stack/swarm/deploy_composefile.go:178:1: directive `//nolint:gocyclo` is unused for linter "gocyclo" (nolintlint)
    //nolint:gocyclo
    ^

Signed-off-by: Sebastiaan van Stijn <[email protected]>
(cherry picked from commit 491407b)
Signed-off-by: Cory Snider <[email protected]>
    cli/command/manifest/inspect_test.go:9:2: ST1019: package "github.com/docker/cli/cli/manifest/types" is being imported more than once (stylecheck)
        "github.com/docker/cli/cli/manifest/types"
        ^
    cli/command/manifest/inspect_test.go:10:2: ST1019(related information): other import of "github.com/docker/cli/cli/manifest/types" (stylecheck)
        manifesttypes "github.com/docker/cli/cli/manifest/types"
        ^
    cli/command/stack/swarm/deploy_composefile.go:14:2: ST1019: package "github.com/docker/docker/client" is being imported more than once (stylecheck)
        apiclient "github.com/docker/docker/client"
        ^
    cli/command/stack/swarm/deploy_composefile.go:15:2: ST1019(related information): other import of "github.com/docker/docker/client" (stylecheck)
        dockerclient "github.com/docker/docker/client"
        ^

Signed-off-by: Sebastiaan van Stijn <[email protected]>
(cherry picked from commit ce01160)
Signed-off-by: Cory Snider <[email protected]>
    cli/command/cli_options_test.go:29:2: os.Setenv() can be replaced by `t.Setenv()` in TestWithContentTrustFromEnv (tenv)
        os.Setenv(envvar, "true")
        ^
    cli/command/cli_options_test.go:31:2: os.Setenv() can be replaced by `t.Setenv()` in TestWithContentTrustFromEnv (tenv)
        os.Setenv(envvar, "false")
        ^
    cli/command/cli_options_test.go:33:2: os.Setenv() can be replaced by `t.Setenv()` in TestWithContentTrustFromEnv (tenv)
        os.Setenv(envvar, "invalid")
        ^

Signed-off-by: Sebastiaan van Stijn <[email protected]>
(cherry picked from commit cef8581)
Signed-off-by: Cory Snider <[email protected]>
    opts/envfile_test.go:157:5: ST1017: don't use Yoda conditions (stylecheck)
        if 1 != len(variables) {
           ^

Signed-off-by: Sebastiaan van Stijn <[email protected]>
(cherry picked from commit b508b0f)
Signed-off-by: Cory Snider <[email protected]>
Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

overall looks good; some considerations;

  • perhaps we should consider skipping the commit that changes the error messages
  • for the io/ioutil changes; I'd also be fine to add a exclusion rule (I think GolangCi-lint has one for this); while it's marked deprecated, (AFAIK), it won't be removed until Go 2, so in reality, it's only to satisfy the linter, so if we prefer to keep a smaller diff, it's worth considering.

@corhere
Copy link
Contributor Author

corhere commented Feb 28, 2023

  • for the io/ioutil changes; I'd also be fine to add a exclusion rule (I think GolangCi-lint has one for this); while it's marked deprecated, (AFAIK), it won't be removed until Go 2, so in reality, it's only to satisfy the linter, so if we prefer to keep a smaller diff, it's worth considering.

I figure it'd be nice to have to reduce the delta between branches for backports, hopefully making it slightly easier on @neersighted and myself to maintain this branch over the coming months. But that's a trivial matter and the work to backport those changes is a sunk cost so I have no strong opinion one way or the other. I'll leave it up to you and the other reviewers to decide.

@neersighted
Copy link
Member

I don't have strong opinions; given the work is already done, it could possibly reduce conflicts in future merges, so I think it's fine to stay.

corhere and others added 4 commits March 1, 2023 10:21
…lecheck)

Changing the output of errors would be unexpected in a patch release.

Signed-off-by: Cory Snider <[email protected]>
Picking 2 seconds, although that's just a randomly picked timeout;
given that this is only for testing, it's not too important.

    e2e/plugin/basic/basic.go:25:12: G112: Potential Slowloris Attack because ReadHeaderTimeout is not configured in the http.Server (gosec)
        server := http.Server{
            Addr:    l.Addr().String(),
            Handler: http.NewServeMux(),
        }

Signed-off-by: Sebastiaan van Stijn <[email protected]>
(cherry picked from commit 9865420)
Signed-off-by: Cory Snider <[email protected]>
…tedassign)

    cli/command/container/opts.go:928:2: assigned to src, but reassigned without using the value (wastedassign)
        src := ""
        ^

Signed-off-by: Sebastiaan van Stijn <[email protected]>
(cherry picked from commit 3dfdaa6)
Signed-off-by: Cory Snider <[email protected]>
Remove the "deadcode", "structcheck", and "varcheck" linters, as they are
deprecated:

    WARN [runner] The linter 'deadcode' is deprecated (since v1.49.0) due to: The owner seems to have abandoned the linter.  Replaced by unused.
    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.

And ignore gosec G113, which only affects gp < 1.16.14. and go < 1.17.7

    opts/opts.go:398:13: G113: Potential uncontrolled memory consumption in Rat.SetString (CVE-2022-23772) (gosec)
        cpu, ok := new(big.Rat).SetString(value)
                   ^

Signed-off-by: Sebastiaan van Stijn <[email protected]>
(cherry picked from commit 0dd2c18)
Signed-off-by: Cory Snider <[email protected]>
@corhere corhere force-pushed the 20.10_backport_update_golangci_lint_v1.49.0 branch from 3092164 to 48151d4 Compare March 1, 2023 15:26
@thaJeztah
Copy link
Member

Thanks! Yes, I'm fine reducing the diff; the ioutil changes are well-understood, and should have no risk in itself. Let me get this one in 👍

Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants