diff --git a/app/graphql/mutations/ci/job/retry.rb b/app/graphql/mutations/ci/job/retry.rb index 50e9c51c9e7078575b566d7df4c1359205813a0e..bfb9b902cc583055b4b1137e25d73ff526f88e69 100644 --- a/app/graphql/mutations/ci/job/retry.rb +++ b/app/graphql/mutations/ci/job/retry.rb @@ -11,13 +11,20 @@ class Retry < Base null: true, description: 'Job after the mutation.' + argument :variables, [::Types::Ci::VariableInputType], + required: false, + default_value: [], + replace_null_with_default: true, + description: 'Variables to use when retrying a manual job.' + authorize :update_build - def resolve(id:) + def resolve(id:, variables:) job = authorized_find!(id: id) project = job.project + variables = variables.map(&:to_h) - response = ::Ci::RetryJobService.new(project, current_user).execute(job) + response = ::Ci::RetryJobService.new(project, current_user).execute(job, variables: variables) if response.success? { diff --git a/app/graphql/types/ci/job_type.rb b/app/graphql/types/ci/job_type.rb index 3444c76da0860cfb60942ee0021054cc1acd13f3..4ea9a016e74e4edd289258bd8bccb0b64c241c48 100644 --- a/app/graphql/types/ci/job_type.rb +++ b/app/graphql/types/ci/job_type.rb @@ -194,7 +194,7 @@ def triggered end def manual_variables - if object.manual? && object.respond_to?(:job_variables) + if object.action? && object.respond_to?(:job_variables) object.job_variables else [] diff --git a/app/graphql/types/ci/variable_input_type.rb b/app/graphql/types/ci/variable_input_type.rb new file mode 100644 index 0000000000000000000000000000000000000000..193ca6ffe4e2f3118673f55081d17b7bb0abf709 --- /dev/null +++ b/app/graphql/types/ci/variable_input_type.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +module Types + module Ci + class VariableInputType < BaseInputObject + graphql_name 'CiVariableInput' + description 'Attributes for defining a CI/CD variable.' + + argument :key, GraphQL::Types::String, description: 'Name of the variable.' + argument :value, GraphQL::Types::String, description: 'Value of the variable.' + end + end +end diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index 2b72a4c4c89c458d4dfa497cf8be2f89f4b21f34..78212bb51f4030e9ad14dec0503faf96f2c2e30a 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -1152,6 +1152,17 @@ def each_report(report_types) end end + def clone(current_user:, new_job_variables_attributes: []) + new_build = super + + if action? && new_job_variables_attributes.any? + new_build.job_variables = [] + new_build.job_variables_attributes = new_job_variables_attributes + end + + new_build + end + protected def run_status_commit_hooks! diff --git a/app/models/ci/processable.rb b/app/models/ci/processable.rb index f666629c8fdc5f960c120bd13ff72656928894e4..a2ff49077be14d1ad0dc839a448080f65b7cb740 100644 --- a/app/models/ci/processable.rb +++ b/app/models/ci/processable.rb @@ -101,7 +101,7 @@ def self.populate_scheduling_type! :merge_train_pipeline?, to: :pipeline - def clone(current_user:) + def clone(current_user:, new_job_variables_attributes: []) new_attributes = self.class.clone_accessors.to_h do |attribute| [attribute, public_send(attribute)] # rubocop:disable GitlabSecurity/PublicSend end diff --git a/app/services/ci/retry_job_service.rb b/app/services/ci/retry_job_service.rb index e0ced3d0197759acceb8e7f5491bd882657517fb..25bda8a6380e0830589be79f40fa628e28599a7c 100644 --- a/app/services/ci/retry_job_service.rb +++ b/app/services/ci/retry_job_service.rb @@ -4,10 +4,10 @@ module Ci class RetryJobService < ::BaseService include Gitlab::Utils::StrongMemoize - def execute(job) + def execute(job, variables: []) if job.retryable? job.ensure_scheduling_type! - new_job = retry_job(job) + new_job = retry_job(job, variables: variables) ServiceResponse.success(payload: { job: new_job }) else @@ -19,7 +19,7 @@ def execute(job) end # rubocop: disable CodeReuse/ActiveRecord - def clone!(job) + def clone!(job, variables: []) # Cloning a job requires a strict type check to ensure # the attributes being used for the clone are taken straight # from the model and not overridden by other abstractions. @@ -27,7 +27,7 @@ def clone!(job) check_access!(job) - new_job = job.clone(current_user: current_user) + new_job = job.clone(current_user: current_user, new_job_variables_attributes: variables) new_job.run_after_commit do ::Ci::CopyCrossDatabaseAssociationsService.new.execute(job, new_job) @@ -55,8 +55,8 @@ def clone!(job) def check_assignable_runners!(job); end - def retry_job(job) - clone!(job).tap do |new_job| + def retry_job(job, variables: []) + clone!(job, variables: variables).tap do |new_job| check_assignable_runners!(new_job) if new_job.is_a?(Ci::Build) next if new_job.failed? diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index 4e021da238de27f551863c5c7baa43b6738bee5a..61bedf09ffbd2226187c528a1c44f630e5d1d75a 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -3510,6 +3510,7 @@ Input type: `JobRetryInput` | ---- | ---- | ----------- | | <a id="mutationjobretryclientmutationid"></a>`clientMutationId` | [`String`](#string) | A unique identifier for the client performing the mutation. | | <a id="mutationjobretryid"></a>`id` | [`CiBuildID!`](#cibuildid) | ID of the job to mutate. | +| <a id="mutationjobretryvariables"></a>`variables` | [`[CiVariableInput!]`](#civariableinput) | Variables to use when retrying a manual job. | #### Fields @@ -22104,6 +22105,17 @@ Field that are available while modifying the custom mapping attributes for an HT | <a id="boardissueinputweight"></a>`weight` | [`String`](#string) | Filter by weight. | | <a id="boardissueinputweightwildcardid"></a>`weightWildcardId` | [`WeightWildcardId`](#weightwildcardid) | Filter by weight ID wildcard. Incompatible with weight. | +### `CiVariableInput` + +Attributes for defining a CI/CD variable. + +#### Arguments + +| Name | Type | Description | +| ---- | ---- | ----------- | +| <a id="civariableinputkey"></a>`key` | [`String!`](#string) | Name of the variable. | +| <a id="civariableinputvalue"></a>`value` | [`String!`](#string) | Value of the variable. | + ### `CommitAction` #### Arguments diff --git a/spec/graphql/types/ci/variable_input_type_spec.rb b/spec/graphql/types/ci/variable_input_type_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..a56b6287deec3b3ec6a2898343fdd2ba476bf656 --- /dev/null +++ b/spec/graphql/types/ci/variable_input_type_spec.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe GitlabSchema.types['CiVariableInput'] do + include GraphqlHelpers + + it 'has the correct arguments' do + expect(described_class.arguments.keys).to match_array(%w[key value]) + end +end diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index 6cdc0ef9d9bfd7d7ec514d6ca21d8b04afa1c4b6..c3ae24c20c9e908f0a9595703b184ad328a49c3f 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -5583,4 +5583,58 @@ def run_job_without_exception let!(:model) { create(:ci_build, user: create(:user)) } let!(:parent) { model.user } end + + describe '#clone' do + let_it_be(:user) { FactoryBot.build(:user) } + + context 'when given new job variables' do + context 'when the cloned build has an action' do + it 'applies the new job variables' do + build = create(:ci_build, :actionable) + create(:ci_job_variable, job: build, key: 'TEST_KEY', value: 'old value') + create(:ci_job_variable, job: build, key: 'OLD_KEY', value: 'i will not live for long') + + new_build = build.clone(current_user: user, new_job_variables_attributes: [ + { key: 'TEST_KEY', value: 'new value' }, + { key: 'NEW_KEY', value: 'exciting new value' } + ]) + new_build.save! + + expect(new_build.job_variables.count).to be(2) + expect(new_build.job_variables.pluck(:key)).to contain_exactly('TEST_KEY', 'NEW_KEY') + expect(new_build.job_variables.map(&:value)).to contain_exactly('new value', 'exciting new value') + end + end + + context 'when the cloned build does not have an action' do + it 'applies the old job variables' do + build = create(:ci_build) + create(:ci_job_variable, job: build, key: 'TEST_KEY', value: 'old value') + + new_build = build.clone(current_user: user, new_job_variables_attributes: [ + { key: 'TEST_KEY', value: 'new value' } + ]) + new_build.save! + + expect(new_build.job_variables.count).to be(1) + expect(new_build.job_variables.pluck(:key)).to contain_exactly('TEST_KEY') + expect(new_build.job_variables.map(&:value)).to contain_exactly('old value') + end + end + end + + context 'when not given new job variables' do + it 'applies the old job variables' do + build = create(:ci_build) + create(:ci_job_variable, job: build, key: 'TEST_KEY', value: 'old value') + + new_build = build.clone(current_user: user) + new_build.save! + + expect(new_build.job_variables.count).to be(1) + expect(new_build.job_variables.pluck(:key)).to contain_exactly('TEST_KEY') + expect(new_build.job_variables.map(&:value)).to contain_exactly('old value') + end + end + end end diff --git a/spec/requests/api/graphql/ci/manual_variables_spec.rb b/spec/requests/api/graphql/ci/manual_variables_spec.rb index b7aa76511a3c25743b15a4ad5fd41a18e55b0dfd..a15bac2b8bd491509f8b8e0932400c97aabd9b56 100644 --- a/spec/requests/api/graphql/ci/manual_variables_spec.rb +++ b/spec/requests/api/graphql/ci/manual_variables_spec.rb @@ -35,8 +35,8 @@ project.add_maintainer(user) end - it 'returns the manual variables for the jobs' do - job = create(:ci_build, :manual, pipeline: pipeline) + it 'returns the manual variables for actionable jobs' do + job = create(:ci_build, :actionable, pipeline: pipeline) create(:ci_job_variable, key: 'MANUAL_TEST_VAR', job: job) post_graphql(query, current_user: user) @@ -46,8 +46,8 @@ expect(variables_data.map { |var| var['key'] }).to match_array(['MANUAL_TEST_VAR']) end - it 'does not fetch job variables for jobs that are not manual' do - job = create(:ci_build, pipeline: pipeline) + it 'does not fetch job variables for jobs that are not actionable' do + job = create(:ci_build, pipeline: pipeline, status: :manual) create(:ci_job_variable, key: 'THIS_VAR_WOULD_SHOULD_NEVER_EXIST', job: job) post_graphql(query, current_user: user) diff --git a/spec/requests/api/graphql/mutations/ci/job_retry_spec.rb b/spec/requests/api/graphql/mutations/ci/job_retry_spec.rb index ef640183bd8f7f4a1771ef93159ad1e53f6da4a8..8cf559a372ae3cc71e0f032341dc33131315a3d3 100644 --- a/spec/requests/api/graphql/mutations/ci/job_retry_spec.rb +++ b/spec/requests/api/graphql/mutations/ci/job_retry_spec.rb @@ -47,6 +47,38 @@ expect(new_job).not_to be_retried end + context 'when given CI variables' do + let(:job) { create(:ci_build, :success, :actionable, pipeline: pipeline, name: 'build') } + + let(:mutation) do + variables = { + id: job.to_global_id.to_s, + variables: { key: 'MANUAL_VAR', value: 'test manual var' } + } + + graphql_mutation(:job_retry, variables, + <<-QL + errors + job { + id + } + QL + ) + end + + it 'applies them to a retried manual job' do + post_graphql_mutation(mutation, current_user: user) + + expect(response).to have_gitlab_http_status(:success) + + new_job_id = GitlabSchema.object_from_id(mutation_response['job']['id']).sync.id + new_job = ::Ci::Build.find(new_job_id) + expect(new_job.job_variables.count).to be(1) + expect(new_job.job_variables.first.key).to eq('MANUAL_VAR') + expect(new_job.job_variables.first.value).to eq('test manual var') + end + end + context 'when the job is not retryable' do let(:job) { create(:ci_build, :retried, pipeline: pipeline) } diff --git a/spec/services/ci/retry_job_service_spec.rb b/spec/services/ci/retry_job_service_spec.rb index f042471bd1f22428de23c9d780027e5c47a6660a..b14e4187c7a4c33a5b239a00f168a9a36829934c 100644 --- a/spec/services/ci/retry_job_service_spec.rb +++ b/spec/services/ci/retry_job_service_spec.rb @@ -17,6 +17,7 @@ name: 'test') end + let(:job_variables_attributes) { [{ key: 'MANUAL_VAR', value: 'manual test var' }] } let(:user) { developer } let(:service) { described_class.new(project, user) } @@ -206,6 +207,14 @@ include_context 'retryable bridge' it_behaves_like 'clones the job' + + context 'when given variables' do + let(:new_job) { service.clone!(job, variables: job_variables_attributes) } + + it 'does not give variables to the new bridge' do + expect { new_job }.not_to raise_error + end + end end context 'when the job to be cloned is a build' do @@ -250,6 +259,28 @@ expect { new_job }.not_to change { Environment.count } end end + + context 'when given variables' do + let(:new_job) { service.clone!(job, variables: job_variables_attributes) } + + context 'when the build is actionable' do + let_it_be_with_refind(:job) { create(:ci_build, :actionable, pipeline: pipeline) } + + it 'gives variables to the new build' do + expect(new_job.job_variables.count).to be(1) + expect(new_job.job_variables.first.key).to eq('MANUAL_VAR') + expect(new_job.job_variables.first.value).to eq('manual test var') + end + end + + context 'when the build is not actionable' do + let_it_be_with_refind(:job) { create(:ci_build, pipeline: pipeline) } + + it 'does not give variables to the new build' do + expect(new_job.job_variables.count).to be_zero + end + end + end end end @@ -260,6 +291,14 @@ include_context 'retryable bridge' it_behaves_like 'retries the job' + + context 'when given variables' do + let(:new_job) { service.clone!(job, variables: job_variables_attributes) } + + it 'does not give variables to the new bridge' do + expect { new_job }.not_to raise_error + end + end end context 'when the job to be retried is a build' do @@ -288,6 +327,28 @@ expect { service.execute(job) }.not_to exceed_all_query_limit(control_count) end end + + context 'when given variables' do + let(:new_job) { service.clone!(job, variables: job_variables_attributes) } + + context 'when the build is actionable' do + let_it_be_with_refind(:job) { create(:ci_build, :actionable, pipeline: pipeline) } + + it 'gives variables to the new build' do + expect(new_job.job_variables.count).to be(1) + expect(new_job.job_variables.first.key).to eq('MANUAL_VAR') + expect(new_job.job_variables.first.value).to eq('manual test var') + end + end + + context 'when the build is not actionable' do + let_it_be_with_refind(:job) { create(:ci_build, pipeline: pipeline) } + + it 'does not give variables to the new build' do + expect(new_job.job_variables.count).to be_zero + end + end + end end end end