From 4c247fac80f789b33f902abb6d39c587b4c55a50 Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Sat, 29 Jan 2022 03:15:22 +0000 Subject: [PATCH] Add latest changes from gitlab-org/gitlab@master --- app/finders/ci/jobs_finder.rb | 14 ++++-- .../resolvers/ci/runner_jobs_resolver.rb | 45 +++++++++++++++++ .../resolvers/project_jobs_resolver.rb | 3 +- app/graphql/types/ci/runner_type.rb | 4 ++ app/policies/ci/runner_policy.rb | 4 ++ doc/api/graphql/reference/index.md | 16 ++++++ .../import_export/base/object_builder.rb | 8 ++- .../import_export/group/object_builder.rb | 6 --- .../import_export/project/object_builder.rb | 8 --- spec/finders/ci/jobs_finder_spec.rb | 37 ++++++++++++++ .../resolvers/ci/runner_jobs_resolver_spec.rb | 49 +++++++++++++++++++ spec/graphql/types/ci/runner_type_spec.rb | 2 +- spec/requests/api/graphql/ci/runner_spec.rb | 22 +++++++++ 13 files changed, 198 insertions(+), 20 deletions(-) create mode 100644 app/graphql/resolvers/ci/runner_jobs_resolver.rb create mode 100644 spec/graphql/resolvers/ci/runner_jobs_resolver_spec.rb diff --git a/app/finders/ci/jobs_finder.rb b/app/finders/ci/jobs_finder.rb index 4408c9cdb6d..5fc9c0e1778 100644 --- a/app/finders/ci/jobs_finder.rb +++ b/app/finders/ci/jobs_finder.rb @@ -4,10 +4,11 @@ module Ci class JobsFinder include Gitlab::Allowable - def initialize(current_user:, pipeline: nil, project: nil, params: {}, type: ::Ci::Build) + def initialize(current_user:, pipeline: nil, project: nil, runner: nil, params: {}, type: ::Ci::Build) @pipeline = pipeline @current_user = current_user @project = project + @runner = runner @params = params @type = type raise ArgumentError 'type must be a subclass of Ci::Processable' unless type < ::Ci::Processable @@ -22,10 +23,10 @@ module Ci private - attr_reader :current_user, :pipeline, :project, :params, :type + attr_reader :current_user, :pipeline, :project, :runner, :params, :type def init_collection - pipeline_jobs || project_jobs || all_jobs + pipeline_jobs || project_jobs || runner_jobs || all_jobs end def all_jobs @@ -34,6 +35,13 @@ module Ci type.all end + def runner_jobs + return unless runner + raise Gitlab::Access::AccessDeniedError unless can?(current_user, :read_builds, runner) + + jobs_by_type(runner, type).relevant + end + def project_jobs return unless project raise Gitlab::Access::AccessDeniedError unless can?(current_user, :read_build, project) diff --git a/app/graphql/resolvers/ci/runner_jobs_resolver.rb b/app/graphql/resolvers/ci/runner_jobs_resolver.rb new file mode 100644 index 00000000000..2f6ca09d031 --- /dev/null +++ b/app/graphql/resolvers/ci/runner_jobs_resolver.rb @@ -0,0 +1,45 @@ +# frozen_string_literal: true + +module Resolvers + module Ci + class RunnerJobsResolver < BaseResolver + include Gitlab::Graphql::Authorize::AuthorizeResource + include LooksAhead + + type ::Types::Ci::JobType.connection_type, null: true + authorize :read_builds + authorizes_object! + + argument :statuses, [::Types::Ci::JobStatusEnum], + required: false, + description: 'Filter jobs by status.' + + alias_method :runner, :object + + def ready?(**args) + context[self.class] ||= { executions: 0 } + context[self.class][:executions] += 1 + + raise GraphQL::ExecutionError, "Jobs can be requested for only one runner at a time" if context[self.class][:executions] > 1 + + super + end + + def resolve_with_lookahead(statuses: nil) + jobs = ::Ci::JobsFinder.new(current_user: current_user, runner: runner, params: { scope: statuses }).execute + + apply_lookahead(jobs) + end + + private + + def preloads + { + previous_stage_jobs_and_needs: [:needs, :pipeline], + artifacts: [:job_artifacts], + pipeline: [:user] + } + end + end + end +end diff --git a/app/graphql/resolvers/project_jobs_resolver.rb b/app/graphql/resolvers/project_jobs_resolver.rb index 8a2693ee46b..b09158d475d 100644 --- a/app/graphql/resolvers/project_jobs_resolver.rb +++ b/app/graphql/resolvers/project_jobs_resolver.rb @@ -18,7 +18,8 @@ module Resolvers def ready?(**args) context[self.class] ||= { executions: 0 } context[self.class][:executions] += 1 - raise GraphQL::ExecutionError, "Jobs can only be requested for one project at a time" if context[self.class][:executions] > 1 + + raise GraphQL::ExecutionError, "Jobs can be requested for only one project at a time" if context[self.class][:executions] > 1 super end diff --git a/app/graphql/types/ci/runner_type.rb b/app/graphql/types/ci/runner_type.rb index e3f04ec5814..e13b93dcf0e 100644 --- a/app/graphql/types/ci/runner_type.rb +++ b/app/graphql/types/ci/runner_type.rb @@ -70,6 +70,10 @@ module Types description: 'Groups the runner is associated with. For group runners only.' field :projects, ::Types::ProjectType.connection_type, null: true, description: 'Projects the runner is associated with. For project runners only.' + field :jobs, ::Types::Ci::JobType.connection_type, null: true, + description: 'Jobs assigned to the runner.', + authorize: :read_builds, + resolver: ::Resolvers::Ci::RunnerJobsResolver def job_count # We limit to 1 above the JOB_COUNT_LIMIT to indicate that more items exist after JOB_COUNT_LIMIT diff --git a/app/policies/ci/runner_policy.rb b/app/policies/ci/runner_policy.rb index 43478cf36c2..bdbe7021276 100644 --- a/app/policies/ci/runner_policy.rb +++ b/app/policies/ci/runner_policy.rb @@ -11,6 +11,10 @@ module Ci rule { anonymous }.prevent_all + rule { admin }.policy do + enable :read_builds + end + rule { admin | owned_runner }.policy do enable :assign_runner enable :read_runner diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index 74b871adb20..d7dd0f26f3f 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -9085,6 +9085,22 @@ Represents the total number of issues and their weights for a particular day. #### Fields with arguments +##### `CiRunner.jobs` + +Jobs assigned to the runner. + +Returns [`CiJobConnection`](#cijobconnection). + +This field returns a [connection](#connections). It accepts the +four standard [pagination arguments](#connection-pagination-arguments): +`before: String`, `after: String`, `first: Int`, `last: Int`. + +###### Arguments + +| Name | Type | Description | +| ---- | ---- | ----------- | +| `statuses` | [`[CiJobStatus!]`](#cijobstatus) | Filter jobs by status. | + ##### `CiRunner.status` Status of the runner. diff --git a/lib/gitlab/import_export/base/object_builder.rb b/lib/gitlab/import_export/base/object_builder.rb index 5e9c8292c1e..7dee0f783cc 100644 --- a/lib/gitlab/import_export/base/object_builder.rb +++ b/lib/gitlab/import_export/base/object_builder.rb @@ -31,12 +31,18 @@ module Gitlab def find find_with_cache do - find_object || klass.create(prepare_attributes) + find_object || create_object end end protected + def create_object + klass.transaction do + klass.create(prepare_attributes) + end + end + def where_clauses raise NotImplementedError end diff --git a/lib/gitlab/import_export/group/object_builder.rb b/lib/gitlab/import_export/group/object_builder.rb index e171a31348e..43cc7a78a61 100644 --- a/lib/gitlab/import_export/group/object_builder.rb +++ b/lib/gitlab/import_export/group/object_builder.rb @@ -9,12 +9,6 @@ module Gitlab # `Group::ObjectBuilder.build(Label, label_attributes)` # finds or initializes a label with the given attributes. class ObjectBuilder < Base::ObjectBuilder - def self.build(*args) - ::Group.transaction do - super - end - end - def initialize(klass, attributes) super diff --git a/lib/gitlab/import_export/project/object_builder.rb b/lib/gitlab/import_export/project/object_builder.rb index 1baad4d3aaa..8e272e5f431 100644 --- a/lib/gitlab/import_export/project/object_builder.rb +++ b/lib/gitlab/import_export/project/object_builder.rb @@ -13,14 +13,6 @@ module Gitlab # # It also adds some logic around Group Labels/Milestones for edge cases. class ObjectBuilder < Base::ObjectBuilder - def self.build(*args) - ::Gitlab::Database::QueryAnalyzers::PreventCrossDatabaseModification.allow_cross_database_modification_within_transaction(url: 'https://gitlab.com/gitlab-org/gitlab/-/issues/350091') do - ::Project.transaction do - super - end - end - end - def initialize(klass, attributes) super diff --git a/spec/finders/ci/jobs_finder_spec.rb b/spec/finders/ci/jobs_finder_spec.rb index ab056dd26e8..959716b1fd3 100644 --- a/spec/finders/ci/jobs_finder_spec.rb +++ b/spec/finders/ci/jobs_finder_spec.rb @@ -126,4 +126,41 @@ RSpec.describe Ci::JobsFinder, '#execute' do end end end + + context 'a runner is present' do + let_it_be(:runner) { create(:ci_runner, :project, projects: [project]) } + let_it_be(:job_4) { create(:ci_build, :success, runner: runner) } + + subject { described_class.new(current_user: user, runner: runner, params: params).execute } + + context 'user has access to the runner', :enable_admin_mode do + let(:user) { admin } + + it 'returns jobs for the specified project' do + expect(subject).to match_array([job_4]) + end + end + + context 'user has no access to project builds' do + let_it_be(:guest) { create(:user) } + + let(:user) { guest } + + before do + project.add_guest(guest) + end + + it 'returns no jobs' do + expect(subject).to be_empty + end + end + + context 'without user' do + let(:user) { nil } + + it 'returns no jobs' do + expect(subject).to be_empty + end + end + end end diff --git a/spec/graphql/resolvers/ci/runner_jobs_resolver_spec.rb b/spec/graphql/resolvers/ci/runner_jobs_resolver_spec.rb new file mode 100644 index 00000000000..53b673e255b --- /dev/null +++ b/spec/graphql/resolvers/ci/runner_jobs_resolver_spec.rb @@ -0,0 +1,49 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Resolvers::Ci::RunnerJobsResolver do + include GraphqlHelpers + + let_it_be(:project) { create(:project, :repository) } + let_it_be(:pipeline) { create(:ci_pipeline, project: project) } + let_it_be(:irrelevant_pipeline) { create(:ci_pipeline, project: project) } + + let!(:build_one) { create(:ci_build, :success, name: 'Build One', runner: runner, pipeline: pipeline) } + let!(:build_two) { create(:ci_build, :success, name: 'Build Two', runner: runner, pipeline: pipeline) } + let!(:build_three) { create(:ci_build, :failed, name: 'Build Three', runner: runner, pipeline: pipeline) } + let!(:irrelevant_build) { create(:ci_build, name: 'Irrelevant Build', pipeline: irrelevant_pipeline)} + + let(:args) { {} } + let(:runner) { create(:ci_runner, :project, projects: [project]) } + + subject { resolve_jobs(args) } + + describe '#resolve' do + context 'with authorized user', :enable_admin_mode do + let(:current_user) { create(:user, :admin) } + + context 'with statuses argument' do + let(:args) { { statuses: [Types::Ci::JobStatusEnum.coerce_isolated_input('SUCCESS')] } } + + it { is_expected.to contain_exactly(build_one, build_two) } + end + + context 'without statuses argument' do + it { is_expected.to contain_exactly(build_one, build_two, build_three) } + end + end + + context 'with unauthorized user' do + let(:current_user) { nil } + + it { is_expected.to be_nil } + end + end + + private + + def resolve_jobs(args = {}, context = { current_user: current_user }) + resolve(described_class, obj: runner, args: args, ctx: context) + end +end diff --git a/spec/graphql/types/ci/runner_type_spec.rb b/spec/graphql/types/ci/runner_type_spec.rb index 3047817a952..77eac658c19 100644 --- a/spec/graphql/types/ci/runner_type_spec.rb +++ b/spec/graphql/types/ci/runner_type_spec.rb @@ -12,7 +12,7 @@ RSpec.describe GitlabSchema.types['CiRunner'] do id description created_at contacted_at maximum_timeout access_level active paused status version short_sha revision locked run_untagged ip_address runner_type tag_list project_count job_count admin_url edit_admin_url user_permissions executor_name - groups projects + groups projects jobs ] expect(described_class).to include_graphql_fields(*expected_fields) diff --git a/spec/requests/api/graphql/ci/runner_spec.rb b/spec/requests/api/graphql/ci/runner_spec.rb index 647d64002ef..fa16b9e1ddd 100644 --- a/spec/requests/api/graphql/ci/runner_spec.rb +++ b/spec/requests/api/graphql/ci/runner_spec.rb @@ -77,6 +77,7 @@ RSpec.describe 'Query.runner(id)' do 'runnerType' => runner.instance_type? ? 'INSTANCE_TYPE' : 'PROJECT_TYPE', 'executorName' => runner.executor_type&.dasherize, 'jobCount' => 0, + 'jobs' => a_hash_including("count" => 0, "nodes" => [], "pageInfo" => anything), 'projectCount' => nil, 'adminUrl' => "http://localhost/admin/runners/#{runner.id}", 'userPermissions' => { @@ -308,12 +309,25 @@ RSpec.describe 'Query.runner(id)' do let!(:job) { create(:ci_build, runner: project_runner1) } context 'requesting projects and counts for projects and jobs' do + let(:jobs_fragment) do + %( + jobs { + count + nodes { + id + status + } + } + ) + end + let(:query) do %( query { projectRunner1: runner(id: "#{project_runner1.to_global_id}") { projectCount jobCount + #{jobs_fragment} projects { nodes { id @@ -323,6 +337,7 @@ RSpec.describe 'Query.runner(id)' do projectRunner2: runner(id: "#{project_runner2.to_global_id}") { projectCount jobCount + #{jobs_fragment} projects { nodes { id @@ -332,6 +347,7 @@ RSpec.describe 'Query.runner(id)' do activeInstanceRunner: runner(id: "#{active_instance_runner.to_global_id}") { projectCount jobCount + #{jobs_fragment} projects { nodes { id @@ -355,6 +371,10 @@ RSpec.describe 'Query.runner(id)' do expect(runner1_data).to match a_hash_including( 'jobCount' => 1, + 'jobs' => a_hash_including( + "count" => 1, + "nodes" => [{ "id" => job.to_global_id.to_s, "status" => job.status.upcase }] + ), 'projectCount' => 2, 'projects' => { 'nodes' => [ @@ -364,12 +384,14 @@ RSpec.describe 'Query.runner(id)' do }) expect(runner2_data).to match a_hash_including( 'jobCount' => 0, + 'jobs' => nil, # returning jobs not allowed for more than 1 runner (see RunnerJobsResolver) 'projectCount' => 0, 'projects' => { 'nodes' => [] }) expect(runner3_data).to match a_hash_including( 'jobCount' => 0, + 'jobs' => nil, # returning jobs not allowed for more than 1 runner (see RunnerJobsResolver) 'projectCount' => nil, 'projects' => nil) end