Commit 2bbee63e authored by Shinya Maeda's avatar Shinya Maeda

Merge branch 'new-ff-remaining-crud' into 'master'

Support Version 2 Feature Flags in API POST, PUT, and DELETE

See merge request gitlab-org/gitlab!27026
parents 63046e8b ba885679
......@@ -34,7 +34,7 @@ module Operations
before_create :build_default_scope, if: -> { legacy_flag? && scopes.none? }
accepts_nested_attributes_for :scopes, allow_destroy: true
accepts_nested_attributes_for :strategies
accepts_nested_attributes_for :strategies, allow_destroy: true
scope :ordered, -> { order(:name) }
......
......@@ -26,6 +26,8 @@ module Operations
validate :parameters_validations, if: -> { errors[:name].blank? }
accepts_nested_attributes_for :scopes, allow_destroy: true
private
def parameters_validations
......
......@@ -30,9 +30,7 @@ module API
.new(user_project, current_user, declared_params(include_missing: false))
.execute
present paginate(feature_flags),
with: EE::API::Entities::FeatureFlag,
feature_flags_new_version_enabled: feature_flags_new_version_enabled?
present_entity(paginate(feature_flags))
end
desc 'Create a new feature flag' do
......@@ -42,26 +40,39 @@ module API
params do
requires :name, type: String, desc: 'The name of feature flag'
optional :description, type: String, desc: 'The description of the feature flag'
optional :version, type: String, desc: 'The version of the feature flag', values: Operations::FeatureFlag.versions.keys
optional :scopes, type: Array do
requires :environment_scope, type: String, desc: 'The environment scope of the scope'
requires :active, type: Boolean, desc: 'Active/inactive of the scope'
requires :strategies, type: JSON, desc: 'The strategies of the scope'
end
optional :strategies, type: Array do
requires :name, type: String, desc: 'The strategy name'
requires :parameters, type: JSON, desc: 'The strategy parameters'
optional :scopes, type: Array do
requires :environment_scope, type: String, desc: 'The environment scope of the scope'
end
end
end
post do
authorize_create_feature_flag!
attrs = declared_params(include_missing: false)
ensure_post_version_2_flags_enabled! if attrs[:version] == 'new_version_flag'
rename_key(attrs, :scopes, :scopes_attributes)
rename_key(attrs, :strategies, :strategies_attributes)
update_value(attrs, :strategies_attributes) do |strategies|
strategies.map { |s| rename_key(s, :scopes, :scopes_attributes) }
end
result = ::FeatureFlags::CreateService
.new(user_project, current_user, attrs)
.execute
if result[:status] == :success
present result[:feature_flag],
with: EE::API::Entities::FeatureFlag,
feature_flags_new_version_enabled: feature_flags_new_version_enabled?
present_entity(result[:feature_flag])
else
render_api_error!(result[:message], result[:http_status])
end
......@@ -79,9 +90,7 @@ module API
get do
authorize_read_feature_flag!
present feature_flag,
with: EE::API::Entities::FeatureFlag,
feature_flags_new_version_enabled: feature_flags_new_version_enabled?
present_entity(feature_flag)
end
desc 'Enable a strategy for a feature flag on an environment' do
......@@ -94,13 +103,14 @@ module API
end
post :enable do
not_found! unless Feature.enabled?(:feature_flag_api, user_project)
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 result[:feature_flag], with: EE::API::Entities::FeatureFlag
present_entity(result[:feature_flag])
else
render_api_error!(result[:message], result[:http_status])
end
......@@ -116,13 +126,55 @@ module API
end
post :disable do
not_found! unless Feature.enabled?(:feature_flag_api, user_project)
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 result[:feature_flag], with: EE::API::Entities::FeatureFlag
present_entity(result[:feature_flag])
else
render_api_error!(result[:message], result[:http_status])
end
end
desc 'Update a feature flag' do
detail 'This feature will be introduced in GitLab 13.1 if feature_flags_new_version feature flag is removed'
success EE::API::Entities::FeatureFlag
end
params do
optional :description, type: String, desc: 'The description of the feature flag'
optional :strategies, type: Array do
optional :id, type: Integer, desc: 'The strategy id'
optional :name, type: String, desc: 'The strategy type'
optional :parameters, type: JSON, desc: 'The strategy parameters'
optional :_destroy, type: Boolean, desc: 'Delete the strategy when true'
optional :scopes, type: Array do
optional :id, type: Integer, desc: 'The environment scope id'
optional :environment_scope, type: String, desc: 'The environment scope of the scope'
optional :_destroy, type: Boolean, desc: 'Delete the scope when true'
end
end
end
put do
not_found! unless feature_flags_new_version_enabled?
authorize_update_feature_flag!
render_api_error!('PUT operations are not supported for legacy feature flags', :unprocessable_entity) if feature_flag.legacy_flag?
attrs = declared_params(include_missing: false)
rename_key(attrs, :strategies, :strategies_attributes)
update_value(attrs, :strategies_attributes) do |strategies|
strategies.map { |s| rename_key(s, :scopes, :scopes_attributes) }
end
result = ::FeatureFlags::UpdateService
.new(user_project, current_user, attrs)
.execute(feature_flag)
if result[:status] == :success
present_entity(result[:feature_flag])
else
render_api_error!(result[:message], result[:http_status])
end
......@@ -140,9 +192,7 @@ module API
.execute(feature_flag)
if result[:status] == :success
present result[:feature_flag],
with: EE::API::Entities::FeatureFlag,
feature_flags_new_version_enabled: feature_flags_new_version_enabled?
present_entity(result[:feature_flag])
else
render_api_error!(result[:message], result[:http_status])
end
......@@ -163,18 +213,38 @@ module API
authorize! :create_feature_flag, user_project
end
def authorize_update_feature_flag!
authorize! :update_feature_flag, feature_flag
end
def authorize_destroy_feature_flag!
authorize! :destroy_feature_flag, feature_flag
end
def present_entity(result)
present result,
with: EE::API::Entities::FeatureFlag,
feature_flags_new_version_enabled: feature_flags_new_version_enabled?
end
def ensure_post_version_2_flags_enabled!
unless feature_flags_new_version_enabled?
render_api_error!('Version 2 flags are not enabled for this project', :unprocessable_entity)
end
end
def feature_flag
@feature_flag ||= if Feature.enabled?(:feature_flags_new_version, user_project)
@feature_flag ||= if feature_flags_new_version_enabled?
user_project.operations_feature_flags.find_by_name!(params[:name])
else
user_project.operations_feature_flags.legacy_flag.find_by_name!(params[:name])
end
end
def new_version_flag_present?
user_project.operations_feature_flags.new_version_flag.find_by_name(params[:name]).present?
end
def feature_flags_new_version_enabled?
Feature.enabled?(:feature_flags_new_version, user_project)
end
......@@ -183,6 +253,11 @@ module API
hash[new_key] = hash.delete(old_key) if hash.key?(old_key)
hash
end
def update_value(hash, key)
hash[key] = yield(hash[key]) if hash.key?(key)
hash
end
end
end
end
......@@ -238,6 +238,14 @@ describe API::FeatureFlags do
end
describe 'POST /projects/:id/feature_flags' do
def default_scope
{
environment_scope: '*',
active: false,
strategies: [{ name: 'default', parameters: {} }].to_json
}
end
subject do
post api("/projects/#{project.id}/feature_flags", user), params: params
end
......@@ -260,6 +268,16 @@ describe API::FeatureFlags do
expect(feature_flag.description).to eq(params[:description])
end
it 'defaults to a version 1 (legacy) feature flag' do
subject
expect(response).to have_gitlab_http_status(:created)
expect(response).to match_response_schema('public_api/v4/feature_flag', dir: 'ee')
feature_flag = project.operations_feature_flags.last
expect(feature_flag.version).to eq('legacy_flag')
end
it_behaves_like 'check user permission'
it 'returns version' do
......@@ -341,12 +359,100 @@ describe API::FeatureFlags do
end
end
def default_scope
{
environment_scope: '*',
active: false,
strategies: [{ name: 'default', parameters: {} }].to_json
}
context 'when creating a version 2 feature flag' do
it 'creates a new feature flag' do
params = {
name: 'new-feature',
version: 'new_version_flag'
}
post api("/projects/#{project.id}/feature_flags", user), params: params
expect(response).to have_gitlab_http_status(:created)
expect(response).to match_response_schema('public_api/v4/feature_flag', dir: 'ee')
feature_flag = project.operations_feature_flags.last
expect(feature_flag.name).to eq(params[:name])
expect(feature_flag.version).to eq('new_version_flag')
end
it 'creates a new feature flag with strategies' do
params = {
name: 'new-feature',
version: 'new_version_flag',
strategies: [{
name: 'userWithId',
parameters: { 'userIds': 'user1' }
}]
}
post api("/projects/#{project.id}/feature_flags", user), params: params
expect(response).to have_gitlab_http_status(:created)
expect(response).to match_response_schema('public_api/v4/feature_flag', dir: 'ee')
feature_flag = project.operations_feature_flags.last
expect(feature_flag.name).to eq(params[:name])
expect(feature_flag.version).to eq('new_version_flag')
expect(feature_flag.strategies.map { |s| s.slice(:name, :parameters).deep_symbolize_keys }).to eq([{
name: 'userWithId',
parameters: { userIds: 'user1' }
}])
end
it 'creates a new feature flag with strategies with scopes' do
params = {
name: 'new-feature',
version: 'new_version_flag',
strategies: [{
name: 'gradualRolloutUserId',
parameters: { groupId: 'default', percentage: '50' },
scopes: [{
environment_scope: 'staging'
}]
}]
}
post api("/projects/#{project.id}/feature_flags", user), params: params
expect(response).to have_gitlab_http_status(:created)
expect(response).to match_response_schema('public_api/v4/feature_flag', dir: 'ee')
feature_flag = project.operations_feature_flags.last
expect(feature_flag.name).to eq(params[:name])
expect(feature_flag.version).to eq('new_version_flag')
expect(feature_flag.strategies.map { |s| s.slice(:name, :parameters).deep_symbolize_keys }).to eq([{
name: 'gradualRolloutUserId',
parameters: { groupId: 'default', percentage: '50' }
}])
expect(feature_flag.strategies.first.scopes.map { |s| s.slice(:environment_scope).deep_symbolize_keys }).to eq([{
environment_scope: 'staging'
}])
end
it 'returns a 422 when the feature flag is disabled' do
stub_feature_flags(feature_flags_new_version: false)
params = {
name: 'new-feature',
version: 'new_version_flag'
}
post api("/projects/#{project.id}/feature_flags", user), params: params
expect(response).to have_gitlab_http_status(:unprocessable_entity)
expect(json_response).to eq({ 'message' => 'Version 2 flags are not enabled for this project' })
expect(project.operations_feature_flags.count).to eq(0)
end
end
context 'when given invalid parameters' do
it 'responds with a 400 when given an invalid version' do
params = { name: 'new-feature', version: 'bad_value' }
post api("/projects/#{project.id}/feature_flags", user), params: params
expect(response).to have_gitlab_http_status(:bad_request)
end
end
end
......@@ -374,6 +480,18 @@ describe API::FeatureFlags do
expect(response).to match_response_schema('public_api/v4/feature_flag', dir: 'ee')
expect(feature_flag.name).to eq(params[:name])
expect(scope.strategies).to eq([JSON.parse(params[:strategy])])
expect(feature_flag.version).to eq('legacy_flag')
end
it 'returns the flag version and strategies in the json response' do
subject
expect(response).to have_gitlab_http_status(:ok)
expect(response).to match_response_schema('public_api/v4/feature_flag', dir: 'ee')
expect(json_response.slice('version', 'strategies')).to eq({
'version' => 'legacy_flag',
'strategies' => []
})
end
it_behaves_like 'check user permission'
......@@ -423,6 +541,20 @@ describe API::FeatureFlags do
end
end
end
context 'with a version 2 flag' do
let!(:feature_flag) { create(:operations_feature_flag, :new_version_flag, project: project, name: params[:name]) }
it 'does not change the flag and returns an unprocessable_entity response' do
subject
expect(response).to have_gitlab_http_status(:unprocessable_entity)
expect(json_response).to eq({ 'message' => 'Version 2 flags not supported' })
feature_flag.reload
expect(feature_flag.scopes).to eq([])
expect(feature_flag.strategies).to eq([])
end
end
end
describe 'POST /projects/:id/feature_flags/:name/disable' do
......@@ -472,6 +604,17 @@ describe API::FeatureFlags do
.to eq([{ name: 'userWithId', parameters: { userIds: 'Project:2' } }.deep_stringify_keys])
end
it 'returns the flag version and strategies in the json response' do
subject
expect(response).to have_gitlab_http_status(:ok)
expect(response).to match_response_schema('public_api/v4/feature_flag', dir: 'ee')
expect(json_response.slice('version', 'strategies')).to eq({
'version' => 'legacy_flag',
'strategies' => []
})
end
it_behaves_like 'check user permission'
context 'when strategies become empty array after the removal' do
......@@ -501,6 +644,268 @@ describe API::FeatureFlags do
it_behaves_like 'not found'
end
end
context 'with a version 2 feature flag' do
let!(:feature_flag) { create(:operations_feature_flag, :new_version_flag, project: project, name: params[:name]) }
it 'does not change the flag and returns an unprocessable_entity response' do
subject
expect(response).to have_gitlab_http_status(:unprocessable_entity)
expect(json_response).to eq({ 'message' => 'Version 2 flags not supported' })
feature_flag.reload
expect(feature_flag.scopes).to eq([])
expect(feature_flag.strategies).to eq([])
end
end
end
describe 'PUT /projects/:id/feature_flags/:name' do
context 'with a legacy feature flag' do
let!(:feature_flag) do
create(:operations_feature_flag, :legacy_flag, project: project,
name: 'feature1', description: 'old description')
end
it 'returns a 404 if the feature is disabled' do
stub_feature_flags(feature_flags_new_version: false)
params = { description: 'new description' }
put api("/projects/#{project.id}/feature_flags/feature1", user), params: params
expect(response).to have_gitlab_http_status(:not_found)
expect(feature_flag.reload.description).to eq('old description')
end
it 'returns a 422' do
params = { description: 'new description' }
put api("/projects/#{project.id}/feature_flags/feature1", user), params: params
expect(response).to have_gitlab_http_status(:unprocessable_entity)
expect(json_response).to eq({ 'message' => 'PUT operations are not supported for legacy feature flags' })
expect(feature_flag.reload.description).to eq('old description')
end
end
context 'with a version 2 feature flag' do
let!(:feature_flag) do
create(:operations_feature_flag, :new_version_flag, project: project,
name: 'feature1', description: 'old description')
end
it 'returns a 404 if the feature is disabled' do
stub_feature_flags(feature_flags_new_version: false)
params = { description: 'new description' }
put api("/projects/#{project.id}/feature_flags/feature1", user), params: params
expect(response).to have_gitlab_http_status(:not_found)
expect(feature_flag.reload.description).to eq('old description')
end
it 'returns a 404 if the feature flag does not exist' do
params = { description: 'new description' }
put api("/projects/#{project.id}/feature_flags/other_flag_name", user), params: params
expect(response).to have_gitlab_http_status(:not_found)
expect(feature_flag.reload.description).to eq('old description')
end
it 'forbids a request for a reporter' do
params = { description: 'new description' }
put api("/projects/#{project.id}/feature_flags/feature1", reporter), params: params
expect(response).to have_gitlab_http_status(:forbidden)
expect(feature_flag.reload.description).to eq('old description')
end
it 'returns an error for an invalid update' do
strategy = create(:operations_strategy, feature_flag: feature_flag, name: 'default', parameters: {})
params = {
strategies: [{
id: strategy.id,
name: 'gradualRolloutUserId',
parameters: { bad: 'params' }
}]
}
put api("/projects/#{project.id}/feature_flags/feature1", user), params: params
expect(response).to have_gitlab_http_status(:bad_request)
expect(json_response['message']).not_to be_nil
result = feature_flag.reload.strategies.map { |s| s.slice(:id, :name, :parameters).deep_symbolize_keys }
expect(result).to eq([{
id: strategy.id,
name: 'default',
parameters: {}
}])
end
it 'updates the feature flag' do
params = { description: 'new description' }
put api("/projects/#{project.id}/feature_flags/feature1", user), params: params
expect(response).to have_gitlab_http_status(:ok)
expect(response).to match_response_schema('public_api/v4/feature_flag', dir: 'ee')
expect(feature_flag.reload.description).to eq('new description')
end
it 'ignores a provided version parameter' do
params = { description: 'other description', version: 'bad_value' }
put api("/projects/#{project.id}/feature_flags/feature1", user), params: params
expect(response).to have_gitlab_http_status(:ok)
expect(response).to match_response_schema('public_api/v4/feature_flag', dir: 'ee')
expect(feature_flag.reload.description).to eq('other description')
end
it 'returns the feature flag json' do
params = { description: 'new description' }
put api("/projects/#{project.id}/feature_flags/feature1", user), params: params
expect(response).to have_gitlab_http_status(:ok)
expect(response).to match_response_schema('public_api/v4/feature_flag', dir: 'ee')
feature_flag.reload
expect(json_response).to eq({
'name' => 'feature1',
'description' => 'new description',
'created_at' => feature_flag.created_at.as_json,
'updated_at' => feature_flag.updated_at.as_json,
'scopes' => [],
'strategies' => [],
'version' => 'new_version_flag'
})
end
it 'updates an existing feature flag strategy' do
strategy = create(:operations_strategy, feature_flag: feature_flag, name: 'default', parameters: {})
params = {
strategies: [{
id: strategy.id,
name: 'gradualRolloutUserId',
parameters: { groupId: 'default', percentage: '10' }
}]
}
put api("/projects/#{project.id}/feature_flags/feature1", user), params: params
expect(response).to have_gitlab_http_status(:ok)
expect(response).to match_response_schema('public_api/v4/feature_flag', dir: 'ee')
result = feature_flag.reload.strategies.map { |s| s.slice(:id, :name, :parameters).deep_symbolize_keys }
expect(result).to eq([{
id: strategy.id,
name: 'gradualRolloutUserId',
parameters: { groupId: 'default', percentage: '10' }
}])
end
it 'creates a new feature flag strategy' do
strategy = create(:operations_strategy, feature_flag: feature_flag, name: 'default', parameters: {})
params = {
strategies: [{
name: 'gradualRolloutUserId',
parameters: { groupId: 'default', percentage: '10' }
}]
}
put api("/projects/#{project.id}/feature_flags/feature1", user), params: params
expect(response).to have_gitlab_http_status(:ok)
expect(response).to match_response_schema('public_api/v4/feature_flag', dir: 'ee')
result = feature_flag.reload.strategies
.map { |s| s.slice(:id, :name, :parameters).deep_symbolize_keys }
.sort_by { |s| s[:name] }
expect(result.first[:id]).to eq(strategy.id)
expect(result.map { |s| s.slice(:name, :parameters) }).to eq([{
name: 'default',
parameters: {}
}, {
name: 'gradualRolloutUserId',
parameters: { groupId: 'default', percentage: '10' }
}])
end
it 'deletes a feature flag strategy' do
strategy_a = create(:operations_strategy, feature_flag: feature_flag, name: 'default', parameters: {})
strategy_b = create(:operations_strategy, feature_flag: feature_flag,
name: 'userWithId', parameters: { userIds: 'userA,userB' })
params = {
strategies: [{
id: strategy_a.id,
name: 'default',
parameters: {},
_destroy: true
}, {
id: strategy_b.id,
name: 'userWithId',
parameters: { userIds: 'userB' }
}]
}
put api("/projects/#{project.id}/feature_flags/feature1", user), params: params
expect(response).to have_gitlab_http_status(:ok)
expect(response).to match_response_schema('public_api/v4/feature_flag', dir: 'ee')
result = feature_flag.reload.strategies
.map { |s| s.slice(:id, :name, :parameters).deep_symbolize_keys }
.sort_by { |s| s[:name] }
expect(result).to eq([{
id: strategy_b.id,
name: 'userWithId',
parameters: { userIds: 'userB' }
}])
end
it 'updates an existing feature flag scope' do
strategy = create(:operations_strategy, feature_flag: feature_flag, name: 'default', parameters: {})
scope = create(:operations_scope, strategy: strategy, environment_scope: '*')
params = {
strategies: [{
id: strategy.id,
scopes: [{
id: scope.id,
environment_scope: 'production'
}]
}]
}
put api("/projects/#{project.id}/feature_flags/feature1", user), params: params
expect(response).to have_gitlab_http_status(:ok)
expect(response).to match_response_schema('public_api/v4/feature_flag', dir: 'ee')
result = feature_flag.reload.strategies.first.scopes.map { |s| s.slice(:id, :environment_scope).deep_symbolize_keys }
expect(result).to eq([{
id: scope.id,
environment_scope: 'production'
}])
end
it 'deletes an existing feature flag scope' do
strategy = create(:operations_strategy, feature_flag: feature_flag, name: 'default', parameters: {})
scope = create(:operations_scope, strategy: strategy, environment_scope: '*')
params = {
strategies: [{
id: strategy.id,
scopes: [{
id: scope.id,
_destroy: true
}]
}]
}
put api("/projects/#{project.id}/feature_flags/feature1", user), params: params
expect(response).to have_gitlab_http_status(:ok)
expect(response).to match_response_schema('public_api/v4/feature_flag', dir: 'ee')
expect(feature_flag.reload.strategies.first.scopes.count).to eq(0)
end
end
end
describe 'DELETE /projects/:id/feature_flags/:name' do
......@@ -533,5 +938,23 @@ describe API::FeatureFlags do
expect(response).to have_gitlab_http_status(:ok)
expect(json_response.key?('version')).to eq(false)
end
context 'with a version 2 feature flag' do
let!(:feature_flag) { create(:operations_feature_flag, :new_version_flag, project: project) }
it 'destroys the flag' do
expect { subject }.to change { Operations::FeatureFlag.count }.by(-1)
expect(response).to have_gitlab_http_status(:ok)
end
it 'returns a 404 if the feature is disabled' do
stub_feature_flags(feature_flags_new_version: false)
expect { subject }.not_to change { Operations::FeatureFlag.count }
expect(response).to have_gitlab_http_status(:not_found)
end
end
end
end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment