Commit 77e858ae authored by Kamil Trzciński's avatar Kamil Trzciński

Improve `bin/feature-flag` script

This makes:

- `licensed` flags to be `ee-only`
- ensure that resulting YAML is always ordered
parent 7abc28a3
...@@ -153,6 +153,10 @@ class FeatureFlagOptionParser ...@@ -153,6 +153,10 @@ class FeatureFlagOptionParser
end end
end end
def read_ee_only(options)
TYPES.dig(options.type, :ee_only)
end
def read_rollout_issue_url(options) def read_rollout_issue_url(options)
return unless TYPES.dig(options.type, :rollout_issue) return unless TYPES.dig(options.type, :rollout_issue)
...@@ -204,6 +208,7 @@ class FeatureFlagCreator ...@@ -204,6 +208,7 @@ class FeatureFlagCreator
# Read type from $stdin unless is already set # Read type from $stdin unless is already set
options.type ||= FeatureFlagOptionParser.read_type options.type ||= FeatureFlagOptionParser.read_type
options.ee ||= FeatureFlagOptionParser.read_ee_only(options)
options.group ||= FeatureFlagOptionParser.read_group options.group ||= FeatureFlagOptionParser.read_group
options.introduced_by_url ||= FeatureFlagOptionParser.read_introduced_by_url options.introduced_by_url ||= FeatureFlagOptionParser.read_introduced_by_url
options.rollout_issue_url ||= FeatureFlagOptionParser.read_rollout_issue_url(options) options.rollout_issue_url ||= FeatureFlagOptionParser.read_rollout_issue_url(options)
...@@ -224,14 +229,22 @@ class FeatureFlagCreator ...@@ -224,14 +229,22 @@ class FeatureFlagCreator
private private
def contents def contents
YAML.dump( # Slice is used to ensure that YAML keys
# are always ordered in a predictable way
config_hash.slice(
*::Feature::Shared::PARAMS.map(&:to_s)
).to_yaml
end
def config_hash
{
'name' => options.name, 'name' => options.name,
'introduced_by_url' => options.introduced_by_url, 'introduced_by_url' => options.introduced_by_url,
'rollout_issue_url' => options.rollout_issue_url, 'rollout_issue_url' => options.rollout_issue_url,
'group' => options.group.to_s, 'group' => options.group,
'type' => options.type.to_s, 'type' => options.type.to_s,
'default_enabled' => FeatureFlagOptionParser.read_default_enabled(options) 'default_enabled' => FeatureFlagOptionParser.read_default_enabled(options)
).strip }
end end
def write def write
......
...@@ -9,12 +9,14 @@ class Feature ...@@ -9,12 +9,14 @@ class Feature
# optional: defines if a on-disk definition is required for this feature flag type # optional: defines if a on-disk definition is required for this feature flag type
# rollout_issue: defines if `bin/feature-flag` asks for rollout issue # rollout_issue: defines if `bin/feature-flag` asks for rollout issue
# default_enabled: defines a default state of a feature flag when created by `bin/feature-flag` # default_enabled: defines a default state of a feature flag when created by `bin/feature-flag`
# ee_only: defines that a feature flag can only be created in a context of EE
# example: usage being shown when exception is raised # example: usage being shown when exception is raised
TYPES = { TYPES = {
development: { development: {
description: 'Short lived, used to enable unfinished code to be deployed', description: 'Short lived, used to enable unfinished code to be deployed',
optional: false, optional: false,
rollout_issue: true, rollout_issue: true,
ee_only: false,
default_enabled: false, default_enabled: false,
example: <<-EOS example: <<-EOS
Feature.enabled?(:my_feature_flag, project) Feature.enabled?(:my_feature_flag, project)
...@@ -26,6 +28,7 @@ class Feature ...@@ -26,6 +28,7 @@ class Feature
description: "Long-lived feature flags that control operational aspects of GitLab's behavior", description: "Long-lived feature flags that control operational aspects of GitLab's behavior",
optional: true, optional: true,
rollout_issue: false, rollout_issue: false,
ee_only: false,
default_enabled: false, default_enabled: false,
example: <<-EOS example: <<-EOS
Feature.enabled?(:my_ops_flag, type: ops) Feature.enabled?(:my_ops_flag, type: ops)
...@@ -36,6 +39,7 @@ class Feature ...@@ -36,6 +39,7 @@ class Feature
description: 'Permanent feature flags used to temporarily disable licensed features.', description: 'Permanent feature flags used to temporarily disable licensed features.',
optional: true, optional: true,
rollout_issue: false, rollout_issue: false,
ee_only: true,
default_enabled: true, default_enabled: true,
example: <<-EOS example: <<-EOS
project.feature_available?(:my_licensed_feature) project.feature_available?(:my_licensed_feature)
...@@ -44,6 +48,8 @@ class Feature ...@@ -44,6 +48,8 @@ class Feature
} }
}.freeze }.freeze
# The ordering of PARAMS defines an order in YAML
# This is done to ease the file comparison
PARAMS = %i[ PARAMS = %i[
name name
introduced_by_url introduced_by_url
......
...@@ -8,10 +8,6 @@ load File.expand_path('../../bin/feature-flag', __dir__) ...@@ -8,10 +8,6 @@ load File.expand_path('../../bin/feature-flag', __dir__)
RSpec.describe 'bin/feature-flag' do RSpec.describe 'bin/feature-flag' do
using RSpec::Parameterized::TableSyntax using RSpec::Parameterized::TableSyntax
before do
skip_feature_flags_yaml_validation
end
describe FeatureFlagCreator do describe FeatureFlagCreator do
let(:argv) { %w[feature-flag-name -t development -g group::memory -i https://url -m http://url] } let(:argv) { %w[feature-flag-name -t development -g group::memory -i https://url -m http://url] }
let(:options) { FeatureFlagOptionParser.parse(argv) } let(:options) { FeatureFlagOptionParser.parse(argv) }
...@@ -244,5 +240,18 @@ RSpec.describe 'bin/feature-flag' do ...@@ -244,5 +240,18 @@ RSpec.describe 'bin/feature-flag' do
end end
end end
end end
describe '.read_ee_only' do
where(:type, :is_ee_only) do
:development | false
:licensed | true
end
with_them do
let(:options) { OpenStruct.new(name: 'foo', type: type) }
it { expect(described_class.read_ee_only(options)).to eq(is_ee_only) }
end
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