upgrade golangci-lint

[mweibel]

What type of PR is this?
/kind cleanup

What this PR does / why we need it:
I couldn't run golangci-lint locally due to memory/cpu spikes. In version 1.51 they improved that a lot (by also building with go 1.20). This upgrades the linter.

Special notes for your reviewer:
I had to rename the build tag to capztools because when running with tools build tags there's an issue with another package:

$ go install -tags tools github.com/golangci/golangci-lint/cmd/[email protected]
../../../go/pkg/mod/github.com/ryancurrah/[email protected]/tools.go:5:8: import "github.com/t-yuki/gocover-cobertura" is a program, not an importable package

I'm not 100% certain why since my Go module know-how is still not that great unfortunately. I suspect that module also uses tools build tag for something and is imported by golangci-lint.

Additionally, there are a couple of linting issues after upgrading:

azure/regional_baseuri.go:27:6: type `aliasAuth` is unused (unused)
type aliasAuth = Authorizer
     ^
azure/services/virtualnetworks/virtualnetworks_test.go:46:2: var `managedVnet` is unused (unused)
        managedVnet = network.VirtualNetwork{
        ^
azure/services/privatedns/privatedns_test.go:92:2: var `unmanagedTags` is unused (unused)
        unmanagedTags = resources.TagsResource{
        ^

I cleaned them up. aliasAuth is a false positive unfortunately.

Release note:

NONE

[k8s-ci-robot]

Hi @mweibel. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[mboersma]

/ok-to-test

[mweibel]

/test pull-cluster-api-provider-azure-e2e

[mweibel]

/retest

[mboersma]

/lgtm
/assign @CecileRobertMichon @jackfrancis

[k8s-ci-robot]

LGTM label has been added.

Git tree hash: 20779c14c7c925faabbf7298acf70b666ccbfc3e

[CecileRobertMichon]

/lgtm
/approve

[mweibel]

/retest

[mweibel]

Required unknown is a bit weird - is that not defined for pull-cluster-api-provider-azure-e2e?

[mweibel]

hmm is pull-cluster-api-provider-azure-e2e flaky?

[nawazkh]

I am a little confused, Why can't we use the below here?

Suggested change

	type aliasAuth = Authorizer //nolint:unused // This is a false positive since aliasAuth is used right below.
	type aliasAuth Authorizer

[mweibel]

The relevant check reports this as unused even though it’s used (false positive). I briefly searched for issues related to it in the staticcheck repo and found a few with the mention that they‘ll work on it.

[nawazkh]

Sorry, I did not frame the question right. I was wondering why is type aliasAuth = Authorizer and not type aliasAuth Authorizer. Looks like it doesn't really matter since aliasAuth also turns out to be an interface type in both the scenarios.

[mweibel]

that's a good question. I tried adjusting the code according to your suggestion and lint doesn't complain anymore (of course also removed the nolint comment) and tests seem to pass too.

I wonder what the original intention was of adding that type alias though? The difference is explained quite well on this page but I wonder why it's needed at anyway? Why not just work with Authorizer itself?

[CecileRobertMichon]

should we try just removing it then? possibly it was needed at some point when it was added and that reason no longer exists.

[mweibel]

I tried to remove it and that led me to a way too long investigation why it doesn't work.
Summary: the type alias is needed because type Authorizer also has a method named Authorizer() (same name) and the compiler & linter gets confused when type baseURIAdapter wants to embed the interface Authorizer

more or less minimum reproducing code: https://go.dev/play/p/nwjcUf9x4DI
how it would work without type alias (rename Authorizer method to Authorizor e.g.): https://go.dev/play/p/A5kCr0FrBPE

I think we can just settle to keep a type alias though. I'll use the other form @nawazkh proposed even though it has a slightly different semantic meaning.

[mweibel]

code is updated & commented. PTAL @CecileRobertMichon @nawazkh

[mweibel]

/retest

[CecileRobertMichon]

Failed with the kube-proxy image pull error again, that should be fixed now...

/retest

[mweibel]

FYI @mboersma @CecileRobertMichon tests passed here but LGTM has been removed since I rebased on top of latest main.

[mweibel]

/retest

[CecileRobertMichon]

/lgtm
/retest

thanks for the investigation @mweibel!

[k8s-ci-robot]

LGTM label has been added.

Git tree hash: d51526f441d985d441b7284fba9f1221c3db6d9d

[mboersma]

/lgtm
/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: CecileRobertMichon, mboersma

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [CecileRobertMichon,mboersma]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[CecileRobertMichon]

ran into #3227 again...

/retest

[CecileRobertMichon]

going to cherry-pick this because kpromo new version runs into the tools issue... if the bot lets me

/cherry-pick release-1.8 release-1.7

[k8s-infra-cherrypick-robot]

@CecileRobertMichon: cannot checkout release-1.8 release-1.7: error checking out release-1.8 release-1.7: exit status 1. output: error: pathspec 'release-1.8 release-1.7' did not match any file(s) known to git

In response to this:

going to cherry-pick this because kpromo new version runs into the tools issue... if the bot lets me

/cherry-pick release-1.8 release-1.7

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[CecileRobertMichon]

ok bot

/cherry-pick release-1.8

[k8s-infra-cherrypick-robot]

@CecileRobertMichon: new pull request created: #3321

In response to this:

ok bot

/cherry-pick release-1.8
/cherry-pick release-1.7

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[CecileRobertMichon]

/cherry-pick release-1.7

[k8s-infra-cherrypick-robot]

@CecileRobertMichon: #3242 failed to apply on top of branch "release-1.7":

Applying: upgrade golangci-lint
Using index info to reconstruct a base tree...
M	Makefile
M	azure/services/privatedns/privatedns_test.go
M	azure/services/virtualnetworks/virtualnetworks_test.go
Falling back to patching base and 3-way merge...
Auto-merging azure/services/virtualnetworks/virtualnetworks_test.go
CONFLICT (content): Merge conflict in azure/services/virtualnetworks/virtualnetworks_test.go
Auto-merging azure/services/privatedns/privatedns_test.go
CONFLICT (content): Merge conflict in azure/services/privatedns/privatedns_test.go
Auto-merging Makefile
CONFLICT (content): Merge conflict in Makefile
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 upgrade golangci-lint
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

In response to this:

/cherry-pick release-1.7

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.