Merge branch 'ee-34078-allow-to-enable-feature-flags-with-more-granularity' into 'master'

[EE] Allow the feature flags to be enabled/disabled with more granularity

See merge request !2290

app/models/concerns/feature_gate.rb

+module FeatureGate
+  def flipper_id
+    return nil if new_record?
+
+    "#{self.class.name}:#{id}"
+  end
+end

app/models/user.rb

@@ -11,6 +11,7 @@ class User < ActiveRecord::Base
   include CaseSensitivity
   include TokenAuthenticatable
   include IgnorableColumn
+  include FeatureGate
   prepend EE::GeoAwareAvatar
   prepend EE::User
 

changelogs/unreleased/34078-allow-to-enable-feature-flags-with-more-granularity.yml

+---
+title: Allow the feature flags to be enabled/disabled with more granularity
+merge_request: 12357
+author:

doc/api/features.md

@@ -58,6 +58,10 @@ 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 |
+| `feature_group` | string | no | A Feature group name |
+| `user` | string | no | A GitLab username |
+
+Note that `feature_group` and `user` are mutually exclusive.
 
 ```bash
 curl --data "value=30" --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v4/features/new_library

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[:feature_group]
+          Feature.group(params[:feature_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,22 @@ module API
       end
       params do
         requires :value, type: String, desc: '`true` or `false` to enable/disable, an integer for percentage of time'
+        optional :feature_group, type: String, desc: 'A Feature group name'
+        optional :user, type: String, desc: 'A GitLab username'
+        mutually_exclusive :feature_group, :user
       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

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

spec/models/concerns/feature_gate_spec.rb

+require 'spec_helper'
+
+describe FeatureGate do
+  describe 'User' do
+    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
+  end
+end

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 feature_group=perf_team' do
+          post api("/features/#{feature_name}", admin), value: 'true', feature_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 feature_group=perf_team' do
+          post api("/features/#{feature_name}", admin), value: 'true', feature_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 feature_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', feature_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