From 0152a8257e6f57d7fa2b884978a0b3775112f7f2 Mon Sep 17 00:00:00 2001 From: Surabhi Suman Date: Wed, 26 Nov 2025 11:10:10 +0530 Subject: [PATCH] Create internal git refs instead of branch for ci workloads ::Ci::Workload relies on git branch to create pipelines and use the branches for commits and merge requests. This change removes the dependency on workload branch and creates only git refs to create pipeline. Workload jobs can create branch as part of script execution Relates to: https://gitlab.com/gitlab-org/gitlab/-/work_items/577239 EE: true --- .../ci/workloads/workload_branch_service.rb | 47 ++++-- ...e_internal_refs_for_workload_pipelines.yml | 10 ++ .../duo_workflows/start_workflow_service.rb | 11 +- .../services/ai/flow_triggers/run_service.rb | 2 +- .../start_workflow_service_spec.rb | 27 +++- .../ai/flow_triggers/run_service_spec.rb | 96 +++++++----- .../ci/workloads/run_workload_service_spec.rb | 24 +++ .../workloads/workload_branch_service_spec.rb | 148 +++++++++++++----- 8 files changed, 265 insertions(+), 100 deletions(-) create mode 100644 config/feature_flags/wip/use_internal_refs_for_workload_pipelines.yml diff --git a/app/services/ci/workloads/workload_branch_service.rb b/app/services/ci/workloads/workload_branch_service.rb index b070b45ebe3aac..dda96790b421d9 100644 --- a/app/services/ci/workloads/workload_branch_service.rb +++ b/app/services/ci/workloads/workload_branch_service.rb @@ -10,28 +10,51 @@ def initialize(project:, source_branch:, current_user:) end def execute + workload_ref = "workloads/#{SecureRandom.hex[0..10]}" + source_ref = @project.repository.branch_exists?(@source_branch) ? @source_branch : default_branch + + if Feature.enabled?(:use_internal_refs_for_workload_pipelines, @project) + create_internal_refs(source_ref, workload_ref) + else + create_workload_branch(source_ref, workload_ref) + end + rescue Gitlab::Git::CommandError => e + Gitlab::ErrorTracking.track_exception(e) + ServiceResponse.error(message: 'Failed to create workload ref') + end + + private + + def create_internal_refs(source_ref, workload_ref) + source_sha = @project.repository.commit(source_ref)&.sha + return ServiceResponse.error(message: 'Source ref not found') unless source_sha + + workload_ref_path = "refs/#{workload_ref}" + if @project.repository.ref_exists?(workload_ref_path) + return ServiceResponse.error(message: 'Ref already exists') + end + + result = @project.repository.create_ref(source_sha, workload_ref_path) + return ServiceResponse.error(message: 'Error in git ref creation') unless result + + ServiceResponse.success(payload: { ref: workload_ref_path }) + end + + def create_workload_branch(source_ref, workload_ref) unless @current_user.can?(:push_code, @project) return ServiceResponse.error(message: 'You are not allowed to create branches in this project') end - branch_name = "workloads/#{SecureRandom.hex[0..10]}" - if @project.repository.branch_exists?(branch_name) + if @project.repository.branch_exists?(workload_ref) return ServiceResponse.error(message: 'Branch already exists') end - ref = @project.repository.branch_exists?(@source_branch) ? @source_branch : default_branch - - repo_branch = @project.repository.add_branch(@current_user, branch_name, ref, skip_ci: true) - return ServiceResponse.error(message: 'Error in git branch creation') unless repo_branch + result = @project.repository.add_branch(@current_user, workload_ref, source_ref, skip_ci: true) + return ServiceResponse.error(message: 'Failed to create branch') unless result - ServiceResponse.success(payload: { branch_name: branch_name }) - rescue Gitlab::Git::CommandError => e - Gitlab::ErrorTracking.track_exception(e) - ServiceResponse.error(message: 'Failed to create branch') + ServiceResponse.success(payload: { ref: workload_ref }) end - private - def default_branch @project.default_branch_or_main end diff --git a/config/feature_flags/wip/use_internal_refs_for_workload_pipelines.yml b/config/feature_flags/wip/use_internal_refs_for_workload_pipelines.yml new file mode 100644 index 00000000000000..d6f884824a988a --- /dev/null +++ b/config/feature_flags/wip/use_internal_refs_for_workload_pipelines.yml @@ -0,0 +1,10 @@ +--- +name: use_internal_refs_for_workload_pipelines +description: Feature flag to use internal git refs instead of branches for ci workloads +feature_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/577239 +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/215525 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/583179 +milestone: '18.7' +group: group::agent foundations +type: wip +default_enabled: false diff --git a/ee/app/services/ai/duo_workflows/start_workflow_service.rb b/ee/app/services/ai/duo_workflows/start_workflow_service.rb index 9a6ec69b81f100..bd4d6127aa35d1 100644 --- a/ee/app/services/ai/duo_workflows/start_workflow_service.rb +++ b/ee/app/services/ai/duo_workflows/start_workflow_service.rb @@ -46,7 +46,7 @@ def execute branch_response = create_workload_branch return branch_response unless branch_response.success? - @ref = branch_response.payload[:branch_name] + @ref = branch_response.payload[:ref] service = ::Ci::Workloads::RunWorkloadService.new( project: project, current_user: @workload_user, @@ -174,12 +174,17 @@ def main_workflow_commands %(echo $DUO_WORKFLOW_DEFINITION), %(echo $DUO_WORKFLOW_GOAL), %(echo $DUO_WORKFLOW_SOURCE_BRANCH), - %(git checkout $CI_WORKLOAD_REF), %(echo $DUO_WORKFLOW_FLOW_CONFIG), %(echo $DUO_WORKFLOW_FLOW_CONFIG_SCHEMA_VERSION), %(echo $DUO_WORKFLOW_ADDITIONAL_CONTEXT_CONTENT), %(echo Starting Workflow #{@workflow.id}) - ] + set_up_executor_commands + ] + branch_checkout_commands + set_up_executor_commands + end + + def branch_checkout_commands + return [] if Feature.enabled?(:use_internal_refs_for_workload_pipelines, project) + + [%(git checkout $CI_WORKLOAD_REF)] end def set_up_executor_commands diff --git a/ee/app/services/ai/flow_triggers/run_service.rb b/ee/app/services/ai/flow_triggers/run_service.rb index f88b7def76277a..733dff8b19e5f1 100644 --- a/ee/app/services/ai/flow_triggers/run_service.rb +++ b/ee/app/services/ai/flow_triggers/run_service.rb @@ -182,7 +182,7 @@ def branch_args branch_response = workload_branch_service.execute return branch_response unless branch_response.success? - ServiceResponse.success(payload: { ref: branch_response.payload[:branch_name] }) + ServiceResponse.success(payload: { ref: branch_response.payload[:ref] }) end def composite_identity_token diff --git a/ee/spec/services/ai/duo_workflows/start_workflow_service_spec.rb b/ee/spec/services/ai/duo_workflows/start_workflow_service_spec.rb index 9e8e9d58cb2394..36b1cf27f5833a 100644 --- a/ee/spec/services/ai/duo_workflows/start_workflow_service_spec.rb +++ b/ee/spec/services/ai/duo_workflows/start_workflow_service_spec.rb @@ -51,8 +51,7 @@ workload_id: workload_id) workload = Ci::Workloads::Workload.find_by_id([workload_id]) - expect(workload.branch_name).to start_with('workloads/') - expect(workload.branch_name).to start_with('workloads/') + expect(workload.branch_name).to start_with('refs/workloads/') end end @@ -860,7 +859,6 @@ def standard_context_content(parsed_context) %(echo $DUO_WORKFLOW_DEFINITION), %(echo $DUO_WORKFLOW_GOAL), %(echo $DUO_WORKFLOW_SOURCE_BRANCH), - %(git checkout $CI_WORKLOAD_REF), %(echo $DUO_WORKFLOW_FLOW_CONFIG), %(echo $DUO_WORKFLOW_FLOW_CONFIG_SCHEMA_VERSION), %(echo $DUO_WORKFLOW_ADDITIONAL_CONTEXT_CONTENT), @@ -1065,9 +1063,9 @@ def standard_context_content(parsed_context) expect(Ci::Workloads::RunWorkloadService).to have_received(:new) do |workload_definition:, **_kwargs| commands = workload_definition.commands expect(commands[0]).to eq('npm install') - expect(commands[1..8]).to eq(shared_main_commands) - expect(commands[9..12]).to eq(cli_install_commands) - expect(commands[13..]).to eq(wrapped_commands) + expect(commands[1..7]).to eq(shared_main_commands) + expect(commands[8..11]).to eq(cli_install_commands) + expect(commands[12..]).to eq(wrapped_commands) end end end @@ -1125,6 +1123,23 @@ def standard_context_content(parsed_context) it_behaves_like 'sandbox is disabled' end end + + context 'when feature flag use_internal_refs_for_workload_pipelines is disabled' do + before do + stub_feature_flags(use_internal_refs_for_workload_pipelines: false) + allow(duo_config).to receive(:setup_script).and_return([]) + end + + it 'prepends the checkout command' do + expect(Ci::Workloads::RunWorkloadService).to receive(:new) do |workload_definition:, **_kwargs| + commands = workload_definition.commands + expect(commands).to include('git checkout $CI_WORKLOAD_REF') + expect(commands.size).to eq(8) + end.and_call_original + + expect(execute).to be_success + end + end end context 'with cache configuration' do diff --git a/ee/spec/services/ai/flow_triggers/run_service_spec.rb b/ee/spec/services/ai/flow_triggers/run_service_spec.rb index bfea52d8dff788..77dfe4bcebe2de 100644 --- a/ee/spec/services/ai/flow_triggers/run_service_spec.rb +++ b/ee/spec/services/ai/flow_triggers/run_service_spec.rb @@ -630,52 +630,72 @@ def expected_gitlab_hostname let_it_be(:resource) { merge_request } - it 'creates workload branch from merge request source branch and passes ref to RunWorkloadService' do - workload_branch_service = instance_double(Ci::Workloads::WorkloadBranchService) - expect(Ci::Workloads::WorkloadBranchService).to receive(:new).with( - current_user: service_account, - project: project, - source_branch: 'feature-branch' - ).and_return(workload_branch_service) - expect(workload_branch_service).to receive(:execute).and_return( - ServiceResponse.success(payload: { branch_name: 'workloads/abc123' }) - ) + shared_examples 'passes workload ref to RunWorkloadService' do |ref_value| + it "creates workload ref from merge request source branch and passes ref #{ref_value} to RunWorkloadService" do + workload_branch_service = instance_double(Ci::Workloads::WorkloadBranchService) + expect(Ci::Workloads::WorkloadBranchService).to receive(:new).with( + current_user: service_account, + project: project, + source_branch: 'feature-branch' + ).and_return(workload_branch_service) + expect(workload_branch_service).to receive(:execute).and_return( + ServiceResponse.success(payload: { ref: ref_value }) + ) - expect(Ci::Workloads::RunWorkloadService).to receive(:new).with( - project: project, - current_user: service_account, - source: :duo_workflow, - workload_definition: an_instance_of(Ci::Workloads::WorkloadDefinition), - ci_variables_included: %w[API_KEY DATABASE_URL], - ref: 'workloads/abc123' - ).and_call_original + expect(Ci::Workloads::RunWorkloadService).to receive(:new).with( + project: project, + current_user: service_account, + source: :duo_workflow, + workload_definition: an_instance_of(Ci::Workloads::WorkloadDefinition), + ci_variables_included: %w[API_KEY DATABASE_URL], + ref: ref_value + ).and_call_original - service.execute(params) + service.execute(params) + end + end + + context 'with workload branch' do + it_behaves_like 'passes workload ref to RunWorkloadService', 'workloads/xyz789' + end + + context 'with workload internal_refs' do + it_behaves_like 'passes workload ref to RunWorkloadService', 'refs/workloads/123' end end context 'when resource is not a MergeRequest' do - it 'creates workload branch without source branch and passes ref to RunWorkloadService' do - workload_branch_service = instance_double(Ci::Workloads::WorkloadBranchService) - expect(Ci::Workloads::WorkloadBranchService).to receive(:new).with( - current_user: service_account, - project: project, - source_branch: nil - ).and_return(workload_branch_service) - expect(workload_branch_service).to receive(:execute).and_return( - ServiceResponse.success(payload: { branch_name: 'workloads/xyz789' }) - ) + shared_examples 'creates workload ref and passes ref to RunWorkloadService' do |ref_value| + it "creates workload ref without source branch and passes ref #{ref_value} to RunWorkloadService" do + workload_branch_service = instance_double(Ci::Workloads::WorkloadBranchService) + expect(Ci::Workloads::WorkloadBranchService).to receive(:new).with( + current_user: service_account, + project: project, + source_branch: nil + ).and_return(workload_branch_service) + expect(workload_branch_service).to receive(:execute).and_return( + ServiceResponse.success(payload: { ref: ref_value }) + ) - expect(Ci::Workloads::RunWorkloadService).to receive(:new).with( - project: project, - current_user: service_account, - source: :duo_workflow, - workload_definition: an_instance_of(Ci::Workloads::WorkloadDefinition), - ci_variables_included: %w[API_KEY DATABASE_URL], - ref: 'workloads/xyz789' - ).and_call_original + expect(Ci::Workloads::RunWorkloadService).to receive(:new).with( + project: project, + current_user: service_account, + source: :duo_workflow, + workload_definition: an_instance_of(Ci::Workloads::WorkloadDefinition), + ci_variables_included: %w[API_KEY DATABASE_URL], + ref: ref_value + ).and_call_original - service.execute(params) + service.execute(params) + end + end + + context 'with workload branch' do + it_behaves_like 'creates workload ref and passes ref to RunWorkloadService', 'workloads/xyz789' + end + + context 'with workload internal_refs' do + it_behaves_like 'creates workload ref and passes ref to RunWorkloadService', 'refs/workloads/123' end end diff --git a/spec/services/ci/workloads/run_workload_service_spec.rb b/spec/services/ci/workloads/run_workload_service_spec.rb index 9507224fc016d5..0490395a04675f 100644 --- a/spec/services/ci/workloads/run_workload_service_spec.rb +++ b/spec/services/ci/workloads/run_workload_service_spec.rb @@ -103,6 +103,30 @@ end end + context 'with internal refs for workloads' do + let(:ref) { 'refs/workloads/123' } + + before do + source_ref = project.default_branch_or_main + source_sha = project.repository.commit(source_ref)&.sha + project.repository.create_ref(source_sha, ref) + end + + it 'starts a pipeline to execute workload' do + expect_next_instance_of(Ci::CreatePipelineService, project, user, + hash_including(ref: ref)) do |pipeline_service| + expect(pipeline_service).to receive(:execute) + .and_call_original + end + result = execute + expect(result).to be_success + + workload = result.payload + expect(workload).to be_a(Ci::Workloads::Workload) + expect(workload.pipeline).to be_present + end + end + context 'with unsupported source' do let(:source) { :foo } diff --git a/spec/services/ci/workloads/workload_branch_service_spec.rb b/spec/services/ci/workloads/workload_branch_service_spec.rb index 4f0235b06003c0..b58d1a5a210198 100644 --- a/spec/services/ci/workloads/workload_branch_service_spec.rb +++ b/spec/services/ci/workloads/workload_branch_service_spec.rb @@ -16,111 +16,179 @@ allow(SecureRandom).to receive(:hex).and_return('abcdef12345') end - context 'when user does not have access to push code to the project' do - let(:user) { create(:user) } - - it 'returns an error response' do - result = execute - expect(result).to be_error - expect(result.message).to eq('You are not allowed to create branches in this project') - end - end - context 'when source branch exists' do before do project.repository.create_branch(source_branch, project.default_branch) end - it 'creates a new workload branch from the source branch' do - expected_branch_name = 'workloads/abcdef12345' + it 'creates a new workload ref from the source branch' do + workload_ref = 'refs/workloads/abcdef12345' + source_sha = project.repository.commit(source_branch).sha - expect(project.repository).to receive(:add_branch) - .with(user, expected_branch_name, source_branch, skip_ci: true) - .and_return(double) + expect(project.repository).to receive(:create_ref) + .with(source_sha, workload_ref) + .and_return(true) result = execute expect(result).to be_success - expect(result.payload[:branch_name]).to eq(expected_branch_name) + expect(result.payload[:ref]).to eq(workload_ref) end end context 'when source branch does not exist' do let(:source_branch) { 'non-existent-branch' } - it 'creates a new workload branch from the default branch' do - expected_branch_name = 'workloads/abcdef12345' + it 'creates a new workload ref from the default branch' do + workload_ref = 'refs/workloads/abcdef12345' + default_sha = project.repository.commit(project.default_branch_or_main).sha - expect(project.repository).to receive(:add_branch) - .with(user, expected_branch_name, project.default_branch_or_main, skip_ci: true) - .and_return(double) + expect(project.repository).to receive(:create_ref) + .with(default_sha, workload_ref) + .and_return(true) result = execute expect(result).to be_success - expect(result.payload[:branch_name]).to eq(expected_branch_name) + expect(result.payload[:ref]).to eq(workload_ref) end end context 'when source branch is nil' do let(:source_branch) { nil } - it 'creates a new workload branch from the default branch' do - expected_branch_name = 'workloads/abcdef12345' + it 'creates a new workload ref from the default branch' do + workload_ref = 'refs/workloads/abcdef12345' + default_sha = project.repository.commit(project.default_branch_or_main).sha - expect(project.repository).to receive(:add_branch) - .with(user, expected_branch_name, project.default_branch_or_main, skip_ci: true) - .and_return(double) + expect(project.repository).to receive(:create_ref) + .with(default_sha, workload_ref) + .and_return(true) result = execute expect(result).to be_success - expect(result.payload[:branch_name]).to eq(expected_branch_name) + expect(result.payload[:ref]).to eq(workload_ref) end end - context 'when git branch creation fails' do + context 'when git ref creation fails' do before do - allow(project.repository).to receive(:add_branch).and_return(nil) + allow(project.repository).to receive(:create_ref).and_return(nil) end it 'returns an error response' do result = execute expect(result).to be_error - expect(result.message).to eq('Error in git branch creation') + expect(result.message).to eq('Error in git ref creation') end end - context 'when branch name already exists' do + context 'when ref already exists' do before do - allow(project.repository).to receive(:branch_exists?) - .with(match(%r{^workloads/\w+})) - .and_return(true) + allow(project.repository).to receive(:ref_exists?) + .with(match(%r{^refs/workloads/\w+})) + .and_return(true) end it 'returns an error response' do result = execute expect(result).to be_error - expect(result.message).to eq('Branch already exists') + expect(result.message).to eq('Ref already exists') + end + end + + context 'when source ref is not found' do + before do + allow(project.repository).to receive(:commit).and_return(nil) + end + + it 'returns an error response' do + result = execute + + expect(result).to be_error + expect(result.message).to eq('Source ref not found') end end context 'when git command raises an error' do before do - allow(project.repository).to receive(:add_branch) - .and_raise(Gitlab::Git::CommandError.new('git error')) + allow(project.repository).to receive(:create_ref) + .and_raise(Gitlab::Git::CommandError.new('git error')) end it 'tracks the exception and returns an error response' do expect(Gitlab::ErrorTracking).to receive(:track_exception) - .with(instance_of(Gitlab::Git::CommandError)) + .with(instance_of(Gitlab::Git::CommandError)) result = execute expect(result).to be_error - expect(result.message).to eq('Failed to create branch') + expect(result.message).to eq('Failed to create workload ref') + end + end + + context 'when feature flag is disabled' do + before do + stub_feature_flags(use_internal_refs_for_workload_pipelines: false) + end + + context 'when user does not have access to push code to the project' do + let(:user) { create(:user) } + + it 'returns an error response' do + result = execute + expect(result).to be_error + expect(result.message).to eq('You are not allowed to create branches in this project') + end + end + + context 'when git branch creation fails' do + before do + allow(project.repository).to receive(:add_branch).and_return(nil) + end + + it 'returns an error response' do + result = execute + + expect(result).to be_error + expect(result.message).to eq('Failed to create branch') + end + end + + context 'when branch name already exists' do + before do + allow(project.repository).to receive(:branch_exists?).with("feature-branch").and_return(true) + allow(project.repository).to receive(:branch_exists?) + .with(match(%r{^workloads/\w+})) + .and_return(true) + end + + it 'returns an error response' do + result = execute + + expect(result).to be_error + expect(result.message).to eq('Branch already exists') + end + end + + context 'when git command raises an error' do + before do + allow(project.repository).to receive(:add_branch) + .and_raise(Gitlab::Git::CommandError.new('git error')) + end + + it 'tracks the exception and returns an error response' do + expect(Gitlab::ErrorTracking).to receive(:track_exception) + .with(instance_of(Gitlab::Git::CommandError)) + + result = execute + + expect(result).to be_error + expect(result.message).to eq('Failed to create workload ref') + end end end end -- GitLab