this commit scaffolds golangci-lint for capz

Adds
- pr-golangci-lint job
- exports GO_VERSION in the makefile that is used in other make targets
- updates .golangci.yml with CAPI changes, and also rebases CAPZ additions to it.
- update .golangci.yml
- ignore package comments across CAPZ

.github/workflows/pr-golangci-lint.yaml

@@ -0,0 +1,49 @@
+name: PR golangci-lint
+
+on:
+  pull_request:
+    types: [opened, edited, synchronize, reopened]
+
+# Remove all permissions from GITHUB_TOKEN except metadata.
+permissions: {}
+
+jobs:
+  golangci:
+    name: lint
+    runs-on: ubuntu-latest
+    strategy:
+      fail-fast: false
+      matrix:
+        working-directory:
+          - ""
+          - test
+    steps:
+      - uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332 # tag=v4.1.7
+
+      - name: Calculate go version
+        id: vars
+        run: echo "go_version=$(make go-version)" >> $GITHUB_OUTPUT
+
+      - name: Set up Go
+        uses: actions/setup-go@0a12ed9d6a96ab950c8f026ed9f722fe0da7ef32 # tag=v5.0.2
+        with:
+          go-version: ${{ steps.vars.outputs.go_version }}
+
+      - name: Cache golangci-lint binary
+        id: golangci-cache
+        uses: actions/cache@v3
+        with:
+          # Cache location used by golangci-lint-action
+          path: ~/.cache/golangci-lint
+          # Change this when updating golangci-lint version
+          key: golangci-lint-v1.60.2
+          restore-keys: |
+            golangci-lint-
+
+      - name: golangci-lint
+        uses: golangci/golangci-lint-action@aaa42aa0628b4ae2578232a66b541047968fac86 # tag=v6.1.0
+        with:
+          # update the version when updating golangci-lint
+          version: v1.60.2
+          args: --out-format=colored-line-number
+          working-directory: ${{matrix.working-directory}}

.golangci.yml

@@ -1,64 +1,98 @@
 run:
-  deadline: 10m
-  skip-files:
-  - 'zz_generated\.(\w*)\.go$'
+  timeout: 10m
   build-tags:
     - e2e
+  allow-parallel-runners: true
+
 linters:
   disable-all: true
   enable:
-    - asasalint
-    - asciicheck
-    - bidichk
-    - bodyclose
-    - containedctx
-    - decorder
-    - dogsled
-    - dupword
-    - durationcheck
-    - errcheck
-    - errchkjson
-    - errorlint
-    - execinquery
-    - exportloopref
-    - gci
-    - ginkgolinter
-    - goconst
-    - gocritic
-    - gocyclo
-    - godot
-    - gofmt
-    - goimports
-    - goprintffuncname
-    - gosec
-    - gosimple
-    - govet
-    - importas
-    - ineffassign
-    - loggercheck
-    - misspell
-    - nakedret
-    - nilerr
-    - noctx
-    - nolintlint
-    - nosprintfhostport
-    - predeclared
-    - reassign
-    - revive
-    - rowserrcheck
-    - staticcheck
-    - stylecheck
-    - thelper
-    - typecheck
-    - unconvert
-    - unparam
-    - unused
-    - usestdlibvars
-    - whitespace
-  # Run with --fast=false for more extensive checks
-  fast: true
+    - asasalint # warns about passing []any to func(...any) without expanding it
+    - asciicheck # non ascii symbols
+    - bidichk # dangerous unicode sequences
+    - bodyclose # unclosed http bodies
+    - containedctx # context.Context nested in a struct
+    - copyloopvar # copying loop variables
+    - dogsled # too many blank identifiers in assignments
+    - dupword # duplicate words
+    - durationcheck # multiplying two durations
+    - errcheck # unchecked errors
+    - errchkjson # invalid types passed to json encoder
+    - gci # ensures imports are organized
+    - ginkgolinter # ginkgo and gomega
+    - goconst # strings that can be replaced by constants
+    - gocritic # bugs, performance, style (we could add custom ones to this one)
+    - godot # checks that comments end in a period
+    - gofmt # warns about incorrect use of fmt functions
+    - goimports # import formatting
+    - goprintffuncname # printft-like functions should be named with f at the end
+    - gosec # potential security problems
+    - gosimple # simplify code
+    - govet # basically 'go vet'
+    - importas # consistent import aliases
+    - ineffassign # ineffectual assignments
+    - intrange # suggest using integer range in for loops
+    - loggercheck # check for even key/value pairs in logger calls
+    - misspell # spelling
+    - nakedret # naked returns (named return parameters and an empty return)
+    - nilerr # returning nil after checking err is not nil
+    - noctx # http requests without context.Context
+    - nolintlint # badly formatted nolint directives
+    - nosprintfhostport # using sprintf to construct host:port in a URL
+    - prealloc # suggest preallocating slices
+    - predeclared # shadowing predeclared identifiers
+    - revive # better version of golint
+    - staticcheck # some of staticcheck's rules
+    - stylecheck # another replacement for golint
+    - tenv # using os.Setenv instead of t.Setenv in tests
+    - thelper # test helpers not starting with t.Helper()
+    - unconvert # unnecessary type conversions
+    - unparam # unused function parameters
+    - unused # unused constants, variables,functions, types
+    - usestdlibvars # using variables/constants from the standard library
+    - whitespace # unnecessary newlines
 
 linters-settings:
+  gosec:
+    excludes:
+      - G307 # Deferring unsafe method "Close" on type "\*os.File"
+      - G108 # Profiling endpoint is automatically exposed on /debug/pprof
+      - G115 # integer overflow conversion int -> int32
+  gci:
+    sections:
+      - standard # Standard section: captures all standard packages.
+      - default # Default section: contains all imports that could not be matched to another section type.
+      - prefix(sigs.k8s.io/cluster-api-provider-azure) # Custom section: groups all imports with the specified Prefix.
+    custom-order: true
+  ginkgolinter:
+    forbid-focus-container: true
+  goconst:
+    ignore-tests: true
+  godot:
+    #   declarations - for top level declaration comments (default);
+    #   toplevel     - for top level comments;
+    #   all          - for all comments.
+    scope: toplevel
+    exclude:
+      - '^ \+.*'
+      - '^ ANCHOR.*'
+  gocritic:
+    enabled-tags:
+      - "experimental"
+    disabled-checks:
+      - appendAssign
+      - dupImport # https://github.com/go-critic/go-critic/issues/845
+      - evalOrder
+      - ifElseChain
+      - octalLiteral
+      - regexpSimplify
+      - sloppyReassign
+      - truncateCmp
+      - typeDefFirst
+      - unnamedResult
+      - unnecessaryDefer
+      - whyNoLint
+      - wrapperFunc
   importas:
     no-unaliased: true
     alias:
@@ -102,34 +136,49 @@ linters-settings:
       # Deprecated
       - pkg: github.com/Azure/go-autorest/autorest/to
         alias: deprecated-use-k8s.io-utils-pointer
-  gocritic:
-    enabled-tags:
-      - "experimental"
-  godot:
-    #   declarations - for top level declaration comments (default);
-    #   toplevel     - for top level comments;
-    #   all          - for all comments.
-    scope: toplevel
-    exclude:
-    - '^ \+.*'
-    - '^ ANCHOR.*'
-  gosec:
-    excludes:
-    - G307 # Deferring unsafe method "Close" on type "\*os.File"
-    - G108 # Profiling endpoint is automatically exposed on /debug/pprof
+  nolintlint:
+    allow-unused: false
+    require-specific: true
   revive:
     rules:
+      # The following rules are recommended https://github.com/mgechev/revive#recommended-configuration
+      - name: blank-imports
+      - name: context-as-argument
+      - name: context-keys-type
+      - name: dot-imports
+      - name: error-return
+      - name: error-strings
+      - name: error-naming
+      - name: if-return
+      - name: increment-decrement
+      - name: var-naming
+      - name: var-declaration
+      - name: package-comments
+      - name: range
+      - name: receiver-naming
+      - name: time-naming
+      - name: unexported-return
+      - name: indent-error-flow
+      - name: errorf
+      - name: empty-block
+      - name: superfluous-else
+      - name: unused-parameter
+      - name: unreachable-code
+      - name: redefines-builtin-id
+      #
+      # Rules in addition to the recommended configuration above.
+      #
+      - name: bool-literal-in-expr
+      - name: constant-logical-expr
       - name: exported
         arguments:
           - disableStutteringCheck
-  staticcheck:
-    go: "1.22"
-  stylecheck:
-    go: "1.22"
   unused:
     go: "1.22"
 
 issues:
+  exclude-files:
+    - 'zz_generated\.(\w*)\.go$'
   exclude-rules:
     - path: '(\w*)conversion.go'
       text: "use underscores in Go names|receiver name (.+) should be consistent|methods on the same type should have the same receiver name"
@@ -145,8 +194,114 @@ issues:
       text: exported (.+) should have comment( \(or a comment on this block\))? or be unexported
     - source: \"github.com/onsi/(ginkgo/v2|gomega)\"
       text: "should not use dot imports"
-  include:
-  - EXC0012  # revive: check for comments
-  - EXC0014  # revive: check for comments
+    # Exclude some packages or code to require comments, for example test code, or fake clients.
+    - linters:
+      - revive
+      text: exported (method|function|type|const) (.+) should have comment or be unexported
+      source: (func|type).*Fake.*
+    - linters:
+      - revive
+      text: exported (method|function|type|const) (.+) should have comment or be unexported
+      path: fake_\.go
+    # Dot imports for gomega and ginkgo are allowed
+    # within test files and test utils.
+    - linters:
+      - revive
+      - stylecheck
+      path: _test\.go
+      text: should not use dot imports
+    - linters:
+      - revive
+      # Checking if an error is nil to just after return the error or nil is redundant
+      text: "if-return: redundant if ...; err != nil check, just return error instead"
+      # Ignoring stylistic checks for generated code
+      path: .*(api|types|test)\/.*\/conversion.*\.go$
+    - linters:
+      - revive
+      # Exported function and methods should have comments. This warns on undocumented exported functions and methods.
+      text: exported (method|function|type|const) (.+) should have comment or be unexported
+      # Ignoring stylistic checks for generated code
+      path: .*(api|types|test)\/.*\/conversion.*\.go$
+    - linters:
+      - revive
+      # By convention, receiver names in a method should reflect their identity.
+      text: "receiver-naming: receiver name"
+      # Ignoring stylistic checks for generated code
+      path: .*(api|types)\/.*\/conversion.*\.go$
+    - linters:
+      - stylecheck
+      text: "ST1016: methods on the same type should have the same receiver name"
+      path: .*(api|types)\/.*\/conversion.*\.go$
+    # We don't care about defer in for loops in test files.
+    - linters:
+      - gocritic
+      text: "deferInLoop: Possible resource leak, 'defer' is called in the 'for' loop"
+      path: _test\.go
+      # Ignore non-constant format string in call to condition utils
+    - linters:
+      - govet
+      text: "non-constant format string in call to sigs\\.k8s\\.io\\/cluster-api\\/util\\/conditions\\."
+    # for i = x; i < y; i++ {} is already optimal; Ignore this linter message
+    - linters:
+      - intrange
+      text: "for loop can be changed to use an integer range"
+    # Ignore the return value of below functions in test files
+    - linters:
+      - errcheck
+      text: Error return value of (`os.Setenv`|`fmt.Fprintf`|`resp.Body.Close`|`os.Unsetenv`|`fmt.Fprintln`|`[\w\.]+\.Close`|`[\w\.]+\.Flush`|`[\w\.]+\.RemoveAll`) is not checked
+      path: (^test\/e2e\/.*\.go$|.*_test\.go$)
+    # Do not validate file paths in tests
+    - linters:
+      - gosec
+      text: "G304: Potential file inclusion via variable"
+      path: ^test\/e2e\/.*\.go$
+    # Ignore the elevated access that "others" have for dirs/files created in tests with 0755 permissions
+    - linters:
+      - gosec
+      text: "G301: Expect directory permissions to be 0750 or less"
+      path: ^test\/e2e\/.*\.go$
+    # Ignore the elevated access that "group" and "others" have for dirs/files opened in tests with 0644 permissions
+    - linters:
+      - gosec
+      text: "G302: Expect file permissions to be 0600 or less"
+      path: ^test\/e2e\/.*\.go$
+    # Ignore unhandled errors in test files when using os.Setenv
+    - linters:
+      - gosec
+      text: "G104: Errors unhandled."
+      path: ^test\/e2e\/.*\.go$
+    # Ignore the unhandled errors on using os.Setenv in test files
+    - linters:
+      - gosec
+      text: "G104: Errors unhandled."
+      path: .*_test\.go$
+    # Ignore unused parameters in test files
+    - linters:
+      - revive
+      text: "^unused-parameter: parameter '.*' seems to be unused, consider removing or renaming it as _$"
+      path: .*(_test|test)\.go$
+    # Ignore adding package comments for all files. TODO: Ideally, we should add package comments to all the packages files.
+    - linters:
+      - revive
+      text: "^package-comments: package comment should be of the form \".*\"$"
+      path: .*\.go$
+    # Ignore adding package comments for all files. TODO: Ideally, we should add package comments to all the packages files.
+    - linters:
+        - revive
+      text: "package-comments: should have a package comment"
+      path: .*\.go$
+    # Ignore adding package comments for all files. TODO: Ideally, we should add package comments to all the packages files.
+    - linters:
+        - stylecheck
+      text: "ST1000: at least one file in a package should have a package comment"
+      path: .*\.go$
+    # Ignore adding package comments for all files. TODO: Ideally, we should add package comments to all the packages files.
+    - linters:
+        - stylecheck
+      text: "ST1000: package comment should be of the form \".*\"$"
+      path: .*\.go$
+  # We are disabling default golangci exclusions because we want to help reviewers to focus on reviewing the most relevant
+  # changes in PRs and avoid nitpicking.
+  exclude-use-default: false
   max-issues-per-linter: 0
   max-same-issues: 0