diff options
author | Grzegorz Bizon <grzesiek.bizon@gmail.com> | 2018-10-18 14:13:26 +0200 |
---|---|---|
committer | Grzegorz Bizon <grzesiek.bizon@gmail.com> | 2018-10-18 14:13:26 +0200 |
commit | 39dac14e9caad660487eb106428369663767d127 (patch) | |
tree | a38ba54cf140b7d4df3d29abb9ccbf8eeee66cb9 /lib/gitlab/ci | |
parent | e0830a748a9efe817bcc48f7d0bf7fc5282f58ea (diff) | |
download | gitlab-ce-39dac14e9caad660487eb106428369663767d127.tar.gz |
Refactor `include` code and improve error reporting
Diffstat (limited to 'lib/gitlab/ci')
-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 |
4 files changed, 94 insertions, 37 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 |