diff --git a/app/services/ml/create_model_service.rb b/app/services/ml/create_model_service.rb index 46035194a12f5b4dfa1894bad66141c4dfbfb13e..331441e510558904f575c89c5551189a36c63552 100644 --- a/app/services/ml/create_model_service.rb +++ b/app/services/ml/create_model_service.rb @@ -35,6 +35,7 @@ def execute ) add_metadata(model, @metadata) + audit_creation_event(model) success(model) end @@ -69,5 +70,17 @@ def add_metadata(model, metadata_key_value) def experiment_name Ml::Model.prefixed_experiment(@name) end + + def audit_creation_event(model) + audit_context = { + name: 'ml_model_created', + author: @user, + scope: @project, + target: model, + message: "MlModel #{model.name} created" + } + + ::Gitlab::Audit::Auditor.audit(audit_context) + end end end diff --git a/app/services/ml/destroy_model_service.rb b/app/services/ml/destroy_model_service.rb index f2a33d58cd88282838ba6dd885ff0fd802e56c34..1ff9e08e6b13fa1437138a2d16487ce19374eb32 100644 --- a/app/services/ml/destroy_model_service.rb +++ b/app/services/ml/destroy_model_service.rb @@ -17,6 +17,8 @@ def execute return error unless @model.destroy + audit_destroy_event(@model) + success end @@ -37,5 +39,17 @@ def packages_not_deleted(error_message) def payload { model: @model } end + + def audit_destroy_event(model) + audit_context = { + name: 'ml_model_destroyed', + author: @user, + scope: model.project, + target: model, + message: "MlModel #{model.name} destroyed" + } + + ::Gitlab::Audit::Auditor.audit(audit_context) + end end end diff --git a/config/audit_events/types/ml_model_created.yml b/config/audit_events/types/ml_model_created.yml new file mode 100644 index 0000000000000000000000000000000000000000..c455c96bc03bb628a48a26a5d10884542bf11429 --- /dev/null +++ b/config/audit_events/types/ml_model_created.yml @@ -0,0 +1,9 @@ +name: ml_model_created +description: ML model is created +introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/463215 +introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/165011 +feature_category: mlops +milestone: '17.4' +saved_to_database: true +scope: [Project] +streamed: true diff --git a/config/audit_events/types/ml_model_destroyed.yml b/config/audit_events/types/ml_model_destroyed.yml new file mode 100644 index 0000000000000000000000000000000000000000..fb750103cf8f76b7e252b7e34a7f59089b35c94f --- /dev/null +++ b/config/audit_events/types/ml_model_destroyed.yml @@ -0,0 +1,9 @@ +name: ml_model_destroyed +description: ML model destroyed +introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/463215 +introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/165011 +feature_category: mlops +milestone: '17.4' +saved_to_database: true +scope: [Project] +streamed: true diff --git a/doc/user/compliance/audit_event_types.md b/doc/user/compliance/audit_event_types.md index fe2e20c830f392af6b0a25659d062945321b9dc4..42ca0fcd07b5efe486536bf22872290aa6efc148 100644 --- a/doc/user/compliance/audit_event_types.md +++ b/doc/user/compliance/audit_event_types.md @@ -379,6 +379,8 @@ Audit event types belong to the following product categories. | Name | Description | Saved to database | Streamed | Introduced in | Scope | |:------------|:------------|:------------------|:---------|:--------------|:--------------| | [`project_feature_model_experiments_access_level_updated`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/121027) | Model experiments access level was updated | **{check-circle}** Yes | **{check-circle}** Yes | GitLab [16.1](https://gitlab.com/gitlab-org/gitlab/-/issues/412384) | Project | +| [`ml_model_created`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/165011) | ML model is created | **{check-circle}** Yes | **{check-circle}** Yes | GitLab [17.4](https://gitlab.com/gitlab-org/gitlab/-/issues/463215) | Project | +| [`ml_model_destroyed`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/165011) | ML model destroyed | **{check-circle}** Yes | **{check-circle}** Yes | GitLab [17.4](https://gitlab.com/gitlab-org/gitlab/-/issues/463215) | Project | | [`project_feature_model_registry_access_level_updated`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/138399) | Model registry access level was updated | **{check-circle}** Yes | **{check-circle}** Yes | GitLab [16.7](https://gitlab.com/gitlab-org/gitlab/-/issues/412734) | Project | ### Not categorized diff --git a/spec/services/ml/create_model_service_spec.rb b/spec/services/ml/create_model_service_spec.rb index dd4d41b49955e7465e4098227325aac15d917373..c6341cd4c2560e9661faea9653f9f7996219baeb 100644 --- a/spec/services/ml/create_model_service_spec.rb +++ b/spec/services/ml/create_model_service_spec.rb @@ -9,24 +9,38 @@ let_it_be(:description) { 'description' } let_it_be(:metadata) { [] } + let(:audit_event) do + { + name: 'ml_model_created', + author: user, + scope: project + } + end + before do allow(Gitlab::InternalEvents).to receive(:track_event) + allow(Gitlab::Audit::Auditor).to receive(:audit).and_call_original end subject(:create_model) { described_class.new(project, name, user, description, metadata).execute } - describe '#execute' do + describe '#execute', :aggregate_failures do subject(:model_payload) { create_model.payload } + let(:audit_context) do + audit_event.merge(target: model_payload, message: "MlModel #{name} created") + end + context 'when model name is not supplied' do let(:name) { nil } let(:project) { existing_model.project } - it 'returns a model with errors', :aggregate_failures do + it 'returns a model with errors' do expect { create_model }.not_to change { Ml::Model.count } expect(create_model).to be_error expect(Gitlab::InternalEvents).not_to have_received(:track_event) expect(create_model.message).to include("Name can't be blank") + expect(Gitlab::Audit::Auditor).not_to have_received(:audit) end end @@ -34,13 +48,15 @@ let(:name) { 'new_model' } let(:project) { existing_model.project } - it 'creates a model', :aggregate_failures do + it 'creates a model' do expect { create_model }.to change { Ml::Model.count }.by(1) expect(Gitlab::InternalEvents).to have_received(:track_event).with( 'model_registry_ml_model_created', { project: project, user: user } ) + expect(Gitlab::Audit::Auditor).to have_received(:audit).with(audit_context) + expect(model_payload.name).to eq('new_model') expect(model_payload.default_experiment.name).to eq('[model]new_model') end @@ -50,13 +66,14 @@ let(:name) { existing_model.name } let(:project) { another_project } - it 'creates a model', :aggregate_failures do + it 'creates a model' do expect { create_model }.to change { Ml::Model.count }.by(1) expect(Gitlab::InternalEvents).to have_received(:track_event).with( 'model_registry_ml_model_created', { project: project, user: user } ) + expect(Gitlab::Audit::Auditor).to have_received(:audit).with(audit_context) expect(model_payload.name).to eq(name) end end @@ -65,11 +82,12 @@ let(:name) { existing_model.name } let(:project) { existing_model.project } - it 'returns a model with errors', :aggregate_failures do + it 'returns a model with errors' do expect { create_model }.not_to change { Ml::Model.count } expect(create_model).to be_error expect(Gitlab::InternalEvents).not_to have_received(:track_event) expect(create_model.message).to eq(["Name should be unique in the project"]) + expect(Gitlab::Audit::Auditor).not_to have_received(:audit) end end @@ -78,10 +96,11 @@ let(:project) { existing_model.project } let(:metadata) { [{ key: 'key1', value: 'value1' }, { key: 'key2', value: 'value2' }] } - it 'creates metadata records', :aggregate_failures do + it 'creates metadata records' do expect { create_model }.to change { Ml::Model.count }.by(1) expect(model_payload.name).to eq(name) + expect(Gitlab::Audit::Auditor).to have_received(:audit).with(audit_context) expect(model_payload.metadata.count).to be 2 end end @@ -92,8 +111,9 @@ let(:project) { existing_model.project } let(:metadata) { [{ key: 'key1', value: 'value1' }, { key: 'key1', value: 'value2' }] } - it 'raises an error', :aggregate_failures do + it 'raises an error' do expect { create_model }.to raise_error(ActiveRecord::RecordInvalid) + expect(Gitlab::Audit::Auditor).not_to have_received(:audit) end end @@ -103,8 +123,9 @@ let(:project) { existing_model.project } let(:metadata) { [{ key: 'key1', value: 'value1' }, { key: '', value: 'value2' }] } - it 'raises an error', :aggregate_failures do + it 'raises an error' do expect { create_model }.to raise_error(ActiveRecord::RecordInvalid) + expect(Gitlab::Audit::Auditor).not_to have_received(:audit) end end end diff --git a/spec/services/ml/destroy_model_service_spec.rb b/spec/services/ml/destroy_model_service_spec.rb index 0e21d9dc39c9b58ae52a95d7d8e3528648a5bcbb..d1c70bab5621d3c5a3015e406e6299c491d5adee 100644 --- a/spec/services/ml/destroy_model_service_spec.rb +++ b/spec/services/ml/destroy_model_service_spec.rb @@ -9,8 +9,21 @@ let(:model) { model0 } let(:service) { described_class.new(model, user) } + let(:audit_event) do + { + name: 'ml_model_destroyed', + author: user, + scope: model.project, + message: "MlModel #{model.name} destroyed", + target: model + } + end + + before do + allow(Gitlab::Audit::Auditor).to receive(:audit).and_call_original + end - describe '#execute' do + describe '#execute', :aggregate_failures do subject(:service_result) { service.execute } context 'when model fails to delete' do @@ -18,17 +31,19 @@ allow(model).to receive(:destroy).and_return(false) expect(service_result).to be_error + expect(Gitlab::Audit::Auditor).not_to have_received(:audit) end end context 'when a model exists' do - it 'destroys the model', :aggregate_failures do + it 'destroys the model' do allow_next_instance_of(Packages::MarkPackagesForDestructionService) do |instance| allow(instance).to receive(:execute).and_return ServiceResponse.success(message: "") end expect { service_result }.to change { Ml::Model.count }.by(-1).and change { Ml::ModelVersion.count }.by(-1) expect(service_result).to be_success + expect(Gitlab::Audit::Auditor).to have_received(:audit).with(audit_event) end context 'when a package cannot be marked for destruction' do @@ -40,10 +55,11 @@ end end - it 'returns success with warning', :aggregate_failures do + it 'returns success with warning' do expect { service_result }.not_to change { Ml::Model.count } expect(service_result).to be_error expect(service_result.message).to eq("An error") + expect(Gitlab::Audit::Auditor).not_to have_received(:audit) end end end