diff --git a/config/feature_flags/gitlab_com_derisk/ci_text_interpolation.yml b/config/feature_flags/gitlab_com_derisk/ci_text_interpolation.yml deleted file mode 100644 index 32dd15e16b04a58088deeb068493d5055b7b9237..0000000000000000000000000000000000000000 --- a/config/feature_flags/gitlab_com_derisk/ci_text_interpolation.yml +++ /dev/null @@ -1,9 +0,0 @@ ---- -name: ci_text_interpolation -feature_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/433002 -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/142009 -rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/435177 -milestone: '16.9' -group: group::pipeline authoring -type: gitlab_com_derisk -default_enabled: false diff --git a/lib/gitlab/ci/config/interpolation/text_interpolator.rb b/lib/gitlab/ci/config/interpolation/text_interpolator.rb deleted file mode 100644 index 773defbfa37b524ce7dbb96c7ffb528486e5c003..0000000000000000000000000000000000000000 --- a/lib/gitlab/ci/config/interpolation/text_interpolator.rb +++ /dev/null @@ -1,100 +0,0 @@ -# frozen_string_literal: true - -module Gitlab - module Ci - class Config - module Interpolation - ## - # Performs CI config file interpolation and either returns the interpolated result or interpolation errors. - # - class TextInterpolator - attr_reader :errors - - def initialize(yaml_documents, input_args, variables) - @yaml_documents = yaml_documents - @input_args = input_args.to_h - @variables = variables - @errors = [] - @interpolated = false - end - - def valid? - errors.none? - end - - def to_result - @result - end - - def error_message - # Interpolator can have multiple error messages, like: ["interpolation interrupted by errors", "unknown - # interpolation key: `abc`"] ? - # - # We are joining them together into a single one, because only one error can be surfaced when an external - # file gets included and is invalid. The limit to three error messages combined is more than required. - # - errors.first(3).join(', ') - end - - def interpolate! - if inputs_without_header? - return errors.push( - _('Given inputs not defined in the `spec` section of the included configuration file')) - end - - return @result ||= yaml_documents.content unless yaml_documents.header - - return errors.concat(header.errors) unless header.valid? - return errors.concat(inputs.errors) unless inputs.valid? - return errors.concat(context.errors) unless context.valid? - return errors.concat(template.errors) unless template.valid? - - @interpolated = true - - @result ||= template.interpolated - end - - def interpolated? - @interpolated - end - - private - - attr_reader :yaml_documents, :input_args, :variables - - def inputs_without_header? - input_args.any? && !yaml_documents.header - end - - def header - @header ||= Header::Root.new(yaml_documents.header).tap do |header| - header.key = 'header' - - header.compose! - end - end - - def content - @content ||= yaml_documents.content - end - - def spec - @spec ||= header.inputs_value - end - - def inputs - @inputs ||= Inputs.new(spec, input_args) - end - - def context - @context ||= Context.new({ inputs: inputs.to_hash }, variables: variables) - end - - def template - @template ||= TextTemplate.new(content, context) - end - end - end - end - end -end diff --git a/lib/gitlab/ci/config/interpolation/text_template.rb b/lib/gitlab/ci/config/interpolation/text_template.rb deleted file mode 100644 index e1f5d368e881ebaca279bd6ef3ff59286a2290cf..0000000000000000000000000000000000000000 --- a/lib/gitlab/ci/config/interpolation/text_template.rb +++ /dev/null @@ -1,59 +0,0 @@ -# frozen_string_literal: true - -module Gitlab - module Ci - class Config - module Interpolation - class TextTemplate - MAX_BLOCKS = 10_000 - - def initialize(content, ctx) - @content = content - @ctx = Interpolation::Context.fabricate(ctx) - @errors = [] - @blocks = {} - - interpolate! if valid? - end - - def valid? - errors.none? - end - - def errors - @errors + ctx.errors + blocks.values.flat_map(&:errors) - end - - def interpolated - @result if valid? - end - - private - - attr_reader :blocks, :content, :ctx - - def interpolate! - return @errors.push('config too large') if content.bytesize > max_total_yaml_size_bytes - - @result = Interpolation::Block.match(content) do |matched, data| - block = (blocks[matched] ||= Interpolation::Block.new(matched, data, ctx)) - - break @errors.push('too many interpolation blocks') if blocks.count > MAX_BLOCKS - break unless block.valid? - - if block.value.is_a?(String) - block.value - else - block.value.to_json - end - end - end - - def max_total_yaml_size_bytes - Gitlab::CurrentSettings.current_application_settings.ci_max_total_yaml_size_bytes - end - end - end - end - end -end diff --git a/lib/gitlab/ci/config/yaml/loader.rb b/lib/gitlab/ci/config/yaml/loader.rb index bf20cd9c027b0f0f9cf3668447950e65ac3aa891..0168def5f127fb74c998e066b028142cd3412b94 100644 --- a/lib/gitlab/ci/config/yaml/loader.rb +++ b/lib/gitlab/ci/config/yaml/loader.rb @@ -17,18 +17,16 @@ def initialize(content, inputs: {}, variables: []) end def load - if Feature.disabled?(:ci_text_interpolation, Feature.current_request, type: :gitlab_com_derisk) - return legacy_load - end + yaml_result = load_uninterpolated_yaml + + return yaml_result unless yaml_result.valid? - interpolator = Interpolation::TextInterpolator.new(yaml_documents, inputs, variables) + interpolator = Interpolation::Interpolator.new(yaml_result, inputs, variables) interpolator.interpolate! if interpolator.valid? - loaded_yaml = yaml(interpolator.to_result).load! - - Yaml::Result.new(config: loaded_yaml, error: nil, interpolated: interpolator.interpolated?) + Yaml::Result.new(config: interpolator.to_hash, error: nil, interpolated: interpolator.interpolated?) else Yaml::Result.new(error: interpolator.error_message, interpolated: interpolator.interpolated?) end @@ -46,38 +44,6 @@ def load_uninterpolated_yaml attr_reader :content, :inputs, :variables - def yaml(content) - ensure_custom_tags - - ::Gitlab::Config::Loader::Yaml.new(content, additional_permitted_classes: AVAILABLE_TAGS) - end - - def yaml_documents - docs = content - .split(::Gitlab::Config::Loader::MultiDocYaml::MULTI_DOC_DIVIDER, MAX_DOCUMENTS + 1) - .map { |d| yaml(d) } - - docs.reject!(&:blank?) - - Yaml::Documents.new(docs) - end - - def legacy_load - yaml_result = load_uninterpolated_yaml - - return yaml_result unless yaml_result.valid? - - interpolator = Interpolation::Interpolator.new(yaml_result, inputs, variables) - - interpolator.interpolate! - - if interpolator.valid? - Yaml::Result.new(config: interpolator.to_hash, error: nil, interpolated: interpolator.interpolated?) - else - Yaml::Result.new(error: interpolator.error_message, interpolated: interpolator.interpolated?) - end - end - def load_yaml! ensure_custom_tags diff --git a/spec/lib/gitlab/ci/config/interpolation/text_interpolator_spec.rb b/spec/lib/gitlab/ci/config/interpolation/text_interpolator_spec.rb deleted file mode 100644 index 20e7e3d0d2641746f01b3a7bbf542da95498dd82..0000000000000000000000000000000000000000 --- a/spec/lib/gitlab/ci/config/interpolation/text_interpolator_spec.rb +++ /dev/null @@ -1,139 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Gitlab::Ci::Config::Interpolation::TextInterpolator, feature_category: :pipeline_composition, quarantine: 'https://gitlab.com/gitlab-org/gitlab/-/issues/440667' do - let(:arguments) { { website: 'gitlab.com' } } - let(:content) { ::Gitlab::Config::Loader::Yaml.new("test: 'deploy $[[ inputs.website ]]'") } - let(:header) { ::Gitlab::Config::Loader::Yaml.new("spec:\n inputs:\n website: ") } - let(:documents) { ::Gitlab::Ci::Config::Yaml::Documents.new([header, content]) } - - subject(:interpolator) { described_class.new(documents, arguments, []) } - - context 'when input data is valid' do - it 'correctly interpolates the config' do - interpolator.interpolate! - - expect(interpolator).to be_interpolated - expect(interpolator).to be_valid - expect(interpolator.to_result).to eq("test: 'deploy gitlab.com'") - end - end - - context 'when interpolation is not used' do - let(:arguments) { nil } - let(:content) { ::Gitlab::Config::Loader::Yaml.new("test: 'deploy production'") } - let(:documents) { ::Gitlab::Ci::Config::Yaml::Documents.new([content]) } - - it 'returns original content' do - interpolator.interpolate! - - expect(interpolator).not_to be_interpolated - expect(interpolator).to be_valid - expect(interpolator.to_result).to eq("test: 'deploy production'") - end - end - - context 'when spec header is missing but inputs are specified' do - let(:documents) { ::Gitlab::Ci::Config::Yaml::Documents.new([content]) } - - it 'surfaces an error about invalid inputs' do - interpolator.interpolate! - - expect(interpolator).not_to be_valid - expect(interpolator.error_message).to eq( - 'Given inputs not defined in the `spec` section of the included configuration file' - ) - end - end - - context 'when spec header is invalid' do - let(:header) { ::Gitlab::Config::Loader::Yaml.new("spec:\n arguments:\n website:") } - - it 'surfaces an error about invalid header' do - interpolator.interpolate! - - expect(interpolator).not_to be_valid - expect(interpolator.error_message).to eq('header:spec config contains unknown keys: arguments') - end - end - - context 'when provided interpolation argument is invalid' do - let(:arguments) { { website: ['gitlab.com'] } } - - it 'returns an error about the invalid argument' do - interpolator.interpolate! - - expect(interpolator).not_to be_valid - expect(interpolator.error_message).to eq('`website` input: provided value is not a string') - end - end - - context 'when interpolation block is invalid' do - let(:content) { ::Gitlab::Config::Loader::Yaml.new("test: 'deploy $[[ inputs.abc ]]'") } - - it 'returns an error about the invalid block' do - interpolator.interpolate! - - expect(interpolator).not_to be_valid - expect(interpolator.error_message).to eq('unknown interpolation key: `abc`') - end - end - - context 'when multiple interpolation blocks are invalid' do - let(:content) do - ::Gitlab::Config::Loader::Yaml.new("test: 'deploy $[[ inputs.something.abc ]] $[[ inputs.cde ]] $[[ efg ]]'") - end - - it 'stops execution after the first invalid block' do - interpolator.interpolate! - - expect(interpolator).not_to be_valid - expect(interpolator.error_message).to eq('unknown interpolation key: `something`') - end - end - - context 'when there are many invalid arguments' do - let(:header) do - ::Gitlab::Config::Loader::Yaml.new( - <<~YAML - spec: - inputs: - allow_failure: - type: boolean - image: - parallel: - type: number - website: - YAML - ) - end - - let(:content) do - ::Gitlab::Config::Loader::Yaml.new( - "test: 'deploy $[[ inputs.website ]] $[[ inputs.parallel ]] $[[ inputs.allow_failure ]] $[[ inputs.image ]]'" - ) - end - - let(:arguments) do - { allow_failure: 'no', parallel: 'yes', website: 8 } - end - - it 'reports a maximum of 3 errors in the error message' do - interpolator.interpolate! - - expect(interpolator).not_to be_valid - expect(interpolator.error_message).to eq( - '`allow_failure` input: provided value is not a boolean, ' \ - '`image` input: required value has not been provided, ' \ - '`parallel` input: provided value is not a number' - ) - expect(interpolator.errors).to contain_exactly( - '`allow_failure` input: provided value is not a boolean', - '`image` input: required value has not been provided', - '`parallel` input: provided value is not a number', - '`website` input: provided value is not a string' - ) - end - end -end diff --git a/spec/lib/gitlab/ci/config/interpolation/text_template_spec.rb b/spec/lib/gitlab/ci/config/interpolation/text_template_spec.rb deleted file mode 100644 index 4fda0533da134eeafc2cabb1befe033a3a54599f..0000000000000000000000000000000000000000 --- a/spec/lib/gitlab/ci/config/interpolation/text_template_spec.rb +++ /dev/null @@ -1,105 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Gitlab::Ci::Config::Interpolation::TextTemplate, feature_category: :pipeline_composition, quarantine: 'https://gitlab.com/gitlab-org/gitlab/-/issues/440667' do - subject(:template) { described_class.new(config, ctx) } - - let(:config) do - <<~CFG - test: - spec: - env: $[[ inputs.env ]] - - $[[ inputs.key ]]: - name: $[[ inputs.key ]] - script: my-value - parallel: $[[ inputs.parallel ]] - CFG - end - - let(:ctx) do - { inputs: { env: 'dev', key: 'abc', parallel: 6 } } - end - - it 'interpolates the values properly' do - expect(template.interpolated).to eq <<~RESULT - test: - spec: - env: dev - - abc: - name: abc - script: my-value - parallel: 6 - RESULT - end - - context 'when the config has an unknown interpolation key' do - let(:config) { '$[[ xxx.yyy ]]: abc' } - - it 'does not interpolate the config' do - expect(template).not_to be_valid - expect(template.interpolated).to be_nil - expect(template.errors).to contain_exactly('unknown interpolation key: `xxx`') - end - end - - context 'when template consists of nested arrays with hashes and values' do - let(:config) do - <<~CFG - test: - - a-$[[ inputs.key ]]-b - - c-$[[ inputs.key ]]-d: - d-$[[ inputs.key ]]-e - val: 1 - CFG - end - - it 'performs a valid interpolation' do - result = <<~RESULT - test: - - a-abc-b - - c-abc-d: - d-abc-e - val: 1 - RESULT - - expect(template).to be_valid - expect(template.interpolated).to eq result - end - end - - context 'when template contains symbols that need interpolation' do - subject(:template) do - described_class.new("'$[[ inputs.key ]]': 'cde'", ctx) - end - - it 'performs a valid interpolation' do - expect(template).to be_valid - expect(template.interpolated).to eq("'abc': 'cde'") - end - end - - context 'when template is too large' do - before do - stub_application_setting(ci_max_total_yaml_size_bytes: 1) - end - - it 'returns an error' do - expect(template.interpolated).to be_nil - expect(template.errors).to contain_exactly('config too large') - end - end - - context 'when there are too many interpolation blocks' do - before do - stub_const("#{described_class}::MAX_BLOCKS", 1) - end - - it 'returns an error' do - expect(template.interpolated).to be_nil - expect(template.errors).to contain_exactly('too many interpolation blocks') - end - end -end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index b48e366fee3aa60d19b24020e1632e91bd576206..fa5ffd2c87dc62da4c55b2b91e8d3e298ee48f95 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -321,11 +321,6 @@ # Postgres is the primary data source, and ClickHouse only when enabled in certain cases. stub_feature_flags(clickhouse_data_collection: false) - # The code under this flag will be removed soon - # We are temporarily keeping it in place while we confirm some assumptions - # https://gitlab.com/gitlab-org/gitlab/-/issues/440667 - stub_feature_flags(ci_text_interpolation: false) - # The Vue version of the merge request list app is missing a lot of information # disabling this for now whilst we work on it across multiple merge requests stub_feature_flags(vue_merge_request_list: false)