chore: add golangci-lint (#132)

Author: enocom

.github/workflows/lint.yaml

@@ -40,22 +40,21 @@ jobs:
             } catch (e) {
               console.log('Failed to remove label. Another job may have already removed it!');
             }
-
       - name: Setup Go
         uses: actions/setup-go@v3
         with:
           go-version: "1.19"
-
-      - name: Install goimports
-        run: go install golang.org/x/tools/cmd/goimports@latest
-
       - name: Checkout code
         uses: actions/checkout@v3
         with:
           ref: ${{ github.event.pull_request.head.sha }}
           repository: ${{ github.event.pull_request.head.repo.full_name }}
-
-      - run: goimports -w .
-      - run: go mod tidy
-      - name: Verify no changes from goimports and go mod tidy. If you're reading this and the check has failed, run `goimports -w . && go mod tidy`.
-        run: git diff --exit-code
+      - name: >
+          Verify go mod tidy. If you're reading this and the check has
+          failed, run `goimports -w . && go mod tidy && golangci-lint run`
+        run: |
+          go mod tidy && git diff --exit-code
+      - name: golangci-lint
+        uses: golangci/golangci-lint-action@v3
+        with:
+          version: latest

.golangci.yml

@@ -0,0 +1,50 @@
+# 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.
+# .golangci.yml
+linters:
+  disable-all: true
+  enable:
+    - goimports
+    - revive
+issues:
+  exclude-use-default: false
+linters-settings:
+  revive:
+    rules:
+      - name: blank-imports
+      - name: context-as-argument
+      - name: context-keys-type
+      - name: dot-imports
+      - name: error-return
+      - name: error-strings
+      - name: error-naming
+      - name: exported
+      - name: if-return
+      - name: increment-decrement
+      - name: var-naming
+      - name: var-declaration
+      - 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
+      - name: range-val-in-closure
+      - name: range-val-address
+      - name: import-shadowing

dialer_test.go

@@ -17,7 +17,7 @@ package alloydbconn
 import (
 	"context"
 	"errors"
-	"io/ioutil"
+	"io"
 	"net"
 	"os"
 	"strings"
@@ -69,7 +69,7 @@ func TestDialerCanConnectToInstance(t *testing.T) {
 	}
 	defer conn.Close()
 
-	data, err := ioutil.ReadAll(conn)
+	data, err := io.ReadAll(conn)
 	if err != nil {
 		t.Fatalf("expected ReadAll to succeed, got error %v", err)
 	}

driver/pgxv4/postgres_test.go

@@ -23,7 +23,7 @@ import (
 )
 
 // Example shows how to use the AlloyDB driver
-func ExamplePostgresConnection() {
+func ExampleRegisterDriver() {
 	// Note that sslmode=disable is required it does not mean that the connection
 	// is unencrypted. All connections via the proxy are completely encrypted.
 	pgxv4.RegisterDriver("alloydb")

internal/alloydb/instance.go

@@ -67,11 +67,6 @@ func parseInstURI(cn string) (instanceURI, error) {
 	return c, nil
 }
 
-type metadata struct {
-	ipAddrs map[string]string
-	version string
-}
-
 // refreshOperation is a pending result of a refresh operation of data used to connect securely. It should
 // only be initialized by the Instance struct as part of a refresh cycle.
 type refreshOperation struct {

internal/alloydb/refresh_test.go

@@ -114,7 +114,8 @@ func TestRefreshFailsFast(t *testing.T) {
 	}
 
 	// force the rate limiter to throttle with a timed out context
-	ctx, _ = context.WithTimeout(context.Background(), time.Millisecond)
+	ctx, cancel = context.WithTimeout(context.Background(), time.Millisecond)
+	defer cancel()
 	_, err = r.performRefresh(ctx, cn, RSAKey)
 
 	var wantErr *errtype.DialError

internal/alloydbapi/client.go

@@ -19,25 +19,30 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
-	"io/ioutil"
+	"io"
 	"net/http"
 
 	"google.golang.org/api/googleapi"
 	"google.golang.org/api/option"
 	htransport "google.golang.org/api/transport/http"
 )
 
+// ConnectionInfoResponse is the response from the connection info endpoint.
 type ConnectionInfoResponse struct {
 	ServerResponse googleapi.ServerResponse
 	IPAddress      string `json:"ipAddress"`
 	InstanceUID    string `json:"instanceUid"`
 }
 
+// GenerateClientCertificateRequest is the request to generate a client
+// certificate.
 type GenerateClientCertificateRequest struct {
 	PemCSR              string `json:"pemCsr"`
 	CertificateDuration string `json:"certDuration"`
 }
 
+// GenerateClientCertificateResponse is the response from the certificate
+// endpoint.
 type GenerateClientCertificateResponse struct {
 	ServerResponse      googleapi.ServerResponse
 	PemCertificate      string   `json:"pemCertificate"`
@@ -47,13 +52,15 @@ type GenerateClientCertificateResponse struct {
 // baseURL is the production API endpoint of the AlloyDB Admin API
 const baseURL = "https://alloydb.googleapis.com/v1beta"
 
+// Client is an API client to the AlloyDB Rest API
 type Client struct {
 	client *http.Client
 	// endpoint is the base URL for the AlloyDB Admin API (e.g.
 	// https://alloydb.googleapis.com/v1beta)
 	endpoint string
 }
 
+// NewClient initializes a Client.
 func NewClient(ctx context.Context, opts ...option.ClientOption) (*Client, error) {
 	os := append([]option.ClientOption{
 		option.WithEndpoint(baseURL),
@@ -69,6 +76,7 @@ func NewClient(ctx context.Context, opts ...option.ClientOption) (*Client, error
 	return &Client{client: client, endpoint: endpoint}, nil
 }
 
+// ConnectionInfo retrieves connection info for the provided instance.
 func (c *Client) ConnectionInfo(ctx context.Context, project, region, cluster, instance string) (ConnectionInfoResponse, error) {
 	u := fmt.Sprintf(
 		"%s/projects/%s/locations/%s/clusters/%s/instances/%s/connectionInfo",
@@ -87,7 +95,7 @@ func (c *Client) ConnectionInfo(ctx context.Context, project, region, cluster, i
 	// If the status code is 300 or greater, capture any information in the
 	// response and return it as part of the error.
 	if res.StatusCode >= http.StatusMultipleChoices {
-		body, err := ioutil.ReadAll(res.Body)
+		body, err := io.ReadAll(res.Body)
 		if err != nil {
 			return ConnectionInfoResponse{}, err
 		}
@@ -110,6 +118,7 @@ func (c *Client) ConnectionInfo(ctx context.Context, project, region, cluster, i
 	return ret, nil
 }
 
+// GenerateClientCert creates a client certificate using the provided CSR.
 func (c *Client) GenerateClientCert(ctx context.Context, project, region, cluster string, csr []byte) (GenerateClientCertificateResponse, error) {
 	u := fmt.Sprintf(
 		"%s/projects/%s/locations/%s/clusters/%s:generateClientCertificate",
@@ -134,7 +143,7 @@ func (c *Client) GenerateClientCert(ctx context.Context, project, region, cluste
 	// If the status code is 300 or greater, capture any information in the
 	// response and return it as part of the error.
 	if res.StatusCode >= http.StatusMultipleChoices {
-		body, err := ioutil.ReadAll(res.Body)
+		body, err := io.ReadAll(res.Body)
 		if err != nil {
 			return GenerateClientCertificateResponse{}, err
 		}

internal/mock/alloydbadmin.go

@@ -25,7 +25,7 @@ import (
 	"encoding/json"
 	"encoding/pem"
 	"fmt"
-	"io/ioutil"
+	"io"
 	"math/big"
 	"net"
 	"net/http"
@@ -37,26 +37,32 @@ import (
 	"cloud.google.com/go/alloydbconn/internal/alloydbapi"
 )
 
+// Option configures a FakeAlloyDBInstance
 type Option func(*FakeAlloyDBInstance)
 
+// WithIPAddr sets the IP address of the instance.
 func WithIPAddr(addr string) Option {
 	return func(f *FakeAlloyDBInstance) {
 		f.ipAddr = addr
 	}
 }
 
+// WithServerName sets the name that server uses to identify itself in the TLS
+// handshake.
 func WithServerName(name string) Option {
 	return func(f *FakeAlloyDBInstance) {
 		f.serverName = name
 	}
 }
 
+// WithCertExpiry sets the expiration time of the fake instance
 func WithCertExpiry(expiry time.Time) Option {
 	return func(f *FakeAlloyDBInstance) {
 		f.certExpiry = expiry
 	}
 }
 
+// FakeAlloyDBInstance represents the server side proxy.
 type FakeAlloyDBInstance struct {
 	project string
 	region  string
@@ -92,6 +98,7 @@ var (
 	serverKey     = mustGenerateKey()
 )
 
+// NewFakeInstance creates a Fake AlloyDB instance.
 func NewFakeInstance(proj, reg, clust, name string, opts ...Option) FakeAlloyDBInstance {
 	f := FakeAlloyDBInstance{
 		project:    proj,
@@ -167,6 +174,9 @@ func NewFakeInstance(proj, reg, clust, name string, opts ...Option) FakeAlloyDBI
 	}
 	signedServer, err := x509.CreateCertificate(
 		rand.Reader, serverTemplate, rootCert, &serverKey.PublicKey, rootCAKey)
+	if err != nil {
+		panic(err)
+	}
 	serverCert, err := x509.ParseCertificate(signedServer)
 	if err != nil {
 		panic(err)
@@ -183,18 +193,6 @@ func NewFakeInstance(proj, reg, clust, name string, opts ...Option) FakeAlloyDBI
 	return f
 }
 
-func (f FakeAlloyDBInstance) clientCert() *x509.Certificate {
-	return &x509.Certificate{
-		SerialNumber: &big.Int{},
-		Subject: pkix.Name{
-			CommonName: "alloydb-client",
-		},
-		NotBefore:             time.Now(),
-		NotAfter:              time.Now().AddDate(0, 0, 1),
-		BasicConstraintsValid: true,
-	}
-}
-
 // Request represents a HTTP request for a test Server to mock responses for.
 //
 // Use NewRequest to initialize new Requests.
@@ -252,7 +250,7 @@ func CreateEphemeralSuccess(i FakeAlloyDBInstance, ct int) *Request {
 		reqCt: ct,
 		handle: func(resp http.ResponseWriter, req *http.Request) {
 			// Read the body from the request.
-			b, err := ioutil.ReadAll(req.Body)
+			b, err := io.ReadAll(req.Body)
 			defer req.Body.Close()
 			if err != nil {
 				http.Error(resp, fmt.Errorf("unable to read body: %w", err).Error(), http.StatusBadRequest)
@@ -291,6 +289,10 @@ func CreateEphemeralSuccess(i FakeAlloyDBInstance, ct int) *Request {
 
 			cert, err := x509.CreateCertificate(
 				rand.Reader, template, i.intermedCert, template.PublicKey, i.intermedKey)
+			if err != nil {
+				http.Error(resp, fmt.Errorf("unable to create certificate: %w", err).Error(), http.StatusBadRequest)
+				return
+			}
 
 			certPEM := &bytes.Buffer{}
 			pem.Encode(certPEM, &pem.Block{Type: "CERTIFICATE", Bytes: cert})

options.go

@@ -17,9 +17,9 @@ package alloydbconn
 import (
 	"context"
 	"crypto/rsa"
-	"io/ioutil"
 	"net"
 	"net/http"
+	"os"
 	"time"
 
 	"cloud.google.com/go/alloydbconn/errtype"
@@ -28,6 +28,7 @@ import (
 	apiopt "google.golang.org/api/option"
 )
 
+// CloudPlatformScope is the default OAuth2 scope set on the API client.
 const CloudPlatformScope = "https://www.googleapis.com/auth/cloud-platform"
 
 // An Option is an option for configuring a Dialer.
@@ -59,7 +60,7 @@ func WithOptions(opts ...Option) Option {
 // authentication.
 func WithCredentialsFile(filename string) Option {
 	return func(d *dialerConfig) {
-		b, err := ioutil.ReadFile(filename)
+		b, err := os.ReadFile(filename)
 		if err != nil {
 			d.err = errtype.NewConfigError(err.Error(), "n/a")
 			return