From a4032ae6aef2e5dbd8a63b5a78fa3b334ab8e98f Mon Sep 17 00:00:00 2001
From: Avielle Wolfe <awolfe@gitlab.com>
Date: Mon, 26 Feb 2024 14:58:54 +0000
Subject: [PATCH] Fix boolean and number input types
Boolean and number input types were not be respected by CI
interpolation. They were being cast to strings when interpolated by
`gsub`. This commit fixes that by replacing the entire node with the
input value when the interpolation block is the entire length of the
node. In other cases, we can assume that the input is being inserted
into a string, so it's safe for us to use `gsub` to cast the value to a
string and perform the insertion.
This commit also ensures that the ability to match and replace multiple
interpolation blocks in a single node is not affected. A previous
attempt to fix this bug caused an incident because it only interpolated
the first block in a node.
Changelog: fixed
Issue: https://gitlab.com/gitlab-org/gitlab/-/issues/434826
---
.../gitlab_com_derisk/ci_fix_input_types.yml | 9 +++
lib/gitlab/ci/config/interpolation/block.rb | 22 ++++--
.../ci/config/interpolation/template.rb | 71 ++++++++++++++++---
.../ci/config/interpolation/block_spec.rb | 12 ++++
.../ci/config/interpolation/template_spec.rb | 34 +++++++--
spec/lib/gitlab/ci/config/yaml/loader_spec.rb | 4 +-
6 files changed, 131 insertions(+), 21 deletions(-)
create mode 100644 config/feature_flags/gitlab_com_derisk/ci_fix_input_types.yml
diff --git a/config/feature_flags/gitlab_com_derisk/ci_fix_input_types.yml b/config/feature_flags/gitlab_com_derisk/ci_fix_input_types.yml
new file mode 100644
index 000000000000..b910822c618d
--- /dev/null
+++ b/config/feature_flags/gitlab_com_derisk/ci_fix_input_types.yml
@@ -0,0 +1,9 @@
+---
+name: ci_fix_input_types
+feature_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/434826
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/145257
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/443301
+milestone: '16.10'
+group: group::pipeline authoring
+type: gitlab_com_derisk
+default_enabled: false
diff --git a/lib/gitlab/ci/config/interpolation/block.rb b/lib/gitlab/ci/config/interpolation/block.rb
index dc1ca423a816..0c9a83fed1d5 100644
--- a/lib/gitlab/ci/config/interpolation/block.rb
+++ b/lib/gitlab/ci/config/interpolation/block.rb
@@ -17,7 +17,15 @@ class Block
PATTERN = /(?<block>\$\[\[\s*(?<data>\S{1}.*?\S{1})\s*\]\])/
MAX_FUNCTIONS = 3
- attr_reader :block, :data, :ctx, :errors
+ attr_reader :data, :ctx, :errors
+
+ def self.match(data)
+ return data unless data.is_a?(String) && data.include?(PREFIX)
+
+ data.gsub(PATTERN) do
+ yield ::Regexp.last_match(1), ::Regexp.last_match(2)
+ end
+ end
def initialize(block, data, ctx)
@block = block
@@ -43,16 +51,18 @@ def value
@value
end
- def self.match(data)
- return data unless data.is_a?(String) && data.include?(PREFIX)
+ def length
+ block.length
+ end
- data.gsub(PATTERN) do
- yield ::Regexp.last_match(1), ::Regexp.last_match(2)
- end
+ def to_s
+ block
end
private
+ attr_reader :block
+
# We expect the block data to be a string with one or more entities delimited by pipes:
# <access> | <function1> | <function2> | ... <functionN>
def evaluate!
diff --git a/lib/gitlab/ci/config/interpolation/template.rb b/lib/gitlab/ci/config/interpolation/template.rb
index 2a0f98480024..697be852c81b 100644
--- a/lib/gitlab/ci/config/interpolation/template.rb
+++ b/lib/gitlab/ci/config/interpolation/template.rb
@@ -10,6 +10,8 @@ class Template
attr_reader :blocks, :ctx
MAX_BLOCKS = 10_000
+ BLOCK_PATTERN = /(?<block>\$\[\[\s*(?<data>\S{1}.*?\S{1})\s*\]\])/
+ BLOCK_PREFIX = '$[['
def initialize(config, ctx)
@config = Interpolation::Config.fabricate(config)
@@ -39,20 +41,73 @@ def interpolated
private
def interpolate!
- @result = @config.replace! do |data|
+ @result = @config.replace! do |node|
break if @errors.any?
- Interpolation::Block.match(data) do |block, data|
- block = (@blocks[block] ||= Interpolation::Block.new(block, data, ctx))
-
- break @errors.push('too many interpolation blocks') if @blocks.size > MAX_BLOCKS
- break unless block.valid?
-
- block.value
+ if Feature.enabled?(:ci_fix_input_types, Feature.current_request, type: :gitlab_com_derisk)
+ interpolate_with_fixed_types!(node)
+ else
+ legacy_interpolate!(node)
end
end
end
strong_memoize_attr :interpolate!
+
+ def interpolate_with_fixed_types!(node)
+ return node unless node_might_contain_interpolation_block?(node)
+
+ matches = node.scan(BLOCK_PATTERN)
+ return node if matches.empty?
+
+ blocks = interpolate_blocks(matches)
+ return unless @errors.none? && blocks.present?
+
+ get_interpolated_node_content!(node, blocks)
+ end
+
+ def legacy_interpolate!(node)
+ Interpolation::Block.match(node) do |block, data|
+ block = (@blocks[block] ||= Interpolation::Block.new(block, data, ctx))
+
+ break @errors.push('too many interpolation blocks') if @blocks.size > MAX_BLOCKS
+ break unless block.valid?
+
+ block.value
+ end
+ end
+
+ def node_might_contain_interpolation_block?(node)
+ node.is_a?(String) && node.include?(BLOCK_PREFIX)
+ end
+
+ def interpolate_blocks(matches)
+ matches.map do |match, data|
+ block = (@blocks[match] ||= Interpolation::Block.new(match, data, ctx))
+
+ break @errors.push('too many interpolation blocks') if @blocks.size > MAX_BLOCKS
+ break unless block.valid?
+
+ block
+ end
+ end
+
+ def get_interpolated_node_content!(node, blocks)
+ if used_inside_a_string?(node, blocks)
+ interpolate_string_node!(node, blocks)
+ else
+ blocks.first.value
+ end
+ end
+
+ def interpolate_string_node!(node, blocks)
+ blocks.reduce(node) do |interpolated_node, block|
+ interpolated_node.gsub(block.to_s, block.value.to_s)
+ end
+ end
+
+ def used_inside_a_string?(node, blocks)
+ blocks.count > 1 || node.length != blocks.first.length
+ end
end
end
end
diff --git a/spec/lib/gitlab/ci/config/interpolation/block_spec.rb b/spec/lib/gitlab/ci/config/interpolation/block_spec.rb
index 7b9111f11d37..02bbbcb5a17a 100644
--- a/spec/lib/gitlab/ci/config/interpolation/block_spec.rb
+++ b/spec/lib/gitlab/ci/config/interpolation/block_spec.rb
@@ -109,4 +109,16 @@
end
end
end
+
+ describe '#to_s' do
+ it 'returns the interpolation block' do
+ expect(subject.to_s).to eq(block)
+ end
+ end
+
+ describe '#length' do
+ it 'returns the length of the interpolation block' do
+ expect(subject.length).to eq(block.length)
+ end
+ end
end
diff --git a/spec/lib/gitlab/ci/config/interpolation/template_spec.rb b/spec/lib/gitlab/ci/config/interpolation/template_spec.rb
index c7d888225584..647c698edb79 100644
--- a/spec/lib/gitlab/ci/config/interpolation/template_spec.rb
+++ b/spec/lib/gitlab/ci/config/interpolation/template_spec.rb
@@ -1,6 +1,6 @@
# frozen_string_literal: true
-require 'fast_spec_helper'
+require 'spec_helper'
RSpec.describe Gitlab::Ci::Config::Interpolation::Template, feature_category: :pipeline_composition do
subject { described_class.new(YAML.safe_load(config), ctx) }
@@ -13,16 +13,18 @@
$[[ inputs.key ]]:
name: $[[ inputs.key ]]
- script: my-value
+ parallel: $[[ inputs.parallel ]]
+ allow_failure: $[[ inputs.allow_failure ]]
+ script: 'echo "This job makes $[[ inputs.parallel ]] jobs for the $[[ inputs.env ]] env"'
CFG
end
let(:ctx) do
- { inputs: { env: 'dev', key: 'abc' } }
+ { inputs: { allow_failure: true, env: 'dev', key: 'abc', parallel: 6 } }
end
it 'collects interpolation blocks' do
- expect(subject.size).to eq 2
+ expect(subject.size).to eq 4
end
it 'interpolates the values properly' do
@@ -33,10 +35,32 @@
abc:
name: abc
- script: my-value
+ parallel: 6
+ allow_failure: true
+ script: 'echo "This job makes 6 jobs for the dev env"'
RESULT
end
+ context 'when ci_fix_input_types is disabled' do
+ before do
+ stub_feature_flags(ci_fix_input_types: false)
+ end
+
+ it 'interpolates all values as strings' do
+ expect(subject.interpolated).to eq YAML.safe_load <<~RESULT
+ test:
+ spec:
+ env: dev
+
+ abc:
+ name: abc
+ parallel: '6'
+ allow_failure: 'true'
+ script: 'echo "This job makes 6 jobs for the dev env"'
+ RESULT
+ end
+ end
+
context 'when interpolation can not be performed' do
let(:config) { '$[[ xxx.yyy ]]: abc' }
diff --git a/spec/lib/gitlab/ci/config/yaml/loader_spec.rb b/spec/lib/gitlab/ci/config/yaml/loader_spec.rb
index f8563533b04c..2b1b85baeaea 100644
--- a/spec/lib/gitlab/ci/config/yaml/loader_spec.rb
+++ b/spec/lib/gitlab/ci/config/yaml/loader_spec.rb
@@ -16,7 +16,7 @@
'echo "Building with clang and optimization level 3"',
'echo "1.0.0"'
],
- parallel: '2'
+ parallel: 2
},
'my-job-test': {
stage: 'build',
@@ -24,7 +24,7 @@
'echo "Testing with pytest"',
'if [ true == true ]; then echo "Coverage is enabled"; fi'
],
- allow_failure: 'false'
+ allow_failure: false
},
'my-job-deploy': {
stage: 'build',
--
GitLab