From e33570fa7de143650584f7b389bb9331a87ed83e Mon Sep 17 00:00:00 2001 From: lulalala Date: Thu, 22 Feb 2024 16:53:40 +0800 Subject: [PATCH 1/3] Extract shared example --- .../namespace_setting_changes_auditor_spec.rb | 89 +++++++++++-------- 1 file changed, 50 insertions(+), 39 deletions(-) diff --git a/ee/spec/lib/audit/namespace_setting_changes_auditor_spec.rb b/ee/spec/lib/audit/namespace_setting_changes_auditor_spec.rb index 93913c8751f861..4a232603feb165 100644 --- a/ee/spec/lib/audit/namespace_setting_changes_auditor_spec.rb +++ b/ee/spec/lib/audit/namespace_setting_changes_auditor_spec.rb @@ -16,52 +16,63 @@ stub_licensed_features(extended_audit_events: true, external_audit_events: true) end - context 'when namespace setting is updated' do - context 'when ai-related settings are changed' do - let(:group) { create(:group_with_plan, plan: :ultimate_plan, trial_ends_on: Date.tomorrow) } - - before do - allow(Gitlab).to receive(:com?).and_return(true) - stub_licensed_features(ai_features: true, experimental_features: true) - stub_ee_application_setting(should_check_namespace_plan: true) - end + shared_examples 'audited setting' do |attribute, event_name| + before do + group.namespace_settings.update!(attribute => prev_value) + end - context 'when experiment_features_enabled is changed' do - where(:prev_value, :new_value) do - true | false - false | true - end + it 'creates an audit event' do + group.namespace_settings.update!(attribute => new_value) - with_them do - before do - group.namespace_settings.update!(experiment_features_enabled: prev_value) - end - - it 'creates an audit event' do - group.namespace_settings.update!(experiment_features_enabled: new_value) - - expect { auditor.execute }.to change { AuditEvent.count }.by(1) - audit_details = { - change: :experiment_features_enabled, - from: prev_value, - to: new_value, - target_details: group.full_path - } - expect(AuditEvent.last.details).to include(audit_details) - end - end + expect { auditor.execute }.to change { AuditEvent.count }.by(1) + audit_details = { + change: attribute, + from: prev_value, + to: new_value, + target_details: group.full_path + } + expect(AuditEvent.last.details).to include(audit_details) + end + + it 'streams correct audit event stream' do + group.namespace_settings.update!(attribute => new_value) + + expect(AuditEvents::AuditEventStreamingWorker).to receive(:perform_async).with( + event_name, anything, anything) + + auditor.execute + end + + context 'when attribute is not changed' do + it 'does not create an audit event' do + group.namespace_settings.update!(attribute => prev_value) + + expect { auditor.execute }.not_to change { AuditEvent.count } end + end + end - context 'when experiment_features_enabled is not changed' do - before do - group.namespace_settings.update!(experiment_features_enabled: true) - end + context 'for boolean changes' do + where(:prev_value, :new_value) do + true | false + false | true + end - it 'does not create an audit event' do - group.namespace_settings.update!(experiment_features_enabled: true) + with_them do + context 'when ai-related settings are changed', :saas do + let_it_be(:group) { create(:group_with_plan, plan: :ultimate_plan, trial_ends_on: Date.tomorrow) } + let_it_be(:destination) { create(:external_audit_event_destination, group: group) } - expect { auditor.execute }.not_to change { AuditEvent.count } + before do + stub_licensed_features( + ai_features: true, + experimental_features: true, + extended_audit_events: true, + external_audit_events: true) + stub_ee_application_setting(should_check_namespace_plan: true) end + + it_behaves_like 'audited setting', :experiment_features_enabled, 'experiment_features_enabled_updated' end end end -- GitLab From 70a36a9121a034864819948429266813ba84a9d4 Mon Sep 17 00:00:00 2001 From: lulalala Date: Thu, 22 Feb 2024 15:49:16 +0800 Subject: [PATCH 2/3] Audit duo_features_enabled_updated event --- doc/administration/audit_event_types.md | 6 ++++ .../types/duo_features_enabled_updated.yml | 10 ++++++ .../namespace_setting_changes_auditor.rb | 1 + .../audit/project_setting_changes_auditor.rb | 7 ++++ .../namespace_setting_changes_auditor_spec.rb | 1 + .../project_setting_changes_auditor_spec.rb | 33 +++++++++++++++++++ 6 files changed, 58 insertions(+) create mode 100644 ee/config/audit_events/types/duo_features_enabled_updated.yml diff --git a/doc/administration/audit_event_types.md b/doc/administration/audit_event_types.md index c5f9dc7c0b58e1..e9114197ea0866 100644 --- a/doc/administration/audit_event_types.md +++ b/doc/administration/audit_event_types.md @@ -43,6 +43,12 @@ NOTE: Audit event types with the `-` scope are limited to either project, group, or instance and user audit events, but the [information on the scope](https://gitlab.com/gitlab-org/gitlab/-/issues/438620) is still being added to the documentation. +### Ai framework + +| Name | Description | Saved to database | Streamed | Introduced in | Scope | +|:------------|:------------|:------------------|:---------|:--------------|:--------------| +| [`duo_features_enabled_updated`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/145509) | GitLab Duo Features enabled setting on group or project changed| **{check-circle}** Yes | **{check-circle}** Yes | GitLab [16.10](https://gitlab.com/gitlab-org/gitlab/-/issues/442485) | Group, Project | + ### Audit events | Name | Description | Saved to database | Streamed | Introduced in | Scope | diff --git a/ee/config/audit_events/types/duo_features_enabled_updated.yml b/ee/config/audit_events/types/duo_features_enabled_updated.yml new file mode 100644 index 00000000000000..e389328d37dc38 --- /dev/null +++ b/ee/config/audit_events/types/duo_features_enabled_updated.yml @@ -0,0 +1,10 @@ +--- +name: duo_features_enabled_updated +description: GitLab Duo Features enabled setting on group or project changed +introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/442485 +introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/145509 +feature_category: ai_framework +milestone: '16.10' +saved_to_database: true +streamed: true +scope: [Group, Project] diff --git a/ee/lib/audit/namespace_setting_changes_auditor.rb b/ee/lib/audit/namespace_setting_changes_auditor.rb index 90229c77e0823d..c35059d4bbf8b0 100644 --- a/ee/lib/audit/namespace_setting_changes_auditor.rb +++ b/ee/lib/audit/namespace_setting_changes_auditor.rb @@ -3,6 +3,7 @@ module Audit class NamespaceSettingChangesAuditor < BaseChangesAuditor EVENT_NAME_PER_COLUMN = { + duo_features_enabled: 'duo_features_enabled_updated', experiment_features_enabled: 'experiment_features_enabled_updated' }.freeze diff --git a/ee/lib/audit/project_setting_changes_auditor.rb b/ee/lib/audit/project_setting_changes_auditor.rb index 8b360912c0cd88..ac39b9cf6bc32f 100644 --- a/ee/lib/audit/project_setting_changes_auditor.rb +++ b/ee/lib/audit/project_setting_changes_auditor.rb @@ -32,6 +32,13 @@ def execute model: model, event_type: 'squash_commit_template_updated' ) + audit_changes( + :duo_features_enabled, + as: 'duo_features_enabled', + entity: @project, + model: model, + event_type: 'duo_features_enabled_updated' + ) end def attributes_from_auditable_model(column) diff --git a/ee/spec/lib/audit/namespace_setting_changes_auditor_spec.rb b/ee/spec/lib/audit/namespace_setting_changes_auditor_spec.rb index 4a232603feb165..18b42a3f853b11 100644 --- a/ee/spec/lib/audit/namespace_setting_changes_auditor_spec.rb +++ b/ee/spec/lib/audit/namespace_setting_changes_auditor_spec.rb @@ -73,6 +73,7 @@ end it_behaves_like 'audited setting', :experiment_features_enabled, 'experiment_features_enabled_updated' + it_behaves_like 'audited setting', :duo_features_enabled, 'duo_features_enabled_updated' end end end diff --git a/ee/spec/lib/audit/project_setting_changes_auditor_spec.rb b/ee/spec/lib/audit/project_setting_changes_auditor_spec.rb index d03f58e96492d4..d3957e931be63a 100644 --- a/ee/spec/lib/audit/project_setting_changes_auditor_spec.rb +++ b/ee/spec/lib/audit/project_setting_changes_auditor_spec.rb @@ -139,6 +139,39 @@ project_setting_changes_auditor.execute end end + + context 'when duo_features_enabled is changed' do + where(:prev_value, :new_value) do + true | false + false | true + end + + with_them do + before do + project.project_setting.update!(duo_features_enabled: prev_value) + end + + it 'creates an audit event' do + project.project_setting.update!(duo_features_enabled: new_value) + + expect { project_setting_changes_auditor.execute }.to change(AuditEvent, :count).by(1) + expect(AuditEvent.last.details).to include({ + change: 'duo_features_enabled', + from: prev_value, + to: new_value + }) + end + + it 'streams correct audit event stream' do + project.project_setting.update!(duo_features_enabled: new_value) + + expect(AuditEvents::AuditEventStreamingWorker).to receive(:perform_async).with( + 'duo_features_enabled_updated', anything, anything) + + project_setting_changes_auditor.execute + end + end + end end end end -- GitLab From 5be721ae2349637c7dc83cee3bac54e75e6e511b Mon Sep 17 00:00:00 2001 From: lulalala Date: Mon, 26 Feb 2024 12:59:05 +0800 Subject: [PATCH 3/3] Refactor project_setting_changes_auditor_spec.rb --- .../project_setting_changes_auditor_spec.rb | 169 ++++++------------ 1 file changed, 54 insertions(+), 115 deletions(-) diff --git a/ee/spec/lib/audit/project_setting_changes_auditor_spec.rb b/ee/spec/lib/audit/project_setting_changes_auditor_spec.rb index d3957e931be63a..04f0366d560b89 100644 --- a/ee/spec/lib/audit/project_setting_changes_auditor_spec.rb +++ b/ee/spec/lib/audit/project_setting_changes_auditor_spec.rb @@ -9,12 +9,46 @@ let_it_be(:group) { create(:group) } let_it_be(:destination) { create(:external_audit_event_destination, group: group) } let_it_be(:project) { create(:project, group: group) } - let_it_be(:project_setting_changes_auditor) { described_class.new(user, project.project_setting, project) } + let_it_be(:auditor) { described_class.new(user, project.project_setting, project) } before do stub_licensed_features(extended_audit_events: true, external_audit_events: true) end + shared_examples 'audited setting' do |attribute, event_name| + before do + project.project_setting.update!(attribute => prev_value) + end + + it 'creates an audit event' do + project.project_setting.update!(attribute => new_value) + + expect { auditor.execute }.to change(AuditEvent, :count).by(1) + expect(AuditEvent.last.details).to include({ + change: attribute.to_s, + from: prev_value, + to: new_value + }) + end + + it 'streams correct audit event stream' do + project.project_setting.update!(attribute => new_value) + + expect(AuditEvents::AuditEventStreamingWorker).to receive(:perform_async).with( + event_name, anything, anything) + + auditor.execute + end + + context 'when attribute is not changed' do + it 'does not create an audit event' do + project.project_setting.update!(attribute => prev_value) + + expect { auditor.execute }.not_to change { AuditEvent.count } + end + end + end + context 'when project setting is updated' do options = ProjectSetting.squash_options.keys options.each do |prev_value| @@ -28,7 +62,7 @@ it 'creates an audit event' do project.project_setting.update!(squash_option: new_value) - expect { project_setting_changes_auditor.execute }.to change(AuditEvent, :count).by(1) + expect { auditor.execute }.to change(AuditEvent, :count).by(1) expect(AuditEvent.last.details).to include( { custom_message: "Changed squash option to #{project.project_setting.human_squash_option}" @@ -41,135 +75,40 @@ expect(AuditEvents::AuditEventStreamingWorker).to receive(:perform_async).with( 'squash_option_updated', anything, anything) - project_setting_changes_auditor.execute + auditor.execute end else it 'does not create audit event' do project.project_setting.update!(squash_option: new_value) - expect { project_setting_changes_auditor.execute }.to not_change { AuditEvent.count } + expect { auditor.execute }.to not_change { AuditEvent.count } end end end end end + end - context 'when allow_merge_on_skipped_pipeline is changed' do - where(:prev_value, :new_value) do - true | false - false | true - end - - with_them do - before do - project.project_setting.update!(allow_merge_on_skipped_pipeline: prev_value) - end - - it 'creates an audit event' do - project.project_setting.update!(allow_merge_on_skipped_pipeline: new_value) - - expect { project_setting_changes_auditor.execute }.to change(AuditEvent, :count).by(1) - expect(AuditEvent.last.details).to include({ - change: 'allow_merge_on_skipped_pipeline', - from: prev_value, - to: new_value - }) - end - - it 'streams correct audit event stream' do - project.project_setting.update!(allow_merge_on_skipped_pipeline: new_value) - - expect(AuditEvents::AuditEventStreamingWorker).to receive(:perform_async).with( - 'allow_merge_on_skipped_pipeline_updated', anything, anything) - - project_setting_changes_auditor.execute - end - end + context 'for string changes' do + where(:prev_value, :new_value) do + 'old' | 'new' end - context 'when squash_commit_template is changed' do - before do - project.project_setting.update!(squash_commit_template: 'old squash commit template') - end - - it 'creates an audit event' do - project.project_setting.update!(squash_commit_template: 'new squash commit template') - - expect { project_setting_changes_auditor.execute }.to change(AuditEvent, :count).by(1) - expect(AuditEvent.last.details).to include({ - change: 'squash_commit_template', - from: 'old squash commit template', - to: 'new squash commit template' - }) - end - - it 'streams correct audit event stream' do - project.project_setting.update!(squash_commit_template: 'new squash commit template') - - expect(AuditEvents::AuditEventStreamingWorker).to receive(:perform_async).with( - 'squash_commit_template_updated', anything, anything) - - project_setting_changes_auditor.execute - end + with_them do + it_behaves_like 'audited setting', :merge_commit_template, 'merge_commit_template_updated' + it_behaves_like 'audited setting', :squash_commit_template, 'squash_commit_template_updated' end + end - context 'when merge_commit_template is changed' do - before do - project.project_setting.update!(merge_commit_template: 'old merge commit template') - end - - it 'creates an audit event' do - project.project_setting.update!(merge_commit_template: 'new merge commit template') - - aggregate_failures do - expect { project_setting_changes_auditor.execute }.to change(AuditEvent, :count).by(1) - expect(AuditEvent.last.details).to include({ - change: 'merge_commit_template', - from: 'old merge commit template', - to: 'new merge commit template' - }) - end - end - - it 'streams correct audit event stream' do - project.project_setting.update!(merge_commit_template: 'new merge commit template') - - expect(AuditEvents::AuditEventStreamingWorker).to receive(:perform_async).with( - 'merge_commit_template_updated', anything, anything) - - project_setting_changes_auditor.execute - end + context 'for boolean changes' do + where(:prev_value, :new_value) do + true | false + false | true end - context 'when duo_features_enabled is changed' do - where(:prev_value, :new_value) do - true | false - false | true - end - - with_them do - before do - project.project_setting.update!(duo_features_enabled: prev_value) - end - - it 'creates an audit event' do - project.project_setting.update!(duo_features_enabled: new_value) - - expect { project_setting_changes_auditor.execute }.to change(AuditEvent, :count).by(1) - expect(AuditEvent.last.details).to include({ - change: 'duo_features_enabled', - from: prev_value, - to: new_value - }) - end - - it 'streams correct audit event stream' do - project.project_setting.update!(duo_features_enabled: new_value) - - expect(AuditEvents::AuditEventStreamingWorker).to receive(:perform_async).with( - 'duo_features_enabled_updated', anything, anything) - - project_setting_changes_auditor.execute - end + with_them do + context 'when ai-related settings are changed', :saas do + it_behaves_like 'audited setting', :duo_features_enabled, 'duo_features_enabled_updated' + it_behaves_like 'audited setting', :allow_merge_on_skipped_pipeline, 'allow_merge_on_skipped_pipeline_updated' end end end -- GitLab