Skip to content
Snippets Groups Projects
Verified Commit bba39ef3 authored by Avielle Wolfe's avatar Avielle Wolfe :three:
Browse files

Remove CI header presence check under FF

This commit adds a `ci_remove_header_presence_check` feature flag. When
enabled, it removes the conditional that skips interpolation if the
content has no header for CI components (all other include types will
still skip interpolation if there is no header).

We will slowly enable the feature flag and monitor the performance of
`include:component`. This will let us know if we can add the new
`context` interpolation feature without hurting performance too much.

Issue: #438275
parent 78cc02e5
No related branches found
Tags v1.0.1-beta
No related merge requests found
---
name: ci_remove_header_presence_check
feature_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/438275
introduced_by_url:
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/474888
milestone: '17.3'
group: group::pipeline authoring
type: gitlab_com_derisk
default_enabled: false
......@@ -115,7 +115,8 @@ def content_inputs
def content_result
context.logger.instrument(:config_file_fetch_content_hash) do
::Gitlab::Ci::Config::Yaml::Loader.new(
content, inputs: content_inputs, variables: context.variables
content, inputs: content_inputs, variables: context.variables,
metadata: { include_type: metadata[:type], project: context.project }
).load
end
end
......
......@@ -10,12 +10,13 @@ module Interpolation
class Interpolator
attr_reader :config, :args, :variables, :errors
def initialize(config, args, variables)
def initialize(config, args, variables, metadata: {})
@config = config
@args = args.to_h
@variables = variables
@errors = []
@interpolated = false
@metadata = metadata
end
def valid?
......@@ -44,10 +45,10 @@ def interpolate!
_('Given inputs not defined in the `spec` section of the included configuration file'))
end
return @result ||= config.content unless config.has_header?
return @result ||= config.content unless interpolation_needed?
return @errors.concat(header.errors) unless header.valid?
return @errors.concat(inputs.errors) unless inputs.valid?
return @errors.concat(header.errors) if config.has_header? && !header.valid?
return @errors.concat(inputs.errors) if config.has_header? && !inputs.valid?
return @errors.concat(context.errors) unless context.valid?
return @errors.concat(template.errors) unless template.valid?
......@@ -62,6 +63,15 @@ def interpolated?
private
attr_reader :metadata
def interpolation_needed?
config.has_header? || (
Feature.enabled?(:ci_remove_header_presence_check, metadata[:project]) &&
metadata[:include_type] == :component
)
end
def inputs_without_header?
args.any? && !config.has_header?
end
......@@ -83,6 +93,8 @@ def spec
end
def inputs
return {} unless config.has_header?
@inputs ||= Inputs.new(spec, args)
end
......
......@@ -10,10 +10,11 @@ class Loader
AVAILABLE_TAGS = [Config::Yaml::Tags::Reference].freeze
MAX_DOCUMENTS = 2
def initialize(content, inputs: {}, variables: [])
def initialize(content, inputs: {}, variables: [], metadata: {})
@content = content
@inputs = inputs
@variables = variables
@metadata = metadata
end
def load
......@@ -21,7 +22,7 @@ def load
return yaml_result unless yaml_result.valid?
interpolator = Interpolation::Interpolator.new(yaml_result, inputs, variables)
interpolator = Interpolation::Interpolator.new(yaml_result, inputs, variables, metadata: metadata)
interpolator.interpolate!
......@@ -42,7 +43,7 @@ def load_uninterpolated_yaml
private
attr_reader :content, :inputs, :variables
attr_reader :content, :inputs, :variables, :metadata
def load_yaml!
ensure_custom_tags
......
......@@ -565,5 +565,63 @@
end
end
end
context 'when ci_remove_header_presence_check is enabled' do
before do
stub_feature_flags(ci_remove_header_presence_check: true)
end
context 'when fetching a component with no header' do
let(:values) do
{
include: { component: "#{Gitlab.config.gitlab.host}/#{another_project.full_path}/component-x@master" }
}
end
let(:other_project_files) do
{
'/templates/component-x/template.yml' => <<~YAML
component_x_job:
script: $[[ inputs.script ]]
YAML
}
end
before do
another_project.add_developer(user)
end
it 'interpolates the content' do
expect { processor.perform }.to raise_error(
described_class::IncludeError, /unknown interpolation key: `script`/
)
end
end
context 'when fetching any non-component with no header' do
let(:values) do
{
include: { local: 'included.yml' }
}
end
let(:project_files) do
{
'included.yml' => <<~YAML
component_x_job:
script: $[[ inputs.script ]]
YAML
}
end
it 'does not interpolate the content' do
expect(processor.perform).to eq({
component_x_job: {
script: '$[[ inputs.script ]]'
}
})
end
end
end
end
end
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment