Commit d416cf01 authored by Shinya Maeda's avatar Shinya Maeda

Drop support of legacy feature flag

Legacy feature flags have been deprecated for a long time
and to be dropped in GitLab 14.0.

Changelog: removed
parent 9c4698dc
...@@ -11,6 +11,7 @@ class Projects::FeatureFlagsController < Projects::ApplicationController ...@@ -11,6 +11,7 @@ class Projects::FeatureFlagsController < Projects::ApplicationController
before_action :feature_flag, only: [:edit, :update, :destroy] before_action :feature_flag, only: [:edit, :update, :destroy]
before_action :ensure_flag_writable!, only: [:update] before_action :ensure_flag_writable!, only: [:update]
before_action :exclude_legacy_flags_check, only: [:edit]
before_action do before_action do
push_frontend_feature_flag(:feature_flag_permissions) push_frontend_feature_flag(:feature_flag_permissions)
...@@ -63,7 +64,6 @@ class Projects::FeatureFlagsController < Projects::ApplicationController ...@@ -63,7 +64,6 @@ class Projects::FeatureFlagsController < Projects::ApplicationController
end end
def edit def edit
exclude_legacy_flags_check
end end
def update def update
...@@ -108,6 +108,12 @@ class Projects::FeatureFlagsController < Projects::ApplicationController ...@@ -108,6 +108,12 @@ class Projects::FeatureFlagsController < Projects::ApplicationController
end end
end end
def exclude_legacy_flags_check
if feature_flag.legacy_flag?
not_found
end
end
def create_params def create_params
params.require(:operations_feature_flag) params.require(:operations_feature_flag)
.permit(:name, :description, :active, :version, .permit(:name, :description, :active, :version,
...@@ -159,12 +165,4 @@ class Projects::FeatureFlagsController < Projects::ApplicationController ...@@ -159,12 +165,4 @@ class Projects::FeatureFlagsController < Projects::ApplicationController
render json: { message: messages }, render json: { message: messages },
status: status status: status
end end
def exclude_legacy_flags_check
if Feature.enabled?(:remove_legacy_flags, project, default_enabled: :yaml) &&
Feature.disabled?(:remove_legacy_flags_override, project, default_enabled: :yaml) &&
feature_flag.legacy_flag?
not_found
end
end
end end
...@@ -24,11 +24,7 @@ class FeatureFlagsFinder ...@@ -24,11 +24,7 @@ class FeatureFlagsFinder
private private
def feature_flags def feature_flags
if exclude_legacy_flags? project.operations_feature_flags.new_version_only
project.operations_feature_flags.new_version_only
else
project.operations_feature_flags
end
end end
def by_scope(items) def by_scope(items)
...@@ -41,9 +37,4 @@ class FeatureFlagsFinder ...@@ -41,9 +37,4 @@ class FeatureFlagsFinder
items items
end end
end end
def exclude_legacy_flags?
Feature.enabled?(:remove_legacy_flags, project, default_enabled: :yaml) &&
Feature.disabled?(:remove_legacy_flags_override, project, default_enabled: :yaml)
end
end end
# frozen_string_literal: true
module FeatureFlags
class DisableService < BaseService
def execute
return error('Feature Flag not found', 404) unless feature_flag_by_name
return error('Feature Flag Scope not found', 404) unless feature_flag_scope_by_environment_scope
return error('Strategy not found', 404) unless strategy_exist_in_persisted_data?
::FeatureFlags::UpdateService
.new(project, current_user, update_params)
.execute(feature_flag_by_name)
end
private
def update_params
if remaining_strategies.empty?
params_to_destroy_scope
else
params_to_update_scope
end
end
def remaining_strategies
strong_memoize(:remaining_strategies) do
feature_flag_scope_by_environment_scope.strategies.reject do |strategy|
strategy['name'] == params[:strategy]['name'] &&
strategy['parameters'] == params[:strategy]['parameters']
end
end
end
def strategy_exist_in_persisted_data?
feature_flag_scope_by_environment_scope.strategies != remaining_strategies
end
def params_to_destroy_scope
{ scopes_attributes: [{ id: feature_flag_scope_by_environment_scope.id, _destroy: true }] }
end
def params_to_update_scope
{ scopes_attributes: [{ id: feature_flag_scope_by_environment_scope.id, strategies: remaining_strategies }] }
end
end
end
# frozen_string_literal: true
module FeatureFlags
class EnableService < BaseService
def execute
if feature_flag_by_name
update_feature_flag
else
create_feature_flag
end
end
private
def create_feature_flag
::FeatureFlags::CreateService
.new(project, current_user, create_params)
.execute
end
def update_feature_flag
::FeatureFlags::UpdateService
.new(project, current_user, update_params)
.execute(feature_flag_by_name)
end
def create_params
if params[:environment_scope] == '*'
params_to_create_flag_with_default_scope
else
params_to_create_flag_with_additional_scope
end
end
def update_params
if feature_flag_scope_by_environment_scope
params_to_update_scope
else
params_to_create_scope
end
end
def params_to_create_flag_with_default_scope
{
name: params[:name],
scopes_attributes: [
{
active: true,
environment_scope: '*',
strategies: [params[:strategy]]
}
]
}
end
def params_to_create_flag_with_additional_scope
{
name: params[:name],
scopes_attributes: [
{
active: false,
environment_scope: '*'
},
{
active: true,
environment_scope: params[:environment_scope],
strategies: [params[:strategy]]
}
]
}
end
def params_to_create_scope
{
scopes_attributes: [{
active: true,
environment_scope: params[:environment_scope],
strategies: [params[:strategy]]
}]
}
end
def params_to_update_scope
{
scopes_attributes: [{
id: feature_flag_scope_by_environment_scope.id,
active: true,
strategies: feature_flag_scope_by_environment_scope.strategies | [params[:strategy]]
}]
}
end
end
end
---
name: feature_flag_api
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/18198
rollout_issue_url:
milestone: '12.4'
type: development
group: group::release
default_enabled: false
---
name: remove_legacy_flags
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/62484
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/332243
milestone: '14.0'
type: development
group: group::release
default_enabled: false
---
name: remove_legacy_flags_override
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/62484
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/332243
milestone: '14.0'
type: development
group: group::release
default_enabled: false
This diff is collapsed.
This diff is collapsed.
...@@ -168,7 +168,6 @@ module API ...@@ -168,7 +168,6 @@ module API
mount ::API::ErrorTracking mount ::API::ErrorTracking
mount ::API::Events mount ::API::Events
mount ::API::FeatureFlags mount ::API::FeatureFlags
mount ::API::FeatureFlagScopes
mount ::API::FeatureFlagsUserLists mount ::API::FeatureFlagsUserLists
mount ::API::Features mount ::API::Features
mount ::API::Files mount ::API::Files
......
# frozen_string_literal: true
module API
class FeatureFlagScopes < ::API::Base
include PaginationParams
ENVIRONMENT_SCOPE_ENDPOINT_REQUIREMENTS = FeatureFlags::FEATURE_FLAG_ENDPOINT_REQUIREMENTS
.merge(environment_scope: API::NO_SLASH_URL_PART_REGEX)
feature_category :feature_flags
before do
authorize_read_feature_flags!
end
params do
requires :id, type: String, desc: 'The ID of a project'
end
resource 'projects/:id', requirements: API::NAMESPACE_OR_PROJECT_REQUIREMENTS do
resource :feature_flag_scopes do
desc 'Get all effective feature flags under the environment' do
detail 'This feature was introduced in GitLab 12.5'
success ::API::Entities::FeatureFlag::DetailedLegacyScope
end
params do
requires :environment, type: String, desc: 'The environment name'
end
get do
present scopes_for_environment, with: ::API::Entities::FeatureFlag::DetailedLegacyScope
end
end
params do
requires :name, type: String, desc: 'The name of the feature flag'
end
resource 'feature_flags/:name', requirements: FeatureFlags::FEATURE_FLAG_ENDPOINT_REQUIREMENTS do
resource :scopes do
desc 'Get all scopes of a feature flag' do
detail 'This feature was introduced in GitLab 12.5'
success ::API::Entities::FeatureFlag::LegacyScope
end
params do
use :pagination
end
get do
present paginate(feature_flag.scopes), with: ::API::Entities::FeatureFlag::LegacyScope
end
desc 'Create a scope of a feature flag' do
detail 'This feature was introduced in GitLab 12.5'
success ::API::Entities::FeatureFlag::LegacyScope
end
params do
requires :environment_scope, type: String, desc: 'The environment scope of the scope'
requires :active, type: Boolean, desc: 'Whether the scope is active'
requires :strategies, type: JSON, desc: 'The strategies of the scope'
end
post do
authorize_update_feature_flag!
result = ::FeatureFlags::UpdateService
.new(user_project, current_user, scopes_attributes: [declared_params])
.execute(feature_flag)
if result[:status] == :success
present scope, with: ::API::Entities::FeatureFlag::LegacyScope
else
render_api_error!(result[:message], result[:http_status])
end
end
params do
requires :environment_scope, type: String, desc: 'URL-encoded environment scope'
end
resource ':environment_scope', requirements: ENVIRONMENT_SCOPE_ENDPOINT_REQUIREMENTS do
desc 'Get a scope of a feature flag' do
detail 'This feature was introduced in GitLab 12.5'
success ::API::Entities::FeatureFlag::LegacyScope
end
get do
present scope, with: ::API::Entities::FeatureFlag::LegacyScope
end
desc 'Update a scope of a feature flag' do
detail 'This feature was introduced in GitLab 12.5'
success ::API::Entities::FeatureFlag::LegacyScope
end
params do
optional :active, type: Boolean, desc: 'Whether the scope is active'
optional :strategies, type: JSON, desc: 'The strategies of the scope'
end
put do
authorize_update_feature_flag!
scope_attributes = declared_params.merge(id: scope.id)
result = ::FeatureFlags::UpdateService
.new(user_project, current_user, scopes_attributes: [scope_attributes])
.execute(feature_flag)
if result[:status] == :success
updated_scope = result[:feature_flag].scopes
.find { |scope| scope.environment_scope == params[:environment_scope] }
present updated_scope, with: ::API::Entities::FeatureFlag::LegacyScope
else
render_api_error!(result[:message], result[:http_status])
end
end
desc 'Delete a scope from a feature flag' do
detail 'This feature was introduced in GitLab 12.5'
success ::API::Entities::FeatureFlag::LegacyScope
end
delete do
authorize_update_feature_flag!
param = { scopes_attributes: [{ id: scope.id, _destroy: true }] }
result = ::FeatureFlags::UpdateService
.new(user_project, current_user, param)
.execute(feature_flag)
if result[:status] == :success
status :no_content
else
render_api_error!(result[:message], result[:http_status])
end
end
end
end
end
end
helpers do
def authorize_read_feature_flags!
authorize! :read_feature_flag, user_project
end
def authorize_update_feature_flag!
authorize! :update_feature_flag, feature_flag
end
def feature_flag
@feature_flag ||= user_project.operations_feature_flags
.find_by_name!(params[:name])
end
def scope
@scope ||= feature_flag.scopes
.find_by_environment_scope!(CGI.unescape(params[:environment_scope]))
end
def scopes_for_environment
Operations::FeatureFlagScope
.for_unleash_client(user_project, params[:environment])
end
end
end
end
...@@ -95,54 +95,6 @@ module API ...@@ -95,54 +95,6 @@ module API
present_entity(feature_flag) present_entity(feature_flag)
end end
desc 'Enable a strategy for a feature flag on an environment' do
detail 'This feature was introduced in GitLab 12.5'
success ::API::Entities::FeatureFlag
end
params do
requires :environment_scope, type: String, desc: 'The environment scope of the feature flag'
requires :strategy, type: JSON, desc: 'The strategy to be enabled on the scope'
end
post :enable do
not_found! unless Feature.enabled?(:feature_flag_api, user_project)
exclude_legacy_flags_check!
render_api_error!('Version 2 flags not supported', :unprocessable_entity) if new_version_flag_present?
result = ::FeatureFlags::EnableService
.new(user_project, current_user, params).execute
if result[:status] == :success
status :ok
present_entity(result[:feature_flag])
else
render_api_error!(result[:message], result[:http_status])
end
end
desc 'Disable a strategy for a feature flag on an environment' do
detail 'This feature is going to be introduced in GitLab 12.5 if `feature_flag_api` feature flag is removed'
success ::API::Entities::FeatureFlag
end
params do
requires :environment_scope, type: String, desc: 'The environment scope of the feature flag'
requires :strategy, type: JSON, desc: 'The strategy to be disabled on the scope'
end
post :disable do
not_found! unless Feature.enabled?(:feature_flag_api, user_project)
exclude_legacy_flags_check!
render_api_error!('Version 2 flags not supported', :unprocessable_entity) if feature_flag.new_version_flag?
result = ::FeatureFlags::DisableService
.new(user_project, current_user, params).execute
if result[:status] == :success
status :ok
present_entity(result[:feature_flag])
else
render_api_error!(result[:message], result[:http_status])
end
end
desc 'Update a feature flag' do desc 'Update a feature flag' do
detail 'This feature was introduced in GitLab 13.2' detail 'This feature was introduced in GitLab 13.2'
success ::API::Entities::FeatureFlag success ::API::Entities::FeatureFlag
...@@ -255,9 +207,7 @@ module API ...@@ -255,9 +207,7 @@ module API
end end
def exclude_legacy_flags_check! def exclude_legacy_flags_check!
if Feature.enabled?(:remove_legacy_flags, project, default_enabled: :yaml) && if feature_flag.legacy_flag?
Feature.disabled?(:remove_legacy_flags_override, project, default_enabled: :yaml) &&
feature_flag.legacy_flag?
not_found! not_found!
end end
end end
......
...@@ -69,21 +69,7 @@ module API ...@@ -69,21 +69,7 @@ module API
def feature_flags def feature_flags
return [] unless unleash_app_name.present? return [] unless unleash_app_name.present?
legacy_flags = Operations::FeatureFlag.for_unleash_client(project, unleash_app_name)
if exclude_legacy_flags?
[]
else
Operations::FeatureFlagScope.for_unleash_client(project, unleash_app_name)
end
new_version_flags = Operations::FeatureFlag.for_unleash_client(project, unleash_app_name)
legacy_flags + new_version_flags
end
def exclude_legacy_flags?
Feature.enabled?(:remove_legacy_flags, project, default_enabled: :yaml) &&
Feature.disabled?(:remove_legacy_flags_override, project, default_enabled: :yaml)
end end
end end
end end
......
...@@ -154,60 +154,6 @@ RSpec.describe Projects::FeatureFlagsController do ...@@ -154,60 +154,6 @@ RSpec.describe Projects::FeatureFlagsController do
end end
end end
context 'when feature flags have additional scopes' do
let!(:feature_flag_active_scope) do
create(:operations_feature_flag_scope,
feature_flag: feature_flag_active,
environment_scope: 'production',
active: false)
end
let!(:feature_flag_inactive_scope) do
create(:operations_feature_flag_scope,
feature_flag: feature_flag_inactive,
environment_scope: 'staging',
active: false)
end
it 'returns a correct summary' do
subject
expect(json_response['count']['all']).to eq(2)
expect(json_response['count']['enabled']).to eq(1)
expect(json_response['count']['disabled']).to eq(1)
end
it 'recognizes feature flag 1 as active' do
subject
expect(json_response['feature_flags'].first['active']).to be_truthy
end
it 'recognizes feature flag 2 as inactive' do
subject
expect(json_response['feature_flags'].second['active']).to be_falsy
end
it 'has ordered scopes' do
subject
expect(json_response['feature_flags'][0]['scopes'][0]['id'])
.to be < json_response['feature_flags'][0]['scopes'][1]['id']
expect(json_response['feature_flags'][1]['scopes'][0]['id'])
.to be < json_response['feature_flags'][1]['scopes'][1]['id']
end
it 'does not have N+1 problem' do
recorded = ActiveRecord::QueryRecorder.new { subject }
related_count = recorded.log
.count { |query| query.include?('operations_feature_flag') }
expect(related_count).to be_within(5).of(2)
end
end
context 'with version 1 and 2 feature flags' do context 'with version 1 and 2 feature flags' do
let!(:new_version_feature_flag) do let!(:new_version_feature_flag) do
create(:operations_feature_flag, :new_version_flag, project: project, name: 'feature_flag_c') create(:operations_feature_flag, :new_version_flag, project: project, name: 'feature_flag_c')
...@@ -235,7 +181,7 @@ RSpec.describe Projects::FeatureFlagsController do ...@@ -235,7 +181,7 @@ RSpec.describe Projects::FeatureFlagsController do
subject { get(:show, params: params, format: :json) } subject { get(:show, params: params, format: :json) }
let!(:feature_flag) do let!(:feature_flag) do
create(:operations_feature_flag, project: project) create(:operations_feature_flag, :legacy_flag, project: project)
end end
let(:params) do let(:params) do
...@@ -375,7 +321,7 @@ RSpec.describe Projects::FeatureFlagsController do ...@@ -375,7 +321,7 @@ RSpec.describe Projects::FeatureFlagsController do
subject { get(:edit, params: params) } subject { get(:edit, params: params) }
context 'with legacy flags' do context 'with legacy flags' do
let!(:feature_flag) { create(:operations_feature_flag, project: project) } let!(:feature_flag) { create(:operations_feature_flag, :legacy_flag, project: project) }
let(:params) do let(:params) do
{ {
...@@ -385,29 +331,13 @@ RSpec.describe Projects::FeatureFlagsController do ...@@ -385,29 +331,13 @@ RSpec.describe Projects::FeatureFlagsController do
} }
end end
context 'removed' do it 'returns not found' do
before do is_expected.to have_gitlab_http_status(:not_found)
stub_feature_flags(remove_legacy_flags: true, remove_legacy_flags_override: false)
end
it 'returns not found' do
is_expected.to have_gitlab_http_status(:not_found)
end
end
context 'removed' do
before do
stub_feature_flags(remove_legacy_flags: false)
end
it 'returns ok' do
is_expected.to have_gitlab_http_status(:ok)
end
end end
end end
context 'with new version flags' do context 'with new version flags' do
let!(:feature_flag) { create(:operations_feature_flag, :new_version_flag, project: project) } let!(:feature_flag) { create(:operations_feature_flag, project: project) }
let(:params) do let(:params) do
{ {
...@@ -814,7 +744,7 @@ RSpec.describe Projects::FeatureFlagsController do ...@@ -814,7 +744,7 @@ RSpec.describe Projects::FeatureFlagsController do
describe 'DELETE destroy.json' do describe 'DELETE destroy.json' do
subject { delete(:destroy, params: params, format: :json) } subject { delete(:destroy, params: params, format: :json) }
let!(:feature_flag) { create(:operations_feature_flag, project: project) } let!(:feature_flag) { create(:operations_feature_flag, :legacy_flag, project: project) }
let(:params) do let(:params) do
{ {
......
...@@ -2,7 +2,7 @@ ...@@ -2,7 +2,7 @@
FactoryBot.define do FactoryBot.define do
factory :operations_feature_flag_scope, class: 'Operations::FeatureFlagScope' do factory :operations_feature_flag_scope, class: 'Operations::FeatureFlagScope' do
association :feature_flag, factory: :operations_feature_flag association :feature_flag, factory: [:operations_feature_flag, :legacy_flag]
active { true } active { true }
strategies { [{ name: "default", parameters: {} }] } strategies { [{ name: "default", parameters: {} }] }
sequence(:environment_scope) { |n| "review/patch-#{n}" } sequence(:environment_scope) { |n| "review/patch-#{n}" }
......
...@@ -5,6 +5,7 @@ FactoryBot.define do ...@@ -5,6 +5,7 @@ FactoryBot.define do
sequence(:name) { |n| "feature_flag_#{n}" } sequence(:name) { |n| "feature_flag_#{n}" }
project project
active { true } active { true }
version { :new_version_flag }
trait :legacy_flag do trait :legacy_flag do
version { Operations::FeatureFlag.versions['legacy_flag'] } version { Operations::FeatureFlag.versions['legacy_flag'] }
......
...@@ -18,65 +18,21 @@ RSpec.describe 'User sees feature flag list', :js do ...@@ -18,65 +18,21 @@ RSpec.describe 'User sees feature flag list', :js do
context 'with legacy feature flags' do context 'with legacy feature flags' do
before do before do
create_flag(project, 'ci_live_trace', false).tap do |feature_flag| create_flag(project, 'ci_live_trace', false, version: :legacy_flag).tap do |feature_flag|
create_scope(feature_flag, 'review/*', true) create_scope(feature_flag, 'review/*', true)
end end
create_flag(project, 'drop_legacy_artifacts', false) create_flag(project, 'drop_legacy_artifacts', false, version: :legacy_flag)
create_flag(project, 'mr_train', true).tap do |feature_flag| create_flag(project, 'mr_train', true, version: :legacy_flag).tap do |feature_flag|
create_scope(feature_flag, 'production', false) create_scope(feature_flag, 'production', false)
end end
end end
it 'user sees the first flag' do it 'shows empty page' do
visit(project_feature_flags_path(project))
within_feature_flag_row(1) do
expect(page.find('.js-feature-flag-id')).to have_content('^1')
expect(page.find('.feature-flag-name')).to have_content('ci_live_trace')
expect_status_toggle_button_not_to_be_checked
within_feature_flag_scopes do
expect(page.find('[data-qa-selector="feature-flag-scope-muted-badge"]:nth-child(1)')).to have_content('*')
expect(page.find('[data-qa-selector="feature-flag-scope-info-badge"]:nth-child(2)')).to have_content('review/*')
end
end
end
it 'user sees the second flag' do
visit(project_feature_flags_path(project))
within_feature_flag_row(2) do
expect(page.find('.js-feature-flag-id')).to have_content('^2')
expect(page.find('.feature-flag-name')).to have_content('drop_legacy_artifacts')
expect_status_toggle_button_not_to_be_checked
within_feature_flag_scopes do
expect(page.find('[data-qa-selector="feature-flag-scope-muted-badge"]:nth-child(1)')).to have_content('*')
end
end
end
it 'user sees the third flag' do
visit(project_feature_flags_path(project))
within_feature_flag_row(3) do
expect(page.find('.js-feature-flag-id')).to have_content('^3')
expect(page.find('.feature-flag-name')).to have_content('mr_train')
expect_status_toggle_button_to_be_checked
within_feature_flag_scopes do
expect(page.find('[data-qa-selector="feature-flag-scope-info-badge"]:nth-child(1)')).to have_content('*')
expect(page.find('[data-qa-selector="feature-flag-scope-muted-badge"]:nth-child(2)')).to have_content('production')
end
end
end
it 'user sees the status toggle disabled' do
visit(project_feature_flags_path(project)) visit(project_feature_flags_path(project))
within_feature_flag_row(1) do expect(page).to have_text 'Get started with feature flags'
expect_status_toggle_button_to_be_disabled expect(page).to have_selector('.btn-confirm', text: 'New feature flag')
end expect(page).to have_selector('[data-qa-selector="configure_feature_flags_button"]', text: 'Configure')
end end
end end
......
...@@ -73,16 +73,16 @@ RSpec.describe 'User updates feature flag', :js do ...@@ -73,16 +73,16 @@ RSpec.describe 'User updates feature flag', :js do
context 'with a legacy feature flag' do context 'with a legacy feature flag' do
let!(:feature_flag) do let!(:feature_flag) do
create_flag(project, 'ci_live_trace', true, create_flag(project, 'ci_live_trace', true,
description: 'For live trace feature') description: 'For live trace feature',
version: :legacy_flag)
end end
let!(:scope) { create_scope(feature_flag, 'review/*', true) } let!(:scope) { create_scope(feature_flag, 'review/*', true) }
it 'the user cannot edit the flag' do it 'shows not found error' do
visit(edit_project_feature_flag_path(project, feature_flag)) visit(edit_project_feature_flag_path(project, feature_flag))
expect(page).to have_text 'This feature flag is read-only, and it will be removed in 14.0.' expect(page).to have_text 'Page Not Found'
expect(page).to have_css('button.js-ff-submit.disabled')
end end
end end
end end
...@@ -24,10 +24,6 @@ RSpec.describe FeatureFlagsFinder do ...@@ -24,10 +24,6 @@ RSpec.describe FeatureFlagsFinder do
let!(:feature_flag_2) { create(:operations_feature_flag, name: 'flag-b', project: project) } let!(:feature_flag_2) { create(:operations_feature_flag, name: 'flag-b', project: project) }
let(:args) { {} } let(:args) { {} }
before do
stub_feature_flags(remove_legacy_flags: false)
end
it 'returns feature flags ordered by name' do it 'returns feature flags ordered by name' do
is_expected.to eq([feature_flag_1, feature_flag_2]) is_expected.to eq([feature_flag_1, feature_flag_2])
end end
...@@ -77,21 +73,11 @@ RSpec.describe FeatureFlagsFinder do ...@@ -77,21 +73,11 @@ RSpec.describe FeatureFlagsFinder do
end end
end end
context 'when new version flags are enabled' do context 'with a legacy flag' do
let!(:feature_flag_3) { create(:operations_feature_flag, :new_version_flag, name: 'flag-c', project: project) } let!(:feature_flag_3) { create(:operations_feature_flag, :legacy_flag, name: 'flag-c', project: project) }
it 'returns new and legacy flags' do
is_expected.to eq([feature_flag_1, feature_flag_2, feature_flag_3])
end
context 'when legacy flags are disabled' do it 'returns new flags' do
before do is_expected.to eq([feature_flag_1, feature_flag_2])
stub_feature_flags(remove_legacy_flags_override: false, remove_legacy_flags: true)
end
it 'returns only new flags' do
is_expected.to eq([feature_flag_3])
end
end end
end end
end end
......
...@@ -29,7 +29,7 @@ RSpec.describe Operations::FeatureFlagScope do ...@@ -29,7 +29,7 @@ RSpec.describe Operations::FeatureFlagScope do
end end
context 'when environment scope of a default scope is updated' do context 'when environment scope of a default scope is updated' do
let!(:feature_flag) { create(:operations_feature_flag) } let!(:feature_flag) { create(:operations_feature_flag, :legacy_flag) }
let!(:scope_default) { feature_flag.default_scope } let!(:scope_default) { feature_flag.default_scope }
it 'keeps default scope intact' do it 'keeps default scope intact' do
...@@ -41,7 +41,7 @@ RSpec.describe Operations::FeatureFlagScope do ...@@ -41,7 +41,7 @@ RSpec.describe Operations::FeatureFlagScope do
end end
context 'when a default scope is destroyed' do context 'when a default scope is destroyed' do
let!(:feature_flag) { create(:operations_feature_flag) } let!(:feature_flag) { create(:operations_feature_flag, :legacy_flag) }
let!(:scope_default) { feature_flag.default_scope } let!(:scope_default) { feature_flag.default_scope }
it 'prevents from destroying the default scope' do it 'prevents from destroying the default scope' do
......
...@@ -181,7 +181,7 @@ RSpec.describe Operations::FeatureFlag do ...@@ -181,7 +181,7 @@ RSpec.describe Operations::FeatureFlag do
end end
context 'when the feature flag is active and all scopes are inactive' do context 'when the feature flag is active and all scopes are inactive' do
let!(:feature_flag) { create(:operations_feature_flag, active: true) } let!(:feature_flag) { create(:operations_feature_flag, :legacy_flag, active: true) }
it 'returns the flag' do it 'returns the flag' do
feature_flag.default_scope.update!(active: false) feature_flag.default_scope.update!(active: false)
...@@ -199,7 +199,7 @@ RSpec.describe Operations::FeatureFlag do ...@@ -199,7 +199,7 @@ RSpec.describe Operations::FeatureFlag do
end end
context 'when the feature flag is inactive and all scopes are active' do context 'when the feature flag is inactive and all scopes are active' do
let!(:feature_flag) { create(:operations_feature_flag, active: false) } let!(:feature_flag) { create(:operations_feature_flag, :legacy_flag, active: false) }
it 'does not return the flag' do it 'does not return the flag' do
feature_flag.default_scope.update!(active: true) feature_flag.default_scope.update!(active: true)
...@@ -221,7 +221,7 @@ RSpec.describe Operations::FeatureFlag do ...@@ -221,7 +221,7 @@ RSpec.describe Operations::FeatureFlag do
end end
context 'when the feature flag is active and all scopes are inactive' do context 'when the feature flag is active and all scopes are inactive' do
let!(:feature_flag) { create(:operations_feature_flag, active: true) } let!(:feature_flag) { create(:operations_feature_flag, :legacy_flag, active: true) }
it 'does not return the flag' do it 'does not return the flag' do
feature_flag.default_scope.update!(active: false) feature_flag.default_scope.update!(active: false)
...@@ -239,7 +239,7 @@ RSpec.describe Operations::FeatureFlag do ...@@ -239,7 +239,7 @@ RSpec.describe Operations::FeatureFlag do
end end
context 'when the feature flag is inactive and all scopes are active' do context 'when the feature flag is inactive and all scopes are active' do
let!(:feature_flag) { create(:operations_feature_flag, active: false) } let!(:feature_flag) { create(:operations_feature_flag, :legacy_flag, active: false) }
it 'returns the flag' do it 'returns the flag' do
feature_flag.default_scope.update!(active: true) feature_flag.default_scope.update!(active: true)
......
This diff is collapsed.
This diff is collapsed.
...@@ -176,34 +176,9 @@ RSpec.describe API::Unleash do ...@@ -176,34 +176,9 @@ RSpec.describe API::Unleash do
it_behaves_like 'authenticated request' it_behaves_like 'authenticated request'
context 'with version 1 (legacy) feature flags' do context 'with version 1 (legacy) feature flags' do
let(:feature_flag) { create(:operations_feature_flag, project: project, name: 'feature1', active: true, version: 1) } let(:feature_flag) { create(:operations_feature_flag, :legacy_flag, project: project, name: 'feature1', active: true, version: 1) }
it_behaves_like 'support multiple environments' it 'does not return a legacy feature flag' do
context 'with a list of feature flags' do
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
expect(response).to have_gitlab_http_status(:ok)
expect(json_response['version']).to eq(1)
expect(json_response['features']).not_to be_empty
expect(json_response['features'].map { |f| f['name'] }.sort).to eq(%w[feature1 feature2])
expect(json_response['features'].sort_by {|f| f['name'] }.map { |f| f['enabled'] }).to eq([true, false])
end
it 'matches json schema' do
subject
expect(response).to have_gitlab_http_status(:ok)
expect(response).to match_response_schema('unleash/unleash')
end
end
it 'returns a feature flag strategy' do
create(:operations_feature_flag_scope, create(:operations_feature_flag_scope,
feature_flag: feature_flag, feature_flag: feature_flag,
environment_scope: 'sandbox', environment_scope: 'sandbox',
...@@ -215,81 +190,7 @@ RSpec.describe API::Unleash do ...@@ -215,81 +190,7 @@ RSpec.describe API::Unleash do
get api(features_url), headers: headers get api(features_url), headers: headers
expect(response).to have_gitlab_http_status(:ok) expect(response).to have_gitlab_http_status(:ok)
expect(json_response['features'].first['enabled']).to eq(true) expect(json_response['features']).to be_empty
strategies = json_response['features'].first['strategies']
expect(strategies).to eq([{
"name" => "gradualRolloutUserId",
"parameters" => {
"percentage" => "50",
"groupId" => "default"
}
}])
end
it 'returns a default strategy for a scope' do
create(:operations_feature_flag_scope, feature_flag: feature_flag, environment_scope: 'sandbox', active: true)
headers = { "UNLEASH-INSTANCEID" => client.token, "UNLEASH-APPNAME" => "sandbox" }
get api(features_url), headers: headers
expect(response).to have_gitlab_http_status(:ok)
expect(json_response['features'].first['enabled']).to eq(true)
strategies = json_response['features'].first['strategies']
expect(strategies).to eq([{ "name" => "default", "parameters" => {} }])
end
it 'returns multiple strategies for a feature flag' do
create(:operations_feature_flag_scope,
feature_flag: feature_flag,
environment_scope: 'staging',
active: true,
strategies: [{ name: "userWithId", parameters: { userIds: "max,fred" } },
{ name: "gradualRolloutUserId",
parameters: { groupId: "default", percentage: "50" } }])
headers = { "UNLEASH-INSTANCEID" => client.token, "UNLEASH-APPNAME" => "staging" }
get api(features_url), headers: headers
expect(response).to have_gitlab_http_status(:ok)
expect(json_response['features'].first['enabled']).to eq(true)
strategies = json_response['features'].first['strategies'].sort_by { |s| s['name'] }
expect(strategies).to eq([{
"name" => "gradualRolloutUserId",
"parameters" => {
"percentage" => "50",
"groupId" => "default"
}
}, {
"name" => "userWithId",
"parameters" => {
"userIds" => "max,fred"
}
}])
end
it 'returns a disabled feature when the flag is disabled' do
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" }
get api(features_url), headers: headers
expect(response).to have_gitlab_http_status(:ok)
expect(json_response['features'].first['enabled']).to eq(false)
end
context "with an inactive scope" do
let!(:scope) { create(:operations_feature_flag_scope, feature_flag: feature_flag, environment_scope: 'production', active: false, strategies: [{ name: "default", parameters: {} }]) }
let(:headers) { { "UNLEASH-INSTANCEID" => client.token, "UNLEASH-APPNAME" => "production" } }
it 'returns a disabled feature' do
get api(features_url), headers: headers
expect(response).to have_gitlab_http_status(:ok)
feature_json = json_response['features'].first
expect(feature_json['enabled']).to eq(false)
expect(feature_json['strategies']).to eq([{ 'name' => 'default', 'parameters' => {} }])
end
end end
end end
...@@ -534,89 +435,6 @@ RSpec.describe API::Unleash do ...@@ -534,89 +435,6 @@ RSpec.describe API::Unleash do
}]) }])
end end
end end
context 'when mixing version 1 and version 2 feature flags' do
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
it 'returns new flags when legacy flags are disabled' do
stub_feature_flags(remove_legacy_flags_override: false, remove_legacy_flags: true)
feature_flag_a = create(:operations_feature_flag, :new_version_flag, project: project,
name: 'feature_a', active: true)
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, :legacy_flag, project: project,
name: 'feature_b', active: true)
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' }
}]
}])
end
end
end end
end end
......
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe FeatureFlags::DisableService do
include FeatureFlagHelpers
let_it_be(:project) { create(:project) }
let_it_be(:user) { create(:user) }
let(:params) { {} }
let(:service) { described_class.new(project, user, params) }
before_all do
project.add_developer(user)
end
describe '#execute' do
subject { service.execute }
context 'with params to disable default strategy on prd scope' do
let(:params) do
{
name: 'awesome',
environment_scope: 'prd',
strategy: { name: 'userWithId', parameters: { 'userIds': 'User:1' } }.deep_stringify_keys
}
end
context 'when there is a persisted feature flag' do
let!(:feature_flag) { create_flag(project, params[:name]) }
context 'when there is a persisted scope' do
let!(:scope) do
create_scope(feature_flag, params[:environment_scope], true, strategies)
end
context 'when there is a persisted strategy' do
let(:strategies) do
[
{ name: 'userWithId', parameters: { 'userIds': 'User:1' } }.deep_stringify_keys,
{ name: 'userWithId', parameters: { 'userIds': 'User:2' } }.deep_stringify_keys
]
end
it 'deletes the specified strategy' do
subject
scope.reload
expect(scope.strategies.count).to eq(1)
expect(scope.strategies).not_to include(params[:strategy])
end
context 'when strategies will be empty' do
let(:strategies) { [params[:strategy]] }
it 'deletes the persisted scope' do
subject
expect(feature_flag.scopes.exists?(environment_scope: params[:environment_scope]))
.to eq(false)
end
end
end
context 'when there is no persisted strategy' do
let(:strategies) { [{ name: 'default', parameters: {} }] }
it 'returns error' do
expect(subject[:status]).to eq(:error)
expect(subject[:message]).to include('Strategy not found')
end
end
end
context 'when there is no persisted scope' do
it 'returns error' do
expect(subject[:status]).to eq(:error)
expect(subject[:message]).to include('Feature Flag Scope not found')
end
end
end
context 'when there is no persisted feature flag' do
it 'returns error' do
expect(subject[:status]).to eq(:error)
expect(subject[:message]).to include('Feature Flag not found')
end
end
end
end
end
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe FeatureFlags::EnableService do
include FeatureFlagHelpers
let_it_be(:project) { create(:project) }
let_it_be(:user) { create(:user) }
let(:params) { {} }
let(:service) { described_class.new(project, user, params) }
before_all do
project.add_developer(user)
end
describe '#execute' do
subject { service.execute }
context 'with params to enable default strategy on prd scope' do
let(:params) do
{
name: 'awesome',
environment_scope: 'prd',
strategy: { name: 'default', parameters: {} }.stringify_keys
}
end
context 'when there is no persisted feature flag' do
it 'creates a new feature flag with scope' do
feature_flag = subject[:feature_flag]
scope = feature_flag.scopes.find_by_environment_scope(params[:environment_scope])
expect(subject[:status]).to eq(:success)
expect(feature_flag.name).to eq(params[:name])
expect(feature_flag.default_scope).not_to be_active
expect(scope).to be_active
expect(scope.strategies).to include(params[:strategy])
end
context 'when params include default scope' do
let(:params) do
{
name: 'awesome',
environment_scope: '*',
strategy: { name: 'userWithId', parameters: { 'userIds': 'abc' } }.deep_stringify_keys
}
end
it 'create a new feature flag with an active default scope with the specified strategy' do
feature_flag = subject[:feature_flag]
expect(subject[:status]).to eq(:success)
expect(feature_flag.default_scope).to be_active
expect(feature_flag.default_scope.strategies).to include(params[:strategy])
end
end
end
context 'when there is a persisted feature flag' do
let!(:feature_flag) { create_flag(project, params[:name]) }
context 'when there is no persisted scope' do
it 'creates a new scope for the persisted feature flag' do
feature_flag = subject[:feature_flag]
scope = feature_flag.scopes.find_by_environment_scope(params[:environment_scope])
expect(subject[:status]).to eq(:success)
expect(feature_flag.name).to eq(params[:name])
expect(scope).to be_active
expect(scope.strategies).to include(params[:strategy])
end
end
context 'when there is a persisted scope' do
let!(:feature_flag_scope) do
create_scope(feature_flag, params[:environment_scope], active, strategies)
end
let(:active) { true }
context 'when the persisted scope does not have the specified strategy yet' do
let(:strategies) { [{ name: 'userWithId', parameters: { 'userIds': 'abc' } }] }
it 'adds the specified strategy to the scope' do
subject
feature_flag_scope.reload
expect(feature_flag_scope.strategies).to include(params[:strategy])
end
context 'when the persisted scope is inactive' do
let(:active) { false }
it 'reactivates the scope' do
expect { subject }
.to change { feature_flag_scope.reload.active }.from(false).to(true)
end
end
end
context 'when the persisted scope has the specified strategy already' do
let(:strategies) { [params[:strategy]] }
it 'does not add a duplicated strategy to the scope' do
expect { subject }
.not_to change { feature_flag_scope.reload.strategies.count }
end
end
end
end
end
context 'when strategy is not specified in params' do
let(:params) do
{
name: 'awesome',
environment_scope: 'prd'
}
end
it 'returns error' do
expect(subject[:status]).to eq(:error)
expect(subject[:message]).to include('Scopes strategies must be an array of strategy hashes')
end
end
context 'when environment scope is not specified in params' do
let(:params) do
{
name: 'awesome',
strategy: { name: 'default', parameters: {} }.stringify_keys
}
end
it 'returns error' do
expect(subject[:status]).to eq(:error)
expect(subject[:message]).to include("Scopes environment scope can't be blank")
end
end
context 'when name is not specified in params' do
let(:params) do
{
environment_scope: 'prd',
strategy: { name: 'default', parameters: {} }.stringify_keys
}
end
it 'returns error' do
expect(subject[:status]).to eq(:error)
expect(subject[:message]).to include("Name can't be blank")
end
end
end
end
...@@ -121,150 +121,5 @@ RSpec.describe FeatureFlags::UpdateService do ...@@ -121,150 +121,5 @@ RSpec.describe FeatureFlags::UpdateService do
subject subject
end end
end end
context 'when scope active state is changed' do
let(:params) do
{
scopes_attributes: [{ id: feature_flag.scopes.first.id, active: false }]
}
end
it 'creates audit event about changing active state' do
expect { subject }.to change { AuditEvent.count }.by(1)
expect(audit_event_message).to(
include("Updated rule <strong>*</strong> active state "\
"from <strong>true</strong> to <strong>false</strong>.")
)
end
end
context 'when scope is renamed' do
let(:changed_scope) { feature_flag.scopes.create!(environment_scope: 'review', active: true) }
let(:params) do
{
scopes_attributes: [{ id: changed_scope.id, environment_scope: 'staging' }]
}
end
it 'creates audit event with changed name' do
expect { subject }.to change { AuditEvent.count }.by(1)
expect(audit_event_message).to(
include("Updated rule <strong>staging</strong> environment scope "\
"from <strong>review</strong> to <strong>staging</strong>.")
)
end
context 'when scope can not be updated' do
let(:params) do
{
scopes_attributes: [{ id: changed_scope.id, environment_scope: '' }]
}
end
it 'returns error status' do
expect(subject[:status]).to eq(:error)
end
it 'returns error messages' do
expect(subject[:message]).to include("Scopes environment scope can't be blank")
end
it 'does not create audit event' do
expect { subject }.not_to change { AuditEvent.count }
end
end
end
context 'when scope is deleted' do
let(:deleted_scope) { feature_flag.scopes.create!(environment_scope: 'review', active: true) }
let(:params) do
{
scopes_attributes: [{ id: deleted_scope.id, '_destroy': true }]
}
end
it 'creates audit event with deleted scope' do
expect { subject }.to change { AuditEvent.count }.by(1)
expect(audit_event_message).to include("Deleted rule <strong>review</strong>.")
end
context 'when scope can not be deleted' do
before do
allow(deleted_scope).to receive(:destroy).and_return(false)
end
it 'does not create audit event' do
expect do
subject
end.to not_change { AuditEvent.count }.and raise_error(ActiveRecord::RecordNotDestroyed)
end
end
end
context 'when new scope is being added' do
let(:new_environment_scope) { 'review' }
let(:params) do
{
scopes_attributes: [{ environment_scope: new_environment_scope, active: true }]
}
end
it 'creates audit event with new scope' do
expected = 'Created rule <strong>review</strong> and set it as <strong>active</strong> '\
'with strategies <strong>[{"name"=>"default", "parameters"=>{}}]</strong>.'
subject
expect(audit_event_message).to include(expected)
end
context 'when scope can not be created' do
let(:new_environment_scope) { '' }
it 'returns error status' do
expect(subject[:status]).to eq(:error)
end
it 'returns error messages' do
expect(subject[:message]).to include("Scopes environment scope can't be blank")
end
it 'does not create audit event' do
expect { subject }.not_to change { AuditEvent.count }
end
end
end
context 'when the strategy is changed' do
let(:scope) do
create(:operations_feature_flag_scope,
feature_flag: feature_flag,
environment_scope: 'sandbox',
strategies: [{ name: "default", parameters: {} }])
end
let(:params) do
{
scopes_attributes: [{
id: scope.id,
environment_scope: 'sandbox',
strategies: [{
name: 'gradualRolloutUserId',
parameters: {
groupId: 'mygroup',
percentage: "40"
}
}]
}]
}
end
it 'creates an audit event' do
expected = %r{Updated rule <strong>sandbox</strong> strategies from <strong>.*</strong> to <strong>.*</strong>.}
expect { subject }.to change { AuditEvent.count }.by(1)
expect(audit_event_message).to match(expected)
end
end
end end
end end
# frozen_string_literal: true # frozen_string_literal: true
module FeatureFlagHelpers module FeatureFlagHelpers
def create_flag(project, name, active = true, description: nil, version: Operations::FeatureFlag.versions['legacy_flag']) def create_flag(project, name, active = true, description: nil, version: Operations::FeatureFlag.versions['new_version_flag'])
create(:operations_feature_flag, name: name, active: active, version: version, create(:operations_feature_flag, name: name, active: active, version: version,
description: description, project: project) description: description, project: project)
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