diff options
Diffstat (limited to 'doc/development')
| -rw-r--r-- | doc/development/migration_style_guide.md | 25 | ||||
| -rw-r--r-- | doc/development/shell_commands.md | 2 | ||||
| -rw-r--r-- | doc/development/testing_guide/best_practices.md | 12 |
3 files changed, 28 insertions, 11 deletions
diff --git a/doc/development/migration_style_guide.md b/doc/development/migration_style_guide.md index 529b0802991..5764c876e4d 100644 --- a/doc/development/migration_style_guide.md +++ b/doc/development/migration_style_guide.md @@ -906,9 +906,14 @@ end Table **has records** but **no foreign keys**: -- First release: Remove the application code related to the table, such as models, -controllers and services. -- Second release: Use the `drop_table` method in your migration. +- Remove the application code related to the table, such as models, + controllers and services. +- In a post-deployment migration, use `drop_table`. + +This can all be in a single migration if you're sure the code is not used. +If you want to reduce risk slightly, consider putting the migrations into a +second merge request after the application changes are merged. This approach +provides an opportunity to roll back. ```ruby def up @@ -922,12 +927,16 @@ end Table **has foreign keys**: -- First release: Remove the application code related to the table, such as models, -controllers, and services. -- Second release: Remove the foreign keys using the `with_lock_retries` -helper method. Use `drop_table` in another migration file. +- Remove the application code related to the table, such as models, + controllers, and services. +- In a post-deployment migration, remove the foreign keys using the + `with_lock_retries` helper method. In another subsequent post-deployment + migration, use `drop_table`. -**Migrations for the second release:** +This can all be in a single migration if you're sure the code is not used. +If you want to reduce risk slightly, consider putting the migrations into a +second merge request after the application changes are merged. This approach +provides an opportunity to roll back. Removing the foreign key on the `projects` table: diff --git a/doc/development/shell_commands.md b/doc/development/shell_commands.md index 3935e98199a..d78a005d76b 100644 --- a/doc/development/shell_commands.md +++ b/doc/development/shell_commands.md @@ -71,6 +71,8 @@ FileUtils.touch myfile This coding style could have prevented CVE-2013-4546. +See also <https://gitlab.com/gitlab-org/gitlab/-/merge_requests/93030>, and <https://starlabs.sg/blog/2022/07-gitlab-project-import-rce-analysis-cve-2022-2185/> for another example. + ## Separate options from arguments with -- Make the difference between options and arguments clear to the argument parsers of system commands with `--`. This is supported by many but not all Unix commands. diff --git a/doc/development/testing_guide/best_practices.md b/doc/development/testing_guide/best_practices.md index aa7344f8506..b6bf3c7805a 100644 --- a/doc/development/testing_guide/best_practices.md +++ b/doc/development/testing_guide/best_practices.md @@ -933,9 +933,7 @@ In most specs, the Rails cache is actually an in-memory store. This is replaced between specs, so calls to `Rails.cache.read` and `Rails.cache.write` are safe. However, if a spec makes direct Redis calls, it should mark itself with the `:clean_gitlab_redis_cache`, `:clean_gitlab_redis_shared_state` or -`:clean_gitlab_redis_queues` traits as appropriate. To avoid triggering rate -limiting in specs, mark the spec with the `:clean_gitlab_redis_rate_limiting` -trait. +`:clean_gitlab_redis_queues` traits as appropriate. #### Background jobs / Sidekiq @@ -969,6 +967,14 @@ it "really connects to Prometheus", :permit_dns do And if you need more specific control, the DNS blocking is implemented in `spec/support/helpers/dns_helpers.rb` and these methods can be called elsewhere. +#### Rate Limiting + +[Rate limiting](../../security/rate_limits.md) is enabled in the test suite. Rate limits +may be triggered in feature specs that use the `:js` trait. In most cases, triggering rate +limiting can be avoided by marking the spec with the `:clean_gitlab_redis_rate_limiting` +trait. This trait clears the rate limiting data stored in Redis cache between specs. If +a single test triggers the rate limit, the `:disable_rate_limit` can be used instead. + #### Stubbing File methods In the situations where you need to |