Add latest changes from gitlab-org/gitlab@13-3-stable-ee

3 files changed, 340 insertions, 79 deletions

diff --git a/doc/development/feature_flags/controls.md b/doc/development/feature_flags/controls.md
index 9e7ce74cc0c..09260be1264 100644
--- a/doc/development/feature_flags/controls.md
+++ b/doc/development/feature_flags/controls.md
@@ -140,7 +140,7 @@ run the following in Slack:
 This sets a feature flag to `true` based on the following formula:
 
 ```ruby
-feature_flag_state = Zlib.crc32("some_feature<Actor>:#{actor.id}") % (100 * 1_000) < 25 * 1_000]
+feature_flag_state = Zlib.crc32("some_feature<Actor>:#{actor.id}") % (100 * 1_000) < 25 * 1_000
 # where <Actor>: is a `User`, `Group`, `Project` and actor is an instance
 ```
 
diff --git a/doc/development/feature_flags/development.md b/doc/development/feature_flags/development.md
index 0b918478668..9b30187fcd1 100644
--- a/doc/development/feature_flags/development.md
+++ b/doc/development/feature_flags/development.md
@@ -1,19 +1,219 @@
 # Developing with feature flags
 
-In general, it's better to have a group- or user-based gate, and you should prefer
-it over the use of percentage gates. This would make debugging easier, as you
-filter for example logs and errors based on actors too. Furthermore, this allows
-for enabling for the `gitlab-org` or `gitlab-com` group first, while the rest of
+This document provides guidelines on how to use feature flags
+in the GitLab codebase to conditionally enable features
+and test them.
+
+Features that are developed and merged behind a feature flag
+should not include a changelog entry. The entry should be added either in the merge
+request removing the feature flag or the merge request where the default value of
+the feature flag is set to enabled. If the feature contains any database migrations, it
+*should* include a changelog entry for the database changes.
+
+CAUTION: **Caution:**
+All newly-introduced feature flags should be [disabled by default](process.md#feature-flags-in-gitlab-development).
+
+NOTE: **Note:**
+This document is the subject of continued work as part of an epic to [improve internal usage of Feature Flags](https://gitlab.com/groups/gitlab-org/-/epics/3551). Raise any suggestions as new issues and attach them to the epic.
+
+## Types of feature flags
+
+Currently, only a single type of feature flag is available.
+Additional feature flag types will be provided in the future,
+with descriptions for their usage.
+
+### `development` type
+
+`development` feature flags are short-lived feature flags,
+used so that unfinished code can be deployed in production.
+
+A `development` feature flag should have a rollout issue,
+ideally created using the [Feature Flag Roll Out template](https://gitlab.com/gitlab-org/gitlab/-/blob/master/.gitlab/issue_templates/Feature%20Flag%20Roll%20Out.md).
+
+NOTE: **Note:**
+This is the default type used when calling `Feature.enabled?`.
+
+## Feature flag definition and validation
+
+> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/229161) in GitLab 13.3.
+
+During development (`RAILS_ENV=development`) or testing (`RAILS_ENV=test`) all feature flag usage is being strictly validated.
+
+This process is meant to ensure consistent feature flag usage in the codebase. All feature flags **must**:
+
+- Be known. Only use feature flags that are explicitly defined.
+- Not be defined twice. They have to be defined either in FOSS or EE, but not both.
+- Use a valid and consistent `type:` across all invocations.
+- Use the same `default_enabled:` across all invocations.
+- Have an owner.
+
+All feature flags known to GitLab are self-documented in YAML files stored in:
+
+- [`config/feature_flags`](https://gitlab.com/gitlab-org/gitlab/-/tree/master/config/feature_flags)
+- [`ee/config/feature_flags`](https://gitlab.com/gitlab-org/gitlab/-/tree/master/ee/config/feature_flags)
+
+Each feature flag is defined in a separate YAML file consisting of a number of fields:
+
+| Field               | Required | Description                                                    |
+|---------------------|----------|----------------------------------------------------------------|
+| `name`              | yes      | Name of the feature flag.                                      |
+| `type`              | yes      | Type of feature flag.                                          |
+| `default_enabled`   | yes      | The default state of the feature flag that is strongly validated, with `default_enabled:` passed as an argument. |
+| `introduced_by_url` | no       | The URL to the Merge Request that introduced the feature flag. |
+| `rollout_issue_url` | no       | The URL to the Issue covering the feature flag rollout.        |
+| `group`             | no       | The [group](https://about.gitlab.com/handbook/product/product-categories/#devops-stages) that owns the feature flag. |
+
+TIP: **Tip:**
+All validations are skipped when running in `RAILS_ENV=production`.
+
+## Create a new feature flag
+
+The GitLab codebase provides [`bin/feature-flag`](https://gitlab.com/gitlab-org/gitlab/-/blob/master/bin/feature-flag),
+a dedicated tool to create new feature flag definitions.
+The tool asks various questions about the new feature flag, then creates
+a YAML definition in `config/feature_flags` or `ee/config/feature_flags`.
+
+Only feature flags that have a YAML definition file can be used when running the development or testing environments.
+
+```shell
+$ bin/feature-flag my-feature-flag
+>> Please specify the group introducing feature flag, like `group::apm`:
+?> group::memory
+
+>> If you have MR open, can you paste the URL here? (or enter to skip)
+?> https://gitlab.com/gitlab-org/gitlab/-/merge_requests/38602
+
+>> Open this URL and fill the rest of details:
+https://gitlab.com/gitlab-org/gitlab/-/issues/new?issue%5Btitle%5D=%5BFeature+flag%5D+Rollout+of+%60test-flag%60&issuable_template=Feature+Flag+Roll+Out
+
+>> Paste URL of `rollout issue` here, or enter to skip:
+?> https://gitlab.com/gitlab-org/gitlab/-/issues/232533
+create config/feature_flags/development/test-flag.yml
+---
+name: test-flag
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/38602
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/232533
+group: group::memory
+type: development
+default_enabled: false
+```
+
+TIP: **Tip:**
+To create a feature flag that is only used in EE, add the `--ee` flag: `bin/feature-flag --ee`
+
+## Develop with a feature flag
+
+There are two main ways of using Feature Flags in the GitLab codebase:
+
+- [Backend code (Rails)](#backend)
+- [Frontend code (VueJS)](#frontend)
+
+### Backend
+
+The feature flag interface is defined in [`lib/feature.rb`](https://gitlab.com/gitlab-org/gitlab/-/blob/master/lib/feature.rb).
+This interface provides a set of methods to check if the feature flag is enabled or disabled:
+
+```ruby
+if Feature.enabled?(:my_feature_flag, project)
+  # execute code if feature flag is enabled
+else
+  # execute code if feature flag is disabled
+end
+
+if Feature.disabled?(:my_feature_flag, project)
+  # execute code if feature flag is disabled
+end
+```
+
+In rare cases you may want to make a feature enabled by default. If so, explain the reasoning
+in the merge request. Use `default_enabled: true` when checking the feature flag state:
+
+```ruby
+if Feature.enabled?(:feature_flag, project, default_enabled: true)
+  # execute code if feature flag is enabled
+else
+  # execute code if feature flag is disabled
+end
+
+if Feature.disabled?(:my_feature_flag, project, default_enabled: true)
+  # execute code if feature flag is disabled
+end
+```
+
+### Frontend
+
+Use the `push_frontend_feature_flag` method for frontend code, which is
+available to all controllers that inherit from `ApplicationController`. You can use
+this method to expose the state of a feature flag, for example:
+
+```ruby
+before_action do
+  # Prefer to scope it per project or user e.g.
+  push_frontend_feature_flag(:vim_bindings, project)
+end
+
+def index
+  # ...
+end
+
+def edit
+  # ...
+end
+```
+
+You can then check the state of the feature flag in JavaScript as follows:
+
+```javascript
+if ( gon.features.vimBindings ) {
+  // ...
+}
+```
+
+The name of the feature flag in JavaScript is always camelCase,
+so checking for `gon.features.vim_bindings` would not work.
+
+See the [Vue guide](../fe_guide/vue.md#accessing-feature-flags) for details about
+how to access feature flags in a Vue component.
+
+In rare cases you may want to make a feature enabled by default. If so, explain the reasoning
+in the merge request. Use `default_enabled: true` when checking the feature flag state:
+
+```ruby
+before_action do
+  # Prefer to scope it per project or user e.g.
+  push_frontend_feature_flag(:vim_bindings, project, default_enabled: true)
+end
+```
+
+### Feature actors
+
+**It is strongly advised to use actors with feature flags.** Actors provide a simple
+way to enable a feature flag only for a given project, group or user. This makes debugging
+easier, as you can filter logs and errors for example, based on actors. This also makes it possible
+to enable the feature on the `gitlab-org` or `gitlab-com` groups first, while the rest of
 the users aren't impacted.
 
+Actors also provide an easy way to do a percentage rollout of a feature in a sticky way.
+If a 1% rollout enabled a feature for a specific actor, that actor will continue to have the feature enabled at
+10%, 50%, and 100%.
+
+GitLab currently supports the following models as feature flag actors:
+
+- `User`
+- `Project`
+- `Group`
+
+The actor is a second parameter of the `Feature.enabled?` call. The
+same actor type must be used consistently for all invocations of `Feature.enabled?`.
+
 ```ruby
-# Good
 Feature.enabled?(:feature_flag, project)
-
-# Avoid, if possible
-Feature.enabled?(:feature_flag)
+Feature.enabled?(:feature_flag, group)
+Feature.enabled?(:feature_flag, user)
 ```
 
+### Enable additional objects as actors
+
 To use feature gates based on actors, the model needs to respond to
 `flipper_id`. For example, to enable for the Foo model:
 
@@ -26,29 +226,18 @@ end
 Only models that `include FeatureGate` or expose `flipper_id` method can be
 used as an actor for `Feature.enabled?`.
 
-Features that are developed and are intended to be merged behind a feature flag
-should not include a changelog entry. The entry should either be added in the merge
-request removing the feature flag or the merge request where the default value of
-the feature flag is set to true. If the feature contains any DB migration it
-should include a changelog entry for DB changes.
-
-If you need the feature flag to be on automatically, use `default_enabled: true`
-when checking:
+### Feature flags for licensed features
 
-```ruby
-Feature.enabled?(:feature_flag, project, default_enabled: true)
-```
+If a feature is license-gated, there's no need to add an additional
+explicit feature flag check since the flag will be checked as part of the
+`License.feature_available?` call. Similarly, there's no need to "clean up" a
+feature flag once the feature has reached general availability.
 
 The [`Project#feature_available?`](https://gitlab.com/gitlab-org/gitlab/blob/4cc1c62918aa4c31750cb21dfb1a6c3492d71080/app/models/project_feature.rb#L63-68),
 [`Namespace#feature_available?`](https://gitlab.com/gitlab-org/gitlab/blob/4cc1c62918aa4c31750cb21dfb1a6c3492d71080/ee/app/models/ee/namespace.rb#L71-85) (EE), and
 [`License.feature_available?`](https://gitlab.com/gitlab-org/gitlab/blob/4cc1c62918aa4c31750cb21dfb1a6c3492d71080/ee/app/models/license.rb#L293-300) (EE) methods all implicitly check for
 a by default enabled feature flag with the same name as the provided argument.
 
-For example if a feature is license-gated, there's no need to add an additional
-explicit feature flag check since the flag will be checked as part of the
-`License.feature_available?` call. Similarly, there's no need to "clean up" a
-feature flag once the feature has reached general availability.
-
 You'd still want to use an explicit `Feature.enabled?` check if your new feature
 isn't gated by a License or Plan.
 
@@ -58,7 +247,7 @@ the feature flag check will default to `true`.**
 
 This is relevant when developing the feature using
 [several smaller merge requests](https://about.gitlab.com/handbook/values/#make-small-merge-requests), or when the feature is considered to be an
-[alpha or beta](https://about.gitlab.com/handbook/product/#alpha-beta-ga), and
+[alpha or beta](https://about.gitlab.com/handbook/product/gitlab-the-product/#alpha-beta-ga), and
 should not be available by default.
 
 As an example, if you were to ship the frontend half of a feature without the
@@ -67,13 +256,10 @@ also ready to be shipped. To make sure this feature is disabled for both
 GitLab.com and self-managed instances, you should use the
 [`Namespace#alpha_feature_available?`](https://gitlab.com/gitlab-org/gitlab/blob/458749872f4a8f27abe8add930dbb958044cb926/ee/app/models/ee/namespace.rb#L113) or
 [`Namespace#beta_feature_available?`](https://gitlab.com/gitlab-org/gitlab/blob/458749872f4a8f27abe8add930dbb958044cb926/ee/app/models/ee/namespace.rb#L100-112)
-method, according to our [definitions](https://about.gitlab.com/handbook/product/#alpha-beta-ga). This ensures the feature is disabled unless the feature flag is
+method, according to our [definitions](https://about.gitlab.com/handbook/product/gitlab-the-product/#alpha-beta-ga). This ensures the feature is disabled unless the feature flag is
 _explicitly_ enabled.
 
-## Feature groups
-
-Starting from GitLab 9.4 we support feature groups via
-[Flipper groups](https://github.com/jnunemaker/flipper/blob/v0.10.2/docs/Gates.md#2-group).
+### Feature groups
 
 Feature groups must be defined statically in `lib/feature.rb` (in the
 `.register_feature_groups` method), but their implementation can obviously be
@@ -82,50 +268,144 @@ dynamic (querying the DB etc.).
 Once defined in `lib/feature.rb`, you will be able to activate a
 feature for a given feature group via the [`feature_group` parameter of the features API](../../api/features.md#set-or-create-a-feature)
 
-### Frontend
+### Enabling a feature flag locally (in development)
 
-For frontend code you can use the method `push_frontend_feature_flag`, which is
-available to all controllers that inherit from `ApplicationController`. Using
-this method you can expose the state of a feature flag as follows:
+In the rails console (`rails c`), enter the following command to enable a feature flag:
 
 ```ruby
-before_action do
-  # Prefer to scope it per project or user e.g.
-  push_frontend_feature_flag(:vim_bindings, project)
+Feature.enable(:feature_flag_name)
+```
 
-  # Avoid, if possible
-  push_frontend_feature_flag(:vim_bindings)
-end
+Similarly, the following command will disable a feature flag:
 
-def index
-  # ...
-end
+```ruby
+Feature.disable(:feature_flag_name)
+```
 
-def edit
-  # ...
-end
+You can also enable a feature flag for a given gate:
+
+```ruby
+Feature.enable(:feature_flag_name, Project.find_by_full_path("root/my-project"))
 ```
 
-You can then check for the state of the feature flag in JavaScript as follows:
+## Feature flags in tests
 
-```javascript
-if ( gon.features.vimBindings ) {
-  // ...
-}
+Introducing a feature flag into the codebase creates an additional codepath that should be tested.
+It is strongly advised to test all code affected by a feature flag, both when **enabled** and **disabled**
+to ensure the feature works properly.
+
+NOTE: **Note:**
+When using the testing environment, all feature flags are enabled by default.
+
+To disable a feature flag in a test, use the `stub_feature_flags`
+helper. For example, to globally disable the `ci_live_trace` feature
+flag in a test:
+
+```ruby
+stub_feature_flags(ci_live_trace: false)
+
+Feature.enabled?(:ci_live_trace) # => false
 ```
 
-The name of the feature flag in JavaScript will always be camelCased, meaning
-that checking for `gon.features.vim_bindings` would not work.
+If you wish to set up a test where a feature flag is enabled only
+for some actors and not others, you can specify this in options
+passed to the helper. For example, to enable the `ci_live_trace`
+feature flag for a specific project:
 
-See the [Vue guide](../fe_guide/vue.md#accessing-feature-flags) for details about
-how to access feature flags in a Vue component.
+```ruby
+project1, project2 = build_list(:project, 2)
 
-### Specs
+# Feature will only be enabled for project1
+stub_feature_flags(ci_live_trace: project1)
+
+Feature.enabled?(:ci_live_trace) # => false
+Feature.enabled?(:ci_live_trace, project1) # => true
+Feature.enabled?(:ci_live_trace, project2) # => false
+```
+
+The behavior of FlipperGate is as follows:
+
+1. You can enable an override for a specified actor to be enabled
+1. You can disable (remove) an override for a specified actor,
+   falling back to default state
+1. There's no way to model that you explicitly disable a specified actor
+
+```ruby
+Feature.enable(:my_feature)
+Feature.disable(:my_feature, project1)
+Feature.enabled?(:my_feature) # => true
+Feature.enabled?(:my_feature, project1) # => true
+
+Feature.disable(:my_feature2)
+Feature.enable(:my_feature2, project1)
+Feature.enabled?(:my_feature2) # => false
+Feature.enabled?(:my_feature2, project1) # => true
+```
+
+### `stub_feature_flags` vs `Feature.enable*`
+
+It is preferred to use `stub_feature_flags` to enable feature flags
+in the testing environment. This method provides a simple and well described
+interface for simple use cases.
+
+However, in some cases more complex behavior needs to be tested,
+like percentage rollouts of feature flags. This can be done using
+`.enable_percentage_of_time` or `.enable_percentage_of_actors`:
+
+```ruby
+# Good: feature needs to be explicitly disabled, as it is enabled by default if not defined
+stub_feature_flags(my_feature: false)
+stub_feature_flags(my_feature: true)
+stub_feature_flags(my_feature: project)
+stub_feature_flags(my_feature: [project, project2])
+
+# Bad
+Feature.enable(:my_feature_2)
+
+# Good: enable my_feature for 50% of time
+Feature.enable_percentage_of_time(:my_feature_3, 50)
+
+# Good: enable my_feature for 50% of actors/gates/things
+Feature.enable_percentage_of_actors(:my_feature_4, 50)
+```
+
+Each feature flag that has a defined state will be persisted
+during test execution time:
+
+```ruby
+Feature.persisted_names.include?('my_feature') => true
+Feature.persisted_names.include?('my_feature_2') => true
+Feature.persisted_names.include?('my_feature_3') => true
+Feature.persisted_names.include?('my_feature_4') => true
+```
+
+### Stubbing actor
+
+When you want to enable a feature flag for a specific actor only,
+you can stub its representation. A gate that is passed
+as an argument to `Feature.enabled?` and `Feature.disabled?` must be an object
+that includes `FeatureGate`.
+
+In specs you can use the `stub_feature_flag_gate` method that allows you to
+quickly create a custom actor:
+
+```ruby
+gate = stub_feature_flag_gate('CustomActor')
+
+stub_feature_flags(ci_live_trace: gate)
+
+Feature.enabled?(:ci_live_trace) # => false
+Feature.enabled?(:ci_live_trace, gate) # => true
+```
+
+### Controlling feature flags engine in tests
 
 Our Flipper engine in the test environment works in a memory mode `Flipper::Adapters::Memory`.
 `production` and `development` modes use `Flipper::Adapters::ActiveRecord`.
 
-### `stub_feature_flags: true` (default and preferred)
+You can control whether the `Flipper::Adapters::Memory` or `ActiveRecord` mode is being used.
+
+#### `stub_feature_flags: true` (default and preferred)
 
 In this mode Flipper is configured to use `Flipper::Adapters::Memory` and mark all feature
 flags to be on-by-default and persisted on a first use. This overwrites the `default_enabled:`
@@ -145,23 +425,3 @@ a mode that is used by `production` and `development`.
 
 You should use this mode only when you really want to tests aspects of Flipper
 with how it interacts with `ActiveRecord`.
-
-### Enabling a feature flag (in development)
-
-In the rails console (`rails c`), enter the following command to enable your feature flag
-
-```ruby
-Feature.enable(:feature_flag_name)
-```
-
-Similarly, the following command will disable a feature flag:
-
-```ruby
-Feature.disable(:feature_flag_name)
-```
-
-You can as well enable feature flag for a given gate:
-
-```ruby
-Feature.enable(:feature_flag_name, Project.find_by_full_path("root/my-project"))
-```
diff --git a/doc/development/feature_flags/process.md b/doc/development/feature_flags/process.md
index b053838964b..5dc3cf44a6e 100644
--- a/doc/development/feature_flags/process.md
+++ b/doc/development/feature_flags/process.md
@@ -24,7 +24,8 @@ should be leveraged:
 - When development of a feature will be spread across multiple merge
   requests, you can use the following workflow:
 
-  1. Introduce a feature flag which is **off** by default, in the first merge request.
+  1. [Create a new feature flag](development.md#create-a-new-feature-flag)
+     which is **off** by default, in the first merge request.
   1. Submit incremental changes via one or more merge requests, ensuring that any
      new code added can only be reached if the feature flag is **on**.
      You can keep the feature flag enabled on your local GDK during development.