diff options
author | Douwe Maan <douwe@gitlab.com> | 2018-10-22 23:11:08 +0300 |
---|---|---|
committer | Douwe Maan <douwe@gitlab.com> | 2018-10-22 23:11:08 +0300 |
commit | b887d106cc3aaf00ac2e7af5b838009d1c9d6df3 (patch) | |
tree | a18a51cf51bbea78c6636f15a9e238820fd7cd3f /lib/gitlab/ci | |
parent | ec748a8f195cb206ad78962a99ad91f81b70b9bb (diff) | |
parent | a91899be10aa0808171185ba5377d243e4716efe (diff) |
Merge branch 'feature/gb/improve-include-config-errors-reporting' into 'master'
Improve validation errors for external CI/CD configuration
Closes #51369
See merge request gitlab-org/gitlab-ce!22394
Diffstat (limited to 'lib/gitlab/ci')
-rw-r--r-- | lib/gitlab/ci/config.rb | 8 | ||||
-rw-r--r-- | lib/gitlab/ci/config/external/file/base.rb | 75 | ||||
-rw-r--r-- | lib/gitlab/ci/config/external/file/local.rb | 42 | ||||
-rw-r--r-- | lib/gitlab/ci/config/external/file/remote.rb | 49 | ||||
-rw-r--r-- | lib/gitlab/ci/config/external/mapper.rb | 33 | ||||
-rw-r--r-- | lib/gitlab/ci/config/external/processor.rb | 50 | ||||
-rw-r--r-- | lib/gitlab/ci/external/file/base.rb | 29 | ||||
-rw-r--r-- | lib/gitlab/ci/external/file/local.rb | 34 | ||||
-rw-r--r-- | lib/gitlab/ci/external/file/remote.rb | 30 | ||||
-rw-r--r-- | lib/gitlab/ci/external/mapper.rb | 32 | ||||
-rw-r--r-- | lib/gitlab/ci/external/processor.rb | 52 |
11 files changed, 253 insertions, 181 deletions
diff --git a/lib/gitlab/ci/config.rb b/lib/gitlab/ci/config.rb index fe98d25af29..fedaf18ef30 100644 --- a/lib/gitlab/ci/config.rb +++ b/lib/gitlab/ci/config.rb @@ -13,10 +13,10 @@ module Gitlab @global = Entry::Global.new(@config) @global.compose! - rescue Loader::FormatError, Extendable::ExtensionError => e + rescue Loader::FormatError, + Extendable::ExtensionError, + External::Processor::IncludeError => e raise Config::ConfigError, e.message - rescue ::Gitlab::Ci::External::Processor::FileError => e - raise ::Gitlab::Ci::YamlProcessor::ValidationError, e.message end def valid? @@ -81,7 +81,7 @@ module Gitlab def process_external_files(config, project, opts) sha = opts.fetch(:sha) { project.repository.root_ref_sha } - ::Gitlab::Ci::External::Processor.new(config, project, sha).perform + Config::External::Processor.new(config, project, sha).perform end end end diff --git a/lib/gitlab/ci/config/external/file/base.rb b/lib/gitlab/ci/config/external/file/base.rb new file mode 100644 index 00000000000..15ca47ef60e --- /dev/null +++ b/lib/gitlab/ci/config/external/file/base.rb @@ -0,0 +1,75 @@ +# frozen_string_literal: true + +module Gitlab + module Ci + class Config + 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! + end + + def invalid_extension? + !::File.basename(location).match(YAML_WHITELIST_EXTENSION) + end + + def valid? + errors.none? + end + + def error_message + errors.first + end + + def content + raise NotImplementedError, 'subclass must implement fetching raw content' + end + + def to_hash + @hash ||= Ci::Config::Loader.new(content).load! + rescue Ci::Config::Loader::FormatError + nil + end + + protected + + def validate! + validate_location! + validate_content! if errors.none? + validate_hash! if errors.none? + end + + def validate_location! + if invalid_extension? + errors.push("Included file `#{location}` does not have YAML extension!") + end + end + + def validate_content! + if content.blank? + errors.push("Included file `#{location}` is empty or does not exist!") + end + end + + def validate_hash! + if to_hash.blank? + errors.push("Included file `#{location}` does not have valid YAML syntax!") + end + end + end + end + end + end + end +end diff --git a/lib/gitlab/ci/config/external/file/local.rb b/lib/gitlab/ci/config/external/file/local.rb new file mode 100644 index 00000000000..2a256aff65c --- /dev/null +++ b/lib/gitlab/ci/config/external/file/local.rb @@ -0,0 +1,42 @@ +# frozen_string_literal: true + +module Gitlab + module Ci + class Config + module External + module File + class Local < Base + include Gitlab::Utils::StrongMemoize + + attr_reader :project, :sha + + def initialize(location, opts = {}) + @project = opts.fetch(:project) + @sha = opts.fetch(:sha) + + super + end + + def content + strong_memoize(:content) { fetch_local_content } + end + + private + + def validate_content! + 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 + end + end + end + end + end +end diff --git a/lib/gitlab/ci/config/external/file/remote.rb b/lib/gitlab/ci/config/external/file/remote.rb new file mode 100644 index 00000000000..86fa5ad8800 --- /dev/null +++ b/lib/gitlab/ci/config/external/file/remote.rb @@ -0,0 +1,49 @@ +# frozen_string_literal: true + +module Gitlab + module Ci + class Config + module External + module File + class Remote < Base + include Gitlab::Utils::StrongMemoize + + def content + strong_memoize(:content) { fetch_remote_content } + 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 + begin + response = Gitlab::HTTP.get(location) + rescue SocketError + errors.push("Remote file `#{location}` could not be fetched because of a socket error!") + rescue Timeout::Error + errors.push("Remote file `#{location}` could not be fetched because of a timeout error!") + rescue Gitlab::HTTP::Error + errors.push("Remote file `#{location}` could not be fetched because of HTTP error!") + rescue Gitlab::HTTP::BlockedUrlError => e + errors.push("Remote file could not be fetched because #{e}!") + end + + if response&.code.to_i >= 400 + errors.push("Remote file `#{location}` could not be fetched because of HTTP code `#{response.code}` error!") + end + + response.to_s if errors.none? + end + end + end + end + end + end +end diff --git a/lib/gitlab/ci/config/external/mapper.rb b/lib/gitlab/ci/config/external/mapper.rb new file mode 100644 index 00000000000..def3563e505 --- /dev/null +++ b/lib/gitlab/ci/config/external/mapper.rb @@ -0,0 +1,33 @@ +# frozen_string_literal: true + +module Gitlab + module Ci + class Config + module External + class Mapper + def initialize(values, project, sha) + @locations = Array(values.fetch(:include, [])) + @project = project + @sha = sha + end + + def process + locations.map { |location| build_external_file(location) } + end + + private + + attr_reader :locations, :project, :sha + + def build_external_file(location) + if ::Gitlab::UrlSanitizer.valid?(location) + External::File::Remote.new(location) + else + External::File::Local.new(location, project: project, sha: sha) + end + end + end + end + end + end +end diff --git a/lib/gitlab/ci/config/external/processor.rb b/lib/gitlab/ci/config/external/processor.rb new file mode 100644 index 00000000000..eae0bdeb644 --- /dev/null +++ b/lib/gitlab/ci/config/external/processor.rb @@ -0,0 +1,50 @@ +# frozen_string_literal: true + +module Gitlab + module Ci + class Config + module External + class Processor + IncludeError = Class.new(StandardError) + + def initialize(values, project, sha) + @values = values + @external_files = External::Mapper.new(values, project, sha).process + @content = {} + end + + def perform + return @values if @external_files.empty? + + validate_external_files! + merge_external_files! + append_inline_content! + remove_include_keyword! + end + + private + + def validate_external_files! + @external_files.each do |file| + raise IncludeError, file.error_message unless file.valid? + end + end + + def merge_external_files! + @external_files.each do |file| + @content.deep_merge!(file.to_hash) + end + end + + def append_inline_content! + @content.deep_merge!(@values) + end + + def remove_include_keyword! + @content.tap { @content.delete(:include) } + end + end + end + end + end +end diff --git a/lib/gitlab/ci/external/file/base.rb b/lib/gitlab/ci/external/file/base.rb deleted file mode 100644 index f4da07b0b02..00000000000 --- a/lib/gitlab/ci/external/file/base.rb +++ /dev/null @@ -1,29 +0,0 @@ -# frozen_string_literal: true - -module Gitlab - module Ci - module External - module File - class Base - YAML_WHITELIST_EXTENSION = /(yml|yaml)$/i.freeze - - def initialize(location, opts = {}) - @location = location - end - - def valid? - location.match(YAML_WHITELIST_EXTENSION) && content - end - - def content - raise NotImplementedError, 'content must be implemented and return a string or nil' - end - - def error_message - raise NotImplementedError, 'error_message must be implemented and return a string' - end - end - end - end - end -end diff --git a/lib/gitlab/ci/external/file/local.rb b/lib/gitlab/ci/external/file/local.rb deleted file mode 100644 index 1aa7f687507..00000000000 --- a/lib/gitlab/ci/external/file/local.rb +++ /dev/null @@ -1,34 +0,0 @@ -# frozen_string_literal: true - -module Gitlab - module Ci - module External - module File - class Local < Base - attr_reader :location, :project, :sha - - def initialize(location, opts = {}) - super - - @project = opts.fetch(:project) - @sha = opts.fetch(:sha) - end - - def content - @content ||= fetch_local_content - end - - def error_message - "Local file '#{location}' is not valid." - end - - private - - def fetch_local_content - project.repository.blob_data_at(sha, location) - end - end - end - end - end -end diff --git a/lib/gitlab/ci/external/file/remote.rb b/lib/gitlab/ci/external/file/remote.rb deleted file mode 100644 index 59bb3e8999e..00000000000 --- a/lib/gitlab/ci/external/file/remote.rb +++ /dev/null @@ -1,30 +0,0 @@ -# frozen_string_literal: true - -module Gitlab - module Ci - module External - module File - class Remote < Base - include Gitlab::Utils::StrongMemoize - attr_reader :location - - def content - return @content if defined?(@content) - - @content = strong_memoize(:content) do - begin - Gitlab::HTTP.get(location) - rescue Gitlab::HTTP::Error, Timeout::Error, SocketError, Gitlab::HTTP::BlockedUrlError - nil - end - end - end - - def error_message - "Remote file '#{location}' is not valid." - end - end - end - end - end -end diff --git a/lib/gitlab/ci/external/mapper.rb b/lib/gitlab/ci/external/mapper.rb deleted file mode 100644 index 58bd6a19acf..00000000000 --- a/lib/gitlab/ci/external/mapper.rb +++ /dev/null @@ -1,32 +0,0 @@ -# frozen_string_literal: true - -module Gitlab - module Ci - module External - class Mapper - def initialize(values, project, sha) - @locations = Array(values.fetch(:include, [])) - @project = project - @sha = sha - end - - def process - locations.map { |location| build_external_file(location) } - end - - private - - attr_reader :locations, :project, :sha - - def build_external_file(location) - if ::Gitlab::UrlSanitizer.valid?(location) - Gitlab::Ci::External::File::Remote.new(location) - else - options = { project: project, sha: sha } - Gitlab::Ci::External::File::Local.new(location, options) - end - end - end - end - end -end diff --git a/lib/gitlab/ci/external/processor.rb b/lib/gitlab/ci/external/processor.rb deleted file mode 100644 index 76cf3ce89f9..00000000000 --- a/lib/gitlab/ci/external/processor.rb +++ /dev/null @@ -1,52 +0,0 @@ -# frozen_string_literal: true - -module Gitlab - module Ci - module External - class Processor - FileError = Class.new(StandardError) - - def initialize(values, project, sha) - @values = values - @external_files = Gitlab::Ci::External::Mapper.new(values, project, sha).process - @content = {} - end - - def perform - 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 - end - - private - - attr_reader :values, :external_files, :content - - def validate_external_file(external_file) - unless external_file.valid? - raise FileError, external_file.error_message - end - end - - def content_of(external_file) - Gitlab::Ci::Config::Loader.new(external_file.content).load! - end - - def append_inline_content - @content.deep_merge!(@values) - end - - def remove_include_keyword - content.delete(:include) - content - end - end - end - end -end |