cleanup: address golangci 'funcorder' linter problems

The new 'funcorder' linter expects all public functions to be placed
before private functions of a struct. Many private functions needed
moving further down into their files.

Some files had many issues reported. To reduce the churn in those files,
they have been annotated with a `//nolint:funcorder` comment.

Signed-off-by: Niels de Vos <ndevos@ibm.com>

internal/util/conn_pool.go

@@ -59,23 +59,6 @@ func NewConnPool(interval, expiry time.Duration) *ConnPool {
 	return &cp
 }
 
-// loop through all cp.conns and destroy objects that have not been used for cp.expiry.
-func (cp *ConnPool) gc() {
-	cp.lock.Lock()
-	defer cp.lock.Unlock()
-
-	now := time.Now()
-	for key, ce := range cp.conns {
-		if ce.users == 0 && (now.Sub(ce.lastUsed)) > cp.expiry {
-			ce.destroy()
-			delete(cp.conns, key)
-		}
-	}
-
-	// schedule the next gc() run
-	cp.timer.Reset(cp.interval)
-}
-
 // Destroy stops the garbage collector and destroys all connections in the pool.
 func (cp *ConnPool) Destroy() {
 	cp.timer.Stop()
@@ -94,30 +77,6 @@ func (cp *ConnPool) Destroy() {
 	}
 }
 
-func (cp *ConnPool) generateUniqueKey(monitors, user, keyfile string) (string, error) {
-	// the keyfile can be unique for operations, contents will be the same
-	key, err := os.ReadFile(keyfile) // #nosec:G304, file inclusion via variable.
-	if err != nil {
-		return "", fmt.Errorf("could not open keyfile %s: %w", keyfile, err)
-	}
-
-	return fmt.Sprintf("%s|%s|%s", monitors, user, string(key)), nil
-}
-
-// getExisting returns the existing rados.Conn associated with the unique key.
-//
-// Requires: locked cp.lock because of ce.get().
-func (cp *ConnPool) getConn(unique string) *rados.Conn {
-	ce, exists := cp.conns[unique]
-	if exists {
-		ce.get()
-
-		return ce.conn
-	}
-
-	return nil
-}
-
 // Get returns a rados.Conn for the given arguments. Creates a new rados.Conn in
 // case there is none. Use the returned rados.Conn to reduce the reference
 // count with ConnPool.Put(unique).
@@ -206,6 +165,47 @@ func (cp *ConnPool) Put(conn *rados.Conn) {
 	}
 }
 
+// loop through all cp.conns and destroy objects that have not been used for cp.expiry.
+func (cp *ConnPool) gc() {
+	cp.lock.Lock()
+	defer cp.lock.Unlock()
+
+	now := time.Now()
+	for key, ce := range cp.conns {
+		if ce.users == 0 && (now.Sub(ce.lastUsed)) > cp.expiry {
+			ce.destroy()
+			delete(cp.conns, key)
+		}
+	}
+
+	// schedule the next gc() run
+	cp.timer.Reset(cp.interval)
+}
+
+func (cp *ConnPool) generateUniqueKey(monitors, user, keyfile string) (string, error) {
+	// the keyfile can be unique for operations, contents will be the same
+	key, err := os.ReadFile(keyfile) // #nosec:G304, file inclusion via variable.
+	if err != nil {
+		return "", fmt.Errorf("could not open keyfile %s: %w", keyfile, err)
+	}
+
+	return fmt.Sprintf("%s|%s|%s", monitors, user, string(key)), nil
+}
+
+// getExisting returns the existing rados.Conn associated with the unique key.
+//
+// Requires: locked cp.lock because of ce.get().
+func (cp *ConnPool) getConn(unique string) *rados.Conn {
+	ce, exists := cp.conns[unique]
+	if exists {
+		ce.get()
+
+		return ce.conn
+	}
+
+	return nil
+}
+
 // Add a reference to the connEntry.
 // /!\ Only call this while holding the ConnPool.lock.
 func (ce *connEntry) get() {

internal/util/credentials.go

@@ -43,6 +43,48 @@ type Credentials struct {
 	KeyFile string
 }
 
+// NewUserCredentials creates new user credentials from secret.
+func NewUserCredentials(secrets map[string]string) (*Credentials, error) {
+	return newCredentialsFromSecret(credUserID, credUserKey, secrets)
+}
+
+// NewAdminCredentials creates new admin credentials from secret.
+func NewAdminCredentials(secrets map[string]string) (*Credentials, error) {
+	// Use userID and userKey if found else fallback to adminID and adminKey
+	if cred, err := newCredentialsFromSecret(credUserID, credUserKey, secrets); err == nil {
+		return cred, nil
+	}
+	log.WarningLogMsg("adminID and adminKey are deprecated, please use userID and userKey instead")
+
+	return newCredentialsFromSecret(credAdminID, credAdminKey, secrets)
+}
+
+// NewUserCredentialsWithMigration takes secret map from the request and validate it is
+// a migration secret, if yes, it continues to create CR from it after parsing the migration
+// secret. If it is not a migration it will continue the attempt to create credentials from it
+// without parsing the secret. This function returns credentials and error.
+func NewUserCredentialsWithMigration(secrets map[string]string) (*Credentials, error) {
+	if isMigrationSecret(secrets) {
+		migSecret, err := ParseAndSetSecretMapFromMigSecret(secrets)
+		if err != nil {
+			return nil, err
+		}
+		secrets = migSecret
+	}
+	cr, cErr := NewUserCredentials(secrets)
+	if cErr != nil {
+		return nil, cErr
+	}
+
+	return cr, nil
+}
+
+// DeleteCredentials removes the KeyFile.
+func (cr *Credentials) DeleteCredentials() {
+	// don't complain about unhandled error
+	_ = os.Remove(cr.KeyFile)
+}
+
 func storeKey(key string) (string, error) {
 	tmpfile, err := os.CreateTemp(tmpKeyFileLocation, tmpKeyFileNamePrefix)
 	if err != nil {
@@ -99,28 +141,6 @@ func newCredentialsFromSecret(idField, keyField string, secrets map[string]strin
 	return c, err
 }
 
-// DeleteCredentials removes the KeyFile.
-func (cr *Credentials) DeleteCredentials() {
-	// don't complain about unhandled error
-	_ = os.Remove(cr.KeyFile)
-}
-
-// NewUserCredentials creates new user credentials from secret.
-func NewUserCredentials(secrets map[string]string) (*Credentials, error) {
-	return newCredentialsFromSecret(credUserID, credUserKey, secrets)
-}
-
-// NewAdminCredentials creates new admin credentials from secret.
-func NewAdminCredentials(secrets map[string]string) (*Credentials, error) {
-	// Use userID and userKey if found else fallback to adminID and adminKey
-	if cred, err := newCredentialsFromSecret(credUserID, credUserKey, secrets); err == nil {
-		return cred, nil
-	}
-	log.WarningLogMsg("adminID and adminKey are deprecated, please use userID and userKey instead")
-
-	return newCredentialsFromSecret(credAdminID, credAdminKey, secrets)
-}
-
 // GetMonValFromSecret returns monitors from secret.
 func GetMonValFromSecret(secrets map[string]string) (string, error) {
 	if mons, ok := secrets[credMonitors]; ok {
@@ -160,23 +180,3 @@ func isMigrationSecret(secrets map[string]string) bool {
 	// was hit on migration request compared to general one.
 	return len(secrets) != 0 && secrets[migUserKey] != ""
 }
-
-// NewUserCredentialsWithMigration takes secret map from the request and validate it is
-// a migration secret, if yes, it continues to create CR from it after parsing the migration
-// secret. If it is not a migration it will continue the attempt to create credentials from it
-// without parsing the secret. This function returns credentials and error.
-func NewUserCredentialsWithMigration(secrets map[string]string) (*Credentials, error) {
-	if isMigrationSecret(secrets) {
-		migSecret, err := ParseAndSetSecretMapFromMigSecret(secrets)
-		if err != nil {
-			return nil, err
-		}
-		secrets = migSecret
-	}
-	cr, cErr := NewUserCredentials(secrets)
-	if cErr != nil {
-		return nil, cErr
-	}
-
-	return cr, nil
-}

internal/util/idlocker.go

@@ -108,6 +108,59 @@ func NewOperationLock() *OperationLock {
 	}
 }
 
+// GetSnapshotCreateLock gets the snapshot lock on given volumeID.
+func (ol *OperationLock) GetSnapshotCreateLock(volumeID string) error {
+	return ol.tryAcquire(createOp, volumeID)
+}
+
+// GetCloneLock gets the clone lock on given volumeID.
+func (ol *OperationLock) GetCloneLock(volumeID string) error {
+	return ol.tryAcquire(cloneOpt, volumeID)
+}
+
+// GetDeleteLock gets the delete lock on given volumeID,ensures that there is
+// no clone,restore and expand operation on given volumeID.
+func (ol *OperationLock) GetDeleteLock(volumeID string) error {
+	return ol.tryAcquire(deleteOp, volumeID)
+}
+
+// GetRestoreLock gets the restore lock on given volumeID,ensures that there is
+// no delete operation on given volumeID.
+func (ol *OperationLock) GetRestoreLock(volumeID string) error {
+	return ol.tryAcquire(restoreOp, volumeID)
+}
+
+// GetExpandLock gets the expand lock on given volumeID,ensures that there is
+// no delete and clone operation on given volumeID.
+func (ol *OperationLock) GetExpandLock(volumeID string) error {
+	return ol.tryAcquire(expandOp, volumeID)
+}
+
+// ReleaseSnapshotCreateLock releases the create lock on given volumeID.
+func (ol *OperationLock) ReleaseSnapshotCreateLock(volumeID string) {
+	ol.release(createOp, volumeID)
+}
+
+// ReleaseCloneLock releases the clone lock on given volumeID.
+func (ol *OperationLock) ReleaseCloneLock(volumeID string) {
+	ol.release(cloneOpt, volumeID)
+}
+
+// ReleaseDeleteLock releases the delete lock on given volumeID.
+func (ol *OperationLock) ReleaseDeleteLock(volumeID string) {
+	ol.release(deleteOp, volumeID)
+}
+
+// ReleaseRestoreLock releases the restore lock on given volumeID.
+func (ol *OperationLock) ReleaseRestoreLock(volumeID string) {
+	ol.release(restoreOp, volumeID)
+}
+
+// ReleaseExpandLock releases the expand lock on given volumeID.
+func (ol *OperationLock) ReleaseExpandLock(volumeID string) {
+	ol.release(expandOp, volumeID)
+}
+
 // tryAcquire tries to acquire the lock for operating on volumeID and returns true if successful.
 // If another operation is already using volumeID, returns false.
 func (ol *OperationLock) tryAcquire(op operation, volumeID string) error {
@@ -178,59 +231,6 @@ func (ol *OperationLock) tryAcquire(op operation, volumeID string) error {
 	return nil
 }
 
-// GetSnapshotCreateLock gets the snapshot lock on given volumeID.
-func (ol *OperationLock) GetSnapshotCreateLock(volumeID string) error {
-	return ol.tryAcquire(createOp, volumeID)
-}
-
-// GetCloneLock gets the clone lock on given volumeID.
-func (ol *OperationLock) GetCloneLock(volumeID string) error {
-	return ol.tryAcquire(cloneOpt, volumeID)
-}
-
-// GetDeleteLock gets the delete lock on given volumeID,ensures that there is
-// no clone,restore and expand operation on given volumeID.
-func (ol *OperationLock) GetDeleteLock(volumeID string) error {
-	return ol.tryAcquire(deleteOp, volumeID)
-}
-
-// GetRestoreLock gets the restore lock on given volumeID,ensures that there is
-// no delete operation on given volumeID.
-func (ol *OperationLock) GetRestoreLock(volumeID string) error {
-	return ol.tryAcquire(restoreOp, volumeID)
-}
-
-// GetExpandLock gets the expand lock on given volumeID,ensures that there is
-// no delete and clone operation on given volumeID.
-func (ol *OperationLock) GetExpandLock(volumeID string) error {
-	return ol.tryAcquire(expandOp, volumeID)
-}
-
-// ReleaseSnapshotCreateLock releases the create lock on given volumeID.
-func (ol *OperationLock) ReleaseSnapshotCreateLock(volumeID string) {
-	ol.release(createOp, volumeID)
-}
-
-// ReleaseCloneLock releases the clone lock on given volumeID.
-func (ol *OperationLock) ReleaseCloneLock(volumeID string) {
-	ol.release(cloneOpt, volumeID)
-}
-
-// ReleaseDeleteLock releases the delete lock on given volumeID.
-func (ol *OperationLock) ReleaseDeleteLock(volumeID string) {
-	ol.release(deleteOp, volumeID)
-}
-
-// ReleaseRestoreLock releases the restore lock on given volumeID.
-func (ol *OperationLock) ReleaseRestoreLock(volumeID string) {
-	ol.release(restoreOp, volumeID)
-}
-
-// ReleaseExpandLock releases the expand lock on given volumeID.
-func (ol *OperationLock) ReleaseExpandLock(volumeID string) {
-	ol.release(expandOp, volumeID)
-}
-
 // release deletes the lock on volumeID.
 func (ol *OperationLock) release(op operation, volumeID string) {
 	ol.mux.Lock()

internal/util/reftracker/radoswrapper/fakerados.go

@@ -134,15 +134,6 @@ func (c *FakeIOContext) GetLastVersion() (uint64, error) {
 	return c.LastObjVersion, nil
 }
 
-func (c *FakeIOContext) getObj(oid string) (*FakeObj, error) {
-	obj, ok := c.Rados.Objs[oid]
-	if !ok {
-		return nil, rados.ErrNotFound
-	}
-
-	return obj, nil
-}
-
 func (c *FakeIOContext) GetXattr(oid, key string, data []byte) (int, error) {
 	obj, ok := c.Rados.Objs[oid]
 	if !ok {
@@ -200,6 +191,15 @@ func (c *FakeIOContext) CreateReadOp() ReadOpW {
 	}
 }
 
+func (c *FakeIOContext) getObj(oid string) (*FakeObj, error) {
+	obj, ok := c.Rados.Objs[oid]
+	if !ok {
+		return nil, rados.ErrNotFound
+	}
+
+	return obj, nil
+}
+
 func (r *FakeReadOp) Operate(oid string) error {
 	r.oid = oid