diff options
| -rw-r--r-- | lib/gitlab/ci/config/external/file/base.rb | 48 | ||||
| -rw-r--r-- | lib/gitlab/ci/config/external/file/local.rb | 24 | ||||
| -rw-r--r-- | lib/gitlab/ci/config/external/file/remote.rb | 25 | ||||
| -rw-r--r-- | lib/gitlab/ci/config/external/processor.rb | 34 | ||||
| -rw-r--r-- | spec/lib/gitlab/ci/config/external/file/local_spec.rb | 2 | ||||
| -rw-r--r-- | spec/lib/gitlab/ci/config/external/file/remote_spec.rb | 2 | ||||
| -rw-r--r-- | spec/lib/gitlab/ci/config/external/processor_spec.rb | 9 | ||||
| -rw-r--r-- | spec/lib/gitlab/ci/config_spec.rb | 2 |
8 files changed, 103 insertions, 43 deletions
diff --git a/lib/gitlab/ci/config/external/file/base.rb b/lib/gitlab/ci/config/external/file/base.rb index dc96c97b129..390db997c0b 100644 --- a/lib/gitlab/ci/config/external/file/base.rb +++ b/lib/gitlab/ci/config/external/file/base.rb @@ -6,22 +6,62 @@ module Gitlab module External module File class Base + include Gitlab::Utils::StrongMemoize + + attr_reader :location, :opts, :errors + YAML_WHITELIST_EXTENSION = /(yml|yaml)$/i.freeze def initialize(location, opts = {}) @location = location + @opts = opts + @errors = [] + + validate_location! + validate_content! + validate_hash! + end + + def invalid_extension? + !location.match(YAML_WHITELIST_EXTENSION) end def valid? - location.match(YAML_WHITELIST_EXTENSION) && content + errors.none? + end + + def error_message + errors.first end def content - raise NotImplementedError, 'content must be implemented and return a string or nil' + raise NotImplementedError, 'subclass must implement fetching raw content' end - def error_message - raise NotImplementedError, 'error_message must be implemented and return a string' + def to_hash + @hash ||= Ci::Config::Loader.new(content).load! + rescue Ci::Config::Loader::FormatError + nil + end + + protected + + def validate_location! + if invalid_extension? + errors.push("Included file `#{location}` does not have YAML extension!") + end + end + + def validate_content! + if errors.none? && content.blank? + errors.push("Included file `#{location}` is empty or does not exist!") + end + end + + def validate_hash! + if errors.none? && to_hash.blank? + errors.push("Included file `#{location}` does not have valid YAML syntax!") + end end end end diff --git a/lib/gitlab/ci/config/external/file/local.rb b/lib/gitlab/ci/config/external/file/local.rb index 8c0ffd36449..acf4201a672 100644 --- a/lib/gitlab/ci/config/external/file/local.rb +++ b/lib/gitlab/ci/config/external/file/local.rb @@ -6,25 +6,33 @@ module Gitlab module External module File class Local < Base - attr_reader :location, :project, :sha + include Gitlab::Utils::StrongMemoize - def initialize(location, opts = {}) - super + attr_reader :project, :sha + def initialize(location, opts = {}) @project = opts.fetch(:project) @sha = opts.fetch(:sha) - end - def content - @content ||= fetch_local_content + super end - def error_message - "Local file '#{location}' is not valid." + def content + strong_memoize(:content) { fetch_local_content } end private + def validate_content! + return if errors.any? + + if content.nil? + errors.push("Local file `#{location}` does not exist!") + elsif content.blank? + errors.push("Local file `#{location}` is empty!") + end + end + def fetch_local_content project.repository.blob_data_at(sha, location) end diff --git a/lib/gitlab/ci/config/external/file/remote.rb b/lib/gitlab/ci/config/external/file/remote.rb index 291fe87849c..b1ba37a3959 100644 --- a/lib/gitlab/ci/config/external/file/remote.rb +++ b/lib/gitlab/ci/config/external/file/remote.rb @@ -7,21 +7,34 @@ module Gitlab module File class Remote < Base include Gitlab::Utils::StrongMemoize - attr_reader :location def content strong_memoize(:content) { fetch_remote_content } end - def error_message - "Remote file '#{location}' is not valid." - end - private + def validate_location! + super + + unless ::Gitlab::UrlSanitizer.valid?(location) + errors.push("Remote file `#{location}` does not have a valid address!") + end + end + def fetch_remote_content Gitlab::HTTP.get(location) - rescue Gitlab::HTTP::Error, Timeout::Error, SocketError, Gitlab::HTTP::BlockedUrlError + rescue SocketError + errors.push("Remote file `#{location}` could not be fetched because of a socket error!") + nil + rescue Timeout::Error + errors.push("Remote file `#{location}` could not be fetched because of a timeout error!") + nil + rescue Gitlab::HTTP::Error + errors.push("Remote file `#{location}` could not be fetched because of a HTTP error!") + nil + rescue Gitlab::HTTP::BlockedUrlError + errors.push("Remote file `#{location}` could not be fetched because the URL is blocked!") nil end end diff --git a/lib/gitlab/ci/config/external/processor.rb b/lib/gitlab/ci/config/external/processor.rb index 4f07494925a..eae0bdeb644 100644 --- a/lib/gitlab/ci/config/external/processor.rb +++ b/lib/gitlab/ci/config/external/processor.rb @@ -14,38 +14,34 @@ module Gitlab end def perform - return values if external_files.empty? + return @values if @external_files.empty? - external_files.each do |external_file| - validate_external_file(external_file) - @content.deep_merge!(content_of(external_file)) - end - - append_inline_content - remove_include_keyword + validate_external_files! + merge_external_files! + append_inline_content! + remove_include_keyword! end private - attr_reader :values, :external_files, :content - - def validate_external_file(external_file) - unless external_file.valid? - raise IncludeError, external_file.error_message + def validate_external_files! + @external_files.each do |file| + raise IncludeError, file.error_message unless file.valid? end end - def content_of(external_file) - Config::Loader.new(external_file.content).load! + def merge_external_files! + @external_files.each do |file| + @content.deep_merge!(file.to_hash) + end end - def append_inline_content + def append_inline_content! @content.deep_merge!(@values) end - def remove_include_keyword - content.delete(:include) - content + def remove_include_keyword! + @content.tap { @content.delete(:include) } end end end diff --git a/spec/lib/gitlab/ci/config/external/file/local_spec.rb b/spec/lib/gitlab/ci/config/external/file/local_spec.rb index 5c0e2eee71f..2708d8d5b6b 100644 --- a/spec/lib/gitlab/ci/config/external/file/local_spec.rb +++ b/spec/lib/gitlab/ci/config/external/file/local_spec.rb @@ -72,7 +72,7 @@ describe Gitlab::Ci::Config::External::File::Local do let(:location) { '/lib/gitlab/ci/templates/non-existent-file.yml' } it 'should return an error message' do - expect(local_file.error_message).to eq("Local file '#{location}' is not valid.") + expect(local_file.error_message).to eq("Local file `#{location}` does not exist!") end end end diff --git a/spec/lib/gitlab/ci/config/external/file/remote_spec.rb b/spec/lib/gitlab/ci/config/external/file/remote_spec.rb index 72853797bcd..e4682852014 100644 --- a/spec/lib/gitlab/ci/config/external/file/remote_spec.rb +++ b/spec/lib/gitlab/ci/config/external/file/remote_spec.rb @@ -108,7 +108,7 @@ describe Gitlab::Ci::Config::External::File::Remote do let(:location) { 'not-valid://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml' } it 'should return an error message' do - expect(remote_file.error_message).to eq("Remote file '#{location}' is not valid.") + expect(remote_file.error_message).to eq("Remote file `#{location}` does not have a valid address!") end end end diff --git a/spec/lib/gitlab/ci/config/external/processor_spec.rb b/spec/lib/gitlab/ci/config/external/processor_spec.rb index ede6e769cfe..1a05f716247 100644 --- a/spec/lib/gitlab/ci/config/external/processor_spec.rb +++ b/spec/lib/gitlab/ci/config/external/processor_spec.rb @@ -21,7 +21,7 @@ describe Gitlab::Ci::Config::External::Processor do it 'should raise an error' do expect { processor.perform }.to raise_error( described_class::IncludeError, - "Local file '/lib/gitlab/ci/templates/non-existent-file.yml' is not valid." + "Local file `/lib/gitlab/ci/templates/non-existent-file.yml` does not exist!" ) end end @@ -37,7 +37,7 @@ describe Gitlab::Ci::Config::External::Processor do it 'should raise an error' do expect { processor.perform }.to raise_error( described_class::IncludeError, - "Remote file '#{remote_file}' is not valid." + "Remote file `#{remote_file}` could not be fetched because of a socket error!" ) end end @@ -159,7 +159,10 @@ describe Gitlab::Ci::Config::External::Processor do end it 'should raise an error' do - expect { processor.perform }.to raise_error(Gitlab::Ci::Config::Loader::FormatError) + expect { processor.perform }.to raise_error( + described_class::IncludeError, + "Included file `/lib/gitlab/ci/templates/template.yml` does not have valid YAML syntax!" + ) end end diff --git a/spec/lib/gitlab/ci/config_spec.rb b/spec/lib/gitlab/ci/config_spec.rb index 8925b17be58..975e11e8cc1 100644 --- a/spec/lib/gitlab/ci/config_spec.rb +++ b/spec/lib/gitlab/ci/config_spec.rb @@ -201,7 +201,7 @@ describe Gitlab::Ci::Config do it 'raises error YamlProcessor validationError' do expect { config }.to raise_error( described_class::ConfigError, - "Local file 'invalid' is not valid." + "Included file `invalid` does not have YAML extension!" ) end end |