Updated the set of Golang linters to include many more

.golangci.yml

@@ -4,25 +4,35 @@ linters:
 
   # Additional linters enabled
   enable:
+    - asasalint
     - asciicheck
     - bidichk
     - bodyclose
+    - canonicalheader
+    - containedctx
     - contextcheck
     - copyloopvar
     - cyclop
     - decorder
     - dogsled
+    - dupl
+    - dupword
     - durationcheck
+    - err113
+    - errchkjson
     - errname
     - errorlint
     - exhaustive
+    - fatcontext
     - gci
     - gochecknoglobals
+    - gocognit
     - goconst
     - gocritic
     - godot
     - gofmt
     - gomoddirectives
+    - gomodguard
     - goprintffuncname
     - gosec
     - importas
@@ -31,14 +41,25 @@ linters:
     - nakedret
     - nilerr
     - nilnil
+    - noctx
+    - nolintlint
     - nosprintfhostport
+    - perfsprint
     - prealloc
     - predeclared
+    - reassign
     - revive
+    - sqlclosecheck
+    - tagalign
+    - tagliatelle
     - tenv
+    - testifylint
     - testpackage
+    - thelper
+    - tparallel
     - unconvert
     - unparam
+    - usestdlibvars
     - wastedassign
     - whitespace
     - wrapcheck

CHANGELOG.md

@@ -4,6 +4,29 @@ NOTES:
 
 * Updated repository Go version to `1.23.3`
 * Updated all dependencies to latest
+* Updated [golangci-lint](https://golangci-lint.run/) linters
+  * Added `asasalint`
+  * Added `canonicalheader`
+  * Added `containedctx`
+  * Added `dupl`
+  * Added `dupword`
+  * Added `err113`
+  * Added `errchkjson`
+  * Added `fatcontext`
+  * Added `gocognit`
+  * Added `gomodguard`
+  * Added `noctx`
+  * Added `nolintlint`
+  * Added `perfsprint`
+  * Added `reassign`
+  * Added `sqlclosecheck`
+  * Added `tagalign`
+  * Added `tagliatelle`
+  * Added `testifylint`
+  * Added `thelper`
+  * Added `tparallel`
+  * Added `usestdlibvars`
+* Because of `err113` linter, created a couple of error types
 
 ## 1.2.0 (October 31, 2024)
 

Makefile

@@ -14,6 +14,9 @@ install: build
 lint:
 	golangci-lint run
 
+lint-fix:
+	golangci-lint run --fix
+
 # Generates the documentation that eventually gets published here:
 # https://registry.terraform.io/providers/tfzk/zookeeper/latest/docs.
 generate:

internal/client/client.go

@@ -35,11 +35,16 @@ type ZNode struct {
 
 // Re-exporting errors from ZK library for better encapsulation.
 var (
-	ErrorZNodeAlreadyExists = zk.ErrNodeExists
-	ErrorZNodeDoesNotExist  = zk.ErrNoNode
-	ErrorZNodeHasChildren   = zk.ErrNotEmpty
-	ErrorConnectionClosed   = zk.ErrConnectionClosed
-	ErrorInvalidArguments   = zk.ErrBadArguments
+	ErrZNodeAlreadyExists = zk.ErrNodeExists
+	ErrZNodeDoesNotExist  = zk.ErrNoNode
+	ErrZNodeHasChildren   = zk.ErrNotEmpty
+	ErrConnectionClosed   = zk.ErrConnectionClosed
+	ErrInvalidArguments   = zk.ErrBadArguments
+)
+
+var (
+	// ErrUserPassBothOrNone returned when only one of username and password is specified: either both or none is allowed.
+	ErrUserPassBothOrNone = errors.New("both username and password must be specified together")
 )
 
 const (
@@ -86,7 +91,7 @@ func NewClient(servers string, sessionTimeoutSec int, username string, password
 	fmt.Printf("[DEBUG] Connected to ZooKeeper servers %s\n", serversSplit)
 
 	if (username == "") != (password == "") {
-		return nil, fmt.Errorf("both username and password must be specified together")
+		return nil, ErrUserPassBothOrNone
 	}
 
 	if username != "" {
@@ -109,7 +114,7 @@ func NewClient(servers string, sessionTimeoutSec int, username string, password
 func NewClientFromEnv() (*Client, error) {
 	zkServers, ok := os.LookupEnv(EnvZooKeeperServer)
 	if !ok {
-		return nil, fmt.Errorf("missing environment variable: %s", EnvZooKeeperServer)
+		return nil, NewMissingEnvVarError(EnvZooKeeperServer)
 	}
 
 	zkSession, ok := os.LookupEnv(EnvZooKeeperSessionSec)
@@ -134,7 +139,7 @@ func NewClientFromEnv() (*Client, error) {
 // Note that any necessary ZNode parents will be created if absent.
 func (c *Client) Create(path string, data []byte, acl []zk.ACL) (*ZNode, error) {
 	if path[len(path)-1] == zNodePathSeparator {
-		return nil, fmt.Errorf("non-sequential ZNode cannot have path '%s' because it ends in '%c'", path, zNodePathSeparator)
+		return nil, NewNonSeqZNodeCannotEndWithPathSeparatorError(path)
 	}
 
 	return c.doCreate(path, data, 0, acl)
@@ -207,7 +212,7 @@ func (c *Client) createEmptyZNodes(pathsInOrder []string, createFlags int32, acl
 		// a ZNode already existing.
 		if !exists {
 			_, err := c.zkConn.Create(path, nil, createFlags, acl)
-			if err != nil && !errors.Is(err, ErrorZNodeAlreadyExists) {
+			if err != nil && !errors.Is(err, ErrZNodeAlreadyExists) {
 				return fmt.Errorf("failed to create parent ZNode '%s' (createFlags: %d, acl: %v): %w", path, createFlags, acl, err)
 			}
 		}
@@ -246,7 +251,7 @@ func (c *Client) Update(path string, data []byte, acl []zk.ACL) (*ZNode, error)
 	}
 
 	if !exists {
-		return nil, fmt.Errorf("failed to update ZNode '%s': does not exist", path)
+		return nil, NewCannotUpdateDoesNotExistError(path)
 	}
 
 	_, err = c.zkConn.SetACL(path, acl, matchAnyVersion)

internal/client/client_test.go

@@ -5,132 +5,136 @@ import (
 
 	"github.com/go-zookeeper/zk"
 	testifyAssert "github.com/stretchr/testify/assert"
+	testifyRequire "github.com/stretchr/testify/require"
 	"github.com/tfzk/terraform-provider-zookeeper/internal/client"
 )
 
-func initTest(t *testing.T) (*client.Client, *testifyAssert.Assertions) {
+func initTest(t *testing.T) (*client.Client, *testifyAssert.Assertions, *testifyRequire.Assertions) {
+	t.Helper()
 	assert := testifyAssert.New(t)
+	require := testifyRequire.New(t)
 
 	client, err := client.NewClientFromEnv()
-	assert.NoError(err)
+	require.NoError(err)
+	require.NoError(err)
 
-	return client, assert
+	return client, assert, require
 }
 
 func TestClassicCRUD(t *testing.T) {
-	client, assert := initTest(t)
+	client, assert, require := initTest(t)
 	defer client.Close()
 
 	// confirm not exists yet
 	znodeExists, err := client.Exists("/test/ClassicCRUD")
-	assert.NoError(err)
+	require.NoError(err)
 	assert.False(znodeExists)
 
 	// create
 	znode, err := client.Create("/test/ClassicCRUD", []byte("one"), zk.WorldACL(zk.PermAll))
-	assert.NoError(err)
+	require.NoError(err)
 	assert.Equal("/test/ClassicCRUD", znode.Path)
 	assert.Equal([]byte("one"), znode.Data)
 
 	// confirm exists
 	znodeExists, err = client.Exists("/test/ClassicCRUD")
-	assert.NoError(err)
+	require.NoError(err)
 	assert.True(znodeExists)
 
 	// read
 	znode, err = client.Read("/test/ClassicCRUD")
-	assert.NoError(err)
+	require.NoError(err)
 	assert.Equal("/test/ClassicCRUD", znode.Path)
 	assert.Equal([]byte("one"), znode.Data)
 
 	// update
 	znode, err = client.Update("/test/ClassicCRUD", []byte("two"), zk.WorldACL(zk.PermAll))
-	assert.NoError(err)
+	require.NoError(err)
 	assert.Equal("/test/ClassicCRUD", znode.Path)
 	assert.Equal([]byte("two"), znode.Data)
 
 	// delete
 	err = client.Delete("/test/ClassicCRUD")
-	assert.NoError(err)
+	require.NoError(err)
 
 	// confirm not exists
 	znodeExists, err = client.Exists("/test/ClassicCRUD")
-	assert.NoError(err)
+	require.NoError(err)
 	assert.False(znodeExists)
 
 	// confirm container still exists
 	znodeExists, err = client.Exists("/test")
-	assert.NoError(err)
+	require.NoError(err)
 	assert.True(znodeExists)
 
 	// delete container
 	err = client.Delete("/test")
-	assert.NoError(err)
+	require.NoError(err)
 }
 
 func TestCreateSequential(t *testing.T) {
-	client, assert := initTest(t)
+	client, assert, require := initTest(t)
 	defer client.Close()
 
 	noPrefixSeqZNode, err := client.CreateSequential("/test/CreateSequential/", []byte("seq"), zk.WorldACL(zk.PermAll))
-	assert.NoError(err)
+	require.NoError(err)
 	assert.Equal("/test/CreateSequential/0000000000", noPrefixSeqZNode.Path)
 
 	prefixSeqZNode, err := client.CreateSequential("/test/CreateSequentialWithPrefix/prefix-", []byte("seq"), zk.WorldACL(zk.PermAll))
-	assert.NoError(err)
+	require.NoError(err)
 	assert.Equal("/test/CreateSequentialWithPrefix/prefix-0000000000", prefixSeqZNode.Path)
 
 	// delete, recursively
 	err = client.Delete("/test")
-	assert.NoError(err)
+	require.NoError(err)
 }
 
 func TestDigestAuthenticationSuccess(t *testing.T) {
 	t.Setenv(client.EnvZooKeeperUsername, "username")
 	t.Setenv(client.EnvZooKeeperPassword, "password")
-	client, assert := initTest(t)
+	client, assert, require := initTest(t)
 	defer client.Close()
 
 	// Create a ZNode accessible only by the given user
 	acl := zk.DigestACL(zk.PermAll, "username", "password")
 	znode, err := client.Create("/auth-test/DigestAuthentication", []byte("data"), acl)
-	assert.NoError(err)
+	require.NoError(err)
 	assert.Equal("/auth-test/DigestAuthentication", znode.Path)
 	assert.Equal([]byte("data"), znode.Data)
 	assert.Equal(acl, znode.ACL)
 
 	// Make sure it's accessible
 	znode, err = client.Read("/auth-test/DigestAuthentication")
-	assert.NoError(err)
+	require.NoError(err)
 	assert.Equal("/auth-test/DigestAuthentication", znode.Path)
 	assert.Equal([]byte("data"), znode.Data)
 	assert.Equal(acl, znode.ACL)
 
 	// Cleanup
 	err = client.Delete("/auth-test/DigestAuthentication")
-	assert.NoError(err)
+	require.NoError(err)
 	err = client.Delete("/auth-test")
-	assert.NoError(err)
+	require.NoError(err)
 }
 
 func TestFailureWhenReadingZNodeWithIncorrectAuth(t *testing.T) {
 	// Create client authenticated as foo user
 	t.Setenv(client.EnvZooKeeperUsername, "foo")
 	t.Setenv(client.EnvZooKeeperPassword, "password")
-	fooClient, assert := initTest(t)
+	fooClient, assert, require := initTest(t)
 	defer fooClient.Close()
 
 	// Create a ZNode accessible only by foo user
 	acl := zk.DigestACL(zk.PermAll, "foo", "password")
 	znode, err := fooClient.Create("/auth-fail-test/AccessibleOnlyByFoo", []byte("data"), acl)
-	assert.NoError(err)
+	require.NoError(err)
 	assert.Equal("/auth-fail-test/AccessibleOnlyByFoo", znode.Path)
 	assert.Equal([]byte("data"), znode.Data)
 	assert.Equal(acl, znode.ACL)
 
 	// Make sure it's accessible by foo user
 	znode, err = fooClient.Read("/auth-fail-test/AccessibleOnlyByFoo")
-	assert.NoError(err)
+	require.NoError(err)
 	assert.Equal("/auth-fail-test/AccessibleOnlyByFoo", znode.Path)
 	assert.Equal([]byte("data"), znode.Data)
 	assert.Equal(acl, znode.ACL)
@@ -139,52 +143,52 @@ func TestFailureWhenReadingZNodeWithIncorrectAuth(t *testing.T) {
 	t.Setenv(client.EnvZooKeeperUsername, "bar")
 	t.Setenv(client.EnvZooKeeperPassword, "password")
 	barClient, err := client.NewClientFromEnv()
-	assert.NoError(err)
+	require.NoError(err)
 	defer barClient.Close()
 
 	// The node should be inaccessible by bar user
 	_, err = barClient.Read("/auth-fail-test/AccessibleOnlyByFoo")
-	assert.EqualError(err, "failed to read ZNode '/auth-fail-test/AccessibleOnlyByFoo': zk: not authenticated")
+	require.EqualError(err, "failed to read ZNode '/auth-fail-test/AccessibleOnlyByFoo': zk: not authenticated")
 
 	// Cleanup
 	err = fooClient.Delete("/auth-fail-test/AccessibleOnlyByFoo")
-	assert.NoError(err)
+	require.NoError(err)
 	err = fooClient.Delete("/auth-fail-test")
-	assert.NoError(err)
+	require.NoError(err)
 }
 
 func TestFailureWhenCreatingForNonSequentialZNodeEndingInSlash(t *testing.T) {
-	client, assert := initTest(t)
+	client, assert, require := initTest(t)
 	defer client.Close()
 
 	_, err := client.Create("/test/willFail/", nil, zk.WorldACL(zk.PermAll))
-	assert.Error(err)
+	require.Error(err)
 	assert.Equal("non-sequential ZNode cannot have path '/test/willFail/' because it ends in '/'", err.Error())
 }
 
 func TestFailureWhenCreatingWhenZNodeAlreadyExists(t *testing.T) {
-	client, assert := initTest(t)
+	client, assert, require := initTest(t)
 	defer client.Close()
 
 	_, err := client.Create("/test/node", nil, zk.WorldACL(zk.PermAll))
-	assert.NoError(err)
+	require.NoError(err)
 	_, err = client.Create("/test/node", nil, zk.WorldACL(zk.PermAll))
-	assert.Error(err)
+	require.Error(err)
 	assert.Equal("failed to create ZNode '/test/node' (size: 0, createFlags: 0, acl: [{31 world anyone}]): zk: node already exists", err.Error())
 
 	err = client.Delete("/test")
-	assert.NoError(err)
+	require.NoError(err)
 }
 
 func TestFailureWithNonExistingZNodes(t *testing.T) {
-	client, assert := initTest(t)
+	client, assert, require := initTest(t)
 	defer client.Close()
 
 	_, err := client.Read("/does-not-exist")
-	assert.Error(err)
+	require.Error(err)
 	assert.Equal("failed to read ZNode '/does-not-exist': zk: node does not exist", err.Error())
 
 	_, err = client.Update("/also-does-not-exist", nil, zk.WorldACL(zk.PermAll))
-	assert.Error(err)
+	require.Error(err)
 	assert.Equal("failed to update ZNode '/also-does-not-exist': does not exist", err.Error())
 }