summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--app/models/concerns/flippable.rb7
-rw-r--r--app/models/user.rb1
-rw-r--r--changelogs/unreleased/34078-allow-to-enable-feature-flags-with-more-granularity.yml4
-rw-r--r--doc/api/features.md2
-rw-r--r--lib/api/features.rb38
-rw-r--r--lib/feature.rb22
-rw-r--r--spec/models/user_spec.rb14
-rw-r--r--spec/requests/api/features_spec.rb178
8 files changed, 233 insertions, 33 deletions
diff --git a/app/models/concerns/flippable.rb b/app/models/concerns/flippable.rb
new file mode 100644
index 00000000000..341501e8250
--- /dev/null
+++ b/app/models/concerns/flippable.rb
@@ -0,0 +1,7 @@
+module Flippable
+ def flipper_id
+ return nil if new_record?
+
+ "#{self.class.name}:#{id}"
+ end
+end
diff --git a/app/models/user.rb b/app/models/user.rb
index 6dd1b1415d6..bcce260ab08 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -11,6 +11,7 @@ class User < ActiveRecord::Base
include CaseSensitivity
include TokenAuthenticatable
include IgnorableColumn
+ include Flippable
DEFAULT_NOTIFICATION_LEVEL = :participating
diff --git a/changelogs/unreleased/34078-allow-to-enable-feature-flags-with-more-granularity.yml b/changelogs/unreleased/34078-allow-to-enable-feature-flags-with-more-granularity.yml
new file mode 100644
index 00000000000..8ead1b404f3
--- /dev/null
+++ b/changelogs/unreleased/34078-allow-to-enable-feature-flags-with-more-granularity.yml
@@ -0,0 +1,4 @@
+---
+title: Allow the feature flags to be enabled/disabled with more granularity
+merge_request:
+author:
diff --git a/doc/api/features.md b/doc/api/features.md
index 89b8d3ac948..0ca2e637614 100644
--- a/doc/api/features.md
+++ b/doc/api/features.md
@@ -58,6 +58,8 @@ POST /features/:name
| --------- | ---- | -------- | ----------- |
| `name` | string | yes | Name of the feature to create or update |
| `value` | integer/string | yes | `true` or `false` to enable/disable, or an integer for percentage of time |
+| `flipper_group` | string | no | A Flipper group name |
+| `user` | string | no | A GitLab username |
```bash
curl --data "value=30" --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v4/features/new_library
diff --git a/lib/api/features.rb b/lib/api/features.rb
index cff0ba2ddff..4e10e36fce9 100644
--- a/lib/api/features.rb
+++ b/lib/api/features.rb
@@ -2,6 +2,29 @@ module API
class Features < Grape::API
before { authenticated_as_admin! }
+ helpers do
+ def gate_value(params)
+ case params[:value]
+ when 'true'
+ true
+ when '0', 'false'
+ false
+ else
+ params[:value].to_i
+ end
+ end
+
+ def gate_target(params)
+ if params[:flipper_group]
+ Feature.group(params[:flipper_group])
+ elsif params[:user]
+ User.find_by_username(params[:user])
+ else
+ gate_value(params)
+ end
+ end
+ end
+
resource :features do
desc 'Get a list of all features' do
success Entities::Feature
@@ -17,16 +40,21 @@ module API
end
params do
requires :value, type: String, desc: '`true` or `false` to enable/disable, an integer for percentage of time'
+ optional :flipper_group, type: String, desc: 'A Flipper group name'
+ optional :user, type: String, desc: 'A GitLab username'
end
post ':name' do
feature = Feature.get(params[:name])
+ target = gate_target(params)
+ value = gate_value(params)
- if %w(0 false).include?(params[:value])
- feature.disable
- elsif params[:value] == 'true'
- feature.enable
+ case value
+ when true
+ feature.enable(target)
+ when false
+ feature.disable(target)
else
- feature.enable_percentage_of_time(params[:value].to_i)
+ feature.enable_percentage_of_time(value)
end
present feature, with: Entities::Feature, current_user: current_user
diff --git a/lib/feature.rb b/lib/feature.rb
index d3d972564af..363f66ba60e 100644
--- a/lib/feature.rb
+++ b/lib/feature.rb
@@ -12,6 +12,8 @@ class Feature
end
class << self
+ delegate :group, to: :flipper
+
def all
flipper.features.to_a
end
@@ -27,16 +29,24 @@ class Feature
all.map(&:name).include?(feature.name)
end
- def enabled?(key)
- get(key).enabled?
+ def enabled?(key, thing = nil)
+ get(key).enabled?(thing)
+ end
+
+ def enable(key, thing = true)
+ get(key).enable(thing)
+ end
+
+ def disable(key, thing = false)
+ get(key).disable(thing)
end
- def enable(key)
- get(key).enable
+ def enable_group(key, group)
+ get(key).enable_group(group)
end
- def disable(key)
- get(key).disable
+ def disable_group(key, group)
+ get(key).disable_group(group)
end
def flipper
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index 8e895ec6634..05ba887c51f 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -430,6 +430,20 @@ describe User, models: true do
end
end
+ describe '#flipper_id' do
+ context 'when user is not persisted' do
+ let(:user) { build(:user) }
+
+ it { expect(user.flipper_id).to be_nil }
+ end
+
+ context 'when user is persisted' do
+ let(:user) { create(:user) }
+
+ it { expect(user.flipper_id).to eq "User:#{user.id}" }
+ end
+ end
+
describe '#generate_password' do
it "does not generate password by default" do
user = create(:user, password: 'abcdefghe')
diff --git a/spec/requests/api/features_spec.rb b/spec/requests/api/features_spec.rb
index f169e6661d1..0ee0749c7a1 100644
--- a/spec/requests/api/features_spec.rb
+++ b/spec/requests/api/features_spec.rb
@@ -4,6 +4,13 @@ describe API::Features do
let(:user) { create(:user) }
let(:admin) { create(:admin) }
+ before do
+ Flipper.unregister_groups
+ Flipper.register(:perf_team) do |actor|
+ actor.respond_to?(:admin) && actor.admin?
+ end
+ end
+
describe 'GET /features' do
let(:expected_features) do
[
@@ -16,6 +23,14 @@ describe API::Features do
'name' => 'feature_2',
'state' => 'off',
'gates' => [{ 'key' => 'boolean', 'value' => false }]
+ },
+ {
+ 'name' => 'feature_3',
+ 'state' => 'conditional',
+ 'gates' => [
+ { 'key' => 'boolean', 'value' => false },
+ { 'key' => 'groups', 'value' => ['perf_team'] }
+ ]
}
]
end
@@ -23,6 +38,7 @@ describe API::Features do
before do
Feature.get('feature_1').enable
Feature.get('feature_2').disable
+ Feature.get('feature_3').enable Feature.group(:perf_team)
end
it 'returns a 401 for anonymous users' do
@@ -47,30 +63,70 @@ describe API::Features do
describe 'POST /feature' do
let(:feature_name) { 'my_feature' }
- it 'returns a 401 for anonymous users' do
- post api("/features/#{feature_name}")
- expect(response).to have_http_status(401)
- end
+ context 'when the feature does not exist' do
+ it 'returns a 401 for anonymous users' do
+ post api("/features/#{feature_name}")
- it 'returns a 403 for users' do
- post api("/features/#{feature_name}", user)
+ expect(response).to have_http_status(401)
+ end
- expect(response).to have_http_status(403)
- end
+ it 'returns a 403 for users' do
+ post api("/features/#{feature_name}", user)
- it 'creates an enabled feature if passed true' do
- post api("/features/#{feature_name}", admin), value: 'true'
+ expect(response).to have_http_status(403)
+ end
- expect(response).to have_http_status(201)
- expect(Feature.get(feature_name)).to be_enabled
- end
+ context 'when passed value=true' do
+ it 'creates an enabled feature' do
+ post api("/features/#{feature_name}", admin), value: 'true'
- it 'creates a feature with the given percentage if passed an integer' do
- post api("/features/#{feature_name}", admin), value: '50'
+ expect(response).to have_http_status(201)
+ expect(json_response).to eq(
+ 'name' => 'my_feature',
+ 'state' => 'on',
+ 'gates' => [{ 'key' => 'boolean', 'value' => true }])
+ end
+
+ it 'creates an enabled feature for the given Flipper group when passed flipper_group=perf_team' do
+ post api("/features/#{feature_name}", admin), value: 'true', flipper_group: 'perf_team'
+
+ expect(response).to have_http_status(201)
+ expect(json_response).to eq(
+ 'name' => 'my_feature',
+ 'state' => 'conditional',
+ 'gates' => [
+ { 'key' => 'boolean', 'value' => false },
+ { 'key' => 'groups', 'value' => ['perf_team'] }
+ ])
+ end
+
+ it 'creates an enabled feature for the given user when passed user=username' do
+ post api("/features/#{feature_name}", admin), value: 'true', user: user.username
- expect(response).to have_http_status(201)
- expect(Feature.get(feature_name).percentage_of_time_value).to be(50)
+ expect(response).to have_http_status(201)
+ expect(json_response).to eq(
+ 'name' => 'my_feature',
+ 'state' => 'conditional',
+ 'gates' => [
+ { 'key' => 'boolean', 'value' => false },
+ { 'key' => 'actors', 'value' => ["User:#{user.id}"] }
+ ])
+ end
+ end
+
+ it 'creates a feature with the given percentage if passed an integer' do
+ post api("/features/#{feature_name}", admin), value: '50'
+
+ expect(response).to have_http_status(201)
+ expect(json_response).to eq(
+ 'name' => 'my_feature',
+ 'state' => 'conditional',
+ 'gates' => [
+ { 'key' => 'boolean', 'value' => false },
+ { 'key' => 'percentage_of_time', 'value' => 50 }
+ ])
+ end
end
context 'when the feature exists' do
@@ -80,11 +136,83 @@ describe API::Features do
feature.disable # This also persists the feature on the DB
end
- it 'enables the feature if passed true' do
- post api("/features/#{feature_name}", admin), value: 'true'
+ context 'when passed value=true' do
+ it 'enables the feature' do
+ post api("/features/#{feature_name}", admin), value: 'true'
- expect(response).to have_http_status(201)
- expect(feature).to be_enabled
+ expect(response).to have_http_status(201)
+ expect(json_response).to eq(
+ 'name' => 'my_feature',
+ 'state' => 'on',
+ 'gates' => [{ 'key' => 'boolean', 'value' => true }])
+ end
+
+ it 'enables the feature for the given Flipper group when passed flipper_group=perf_team' do
+ post api("/features/#{feature_name}", admin), value: 'true', flipper_group: 'perf_team'
+
+ expect(response).to have_http_status(201)
+ expect(json_response).to eq(
+ 'name' => 'my_feature',
+ 'state' => 'conditional',
+ 'gates' => [
+ { 'key' => 'boolean', 'value' => false },
+ { 'key' => 'groups', 'value' => ['perf_team'] }
+ ])
+ end
+
+ it 'enables the feature for the given user when passed user=username' do
+ post api("/features/#{feature_name}", admin), value: 'true', user: user.username
+
+ expect(response).to have_http_status(201)
+ expect(json_response).to eq(
+ 'name' => 'my_feature',
+ 'state' => 'conditional',
+ 'gates' => [
+ { 'key' => 'boolean', 'value' => false },
+ { 'key' => 'actors', 'value' => ["User:#{user.id}"] }
+ ])
+ end
+ end
+
+ context 'when feature is enabled and value=false is passed' do
+ it 'disables the feature' do
+ feature.enable
+ expect(feature).to be_enabled
+
+ post api("/features/#{feature_name}", admin), value: 'false'
+
+ expect(response).to have_http_status(201)
+ expect(json_response).to eq(
+ 'name' => 'my_feature',
+ 'state' => 'off',
+ 'gates' => [{ 'key' => 'boolean', 'value' => false }])
+ end
+
+ it 'disables the feature for the given Flipper group when passed flipper_group=perf_team' do
+ feature.enable(Feature.group(:perf_team))
+ expect(Feature.get(feature_name).enabled?(admin)).to be_truthy
+
+ post api("/features/#{feature_name}", admin), value: 'false', flipper_group: 'perf_team'
+
+ expect(response).to have_http_status(201)
+ expect(json_response).to eq(
+ 'name' => 'my_feature',
+ 'state' => 'off',
+ 'gates' => [{ 'key' => 'boolean', 'value' => false }])
+ end
+
+ it 'disables the feature for the given user when passed user=username' do
+ feature.enable(user)
+ expect(Feature.get(feature_name).enabled?(user)).to be_truthy
+
+ post api("/features/#{feature_name}", admin), value: 'false', user: user.username
+
+ expect(response).to have_http_status(201)
+ expect(json_response).to eq(
+ 'name' => 'my_feature',
+ 'state' => 'off',
+ 'gates' => [{ 'key' => 'boolean', 'value' => false }])
+ end
end
context 'with a pre-existing percentage value' do
@@ -96,7 +224,13 @@ describe API::Features do
post api("/features/#{feature_name}", admin), value: '30'
expect(response).to have_http_status(201)
- expect(Feature.get(feature_name).percentage_of_time_value).to be(30)
+ expect(json_response).to eq(
+ 'name' => 'my_feature',
+ 'state' => 'conditional',
+ 'gates' => [
+ { 'key' => 'boolean', 'value' => false },
+ { 'key' => 'percentage_of_time', 'value' => 30 }
+ ])
end
end
end
View Lorry Controller Status