Test-infra: Enable golangci-lint for static analysis

[bvwells]

What should be cleaned up or changed:

This is a follow up to PR #19444. golangci-lint can be used to identify various static analysis issues such as dead code, poor formatting, inefficient assignment and use of deprecated APIs and ultimately bugs.

I think that this tool would be useful in identifying issues in the code base.

Provide any links for context:

https://github.com/golangci/golangci-lint

[bvwells]

As promised @alvaroaleman, here is the suggestion to add golangci-lint. I'm happy to have a go at enabling it.

[alvaroaleman]

here is the suggestion to add golangci-lint. I'm happy to have a go at enabling it.

That would be great :) It requires adding a job in config/jobs/kubernetes/test-infra/test-infra-presubmits.yaml

[bvwells]

@alvaroaleman something like this?

  - name: pull-test-infra-verify-golangci-lint
    always_run: true
    run_if_changed: '\.go'
    decorate: true
    spec:
      containers:
        - image: golangci/golangci-lint:v1.29.0
          command:
            - make
          args:
            - golangci-lint
    annotations:
      testgrid-dashboards: presubmits-test-infra
      testgrid-tab-name: verify-golangci-lint

[alvaroaleman]

Yes, except for the run_if_changed. You also need to add a config for it so this actually passes

[bvwells]

Sounds good. I haven't used Bazel much, but was trying to workout how to add golangci-lint to the build chain. Does this need to be done as well?

[alvaroaleman]

@fejta is the one who would have the last word on that, I personally think its fine without and just adding a new job

[fejta]

Does bazelisk run @io_k8s_repo_infra//hack:verify-golangci-lint already work? https://github.com/kubernetes/repo-infra/blob/master/hack/BUILD.bazel#L139

It will be a lot easier if it integrates with the rest of the tooling

[bvwells]

I'll have a play and double check.

[bvwells]

I might be doing something wrong, but I get:

% bazelisk run @io_k8s_repo_infra//hack:verify-golangci-lint       
INFO: Analyzed target @io_k8s_repo_infra//hack:verify-golangci-lint (1 packages loaded, 483 targets configured).
INFO: Found 1 target...
ERROR: /private/var/tmp/_bazel_ben/fbc15622dfbf50d7ebb6ec32abc4e69b/external/com_github_golangci_golangci_lint/pkg/golinters/BUILD.bazel:3:1: GoCompilePkg external/com_github_golangci_golangci_lint/pkg/golinters/darwin_amd64_stripped/go_default_library%/github.com/golangci/golangci-lint/pkg/golinters.a failed (Exit 1) builder failed: error executing command bazel-out/host/bin/external/go_sdk/builder ... (remaining 1 argument(s) skipped)

Use --sandbox_debug to see verbose messages from the sandbox
compilepkg: error running subcommand: exit status 2
/private/var/tmp/_bazel_ben/fbc15622dfbf50d7ebb6ec32abc4e69b/sandbox/darwin-sandbox/272/execroot/io_k8s_test_infra/external/com_github_golangci_golangci_lint/pkg/golinters/godot.go:34:17: undefined: godot.Message
/private/var/tmp/_bazel_ben/fbc15622dfbf50d7ebb6ec32abc4e69b/sandbox/darwin-sandbox/272/execroot/io_k8s_test_infra/external/com_github_golangci_golangci_lint/pkg/golinters/gomodguard.go:47:36: unknown field 'k' in struct literal of type gomodguard.BlockedModule
/private/var/tmp/_bazel_ben/fbc15622dfbf50d7ebb6ec32abc4e69b/sandbox/darwin-sandbox/272/execroot/io_k8s_test_infra/external/com_github_golangci_golangci_lint/pkg/golinters/gomodguard.go:51:43: cannot use m (type gomodguard.BlockedModule) as type map[string]gomodguard.BlockedModule in append
Target @io_k8s_repo_infra//hack:verify-golangci-lint failed to build
Use --verbose_failures to see the command lines of failed build steps.
INFO: Elapsed time: 4.721s, Critical Path: 1.48s
INFO: 0 processes.
FAILED: Build did NOT complete successfully
FAILED: Build did NOT complete successfully

[fejta]

Hmm... What version of repo-infra are we using? What version of golangci-lint does that use? What version are you trying to use in this repo?

[bvwells]

Looks like test-infra v0.0.6

https://github.com/kubernetes/test-infra/blob/53bda1faf44783c1799b4b28edba254aba702509/load.bzl#L25

Golangci-lint v1.25.0
https://github.com/kubernetes/repo-infra/blob/ff5f05a1f1dd8d0488e132c7d185d6323b29dcd0/repos.bzl#L329

[alvaroaleman]

@fejta will the golangci-lint version be configurable? I would prefer not to be forced onto the same version a dependency uses

[bvwells]

Looks like this repo uses v1.27.0

https://github.com/kubernetes/test-infra/blob/53bda1faf44783c1799b4b28edba254aba702509/repos.bzl#L4266

Not sure how the whole build chain hangs together. I'm imagining that this is a conflict...

[fejta]

Has to do with conflicting versions getting declared in these two places:

https://github.com/kubernetes/test-infra/blob/53bda1faf44783c1799b4b28edba254aba702509/WORKSPACE#L26-L32

Can you try repeating L26-28 after the _repo_infra_go_repos()?

[fejta-bot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale