diff --git a/app/models/repository.rb b/app/models/repository.rb index 3cf3879672c404ecc8c69a7ea50915dbdf4f859d..c08b6f1dfc537d60e164083c393cbca3eac5c399 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -259,8 +259,8 @@ def expand_ref(ref) end end - def add_branch(user, branch_name, ref, expire_cache: true) - branch = raw_repository.add_branch(branch_name, user: user, target: ref) + def add_branch(user, branch_name, ref, expire_cache: true, skip_ci: false) + branch = raw_repository.add_branch(branch_name, user: user, target: ref, skip_ci: skip_ci) after_create_branch(expire_cache: expire_cache) diff --git a/app/services/ci/workloads/run_workload_service.rb b/app/services/ci/workloads/run_workload_service.rb index 7885c1b7559cf186655191347269501aeb170186..de92e8ef721d022905d376b7ffdd4a4af6a2d692 100644 --- a/app/services/ci/workloads/run_workload_service.rb +++ b/app/services/ci/workloads/run_workload_service.rb @@ -38,7 +38,7 @@ def create_repository_branch branch_name = "workloads/#{SecureRandom.hex[0..10]}" raise "Branch already exists" if @project.repository.branch_exists?(branch_name) - repo_branch = @project.repository.add_branch(@current_user, branch_name, default_branch) + repo_branch = @project.repository.add_branch(@current_user, branch_name, default_branch, skip_ci: true) raise "Error in git branch creation" unless repo_branch branch_name diff --git a/app/services/post_receive_service.rb b/app/services/post_receive_service.rb index 1f91e914054e31cb29d8e140ee13f6e9b32c67e2..9b1f5c31da55779f37461c9af72725752fcead58 100644 --- a/app/services/post_receive_service.rb +++ b/app/services/post_receive_service.rb @@ -16,7 +16,6 @@ def initialize(user, repository, project, params) def execute response = Gitlab::InternalPostReceive::Response.new - push_options = Gitlab::PushOptions.new(params[:push_options]) mr_options = push_options.get(:merge_request) response.reference_counter_decreased = Gitlab::ReferenceCounter.new(params[:gl_repository]).decrease @@ -55,6 +54,14 @@ def execute response end + def push_options + @push_options ||= begin + options = params[:push_options] || [] + options += [Gitlab::PushOptions::CI_SKIP] if !!params.dig(:gitaly_context, 'skip-ci') + Gitlab::PushOptions.new(options) + end + end + def process_mr_push_options(push_options, changes) Gitlab::QueryLimiting.disable!('https://gitlab.com/gitlab-org/gitlab/-/issues/28494') return unless repository diff --git a/lib/api/internal/base.rb b/lib/api/internal/base.rb index 35da1539058d5e4470faa9b6a2fb3d586ff18043..7e54809c43eb91afeb003db037f46a8b7dba0700 100644 --- a/lib/api/internal/base.rb +++ b/lib/api/internal/base.rb @@ -12,6 +12,8 @@ class Base < ::API::Base api_endpoint = env['api.endpoint'] feature_category = api_endpoint.options[:for].try(:feature_category_for_app, api_endpoint).to_s + add_gitaly_context(params) + if actor.user load_balancer_stick_request(::User, :user, actor.user.id) set_current_organization(user: actor.user) @@ -40,8 +42,12 @@ def lfs_authentication_url(container) container.lfs_http_url_to_repo end + def add_gitaly_context(params) + params[:gitaly_context] = gitaly_context(params) + end + def link_scoped_user(params) - context = gitaly_context(params) + context = params[:gitaly_context] return unless context diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb index cf45865860361c031f16cb6b530edce64138c19b..6e6c2c692159336aa259a1a12a509caac30a3616 100644 --- a/lib/gitlab/git/repository.rb +++ b/lib/gitlab/git/repository.rb @@ -640,9 +640,9 @@ def clean(options = {}) # TODO: implement this method end - def add_branch(branch_name, user:, target:) + def add_branch(branch_name, user:, target:, skip_ci: false) wrapped_gitaly_errors do - gitaly_operation_client.user_create_branch(branch_name, user, target) + gitaly_operation_client.user_create_branch(branch_name, user, target, skip_ci: skip_ci) end end diff --git a/lib/gitlab/gitaly_client/operation_service.rb b/lib/gitlab/gitaly_client/operation_service.rb index fe352ade7978cffd49603bb8b0c0cc3f601ace5a..b6d692bd177d3cb64f2ce41170d4bd749e834d85 100644 --- a/lib/gitlab/gitaly_client/operation_service.rb +++ b/lib/gitlab/gitaly_client/operation_service.rb @@ -66,15 +66,22 @@ def add_tag(tag_name, user, target, message) end end - def user_create_branch(branch_name, user, start_point) + def user_create_branch(branch_name, user, start_point, skip_ci: false) request = Gitaly::UserCreateBranchRequest.new( repository: @gitaly_repo, branch_name: encode_binary(branch_name), user: Gitlab::Git::User.from_gitlab(user).to_gitaly, start_point: encode_binary(start_point) ) + + params = { timeout: GitalyClient.long_timeout } + + if skip_ci + params[:gitaly_context] = { 'skip-ci' => true } + end + response = gitaly_client_call(@repository.storage, :operation_service, - :user_create_branch, request, timeout: GitalyClient.long_timeout) + :user_create_branch, request, **params) branch = response.branch return unless branch diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb index ab9244253b7d8ec6bd96efa8249af40adba1b89e..486d5444495505789e68e4bfae0dbebc4f4b036c 100644 --- a/spec/lib/gitlab/git/repository_spec.rb +++ b/spec/lib/gitlab/git/repository_spec.rb @@ -2154,6 +2154,24 @@ def create_commit(blobs) end end + describe '#add_branch' do + let_it_be(:project) { create(:project, :repository) } + let(:repository) { project.repository.raw } + let(:branch_name) { "branch-to-create" } + + before do + project.add_developer(user) + end + + it "adds the branch and passes skip_ci to gitaly_operation_client" do + expect(repository.gitaly_operation_client).to receive(:user_create_branch) + .with(branch_name, user, 'HEAD', skip_ci: true) + .and_call_original + + repository.add_branch(branch_name, user: user, target: 'HEAD', skip_ci: true) + end + end + describe '#rm_branch' do let(:project) { create(:project, :repository) } let(:repository) { project.repository.raw } diff --git a/spec/lib/gitlab/gitaly_client/operation_service_spec.rb b/spec/lib/gitlab/gitaly_client/operation_service_spec.rb index ee921bf175f9b87b59cf0d24ddce9043c9f21a43..cff2af9cdcb9f60fffd5fc23e431025c463d7778 100644 --- a/spec/lib/gitlab/gitaly_client/operation_service_spec.rb +++ b/spec/lib/gitlab/gitaly_client/operation_service_spec.rb @@ -33,15 +33,33 @@ subject { client.user_create_branch(branch_name, user, start_point) } - it 'sends a user_create_branch message and returns a Gitlab::git::Branch' do + it 'sends a user_create_branch message and returns a Gitlab::Git::Branch' do expect_any_instance_of(Gitaly::OperationService::Stub) - .to receive(:user_create_branch).with(request, kind_of(Hash)) + .to receive(:user_create_branch).with(request, hash_including({ + metadata: hash_not_including("gitaly-client-context-bin") + })) .and_return(response) expect(subject.name).to eq(branch_name) expect(subject.dereferenced_target).to eq(commit) end + context 'when skip_ci: true' do + subject { client.user_create_branch(branch_name, user, start_point, skip_ci: true) } + + it 'passes skip-ci in gitaly_context' do + expect_any_instance_of(Gitaly::OperationService::Stub) + .to receive(:user_create_branch).with(request, hash_including({ + metadata: hash_including({ + "gitaly-client-context-bin" => '{"skip-ci":true}' + }) + })) + .and_return(response) + + expect(subject.name).to eq(branch_name) + end + end + context 'with structured errors' do context 'with CustomHookError' do let(:stdout) { nil } diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index 2ed9137f5a34401572a4202974e0ee19913e3e3a..5c17294727403f63091fb04caf731c8b0882453b 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -1799,7 +1799,7 @@ def expect_to_raise_storage_error it "calls Gitaly's OperationService" do expect_any_instance_of(Gitlab::GitalyClient::OperationService) - .to receive(:user_create_branch).with(branch_name, user, target) + .to receive(:user_create_branch).with(branch_name, user, target, skip_ci: false) .and_return(nil) subject @@ -1832,6 +1832,41 @@ def expect_to_raise_storage_error repository.add_branch(user, branch_name, target, expire_cache: false) end end + + context 'skip_ci' do + context 'when skip_ci: true' do + it 'passes skip-ci: true gitaly context' do + allow(::Gitlab::GitalyClient).to receive(:call).and_call_original + + repository.add_branch(user, branch_name, target, skip_ci: true) + + expect(::Gitlab::GitalyClient).to have_received(:call) + .with(anything, anything, :user_create_branch, anything, gitaly_context: { 'skip-ci' => true }, timeout: anything) + end + end + + context 'when skip_ci: false' do + it 'does not pass gitaly_context' do + allow(::Gitlab::GitalyClient).to receive(:call).and_call_original + + repository.add_branch(user, branch_name, target, skip_ci: false) + + expect(::Gitlab::GitalyClient).to have_received(:call) + .with(anything, anything, :user_create_branch, anything, timeout: anything) + end + end + + context 'when skip_ci not provided' do + it 'does not pass gitaly_context' do + allow(::Gitlab::GitalyClient).to receive(:call).and_call_original + + repository.add_branch(user, branch_name, target) + + expect(::Gitlab::GitalyClient).to have_received(:call) + .with(anything, anything, :user_create_branch, anything, timeout: anything) + end + end + end end shared_examples 'asymmetric cached method' do |method| diff --git a/spec/requests/api/internal/base_spec.rb b/spec/requests/api/internal/base_spec.rb index 933dcc25eacec5d3737b79a7e834496f085666a2..deb12856347388ded352fbbb761b6e262e1acf85 100644 --- a/spec/requests/api/internal/base_spec.rb +++ b/spec/requests/api/internal/base_spec.rb @@ -1470,7 +1470,11 @@ def request let(:gl_repository) { container.repository.gl_repository } let(:messages) { [] } - it 'executes PostReceiveService' do + it 'executes PostReceiveService passing in the gitaly_context' do + expect(PostReceiveService).to receive(:new) + .with(user, container.repository, anything, hash_including({ gitaly_context: anything })) + .and_call_original + subject expect(response).to have_gitlab_http_status(:ok) diff --git a/spec/services/ci/workloads/run_workload_service_spec.rb b/spec/services/ci/workloads/run_workload_service_spec.rb index 33da9775a198a1117e50b68f6c480508d5b54170..435b4b31dbb2713c15aa7ae8f2fd6ef985e94ade 100644 --- a/spec/services/ci/workloads/run_workload_service_spec.rb +++ b/spec/services/ci/workloads/run_workload_service_spec.rb @@ -19,9 +19,13 @@ } end + let(:create_branch) { false } + describe '#execute' do subject(:execute) do - described_class.new(project: project, current_user: user, source: source, workload: workload).execute + described_class + .new(project: project, current_user: user, source: source, workload: workload, create_branch: create_branch) + .execute end context 'when pipeline creation is success' do @@ -54,6 +58,21 @@ expect(build.variables.map(&:key)).not_to include('A_GROUP_VARIABLE') expect(build.variables.map(&:key)).not_to include('A_INSTANCE_VARIABLE') end + + context 'when create_branch: true' do + let(:create_branch) { true } + + it 'creates a new branch with skip_ci and manually runs the pipeline for that branch' do + expect(project.repository).to receive(:add_branch) + .with(user, match(%r{^workloads/\w+}), project.default_branch_or_main, skip_ci: true) + .and_call_original + + expect(execute).to be_success + + pipeline = execute.payload + expect(pipeline.ref).to match(%r{workloads/\w+}) + end + end end context 'with unsupported source' do diff --git a/spec/services/post_receive_service_spec.rb b/spec/services/post_receive_service_spec.rb index a22f7fa9edb091793f4f07e5782126e4a9281db9..026b4fed5675f8ad029d9480a2aebced9193f959 100644 --- a/spec/services/post_receive_service_spec.rb +++ b/spec/services/post_receive_service_spec.rb @@ -15,8 +15,9 @@ let(:gl_repository) { "project-#{project.id}" } let(:branch_name) { 'feature' } let(:reference_counter) { double('ReferenceCounter') } - let(:push_options) { ['ci.skip', 'another push option'] } + let(:push_options) { ['secret_push_protection.skip_all', 'another-ignored-option'] } let(:repository) { project.repository } + let(:gitaly_context) { {} } let(:changes) do "#{Gitlab::Git::SHA1_BLANK_SHA} 570e7b2abdd848b95f2f578043fc23bd6f6fd24d refs/heads/#{branch_name}" @@ -27,7 +28,8 @@ gl_repository: gl_repository, identifier: identifier, changes: changes, - push_options: push_options + push_options: push_options, + gitaly_context: gitaly_context } end @@ -72,7 +74,9 @@ it 'enqueues a PostReceive worker job' do expect(PostReceive).to receive(:perform_async) - .with(gl_repository, identifier, changes, { 'ci' => { 'skip' => true } }) + .with(gl_repository, identifier, changes, { + 'secret_push_protection' => { 'skip_all' => true } + }) subject end @@ -81,12 +85,41 @@ context 'when rename_post_receive_worker feature flag is enabled' do it 'enqueues a PostReceiveWorker worker job' do expect(Repositories::PostReceiveWorker).to receive(:perform_async) - .with(gl_repository, identifier, changes, { 'ci' => { 'skip' => true } }) + .with(gl_repository, identifier, changes, { + 'secret_push_protection' => { 'skip_all' => true } + }) subject end end + context 'when gitaly_context includes skip-ci' do + let(:gitaly_context) { { 'skip-ci' => 'true' } } + + it 'adds ci.skip to push options for PostReceiveWorker' do + expect(Repositories::PostReceiveWorker).to receive(:perform_async) + .with(gl_repository, identifier, changes, { + 'secret_push_protection' => { 'skip_all' => true }, + 'ci' => { 'skip' => true } + }) + + subject + end + + context 'when push_options are not present' do + let(:push_options) { nil } + + it 'only includes ci.skip in push options for PostReceiveWorker' do + expect(Repositories::PostReceiveWorker).to receive(:perform_async) + .with(gl_repository, identifier, changes, { + 'ci' => { 'skip' => true } + }) + + subject + end + end + end + it 'decreases the reference counter and returns the result' do expect(Gitlab::ReferenceCounter).to receive(:new).with(gl_repository) .and_return(reference_counter)