From ae0b270c2c23ee02a31236e586466b9b387dab25 Mon Sep 17 00:00:00 2001 From: Ben McClelland Date: Sat, 7 Oct 2023 11:49:54 -0700 Subject: [PATCH] feat: move local iam cache to a more generic cache mechanism The local IAM accounts were being cached in memory for improved performance, but this can be moved up a layer so that the cache can benefit any configured IAM service. This adds options to disable and tune TTL for cache. The balance for the TTL is that a longer life will send requests to the IAM service less frequently, but could be out of date with the service accounts for that duration. --- auth/iam.go | 24 +++- auth/iam_cache.go | 179 ++++++++++++++++++++++++++++++ auth/iam_internal.go | 5 + auth/iam_ldap.go | 5 + auth/iam_single.go | 7 ++ cmd/versitygw/main.go | 39 ++++++- s3api/controllers/iam_moq_test.go | 37 ++++++ 7 files changed, 290 insertions(+), 6 deletions(-) create mode 100644 auth/iam_cache.go diff --git a/auth/iam.go b/auth/iam.go index c7952ef2..8ab83d6b 100644 --- a/auth/iam.go +++ b/auth/iam.go @@ -16,6 +16,7 @@ package auth import ( "errors" + "time" ) // Account is a gateway IAM account @@ -33,6 +34,7 @@ type IAMService interface { GetUserAccount(access string) (Account, error) DeleteUserAccount(access string) error ListUserAccounts() ([]Account, error) + Shutdown() error } var ErrNoSuchUser = errors.New("user not found") @@ -47,18 +49,36 @@ type Opts struct { LDAPAccessAtr string LDAPSecretAtr string LDAPRoleAtr string + CacheDisable bool + CacheTTL int + CachePrune int } func New(o *Opts) (IAMService, error) { + var svc IAMService + var err error + switch { case o.Dir != "": - return NewInternal(o.Dir) + svc, err = NewInternal(o.Dir) case o.LDAPServerURL != "": - return NewLDAPService(o.LDAPServerURL, o.LDAPBindDN, o.LDAPPassword, + svc, err = NewLDAPService(o.LDAPServerURL, o.LDAPBindDN, o.LDAPPassword, o.LDAPQueryBase, o.LDAPAccessAtr, o.LDAPSecretAtr, o.LDAPRoleAtr, o.LDAPObjClasses) default: // if no iam options selected, default to the single user mode return IAMServiceSingle{}, nil } + + if err != nil { + return nil, err + } + + if o.CacheDisable { + return svc, nil + } + + return NewCache(svc, + time.Duration(o.CacheTTL)*time.Second, + time.Duration(o.CachePrune)*time.Second), nil } diff --git a/auth/iam_cache.go b/auth/iam_cache.go new file mode 100644 index 00000000..5de410c1 --- /dev/null +++ b/auth/iam_cache.go @@ -0,0 +1,179 @@ +// Copyright 2023 Versity Software +// This file is licensed under the Apache License, Version 2.0 +// (the "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package auth + +import ( + "context" + "strings" + "sync" + "time" +) + +// IAMCache is an in memory cache of the IAM accounts +// with expiration. This helps to alleviate the load on +// the real IAM service if the gateway is handling +// many requests. This forwards account updates to the +// underlying service, and returns cached results while +// the in memory account is not expired. +type IAMCache struct { + service IAMService + iamcache *icache + cancel context.CancelFunc +} + +var _ IAMService = &IAMCache{} + +type item struct { + value Account + exp time.Time +} + +type icache struct { + sync.RWMutex + expire time.Duration + items map[string]item +} + +func (i *icache) set(k string, v Account) { + cpy := v + i.Lock() + i.items[k] = item{ + exp: time.Now().Add(i.expire), + value: cpy, + } + i.Unlock() +} + +func (i *icache) get(k string) (Account, bool) { + i.RLock() + v, ok := i.items[k] + i.RUnlock() + if !ok || !v.exp.After(time.Now()) { + return Account{}, false + } + return v.value, true +} + +func (i *icache) Delete(k string) { + i.Lock() + delete(i.items, k) + i.Unlock() +} + +func (i *icache) gcCache(ctx context.Context, interval time.Duration) { + for { + if ctx.Err() != nil { + break + } + + now := time.Now() + + i.Lock() + // prune expired entries + for k, v := range i.items { + if now.After(v.exp) { + delete(i.items, k) + } + } + i.Unlock() + + // sleep for the clean interval or context cancelation, + // whichever comes first + select { + case <-ctx.Done(): + case <-time.After(interval): + } + } +} + +// NewCache initializes an IAM cache for the provided service. The expireTime +// is the duration a cache entry can be valid, and the cleanupInterval is +// how often to scan cache and cleanup expired entries. +func NewCache(service IAMService, expireTime, cleanupInterval time.Duration) *IAMCache { + i := &IAMCache{ + service: service, + iamcache: &icache{ + items: make(map[string]item), + expire: expireTime, + }, + } + + ctx, cancel := context.WithCancel(context.Background()) + go i.iamcache.gcCache(ctx, cleanupInterval) + i.cancel = cancel + + return i +} + +// CreateAccount send create to IAM service and creates an account cache entry +func (c *IAMCache) CreateAccount(account Account) error { + err := c.service.CreateAccount(account) + if err != nil { + return err + } + + // we need a copy of account to be able to store beyond the + // lifetime of the request, otherwise Fiber will reuse and corrupt + // these entries + acct := Account{ + Access: strings.Clone(account.Access), + Secret: strings.Clone(account.Secret), + Role: strings.Clone(account.Role), + } + + c.iamcache.set(acct.Access, acct) + return nil +} + +// GetUserAccount retrieves the cache account if it is in the cache and not +// expired. Otherwise retrieves from underlying IAM service and caches +// result for the expire duration. +func (c *IAMCache) GetUserAccount(access string) (Account, error) { + acct, found := c.iamcache.get(access) + if found { + return acct, nil + } + + a, err := c.service.GetUserAccount(access) + if err != nil { + return Account{}, err + } + + c.iamcache.set(access, a) + return a, nil +} + +// DeleteUserAccount deletes account from IAM service and cache +func (c *IAMCache) DeleteUserAccount(access string) error { + err := c.service.DeleteUserAccount(access) + if err != nil { + return err + } + + c.iamcache.Delete(access) + return nil +} + +// ListUserAccounts is a passthrough to the underlying service and +// does not make use of the cache +func (c *IAMCache) ListUserAccounts() ([]Account, error) { + return c.service.ListUserAccounts() +} + +// Shutdown graceful termination of service +func (c *IAMCache) Shutdown() error { + c.cancel() + return nil +} diff --git a/auth/iam_internal.go b/auth/iam_internal.go index 6b322a9e..0358bae8 100644 --- a/auth/iam_internal.go +++ b/auth/iam_internal.go @@ -144,6 +144,11 @@ func (s *IAMServiceInternal) ListUserAccounts() ([]Account, error) { return accs, nil } +// Shutdown graceful termination of service +func (s *IAMServiceInternal) Shutdown() error { + return nil +} + const ( iamMode = 0600 ) diff --git a/auth/iam_ldap.go b/auth/iam_ldap.go index 38628a66..0f13e3c7 100644 --- a/auth/iam_ldap.go +++ b/auth/iam_ldap.go @@ -126,3 +126,8 @@ func (ld *LdapIAMService) ListUserAccounts() ([]Account, error) { return result, nil } + +// Shutdown graceful termination of service +func (ld *LdapIAMService) Shutdown() error { + return ld.conn.Close() +} diff --git a/auth/iam_single.go b/auth/iam_single.go index 7b033866..cef1de44 100644 --- a/auth/iam_single.go +++ b/auth/iam_single.go @@ -19,6 +19,8 @@ import "fmt" // IAMServiceSingle manages the single tenant (root-only) IAM service type IAMServiceSingle struct{} +var _ IAMService = &IAMServiceSingle{} + // CreateAccount not valid in single tenant mode func (IAMServiceSingle) CreateAccount(account Account) error { return fmt.Errorf("create user not valid in single tenant mode") @@ -38,3 +40,8 @@ func (IAMServiceSingle) DeleteUserAccount(access string) error { func (IAMServiceSingle) ListUserAccounts() ([]Account, error) { return []Account{}, nil } + +// Shutdown graceful termination of service +func (IAMServiceSingle) Shutdown() error { + return nil +} diff --git a/cmd/versitygw/main.go b/cmd/versitygw/main.go index e23d0ec4..8183dc85 100644 --- a/cmd/versitygw/main.go +++ b/cmd/versitygw/main.go @@ -48,6 +48,9 @@ var ( ldapURL, ldapBindDN, ldapPassword string ldapQueryBase, ldapObjClasses string ldapAccessAtr, ldapSecAtr, ldapRoleAtr string + iamCacheDisable bool + iamCacheTTL int + iamCachePrune int ) var ( @@ -256,6 +259,23 @@ func initFlags() []cli.Flag { Usage: "ldap server user role attribute name", Destination: &ldapRoleAtr, }, + &cli.BoolFlag{ + Name: "iam-cache-disable", + Usage: "disable local iam cache", + Destination: &iamCacheDisable, + }, + &cli.IntFlag{ + Name: "iam-cache-ttl", + Usage: "local iam cache entry ttl (seconds)", + Value: 120, + Destination: &iamCacheTTL, + }, + &cli.IntFlag{ + Name: "iam-cache-prune", + Usage: "local iam cache cleanup interval (seconds)", + Value: 3600, + Destination: &iamCachePrune, + }, } } @@ -328,6 +348,9 @@ func runGateway(ctx *cli.Context, be backend.Backend) error { LDAPAccessAtr: ldapAccessAtr, LDAPSecretAtr: ldapSecAtr, LDAPRoleAtr: ldapRoleAtr, + CacheDisable: iamCacheDisable, + CacheTTL: iamCacheTTL, + CachePrune: iamCachePrune, }) if err != nil { return fmt.Errorf("setup iam: %w", err) @@ -387,13 +410,21 @@ Loop: } } } + saveErr := err be.Shutdown() + + err = iam.Shutdown() + if err != nil { + fmt.Fprintf(os.Stderr, "shutdown iam: %v\n", err) + } + if logger != nil { - lerr := logger.Shutdown() - if lerr != nil { - fmt.Fprintf(os.Stderr, "shutdown logger: %v\n", lerr) + err := logger.Shutdown() + if err != nil { + fmt.Fprintf(os.Stderr, "shutdown logger: %v\n", err) } } - return err + + return saveErr } diff --git a/s3api/controllers/iam_moq_test.go b/s3api/controllers/iam_moq_test.go index 686bb881..73499473 100644 --- a/s3api/controllers/iam_moq_test.go +++ b/s3api/controllers/iam_moq_test.go @@ -30,6 +30,9 @@ var _ auth.IAMService = &IAMServiceMock{} // ListUserAccountsFunc: func() ([]auth.Account, error) { // panic("mock out the ListUserAccounts method") // }, +// ShutdownFunc: func() error { +// panic("mock out the Shutdown method") +// }, // } // // // use mockedIAMService in code that requires auth.IAMService @@ -49,6 +52,9 @@ type IAMServiceMock struct { // ListUserAccountsFunc mocks the ListUserAccounts method. ListUserAccountsFunc func() ([]auth.Account, error) + // ShutdownFunc mocks the Shutdown method. + ShutdownFunc func() error + // calls tracks calls to the methods. calls struct { // CreateAccount holds details about calls to the CreateAccount method. @@ -69,11 +75,15 @@ type IAMServiceMock struct { // ListUserAccounts holds details about calls to the ListUserAccounts method. ListUserAccounts []struct { } + // Shutdown holds details about calls to the Shutdown method. + Shutdown []struct { + } } lockCreateAccount sync.RWMutex lockDeleteUserAccount sync.RWMutex lockGetUserAccount sync.RWMutex lockListUserAccounts sync.RWMutex + lockShutdown sync.RWMutex } // CreateAccount calls CreateAccountFunc. @@ -198,3 +208,30 @@ func (mock *IAMServiceMock) ListUserAccountsCalls() []struct { mock.lockListUserAccounts.RUnlock() return calls } + +// Shutdown calls ShutdownFunc. +func (mock *IAMServiceMock) Shutdown() error { + if mock.ShutdownFunc == nil { + panic("IAMServiceMock.ShutdownFunc: method is nil but IAMService.Shutdown was just called") + } + callInfo := struct { + }{} + mock.lockShutdown.Lock() + mock.calls.Shutdown = append(mock.calls.Shutdown, callInfo) + mock.lockShutdown.Unlock() + return mock.ShutdownFunc() +} + +// ShutdownCalls gets all the calls that were made to Shutdown. +// Check the length with: +// +// len(mockedIAMService.ShutdownCalls()) +func (mock *IAMServiceMock) ShutdownCalls() []struct { +} { + var calls []struct { + } + mock.lockShutdown.RLock() + calls = mock.calls.Shutdown + mock.lockShutdown.RUnlock() + return calls +}