Commit 9753741d authored by Kamil Trzciński's avatar Kamil Trzciński Committed by Shinya Maeda

Allow to use only defined feature flags

This changes the API for feature flags to:
- allow to use only defined feature flags
- allow to skip this validation with `?force=1`
- returns information if the feature flag is defined
parent 3b46ce3c
......@@ -37,7 +37,8 @@ Example response:
"key": "boolean",
"value": false
}
]
],
"definition": null
},
{
"name": "my_user_feature",
......@@ -47,7 +48,15 @@ Example response:
"key": "percentage_of_actors",
"value": 34
}
]
],
"definition": {
"name": "my_user_feature",
"introduced_by_url": "https://gitlab.com/gitlab-org/gitlab/-/merge_requests/40880",
"rollout_issue_url": "https://gitlab.com/gitlab-org/gitlab/-/issues/244905",
"group": "group::ci",
"type": "development",
"default_enabled": false
}
},
{
"name": "new_library",
......@@ -57,7 +66,45 @@ Example response:
"key": "boolean",
"value": true
}
]
],
"definition": null
}
]
```
## List all feature definitions
Get a list of all feature definitions.
```plaintext
GET /features/definitions
```
```shell
curl --header "PRIVATE-TOKEN: <your_access_token>" "https://gitlab.example.com/api/v4/features/definitions"
```
Example response:
```json
[
{
"name": "api_kaminari_count_with_limit",
"introduced_by_url": "https://gitlab.com/gitlab-org/gitlab-foss/-/merge_requests/23931",
"rollout_issue_url": null,
"milestone": "11.8",
"type": "ops",
"group": "group::ecosystem",
"default_enabled": false
},
{
"name": "marginalia",
"introduced_by_url": null,
"rollout_issue_url": null,
"milestone": null,
"type": "ops",
"group": null,
"default_enabled": false
}
]
```
......@@ -81,6 +128,7 @@ POST /features/:name
| `user` | string | no | A GitLab username |
| `group` | string | no | A GitLab group's path, for example `gitlab-org` |
| `project` | string | no | A projects path, for example `gitlab-org/gitlab-foss` |
| `force` | boolean | no | Skip feature flag validation checks, ie. YAML definition |
Note that you can enable or disable a feature for a `feature_group`, a `user`,
a `group`, and a `project` in a single API call.
......@@ -104,7 +152,15 @@ Example response:
"key": "percentage_of_time",
"value": 30
}
]
],
"definition": {
"name": "my_user_feature",
"introduced_by_url": "https://gitlab.com/gitlab-org/gitlab/-/merge_requests/40880",
"rollout_issue_url": "https://gitlab.com/gitlab-org/gitlab/-/issues/244905",
"group": "group::ci",
"type": "development",
"default_enabled": false
}
}
```
......@@ -133,7 +189,15 @@ Example response:
"key": "percentage_of_actors",
"value": 42
}
]
],
"definition": {
"name": "my_user_feature",
"introduced_by_url": "https://gitlab.com/gitlab-org/gitlab/-/merge_requests/40880",
"rollout_issue_url": "https://gitlab.com/gitlab-org/gitlab/-/issues/244905",
"group": "group::ci",
"type": "development",
"default_enabled": false
}
}
```
......
......@@ -18,7 +18,10 @@ RSpec.describe API::Features, stub_feature_flags: false do
end
describe 'POST /feature' do
let(:feature_name) { 'my_feature' }
let(:feature_name) do
Feature::Definition.definitions
.values.find(&:development?).name
end
context 'when running on a Geo primary node' do
before do
......@@ -43,6 +46,14 @@ RSpec.describe API::Features, stub_feature_flags: false do
expect(response).to have_gitlab_http_status(:bad_request)
end
context 'when force=1 is set' do
it 'allows to change state' do
post api("/features/#{feature_name}", admin), params: { value: 'true', force: true }
expect(response).to have_gitlab_http_status(:created)
end
end
end
end
......
......@@ -17,6 +17,16 @@ module API
{ key: gate.key, value: value }
end.compact
end
class Definition < Grape::Entity
::Feature::Definition::PARAMS.each do |param|
expose param
end
end
expose :definition, using: Definition do |feature|
::Feature::Definition.definitions[feature.name.to_sym]
end
end
end
end
......@@ -46,6 +46,15 @@ module API
present features, with: Entities::Feature, current_user: current_user
end
desc 'Get a list of all feature definitions' do
success Entities::Feature::Definition
end
get :definitions do
definitions = ::Feature::Definition.definitions.values.map(&:to_h)
present definitions, with: Entities::Feature::Definition, current_user: current_user
end
desc 'Set the gate value for the given feature' do
success Entities::Feature
end
......@@ -56,6 +65,7 @@ module API
optional :user, type: String, desc: 'A GitLab username'
optional :group, type: String, desc: "A GitLab group's path, such as 'gitlab-org'"
optional :project, type: String, desc: 'A projects path, like gitlab-org/gitlab-ce'
optional :force, type: Boolean, desc: 'Skip feature flag validation checks, ie. YAML definition'
mutually_exclusive :key, :feature_group
mutually_exclusive :key, :user
......@@ -63,7 +73,7 @@ module API
mutually_exclusive :key, :project
end
post ':name' do
validate_feature_flag_name!(params[:name])
validate_feature_flag_name!(params[:name]) unless params[:force]
feature = Feature.get(params[:name]) # rubocop:disable Gitlab/AvoidFeatureGet
targets = gate_targets(params)
......
......@@ -136,8 +136,6 @@ class Feature
end
def register_definitions
return unless check_feature_flags_definition?
Feature::Definition.reload!
end
......
......@@ -13,6 +13,12 @@ class Feature
end
end
TYPES.each do |type, _|
define_method("#{type}?") do
attributes[:type].to_sym == type
end
end
def initialize(path, opts = {})
@path = path
@attributes = {}
......@@ -94,6 +100,10 @@ class Feature
@definitions = load_all!
end
def has_definition?(key)
definitions.has_key?(key.to_sym)
end
def valid_usage!(key, type:, default_enabled:)
if definition = definitions[key.to_sym]
definition.valid_usage!(type_in_code: type, default_enabled_in_code: default_enabled)
......@@ -119,10 +129,6 @@ class Feature
private
def load_all!
# We currently do not load feature flag definitions
# in production environments
return [] unless Gitlab.dev_or_test_env?
paths.each_with_object({}) do |glob_path, definitions|
load_all_from_path!(definitions, glob_path)
end
......
......@@ -6,6 +6,18 @@ RSpec.describe API::Features, stub_feature_flags: false do
let_it_be(:user) { create(:user) }
let_it_be(:admin) { create(:admin) }
# Find any `development` feature flag name
let(:known_feature_flag) do
Feature::Definition.definitions
.values.find(&:development?)
end
let(:known_feature_flag_definition_hash) do
a_hash_including(
'type' => 'development'
)
end
before do
Feature.reset
Flipper.unregister_groups
......@@ -22,12 +34,14 @@ RSpec.describe API::Features, stub_feature_flags: false do
{
'name' => 'feature_1',
'state' => 'on',
'gates' => [{ 'key' => 'boolean', 'value' => true }]
'gates' => [{ 'key' => 'boolean', 'value' => true }],
'definition' => nil
},
{
'name' => 'feature_2',
'state' => 'off',
'gates' => [{ 'key' => 'boolean', 'value' => false }]
'gates' => [{ 'key' => 'boolean', 'value' => false }],
'definition' => nil
},
{
'name' => 'feature_3',
......@@ -35,7 +49,14 @@ RSpec.describe API::Features, stub_feature_flags: false do
'gates' => [
{ 'key' => 'boolean', 'value' => false },
{ 'key' => 'groups', 'value' => ['perf_team'] }
]
],
'definition' => nil
},
{
'name' => known_feature_flag.name,
'state' => 'on',
'gates' => [{ 'key' => 'boolean', 'value' => true }],
'definition' => known_feature_flag_definition_hash
}
]
end
......@@ -44,6 +65,7 @@ RSpec.describe API::Features, stub_feature_flags: false do
Feature.enable('feature_1')
Feature.disable('feature_2')
Feature.enable('feature_3', Feature.group(:perf_team))
Feature.enable(known_feature_flag.name)
end
it 'returns a 401 for anonymous users' do
......@@ -67,7 +89,7 @@ RSpec.describe API::Features, stub_feature_flags: false do
end
describe 'POST /feature' do
let(:feature_name) { 'my_feature' }
let(:feature_name) { known_feature_flag.name }
context 'when the feature does not exist' do
it 'returns a 401 for anonymous users' do
......@@ -87,43 +109,49 @@ RSpec.describe API::Features, stub_feature_flags: false do
post api("/features/#{feature_name}", admin), params: { value: 'true' }
expect(response).to have_gitlab_http_status(:created)
expect(json_response).to eq(
'name' => 'my_feature',
expect(json_response).to match(
'name' => feature_name,
'state' => 'on',
'gates' => [{ 'key' => 'boolean', 'value' => true }])
'gates' => [{ 'key' => 'boolean', 'value' => true }],
'definition' => known_feature_flag_definition_hash
)
end
it 'creates an enabled feature for the given Flipper group when passed feature_group=perf_team' do
post api("/features/#{feature_name}", admin), params: { value: 'true', feature_group: 'perf_team' }
expect(response).to have_gitlab_http_status(:created)
expect(json_response).to eq(
'name' => 'my_feature',
expect(json_response).to match(
'name' => feature_name,
'state' => 'conditional',
'gates' => [
{ 'key' => 'boolean', 'value' => false },
{ 'key' => 'groups', 'value' => ['perf_team'] }
])
],
'definition' => known_feature_flag_definition_hash
)
end
it 'creates an enabled feature for the given user when passed user=username' do
post api("/features/#{feature_name}", admin), params: { value: 'true', user: user.username }
expect(response).to have_gitlab_http_status(:created)
expect(json_response).to eq(
'name' => 'my_feature',
expect(json_response).to match(
'name' => feature_name,
'state' => 'conditional',
'gates' => [
{ 'key' => 'boolean', 'value' => false },
{ 'key' => 'actors', 'value' => ["User:#{user.id}"] }
])
],
'definition' => known_feature_flag_definition_hash
)
end
it 'creates an enabled feature for the given user and feature group when passed user=username and feature_group=perf_team' do
post api("/features/#{feature_name}", admin), params: { value: 'true', user: user.username, feature_group: 'perf_team' }
expect(response).to have_gitlab_http_status(:created)
expect(json_response['name']).to eq('my_feature')
expect(json_response['name']).to eq(feature_name)
expect(json_response['state']).to eq('conditional')
expect(json_response['gates']).to contain_exactly(
{ 'key' => 'boolean', 'value' => false },
......@@ -141,13 +169,15 @@ RSpec.describe API::Features, stub_feature_flags: false do
post api("/features/#{feature_name}", admin), params: { value: 'true', project: project.full_path }
expect(response).to have_gitlab_http_status(:created)
expect(json_response).to eq(
'name' => 'my_feature',
expect(json_response).to match(
'name' => feature_name,
'state' => 'conditional',
'gates' => [
{ 'key' => 'boolean', 'value' => false },
{ 'key' => 'actors', 'value' => ["Project:#{project.id}"] }
])
],
'definition' => known_feature_flag_definition_hash
)
end
end
......@@ -156,12 +186,13 @@ RSpec.describe API::Features, stub_feature_flags: false do
post api("/features/#{feature_name}", admin), params: { value: 'true', project: 'mep/to/the/mep/mep' }
expect(response).to have_gitlab_http_status(:created)
expect(json_response).to eq(
"name" => "my_feature",
expect(json_response).to match(
"name" => feature_name,
"state" => "off",
"gates" => [
{ "key" => "boolean", "value" => false }
]
],
'definition' => known_feature_flag_definition_hash
)
end
end
......@@ -175,13 +206,15 @@ RSpec.describe API::Features, stub_feature_flags: false do
post api("/features/#{feature_name}", admin), params: { value: 'true', group: group.full_path }
expect(response).to have_gitlab_http_status(:created)
expect(json_response).to eq(
'name' => 'my_feature',
expect(json_response).to match(
'name' => feature_name,
'state' => 'conditional',
'gates' => [
{ 'key' => 'boolean', 'value' => false },
{ 'key' => 'actors', 'value' => ["Group:#{group.id}"] }
])
],
'definition' => known_feature_flag_definition_hash
)
end
end
......@@ -190,12 +223,13 @@ RSpec.describe API::Features, stub_feature_flags: false do
post api("/features/#{feature_name}", admin), params: { value: 'true', group: 'not/a/group' }
expect(response).to have_gitlab_http_status(:created)
expect(json_response).to eq(
"name" => "my_feature",
expect(json_response).to match(
"name" => feature_name,
"state" => "off",
"gates" => [
{ "key" => "boolean", "value" => false }
]
],
'definition' => known_feature_flag_definition_hash
)
end
end
......@@ -205,26 +239,30 @@ RSpec.describe API::Features, stub_feature_flags: false do
post api("/features/#{feature_name}", admin), params: { value: '50' }
expect(response).to have_gitlab_http_status(:created)
expect(json_response).to eq(
'name' => 'my_feature',
expect(json_response).to match(
'name' => feature_name,
'state' => 'conditional',
'gates' => [
{ 'key' => 'boolean', 'value' => false },
{ 'key' => 'percentage_of_time', 'value' => 50 }
])
],
'definition' => known_feature_flag_definition_hash
)
end
it 'creates a feature with the given percentage of actors if passed an integer' do
post api("/features/#{feature_name}", admin), params: { value: '50', key: 'percentage_of_actors' }
expect(response).to have_gitlab_http_status(:created)
expect(json_response).to eq(
'name' => 'my_feature',
expect(json_response).to match(
'name' => feature_name,
'state' => 'conditional',
'gates' => [
{ 'key' => 'boolean', 'value' => false },
{ 'key' => 'percentage_of_actors', 'value' => 50 }
])
],
'definition' => known_feature_flag_definition_hash
)
end
end
......@@ -238,36 +276,42 @@ RSpec.describe API::Features, stub_feature_flags: false do
post api("/features/#{feature_name}", admin), params: { value: 'true' }
expect(response).to have_gitlab_http_status(:created)
expect(json_response).to eq(
'name' => 'my_feature',
expect(json_response).to match(
'name' => feature_name,
'state' => 'on',
'gates' => [{ 'key' => 'boolean', 'value' => true }])
'gates' => [{ 'key' => 'boolean', 'value' => true }],
'definition' => known_feature_flag_definition_hash
)
end
it 'enables the feature for the given Flipper group when passed feature_group=perf_team' do
post api("/features/#{feature_name}", admin), params: { value: 'true', feature_group: 'perf_team' }
expect(response).to have_gitlab_http_status(:created)
expect(json_response).to eq(
'name' => 'my_feature',
expect(json_response).to match(
'name' => feature_name,
'state' => 'conditional',
'gates' => [
{ 'key' => 'boolean', 'value' => false },
{ 'key' => 'groups', 'value' => ['perf_team'] }
])
],
'definition' => known_feature_flag_definition_hash
)
end
it 'enables the feature for the given user when passed user=username' do
post api("/features/#{feature_name}", admin), params: { value: 'true', user: user.username }
expect(response).to have_gitlab_http_status(:created)
expect(json_response).to eq(
'name' => 'my_feature',
expect(json_response).to match(
'name' => feature_name,
'state' => 'conditional',
'gates' => [
{ 'key' => 'boolean', 'value' => false },
{ 'key' => 'actors', 'value' => ["User:#{user.id}"] }
])
],
'definition' => known_feature_flag_definition_hash
)
end
end
......@@ -279,10 +323,12 @@ RSpec.describe API::Features, stub_feature_flags: false do
post api("/features/#{feature_name}", admin), params: { value: 'false' }
expect(response).to have_gitlab_http_status(:created)
expect(json_response).to eq(
'name' => 'my_feature',
expect(json_response).to match(
'name' => feature_name,
'state' => 'off',
'gates' => [{ 'key' => 'boolean', 'value' => false }])
'gates' => [{ 'key' => 'boolean', 'value' => false }],
'definition' => known_feature_flag_definition_hash
)
end
it 'disables the feature for the given Flipper group when passed feature_group=perf_team' do
......@@ -292,10 +338,12 @@ RSpec.describe API::Features, stub_feature_flags: false do
post api("/features/#{feature_name}", admin), params: { value: 'false', feature_group: 'perf_team' }
expect(response).to have_gitlab_http_status(:created)
expect(json_response).to eq(
'name' => 'my_feature',
expect(json_response).to match(
'name' => feature_name,
'state' => 'off',
'gates' => [{ 'key' => 'boolean', 'value' => false }])
'gates' => [{ 'key' => 'boolean', 'value' => false }],
'definition' => known_feature_flag_definition_hash
)
end
it 'disables the feature for the given user when passed user=username' do
......@@ -305,10 +353,12 @@ RSpec.describe API::Features, stub_feature_flags: false do
post api("/features/#{feature_name}", admin), params: { value: 'false', user: user.username }
expect(response).to have_gitlab_http_status(:created)
expect(json_response).to eq(
'name' => 'my_feature',
expect(json_response).to match(
'name' => feature_name,
'state' => 'off',
'gates' => [{ 'key' => 'boolean', 'value' => false }])
'gates' => [{ 'key' => 'boolean', 'value' => false }],
'definition' => known_feature_flag_definition_hash
)
end
end
......@@ -321,13 +371,15 @@ RSpec.describe API::Features, stub_feature_flags: false do
post api("/features/#{feature_name}", admin), params: { value: '30' }
expect(response).to have_gitlab_http_status(:created)
expect(json_response).to eq(
'name' => 'my_feature',
expect(json_response).to match(
'name' => feature_name,
'state' => 'conditional',
'gates' => [
{ 'key' => 'boolean', 'value' => false },
{ 'key' => 'percentage_of_time', 'value' => 30 }
])
],
'definition' => known_feature_flag_definition_hash
)
end
end
......@@ -340,13 +392,15 @@ RSpec.describe API::Features, stub_feature_flags: false do
post api("/features/#{feature_name}", admin), params: { value: '74', key: 'percentage_of_actors' }
expect(response).to have_gitlab_http_status(:created)
expect(json_response).to eq(
'name' => 'my_feature',
expect(json_response).to match(
'name' => feature_name,
'state' => 'conditional',
'gates' => [
{ 'key' => 'boolean', 'value' => false },
{ 'key' => 'percentage_of_actors', 'value' => 74 }
])
],
'definition' => known_feature_flag_definition_hash
)
end
end
end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment