test: fix flacky coverage in cookiestore cleanup tests (#2257)

Refactor and add more coverage to test flacky coverage in case sessions
which are already deleted are flagged as expired/for deletion.

See coverage drop in pkg/api/cookiestore.go:
8e68255946/indirect-changes

Signed-off-by: Andrei Aaron <aaaron@luxoft.com>

pkg/api/authn_test.go

@@ -8,6 +8,7 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
+	"io/fs"
 	"net/http"
 	"net/http/httptest"
 	"os"
@@ -1010,6 +1011,77 @@ func TestCookiestoreCleanup(t *testing.T) {
 
 		taskScheduler.Shutdown()
 	})
+
+	Convey("Test session expiration checks", t, func() {
+		rootDir := t.TempDir()
+
+		err := os.MkdirAll(path.Join(rootDir, "_sessions"), storageConstants.DefaultDirPerms)
+		So(err, ShouldBeNil)
+
+		sessionPath := path.Join(rootDir, "_sessions", "session_1234")
+
+		err = os.WriteFile(sessionPath, []byte("session"), storageConstants.DefaultFilePerms)
+		So(err, ShouldBeNil)
+
+		Convey("New session file should not be expired", func() {
+			fileInfo, err := os.Stat(sessionPath)
+			So(err, ShouldBeNil)
+
+			dirEntry := fs.FileInfoToDirEntry(fileInfo)
+			So(api.IsExpiredSession(dirEntry), ShouldBeFalse)
+		})
+
+		Convey("Deleted session file should not flagged as expired", func() {
+			fileInfo, err := os.Stat(sessionPath)
+			So(err, ShouldBeNil)
+
+			err = os.Remove(sessionPath)
+			So(err, ShouldBeNil)
+
+			dirEntry := fs.FileInfoToDirEntry(fileInfo)
+			So(api.IsExpiredSession(dirEntry), ShouldBeFalse)
+		})
+
+		// Fix flaky coverage in integration tests
+		Convey("Error on dirEntry.Info()", func() {
+			fileInfo, err := os.Stat(sessionPath)
+			So(err, ShouldBeNil)
+
+			dirEntry := badDirInfo{fileInfo: fileInfo}
+
+			So(api.IsExpiredSession(dirEntry), ShouldBeFalse)
+		})
+
+		Convey("File with invalid name should not be expired", func() {
+			newSessionPath := path.Join(rootDir, "_sessions", "1234")
+			err := os.Rename(sessionPath, newSessionPath)
+			So(err, ShouldBeNil)
+
+			changeTime := time.Now().Add(-4 * time.Hour)
+
+			err = os.Chtimes(newSessionPath, changeTime, changeTime)
+			So(err, ShouldBeNil)
+
+			fileInfo, err := os.Stat(newSessionPath)
+			So(err, ShouldBeNil)
+
+			dirEntry := fs.FileInfoToDirEntry(fileInfo)
+			So(api.IsExpiredSession(dirEntry), ShouldBeFalse)
+		})
+
+		Convey("Old session file should be expired", func() {
+			changeTime := time.Now().Add(-4 * time.Hour)
+
+			err = os.Chtimes(sessionPath, changeTime, changeTime)
+			So(err, ShouldBeNil)
+
+			fileInfo, err := os.Stat(sessionPath)
+			So(err, ShouldBeNil)
+
+			dirEntry := fs.FileInfoToDirEntry(fileInfo)
+			So(api.IsExpiredSession(dirEntry), ShouldBeTrue)
+		})
+	})
 }
 
 type mockUUIDGenerator struct {
@@ -1037,3 +1109,27 @@ type errReader int
 func (errReader) Read(p []byte) (int, error) {
 	return 0, fmt.Errorf("test error") //nolint:goerr113
 }
+
+type badDirInfo struct {
+	fileInfo fs.FileInfo
+}
+
+func (di badDirInfo) IsDir() bool {
+	return di.fileInfo.IsDir()
+}
+
+func (di badDirInfo) Type() fs.FileMode {
+	return di.fileInfo.Mode().Type()
+}
+
+func (di badDirInfo) Info() (fs.FileInfo, error) {
+	return di.fileInfo, ErrUnexpectedError
+}
+
+func (di badDirInfo) Name() string {
+	return di.fileInfo.Name()
+}
+
+func (di badDirInfo) String() string {
+	return fs.FormatDirEntry(di)
+}

pkg/api/cookiestore.go

@@ -90,25 +90,33 @@ func getHashKey() ([]byte, error) {
 	return hashKey, nil
 }
 
+func IsExpiredSession(dirEntry fs.DirEntry) bool {
+	fileInfo, err := dirEntry.Info()
+	if err != nil { // may have been deleted in the meantime
+		return false
+	}
+
+	if !strings.HasPrefix(fileInfo.Name(), "session_") {
+		return false
+	}
+
+	if fileInfo.ModTime().Add(cookiesMaxAge * time.Second).After(time.Now()) {
+		return false
+	}
+
+	return true
+}
+
 func getExpiredSessions(dir string) ([]string, error) {
 	sessions := make([]string, 0)
 
-	err := filepath.WalkDir(dir, func(_ string, dirEntry fs.DirEntry, err error) error {
+	err := filepath.WalkDir(dir, func(filePath string, dirEntry fs.DirEntry, err error) error {
 		if err != nil {
 			return err
 		}
 
-		fileInfo, err := dirEntry.Info()
+		if IsExpiredSession(dirEntry) {
-		if err != nil { // may have been deleted in the meantime
+			sessions = append(sessions, filePath)
-			return nil //nolint: nilerr
-		}
-
-		if !strings.HasPrefix(fileInfo.Name(), "session_") {
-			return nil
-		}
-
-		if fileInfo.ModTime().Add(cookiesMaxAge * time.Second).Before(time.Now()) {
-			sessions = append(sessions, path.Join(dir, fileInfo.Name()))
 		}
 
 		return nil