Run package-and-qa on feature flag config changes

This commit is contained in:
Mark Lapierre 2021-10-19 00:00:37 +00:00 committed by Anastasia McDonald
parent 41fbe34a47
commit cb15131628
13 changed files with 295 additions and 26 deletions

View file

@ -57,12 +57,7 @@ update-qa-cache:
- install_gitlab_gem
script:
- ./scripts/trigger-build omnibus
package-and-qa:
extends:
- .package-and-qa-base
- .qa:rules:package-and-qa
# This job often times out, so temporarily use private runners and a long timeout: https://gitlab.com/gitlab-org/gitlab/-/issues/238563
# These jobs often time out, so temporarily use private runners and a long timeout: https://gitlab.com/gitlab-org/gitlab/-/issues/238563
tags:
- prm
timeout: 4h
@ -71,3 +66,34 @@ package-and-qa:
artifacts: false
- job: build-assets-image
artifacts: false
.package-and-qa-ff-base:
needs:
- detect-tests
variables:
CHANGED_FILES: tmp/changed_files.txt
script:
- export GITLAB_QA_OPTIONS="--set-feature-flags $(scripts/changed-feature-flags --files $(cat $CHANGED_FILES | tr ' ' ',') --state $QA_FF_STATE)"
- echo $GITLAB_QA_OPTIONS
- ./scripts/trigger-build omnibus
package-and-qa:
extends:
- .package-and-qa-base
- .qa:rules:package-and-qa
package-and-qa-ff-enabled:
extends:
- .package-and-qa-base
- .package-and-qa-ff-base
- .qa:rules:package-and-qa:feature-flags
variables:
QA_FF_STATE: "enable"
package-and-qa-ff-disabled:
extends:
- .package-and-qa-base
- .package-and-qa-ff-base
- .qa:rules:package-and-qa:feature-flags
variables:
QA_FF_STATE: "disable"

View file

@ -372,6 +372,9 @@
- "config/helpers/**/*.js"
- "vendor/assets/javascripts/**/*"
.feature-flag-config-patterns: &feature-flag-config-patterns
- "{,ee/}config/feature_flags/**/*.yml"
################
# Shared rules #
################
@ -673,6 +676,9 @@
rules:
- <<: *if-not-ee
when: never
- <<: *if-dot-com-gitlab-org-and-security-merge-request
changes: *feature-flag-config-patterns
when: never
- <<: *if-dot-com-gitlab-org-and-security-merge-request
changes: *ci-qa-patterns
allow_failure: true
@ -686,6 +692,14 @@
- <<: *if-dot-com-gitlab-org-schedule
allow_failure: true
.qa:rules:package-and-qa:feature-flags:
rules:
- <<: *if-not-ee
when: never
- <<: *if-dot-com-gitlab-org-and-security-merge-request
changes: *feature-flag-config-patterns
allow_failure: true
###############
# Rails rules #
###############

View file

@ -79,11 +79,21 @@ for details.
End-to-end tests should pass with a feature flag enabled before it is enabled on Staging or on GitLab.com. Tests that need to be updated should be identified as part of [quad-planning](https://about.gitlab.com/handbook/engineering/quality/quad-planning/). The relevant [counterpart Software Engineer in Test](https://about.gitlab.com/handbook/engineering/quality/#individual-contributors) is responsible for updating the tests or assisting another engineer to do so. However, if a change does not go through quad-planning and a required test update is not made, test failures could block deployment.
If a test enables a feature flag as describe above, it is sufficient to run the `package-and-qa` job in a merge request containing the relevant changes.
Or, if the feature flag and relevant changes have already been merged, you can confirm that the tests
pass on `main`. The end-to-end tests run on `main` every two hours, and the results are posted to a [Test
Session Report, which is available in the testcase-sessions project](https://gitlab.com/gitlab-org/quality/testcase-sessions/-/issues?label_name%5B%5D=found%3Amaster).
### Automatic test execution when a feature flag definition changes
If the relevant tests do not enable the feature flag themselves, you can check if the tests will need
to be updated by opening a draft merge request that enables the flag by default and then running the `package-and-qa` job.
If a merge request adds or edits a [feature flag definition file](../../feature_flags/index.md#feature-flag-definition-and-validation),
two `package-and-qa` jobs will be included automatically in the merge request pipeline. One job will enable the defined
feature flag and the other will disable it. The jobs execute the same suite of tests to confirm that they pass with if
the feature flag is either enabled or disabled.
### Test execution during feature development
If an end-to-end test enables a feature flag, the end-to-end test suite can be used to test changes in a merge request
by running the `package-and-qa` job in the merge request pipeline. If the feature flag and relevant changes have already been merged, you can confirm that the tests
pass on the default branch. The end-to-end tests run on the default branch every two hours, and the results are posted to a [Test
Session Report, which is available in the testcase-sessions project](https://gitlab.com/gitlab-org/quality/testcase-sessions/-/issues?label_name%5B%5D=found%3Amain).
If the relevant tests do not enable the feature flag themselves, you can check if the tests will need to be updated by opening
a draft merge request that enables the flag by default via a [feature flag definition file](../../feature_flags/index.md#feature-flag-definition-and-validation).
That will [automatically execute the end-to-end test suite](#automatic-test-execution-when-a-feature-flag-definition-changes).
The merge request can be closed once the tests pass. If you need assistance to update the tests, please contact the relevant [stable counterpart in the Quality department](https://about.gitlab.com/handbook/engineering/quality/#individual-contributors), or any Software Engineer in Test if there is no stable counterpart for your group.

View file

@ -5,15 +5,16 @@
module QA
module Runtime
class Feature
SetFeatureError = Class.new(RuntimeError)
AuthorizationError = Class.new(RuntimeError)
UnknownScopeError = Class.new(RuntimeError)
UnknownStateError = Class.new(RuntimeError)
class << self
# Documentation: https://docs.gitlab.com/ee/api/features.html
include Support::API
SetFeatureError = Class.new(RuntimeError)
AuthorizationError = Class.new(RuntimeError)
UnknownScopeError = Class.new(RuntimeError)
def remove(key)
request = Runtime::API::Request.new(api_client, "/features/#{key}")
response = delete(request.url)
@ -30,6 +31,23 @@ def disable(key, **scopes)
set_and_verify(key, enable: false, **scopes)
end
# Set one or more flags to their specified state.
#
# @param [Hash] flags The feature flags and desired values, e.g., { 'flag1' => 'enabled', 'flag2' => "disabled" }
# @param [Hash] scopes The scope (user, project, group) to apply the feature flag to.
def set(flags, **scopes)
flags.each_pair do |flag, state|
case state
when 'enabled', 'enable', 'true', 1, true
enable(flag, **scopes)
when 'disabled', 'disable', 'false', 0, false
disable(flag, **scopes)
else
raise UnknownStateError, "Unknown feature flag state: #{state}"
end
end
end
def enabled?(key, **scopes)
feature = JSON.parse(get_features).find { |flag| flag['name'] == key.to_s }
feature && (feature['state'] == 'on' || feature['state'] == 'conditional' && scopes.present? && enabled_scope?(feature['gates'], **scopes))
@ -47,15 +65,15 @@ def enabled_scope?(gates, **scopes)
scopes.each do |key, value|
case key
when :project, :group, :user
actors = gates.filter { |i| i['key'] == 'actors' }.first['value']
break actors.include?("#{key.to_s.capitalize}:#{value.id}")
actors = gates.find { |i| i['key'] == 'actors' }['value']
return actors.include?("#{key.to_s.capitalize}:#{value.id}")
when :feature_group
groups = gates.filter { |i| i['key'] == 'groups' }.first['value']
break groups.include?(value)
else
raise UnknownScopeError, "Unknown scope: #{key}"
groups = gates.find { |i| i['key'] == 'groups' }['value']
return groups.include?(value)
end
end
raise UnknownScopeError, "Unknown scope in: #{scopes}"
end
def get_features

View file

@ -17,6 +17,22 @@ def launch!(argv)
arguments = OptionParser.new do |parser|
options.to_a.each do |opt|
# The argument for the --set-feature-flags option should look something like "flag1=enabled,flag2=disabled"
# Here we translate that string into a hash, e.g.: { 'flag1' => 'enabled', 'flag2' => "disabled" }
if opt.name == :set_feature_flags
parser.on(opt.arg, opt.desc) do |flags|
value = flags.split(',').each_with_object({}) do |pair, hash|
flag_name, flag_value = pair.split('=')
raise '--set-feature-flags requires flag name and flag state for each flag, e.g., flag1=enabled,flag2=disabled' unless flag_name && flag_value
hash[flag_name] = flag_value
end
Runtime::Scenario.define(opt.name, value)
end
next
end
parser.on(opt.arg, opt.desc) do |value|
Runtime::Scenario.define(opt.name, value)
end

View file

@ -8,6 +8,9 @@ module SharedAttributes
attribute :gitlab_address, '--address URL', 'Address of the instance to test'
attribute :enable_feature, '--enable-feature FEATURE_FLAG', 'Enable a feature before running tests'
attribute :disable_feature, '--disable-feature FEATURE_FLAG', 'Disable a feature before running tests'
attribute :set_feature_flags, '--set-feature-flags FEATURE_FLAGS',
'Set one or more feature flags before running tests. ' \
'Specify FEATURE_FLAGS as comma-separated flag=state pairs, e.g., "flag1=enabled,flag2=disabled"'
attribute :parallel, '--parallel', 'Execute tests in parallel'
attribute :loop, '--loop', 'Execute test repeatedly'
end

View file

@ -38,8 +38,8 @@ def perform(options, *args)
Runtime::Release.perform_before_hooks
Runtime::Feature.enable(options[:enable_feature]) if options.key?(:enable_feature)
Runtime::Feature.disable(options[:disable_feature]) if options.key?(:disable_feature) && (@feature_enabled = Runtime::Feature.enabled?(options[:disable_feature]))
Runtime::Feature.set(options[:set_feature_flags]) if options.key?(:set_feature_flags)
Specs::Runner.perform do |specs|
specs.tty = true

View file

@ -175,6 +175,20 @@
expect(described_class.enabled?(feature_flag)).to be_truthy
end
it 'raises an error when the scope is unknown' do
expect(QA::Runtime::API::Request)
.to receive(:new)
.with(api_client, "/features")
.and_return(request)
expect(described_class)
.to receive(:get)
.and_return(
Struct.new(:code, :body)
.new(200, %([{ "name": "a_flag", "state": "conditional", "gates": { "key": "groups", "value": ["foo"] } }])))
expect { described_class.enabled?(feature_flag, scope: 'foo') }.to raise_error(QA::Runtime::Feature::UnknownScopeError)
end
context 'when a project scope is provided' do
it_behaves_like 'checks a feature flag' do
let(:scope) { :project }
@ -212,4 +226,38 @@
end
end
end
describe '.set' do
let(:scope) { { scope: 'actor' } }
it 'raises an error when the flag state is unknown' do
expect(described_class).not_to receive(:enable)
expect(described_class).not_to receive(:disable)
expect { described_class.set({ foo: 'bar' }, **scope) }.to raise_error(QA::Runtime::Feature::UnknownStateError, 'Unknown feature flag state: bar')
end
it 'enables feature flags' do
expect(described_class).to receive(:enable).with(:flag1, scope)
expect(described_class).to receive(:enable).with(:flag2, scope)
expect(described_class).not_to receive(:disable)
described_class.set({ flag1: 'enabled', flag2: 'enable' }, **scope)
end
it 'disables feature flags' do
expect(described_class).to receive(:disable).with(:flag1, scope)
expect(described_class).to receive(:disable).with(:flag2, scope)
expect(described_class).not_to receive(:enable)
described_class.set({ flag1: 'disable', flag2: 'disable' }, **scope)
end
it 'enables and disables feature flags' do
expect(described_class).to receive(:enable).with(:flag1, scope)
expect(described_class).to receive(:disable).with(:flag2, scope)
described_class.set({ flag1: 'enabled', flag2: 'disabled' }, **scope)
end
end
end

59
scripts/changed-feature-flags Executable file
View file

@ -0,0 +1,59 @@
#!/usr/bin/env ruby
# frozen_string_literal: true
require 'yaml'
require 'optparse'
require_relative 'api/default_options'
# This script returns the desired feature flag state as a comma-separated string for the feature flags in the specified files.
# Each desired feature flag state is specified as 'feature-flag=state'.
#
# For example, if the specified files included `config/feature_flags/development/ci_yaml_limit_size.yml` and the desired
# state as specified by the second argument was enabled, the value returned would be `ci_yaml_limit_size=enabled`
class GetFeatureFlagsFromFiles
def initialize(options)
@files = options.delete(:files)
@state = options.delete(:state)
end
def extracted_flags
files.each_with_object([]) do |file_path, all|
next unless file_path =~ %r{/feature_flags/development/.*\.yml}
next unless File.exist?(file_path)
ff_yaml = YAML.safe_load(File.read(file_path))
ff_to_add = "#{ff_yaml['name']}"
ff_to_add += "=#{state}" unless state.to_s.empty?
all << ff_to_add
end.join(',')
end
private
attr_reader :files, :state
end
if $0 == __FILE__
options = API::DEFAULT_OPTIONS.dup
OptionParser.new do |opts|
opts.on("-f", "--files FILES", Array, "Comma-separated list of feature flag config files") do |value|
options[:files] = value
end
opts.on("-s", "--state STATE", String,
"The desired state of the feature flags (enabled or disabled). If not specified the output will only list the feature flags."
) do |value|
options[:state] = value
end
opts.on("-h", "--help", "Prints this help") do
puts opts
exit
end
end.parse!
puts GetFeatureFlagsFromFiles.new(options).extracted_flags
end

View file

@ -154,7 +154,8 @@ module Trigger
'SECURITY_SOURCES' => Trigger.security? ? 'true' : 'false',
'ee' => Trigger.ee? ? 'true' : 'false',
'QA_BRANCH' => ENV['QA_BRANCH'] || 'master',
'CACHE_UPDATE' => ENV['OMNIBUS_GITLAB_CACHE_UPDATE']
'CACHE_UPDATE' => ENV['OMNIBUS_GITLAB_CACHE_UPDATE'],
'GITLAB_QA_OPTIONS' => ENV['GITLAB_QA_OPTIONS']
}
end
end

View file

@ -0,0 +1,73 @@
# frozen_string_literal: true
require 'fast_spec_helper'
load File.expand_path('../../scripts/changed-feature-flags', __dir__)
RSpec.describe 'scripts/changed-feature-flags' do
describe GetFeatureFlagsFromFiles do
let(:ff_dir) { FileUtils.mkdir_p(File.join(Dir.tmpdir, 'feature_flags', 'development')) }
let(:feature_flag_definition1) do
file = Tempfile.new('foo.yml', ff_dir)
file.write(<<~YAML)
---
name: foo_flag
default_enabled: true
YAML
file.rewind
file
end
let(:feature_flag_definition2) do
file = Tempfile.new('bar.yml', ff_dir)
file.write(<<~YAML)
---
name: bar_flag
default_enabled: false
YAML
file.rewind
file
end
let(:feature_flag_definition_invalid_path) do
file = Tempfile.new('foobar.yml')
file.write(<<~YAML)
---
name: not a feature flag
YAML
file.rewind
file
end
after do
FileUtils.remove_entry(ff_dir, true)
end
describe '.extracted_flags' do
it 'returns feature flags' do
subject = described_class.new({ files: [feature_flag_definition1.path, feature_flag_definition2.path] })
expect(subject.extracted_flags).to eq('foo_flag,bar_flag')
end
it 'returns feature flags and their state as enabled' do
subject = described_class.new({ files: [feature_flag_definition1.path, feature_flag_definition2.path], state: 'enabled' })
expect(subject.extracted_flags).to eq('foo_flag=enabled,bar_flag=enabled')
end
it 'returns feature flags and their state as disabled' do
subject = described_class.new({ files: [feature_flag_definition1.path, feature_flag_definition2.path], state: 'disabled' })
expect(subject.extracted_flags).to eq('foo_flag=disabled,bar_flag=disabled')
end
it 'ignores files that are not in the feature_flags/development directory' do
subject = described_class.new({ files: [feature_flag_definition_invalid_path.path] })
expect(subject.extracted_flags).to eq('')
end
end
end
end

View file

@ -28,7 +28,7 @@
context 'when level is unit' do
it 'returns a pattern' do
expect(subject.pattern(:unit))
.to eq("spec/{bin,channels,config,db,dependencies,elastic,elastic_integration,experiments,factories,finders,frontend,graphql,haml_lint,helpers,initializers,javascripts,lib,models,policies,presenters,rack_servers,replicators,routing,rubocop,serializers,services,sidekiq,spam,support_specs,tasks,uploaders,validators,views,workers,tooling}{,/**/}*_spec.rb")
.to eq("spec/{bin,channels,config,db,dependencies,elastic,elastic_integration,experiments,factories,finders,frontend,graphql,haml_lint,helpers,initializers,javascripts,lib,models,policies,presenters,rack_servers,replicators,routing,rubocop,scripts,serializers,services,sidekiq,spam,support_specs,tasks,uploaders,validators,views,workers,tooling}{,/**/}*_spec.rb")
end
end
@ -110,7 +110,7 @@
context 'when level is unit' do
it 'returns a regexp' do
expect(subject.regexp(:unit))
.to eq(%r{spec/(bin|channels|config|db|dependencies|elastic|elastic_integration|experiments|factories|finders|frontend|graphql|haml_lint|helpers|initializers|javascripts|lib|models|policies|presenters|rack_servers|replicators|routing|rubocop|serializers|services|sidekiq|spam|support_specs|tasks|uploaders|validators|views|workers|tooling)})
.to eq(%r{spec/(bin|channels|config|db|dependencies|elastic|elastic_integration|experiments|factories|finders|frontend|graphql|haml_lint|helpers|initializers|javascripts|lib|models|policies|presenters|rack_servers|replicators|routing|rubocop|scripts|serializers|services|sidekiq|spam|support_specs|tasks|uploaders|validators|views|workers|tooling)})
end
end

View file

@ -40,6 +40,7 @@ class TestLevel
replicators
routing
rubocop
scripts
serializers
services
sidekiq