diff --git a/internal/cache/db.go b/internal/cache/db.go index f15cb1d93..73db67f0d 100644 --- a/internal/cache/db.go +++ b/internal/cache/db.go @@ -1,3 +1,4 @@ +// Package cache handles transaction with an underlying database to cache user and group informations. package cache import ( @@ -109,7 +110,7 @@ func New(cacheDir string) (cache *Cache, err error) { break } - if errors.Is(err, errRetryDB{}) { + if errors.Is(err, shouldRetryDBError{}) { if i == 3 { return nil, errors.Unwrap(err) } @@ -171,7 +172,7 @@ func openAndInitDB(path, dirtyFlagPath string) (*bbolt.DB, error) { if err != nil { if errors.Is(err, bbolt.ErrInvalid) { clearDatabase(path, dirtyFlagPath) - return nil, errRetryDB{err: err} + return nil, shouldRetryDBError{err: err} } return nil, fmt.Errorf("can't open database file: %v", err) } @@ -197,13 +198,16 @@ func openAndInitDB(path, dirtyFlagPath string) (*bbolt.DB, error) { // Clear up any unknown buckets var bucketNamesToDelete [][]byte - tx.ForEach(func(name []byte, b *bbolt.Bucket) error { + err = tx.ForEach(func(name []byte, b *bbolt.Bucket) error { if slices.Contains(allBucketsNames, string(name)) { return nil } bucketNamesToDelete = append(bucketNamesToDelete, name) return nil }) + if err != nil { + return err + } for _, bucketName := range bucketNamesToDelete { // We are in a RW transaction. _ = tx.DeleteBucket(bucketName) @@ -295,7 +299,7 @@ func getFromBucket[T any, K int | string](bucket bucketWithName, key K) (T, erro data := bucket.Get(k) if data == nil { - return r, ErrNoDataFound{key: string(k), bucketName: bucket.name} + return r, NoDataFoundError{key: string(k), bucketName: bucket.name} } if err := json.Unmarshal(data, &r); err != nil { @@ -305,34 +309,34 @@ func getFromBucket[T any, K int | string](bucket bucketWithName, key K) (T, erro return r, nil } -// ErrNoDataFound is returned when we didn’t find a matching entry. -type ErrNoDataFound struct { +// NoDataFoundError is returned when we didn’t find a matching entry. +type NoDataFoundError struct { key string bucketName string } // Error implements the error interface to return key/bucket name. -func (err ErrNoDataFound) Error() string { +func (err NoDataFoundError) Error() string { return fmt.Sprintf("no result matching %v in %v", err.key, err.bucketName) } // Is makes this error insensitive to the key and bucket name. -func (ErrNoDataFound) Is(target error) bool { return target == ErrNoDataFound{} } +func (NoDataFoundError) Is(target error) bool { return target == NoDataFoundError{} } -// errRetryDB is returned when we want to retry opening the database. -type errRetryDB struct { +// shouldRetryDBError is returned when we want to retry opening the database. +type shouldRetryDBError struct { err error } // Error implements the error interface. -func (err errRetryDB) Error() string { +func (err shouldRetryDBError) Error() string { return "ErrRetryDB" } // Unwrap allows to unwrap original error. -func (err errRetryDB) Unwrap() error { +func (err shouldRetryDBError) Unwrap() error { return err.err } // Is makes this error insensitive to the key and bucket name. -func (errRetryDB) Is(target error) bool { return target == errRetryDB{} } +func (shouldRetryDBError) Is(target error) bool { return target == shouldRetryDBError{} } diff --git a/internal/cache/db_test.go b/internal/cache/db_test.go index 6094c7bbd..2486ca1d0 100644 --- a/internal/cache/db_test.go +++ b/internal/cache/db_test.go @@ -1,6 +1,7 @@ package cache_test import ( + "errors" "io" "io/fs" "os" @@ -248,9 +249,8 @@ func TestUpdateFromUserInfo(t *testing.T) { requireClearedDatabase(t, c) } return - } else { - require.NoError(t, err) } + require.NoError(t, err) requireNoDirtyFileInDir(t, cacheDir) @@ -259,7 +259,6 @@ func TestUpdateFromUserInfo(t *testing.T) { want := testutils.LoadWithUpdateFromGolden(t, got) require.Equal(t, want, got, "Did not get expected database content") - }) } } @@ -274,7 +273,7 @@ func TestUserByID(t *testing.T) { }{ "Get existing user": {dbFile: "one_user_and_group"}, - "Error on missing user": {wantErrType: cache.ErrNoDataFound{}}, + "Error on missing user": {wantErrType: cache.NoDataFoundError{}}, "Error on invalid database entry": {dbFile: "invalid_entry_in_userByID", wantErrType: shouldError{}}, } for name, tc := range tests { @@ -300,7 +299,7 @@ func TestUserByName(t *testing.T) { }{ "Get existing user": {dbFile: "one_user_and_group"}, - "Error on missing user": {wantErrType: cache.ErrNoDataFound{}}, + "Error on missing user": {wantErrType: cache.NoDataFoundError{}}, "Error on invalid database entry": {dbFile: "invalid_entry_in_userByName", wantErrType: shouldError{}}, } for name, tc := range tests { @@ -354,7 +353,7 @@ func TestGroupByID(t *testing.T) { }{ "Get existing group": {dbFile: "one_user_and_group"}, - "Error on missing group": {wantErrType: cache.ErrNoDataFound{}}, + "Error on missing group": {wantErrType: cache.NoDataFoundError{}}, "Error on invalid database entry": {dbFile: "invalid_entry_in_groupByID", wantErrType: shouldError{}}, "Error as missing userByID": {dbFile: "partially_valid_multiple_users_and_groups_groupByID_groupToUsers", wantErrType: shouldError{}}, } @@ -381,7 +380,7 @@ func TestGroupByName(t *testing.T) { }{ "Get existing group": {dbFile: "one_user_and_group"}, - "Error on missing group": {wantErrType: cache.ErrNoDataFound{}}, + "Error on missing group": {wantErrType: cache.NoDataFoundError{}}, "Error on invalid database entry": {dbFile: "invalid_entry_in_groupByName", wantErrType: shouldError{}}, "Error as missing userByID": {dbFile: "partially_valid_multiple_users_and_groups_groupByID_groupToUsers", wantErrType: shouldError{}}, } @@ -459,7 +458,7 @@ UserToGroups: {} got, err := dumpToYaml(c) require.NoError(t, err, "Created database should be valid yaml content") - require.Equal(t, want, string(got), "Database should only have empty buckets") + require.Equal(t, want, got, "Database should only have empty buckets") } //go:linkname dumpToYaml github.com/ubuntu/authd/internal/cache.(*Cache).dumpToYaml @@ -498,7 +497,7 @@ func requireGetAssertions[E any](t *testing.T, got E, wantErrType, err error, c t.Helper() if wantErrType != nil { - if (wantErrType == cache.ErrNoDataFound{}) { + if (errors.Is(wantErrType, cache.NoDataFoundError{})) { require.ErrorIs(t, err, wantErrType, "Should return no data found") return } diff --git a/internal/cache/export_test.go b/internal/cache/export_test.go index 6192ce9e4..45203c050 100644 --- a/internal/cache/export_test.go +++ b/internal/cache/export_test.go @@ -1,9 +1,9 @@ package cache const ( - // DbName is dbName exported for tests + // DbName is dbName exported for tests. DbName = dbName - // DirtyFlagDbName is dirtyFlagDbName exported for tests + // DirtyFlagDbName is dirtyFlagDbName exported for tests. DirtyFlagDbName = dirtyFlagDbName ) diff --git a/internal/cache/getgroups.go b/internal/cache/getgroups.go index 6d5988b65..0a7651264 100644 --- a/internal/cache/getgroups.go +++ b/internal/cache/getgroups.go @@ -87,8 +87,8 @@ func getGroup[K int | string](c *Cache, bucketName string, key K) (Group, error) // Get id and name of the group. g, err := getFromBucket[groupDB](buckets[bucketName], key) if err != nil { - // no entry is valid, no need to clear the database but return the error. - if !errors.Is(err, ErrNoDataFound{}) { + // no entry is valid, no need to clean the database but return the error. + if !errors.Is(err, NoDataFoundError{}) { c.requestClearDatabase() } return err diff --git a/internal/cache/getusers.go b/internal/cache/getusers.go index a8c225baf..bed1982e0 100644 --- a/internal/cache/getusers.go +++ b/internal/cache/getusers.go @@ -83,7 +83,7 @@ func getUser[K int | string](c *Cache, bucketName string, key K) (u userDB, err u, err = getFromBucket[userDB](bucket, key) if err != nil { - if !errors.Is(err, ErrNoDataFound{}) { + if !errors.Is(err, NoDataFoundError{}) { c.requestClearDatabase() } return err diff --git a/internal/cache/serialization.go b/internal/cache/serialization.go index 923e0504a..279af5450 100644 --- a/internal/cache/serialization.go +++ b/internal/cache/serialization.go @@ -11,6 +11,7 @@ import ( "gopkg.in/yaml.v3" ) +//nolint:unused // This is used for tests, with methods that are using go linking. Not part of exported API. var redactedTimes = map[string]string{ "AAAAATIME": "2004-10-20T11:06:23Z", "BBBBBTIME": "2006-06-01T10:08:04Z", @@ -21,6 +22,8 @@ var redactedTimes = map[string]string{ } // redactTime replace current time by a redacted version. +// +//nolint:unused // This is used for tests, with methods that are using go linking. Not part of exported API. func redactTime(line string) string { re := regexp.MustCompile(`"LastLogin":"(.*?)"`) match := re.FindSubmatch([]byte(line)) @@ -51,6 +54,8 @@ func redactTime(line string) string { } // dumpToYaml deserializes the cache database to a writer in a yaml format. +// +//nolint:unused // This is used for tests, with go linking. Not part of exported API. func (c *Cache) dumpToYaml() (string, error) { d := make(map[string]map[string]string) @@ -75,6 +80,8 @@ func (c *Cache) dumpToYaml() (string, error) { } // dbfromYAML loads a yaml formatted of the buckets and dump it into destDir, with its dbname. +// +//nolint:unused // This is used for tests, with go linking. Not part of exported API. func dbfromYAML(r io.Reader, destDir string) error { dbPath := filepath.Join(destDir, dbName) db, err := bbolt.Open(dbPath, 0600, nil) diff --git a/internal/cache/update.go b/internal/cache/update.go index ddf5b2d01..64db28bf9 100644 --- a/internal/cache/update.go +++ b/internal/cache/update.go @@ -53,7 +53,7 @@ func (c *Cache) UpdateFromUserInfo(u UserInfo) error { previousGroupsForCurrentUser, err := getFromBucket[userToGroupsDB](buckets[userToGroupsBucketName], userDB.UID) // No data is valid and means this is the first insertion. - if err != nil && !errors.Is(err, ErrNoDataFound{}) { + if err != nil && !errors.Is(err, NoDataFoundError{}) { c.requestClearDatabase() return err } @@ -99,7 +99,7 @@ func (c *Cache) UpdateFromUserInfo(u UserInfo) error { // updateUser updates both user buckets with userContent. It handles any potential login rename. func updateUser(buckets map[string]bucketWithName, userContent userDB) { existingUser, err := getFromBucket[userDB](buckets[userByIDBucketName], userContent.UID) - if err != nil && !errors.Is(err, ErrNoDataFound{}) { + if err != nil && !errors.Is(err, NoDataFoundError{}) { slog.Warn(fmt.Sprintf("Could not fetch previous record for user %v: %v", userContent.UID, err)) } @@ -117,7 +117,7 @@ func updateUser(buckets map[string]bucketWithName, userContent userDB) { func updateGroups(buckets map[string]bucketWithName, groupContents []groupDB) { for _, groupContent := range groupContents { existingGroup, err := getFromBucket[groupDB](buckets[groupByIDBucketName], groupContent.GID) - if err != nil && !errors.Is(err, ErrNoDataFound{}) { + if err != nil && !errors.Is(err, NoDataFoundError{}) { slog.Warn(fmt.Sprintf("Could not fetch previous record for group %v: %v", groupContent.GID, err)) } @@ -140,7 +140,7 @@ func updateUsersAndGroups(buckets map[string]bucketWithName, uid int, groupConte currentGIDs = append(currentGIDs, groupContent.GID) grpToUsers, err := getFromBucket[groupToUsersDB](buckets[groupToUsersBucketName], groupContent.GID) // No data is valid and means that this is the first time we record it. - if err != nil && !errors.Is(err, ErrNoDataFound{}) { + if err != nil && !errors.Is(err, NoDataFoundError{}) { return err } @@ -159,7 +159,7 @@ func updateUsersAndGroups(buckets map[string]bucketWithName, uid int, groupConte } grpToUsers, err := getFromBucket[groupToUsersDB](buckets[groupToUsersBucketName], previousGID) // No data means the database was corrupted but we can forgive this (exist in previous user gid but not on system). - if err != nil && !errors.Is(err, ErrNoDataFound{}) { + if err != nil && !errors.Is(err, NoDataFoundError{}) { return err } @@ -182,7 +182,6 @@ func updateUsersAndGroups(buckets map[string]bucketWithName, uid int, groupConte _ = buckets[groupByIDBucketName].Delete([]byte(strconv.Itoa(previousGID))) _ = buckets[groupByNameBucketName].Delete([]byte(group.Name)) _ = buckets[groupToUsersBucketName].Delete([]byte(strconv.Itoa(previousGID))) - } return nil