From e1438ca74550351359ef84a58eda4be031f737c1 Mon Sep 17 00:00:00 2001 From: Furkan Ayhan <furkanayhn@gmail.com> Date: Mon, 23 Oct 2023 12:33:54 +0000 Subject: [PATCH] Refactor usage of .gitlab-ci.yml as the default CI Config Using .gitlab-ci.yml as the default CI Config is distributed across the codebase. In this MR, we are simplifying it for further improvements --- app/helpers/blob_helper.rb | 2 +- app/models/ci/pipeline.rb | 5 +++-- app/models/project.rb | 4 ++-- app/models/repository.rb | 4 ---- .../security/configuration_presenter.rb | 4 ++-- .../google_cloud/generate_pipeline_service.rb | 2 +- .../ci_configuration/sast_parser_service.rb | 8 +++----- doc/ci/troubleshooting.md | 2 +- .../dast/scan_configs/fetch_service.rb | 2 +- lib/api/lint.rb | 2 +- lib/gitlab/file_detector.rb | 2 +- spec/helpers/blob_helper_spec.rb | 2 +- spec/lib/gitlab/file_detector_spec.rb | 2 +- spec/lib/result_spec.rb | 2 +- spec/models/repository_spec.rb | 20 ------------------- .../security/configuration_presenter_spec.rb | 2 +- .../merge_request_widget_entity_spec.rb | 2 +- .../generate_pipeline_service_spec.rb | 16 +++++++-------- 18 files changed, 29 insertions(+), 54 deletions(-) diff --git a/app/helpers/blob_helper.rb b/app/helpers/blob_helper.rb index 0d5b8755a37e..8c199aefd81e 100644 --- a/app/helpers/blob_helper.rb +++ b/app/helpers/blob_helper.rb @@ -300,7 +300,7 @@ def edit_button_tag(blob, common_classes, text, edit_path, project, ref) end def show_suggest_pipeline_creation_celebration? - @blob.path == Gitlab::FileDetector::PATTERNS[:gitlab_ci] && + Gitlab::FileDetector.type_of(@blob.path) == :gitlab_ci && @blob.auxiliary_viewer&.valid?(project: @project, sha: @commit.sha, user: current_user) && @project.uses_default_ci_config? && cookies[suggest_pipeline_commit_cookie_name].present? diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index 0a876d26cc95..d80e117704bd 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -30,8 +30,9 @@ class Pipeline < Ci::ApplicationRecord PROJECT_ROUTE_AND_NAMESPACE_ROUTE = { project: [:project_feature, :route, { namespace: :route }] }.freeze - CONFIG_EXTENSION = '.gitlab-ci.yml' - DEFAULT_CONFIG_PATH = CONFIG_EXTENSION + + DEFAULT_CONFIG_PATH = '.gitlab-ci.yml' + CANCELABLE_STATUSES = (Ci::HasStatus::CANCELABLE_STATUSES + ['manual']).freeze paginates_per 15 diff --git a/app/models/project.rb b/app/models/project.rb index fd226d23e770..0587ff69d3a8 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -2875,7 +2875,7 @@ def jira_subscription_exists? end def uses_default_ci_config? - ci_config_path.blank? || ci_config_path == Gitlab::FileDetector::PATTERNS[:gitlab_ci] + ci_config_path.blank? || Gitlab::FileDetector.type_of(ci_config_path) == :gitlab_ci end def limited_protected_branches(limit) @@ -3026,7 +3026,7 @@ def ci_config_path_or_default end def ci_config_for(sha) - repository.gitlab_ci_yml_for(sha, ci_config_path_or_default) + repository.blob_data_at(sha, ci_config_path_or_default) end def enabled_group_deploy_keys diff --git a/app/models/repository.rb b/app/models/repository.rb index e565de9c4ba7..b35336d47b8b 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -1102,10 +1102,6 @@ def route_map_for(sha) blob_data_at(sha, '.gitlab/route-map.yml') end - def gitlab_ci_yml_for(sha, path = '.gitlab-ci.yml') - blob_data_at(sha, path) - end - def lfsconfig_for(sha) blob_data_at(sha, '.lfsconfig') end diff --git a/app/presenters/projects/security/configuration_presenter.rb b/app/presenters/projects/security/configuration_presenter.rb index f248652befc2..a0d731f0ccfc 100644 --- a/app/presenters/projects/security/configuration_presenter.rb +++ b/app/presenters/projects/security/configuration_presenter.rb @@ -55,8 +55,8 @@ def can_toggle_autofix; end def gitlab_ci_history_path return '' if project.empty_repo? - gitlab_ci = ::Gitlab::FileDetector::PATTERNS[:gitlab_ci] - ::Gitlab::Routing.url_helpers.project_blame_path(project, File.join(project.default_branch_or_main, gitlab_ci)) + ::Gitlab::Routing.url_helpers.project_blame_path( + project, File.join(project.default_branch_or_main, project.ci_config_path_or_default)) end def features diff --git a/app/services/google_cloud/generate_pipeline_service.rb b/app/services/google_cloud/generate_pipeline_service.rb index 30c358687aab..97d008db76b8 100644 --- a/app/services/google_cloud/generate_pipeline_service.rb +++ b/app/services/google_cloud/generate_pipeline_service.rb @@ -67,7 +67,7 @@ def generate_commit_attributes end def default_branch_gitlab_ci_yml - @default_branch_gitlab_ci_yml ||= project.repository.gitlab_ci_yml_for(project.default_branch) + @default_branch_gitlab_ci_yml ||= project.ci_config_for(project.default_branch) end def pipeline_content(include_path) diff --git a/app/services/security/ci_configuration/sast_parser_service.rb b/app/services/security/ci_configuration/sast_parser_service.rb index 16a9efcefdf8..f466dd0b6499 100644 --- a/app/services/security/ci_configuration/sast_parser_service.rb +++ b/app/services/security/ci_configuration/sast_parser_service.rb @@ -89,17 +89,15 @@ def sast_template_attributes def gitlab_ci_yml_attributes @gitlab_ci_yml_attributes ||= begin - config_content = @project.repository.blob_data_at(@project.repository.root_ref_sha, ci_config_file) + config_content = @project.repository.blob_data_at( + @project.repository.root_ref_sha, @project.ci_config_path_or_default + ) return {} unless config_content build_sast_attributes(config_content) end end - def ci_config_file - '.gitlab-ci.yml' - end - def build_sast_attributes(content) options = { project: @project, user: current_user, sha: @project.repository.commit.sha } yaml_result = Gitlab::Ci::YamlProcessor.new(content, options).execute diff --git a/doc/ci/troubleshooting.md b/doc/ci/troubleshooting.md index cc7e55944661..092b47ba1158 100644 --- a/doc/ci/troubleshooting.md +++ b/doc/ci/troubleshooting.md @@ -505,7 +505,7 @@ mr.project.try(:ci_integration) ```ruby project = Project.find_by_full_path('<project_path>') -content = p.repository.gitlab_ci_yml_for(project.repository.root_ref_sha) +content = p.ci_config_for(project.repository.root_ref_sha) Gitlab::Ci::Lint.new(project: project, current_user: User.first).validate(content) ``` diff --git a/ee/app/services/app_sec/dast/scan_configs/fetch_service.rb b/ee/app/services/app_sec/dast/scan_configs/fetch_service.rb index 0170f575fbe9..110ab8763699 100644 --- a/ee/app/services/app_sec/dast/scan_configs/fetch_service.rb +++ b/ee/app/services/app_sec/dast/scan_configs/fetch_service.rb @@ -49,7 +49,7 @@ def fetch_from_policy def fetch_from_project_gitlab_ci_yml return unless project.repository_exists? - yml_dump = project.repository.gitlab_ci_yml_for(project.default_branch) + yml_dump = project.ci_config_for(project.default_branch) result = ::Gitlab::Ci::YamlProcessor .new(yml_dump, project: project, user: current_user, sha: project.repository&.commit&.sha) diff --git a/lib/api/lint.rb b/lib/api/lint.rb index 26619e6924f8..b2f0f54e3807 100644 --- a/lib/api/lint.rb +++ b/lib/api/lint.rb @@ -29,7 +29,7 @@ class Lint < ::API::Base not_found! 'Commit' unless user_project.commit(sha).present? - content = user_project.repository.gitlab_ci_yml_for(sha, user_project.ci_config_path_or_default) + content = user_project.repository.blob_data_at(sha, user_project.ci_config_path_or_default) result = Gitlab::Ci::Lint .new(project: user_project, current_user: current_user, sha: sha) .validate(content, dry_run: params[:dry_run], ref: params[:ref] || user_project.default_branch) diff --git a/lib/gitlab/file_detector.rb b/lib/gitlab/file_detector.rb index 13959f6aa68a..ef8f2d4d61b0 100644 --- a/lib/gitlab/file_detector.rb +++ b/lib/gitlab/file_detector.rb @@ -21,7 +21,7 @@ module FileDetector # Configuration files gitignore: '.gitignore', - gitlab_ci: '.gitlab-ci.yml', + gitlab_ci: ::Ci::Pipeline::DEFAULT_CONFIG_PATH, route_map: '.gitlab/route-map.yml', # Dependency files diff --git a/spec/helpers/blob_helper_spec.rb b/spec/helpers/blob_helper_spec.rb index e832fa2718af..d09e01c959c9 100644 --- a/spec/helpers/blob_helper_spec.rb +++ b/spec/helpers/blob_helper_spec.rb @@ -221,7 +221,7 @@ context 'when file is a pipeline config file' do let(:data) { File.read(Rails.root.join('spec/support/gitlab_stubs/gitlab_ci.yml')) } - let(:blob) { fake_blob(path: Gitlab::FileDetector::PATTERNS[:gitlab_ci], data: data) } + let(:blob) { fake_blob(path: project.ci_config_path_or_default, data: data) } it 'is true' do expect(helper.show_suggest_pipeline_creation_celebration?).to be_truthy diff --git a/spec/lib/gitlab/file_detector_spec.rb b/spec/lib/gitlab/file_detector_spec.rb index 208acf28cc4c..8c0c56ea2c30 100644 --- a/spec/lib/gitlab/file_detector_spec.rb +++ b/spec/lib/gitlab/file_detector_spec.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -require 'fast_spec_helper' +require 'spec_helper' RSpec.describe Gitlab::FileDetector do describe '.types_in_paths' do diff --git a/spec/lib/result_spec.rb b/spec/lib/result_spec.rb index 2b88521fe141..170a2f5e7772 100644 --- a/spec/lib/result_spec.rb +++ b/spec/lib/result_spec.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -require 'fast_spec_helper' +require 'spec_helper' # NOTE: # This spec is intended to serve as documentation examples of idiomatic usage for the `Result` type. diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index 2265d1b39af9..5c766035e98c 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -3100,26 +3100,6 @@ def mock_gitaly(second_response) end end - describe '#gitlab_ci_yml_for' do - let(:project) { create(:project, :repository) } - - before do - repository.create_file(User.last, '.gitlab-ci.yml', 'CONTENT', message: 'Add .gitlab-ci.yml', branch_name: 'master') - end - - context 'when there is a .gitlab-ci.yml at the commit' do - it 'returns the content' do - expect(repository.gitlab_ci_yml_for(repository.commit.sha)).to eq('CONTENT') - end - end - - context 'when there is no .gitlab-ci.yml at the commit' do - it 'returns nil' do - expect(repository.gitlab_ci_yml_for(repository.commit.parent.sha)).to be_nil - end - end - end - describe '#changelog_config' do let(:project) { create(:project, :repository) } let(:changelog_config_path) { Gitlab::Changelog::Config::DEFAULT_FILE_PATH } diff --git a/spec/presenters/projects/security/configuration_presenter_spec.rb b/spec/presenters/projects/security/configuration_presenter_spec.rb index beabccf6639a..fcd170dfd66d 100644 --- a/spec/presenters/projects/security/configuration_presenter_spec.rb +++ b/spec/presenters/projects/security/configuration_presenter_spec.rb @@ -177,7 +177,7 @@ let!(:ci_config) do project.repository.create_file( project.creator, - Gitlab::FileDetector::PATTERNS[:gitlab_ci], + project.ci_config_path_or_default, 'contents go here', message: 'test', branch_name: 'master') diff --git a/spec/serializers/merge_request_widget_entity_spec.rb b/spec/serializers/merge_request_widget_entity_spec.rb index 8a0a2d38187f..e9707265263b 100644 --- a/spec/serializers/merge_request_widget_entity_spec.rb +++ b/spec/serializers/merge_request_widget_entity_spec.rb @@ -88,7 +88,7 @@ let(:role) { :developer } before do - project.repository.create_file(user, Gitlab::FileDetector::PATTERNS[:gitlab_ci], 'CONTENT', message: 'Add .gitlab-ci.yml', branch_name: 'master') + project.repository.create_file(user, project.ci_config_path_or_default, 'CONTENT', message: 'Add .gitlab-ci.yml', branch_name: 'master') end it 'no ci config path' do diff --git a/spec/services/google_cloud/generate_pipeline_service_spec.rb b/spec/services/google_cloud/generate_pipeline_service_spec.rb index 26a1ccb7e3b4..8f49e1af901d 100644 --- a/spec/services/google_cloud/generate_pipeline_service_spec.rb +++ b/spec/services/google_cloud/generate_pipeline_service_spec.rb @@ -32,7 +32,7 @@ response = service.execute ref = response[:commit][:result] - gitlab_ci_yml = project.repository.gitlab_ci_yml_for(ref) + gitlab_ci_yml = project.ci_config_for(ref) expect(response[:status]).to eq(:success) expect(gitlab_ci_yml).to include('https://gitlab.com/gitlab-org/incubation-engineering/five-minute-production/library/-/raw/main/gcp/cloud-run.gitlab-ci.yml') @@ -97,7 +97,7 @@ response = service.execute branch_name = response[:branch_name] - gitlab_ci_yml = project.repository.gitlab_ci_yml_for(branch_name) + gitlab_ci_yml = project.ci_config_for(branch_name) pipeline = Gitlab::Config::Loader::Yaml.new(gitlab_ci_yml).load! expect(response[:status]).to eq(:success) @@ -110,7 +110,7 @@ response = service.execute branch_name = response[:branch_name] - gitlab_ci_yml = project.repository.gitlab_ci_yml_for(branch_name) + gitlab_ci_yml = project.ci_config_for(branch_name) expect(YAML.safe_load(gitlab_ci_yml).keys).to eq(%w[stages build-java test-java include]) end @@ -153,7 +153,7 @@ response = service.execute branch_name = response[:branch_name] - gitlab_ci_yml = project.repository.gitlab_ci_yml_for(branch_name) + gitlab_ci_yml = project.ci_config_for(branch_name) pipeline = Gitlab::Config::Loader::Yaml.new(gitlab_ci_yml).load! expect(response[:status]).to eq(:success) @@ -195,7 +195,7 @@ response = service.execute branch_name = response[:branch_name] - gitlab_ci_yml = project.repository.gitlab_ci_yml_for(branch_name) + gitlab_ci_yml = project.ci_config_for(branch_name) pipeline = Gitlab::Config::Loader::Yaml.new(gitlab_ci_yml).load! expect(response[:status]).to eq(:success) @@ -235,7 +235,7 @@ response = service.execute ref = response[:commit][:result] - gitlab_ci_yml = project.repository.gitlab_ci_yml_for(ref) + gitlab_ci_yml = project.ci_config_for(ref) expect(response[:status]).to eq(:success) expect(gitlab_ci_yml).to include('https://gitlab.com/gitlab-org/incubation-engineering/five-minute-production/library/-/raw/main/gcp/cloud-storage.gitlab-ci.yml') @@ -272,7 +272,7 @@ response = service.execute ref = response[:commit][:result] - gitlab_ci_yml = project.repository.gitlab_ci_yml_for(ref) + gitlab_ci_yml = project.ci_config_for(ref) expect(response[:status]).to eq(:success) expect(gitlab_ci_yml).to include('https://gitlab.com/gitlab-org/incubation-engineering/five-minute-production/library/-/raw/main/gcp/vision-ai.gitlab-ci.yml') @@ -328,7 +328,7 @@ response = service.execute branch_name = response[:branch_name] - gitlab_ci_yml = project.repository.gitlab_ci_yml_for(branch_name) + gitlab_ci_yml = project.ci_config_for(branch_name) pipeline = Gitlab::Config::Loader::Yaml.new(gitlab_ci_yml).load! expect(response[:status]).to eq(:success) -- GitLab