Commit f6324606 authored by drew cimino's avatar drew cimino

File-specific exclusions for CI templates

Splitting by category isn't sufficient anymore since
Verify/Acccessbility is Core and Verify/Browser-Performance is Premium.
We brought the Verify/ category as a whole out of EE, and specifically
excluded BrowserPerformance for the time being.
parent 74fa9819
...@@ -14,12 +14,16 @@ module EE ...@@ -14,12 +14,16 @@ module EE
super.merge(categories_ee) super.merge(categories_ee)
end end
override :disabled_templates
def disabled_templates
[]
end
private private
def categories_ee def categories_ee
{ {
'Security' => 'Security', 'Security' => 'Security'
'Verify' => 'Verify'
} }
end end
end end
......
...@@ -14,7 +14,7 @@ describe Gitlab::Template::GitlabCiYmlTemplate do ...@@ -14,7 +14,7 @@ describe Gitlab::Template::GitlabCiYmlTemplate do
expect(templates).to include('SAST') expect(templates).to include('SAST')
end end
it 'finds the Verify templates' do it 'finds all the Verify templates' do
expect(templates).to include('Browser-Performance') expect(templates).to include('Browser-Performance')
expect(templates).to include('Accessibility') expect(templates).to include('Accessibility')
end end
......
...@@ -5,9 +5,11 @@ module Gitlab ...@@ -5,9 +5,11 @@ module Gitlab
module Template module Template
module Finders module Finders
class GlobalTemplateFinder < BaseTemplateFinder class GlobalTemplateFinder < BaseTemplateFinder
def initialize(base_dir, extension, categories = {}) def initialize(base_dir, extension, categories = {}, exclusions = [])
@categories = categories @categories = categories
@extension = extension @extension = extension
@exclusions = exclusions
super(base_dir) super(base_dir)
end end
...@@ -16,6 +18,8 @@ module Gitlab ...@@ -16,6 +18,8 @@ module Gitlab
end end
def find(key) def find(key)
return if excluded?(key)
file_name = "#{key}#{@extension}" file_name = "#{key}#{@extension}"
# The key is untrusted input, so ensure we can't be directed outside # The key is untrusted input, so ensure we can't be directed outside
...@@ -28,11 +32,20 @@ module Gitlab ...@@ -28,11 +32,20 @@ module Gitlab
def list_files_for(dir) def list_files_for(dir)
dir = "#{dir}/" unless dir.end_with?('/') dir = "#{dir}/" unless dir.end_with?('/')
Dir.glob(File.join(dir, "*#{@extension}")).select { |f| f =~ self.class.filter_regex(@extension) }
Dir.glob(File.join(dir, "*#{@extension}")).select do |f|
next if excluded?(f)
f =~ self.class.filter_regex(@extension)
end
end end
private private
def excluded?(file_name)
@exclusions.include?(file_name)
end
def select_directory(file_name) def select_directory(file_name)
@categories.keys.find do |category| @categories.keys.find do |category|
File.exist?(File.join(category_directory(category), file_name)) File.exist?(File.join(category_directory(category), file_name))
......
...@@ -17,16 +17,25 @@ module Gitlab ...@@ -17,16 +17,25 @@ module Gitlab
{ {
'General' => '', 'General' => '',
'Pages' => 'Pages', 'Pages' => 'Pages',
'Verify' => 'Verify',
'Auto deploy' => 'autodeploy' 'Auto deploy' => 'autodeploy'
} }
end end
def disabled_templates
%w[
Verify/Browser-Performance
]
end
def base_dir def base_dir
Rails.root.join('lib/gitlab/ci/templates') Rails.root.join('lib/gitlab/ci/templates')
end end
def finder(project = nil) def finder(project = nil)
Gitlab::Template::Finders::GlobalTemplateFinder.new(self.base_dir, self.extension, self.categories) Gitlab::Template::Finders::GlobalTemplateFinder.new(
self.base_dir, self.extension, self.categories, self.disabled_templates
)
end end
end end
end end
......
...@@ -15,23 +15,85 @@ describe Gitlab::Template::Finders::GlobalTemplateFinder do ...@@ -15,23 +15,85 @@ describe Gitlab::Template::Finders::GlobalTemplateFinder do
FileUtils.rm_rf(base_dir) FileUtils.rm_rf(base_dir)
end end
subject(:finder) { described_class.new(base_dir, '', 'Foo' => '', 'Bar' => 'bar') } subject(:finder) { described_class.new(base_dir, '', { 'General' => '', 'Bar' => 'Bar' }, exclusions) }
let(:exclusions) { [] }
describe '.find' do describe '.find' do
it 'finds a template in the Foo category' do context 'with a non-prefixed General template' do
before do
create_template!('test-template') create_template!('test-template')
end
it 'finds the template with no prefix' do
expect(finder.find('test-template')).to be_present expect(finder.find('test-template')).to be_present
end end
it 'finds a template in the Bar category' do it 'it does not find a prefixed template' do
create_template!('bar/test-template') expect(finder.find('Bar/test-template')).to be_nil
end
expect(finder.find('test-template')).to be_present it 'does not permit path traversal requests' do
expect { finder.find('../foo') }.to raise_error(/Invalid path/)
end
context 'while listed as an exclusion' do
let(:exclusions) { %w[test-template] }
it 'does not find the template without a prefix' do
expect(finder.find('test-template')).to be_nil
end
it 'does not find the template with a prefix' do
expect(finder.find('Bar/test-template')).to be_nil
end
it 'finds another prefixed template with the same name' do
create_template!('Bar/test-template')
expect(finder.find('test-template')).to be_nil
expect(finder.find('Bar/test-template')).to be_present
end
end
end
context 'with a prefixed template' do
before do
create_template!('Bar/test-template')
end
it 'finds the template with a prefix' do
expect(finder.find('Bar/test-template')).to be_present
end
# NOTE: This spec fails, the template Bar/test-template is found
xit 'it does not find the template without a prefix' do
expect(finder.find('test-template')).to be_nil
end end
it 'does not permit path traversal requests' do it 'does not permit path traversal requests' do
expect { finder.find('../foo') }.to raise_error(/Invalid path/) expect { finder.find('../foo') }.to raise_error(/Invalid path/)
end end
context 'while listed as an exclusion' do
let(:exclusions) { %w[Bar/test-template] }
it 'does not find the template with a prefix' do
expect(finder.find('Bar/test-template')).to be_nil
end
# NOTE: This spec fails, the template Bar/test-template is found
xit 'does not find the template without a prefix' do
expect(finder.find('test-template')).to be_nil
end
it 'finds another non-prefixed template with the same name' do
create_template!('Bar/test-template')
expect(finder.find('test-template')).to be_present
expect(finder.find('Bar/test-template')).to be_nil
end
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