chore: add golangci-lint (GoogleCloudPlatform#145)

.github/workflows/lint.yaml

@@ -24,21 +24,6 @@ jobs:
     name: run lint
     runs-on: ubuntu-latest
     steps:
-      - 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: Remove PR Label
         if: "${{ github.event.action == 'labeled' && github.event.label.name == 'tests: run' }}"
         uses: actions/github-script@v6
@@ -55,3 +40,19 @@ 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: Checkout code
+        uses: actions/checkout@v3
+        with:
+          ref: ${{ github.event.pull_request.head.sha }}
+          repository: ${{ github.event.pull_request.head.repo.full_name }}
+      - name: go mod tidy
+        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

internal/healthcheck/healthcheck_test.go

@@ -58,12 +58,12 @@ func dialTCP(t *testing.T, addr string) net.Conn {
 
 type fakeDialer struct{}
 
-func (*fakeDialer) Dial(ctx context.Context, inst string, opts ...alloydbconn.DialOption) (net.Conn, error) {
+func (*fakeDialer) Dial(_ context.Context, _ string, _ ...alloydbconn.DialOption) (net.Conn, error) {
 	conn, _ := net.Pipe()
 	return conn, nil
 }
 
-func (*fakeDialer) EngineVersion(ctx context.Context, inst string) (string, error) {
+func (*fakeDialer) EngineVersion(_ context.Context, _ string) (string, error) {
 	return "POSTGRES_14", nil
 }
 
@@ -75,7 +75,7 @@ type errorDialer struct {
 	fakeDialer
 }
 
-func (*errorDialer) Dial(ctx context.Context, inst string, opts ...alloydbconn.DialOption) (net.Conn, error) {
+func (*errorDialer) Dial(_ context.Context, _ string, _ ...alloydbconn.DialOption) (net.Conn, error) {
 	return nil, errors.New("errorDialer always errors")
 }
 

internal/log/log.go

@@ -40,14 +40,17 @@ func NewStdLogger(out, err io.Writer) alloydb.Logger {
 	}
 }
 
+// Infof logs informational messages.
 func (l *StdLogger) Infof(format string, v ...interface{}) {
 	l.infoLog.Printf(format, v...)
 }
 
+// Errorf logs error messages.
 func (l *StdLogger) Errorf(format string, v ...interface{}) {
 	l.errLog.Printf(format, v...)
 }
 
+// Debugf logs debug messages.
 func (l *StdLogger) Debugf(format string, v ...interface{}) {
 	l.infoLog.Printf(format, v...)
 }
@@ -57,14 +60,17 @@ type StructuredLogger struct {
 	logger *zap.SugaredLogger
 }
 
+// Infof logs informational messages.
 func (l *StructuredLogger) Infof(format string, v ...interface{}) {
 	l.logger.Infof(format, v...)
 }
 
+// Errorf logs error messages.
 func (l *StructuredLogger) Errorf(format string, v ...interface{}) {
 	l.logger.Errorf(format, v...)
 }
 
+// Debugf logs debug messages.
 func (l *StructuredLogger) Debugf(format string, v ...interface{}) {
 	l.logger.Infof(format, v...)
 }

internal/proxy/fuse.go

@@ -34,7 +34,7 @@ type symlink struct {
 }
 
 // Readlink implements fs.NodeReadlinker and returns the symlink's path.
-func (s *symlink) Readlink(ctx context.Context) ([]byte, syscall.Errno) {
+func (s *symlink) Readlink(_ context.Context) ([]byte, syscall.Errno) {
 	return []byte(s.path), fs.OK
 }
 
@@ -62,7 +62,7 @@ connections.
 
 // Getattr implements fs.NodeGetattrer and indicates that this file is a regular
 // file.
-func (*readme) Getattr(ctx context.Context, f fs.FileHandle, out *fuse.AttrOut) syscall.Errno {
+func (*readme) Getattr(_ context.Context, _ fs.FileHandle, out *fuse.AttrOut) syscall.Errno {
 	*out = fuse.AttrOut{Attr: fuse.Attr{
 		Mode: 0444 | syscall.S_IFREG,
 		Size: uint64(len(readmeText)),
@@ -71,7 +71,7 @@ func (*readme) Getattr(ctx context.Context, f fs.FileHandle, out *fuse.AttrOut)
 }
 
 // Read implements fs.NodeReader and supports incremental reads.
-func (*readme) Read(ctx context.Context, f fs.FileHandle, dest []byte, off int64) (fuse.ReadResult, syscall.Errno) {
+func (*readme) Read(_ context.Context, _ fs.FileHandle, dest []byte, off int64) (fuse.ReadResult, syscall.Errno) {
 	end := int(off) + len(dest)
 	if end > len(readmeText) {
 		end = len(readmeText)
@@ -81,7 +81,7 @@ func (*readme) Read(ctx context.Context, f fs.FileHandle, dest []byte, off int64
 
 // Open implements fs.NodeOpener and supports opening the README as a read-only
 // file.
-func (*readme) Open(ctx context.Context, mode uint32) (fs.FileHandle, uint32, syscall.Errno) {
+func (*readme) Open(_ context.Context, _ uint32) (fs.FileHandle, uint32, syscall.Errno) {
 	df := nodefs.NewDataFile([]byte(readmeText))
 	rf := nodefs.NewReadOnlyFile(df)
 	return rf, 0, fs.OK

internal/proxy/proxy.go

@@ -380,6 +380,8 @@ func (m MultiErr) Error() string {
 	return strings.Join(errs, ", ")
 }
 
+// Close stops the dialer, closes any open FUSE mounts and any open listeners,
+// and optionally waits for all connections to close before exiting.
 func (c *Client) Close() error {
 	mnts := c.mnts
 
@@ -437,7 +439,7 @@ func (c *Client) Close() error {
 
 // serveSocketMount persistently listens to the socketMounts listener and proxies connections to a
 // given AlloyDB instance.
-func (c *Client) serveSocketMount(ctx context.Context, s *socketMount) error {
+func (c *Client) serveSocketMount(_ context.Context, s *socketMount) error {
 	for {
 		cConn, err := s.Accept()
 		if err != nil {

internal/proxy/proxy_other.go

@@ -81,7 +81,7 @@ type fuseMount struct {
 }
 
 // Readdir returns a list of all active Unix sockets in addition to the README.
-func (c *Client) Readdir(ctx context.Context) (fs.DirStream, syscall.Errno) {
+func (c *Client) Readdir(_ context.Context) (fs.DirStream, syscall.Errno) {
 	entries := []fuse.DirEntry{
 		{Name: "README", Mode: 0555 | fuse.S_IFREG},
 	}
@@ -106,7 +106,7 @@ func (c *Client) Readdir(ctx context.Context) (fs.DirStream, syscall.Errno) {
 // socket is connected to the requested Cloud SQL instance. Lookup returns a
 // symlink (instead of the socket itself) so that multiple callers all use the
 // same Unix socket.
-func (c *Client) Lookup(ctx context.Context, instance string, out *fuse.EntryOut) (*fs.Inode, syscall.Errno) {
+func (c *Client) Lookup(ctx context.Context, instance string, _ *fuse.EntryOut) (*fs.Inode, syscall.Errno) {
 	if instance == "README" {
 		return c.NewInode(ctx, &readme{}, fs.StableAttr{}), fs.OK
 	}

internal/proxy/proxy_test.go

@@ -48,7 +48,7 @@ type fakeDialer struct {
 	instances []string
 }
 
-func (f *fakeDialer) Dial(ctx context.Context, inst string, opts ...alloydbconn.DialOption) (net.Conn, error) {
+func (f *fakeDialer) Dial(_ context.Context, inst string, _ ...alloydbconn.DialOption) (net.Conn, error) {
 	f.mu.Lock()
 	defer f.mu.Unlock()
 	f.dialCount++
@@ -77,7 +77,7 @@ type errorDialer struct {
 	fakeDialer
 }
 
-func (*errorDialer) Dial(ctx context.Context, inst string, opts ...alloydbconn.DialOption) (net.Conn, error) {
+func (*errorDialer) Dial(_ context.Context, _ string, _ ...alloydbconn.DialOption) (net.Conn, error) {
 	return nil, errors.New("errorDialer returns error on Dial")
 }
 
@@ -464,7 +464,7 @@ type spyHandler struct {
 	done chan struct{}
 }
 
-func (s *spyHandler) ServeHTTP(res http.ResponseWriter, req *http.Request) {
+func (s *spyHandler) ServeHTTP(res http.ResponseWriter, _ *http.Request) {
 	s.once.Do(func() { close(s.done) })
 	res.WriteHeader(http.StatusNotImplemented)
 }

tests/common_test.go

@@ -29,8 +29,8 @@ import (
 	"github.com/GoogleCloudPlatform/alloydb-auth-proxy/internal/log"
 )
 
-// proxyExec represents an execution of the AlloyDB proxy.
-type proxyExec struct {
+// ProxyExec represents an execution of the AlloyDB proxy.
+type ProxyExec struct {
 	Out io.ReadCloser
 
 	cmd     *cmd.Command
@@ -41,7 +41,7 @@ type proxyExec struct {
 }
 
 // StartProxy returns a proxyExec representing a running instance of the proxy.
-func StartProxy(ctx context.Context, args ...string) (*proxyExec, error) {
+func StartProxy(ctx context.Context, args ...string) (*ProxyExec, error) {
 	ctx, cancel := context.WithCancel(ctx)
 
 	// Open a pipe for tracking the output from the cmd
@@ -51,14 +51,14 @@ func StartProxy(ctx context.Context, args ...string) (*proxyExec, error) {
 		return nil, fmt.Errorf("unable to open stdout pipe: %w", err)
 	}
 	// defer pw.Close()
-	cmd := cmd.NewCommand(cmd.WithLogger(log.NewStdLogger(pw, pw)))
-	cmd.SetArgs(args)
-	cmd.SetOut(pw)
-	cmd.SetErr(pw)
+	c := cmd.NewCommand(cmd.WithLogger(log.NewStdLogger(pw, pw)))
+	c.SetArgs(args)
+	c.SetOut(pw)
+	c.SetErr(pw)
 
-	p := &proxyExec{
+	p := &ProxyExec{
 		Out:     pr,
-		cmd:     cmd,
+		cmd:     c,
 		cancel:  cancel,
 		closers: []io.Closer{pr, pw},
 		done:    make(chan bool),
@@ -67,18 +67,18 @@ func StartProxy(ctx context.Context, args ...string) (*proxyExec, error) {
 	go func() {
 		defer close(p.done)
 		defer cancel()
-		p.err = cmd.ExecuteContext(ctx)
+		p.err = c.ExecuteContext(ctx)
 	}()
 	return p, nil
 }
 
 // Stop sends the TERM signal to the proxy and returns.
-func (p *proxyExec) Stop() {
+func (p *ProxyExec) Stop() {
 	p.cancel()
 }
 
 // Waits until the execution is completed and returns any error.
-func (p *proxyExec) Wait(ctx context.Context) error {
+func (p *ProxyExec) Wait(ctx context.Context) error {
 	select {
 	case <-ctx.Done():
 		return ctx.Err()
@@ -88,7 +88,7 @@ func (p *proxyExec) Wait(ctx context.Context) error {
 }
 
 // Done returns true if the proxy has exited.
-func (p *proxyExec) Done() bool {
+func (p *ProxyExec) Done() bool {
 	select {
 	case <-p.done:
 		return true
@@ -98,7 +98,7 @@ func (p *proxyExec) Done() bool {
 }
 
 // Close releases any resources associated with the instance.
-func (p *proxyExec) Close() {
+func (p *ProxyExec) Close() {
 	p.cancel()
 	for _, c := range p.closers {
 		c.Close()
@@ -108,7 +108,7 @@ func (p *proxyExec) Close() {
 // WaitForServe waits until the proxy ready to serve traffic. Returns any output from
 // the proxy while starting or any errors experienced before the proxy was ready to
 // server.
-func (p *proxyExec) WaitForServe(ctx context.Context) (output string, err error) {
+func (p *ProxyExec) WaitForServe(ctx context.Context) (output string, err error) {
 	// Watch for the "Ready for new connections" to indicate the proxy is listening
 	buf, in, errCh := new(bytes.Buffer), bufio.NewReader(p.Out), make(chan error, 1)
 	go func() {