# The problem As per https://gitlab.com/gitlab-org/configure/general/issues/4#reactivecache-redis we currently have the possibility of our users abusing our Redis instance by returning large payloads from their Kubernetes instances. We should implement 2 layers of defense against this: 1. We should make it easy to see from a monitoring perspective when there are large payloads being stored. Ideally some logging that is also easily monitored that allows tracing large payloads back to the projects or users responsible. 2. We should block any attempts to store something in Redis beyond a certain size. This could be implemented either generically at the Redis level or at the ReactiveCache level (which is more targeting our use of caching K8s data). We would need to determine if there are obvious limits that could be used here. For example I imagine that for all the K8s data we're storing for various data loaded in our K8s integration, monitoring and Serverless stuff there is never any good reason to store a payload larger than 1MB. If that's the case we could just block such payloads in Ruby before they get written into Redis. ##### dev.gitlab.org issue https://dev.gitlab.org/gitlab/gitlabhq/issues/2963 --- ## How we dealt with it We introduced a `class_attribute :reactive_cache_hard_limit` to the `ReactiveCaching` module. If this value is `nil` (default), we won't check for limits. If it's overridden to anything like `self.reactive_cache_hard_limit = 1.megabyte`, then we'll validate the size of the calculated data before saving it to Redis. If the data is over the limit, then we'll track an error on Sentry trying to log which user and project triggered this and the data won't be cached. ## What to do next We suggest that each team should apply limits to their usage of `ReactiveCaching`. Like explained above, the limit is `nil` by default. This means that just by including `ReactiveCaching` no limit will be checked. Each class that includes this module will have to add their own limits. We did it like this because we were uncertain which limits to impose to each usage of it. Since not caching a data that goes over the limit will have user-facing implications. So each team will know better which limit to impose to their own use of `ReactiveCaching`. Once all the existing usage of `ReactiveCaching` is limited, then we can set a default to it. So far we've only implemented it for the `Environment` model. We suggest that other teams also investigate which limit size would work for them, then add the limit to their usage. It's convenient to first guess the limit behind a feature flag, so if the limit starts breaking something, it's easy to turn it off until the limit is adjusted. To put the limit check behind a FF flag an easy way could be to override the `reactive_cache_limit_enabled?` in your class. Like the following: ```ruby # Overrides ReactiveCaching default to activate limit checking behind a FF def reactive_cache_limit_enabled? Feature.enabled?(:reactive_caching_limit_environment, project) end ``` Once the limit is deemed valid in GitLab.com and perhaps also for self-managed by enabling the FF by default, then just remove the FF. Here are all the places where we use `ReactiveCaching`. I gave it a shot in trying to find out which team is the DRI for each class. It would be great if the DRIs open up follow-up issues to add limits to their usage: Please check if you have already created a follow-up issue for this. Once the issue is open, please link to this epic and tick the checkbox. :bow: Please let us know in a comment if you're not the DRI for this file so we can try to guess another team. - [x] **DRI**: @nicholasklick gitlab~3103456 , **File**: `app/models/environment.rb`, Issue: (https://gitlab.com/gitlab-org/gitlab/-/issues/202633) - [ ] **DRI**: @nicholasklick gitlab~3103456 , **File**: `app/finders/clusters/knative_services_finder.rb` (knative) - [ ] **DRI**: @nicholasklick gitlab~3103456 , **File**: `app/finders/projects/serverless/functions_finder.rb` (knative) - [ ] **DRI**: @nicholasklick gitlab~3103456 , **File**: `app/models/clusters/cluster.rb` (cluster detail page) - [ ] **DRI**: @sean_carroll gitlab~3103452 , **File**: `app/models/merge_request.rb` - [ ] **DRI**: @sean_carroll gitlab~3103452 , **File**: `app/models/ssh_host_key.rb` - [x] **DRI**: @crystalpoole gitlab~3103458 , **File**: `app/models/concerns/prometheus_adapter.rb` (https://gitlab.com/gitlab-org/gitlab/-/issues/299266) - [x] **DRI**: @crystalpoole gitlab~3103458 , **File**: `app/models/error_tracking/project_error_tracking_setting.rb` (https://gitlab.com/gitlab-org/gitlab/-/issues/299266) - [x] **DRI**: @crystalpoole gitlab~3103458 , **File**: `app/services/grafana/proxy_service.rb` (https://gitlab.com/gitlab-org/gitlab/-/issues/299266) - [x] **DRI**: @crystalpoole gitlab~3103458 , **File**: `app/services/metrics/dashboard/grafana_metric_embed_service.rb` (https://gitlab.com/gitlab-org/gitlab/-/issues/299266) - [x] **DRI**: @crystalpoole gitlab~3103458 , **File**: `app/services/prometheus/proxy_service.rb` (https://gitlab.com/gitlab-org/gitlab/-/issues/299266) - [ ] **DRI**: @lmcandrew gitlab~4116705 , **File**: `app/models/project_services/bamboo_service.rb` - [x] **DRI**: @cheryl.li gitlab~3103453 , **File**: `app/models/project_services/buildkite_service.rb` (https://gitlab.com/gitlab-org/gitlab/-/issues/293037) - [x] **DRI**: @cheryl.li gitlab~3103453 , **File**: `app/models/project_services/drone_ci_service.rb` (https://gitlab.com/gitlab-org/gitlab/-/issues/293037) - [x] **DRI**: @cheryl.li gitlab~3103453 , **File**: `app/models/project_services/teamcity_service.rb` (https://gitlab.com/gitlab-org/gitlab/-/issues/293037) - [x] **DRI**: @cheryl.li gitlab~3103453 , **File**: `app/services/ci/list_config_variables_service.rb` (https://gitlab.com/gitlab-org/gitlab/-/issues/293036) - [x] **DRI**: @erushton gitlab~3103453 , **File**: `app/services/pod_logs/base_service.rb` (https://gitlab.com/gitlab-org/gitlab/-/issues/293893)