diff --git a/internal/cephfs/nodeserver.go b/internal/cephfs/nodeserver.go index 2e427f52c..b9dd57c8b 100644 --- a/internal/cephfs/nodeserver.go +++ b/internal/cephfs/nodeserver.go @@ -440,7 +440,7 @@ func getBackingSnapshotRoot( return "", status.Error(codes.Internal, err.Error()) } - defer dir.Close() + defer dir.Close() //nolint:errcheck // other more important errors are returned // Read the contents of /.snap directory into a string slice. diff --git a/internal/kms/kmip.go b/internal/kms/kmip.go index 3e25bbd59..529d710b4 100644 --- a/internal/kms/kmip.go +++ b/internal/kms/kmip.go @@ -185,7 +185,7 @@ func (kms *kmipKMS) EncryptDEK(ctx context.Context, _, plainDEK string) (string, if err != nil { return "", err } - defer conn.Close() + defer conn.Close() //nolint:errcheck // more important errors are returned emd := encryptedMetedataDEK{} emd.Nonce, err = generateNonce(nonceSize) @@ -241,7 +241,7 @@ func (kms *kmipKMS) DecryptDEK(ctx context.Context, _, encryptedDEK string) (str if err != nil { return "", err } - defer conn.Close() + defer conn.Close() //nolint:errcheck // more important errors are returned emd := encryptedMetedataDEK{} err = json.Unmarshal([]byte(encryptedDEK), &emd) @@ -346,7 +346,7 @@ func (kms *kmipKMS) connect() (*tls.Conn, error) { } defer func() { if err != nil { - conn.Close() + conn.Close() //nolint:errcheck,gosec // more important failures are returned } }() diff --git a/internal/kms/vault_sa.go b/internal/kms/vault_sa.go index a5361da73..1a286014f 100644 --- a/internal/kms/vault_sa.go +++ b/internal/kms/vault_sa.go @@ -154,6 +154,7 @@ func initVaultTenantSA(args ProviderInitArgs) (EncryptionKMS, error) { // destroys the vaultTenantConnection object. func (kms *vaultTenantSA) Destroy() { if kms.saTokenDir != "" { + //nolint:errcheck // Destroy() isn't supposed to return an error _ = os.RemoveAll(kms.saTokenDir) } diff --git a/internal/rbd/diskusage.go b/internal/rbd/diskusage.go index 0b6b5af4c..756b29e50 100644 --- a/internal/rbd/diskusage.go +++ b/internal/rbd/diskusage.go @@ -21,6 +21,7 @@ import ( "fmt" rbderrors "github.com/ceph/ceph-csi/internal/rbd/errors" + "github.com/ceph/ceph-csi/internal/util/log" ) // Sparsify checks the size of the objects in the RBD image and calls @@ -28,7 +29,7 @@ import ( // of the image. // This function will return ErrImageInUse if the image is in use, since // sparsifying an image on which i/o is in progress is not optimal. -func (ri *rbdImage) Sparsify(_ context.Context) error { +func (ri *rbdImage) Sparsify(ctx context.Context) error { inUse, err := ri.isInUse() if err != nil { return fmt.Errorf("failed to check if image is in use: %w", err) @@ -42,7 +43,12 @@ func (ri *rbdImage) Sparsify(_ context.Context) error { if err != nil { return err } - defer image.Close() + defer func() { + cErr := image.Close() + if cErr != nil { + log.WarningLog(ctx, "resource leak, failed to close image: %v", cErr) + } + }() imageInfo, err := image.Stat() if err != nil { diff --git a/internal/rbd/group.go b/internal/rbd/group.go index cef63765d..87eac02fb 100644 --- a/internal/rbd/group.go +++ b/internal/rbd/group.go @@ -24,6 +24,7 @@ import ( rbderrors "github.com/ceph/ceph-csi/internal/rbd/errors" "github.com/ceph/ceph-csi/internal/rbd/types" + "github.com/ceph/ceph-csi/internal/util/log" ) // AddToGroup adds the image to the group. This is called from the rbd_group @@ -45,7 +46,12 @@ func (rv *rbdVolume) AddToGroup(ctx context.Context, vg types.VolumeGroup) error if err != nil { return fmt.Errorf("failed to open image %q: %w", rv, err) } - defer image.Close() + defer func() { + cErr := image.Close() + if cErr != nil { + log.WarningLog(ctx, "resource leak, failed to close image: %v", cErr) + } + }() info, err := image.GetGroup() if err != nil { @@ -88,7 +94,12 @@ func (rv *rbdVolume) GetVolumeGroupID(ctx context.Context, resolver types.Volume if err != nil { return "", fmt.Errorf("failed to open image %q: %w", rv, err) } - defer image.Close() + defer func() { + cErr := image.Close() + if cErr != nil { + log.WarningLog(ctx, "resource leak, failed to close image: %v", cErr) + } + }() info, err := image.GetGroup() if err != nil { diff --git a/internal/rbd/mirror.go b/internal/rbd/mirror.go index cf7bf7142..4f3c154a1 100644 --- a/internal/rbd/mirror.go +++ b/internal/rbd/mirror.go @@ -103,12 +103,17 @@ func (ri *rbdImage) ToMirror() (types.Mirror, error) { } // EnableMirroring enables mirroring on an image. -func (rm *rbdMirror) EnableMirroring(_ context.Context, mode librbd.ImageMirrorMode) error { +func (rm *rbdMirror) EnableMirroring(ctx context.Context, mode librbd.ImageMirrorMode) error { image, err := rm.open() if err != nil { return fmt.Errorf("failed to open image %q with error: %w", rm, err) } - defer image.Close() + defer func() { + cErr := image.Close() + if cErr != nil { + log.WarningLog(ctx, "resource leak, failed to close image: %v", cErr) + } + }() err = image.MirrorEnable(mode) if err != nil { @@ -119,12 +124,17 @@ func (rm *rbdMirror) EnableMirroring(_ context.Context, mode librbd.ImageMirrorM } // DisableMirroring disables mirroring on an image. -func (rm *rbdMirror) DisableMirroring(_ context.Context, force bool) error { +func (rm *rbdMirror) DisableMirroring(ctx context.Context, force bool) error { image, err := rm.open() if err != nil { return fmt.Errorf("failed to open image %q with error: %w", rm, err) } - defer image.Close() + defer func() { + cErr := image.Close() + if cErr != nil { + log.WarningLog(ctx, "resource leak, failed to close image: %v", cErr) + } + }() err = image.MirrorDisable(force) if err != nil { @@ -135,12 +145,17 @@ func (rm *rbdMirror) DisableMirroring(_ context.Context, force bool) error { } // GetMirroringInfo gets mirroring information of an image. -func (rm *rbdMirror) GetMirroringInfo(_ context.Context) (types.MirrorInfo, error) { +func (rm *rbdMirror) GetMirroringInfo(ctx context.Context) (types.MirrorInfo, error) { image, err := rm.open() if err != nil { return nil, fmt.Errorf("failed to open image %q with error: %w", rm, err) } - defer image.Close() + defer func() { + cErr := image.Close() + if cErr != nil { + log.WarningLog(ctx, "resource leak, failed to close image: %v", cErr) + } + }() info, err := image.GetMirrorImageInfo() if err != nil { @@ -151,12 +166,18 @@ func (rm *rbdMirror) GetMirroringInfo(_ context.Context) (types.MirrorInfo, erro } // Promote promotes image to primary. -func (rm *rbdMirror) Promote(_ context.Context, force bool) error { +func (rm *rbdMirror) Promote(ctx context.Context, force bool) error { image, err := rm.open() if err != nil { return fmt.Errorf("failed to open image %q with error: %w", rm, err) } - defer image.Close() + defer func() { + cErr := image.Close() + if cErr != nil { + log.WarningLog(ctx, "resource leak, failed to close image: %v", cErr) + } + }() + err = image.MirrorPromote(force) if err != nil { return fmt.Errorf("failed to promote image %q with error: %w", rm, err) @@ -196,12 +217,18 @@ func (rm *rbdMirror) ForcePromote(ctx context.Context, cr *util.Credentials) err } // Demote demotes image to secondary. -func (rm *rbdMirror) Demote(_ context.Context) error { +func (rm *rbdMirror) Demote(ctx context.Context) error { image, err := rm.open() if err != nil { return fmt.Errorf("failed to open image %q with error: %w", rm, err) } - defer image.Close() + defer func() { + cErr := image.Close() + if cErr != nil { + log.WarningLog(ctx, "resource leak, failed to close image: %v", cErr) + } + }() + err = image.MirrorDemote() if err != nil { return fmt.Errorf("failed to demote image %q with error: %w", rm, err) @@ -220,7 +247,13 @@ func (rm *rbdMirror) Resync(ctx context.Context) error { if err != nil { return fmt.Errorf("failed to open image %q with error: %w", rm, err) } - defer image.Close() + defer func() { + cErr := image.Close() + if cErr != nil { + log.WarningLog(ctx, "resource leak, failed to close image: %v", cErr) + } + }() + err = image.MirrorResync() if err != nil { return fmt.Errorf("failed to resync image %q with error: %w", rm, err) @@ -270,12 +303,18 @@ func (rm *rbdMirror) Resync(ctx context.Context) error { } // GetGlobalMirroringStatus get the mirroring status of an image. -func (rm *rbdMirror) GetGlobalMirroringStatus(_ context.Context) (types.GlobalStatus, error) { +func (rm *rbdMirror) GetGlobalMirroringStatus(ctx context.Context) (types.GlobalStatus, error) { image, err := rm.open() if err != nil { return nil, fmt.Errorf("failed to open image %q with error: %w", rm, err) } - defer image.Close() + defer func() { + cErr := image.Close() + if cErr != nil { + log.WarningLog(ctx, "resource leak, failed to close image: %v", cErr) + } + }() + statusInfo, err := image.GetGlobalMirrorStatus() if err != nil { return nil, fmt.Errorf("failed to get image mirroring status %q with error: %w", rm, err) diff --git a/internal/rbd/nodeserver.go b/internal/rbd/nodeserver.go index 1bc9f201a..0befaf779 100644 --- a/internal/rbd/nodeserver.go +++ b/internal/rbd/nodeserver.go @@ -1331,7 +1331,7 @@ func (ns *NodeServer) ext4SupportsPrezeroed() bool { return false } - defer os.Remove(tempImgFile.Name()) + defer os.Remove(tempImgFile.Name()) //nolint:errcheck // failed to remove temp file :-( if err = file.CreateSparseFile(tempImgFile, 1); err != nil { log.WarningLog(ctx, "failed to create sparse file: %v", err) diff --git a/internal/rbd/rbd_util.go b/internal/rbd/rbd_util.go index be4f71a56..3d1608cc7 100644 --- a/internal/rbd/rbd_util.go +++ b/internal/rbd/rbd_util.go @@ -515,7 +515,7 @@ func (ri *rbdImage) getImageID() error { if err != nil { return err } - defer image.Close() + defer image.Close() //nolint:errcheck // not a critical failure id, err := image.GetId() if err != nil { @@ -563,7 +563,7 @@ func (ri *rbdImage) isInUse() (bool, error) { // any error should assume something else is using the image return true, err } - defer image.Close() + defer image.Close() //nolint:errcheck // not a critical failure watchers, err := image.ListWatchers() if err != nil { @@ -933,7 +933,7 @@ func (ri *rbdImage) getParentName() (string, error) { if err != nil { return "", err } - defer rbdImage.Close() + defer rbdImage.Close() //nolint:errcheck // not a critical failure parentInfo, err := rbdImage.GetParent() if err != nil { @@ -948,7 +948,7 @@ func (ri *rbdImage) flatten() error { if err != nil { return err } - defer rbdImage.Close() + defer rbdImage.Close() //nolint:errcheck // not a critical failure err = rbdImage.Flatten() if err != nil { @@ -1519,7 +1519,12 @@ func (ri *rbdImage) createSnapshot(ctx context.Context, pOpts *rbdSnapshot) erro if err != nil { return err } - defer image.Close() + defer func() { + cErr := image.Close() + if cErr != nil { + log.WarningLog(ctx, "resource leak, failed to close image: %v", cErr) + } + }() _, err = image.CreateSnapshot(pOpts.RbdSnapName) @@ -1532,7 +1537,12 @@ func (ri *rbdImage) deleteSnapshot(ctx context.Context, pOpts *rbdSnapshot) erro if err != nil { return err } - defer image.Close() + defer func() { + cErr := image.Close() + if cErr != nil { + log.WarningLog(ctx, "resource leak, failed to close image: %v", cErr) + } + }() snap := image.GetSnapshot(pOpts.RbdSnapName) if snap == nil { @@ -1691,7 +1701,7 @@ func (ri *rbdImage) getImageInfo() error { if err != nil { return err } - defer image.Close() + defer image.Close() //nolint:errcheck // not a critical failure imageInfo, err := image.Stat() if err != nil { @@ -1787,7 +1797,7 @@ func (ri *rbdImage) checkSnapExists(rbdSnap *rbdSnapshot) error { if err != nil { return err } - defer image.Close() + defer image.Close() //nolint:errcheck // not a critical failure snaps, err := image.GetSnapshotNames() if err != nil { @@ -1947,7 +1957,7 @@ func (ri *rbdImage) resize(newSize int64) error { if err != nil { return err } - defer image.Close() + defer image.Close() //nolint:errcheck // not a critical failure err = image.Resize(uint64(util.RoundOffVolSize(newSize) * helpers.MiB)) if err != nil { @@ -1964,7 +1974,7 @@ func (ri *rbdImage) GetMetadata(key string) (string, error) { if err != nil { return "", err } - defer image.Close() + defer image.Close() //nolint:errcheck // not a critical failure return image.GetMetadata(key) } @@ -1974,7 +1984,7 @@ func (ri *rbdImage) SetMetadata(key, value string) error { if err != nil { return err } - defer image.Close() + defer image.Close() //nolint:errcheck // not a critical failure return image.SetMetadata(key, value) } @@ -1985,7 +1995,7 @@ func (ri *rbdImage) RemoveMetadata(key string) error { if err != nil { return err } - defer image.Close() + defer image.Close() //nolint:errcheck // not a critical failure return image.RemoveMetadata(key) } @@ -2050,7 +2060,7 @@ func (ri *rbdImage) DeepCopy(dest *rbdImage) error { if err != nil { return err } - defer image.Close() + defer image.Close() //nolint:errcheck // not a critical failure err = image.DeepCopy(dest.ioctx, dest.RbdImageName, opts) if err != nil { @@ -2067,7 +2077,7 @@ func (ri *rbdImage) DisableDeepFlatten() error { if err != nil { return err } - defer image.Close() + defer image.Close() //nolint:errcheck // not a critical failure return image.UpdateFeatures(librbd.FeatureDeepFlatten, false) } @@ -2085,7 +2095,7 @@ func (ri *rbdImage) listSnapAndChildren() (*snapAndChildrenInfo, error) { if err != nil { return nil, err } - defer image.Close() + defer image.Close() //nolint:errcheck // not a critical failure snaps, err := image.GetSnapshotNames() if err != nil { diff --git a/internal/rbd/rbd_util_test.go b/internal/rbd/rbd_util_test.go index edfd80d75..f75bef83c 100644 --- a/internal/rbd/rbd_util_test.go +++ b/internal/rbd/rbd_util_test.go @@ -302,7 +302,6 @@ func TestStrategicActionOnLogFile(t *testing.T) { if _, err = os.Stat(newExt); os.IsNotExist(err) { t.Errorf("compressed logFile (%s) not found: %v", newExt, err) } - os.Remove(newExt) case "remove": if _, err = os.Stat(tt.args.logFile); !os.IsNotExist(err) { t.Errorf("logFile (%s) not removed: %v", tt.args.logFile, err) @@ -311,7 +310,6 @@ func TestStrategicActionOnLogFile(t *testing.T) { if _, err = os.Stat(tt.args.logFile); os.IsNotExist(err) { t.Errorf("logFile (%s) not preserved: %v", tt.args.logFile, err) } - os.Remove(tt.args.logFile) } }) } diff --git a/internal/rbd/snapshot.go b/internal/rbd/snapshot.go index 50e487b9c..8a5489e73 100644 --- a/internal/rbd/snapshot.go +++ b/internal/rbd/snapshot.go @@ -355,7 +355,12 @@ func (rv *rbdVolume) NewSnapshotByID( if err != nil { return nil, fmt.Errorf("failed to open snapshot image %q: %w", snap, err) } - defer image.Close() + defer func() { + cErr := image.Close() + if cErr != nil { + log.WarningLog(ctx, "resource leak, failed to close image: %v", cErr) + } + }() snapImage, err = image.CreateSnapshot(snap.RbdSnapName) if err != nil && !errors.Is(err, librbd.ErrExist) { diff --git a/internal/util/conn_pool_test.go b/internal/util/conn_pool_test.go index 27079ec21..8696764c2 100644 --- a/internal/util/conn_pool_test.go +++ b/internal/util/conn_pool_test.go @@ -79,14 +79,13 @@ func TestConnPool(t *testing.T) { defer cp.Destroy() // create a keyfile with some contents - keyfile := "/tmp/conn_utils.keyfile" + keyfile := t.TempDir() + "/conn_utils.keyfile" err := os.WriteFile(keyfile, []byte("the-key"), 0o600) if err != nil { t.Errorf("failed to create keyfile: %v", err) return } - defer os.Remove(keyfile) var conn *rados.Conn var unique string diff --git a/internal/util/credentials.go b/internal/util/credentials.go index f152ab145..c4e279c47 100644 --- a/internal/util/credentials.go +++ b/internal/util/credentials.go @@ -81,8 +81,7 @@ func NewUserCredentialsWithMigration(secrets map[string]string) (*Credentials, e // DeleteCredentials removes the KeyFile. func (cr *Credentials) DeleteCredentials() { - // don't complain about unhandled error - _ = os.Remove(cr.KeyFile) + os.Remove(cr.KeyFile) //nolint:errcheck,gosec // no way to return/report the issue here } func storeKey(key string) (string, error) { @@ -92,8 +91,7 @@ func storeKey(key string) (string, error) { } defer func() { if err != nil { - // don't complain about unhandled error - _ = os.Remove(tmpfile.Name()) + os.Remove(tmpfile.Name()) //nolint:errcheck,gosec // no way to return/report the issue here } }() diff --git a/internal/util/cryptsetup/cryptsetup.go b/internal/util/cryptsetup/cryptsetup.go index 7423fb417..403dc69db 100644 --- a/internal/util/cryptsetup/cryptsetup.go +++ b/internal/util/cryptsetup/cryptsetup.go @@ -116,13 +116,13 @@ func (l *luksWrapper) AddKey(devicePath, passphrase, newPassphrase, slot string) if err != nil { return err } - defer os.Remove(passFile.Name()) + defer os.Remove(passFile.Name()) //nolint:errcheck // failed to delete temp file :-( newPassFile, err := file.CreateTempFile("luks-", newPassphrase) if err != nil { return err } - defer os.Remove(newPassFile.Name()) + defer os.Remove(newPassFile.Name()) //nolint:errcheck // failed to delete temp file :-( _, stderr, err := l.execCryptsetupCommand( nil, @@ -185,7 +185,7 @@ func (l *luksWrapper) RemoveKey(devicePath, passphrase, slot string) error { if err != nil { return err } - defer os.Remove(keyFile.Name()) + defer os.Remove(keyFile.Name()) //nolint:errcheck // failed to delete temp file :-( _, stderr, err := l.execCryptsetupCommand( nil, @@ -212,7 +212,7 @@ func (l *luksWrapper) VerifyKey(devicePath, passphrase, slot string) (bool, erro if err != nil { return false, err } - defer os.Remove(keyFile.Name()) + defer os.Remove(keyFile.Name()) //nolint:errcheck // failed to delete temp file :-( _, stderr, err := l.execCryptsetupCommand( nil, diff --git a/internal/util/csiconfig_test.go b/internal/util/csiconfig_test.go index 18d47f378..7e6288f54 100644 --- a/internal/util/csiconfig_test.go +++ b/internal/util/csiconfig_test.go @@ -28,24 +28,19 @@ import ( ) var ( - basePath = "./test_artifacts" - csiClusters = "csi-clusters.json" - pathToConfig = basePath + "/" + csiClusters - clusterID1 = "test1" - clusterID2 = "test2" + csiClusters = "csi-clusters.json" + clusterID1 = "test1" + clusterID2 = "test2" ) -func cleanupTestData() { - os.RemoveAll(basePath) -} - func TestCSIConfig(t *testing.T) { t.Parallel() var err error var data string var content string - defer cleanupTestData() + basePath := t.TempDir() + "/test_artifacts" + pathToConfig := basePath + "/" + csiClusters err = os.MkdirAll(basePath, 0o700) if err != nil { diff --git a/internal/util/file/file.go b/internal/util/file/file.go index 7a1e79503..7de436ed7 100644 --- a/internal/util/file/file.go +++ b/internal/util/file/file.go @@ -34,6 +34,7 @@ func CreateTempFile(prefix, contents string) (*os.File, error) { // In case of error, remove the file if it was created defer func() { if err != nil { + //nolint:errcheck // temporary file failed to remove, shrug _ = os.Remove(file.Name()) } }() diff --git a/internal/util/fscrypt/fscrypt.go b/internal/util/fscrypt/fscrypt.go index a7829a044..8bd585c3d 100644 --- a/internal/util/fscrypt/fscrypt.go +++ b/internal/util/fscrypt/fscrypt.go @@ -135,7 +135,7 @@ func fsyncEncryptedDirectory(dirPath string) error { if err != nil { return err } - defer dir.Close() + defer dir.Close() //nolint:errcheck // still open? sync may have returned an error too return dir.Sync() } @@ -284,7 +284,7 @@ func getInodeEncryptedAttribute(p string) (bool, error) { if err != nil { return false, err } - defer file.Close() + defer file.Close() //nolint:errcheck // failed to close, assume SYS_IOCTL returned an error too var attr int _, _, errno := unix.Syscall(unix.SYS_IOCTL, file.Fd(), unix.FS_IOC_GETFLAGS, diff --git a/internal/util/log/log_utils.go b/internal/util/log/log_utils.go index 4e6cd9671..27b173bb4 100644 --- a/internal/util/log/log_utils.go +++ b/internal/util/log/log_utils.go @@ -36,13 +36,13 @@ func GzipLogFile(pathToFile string) error { if err != nil { return err } - defer gf.Close() // #nosec:G307, error on close is not critical here + defer gf.Close() //nolint:errcheck,nosec // G307, error on close is not critical here // Write compressed data. w := gzip.NewWriter(gf) - defer w.Close() + defer w.Close() //nolint:errcheck // error on close is not critical here if _, err = w.Write(content); err != nil { - os.Remove(newExt) // #nosec:G104, not important error to handle + os.Remove(newExt) //nolint:errcheck,gosec // G104: not important error to handle return err } diff --git a/internal/util/log/log_utils_test.go b/internal/util/log/log_utils_test.go index 11467d2dc..7d908d897 100644 --- a/internal/util/log/log_utils_test.go +++ b/internal/util/log/log_utils_test.go @@ -31,7 +31,6 @@ func TestGzipLogFile(t *testing.T) { if err != nil { fmt.Println(err) } - defer os.Remove(logFile.Name()) if err = GzipLogFile(logFile.Name()); err != nil { t.Errorf("GzipLogFile failed: %v", err) @@ -41,6 +40,4 @@ func TestGzipLogFile(t *testing.T) { if _, err = os.Stat(newExt); errors.Is(err, os.ErrNotExist) { t.Errorf("compressed logFile (%s) not found: %v", newExt, err) } - - os.Remove(newExt) } diff --git a/internal/util/pidlimit.go b/internal/util/pidlimit.go index fd93603e6..115b899be 100644 --- a/internal/util/pidlimit.go +++ b/internal/util/pidlimit.go @@ -45,7 +45,7 @@ func getCgroupPidsFile() (string, error) { if err != nil { return "", err } - defer cgroup.Close() // #nosec: error on close is not critical here + defer cgroup.Close() //nolint:errcheck,nosec // error on close is not critical here pidsMax := "" scanner := bufio.NewScanner(cgroup) @@ -88,7 +88,7 @@ func GetPIDLimit() (int, error) { if err != nil { return 0, err } - defer f.Close() // #nosec: error on close is not critical here + defer f.Close() //nolint:errcheck,nosec // error on close is not critical here maxPidsStr, err := bufio.NewReader(f).ReadString('\n') if err != nil && !errors.Is(err, io.EOF) { @@ -128,7 +128,7 @@ func SetPIDLimit(limit int) error { _, err = f.WriteString(limitStr) if err != nil { - f.Close() // #nosec: a write error will be more useful to return + f.Close() //nolint:errcheck,gosec // a write error will be more useful to return return err }