Commit b73e9255 authored by Amy Troschinetz's avatar Amy Troschinetz

Support flexible rollout strategy in the API

**app/models/operations/feature_flags/strategy.rb:**

Adds support for flexibleRollout strategy.

**ee/changelogs/unreleased/feature-flags-rollout-session-random.yml:**

Changelog.

**ee/app/controllers/projects/feature_flags_controller.rb:**

Updates to support permitted parameters for flexibleRollout.

**ee/spec/controllers/projects/feature_flags_controller_spec.rb:**

Tests for the controller to support flexibleRollout.

**ee/spec/requests/api/feature_flags_spec.rb:**

Tests for the API to support flexibleRollout.

**spec/models/operations/feature_flags/strategy_spec.rb:**

Tests for the model to support flexibleRollout.

**doc/api/feature_flags.md:**

Documentation for the new API support.
parent 75fe1f08
......@@ -6,14 +6,17 @@ module Operations
STRATEGY_DEFAULT = 'default'
STRATEGY_GITLABUSERLIST = 'gitlabUserList'
STRATEGY_GRADUALROLLOUTUSERID = 'gradualRolloutUserId'
STRATEGY_FLEXIBLEROLLOUT = 'flexibleRollout'
STRATEGY_USERWITHID = 'userWithId'
STRATEGIES = {
STRATEGY_DEFAULT => [].freeze,
STRATEGY_GITLABUSERLIST => [].freeze,
STRATEGY_GRADUALROLLOUTUSERID => %w[groupId percentage].freeze,
STRATEGY_FLEXIBLEROLLOUT => %w[groupId rollout stickiness].freeze,
STRATEGY_USERWITHID => ['userIds'].freeze
}.freeze
USERID_MAX_LENGTH = 256
STICKINESS_SETTINGS = %w[DEFAULT USERID SESSIONID RANDOM].freeze
self.table_name = 'operations_strategies'
......@@ -67,16 +70,25 @@ module Operations
case name
when STRATEGY_GRADUALROLLOUTUSERID
gradual_rollout_user_id_parameters_validation
when STRATEGY_FLEXIBLEROLLOUT
flexible_rollout_parameters_validation
when STRATEGY_USERWITHID
FeatureFlagUserXidsValidator.validate_user_xids(self, :parameters, parameters['userIds'], 'userIds')
end
end
def within_range?(value, min, max)
return false unless value.is_a?(String)
return false unless value.match?(/\A\d+\z/)
value.to_i.between?(min, max)
end
def gradual_rollout_user_id_parameters_validation
percentage = parameters['percentage']
group_id = parameters['groupId']
unless percentage.is_a?(String) && percentage.match(/\A[1-9]?[0-9]\z|\A100\z/)
unless within_range?(percentage, 0, 100)
parameters_error('percentage must be a string between 0 and 100 inclusive')
end
......@@ -85,6 +97,25 @@ module Operations
end
end
def flexible_rollout_parameters_validation
stickiness = parameters['stickiness']
group_id = parameters['groupId']
rollout = parameters['rollout']
unless STICKINESS_SETTINGS.include?(stickiness)
options = STICKINESS_SETTINGS.to_sentence(last_word_connector: ', or ')
parameters_error("stickiness parameter must be #{options}")
end
unless group_id.is_a?(String) && group_id.match(/\A[a-z]{1,32}\z/)
parameters_error('groupId parameter is invalid')
end
unless within_range?(rollout, 0, 100)
parameters_error('rollout must be a string between 0 and 100 inclusive')
end
end
def parameters_error(message)
errors.add(:parameters, message)
false
......
......@@ -151,7 +151,7 @@ POST /projects/:id/feature_flags
| `description` | string | no | The description of the feature flag. |
| `active` | boolean | no | The active state of the flag. Defaults to true. [Supported](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/38350) in GitLab 13.3 and later. |
| `strategies` | JSON | no | The feature flag [strategies](../operations/feature_flags.md#feature-flag-strategies). |
| `strategies:name` | JSON | no | The strategy name. |
| `strategies:name` | JSON | no | The strategy name. Can be `default`, `gradualRolloutUserId`, `userWithId`, or `gitlabUserList`. In [GitLab 13.5](https://gitlab.com/gitlab-org/gitlab/-/issues/36380) and later, can be [`flexibleRollout`](https://unleash.github.io/docs/activation_strategy#flexiblerollout). |
| `strategies:parameters` | JSON | no | The strategy parameters. |
| `strategies:scopes` | JSON | no | The scopes for the strategy. |
| `strategies:scopes:environment_scope` | string | no | The environment spec for the scope. |
......
......@@ -123,7 +123,9 @@ class Projects::FeatureFlagsController < Projects::ApplicationController
.permit(:name, :description, :active, :version,
scopes_attributes: [:environment_scope, :active,
strategies: [:name, parameters: [:groupId, :percentage, :userIds]]],
strategies_attributes: [:name, :user_list_id, parameters: [:groupId, :percentage, :userIds], scopes_attributes: [:environment_scope]])
strategies_attributes: [:name, :user_list_id,
parameters: [:groupId, :percentage, :userIds, :rollout, :stickiness],
scopes_attributes: [:environment_scope]])
end
def update_params
......@@ -132,7 +134,7 @@ class Projects::FeatureFlagsController < Projects::ApplicationController
scopes_attributes: [:id, :environment_scope, :active, :_destroy,
strategies: [:name, parameters: [:groupId, :percentage, :userIds]]],
strategies_attributes: [:id, :name, :user_list_id, :_destroy,
parameters: [:groupId, :percentage, :userIds],
parameters: [:groupId, :percentage, :userIds, :rollout, :stickiness],
scopes_attributes: [:id, :environment_scope, :_destroy]])
end
......
---
title: Support flexible rollout strategy in the API
merge_request: 43340
author:
type: added
......@@ -693,6 +693,39 @@ RSpec.describe Projects::FeatureFlagsController do
end
end
context 'when creating a version 2 feature flag with a flexibleRollout strategy' do
let(:params) do
{
namespace_id: project.namespace,
project_id: project,
operations_feature_flag: {
name: 'my_feature_flag',
active: true,
version: 'new_version_flag',
strategies_attributes: [{
name: 'flexibleRollout',
parameters: { groupId: 'default', rollout: '15', stickiness: 'DEFAULT' },
scopes_attributes: [{ environment_scope: 'production' }]
}]
}
}
end
it 'creates the new strategy' do
subject
expect(response).to have_gitlab_http_status(:ok)
strategy_json = json_response['strategies'].first
expect(strategy_json['name']).to eq('flexibleRollout')
expect(strategy_json['parameters']).to eq({ 'groupId' => 'default', 'rollout' => '15', 'stickiness' => 'DEFAULT' })
expect(strategy_json['scopes'].count).to eq(1)
scope_json = strategy_json['scopes'].first
expect(scope_json['environment_scope']).to eq('production')
end
end
context 'when creating a version 2 feature flag with a gitlabUserList strategy' do
let!(:user_list) do
create(:operations_feature_flag_user_list, project: project,
......@@ -1360,6 +1393,24 @@ RSpec.describe Projects::FeatureFlagsController do
expect(strategy_json['scopes']).to eq([])
end
it 'creates a flexibleRollout strategy' do
put_request(new_version_flag, strategies_attributes: [{
name: 'flexibleRollout',
parameters: { groupId: 'default', rollout: '30', stickiness: 'DEFAULT' }
}])
expect(response).to have_gitlab_http_status(:ok)
expect(json_response['strategies'].count).to eq(1)
strategy_json = json_response['strategies'].first
expect(strategy_json['name']).to eq('flexibleRollout')
expect(strategy_json['parameters']).to eq({
'groupId' => 'default',
'rollout' => '30',
'stickiness' => 'DEFAULT'
})
expect(strategy_json['scopes']).to eq([])
end
it 'creates a gitlabUserList strategy' do
user_list = create(:operations_feature_flag_user_list, project: project, name: 'My List', user_xids: 'user1,user2')
......
......@@ -446,7 +446,7 @@ RSpec.describe API::FeatureFlags do
}])
end
it 'creates a new feature flag with strategies with scopes' do
it 'creates a new feature flag with gradual rollout strategy with scopes' do
params = {
name: 'new-feature',
version: 'new_version_flag',
......@@ -476,6 +476,36 @@ RSpec.describe API::FeatureFlags do
}])
end
it 'creates a new feature flag with flexible rollout strategy with scopes' do
params = {
name: 'new-feature',
version: 'new_version_flag',
strategies: [{
name: 'flexibleRollout',
parameters: { groupId: 'default', rollout: '50', stickiness: 'DEFAULT' },
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: 'flexibleRollout',
parameters: { groupId: 'default', rollout: '50', stickiness: 'DEFAULT' }
}])
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 = {
......@@ -769,7 +799,7 @@ RSpec.describe API::FeatureFlags do
expect(feature_flag.reload.description).to eq('old description')
end
it 'returns an error for an invalid update' do
it 'returns an error for an invalid update of gradual rollout' do
strategy = create(:operations_strategy, feature_flag: feature_flag, name: 'default', parameters: {})
params = {
strategies: [{
......@@ -791,6 +821,28 @@ RSpec.describe API::FeatureFlags do
}])
end
it 'returns an error for an invalid update of flexible rollout' do
strategy = create(:operations_strategy, feature_flag: feature_flag, name: 'default', parameters: {})
params = {
strategies: [{
id: strategy.id,
name: 'flexibleRollout',
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' }
......@@ -853,7 +905,7 @@ RSpec.describe API::FeatureFlags do
})
end
it 'updates an existing feature flag strategy' do
it 'updates an existing feature flag strategy to be gradual rollout strategy' do
strategy = create(:operations_strategy, feature_flag: feature_flag, name: 'default', parameters: {})
params = {
strategies: [{
......@@ -875,7 +927,29 @@ RSpec.describe API::FeatureFlags do
}])
end
it 'creates a new feature flag strategy' do
it 'updates an existing feature flag strategy to be flexible rollout strategy' do
strategy = create(:operations_strategy, feature_flag: feature_flag, name: 'default', parameters: {})
params = {
strategies: [{
id: strategy.id,
name: 'flexibleRollout',
parameters: { groupId: 'default', rollout: '10', stickiness: 'DEFAULT' }
}]
}
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: 'flexibleRollout',
parameters: { groupId: 'default', rollout: '10', stickiness: 'DEFAULT' }
}])
end
it 'adds a new gradual rollout strategy to a feature flag' do
strategy = create(:operations_strategy, feature_flag: feature_flag, name: 'default', parameters: {})
params = {
strategies: [{
......@@ -901,6 +975,32 @@ RSpec.describe API::FeatureFlags do
}])
end
it 'adds a new gradual flexible strategy to a feature flag' do
strategy = create(:operations_strategy, feature_flag: feature_flag, name: 'default', parameters: {})
params = {
strategies: [{
name: 'flexibleRollout',
parameters: { groupId: 'default', rollout: '10', stickiness: 'DEFAULT' }
}]
}
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: 'flexibleRollout',
parameters: { groupId: 'default', rollout: '10', stickiness: 'DEFAULT' }
}])
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,
......
......@@ -8,7 +8,7 @@ RSpec.describe Operations::FeatureFlags::Strategy do
describe 'validations' do
it do
is_expected.to validate_inclusion_of(:name)
.in_array(%w[default gradualRolloutUserId userWithId gitlabUserList])
.in_array(%w[default gradualRolloutUserId flexibleRollout userWithId gitlabUserList])
.with_message('strategy name is invalid')
end
......@@ -55,9 +55,9 @@ RSpec.describe Operations::FeatureFlags::Strategy do
describe 'percentage' do
where(:invalid_value) do
[50, 40.0, { key: "value" }, "garbage", "00", "01", "101", "-1", "-10", "0100",
"1000", "10.0", "5%", "25%", "100hi", "e100", "30m", " ", "\r\n", "\n", "\t",
"\n10", "20\n", "\n100", "100\n", "\n ", nil]
[50, 40.0, { key: "value" }, "garbage", "101", "-1", "-10", "1000", "10.0", "5%", "25%",
"100hi", "e100", "30m", " ", "\r\n", "\n", "\t", "\n10", "20\n", "\n100", "100\n",
"\n ", nil]
end
with_them do
it 'must be a string value between 0 and 100 inclusive and without a percentage sign' do
......@@ -117,6 +117,134 @@ RSpec.describe Operations::FeatureFlags::Strategy do
end
end
context 'when the strategy name is flexibleRollout' do
valid_parameters = { rollout: '40', groupId: 'mygroup', stickiness: 'DEFAULT' }
where(invalid_parameters: [
nil,
{},
*valid_parameters.to_a.combination(1).to_a.map { |p| p.to_h },
*valid_parameters.to_a.combination(2).to_a.map { |p| p.to_h },
{ **valid_parameters, userIds: '4' },
{ **valid_parameters, extra: nil }
])
with_them do
it 'must have valid parameters for the strategy' do
feature_flag = create(:operations_feature_flag, project: project)
strategy = described_class.create(feature_flag: feature_flag,
name: 'flexibleRollout',
parameters: invalid_parameters)
expect(strategy.errors[:parameters]).to eq(['parameters are invalid'])
end
end
[
[:rollout, '10'],
[:stickiness, 'DEFAULT'],
[:groupId, 'mygroup']
].permutation(3).each do |parameters|
it "allows the parameters in the order #{parameters.map { |p| p.first }.join(', ')}" do
feature_flag = create(:operations_feature_flag, project: project)
strategy = described_class.create(feature_flag: feature_flag,
name: 'flexibleRollout',
parameters: Hash[parameters])
expect(strategy.errors[:parameters]).to be_empty
end
end
describe 'rollout' do
where(invalid_value: [50, 40.0, { key: "value" }, "garbage", "101", "-1", " ", "-10",
"1000", "10.0", "5%", "25%", "100hi", "e100", "30m", "\r\n",
"\n", "\t", "\n10", "20\n", "\n100", "100\n", "\n ", nil])
with_them do
it 'must be a string value between 0 and 100 inclusive and without a percentage sign' do
feature_flag = create(:operations_feature_flag, project: project)
parameters = { stickiness: 'DEFAULT', groupId: 'mygroup', rollout: invalid_value }
strategy = described_class.create(feature_flag: feature_flag,
name: 'flexibleRollout',
parameters: parameters)
expect(strategy.errors[:parameters]).to eq([
'rollout must be a string between 0 and 100 inclusive'
])
end
end
where(valid_value: %w[0 1 10 38 100 93])
with_them do
it 'must be a string value between 0 and 100 inclusive and without a percentage sign' do
feature_flag = create(:operations_feature_flag, project: project)
parameters = { stickiness: 'DEFAULT', groupId: 'mygroup', rollout: valid_value }
strategy = described_class.create(feature_flag: feature_flag,
name: 'flexibleRollout',
parameters: parameters)
expect(strategy.errors[:parameters]).to eq([])
end
end
end
describe 'groupId' do
where(invalid_value: [nil, 4, 50.0, {}, 'spaces bad', 'bad$', '%bad', '<bad', 'bad>',
'!bad', '.bad', 'Bad', 'bad1', "", " ", "b" * 33, "ba_d", "ba\nd"])
with_them do
it 'must be a string value of up to 32 lowercase characters' do
feature_flag = create(:operations_feature_flag, project: project)
parameters = { stickiness: 'DEFAULT', groupId: invalid_value, rollout: '40' }
strategy = described_class.create(feature_flag: feature_flag,
name: 'flexibleRollout',
parameters: parameters)
expect(strategy.errors[:parameters]).to eq(['groupId parameter is invalid'])
end
end
where(valid_value: ["somegroup", "anothergroup", "okay", "g", "a" * 32])
with_them do
it 'must be a string value of up to 32 lowercase characters' do
feature_flag = create(:operations_feature_flag, project: project)
parameters = { stickiness: 'DEFAULT', groupId: valid_value, rollout: '40' }
strategy = described_class.create(feature_flag: feature_flag,
name: 'flexibleRollout',
parameters: parameters)
expect(strategy.errors[:parameters]).to eq([])
end
end
end
describe 'stickiness' do
where(invalid_value: [nil, " ", "default", "DEFAULT\n", "UserId", "USER", "USERID "])
with_them do
it 'must be a string representing a supported stickiness setting' do
feature_flag = create(:operations_feature_flag, project: project)
parameters = { stickiness: invalid_value, groupId: 'mygroup', rollout: '40' }
strategy = described_class.create(feature_flag: feature_flag,
name: 'flexibleRollout',
parameters: parameters)
expect(strategy.errors[:parameters]).to eq([
'stickiness parameter must be DEFAULT, USERID, SESSIONID, or RANDOM'
])
end
end
where(valid_value: %w[DEFAULT USERID SESSIONID RANDOM])
with_them do
it 'must be a string representing a supported stickiness setting' do
feature_flag = create(:operations_feature_flag, project: project)
parameters = { stickiness: valid_value, groupId: 'mygroup', rollout: '40' }
strategy = described_class.create(feature_flag: feature_flag,
name: 'flexibleRollout',
parameters: parameters)
expect(strategy.errors[:parameters]).to eq([])
end
end
end
end
context 'when the strategy name is userWithId' do
where(:invalid_parameters) do
[nil, { userIds: 'sam', percentage: '40' }, { userIds: 'sam', some: 'param' }, { percentage: '40' }, {}]
......@@ -318,6 +446,32 @@ RSpec.describe Operations::FeatureFlags::Strategy do
expect(strategy.errors[:user_list]).to be_empty
end
end
context 'when name is flexibleRollout' do
it 'is invalid when associated with a user list' do
feature_flag = create(:operations_feature_flag, project: project)
user_list = create(:operations_feature_flag_user_list, project: project)
strategy = described_class.create(feature_flag: feature_flag,
name: 'flexibleRollout',
user_list: user_list,
parameters: { groupId: 'default',
rollout: '10',
stickiness: 'DEFAULT' })
expect(strategy.errors[:user_list]).to eq(['must be blank'])
end
it 'is valid without a user list' do
feature_flag = create(:operations_feature_flag, project: project)
strategy = described_class.create(feature_flag: feature_flag,
name: 'flexibleRollout',
parameters: { groupId: 'default',
rollout: '10',
stickiness: 'DEFAULT' })
expect(strategy.errors[:user_list]).to be_empty
end
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