Move linter config into a file, add new linters (#50)

sethvargo · sethvargo · commit 00a5463c94d8 · 2023-01-11T22:14:03.000-05:00
This moves the linting configuration into a file and adds a few new linters that were released since the initial design. One of the nicest ones is goheader, which checks that the Apache license header is added to new Go files. Turns out, we had a slight typo that has been copy-pasted around, so I fixed that too.
diff --git a/.github/workflows/go-lint.yml b/.github/workflows/go-lint.yml
@@ -7,6 +7,10 @@ on:
         description: 'The version of Go to install and use.'
         type: 'string'
         required: true
+      golangci_url:
+        description: 'The URL to a golangci file. This is only used if no file is found in the local directory.'
+        type: 'string'
+        default: 'https://raw.githubusercontent.com/abcxyz/pkg/main/.golangci.yml'
       directory:
         description: 'Directory in which Go files reside.'
         type: 'string'
@@ -60,57 +64,36 @@ jobs:
         with:
           go-version: '${{ inputs.go_version }}'
 
+      - name: 'Lint (download default configuration)'
+        id: 'load-default-config'
+        if: ${{ hashFiles('.golangci.yml', '.golangci.yaml') == '' }}
+        run: |-
+          # Create a unique output file outside of the checkout.
+          GOLANGCI_YML="${RUNNER_TEMP}/${GITHUB_SHA:0:7}.golangci.yml"
+
+          # Download the file, passing in authentication to get a higher rate
+          # limit: https://docs.github.com/en/rest/overview/resources-in-the-rest-api?apiVersion=2022-11-28#rate-limits-for-requests-from-github-actions
+          curl "${{ inputs.golangci_url }}" \
+            --silent \
+            --fail \
+            --location \
+            --header "Authorization: Token ${{ github.token }}" \
+            --output "${GOLANGCI_YML}"
+
+          # Save the result to an output.
+          echo "::notice::Wrote configuration file to ${GOLANGCI_YML}"
+          echo "output-file=${GOLANGCI_YML}" >> "${GITHUB_OUTPUT}"
+
       - name: 'Lint (default configuration)'
-        if: ${{ hashFiles('.golangci.yml') == '' && hashFiles('.golangci.yaml') == '' }}
+        if: ${{ hashFiles('.golangci.yml', '.golangci.yaml') == '' }}
         uses: 'golangci/golangci-lint-action@07db5389c99593f11ad7b44463c2d4233066a9b1' # ratchet:golangci/golangci-lint-action@v3
         with:
           args: |-
-            --enable=${{ join(fromJson('[
-              "asciicheck",
-              "bidichk",
-              "bodyclose",
-              "containedctx",
-              "depguard",
-              "dogsled",
-              "errcheck",
-              "errchkjson",
-              "errname",
-              "errorlint",
-              "exhaustive",
-              "exportloopref",
-              "forcetypeassert",
-              "godot",
-              "gofumpt",
-              "goheader",
-              "goimports",
-              "gomodguard",
-              "goprintffuncname",
-              "gosec",
-              "gosimple",
-              "govet",
-              "ifshort",
-              "ineffassign",
-              "makezero",
-              "noctx",
-              "nolintlint",
-              "prealloc",
-              "predeclared",
-              "revive",
-              "sqlclosecheck",
-              "staticcheck",
-              "stylecheck",
-              "tenv",
-              "thelper",
-              "tparallel",
-              "typecheck",
-              "unconvert",
-              "unused",
-              "whitespace",
-            ]'), ',') }} --max-issues-per-linter=0 --max-same-issues=0 --timeout=5m
+            --config "${{ steps.load-default-config.outputs.output-file }}"
           working-directory: '${{ inputs.directory }}'
 
       - name: 'Lint (custom configuration)'
-        if: ${{ hashFiles('.golangci.yml') != '' && hashFiles('.golangci.yaml') != '' }}
+        if: ${{ hashFiles('.golangci.yml', '.golangci.yaml') != '' }}
         uses: 'golangci/golangci-lint-action@07db5389c99593f11ad7b44463c2d4233066a9b1' # ratchet:golangci/golangci-lint-action@v3
         with:
           working-directory: '${{ inputs.directory }}'
diff --git a/.golangci.yml b/.golangci.yml
@@ -0,0 +1,141 @@
+# Copyright 2023 The Authors (see AUTHORS file)
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+run:
+  # default: '1m'
+  timeout: '5m'
+
+  # default: []
+  build-tags:
+  - 'all'
+
+  # default: []
+  skip-dirs:
+  - 'internal/pb'
+  - 'third_party'
+
+  # default: true
+  skip-dirs-use-default: false
+
+  # default: ''
+  modules-download-mode: 'readonly'
+
+  # default: false
+  allow-parallel-runners: true
+
+linters:
+  enable:
+  - 'asasalint'
+  - 'asciicheck'
+  - 'bidichk'
+  - 'bodyclose'
+  - 'containedctx'
+  - 'depguard'
+  - 'dupword'
+  - 'durationcheck'
+  - 'errcheck'
+  - 'errchkjson'
+  - 'errname'
+  - 'errorlint'
+  - 'execinquery'
+  - 'exhaustive'
+  - 'exportloopref'
+  - 'forcetypeassert'
+  - 'godot'
+  - 'gofmt'
+  - 'gofumpt'
+  - 'goheader'
+  - 'goimports'
+  - 'gomodguard'
+  - 'goprintffuncname'
+  - 'gosec'
+  - 'gosimple'
+  - 'govet'
+  - 'importas'
+  - 'ineffassign'
+  - 'loggercheck'
+  - 'makezero'
+  - 'misspell'
+  - 'noctx'
+  - 'nolintlint'
+  - 'nosprintfhostport'
+  - 'paralleltest'
+  - 'prealloc'
+  - 'predeclared'
+  - 'revive'
+  - 'rowserrcheck'
+  - 'sqlclosecheck'
+  - 'staticcheck'
+  - 'stylecheck'
+  - 'tenv'
+  - 'thelper'
+  - 'typecheck'
+  - 'unconvert'
+  - 'unused'
+  - 'whitespace'
+  - 'wrapcheck'
+
+issues:
+  # default: []
+  exclude:
+  - '^G102:' # gosec: we have to bind to all ifaces in Cloud Run services
+
+  # default: []
+  exclude-rules:
+    # Exclude test files from certain linters
+    - path: '_test.go'
+      linters:
+        - 'wrapcheck'
+
+  # default: 50
+  max-issues-per-linter: 0
+
+  # default: 3
+  max-same-issues: 0
+
+linters-settings:
+  gofumpt:
+    # default: false
+    extra-rules: true
+
+  goheader:
+    values:
+      regexp:
+        YEAR_AUTHOR: '\d{4} .*'
+        INDENTATION: '[\t\f ]{2,}'
+    # default: ""
+    template: |-
+      Copyright {{ YEAR_AUTHOR }}
+
+      Licensed under the Apache License, Version 2.0 (the "License");
+      you may not use this file except in compliance with the License.
+      You may obtain a copy of the License at
+
+      {{ INDENTATION }}http://www.apache.org/licenses/LICENSE-2.0
+
+      Unless required by applicable law or agreed to in writing, software
+      distributed under the License is distributed on an "AS IS" BASIS,
+      WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+      See the License for the specific language governing permissions and
+      limitations under the License.
+
+  wrapcheck:
+    ignoreSigRegexps:
+      - '.ErrorOrNil\('
+      - 'retry\.RetryableError\('
+      - 'status\.Error\('
+
+severity:
+  # default: ''
+  default-severity: error
diff --git a/grpcutil/jwt_auth.go b/grpcutil/jwt_auth.go
@@ -44,7 +44,7 @@ type JWTAuthenticationHandler struct {
 }
 
 // NewJWTAuthenticationHandler returns a JWTAuthenticationHandler with a verifier initialized. Uses defaults
-// for JWT related fields that will retreive a user email when using IAM on GCP.
+// for JWT related fields that will retrieve a user email when using IAM on GCP.
 func NewJWTAuthenticationHandler(ctx context.Context, opts ...JWTAuthOption) (*JWTAuthenticationHandler, error) {
 	j := &JWTAuthenticationHandler{
 		JWTPrefix:          "bearer ",
@@ -68,7 +68,7 @@ func NewJWTAuthenticationHandler(ctx context.Context, opts ...JWTAuthOption) (*J
 	}
 	verifier, err := jwtutil.NewVerifier(ctx, j.Endpoint)
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("failed to create verifier: %w", err)
 	}
 	j.Verifier = verifier
 	return j, nil
diff --git a/mysqltest/docker.go b/mysqltest/docker.go
@@ -110,7 +110,7 @@ func runContainer(conf *config, pool *dockertest.Pool) (*dockertest.Resource, er
 		var extraMsg string
 		switch {
 		case strings.Contains(err.Error(), "no such file"):
-			extraMsg = `. Please install docker: 
+			extraMsg = `. Please install docker:
 		Instructions for Debian: https://docs.docker.com/engine/install/debian/
 		Instructions for Mac: https://docs.docker.com/desktop/mac/install/`
 		case strings.Contains(err.Error(), "permission denied"):
diff --git a/mysqltest/mysqltest_test.go b/mysqltest/mysqltest_test.go
@@ -1,3 +1,17 @@
+// Copyright 2022 The Authors (see AUTHORS file)
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
 package mysqltest
 
 import (
@@ -48,6 +62,8 @@ func TestMustStart_NonexistentVersion(t *testing.T) {
 }
 
 func TestBuildConfig(t *testing.T) {
+	t.Parallel()
+
 	logger := &testLogger{t}
 	conf := buildConfig(
 		WithKillAfterSeconds(1),
diff --git a/terraform/modules/workload-identity-federation/main.tf b/terraform/modules/workload-identity-federation/main.tf
@@ -1,18 +1,17 @@
-/**
- * Copyright 2022 Google LLC
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *      http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
+# Copyright 2022 The Authors (see AUTHORS file)
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
 resource "google_project_service" "serviceusage" {
   project            = var.project_id
   service            = "serviceusage.googleapis.com"
diff --git a/terraform/modules/workload-identity-federation/outputs.tf b/terraform/modules/workload-identity-federation/outputs.tf
@@ -1,18 +1,17 @@
-/**
- * Copyright 2022 Google LLC
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *      http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
+# Copyright 2022 The Authors (see AUTHORS file)
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
 output "pool_name" {
   value = google_iam_workload_identity_pool.pool.name
 }
diff --git a/terraform/modules/workload-identity-federation/variables.tf b/terraform/modules/workload-identity-federation/variables.tf
@@ -1,18 +1,16 @@
-/**
- * Copyright 2022 Google LLC
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *      http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
+# Copyright 2022 The Authors (see AUTHORS file)
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
 
 variable "project_id" {
   type        = string
diff --git a/testutil/integ_test.go b/testutil/integ_test.go
diff --git a/testutil/jwt.go b/testutil/jwt.go
diff --git a/worker/worker.go b/worker/worker.go

Original file line number	Diff line number	Diff line change
`@@ -44,7 +44,7 @@ type JWTAuthenticationHandler struct {`
`44`	`44`	`}`
`45`	`45`
`46`	`46`	`// NewJWTAuthenticationHandler returns a JWTAuthenticationHandler with a verifier initialized. Uses defaults`
`47`		`-// for JWT related fields that will retreive a user email when using IAM on GCP.`
	`47`	`+// for JWT related fields that will retrieve a user email when using IAM on GCP.`
`48`	`48`	`func NewJWTAuthenticationHandler(ctx context.Context, opts ...JWTAuthOption) (*JWTAuthenticationHandler, error) {`
`49`	`49`	`j := &JWTAuthenticationHandler{`
`50`	`50`	`JWTPrefix: "bearer ",`
`@@ -68,7 +68,7 @@ func NewJWTAuthenticationHandler(ctx context.Context, opts ...JWTAuthOption) (*J`
`68`	`68`	`}`
`69`	`69`	`verifier, err := jwtutil.NewVerifier(ctx, j.Endpoint)`
`70`	`70`	`if err != nil {`
`71`		`- return nil, err`
	`71`	`+ return nil, fmt.Errorf("failed to create verifier: %w", err)`
`72`	`72`	`}`
`73`	`73`	`j.Verifier = verifier`
`74`	`74`	`return j, nil`