diff --git a/changelog/unreleased/fix-create-home.md b/changelog/unreleased/fix-create-home.md new file mode 100644 index 00000000000..e51a0482d21 --- /dev/null +++ b/changelog/unreleased/fix-create-home.md @@ -0,0 +1,6 @@ +Bugfix: Fix CreateHome cache + +Move the CreateHome middleware cache to the proxy. + +https://github.com/owncloud/ocis/pull/12128 +https://github.com/owncloud/reva/pull/562 diff --git a/services/gateway/pkg/config/config.go b/services/gateway/pkg/config/config.go index 53699fccce0..92f44c19f4e 100644 --- a/services/gateway/pkg/config/config.go +++ b/services/gateway/pkg/config/config.go @@ -85,18 +85,11 @@ type StorageRegistry struct { // Cache holds cache config type Cache struct { - ProviderCacheStore string `yaml:"provider_cache_store" env:"OCIS_CACHE_STORE;GATEWAY_PROVIDER_CACHE_STORE" desc:"The type of the cache store. Supported values are: 'memory', 'redis-sentinel', 'nats-js-kv', 'noop'. See the text description for details." introductionVersion:"pre5.0"` - ProviderCacheNodes []string `yaml:"provider_cache_nodes" env:"OCIS_CACHE_STORE_NODES;GATEWAY_PROVIDER_CACHE_STORE_NODES" desc:"A list of nodes to access the configured store. This has no effect when 'memory' store is configured. Note that the behaviour how nodes are used is dependent on the library of the configured store. See the Environment Variable Types description for more details." introductionVersion:"pre5.0"` - ProviderCacheDatabase string `yaml:"provider_cache_database" env:"OCIS_CACHE_DATABASE" desc:"The database name the configured store should use." introductionVersion:"pre5.0"` - ProviderCacheTTL time.Duration `yaml:"provider_cache_ttl" env:"OCIS_CACHE_TTL;GATEWAY_PROVIDER_CACHE_TTL" desc:"Default time to live for user info in the cache. Only applied when access tokens has no expiration. See the Environment Variable Types description for more details." introductionVersion:"pre5.0"` - ProviderCacheDisablePersistence bool `yaml:"provider_cache_disable_persistence" env:"OCIS_CACHE_DISABLE_PERSISTENCE;GATEWAY_PROVIDER_CACHE_DISABLE_PERSISTENCE" desc:"Disables persistence of the provider cache. Only applies when store type 'nats-js-kv' is configured. Defaults to false." introductionVersion:"5.0"` - ProviderCacheAuthUsername string `yaml:"provider_cache_auth_username" env:"OCIS_CACHE_AUTH_USERNAME;GATEWAY_PROVIDER_CACHE_AUTH_USERNAME" desc:"The username to use for authentication. Only applies when store type 'nats-js-kv' is configured." introductionVersion:"5.0"` - ProviderCacheAuthPassword string `yaml:"provider_cache_auth_password" env:"OCIS_CACHE_AUTH_PASSWORD;GATEWAY_PROVIDER_CACHE_AUTH_PASSWORD" desc:"The password to use for authentication. Only applies when store type 'nats-js-kv' is configured." introductionVersion:"5.0"` - CreateHomeCacheStore string `yaml:"create_home_cache_store" env:"OCIS_CACHE_STORE;GATEWAY_CREATE_HOME_CACHE_STORE" desc:"The type of the cache store. Supported values are: 'memory', 'redis-sentinel', 'nats-js-kv', 'noop'. See the text description for details." introductionVersion:"pre5.0"` - CreateHomeCacheNodes []string `yaml:"create_home_cache_nodes" env:"OCIS_CACHE_STORE_NODES;GATEWAY_CREATE_HOME_CACHE_STORE_NODES" desc:"A list of nodes to access the configured store. This has no effect when 'memory' store is configured. Note that the behaviour how nodes are used is dependent on the library of the configured store. See the Environment Variable Types description for more details." introductionVersion:"pre5.0"` - CreateHomeCacheDatabase string `yaml:"create_home_cache_database" env:"OCIS_CACHE_DATABASE" desc:"The database name the configured store should use." introductionVersion:"pre5.0"` - CreateHomeCacheTTL time.Duration `yaml:"create_home_cache_ttl" env:"OCIS_CACHE_TTL;GATEWAY_CREATE_HOME_CACHE_TTL" desc:"Default time to live for user info in the cache. Only applied when access tokens has no expiration. See the Environment Variable Types description for more details." introductionVersion:"pre5.0"` - CreateHomeCacheDisablePersistence bool `yaml:"create_home_cache_disable_persistence" env:"OCIS_CACHE_DISABLE_PERSISTENCE;GATEWAY_CREATE_HOME_CACHE_DISABLE_PERSISTENCE" desc:"Disables persistence of the create home cache. Only applies when store type 'nats-js-kv' is configured. Defaults to false." introductionVersion:"5.0"` - CreateHomeCacheAuthUsername string `yaml:"create_home_cache_auth_username" env:"OCIS_CACHE_AUTH_USERNAME;GATEWAY_CREATE_HOME_CACHE_AUTH_USERNAME" desc:"The username to use for authentication. Only applies when store type 'nats-js-kv' is configured." introductionVersion:"5.0"` - CreateHomeCacheAuthPassword string `yaml:"create_home_cache_auth_password" env:"OCIS_CACHE_AUTH_PASSWORD;GATEWAY_CREATE_HOME_CACHE_AUTH_PASSWORD" desc:"The password to use for authentication. Only applies when store type 'nats-js-kv' is configured." introductionVersion:"5.0"` + ProviderCacheStore string `yaml:"provider_cache_store" env:"OCIS_CACHE_STORE;GATEWAY_PROVIDER_CACHE_STORE" desc:"The type of the cache store. Supported values are: 'memory', 'redis-sentinel', 'nats-js-kv', 'noop'. See the text description for details." introductionVersion:"pre5.0"` + ProviderCacheNodes []string `yaml:"provider_cache_nodes" env:"OCIS_CACHE_STORE_NODES;GATEWAY_PROVIDER_CACHE_STORE_NODES" desc:"A list of nodes to access the configured store. This has no effect when 'memory' store is configured. Note that the behaviour how nodes are used is dependent on the library of the configured store. See the Environment Variable Types description for more details." introductionVersion:"pre5.0"` + ProviderCacheDatabase string `yaml:"provider_cache_database" env:"OCIS_CACHE_DATABASE" desc:"The database name the configured store should use." introductionVersion:"pre5.0"` + ProviderCacheTTL time.Duration `yaml:"provider_cache_ttl" env:"OCIS_CACHE_TTL;GATEWAY_PROVIDER_CACHE_TTL" desc:"Default time to live for user info in the cache. Only applied when access tokens has no expiration. See the Environment Variable Types description for more details." introductionVersion:"pre5.0"` + ProviderCacheDisablePersistence bool `yaml:"provider_cache_disable_persistence" env:"OCIS_CACHE_DISABLE_PERSISTENCE;GATEWAY_PROVIDER_CACHE_DISABLE_PERSISTENCE" desc:"Disables persistence of the provider cache. Only applies when store type 'nats-js-kv' is configured. Defaults to false." introductionVersion:"5.0"` + ProviderCacheAuthUsername string `yaml:"provider_cache_auth_username" env:"OCIS_CACHE_AUTH_USERNAME;GATEWAY_PROVIDER_CACHE_AUTH_USERNAME" desc:"The username to use for authentication. Only applies when store type 'nats-js-kv' is configured." introductionVersion:"5.0"` + ProviderCacheAuthPassword string `yaml:"provider_cache_auth_password" env:"OCIS_CACHE_AUTH_PASSWORD;GATEWAY_PROVIDER_CACHE_AUTH_PASSWORD" desc:"The password to use for authentication. Only applies when store type 'nats-js-kv' is configured." introductionVersion:"5.0"` } diff --git a/services/gateway/pkg/config/defaults/defaultconfig.go b/services/gateway/pkg/config/defaults/defaultconfig.go index 5d003bb3790..4bab0339312 100644 --- a/services/gateway/pkg/config/defaults/defaultconfig.go +++ b/services/gateway/pkg/config/defaults/defaultconfig.go @@ -39,14 +39,10 @@ func DefaultConfig() *config.Config { DisableHomeCreationOnLogin: true, TransferExpires: 24 * 60 * 60, Cache: config.Cache{ - ProviderCacheStore: "noop", - ProviderCacheNodes: []string{"127.0.0.1:9233"}, - ProviderCacheDatabase: "cache-providers", - ProviderCacheTTL: 300 * time.Second, - CreateHomeCacheStore: "memory", - CreateHomeCacheNodes: []string{"127.0.0.1:9233"}, - CreateHomeCacheDatabase: "cache-createhome", - CreateHomeCacheTTL: 300 * time.Second, + ProviderCacheStore: "noop", + ProviderCacheNodes: []string{"127.0.0.1:9233"}, + ProviderCacheDatabase: "cache-providers", + ProviderCacheTTL: 300 * time.Second, }, FrontendPublicURL: "https://localhost:9200", diff --git a/services/gateway/pkg/revaconfig/config.go b/services/gateway/pkg/revaconfig/config.go index d4589e5d7fb..44dcda07348 100644 --- a/services/gateway/pkg/revaconfig/config.go +++ b/services/gateway/pkg/revaconfig/config.go @@ -75,16 +75,6 @@ func GatewayConfigFromStruct(cfg *config.Config, logger log.Logger) map[string]i "cache_auth_username": cfg.Cache.ProviderCacheAuthUsername, "cache_auth_password": cfg.Cache.ProviderCacheAuthPassword, }, - "create_personal_space_cache_config": map[string]interface{}{ - "cache_store": cfg.Cache.CreateHomeCacheStore, - "cache_nodes": cfg.Cache.CreateHomeCacheNodes, - "cache_database": cfg.Cache.CreateHomeCacheDatabase, - "cache_table": "create_personal_space", - "cache_ttl": cfg.Cache.CreateHomeCacheTTL, - "cache_disable_persistence": cfg.Cache.CreateHomeCacheDisablePersistence, - "cache_auth_username": cfg.Cache.CreateHomeCacheAuthUsername, - "cache_auth_password": cfg.Cache.CreateHomeCacheAuthPassword, - }, }, "authregistry": map[string]interface{}{ "driver": "static", diff --git a/services/proxy/pkg/middleware/create_home.go b/services/proxy/pkg/middleware/create_home.go index a71f3825354..4caf453ef31 100644 --- a/services/proxy/pkg/middleware/create_home.go +++ b/services/proxy/pkg/middleware/create_home.go @@ -3,6 +3,7 @@ package middleware import ( "net/http" "strconv" + "sync" gateway "github.com/cs3org/go-cs3apis/cs3/gateway/v1beta1" userv1beta1 "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1" @@ -11,7 +12,6 @@ import ( "github.com/owncloud/ocis/v2/ocis-pkg/log" "github.com/owncloud/ocis/v2/services/graph/pkg/errorcode" revactx "github.com/owncloud/reva/v2/pkg/ctx" - "github.com/owncloud/reva/v2/pkg/rgrpc/status" "github.com/owncloud/reva/v2/pkg/rgrpc/todo/pool" "github.com/owncloud/reva/v2/pkg/utils" "google.golang.org/grpc/metadata" @@ -28,6 +28,7 @@ func CreateHome(optionSetters ...Option) func(next http.Handler) http.Handler { logger: logger, revaGatewaySelector: options.RevaGatewaySelector, roleQuotas: options.RoleQuotas, + cache: sync.Map{}, } } } @@ -37,9 +38,10 @@ type createHome struct { logger log.Logger revaGatewaySelector pool.Selectable[gateway.GatewayAPIClient] roleQuotas map[string]uint64 + cache sync.Map // Store users for which personal space has been in memory indefinitely. Persistence isn't critical. } -func (m createHome) ServeHTTP(w http.ResponseWriter, req *http.Request) { +func (m *createHome) ServeHTTP(w http.ResponseWriter, req *http.Request) { if !m.shouldServe(req) { m.next.ServeHTTP(w, req) return @@ -69,13 +71,20 @@ func (m createHome) ServeHTTP(w http.ResponseWriter, req *http.Request) { if err != nil { m.logger.Err(err).Msg("error selecting next gateway client") } else { - createHomeRes, err := client.CreateHome(ctx, createHomeReq) - if err != nil { - m.logger.Err(err).Msg("error calling CreateHome") - } else if createHomeRes.Status.Code != rpc.Code_CODE_OK { - err := status.NewErrorFromCode(createHomeRes.Status.Code, "gateway") - if createHomeRes.Status.Code != rpc.Code_CODE_ALREADY_EXISTS { - m.logger.Err(err).Msg("error when calling Createhome") + key := u.GetId().GetOpaqueId() + if _, exists := m.cache.Load(key); !exists { + createHomeRes, err := client.CreateHome(ctx, createHomeReq) + switch { + case err != nil: + m.logger.Err(err).Msg("error calling CreateHome") + case createHomeRes.GetStatus().GetCode() == rpc.Code_CODE_OK: + m.logger.Debug().Interface("userID", u.GetId().GetOpaqueId()).Msg("personal space created") + m.cache.Store(key, struct{}{}) + case createHomeRes.GetStatus().GetCode() == rpc.Code_CODE_ALREADY_EXISTS: + m.logger.Info().Interface("userID", u.GetId().GetOpaqueId()).Interface("status", createHomeRes.GetStatus()).Msg("personal space already exists") + m.cache.Store(key, struct{}{}) + default: + m.logger.Error().Interface("userID", u.GetId().GetOpaqueId()).Interface("status", createHomeRes.GetStatus()).Msg("personal space creation failed") } } } @@ -83,11 +92,11 @@ func (m createHome) ServeHTTP(w http.ResponseWriter, req *http.Request) { m.next.ServeHTTP(w, req) } -func (m createHome) shouldServe(req *http.Request) bool { +func (m *createHome) shouldServe(req *http.Request) bool { return req.Header.Get("x-access-token") != "" } -func (m createHome) getUserRoles(user *userv1beta1.User) ([]string, error) { +func (m *createHome) getUserRoles(user *userv1beta1.User) ([]string, error) { var roleIDs []string if err := utils.ReadJSONFromOpaque(user.Opaque, "roles", &roleIDs); err != nil { return nil, err @@ -105,7 +114,7 @@ func (m createHome) getUserRoles(user *userv1beta1.User) ([]string, error) { return dedup, nil } -func (m createHome) checkRoleQuotaLimit(roleIDs []string) (uint64, bool) { +func (m *createHome) checkRoleQuotaLimit(roleIDs []string) (uint64, bool) { if len(roleIDs) == 0 { return 0, false }