Merge branch 'unleash-api-new-strategies' into 'master'

Support New Feature Flags in Unleash API

See merge request gitlab-org/gitlab!26191

ee/app/models/operations/feature_flag.rb

@@ -50,6 +50,14 @@ module Operations
       def preload_relations
         preload(:scopes)
       end
+
+      def for_unleash_client(project, environment)
+        includes(strategies: :scopes)
+          .where(project: project)
+          .merge(Operations::FeatureFlags::Scope.on_environment(environment))
+          .reorder(:id)
+          .references(:operations_scopes)
+      end
     end
 
     private

ee/app/models/operations/feature_flags/scope.rb

+# frozen_string_literal: true
+
+module Operations
+  module FeatureFlags
+    class Scope < ApplicationRecord
+      prepend HasEnvironmentScope
+
+      self.table_name = 'operations_scopes'
+
+      belongs_to :strategy, class_name: 'Operations::FeatureFlags::Strategy'
+    end
+  end
+end

ee/app/models/operations/feature_flags/strategy.rb

@@ -16,6 +16,7 @@ module Operations
       self.table_name = 'operations_strategies'
 
       belongs_to :feature_flag
+      has_many :scopes, class_name: 'Operations::FeatureFlags::Scope'
 
       validates :name,
         inclusion: {

ee/lib/api/unleash.rb

@@ -72,8 +72,13 @@ module API
       def feature_flags
         return [] unless unleash_app_name.present?
 
+        if Feature.enabled?(:feature_flags_new_version, project)
+          Operations::FeatureFlagScope.for_unleash_client(project, unleash_app_name) +
+            Operations::FeatureFlag.for_unleash_client(project, unleash_app_name)
+        else
           Operations::FeatureFlagScope.for_unleash_client(project, unleash_app_name)
         end
       end
     end
+  end
 end

ee/lib/ee/api/entities/unleash_feature.rb

@@ -7,7 +7,7 @@ module EE
         expose :name
         expose :description, unless: ->(feature) { feature.description.nil? }
         expose :active, as: :enabled
-        expose :strategies
+        expose :strategies, using: UnleashStrategy
       end
     end
   end

ee/lib/ee/api/entities/unleash_strategy.rb

+# frozen_string_literal: true
+
+module EE
+  module API
+    module Entities
+      class UnleashStrategy < Grape::Entity
+        expose :name do |strategy|
+          if strategy.respond_to?(:name)
+            strategy.name
+          else
+            strategy['name']
+          end
+        end
+        expose :parameters do |strategy|
+          if strategy.respond_to?(:parameters)
+            strategy.parameters
+          else
+            strategy['parameters']
+          end
+        end
+      end
+    end
+  end
+end

ee/spec/factories/operations/feature_flags/scope.rb

+# frozen_string_literal: true
+
+FactoryBot.define do
+  factory :operations_scope, class: 'Operations::FeatureFlags::Scope' do
+    association :strategy, factory: :operations_strategy
+    sequence(:environment_scope) { |n| "review/patch-#{n}" }
+  end
+end

ee/spec/factories/operations/feature_flags/strategy.rb

+# frozen_string_literal: true
+
+FactoryBot.define do
+  factory :operations_strategy, class: 'Operations::FeatureFlags::Strategy' do
+    association :feature_flag, factory: :operations_feature_flag
+    name { "default" }
+    parameters { {} }
+  end
+end

ee/spec/models/operations/feature_flag_spec.rb

@@ -213,4 +213,45 @@ describe Operations::FeatureFlag do
       end
     end
   end
+
+  describe '.for_unleash_client' do
+    let_it_be(:project) { create(:project) }
+    let!(:feature_flag) do
+      create(:operations_feature_flag, project: project,
+             name: 'feature1', active: true, version: 2)
+    end
+    let!(:strategy) do
+      create(:operations_strategy, feature_flag: feature_flag,
+             name: 'default', parameters: {})
+    end
+
+    it 'matches wild cards in the scope' do
+      create(:operations_scope, strategy: strategy, environment_scope: 'review/*')
+
+      flags = described_class.for_unleash_client(project, 'review/feature-branch')
+
+      expect(flags).to eq([feature_flag])
+    end
+
+    it 'matches wild cards case sensitively' do
+      create(:operations_scope, strategy: strategy, environment_scope: 'Staging/*')
+
+      flags = described_class.for_unleash_client(project, 'staging/feature')
+
+      expect(flags).to eq([])
+    end
+
+    it 'returns feature flags ordered by id' do
+      create(:operations_scope, strategy: strategy, environment_scope: 'production')
+      feature_flag_b = create(:operations_feature_flag, project: project,
+                              name: 'feature2', active: true, version: 2)
+      strategy_b = create(:operations_strategy, feature_flag: feature_flag_b,
+                          name: 'default', parameters: {})
+      create(:operations_scope, strategy: strategy_b, environment_scope: '*')
+
+      flags = described_class.for_unleash_client(project, 'production')
+
+      expect(flags.map(&:id)).to eq([feature_flag.id, feature_flag_b.id])
+    end
+  end
 end

ee/spec/requests/api/unleash_spec.rb

@@ -153,17 +153,20 @@ describe API::Unleash do
     describe "GET #{features_endpoint}" do
       let(:features_url) { features_endpoint.sub(':project_id', project_id.to_s) }
       let(:client) { create(:operations_feature_flags_client, project: project) }
-      let(:feature_flag) { create(:operations_feature_flag, project: project, name: 'feature1', active: true) }
 
       subject { get api(features_url), params: params, headers: headers }
 
       it_behaves_like 'authenticated request'
+
+      context 'with version 1 (legacy) feature flags' do
+        let(:feature_flag) { create(:operations_feature_flag, project: project, name: 'feature1', active: true, version: 1) }
+
         it_behaves_like 'support multiple environments'
 
         context 'with a list of feature flags' do
-        let(:headers) { { "UNLEASH-INSTANCEID" => client.token, "UNLEASH-APPNAME" => "production" }}
-        let!(:enable_feature_flag) { create(:operations_feature_flag, project: project, name: 'feature1', active: true) }
-        let!(:disabled_feature_flag) { create(:operations_feature_flag, project: project, name: 'feature2', active: false) }
+          let(:headers) { { "UNLEASH-INSTANCEID" => client.token, "UNLEASH-APPNAME" => "production" } }
+          let!(:enabled_feature_flag) { create(:operations_feature_flag, project: project, name: 'feature1', active: true, version: 1) }
+          let!(:disabled_feature_flag) { create(:operations_feature_flag, project: project, name: 'feature2', active: false, version: 1) }
 
           it 'responds with a list of features' do
             subject
@@ -248,7 +251,7 @@ describe API::Unleash do
         end
 
         it 'returns a disabled feature when the flag is disabled' do
-        flag = create(:operations_feature_flag, project: project, name: 'test_feature', active: false)
+          flag = create(:operations_feature_flag, project: project, name: 'test_feature', active: false, version: 1)
           create(:operations_feature_flag_scope, feature_flag: flag, environment_scope: 'production', active: true)
           headers = { "UNLEASH-INSTANCEID" => client.token, "UNLEASH-APPNAME" => "production" }
 
@@ -272,6 +275,323 @@ describe API::Unleash do
           end
         end
       end
+
+      context 'with version 2 feature flags' do
+        it 'does not return any flags when the feature flag is disabled' do
+          stub_feature_flags(feature_flags_new_version: false)
+          feature_flag = create(:operations_feature_flag, project: project,
+                                name: 'feature1', active: true, version: 2)
+          strategy = create(:operations_strategy, feature_flag: feature_flag,
+                            name: 'default', parameters: {})
+          create(:operations_scope, strategy: strategy, environment_scope: 'production')
+
+          get api(features_url), headers: { 'UNLEASH-INSTANCEID' => client.token, 'UNLEASH-APPNAME' => 'production' }
+
+          expect(response).to have_gitlab_http_status(:ok)
+          expect(json_response['features']).to eq([])
+        end
+
+        it 'does not return a flag without any strategies' do
+          create(:operations_feature_flag, project: project,
+                 name: 'feature1', active: true, version: 2)
+
+          get api(features_url), headers: { 'UNLEASH-INSTANCEID' => client.token, 'UNLEASH-APPNAME' => 'production' }
+
+          expect(response).to have_gitlab_http_status(:ok)
+          expect(json_response['features']).to be_empty
+        end
+
+        it 'returns a flag with a default strategy' do
+          feature_flag = create(:operations_feature_flag, project: project,
+                                name: 'feature1', active: true, version: 2)
+          strategy = create(:operations_strategy, feature_flag: feature_flag,
+                            name: 'default', parameters: {})
+          create(:operations_scope, strategy: strategy, environment_scope: 'production')
+
+          get api(features_url), headers: { 'UNLEASH-INSTANCEID' => client.token, 'UNLEASH-APPNAME' => 'production' }
+
+          expect(response).to have_gitlab_http_status(:ok)
+          expect(json_response['features']).to eq([{
+            'name' => 'feature1',
+            'enabled' => true,
+            'strategies' => [{
+              'name' => 'default',
+              'parameters' => {}
+            }]
+          }])
+        end
+
+        it 'returns a flag with a userWithId strategy' do
+          feature_flag = create(:operations_feature_flag, project: project,
+                                name: 'feature1', active: true, version: 2)
+          strategy = create(:operations_strategy, feature_flag: feature_flag,
+                            name: 'userWithId', parameters: { userIds: 'user123,user456' })
+          create(:operations_scope, strategy: strategy, environment_scope: 'production')
+
+          get api(features_url), headers: { 'UNLEASH-INSTANCEID' => client.token, 'UNLEASH-APPNAME' => 'production' }
+
+          expect(response).to have_gitlab_http_status(:ok)
+          expect(json_response['features']).to eq([{
+            'name' => 'feature1',
+            'enabled' => true,
+            'strategies' => [{
+              'name' => 'userWithId',
+              'parameters' => { 'userIds' => 'user123,user456' }
+            }]
+          }])
+        end
+
+        it 'returns a flag with multiple strategies' do
+          feature_flag = create(:operations_feature_flag, project: project,
+                                name: 'feature1', active: true, version: 2)
+          strategy_a = create(:operations_strategy, feature_flag: feature_flag,
+                              name: 'userWithId', parameters: { userIds: 'user_a,user_b' })
+          strategy_b = create(:operations_strategy, feature_flag: feature_flag,
+                              name: 'gradualRolloutUserId', parameters: { groupId: 'default', percentage: '45' })
+          create(:operations_scope, strategy: strategy_a, environment_scope: 'production')
+          create(:operations_scope, strategy: strategy_b, environment_scope: 'production')
+
+          get api(features_url), headers: { 'UNLEASH-INSTANCEID' => client.token, 'UNLEASH-APPNAME' => 'production' }
+
+          expect(response).to have_gitlab_http_status(:ok)
+          expect(json_response['features'].map { |f| f['name'] }.sort).to eq(['feature1'])
+          features_json = json_response['features'].map do |feature|
+            feature.merge(feature.slice('strategies').transform_values { |v| v.sort_by { |s| s['name'] } })
+          end
+          expect(features_json).to eq([{
+            'name' => 'feature1',
+            'enabled' => true,
+            'strategies' => [{
+              'name' => 'gradualRolloutUserId',
+              'parameters' => { 'groupId' => 'default', 'percentage' => '45' }
+            }, {
+              'name' => 'userWithId',
+              'parameters' => { 'userIds' => 'user_a,user_b' }
+            }]
+          }])
+        end
+
+        it 'returns only flags matching the environment scope' do
+          feature_flag_a = create(:operations_feature_flag, project: project,
+                                  name: 'feature1', active: true, version: 2)
+          strategy_a = create(:operations_strategy, feature_flag: feature_flag_a)
+          create(:operations_scope, strategy: strategy_a, environment_scope: 'production')
+          feature_flag_b = create(:operations_feature_flag, project: project,
+                                  name: 'feature2', active: true, version: 2)
+          strategy_b = create(:operations_strategy, feature_flag: feature_flag_b)
+          create(:operations_scope, strategy: strategy_b, environment_scope: 'staging')
+
+          get api(features_url), headers: { 'UNLEASH-INSTANCEID' => client.token, 'UNLEASH-APPNAME' => 'staging' }
+
+          expect(response).to have_gitlab_http_status(:ok)
+          expect(json_response['features'].map { |f| f['name'] }.sort).to eq(['feature2'])
+          expect(json_response['features']).to eq([{
+            'name' => 'feature2',
+            'enabled' => true,
+            'strategies' => [{
+              'name' => 'default',
+              'parameters' => {}
+            }]
+          }])
+        end
+
+        it 'returns only strategies matching the environment scope' do
+          feature_flag = create(:operations_feature_flag, project: project,
+                                name: 'feature1', active: true, version: 2)
+          strategy_a = create(:operations_strategy, feature_flag: feature_flag,
+                              name: 'userWithId', parameters: { userIds: 'user2,user8,user4' })
+          create(:operations_scope, strategy: strategy_a, environment_scope: 'production')
+          strategy_b = create(:operations_strategy, feature_flag: feature_flag,
+                              name: 'default', parameters: {})
+          create(:operations_scope, strategy: strategy_b, environment_scope: 'staging')
+
+          get api(features_url), headers: { 'UNLEASH-INSTANCEID' => client.token, 'UNLEASH-APPNAME' => 'production' }
+
+          expect(response).to have_gitlab_http_status(:ok)
+          expect(json_response['features']).to eq([{
+            'name' => 'feature1',
+            'enabled' => true,
+            'strategies' => [{
+              'name' => 'userWithId',
+              'parameters' => { 'userIds' => 'user2,user8,user4' }
+            }]
+          }])
+        end
+
+        it 'returns only flags for the given project' do
+          project_b = create(:project)
+          feature_flag_a = create(:operations_feature_flag, project: project, name: 'feature_a', active: true, version: 2)
+          strategy_a = create(:operations_strategy, feature_flag: feature_flag_a)
+          create(:operations_scope, strategy: strategy_a, environment_scope: 'sandbox')
+          feature_flag_b = create(:operations_feature_flag, project: project_b, name: 'feature_b', active: true, version: 2)
+          strategy_b = create(:operations_strategy, feature_flag: feature_flag_b)
+          create(:operations_scope, strategy: strategy_b, environment_scope: 'sandbox')
+
+          get api(features_url), headers: { 'UNLEASH-INSTANCEID' => client.token, 'UNLEASH-APPNAME' => 'sandbox' }
+
+          expect(response).to have_gitlab_http_status(:ok)
+          expect(json_response['features']).to eq([{
+            'name' => 'feature_a',
+            'enabled' => true,
+            'strategies' => [{
+              'name' => 'default',
+              'parameters' => {}
+            }]
+          }])
+        end
+
+        it 'returns all strategies with a matching scope' do
+          feature_flag = create(:operations_feature_flag, project: project,
+                                name: 'feature1', active: true, version: 2)
+          strategy_a = create(:operations_strategy, feature_flag: feature_flag,
+                              name: 'userWithId', parameters: { userIds: 'user2,user8,user4' })
+          create(:operations_scope, strategy: strategy_a, environment_scope: '*')
+          strategy_b = create(:operations_strategy, feature_flag: feature_flag,
+                              name: 'default', parameters: {})
+          create(:operations_scope, strategy: strategy_b, environment_scope: 'review/*')
+          strategy_c = create(:operations_strategy, feature_flag: feature_flag,
+                              name: 'gradualRolloutUserId', parameters: { groupId: 'default', percentage: '15' })
+          create(:operations_scope, strategy: strategy_c, environment_scope: 'review/patch-1')
+
+          get api(features_url), headers: { 'UNLEASH-INSTANCEID' => client.token, 'UNLEASH-APPNAME' => 'review/patch-1' }
+
+          expect(response).to have_gitlab_http_status(:ok)
+          expect(json_response['features'].first['strategies'].sort_by { |s| s['name'] }).to eq([{
+            'name' => 'default',
+            'parameters' => {}
+          }, {
+            'name' => 'gradualRolloutUserId',
+            'parameters' => { 'groupId' => 'default', 'percentage' => '15' }
+          }, {
+            'name' => 'userWithId',
+            'parameters' => { 'userIds' => 'user2,user8,user4' }
+          }])
+        end
+
+        it 'returns a strategy with more than one matching scope' do
+          feature_flag = create(:operations_feature_flag, project: project,
+                                name: 'feature1', active: true, version: 2)
+          strategy = create(:operations_strategy, feature_flag: feature_flag,
+                            name: 'default', parameters: {})
+          create(:operations_scope, strategy: strategy, environment_scope: 'production')
+          create(:operations_scope, strategy: strategy, environment_scope: '*')
+
+          get api(features_url), headers: { 'UNLEASH-INSTANCEID' => client.token, 'UNLEASH-APPNAME' => 'production' }
+
+          expect(response).to have_gitlab_http_status(:ok)
+          expect(json_response['features']).to eq([{
+            'name' => 'feature1',
+            'enabled' => true,
+            'strategies' => [{
+              'name' => 'default',
+              'parameters' => {}
+            }]
+          }])
+        end
+
+        it 'returns a disabled flag with a matching scope' do
+          feature_flag = create(:operations_feature_flag, project: project,
+                                name: 'myfeature', active: false, version: 2)
+          strategy = create(:operations_strategy, feature_flag: feature_flag,
+                            name: 'default', parameters: {})
+          create(:operations_scope, strategy: strategy, environment_scope: 'production')
+
+          get api(features_url), headers: { 'UNLEASH-INSTANCEID' => client.token, 'UNLEASH-APPNAME' => 'production' }
+
+          expect(response).to have_gitlab_http_status(:ok)
+          expect(json_response['features']).to eq([{
+            'name' => 'myfeature',
+            'enabled' => false,
+            'strategies' => [{
+              'name' => 'default',
+              'parameters' => {}
+            }]
+          }])
+        end
+      end
+
+      context 'when mixing version 1 and version 2 feature flags' do
+        it 'returns only version 1 flags when the new flags are disabled' do
+          stub_feature_flags(feature_flags_new_version: false)
+          feature_flag_a = create(:operations_feature_flag, project: project,
+                                  name: 'feature_a', active: true, version: 2)
+          strategy = create(:operations_strategy, feature_flag: feature_flag_a,
+                            name: 'userWithId', parameters: { userIds: 'user8' })
+          create(:operations_scope, strategy: strategy, environment_scope: 'staging')
+          feature_flag_b = create(:operations_feature_flag, project: project,
+                                  name: 'feature_b', active: true, version: 1)
+          create(:operations_feature_flag_scope, feature_flag: feature_flag_b,
+                 active: true, strategies: [{ name: 'default', parameters: {} }], environment_scope: 'staging')
+
+          get api(features_url), headers: { 'UNLEASH-INSTANCEID' => client.token, 'UNLEASH-APPNAME' => 'staging' }
+
+          expect(response).to have_gitlab_http_status(:ok)
+          expect(json_response['features'].sort_by {|f| f['name']}).to eq([{
+            'name' => 'feature_b',
+            'enabled' => true,
+            'strategies' => [{
+              'name' => 'default',
+              'parameters' => {}
+            }]
+          }])
+        end
+
+        it 'returns both types of flags when both match' do
+          feature_flag_a = create(:operations_feature_flag, project: project,
+                                  name: 'feature_a', active: true, version: 2)
+          strategy = create(:operations_strategy, feature_flag: feature_flag_a,
+                            name: 'userWithId', parameters: { userIds: 'user8' })
+          create(:operations_scope, strategy: strategy, environment_scope: 'staging')
+          feature_flag_b = create(:operations_feature_flag, project: project,
+                                  name: 'feature_b', active: true, version: 1)
+          create(:operations_feature_flag_scope, feature_flag: feature_flag_b,
+                 active: true, strategies: [{ name: 'default', parameters: {} }], environment_scope: 'staging')
+
+          get api(features_url), headers: { 'UNLEASH-INSTANCEID' => client.token, 'UNLEASH-APPNAME' => 'staging' }
+
+          expect(response).to have_gitlab_http_status(:ok)
+          expect(json_response['features'].sort_by {|f| f['name']}).to eq([{
+            'name' => 'feature_a',
+            'enabled' => true,
+            'strategies' => [{
+              'name' => 'userWithId',
+              'parameters' => { 'userIds' => 'user8' }
+            }]
+          }, {
+            'name' => 'feature_b',
+            'enabled' => true,
+            'strategies' => [{
+              'name' => 'default',
+              'parameters' => {}
+            }]
+          }])
+        end
+
+        it 'returns legacy flags when only legacy flags match' do
+          feature_flag_a = create(:operations_feature_flag, project: project,
+                                  name: 'feature_a', active: true, version: 2)
+          strategy = create(:operations_strategy, feature_flag: feature_flag_a,
+                            name: 'userWithId', parameters: { userIds: 'user8' })
+          create(:operations_scope, strategy: strategy, environment_scope: 'production')
+          feature_flag_b = create(:operations_feature_flag, project: project,
+                                  name: 'feature_b', active: true, version: 1)
+          create(:operations_feature_flag_scope, feature_flag: feature_flag_b,
+                 active: true, strategies: [{ name: 'default', parameters: {} }], environment_scope: 'staging')
+
+          get api(features_url), headers: { 'UNLEASH-INSTANCEID' => client.token, 'UNLEASH-APPNAME' => 'staging' }
+
+          expect(response).to have_gitlab_http_status(:ok)
+          expect(json_response['features']).to eq([{
+            'name' => 'feature_b',
+            'enabled' => true,
+            'strategies' => [{
+              'name' => 'default',
+              'parameters' => {}
+            }]
+          }])
+        end
+      end
+    end
   end
 
   describe 'POST /feature_flags/unleash/:project_id/client/register' do