From bccc7262844bc8758b72f0d7f11a01992b6b761a Mon Sep 17 00:00:00 2001
From: Rajendra Kadam <rkadam@gitlab.com>
Date: Wed, 19 Jun 2024 18:59:22 +0000
Subject: [PATCH] Add sha helper to move project_ref_name logic to module
Add specs for sha helper module
Use ShaHelper in CI lint class
---
lib/gitlab/ci/lint.rb | 24 +-----
lib/gitlab/ci/ref_finder.rb | 37 +++++++++
spec/lib/gitlab/ci/lint_spec.rb | 103 +-------------------------
spec/lib/gitlab/ci/ref_finder_spec.rb | 74 ++++++++++++++++++
4 files changed, 113 insertions(+), 125 deletions(-)
create mode 100644 lib/gitlab/ci/ref_finder.rb
create mode 100644 spec/lib/gitlab/ci/ref_finder_spec.rb
diff --git a/lib/gitlab/ci/lint.rb b/lib/gitlab/ci/lint.rb
index e7072616bdbef..12112c63e5207 100644
--- a/lib/gitlab/ci/lint.rb
+++ b/lib/gitlab/ci/lint.rb
@@ -81,7 +81,7 @@ def yaml_processor_result(content, logger)
logger.instrument(:yaml_process, once: true) do
Gitlab::Ci::YamlProcessor.new(content, project: project,
user: current_user,
- ref: project_ref_name,
+ ref: RefFinder.new(project).find_by_sha(sha),
sha: sha,
verify_project_sha: verify_project_sha,
logger: logger).execute
@@ -144,28 +144,6 @@ def build_logger
end
end
end
-
- def project_ref_name
- return unless project
-
- Rails.cache.fetch(['project', project.id, 'ref/containing/sha', sha], expires_in: 5.minutes) do
- break unless project_sha_exists?
-
- project_sha_branch_name || project_sha_tag_name
- end
- end
-
- def project_sha_branch_name
- project.repository.branch_names_contains(sha, limit: 1).first
- end
-
- def project_sha_tag_name
- project.repository.tag_names_contains(sha, limit: 1).first
- end
-
- def project_sha_exists?
- sha && project.repository_exists? && project.commit(sha)
- end
end
end
end
diff --git a/lib/gitlab/ci/ref_finder.rb b/lib/gitlab/ci/ref_finder.rb
new file mode 100644
index 0000000000000..203a3b6735f3d
--- /dev/null
+++ b/lib/gitlab/ci/ref_finder.rb
@@ -0,0 +1,37 @@
+# frozen_string_literal: true
+
+module Gitlab
+ module Ci
+ class RefFinder
+ def initialize(project)
+ @project = project
+ end
+
+ def find_by_sha(sha)
+ return unless project
+
+ Rails.cache.fetch(['project', project.id, 'ref/containing/sha', sha], expires_in: 5.minutes) do
+ break unless project_sha_exists?(sha)
+
+ project_sha_branch_name(sha) || project_sha_tag_name(sha)
+ end
+ end
+
+ private
+
+ attr_reader :project
+
+ def project_sha_branch_name(sha)
+ project.repository.branch_names_contains(sha, limit: 1).first
+ end
+
+ def project_sha_tag_name(sha)
+ project.repository.tag_names_contains(sha, limit: 1).first
+ end
+
+ def project_sha_exists?(sha)
+ sha && project.repository_exists? && project.commit(sha)
+ end
+ end
+ end
+end
diff --git a/spec/lib/gitlab/ci/lint_spec.rb b/spec/lib/gitlab/ci/lint_spec.rb
index 2fe6a34e97e21..ab21a21fe2179 100644
--- a/spec/lib/gitlab/ci/lint_spec.rb
+++ b/spec/lib/gitlab/ci/lint_spec.rb
@@ -226,7 +226,7 @@
end
end
- context 'when a pipeline ref variable is used in an `include`', :use_clean_rails_redis_caching do
+ context 'when a pipeline ref variable is used in an `include`' do
let(:dry_run) { false }
let(:content) do
@@ -290,14 +290,6 @@
)
end
- it 'caches values and calls Gitaly only once for branch names' do
- expect(project.repository).to receive(:branch_names_contains).once.and_call_original
-
- 2.times do
- lint.validate(content, dry_run: dry_run)
- end
- end
-
context 'when the ref is a tag' do
before do
project.repository.add_tag(project.creator, 'test', project.commit.id)
@@ -326,99 +318,6 @@
}
)
end
-
- it 'caches tag names and calls Gitaly only once' do
- expect(project.repository).to receive(:tag_names_contains).once.and_call_original
-
- 2.times { lint.validate(content, dry_run: dry_run) }
- end
- end
- end
-
- context 'when a pipeline ref variable is used in an include and project_sha_exists? returns false' do
- let(:dry_run) { false }
-
- let(:content) do
- <<~YAML
- include:
- - project: "#{project&.full_path}"
- ref: ${CI_COMMIT_REF_NAME}
- file: '.gitlab-ci-include.yml'
-
- show-parent-variable:
- stage : .pre
- script:
- - echo I am running a variable ${CI_COMMIT_REF_NAME}
- YAML
- end
-
- let(:included_content) do
- <<~YAML
- another_job:
- script: echo
- YAML
- end
-
- context 'when project is nil' do
- let(:project) { nil }
-
- it 'passes nil as the ref name to YamlProcessor' do
- expect(Gitlab::Ci::YamlProcessor)
- .to receive(:new)
- .with(content, a_hash_including(ref: nil))
- .and_call_original
-
- subject
- end
- end
-
- context 'when sha is nil' do
- let(:project) { nil }
- let(:sha) { nil }
-
- it 'passes nil as the ref name to YamlProcessor' do
- expect(Gitlab::Ci::YamlProcessor)
- .to receive(:new)
- .with(content, a_hash_including(ref: nil))
- .and_call_original
-
- subject
- end
- end
-
- context 'when project does not have a repository' do
- before do
- allow(project).to receive(:repository_exists?).and_return(false)
- end
-
- it 'passes nil as the ref name to YamlProcessor' do
- expect(Gitlab::Ci::YamlProcessor)
- .to receive(:new)
- .with(content, a_hash_including(ref: nil))
- .and_call_original
-
- subject
- end
- end
-
- context 'when commit does not exist in the project' do
- let(:sha) { 'invalid-sha' }
-
- it 'passes nil as the ref name to YamlProcessor' do
- expect(Gitlab::Ci::YamlProcessor)
- .to receive(:new)
- .with(content, a_hash_including(ref: nil))
- .and_call_original
-
- subject
- end
-
- it 'does not cache ref names and does not call Gitaly' do
- expect(project.repository).not_to receive(:branch_names_contains)
- expect(project.repository).not_to receive(:tag_names_contains)
-
- subject
- end
end
end
diff --git a/spec/lib/gitlab/ci/ref_finder_spec.rb b/spec/lib/gitlab/ci/ref_finder_spec.rb
new file mode 100644
index 0000000000000..9c7cc01e42b6f
--- /dev/null
+++ b/spec/lib/gitlab/ci/ref_finder_spec.rb
@@ -0,0 +1,74 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Gitlab::Ci::RefFinder, feature_category: :pipeline_composition do
+ let_it_be(:project) { create(:project, :repository) }
+ let(:sha) { project&.commit&.sha }
+
+ describe '#find_by_sha' do
+ subject(:find_by_sha) { described_class.new(project).find_by_sha(sha) }
+
+ context 'when sha matches a branch', :use_clean_rails_redis_caching do
+ it 'returns the branch name' do
+ expect(find_by_sha).to eq('2-mb-file')
+ end
+
+ it 'caches branch name and calls Gitaly only once' do
+ expect(project.repository).to receive(:branch_names_contains).once.and_call_original
+
+ 2.times do
+ described_class.new(project).find_by_sha(sha)
+ end
+ end
+
+ context 'when sha matches a tag', :use_clean_rails_redis_caching do
+ let(:tag_name) { 'v1.1.0' }
+ let(:sha) { project.repository.find_tag(tag_name).dereferenced_target.sha }
+
+ before do
+ # the sha of v1.1.0 is also in a a branch so we fake that it's not
+ allow(project.repository).to receive(:branch_names_contains).and_return([])
+ end
+
+ it 'returns the tag name' do
+ expect(find_by_sha).to eq(tag_name)
+ end
+
+ it 'caches tag name and calls Gitaly only once' do
+ expect(project.repository).to receive(:tag_names_contains).once.and_call_original
+
+ 2.times do
+ described_class.new(project).find_by_sha(sha)
+ end
+ end
+ end
+ end
+
+ context 'when project does not exist' do
+ let(:project) { nil }
+
+ it 'returns nil' do
+ expect(find_by_sha).to be_nil
+ end
+ end
+
+ context 'when sha does not exist' do
+ let(:sha) { 'invalid-sha' }
+
+ it 'returns nil' do
+ expect(find_by_sha).to be_nil
+ end
+ end
+
+ context 'when repository does not exist' do
+ before do
+ allow(project).to receive(:repository_exists?).and_return(false)
+ end
+
+ it 'returns nil' do
+ expect(find_by_sha).to be_nil
+ end
+ end
+ end
+end
--
GitLab