Commit a7386773 authored by Grzegorz Bizon's avatar Grzegorz Bizon

Merge branch 'feature/gb/ci-clarify-allowed-regexp-syntax' into 'master'

Clarify allowed regexp syntax used CI config

See merge request gitlab-org/gitlab!78671
parents ff115d40 73a5e1cf
...@@ -213,7 +213,7 @@ module Gitlab ...@@ -213,7 +213,7 @@ module Gitlab
def validate_each(record, attribute, value) def validate_each(record, attribute, value)
unless validate_regexp(value) unless validate_regexp(value)
record.errors.add(attribute, 'must be a regular expression') record.errors.add(attribute, 'must be a regular expression with re2 syntax')
end end
end end
...@@ -238,12 +238,16 @@ module Gitlab ...@@ -238,12 +238,16 @@ module Gitlab
class ArrayOfStringsOrRegexpsValidator < RegexpValidator class ArrayOfStringsOrRegexpsValidator < RegexpValidator
def validate_each(record, attribute, value) def validate_each(record, attribute, value)
unless validate_array_of_strings_or_regexps(value) unless validate_array_of_strings_or_regexps(value)
record.errors.add(attribute, 'should be an array of strings or regexps') record.errors.add(attribute, validation_message)
end end
end end
private private
def validation_message
'should be an array of strings or regular expressions using re2 syntax'
end
def validate_array_of_strings_or_regexps(values) def validate_array_of_strings_or_regexps(values)
values.is_a?(Array) && values.all?(&method(:validate_string_or_regexp)) values.is_a?(Array) && values.all?(&method(:validate_string_or_regexp))
end end
...@@ -259,6 +263,19 @@ module Gitlab ...@@ -259,6 +263,19 @@ module Gitlab
class ArrayOfStringsOrRegexpsWithFallbackValidator < ArrayOfStringsOrRegexpsValidator class ArrayOfStringsOrRegexpsWithFallbackValidator < ArrayOfStringsOrRegexpsValidator
protected protected
# TODO
#
# Remove ArrayOfStringsOrRegexpsWithFallbackValidator class too when
# you are removing the `:allow_unsafe_ruby_regexp` feature flag.
#
def validation_message
if ::Feature.enabled?(:allow_unsafe_ruby_regexp, default_enabled: :yaml)
'should be an array of strings or regular expressions'
else
super
end
end
def fallback def fallback
true true
end end
......
...@@ -36,7 +36,7 @@ module Gitlab ...@@ -36,7 +36,7 @@ module Gitlab
create_untrusted_regexp(matches[:regexp], matches[:flags]) create_untrusted_regexp(matches[:regexp], matches[:flags])
rescue RegexpError rescue RegexpError
raise unless fallback && raise unless fallback &&
Feature.enabled?(:allow_unsafe_ruby_regexp, default_enabled: false) Feature.enabled?(:allow_unsafe_ruby_regexp, default_enabled: :yaml)
if Feature.enabled?(:ci_unsafe_regexp_logger, type: :ops, default_enabled: :yaml) if Feature.enabled?(:ci_unsafe_regexp_logger, type: :ops, default_enabled: :yaml)
Gitlab::AppJsonLogger.info( Gitlab::AppJsonLogger.info(
......
...@@ -88,7 +88,7 @@ RSpec.describe Gitlab::Ci::Config::Entry::Policy do ...@@ -88,7 +88,7 @@ RSpec.describe Gitlab::Ci::Config::Entry::Policy do
describe '#errors' do describe '#errors' do
it 'saves errors' do it 'saves errors' do
expect(entry.errors) expect(entry.errors)
.to include /policy config should be an array of strings or regexps/ .to include /policy config should be an array of strings or regular expressions/
end end
end end
end end
......
...@@ -9,6 +9,10 @@ module Gitlab ...@@ -9,6 +9,10 @@ module Gitlab
subject { described_class.new(config, user: nil).execute } subject { described_class.new(config, user: nil).execute }
before do
stub_feature_flags(allow_unsafe_ruby_regexp: false)
end
shared_examples 'returns errors' do |error_message| shared_examples 'returns errors' do |error_message|
it 'adds a message when an error is encountered' do it 'adds a message when an error is encountered' do
expect(subject.errors).to include(error_message) expect(subject.errors).to include(error_message)
...@@ -609,13 +613,13 @@ module Gitlab ...@@ -609,13 +613,13 @@ module Gitlab
context 'when it is an array of integers' do context 'when it is an array of integers' do
let(:only) { [1, 1] } let(:only) { [1, 1] }
it_behaves_like 'returns errors', 'jobs:rspec:only config should be an array of strings or regexps' it_behaves_like 'returns errors', 'jobs:rspec:only config should be an array of strings or regular expressions using re2 syntax'
end end
context 'when it is invalid regex' do context 'when it is invalid regex' do
let(:only) { ["/*invalid/"] } let(:only) { ["/*invalid/"] }
it_behaves_like 'returns errors', 'jobs:rspec:only config should be an array of strings or regexps' it_behaves_like 'returns errors', 'jobs:rspec:only config should be an array of strings or regular expressions using re2 syntax'
end end
end end
...@@ -633,13 +637,13 @@ module Gitlab ...@@ -633,13 +637,13 @@ module Gitlab
context 'when it is an array of integers' do context 'when it is an array of integers' do
let(:except) { [1, 1] } let(:except) { [1, 1] }
it_behaves_like 'returns errors', 'jobs:rspec:except config should be an array of strings or regexps' it_behaves_like 'returns errors', 'jobs:rspec:except config should be an array of strings or regular expressions using re2 syntax'
end end
context 'when it is invalid regex' do context 'when it is invalid regex' do
let(:except) { ["/*invalid/"] } let(:except) { ["/*invalid/"] }
it_behaves_like 'returns errors', 'jobs:rspec:except config should be an array of strings or regexps' it_behaves_like 'returns errors', 'jobs:rspec:except config should be an array of strings or regular expressions using re2 syntax'
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