From 25760b2b784f74bca0d35af68e775a131ead0688 Mon Sep 17 00:00:00 2001 From: Pedro Pombeiro Date: Wed, 20 Nov 2024 16:59:48 +0100 Subject: [PATCH 1/6] Add audit event to CreateRunnerService --- .../ci/runners/create_runner_service.rb | 4 +- doc/user/compliance/audit_event_types.md | 1 + .../create_runner_audit_event_service.rb | 17 ++++ .../ee/ci/runners/create_runner_service.rb | 28 ++++++ .../audit_events/types/ci_runner_created.yml | 10 ++ .../create_runner_audit_event_service_spec.rb | 97 +++++++++++++++++++ .../mutations/ci/runner/create_spec.rb | 6 +- 7 files changed, 157 insertions(+), 6 deletions(-) create mode 100644 ee/app/services/audit_events/create_runner_audit_event_service.rb create mode 100644 ee/app/services/ee/ci/runners/create_runner_service.rb create mode 100644 ee/config/audit_events/types/ci_runner_created.yml create mode 100644 ee/spec/services/audit_events/create_runner_audit_event_service_spec.rb diff --git a/app/services/ci/runners/create_runner_service.rb b/app/services/ci/runners/create_runner_service.rb index fbf49d1f3f72ce..013cbec7f6a4b5 100644 --- a/app/services/ci/runners/create_runner_service.rb +++ b/app/services/ci/runners/create_runner_service.rb @@ -49,7 +49,7 @@ def normalize_params private - attr_reader :user, :params, :strategy + attr_reader :user, :scope, :params, :strategy def track_runner_event(runner) return if params[:maintenance_note].blank? @@ -73,3 +73,5 @@ def track_runner_event(runner) end end end + +Ci::Runners::CreateRunnerService.prepend_mod diff --git a/doc/user/compliance/audit_event_types.md b/doc/user/compliance/audit_event_types.md index 4ea4a566215e27..b572d145809b4d 100644 --- a/doc/user/compliance/audit_event_types.md +++ b/doc/user/compliance/audit_event_types.md @@ -307,6 +307,7 @@ Audit event types belong to the following product categories. | Name | Description | Saved to database | Introduced in | Scope | |:------------|:------------|:------------------|:---------|:--------------|:--------------| +| [`ci_runner_created`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/173885) | Triggered when a runner is created | **{check-circle}** Yes | GitLab [17.7](https://gitlab.com/gitlab-org/gitlab/-/issues/503315) | Instance, Group, Project | | [`ci_runner_usage_export`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/139578) | A runner usage report is generated | **{check-circle}** Yes | GitLab [16.8](https://gitlab.com/gitlab-org/gitlab/-/issues/426560) | Instance | | [`ci_runners_bulk_deleted`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/173886) | Triggered when runners are deleted in bulk | **{check-circle}** Yes | GitLab [17.7](https://gitlab.com/gitlab-org/gitlab/-/issues/503315) | User | diff --git a/ee/app/services/audit_events/create_runner_audit_event_service.rb b/ee/app/services/audit_events/create_runner_audit_event_service.rb new file mode 100644 index 00000000000000..8e3b7c95eef67b --- /dev/null +++ b/ee/app/services/audit_events/create_runner_audit_event_service.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +module AuditEvents + class CreateRunnerAuditEventService < RunnerAuditEventService + def token_field; end + + def message + return "Created #{runner_type} CI runner" if @runner.valid? + + "Failed to create #{runner_type} CI runner" + end + + def runner_path + super if @runner.persisted? + end + end +end diff --git a/ee/app/services/ee/ci/runners/create_runner_service.rb b/ee/app/services/ee/ci/runners/create_runner_service.rb new file mode 100644 index 00000000000000..6821b6cc9d72fc --- /dev/null +++ b/ee/app/services/ee/ci/runners/create_runner_service.rb @@ -0,0 +1,28 @@ +# frozen_string_literal: true + +module EE + module Ci + module Runners + # Creates a CI Runner and logs an audit event + # + module CreateRunnerService + extend ::Gitlab::Utils::Override + + override :execute + def execute + super.tap do |response| + audit_event(response.payload[:runner]) if response.success? + end + end + + private + + def audit_event(runner) + token_scope = runner.instance_type? ? ::Gitlab::Audit::InstanceScope.new : scope + + ::AuditEvents::CreateRunnerAuditEventService.new(runner, user, token_scope).track_event + end + end + end + end +end diff --git a/ee/config/audit_events/types/ci_runner_created.yml b/ee/config/audit_events/types/ci_runner_created.yml new file mode 100644 index 00000000000000..5f216b888e0951 --- /dev/null +++ b/ee/config/audit_events/types/ci_runner_created.yml @@ -0,0 +1,10 @@ +--- +name: ci_runner_created +description: Triggered when a runner is created +introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/503315 +introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/173885 +feature_category: fleet_visibility +milestone: '17.7' +saved_to_database: true +streamed: true +scope: [Instance, Group, Project] diff --git a/ee/spec/services/audit_events/create_runner_audit_event_service_spec.rb b/ee/spec/services/audit_events/create_runner_audit_event_service_spec.rb new file mode 100644 index 00000000000000..ce45860b41e342 --- /dev/null +++ b/ee/spec/services/audit_events/create_runner_audit_event_service_spec.rb @@ -0,0 +1,97 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe AuditEvents::CreateRunnerAuditEventService, feature_category: :runner do + let_it_be(:user) { create(:user) } + let_it_be(:owner) { create(:user) } + + let(:service) { described_class.new(runner, author, entity) } + let(:common_attrs) do + { + author_id: user.id, + author_name: author.name, + created_at: timestamp, + target_type: runner.class.name, + target_id: runner.id, + ip_address: nil, + details: { + target_type: runner.class.name, + target_id: runner.id, + ip_address: nil + } + } + end + + shared_examples 'expected audit event' do + it 'returns audit event attributes' do + travel_to(timestamp) do + expect(track_event.attributes).to eq("id" => track_event.id, **attrs.stringify_keys) + end + end + end + + shared_context 'when creating runner' do + let(:author) { user } + + let(:attrs) do + common_attrs.deep_merge( + entity_id: entity.id, + entity_type: entity.class.name, + entity_path: entity.full_path, + target_details: target_details, + details: { + author_name: author.name, + custom_message: "Created #{runner.runner_type.chomp('_type')} CI runner", + entity_path: entity.full_path, + target_details: target_details + } + ) + end + + it_behaves_like 'expected audit event' + end + + describe '#track_event' do + before do + stub_licensed_features(admin_audit_log: true) + end + + subject(:track_event) { service.track_event } + + let(:timestamp) { Time.zone.local(2021, 12, 28) } + + context 'for instance runner' do + before do + stub_licensed_features(extended_audit_events: true, admin_audit_log: true) + end + + let_it_be(:runner) { create(:ci_runner, creator_id: owner) } + + it_behaves_like 'expected audit event' + + include_context 'when creating runner' do + let(:entity) { ::Gitlab::Audit::InstanceScope.new } + let(:target_details) { ::Gitlab::Routing.url_helpers.admin_runner_path(runner) } + end + end + + context 'for group runner' do + let_it_be(:entity) { create(:group) } + let_it_be(:runner) { create(:ci_runner, :group, groups: [entity], creator_id: owner) } + + include_context 'when creating runner' do + let(:target_details) { ::Gitlab::Routing.url_helpers.group_runner_path(entity, runner) } + end + end + + context 'for project runner' do + let_it_be(:entity) { create(:project) } + let_it_be(:runner) { create(:ci_runner, :project, projects: [entity], creator_id: owner) } + + include_context 'when creating runner' do + let(:target_details) { ::Gitlab::Routing.url_helpers.project_runner_path(entity, runner) } + end + end + end +end diff --git a/spec/requests/api/graphql/mutations/ci/runner/create_spec.rb b/spec/requests/api/graphql/mutations/ci/runner/create_spec.rb index 844be90851d6ec..5a7a719eb9bc09 100644 --- a/spec/requests/api/graphql/mutations/ci/runner/create_spec.rb +++ b/spec/requests/api/graphql/mutations/ci/runner/create_spec.rb @@ -25,13 +25,9 @@ end let(:mutation) do - variables = { - **mutation_params - } - graphql_mutation( :runner_create, - variables, + mutation_params, <<-QL runner { ephemeralAuthenticationToken -- GitLab From d6d4f8d4c14b1435e089b56ffacf2f97dfc9ec57 Mon Sep 17 00:00:00 2001 From: Pedro Pombeiro Date: Fri, 29 Nov 2024 10:22:00 +0100 Subject: [PATCH 2/6] Remove spacer comment --- ee/app/services/ee/ci/runners/create_runner_service.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/ee/app/services/ee/ci/runners/create_runner_service.rb b/ee/app/services/ee/ci/runners/create_runner_service.rb index 6821b6cc9d72fc..28632771cdf6d9 100644 --- a/ee/app/services/ee/ci/runners/create_runner_service.rb +++ b/ee/app/services/ee/ci/runners/create_runner_service.rb @@ -4,7 +4,6 @@ module EE module Ci module Runners # Creates a CI Runner and logs an audit event - # module CreateRunnerService extend ::Gitlab::Utils::Override -- GitLab From 579345c293199a1ac72938b9081ec45c6e98c0af Mon Sep 17 00:00:00 2001 From: Pedro Pombeiro Date: Fri, 29 Nov 2024 11:37:51 +0100 Subject: [PATCH 3/6] Remove unused failure message --- .../audit_events/create_runner_audit_event_service.rb | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/ee/app/services/audit_events/create_runner_audit_event_service.rb b/ee/app/services/audit_events/create_runner_audit_event_service.rb index 8e3b7c95eef67b..f5fa3629af62c2 100644 --- a/ee/app/services/audit_events/create_runner_audit_event_service.rb +++ b/ee/app/services/audit_events/create_runner_audit_event_service.rb @@ -5,9 +5,7 @@ class CreateRunnerAuditEventService < RunnerAuditEventService def token_field; end def message - return "Created #{runner_type} CI runner" if @runner.valid? - - "Failed to create #{runner_type} CI runner" + "Created #{runner_type} CI runner" end def runner_path -- GitLab From 9825a223816a3fe63ee62b8df10cb9723bac6a10 Mon Sep 17 00:00:00 2001 From: Pedro Pombeiro Date: Wed, 4 Dec 2024 13:08:49 +0100 Subject: [PATCH 4/6] Pass audit event name --- .../create_runner_audit_event_service.rb | 4 +++ .../create_runner_audit_event_service_spec.rb | 33 ++++++------------- 2 files changed, 14 insertions(+), 23 deletions(-) diff --git a/ee/app/services/audit_events/create_runner_audit_event_service.rb b/ee/app/services/audit_events/create_runner_audit_event_service.rb index f5fa3629af62c2..e6f8da0affa814 100644 --- a/ee/app/services/audit_events/create_runner_audit_event_service.rb +++ b/ee/app/services/audit_events/create_runner_audit_event_service.rb @@ -2,6 +2,10 @@ module AuditEvents class CreateRunnerAuditEventService < RunnerAuditEventService + def name + 'ci_runner_created' + end + def token_field; end def message diff --git a/ee/spec/services/audit_events/create_runner_audit_event_service_spec.rb b/ee/spec/services/audit_events/create_runner_audit_event_service_spec.rb index ce45860b41e342..3f08186ddaaa39 100644 --- a/ee/spec/services/audit_events/create_runner_audit_event_service_spec.rb +++ b/ee/spec/services/audit_events/create_runner_audit_event_service_spec.rb @@ -9,24 +9,18 @@ let(:service) { described_class.new(runner, author, entity) } let(:common_attrs) do { - author_id: user.id, - author_name: author.name, - created_at: timestamp, - target_type: runner.class.name, - target_id: runner.id, - ip_address: nil, - details: { - target_type: runner.class.name, - target_id: runner.id, - ip_address: nil - } + name: 'ci_runner_created', + author: author, + target: runner } end shared_examples 'expected audit event' do it 'returns audit event attributes' do travel_to(timestamp) do - expect(track_event.attributes).to eq("id" => track_event.id, **attrs.stringify_keys) + expect(::Gitlab::Audit::Auditor).to receive(:audit).with(a_hash_including(attrs)) + + track_event end end end @@ -36,16 +30,9 @@ let(:attrs) do common_attrs.deep_merge( - entity_id: entity.id, - entity_type: entity.class.name, - entity_path: entity.full_path, - target_details: target_details, - details: { - author_name: author.name, - custom_message: "Created #{runner.runner_type.chomp('_type')} CI runner", - entity_path: entity.full_path, - target_details: target_details - } + message: "Created #{runner.runner_type.chomp('_type')} CI runner", + scope: entity, + target_details: target_details ) end @@ -71,7 +58,7 @@ it_behaves_like 'expected audit event' include_context 'when creating runner' do - let(:entity) { ::Gitlab::Audit::InstanceScope.new } + let(:entity) { an_instance_of(::Gitlab::Audit::InstanceScope) } let(:target_details) { ::Gitlab::Routing.url_helpers.admin_runner_path(runner) } end end -- GitLab From 1996be64254a2dbbcc65e11fedb426e81992101d Mon Sep 17 00:00:00 2001 From: Pedro Pombeiro Date: Thu, 5 Dec 2024 15:54:34 +0100 Subject: [PATCH 5/6] Remove CreateRunnerAuditEventService --- .../create_runner_audit_event_service.rb | 19 ----- .../ee/ci/runners/create_runner_service.rb | 5 +- .../create_runner_audit_event_service_spec.rb | 84 ------------------- locale/gitlab.pot | 3 + 4 files changed, 7 insertions(+), 104 deletions(-) delete mode 100644 ee/app/services/audit_events/create_runner_audit_event_service.rb delete mode 100644 ee/spec/services/audit_events/create_runner_audit_event_service_spec.rb diff --git a/ee/app/services/audit_events/create_runner_audit_event_service.rb b/ee/app/services/audit_events/create_runner_audit_event_service.rb deleted file mode 100644 index e6f8da0affa814..00000000000000 --- a/ee/app/services/audit_events/create_runner_audit_event_service.rb +++ /dev/null @@ -1,19 +0,0 @@ -# frozen_string_literal: true - -module AuditEvents - class CreateRunnerAuditEventService < RunnerAuditEventService - def name - 'ci_runner_created' - end - - def token_field; end - - def message - "Created #{runner_type} CI runner" - end - - def runner_path - super if @runner.persisted? - end - end -end diff --git a/ee/app/services/ee/ci/runners/create_runner_service.rb b/ee/app/services/ee/ci/runners/create_runner_service.rb index 28632771cdf6d9..f61559288f9f7d 100644 --- a/ee/app/services/ee/ci/runners/create_runner_service.rb +++ b/ee/app/services/ee/ci/runners/create_runner_service.rb @@ -19,7 +19,10 @@ def execute def audit_event(runner) token_scope = runner.instance_type? ? ::Gitlab::Audit::InstanceScope.new : scope - ::AuditEvents::CreateRunnerAuditEventService.new(runner, user, token_scope).track_event + ::AuditEvents::RunnerAuditEventService.new( + runner, user, token_scope, + name: 'ci_runner_created', message: s_("Runners|Created %{runner_type} CI runner") + ).track_event end end end diff --git a/ee/spec/services/audit_events/create_runner_audit_event_service_spec.rb b/ee/spec/services/audit_events/create_runner_audit_event_service_spec.rb deleted file mode 100644 index 3f08186ddaaa39..00000000000000 --- a/ee/spec/services/audit_events/create_runner_audit_event_service_spec.rb +++ /dev/null @@ -1,84 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe AuditEvents::CreateRunnerAuditEventService, feature_category: :runner do - let_it_be(:user) { create(:user) } - let_it_be(:owner) { create(:user) } - - let(:service) { described_class.new(runner, author, entity) } - let(:common_attrs) do - { - name: 'ci_runner_created', - author: author, - target: runner - } - end - - shared_examples 'expected audit event' do - it 'returns audit event attributes' do - travel_to(timestamp) do - expect(::Gitlab::Audit::Auditor).to receive(:audit).with(a_hash_including(attrs)) - - track_event - end - end - end - - shared_context 'when creating runner' do - let(:author) { user } - - let(:attrs) do - common_attrs.deep_merge( - message: "Created #{runner.runner_type.chomp('_type')} CI runner", - scope: entity, - target_details: target_details - ) - end - - it_behaves_like 'expected audit event' - end - - describe '#track_event' do - before do - stub_licensed_features(admin_audit_log: true) - end - - subject(:track_event) { service.track_event } - - let(:timestamp) { Time.zone.local(2021, 12, 28) } - - context 'for instance runner' do - before do - stub_licensed_features(extended_audit_events: true, admin_audit_log: true) - end - - let_it_be(:runner) { create(:ci_runner, creator_id: owner) } - - it_behaves_like 'expected audit event' - - include_context 'when creating runner' do - let(:entity) { an_instance_of(::Gitlab::Audit::InstanceScope) } - let(:target_details) { ::Gitlab::Routing.url_helpers.admin_runner_path(runner) } - end - end - - context 'for group runner' do - let_it_be(:entity) { create(:group) } - let_it_be(:runner) { create(:ci_runner, :group, groups: [entity], creator_id: owner) } - - include_context 'when creating runner' do - let(:target_details) { ::Gitlab::Routing.url_helpers.group_runner_path(entity, runner) } - end - end - - context 'for project runner' do - let_it_be(:entity) { create(:project) } - let_it_be(:runner) { create(:ci_runner, :project, projects: [entity], creator_id: owner) } - - include_context 'when creating runner' do - let(:target_details) { ::Gitlab::Routing.url_helpers.project_runner_path(entity, runner) } - end - end - end -end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 11955531edf4e3..03f90e8919b424 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -47354,6 +47354,9 @@ msgstr "" msgid "Runners|Create runner" msgstr "" +msgid "Runners|Created %{runner_type} CI runner" +msgstr "" + msgid "Runners|Created %{timeAgo}" msgstr "" -- GitLab From 13dceb3432d330c257d00ebfabc96fe3d3cec90c Mon Sep 17 00:00:00 2001 From: Pedro Pombeiro Date: Fri, 6 Dec 2024 13:59:47 +0100 Subject: [PATCH 6/6] Remove localization of message --- ee/app/services/ee/ci/runners/create_runner_service.rb | 3 ++- locale/gitlab.pot | 3 --- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/ee/app/services/ee/ci/runners/create_runner_service.rb b/ee/app/services/ee/ci/runners/create_runner_service.rb index f61559288f9f7d..f4cd8975872feb 100644 --- a/ee/app/services/ee/ci/runners/create_runner_service.rb +++ b/ee/app/services/ee/ci/runners/create_runner_service.rb @@ -21,7 +21,8 @@ def audit_event(runner) ::AuditEvents::RunnerAuditEventService.new( runner, user, token_scope, - name: 'ci_runner_created', message: s_("Runners|Created %{runner_type} CI runner") + name: 'ci_runner_created', + message: "Created %{runner_type} CI runner" ).track_event end end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 03f90e8919b424..11955531edf4e3 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -47354,9 +47354,6 @@ msgstr "" msgid "Runners|Create runner" msgstr "" -msgid "Runners|Created %{runner_type} CI runner" -msgstr "" - msgid "Runners|Created %{timeAgo}" msgstr "" -- GitLab