From 6f386126583cd69ab8b98356e15dd51e639637ef Mon Sep 17 00:00:00 2001
From: Furkan Ayhan <furkanayhn@gmail.com>
Date: Thu, 9 Feb 2023 12:57:10 +0100
Subject: [PATCH] Refactor CI includes
This is mostly about redundant controls.
- Removed "next unless artifact_job" from "Artifact#content" because
this method will not be called if it is not valid.
- Removed "next unless creating_child_pipeline?" from "Artifact#content"
because this method will not be called if it is not valid.
- Replaced "return unless context.project" with "return unless valid?"
in the "Local#masked_*" methods because we don't need those methods
unless valid.
- Removed "next unless can_access_local_content?" and
"return unless sha" from "Project#fetch_local_content" because
this method will not be called if it is not valid.
- Removed "next unless project" from "Project#sha" because
this method will not be called if it is not valid.
- Replaced "return unless project" with "valid?" in the
"Project#masked_*" methods because we don't need those methods
unless valid.
---
lib/gitlab/ci/config/external/file/artifact.rb | 4 ----
lib/gitlab/ci/config/external/file/local.rb | 4 +++-
lib/gitlab/ci/config/external/file/project.rb | 9 ++-------
.../ci/config/external/file/project_spec.rb | 18 ++++++++++++------
4 files changed, 17 insertions(+), 18 deletions(-)
diff --git a/lib/gitlab/ci/config/external/file/artifact.rb b/lib/gitlab/ci/config/external/file/artifact.rb
index 8d557463a9a285..0b90d240a156b7 100644
--- a/lib/gitlab/ci/config/external/file/artifact.rb
+++ b/lib/gitlab/ci/config/external/file/artifact.rb
@@ -20,8 +20,6 @@ def initialize(params, context)
def content
strong_memoize(:content) do
- next unless artifact_job
-
Gitlab::Ci::ArtifactFileReader.new(artifact_job).read(location)
rescue Gitlab::Ci::ArtifactFileReader::Error => error
errors.push(error.message)
@@ -56,8 +54,6 @@ def validate_content!
def artifact_job
strong_memoize(:artifact_job) do
- next unless creating_child_pipeline?
-
context.parent_pipeline.find_job_with_archive_artifacts(job_name)
end
end
diff --git a/lib/gitlab/ci/config/external/file/local.rb b/lib/gitlab/ci/config/external/file/local.rb
index 95206233b2ef7c..bb1c304d02b2be 100644
--- a/lib/gitlab/ci/config/external/file/local.rb
+++ b/lib/gitlab/ci/config/external/file/local.rb
@@ -71,6 +71,8 @@ def expand_context_attrs
end
def masked_blob
+ return unless valid?
+
strong_memoize(:masked_blob) do
context.mask_variables_from(
Gitlab::Routing.url_helpers.project_blob_url(context.project, ::File.join(context.sha, location))
@@ -79,7 +81,7 @@ def masked_blob
end
def masked_raw
- return unless context.project
+ return unless valid?
strong_memoize(:masked_raw) do
context.mask_variables_from(
diff --git a/lib/gitlab/ci/config/external/file/project.rb b/lib/gitlab/ci/config/external/file/project.rb
index 248959a0df71d4..f8d4cb27710680 100644
--- a/lib/gitlab/ci/config/external/file/project.rb
+++ b/lib/gitlab/ci/config/external/file/project.rb
@@ -72,9 +72,6 @@ def can_access_local_content?
end
def fetch_local_content
- return unless can_access_local_content?
- return unless sha
-
BatchLoader.for([sha, location])
.batch(key: project) do |locations, loader, args|
context.logger.instrument(:config_file_fetch_project_content) do
@@ -88,8 +85,6 @@ def fetch_local_content
end
def sha
- return unless project
-
strong_memoize(:sha) do
project.commit(ref_name).try(:sha)
end
@@ -119,7 +114,7 @@ def masked_ref_name
end
def masked_blob
- return unless project
+ return unless valid?
strong_memoize(:masked_blob) do
context.mask_variables_from(
@@ -129,7 +124,7 @@ def masked_blob
end
def masked_raw
- return unless project
+ return unless valid?
strong_memoize(:masked_raw) do
context.mask_variables_from(
diff --git a/spec/lib/gitlab/ci/config/external/file/project_spec.rb b/spec/lib/gitlab/ci/config/external/file/project_spec.rb
index 3e990d28bb234b..abe38cdbc3ebc7 100644
--- a/spec/lib/gitlab/ci/config/external/file/project_spec.rb
+++ b/spec/lib/gitlab/ci/config/external/file/project_spec.rb
@@ -230,15 +230,21 @@
}
context 'when project name and ref include masked variables' do
+ let(:project_name) { 'my_project_name' }
+ let(:branch_name) { 'merge-commit-analyze-after' }
+ let(:project) { create(:project, :repository, name: project_name) }
+ let(:namespace_path) { project.namespace.full_path }
+ let(:included_project_sha) { project.commit(branch_name).sha }
+
let(:variables) do
Gitlab::Ci::Variables::Collection.new(
[
- { key: 'VAR1', value: 'a_secret_variable_value1', masked: true },
- { key: 'VAR2', value: 'a_secret_variable_value2', masked: true }
+ { key: 'VAR1', value: project_name, masked: true },
+ { key: 'VAR2', value: branch_name, masked: true }
])
end
- let(:params) { { project: 'a_secret_variable_value1', ref: 'a_secret_variable_value2', file: '/file.yml' } }
+ let(:params) { { project: project.full_path, ref: branch_name, file: '/file.yml' } }
it {
is_expected.to eq(
@@ -246,9 +252,9 @@
context_sha: project_sha,
type: :file,
location: 'file.yml',
- blob: nil,
- raw: nil,
- extra: { project: 'xxxxxxxxxxxxxxxxxxxxxxxx', ref: 'xxxxxxxxxxxxxxxxxxxxxxxx' }
+ blob: "http://localhost/#{namespace_path}/xxxxxxxxxxxxxxx/-/blob/#{included_project_sha}/file.yml",
+ raw: "http://localhost/#{namespace_path}/xxxxxxxxxxxxxxx/-/raw/#{included_project_sha}/file.yml",
+ extra: { project: "#{namespace_path}/xxxxxxxxxxxxxxx", ref: 'xxxxxxxxxxxxxxxxxxxxxxxxxx' }
)
}
end
--
GitLab