From 60db4c56b2f56b42f2ba6c6a2e276f1377b263ee Mon Sep 17 00:00:00 2001 From: Eduardo Bonet Date: Mon, 9 Sep 2024 15:26:42 +0200 Subject: [PATCH 1/2] Adds audit events for updating self-hosted models Refactors update of SelfHostedModels to a service Audits update to self-hosted model configurations Changelog: added --- .../types/self_hosted_model_updated.yml | 9 ++++ doc/user/compliance/audit_event_types.md | 6 +++ .../admin/ai/self_hosted_models_controller.rb | 6 ++- .../mutations/ai/self_hosted_models/update.rb | 26 +++------ .../ai/self_hosted_models/update_service.rb | 40 ++++++++++++++ .../self_hosted_models/update_service_spec.rb | 53 +++++++++++++++++++ 6 files changed, 121 insertions(+), 19 deletions(-) create mode 100644 config/audit_events/types/self_hosted_model_updated.yml create mode 100644 ee/app/services/ai/self_hosted_models/update_service.rb create mode 100644 ee/spec/services/ai/self_hosted_models/update_service_spec.rb diff --git a/config/audit_events/types/self_hosted_model_updated.yml b/config/audit_events/types/self_hosted_model_updated.yml new file mode 100644 index 00000000000000..0746ff0004b68f --- /dev/null +++ b/config/audit_events/types/self_hosted_model_updated.yml @@ -0,0 +1,9 @@ +name: self_hosted_model_updated +description: A self-hosted model configuration was updated +introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/483295 +introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/165520 +feature_category: self-hosted_models +milestone: '17.4' +saved_to_database: true +scope: [Instance, User] +streamed: true diff --git a/doc/user/compliance/audit_event_types.md b/doc/user/compliance/audit_event_types.md index f38d27ef7a4091..a55ae556ce528f 100644 --- a/doc/user/compliance/audit_event_types.md +++ b/doc/user/compliance/audit_event_types.md @@ -449,6 +449,12 @@ Audit event types belong to the following product categories. |:------------|:------------|:------------------|:---------|:--------------|:--------------| | [`policy_project_updated`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/102154) | This event is triggered whenever the security policy project is updated for a project. | **{check-circle}** Yes | **{check-circle}** Yes | GitLab [15.6](https://gitlab.com/gitlab-org/gitlab/-/issues/377877) | Group, Project | +### Self-hosted models + +| Name | Description | Saved to database | Streamed | Introduced in | Scope | +|:------------|:------------|:------------------|:---------|:--------------|:--------------| +| [`self_hosted_model_updated`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/165520) | A self-hosted model configuration was updated | **{check-circle}** Yes | **{check-circle}** Yes | GitLab [17.4](https://gitlab.com/gitlab-org/gitlab/-/issues/483295) | Instance, User | + ### Source code management | Name | Description | Saved to database | Streamed | Introduced in | Scope | diff --git a/ee/app/controllers/admin/ai/self_hosted_models_controller.rb b/ee/app/controllers/admin/ai/self_hosted_models_controller.rb index 7d8cead2550689..5e169fb676aa9c 100644 --- a/ee/app/controllers/admin/ai/self_hosted_models_controller.rb +++ b/ee/app/controllers/admin/ai/self_hosted_models_controller.rb @@ -36,7 +36,11 @@ def edit def update @self_hosted_model = ::Ai::SelfHostedModel.find(params[:id]) - if @self_hosted_model.update(update_self_hosted_model_params) + result = ::Ai::SelfHostedModels::UpdateService.new( + @self_hosted_model, current_user, update_self_hosted_model_params + ).execute + + if result.success? redirect_to admin_ai_self_hosted_models_url, notice: _("Self-Hosted Model was updated") else render :edit diff --git a/ee/app/graphql/mutations/ai/self_hosted_models/update.rb b/ee/app/graphql/mutations/ai/self_hosted_models/update.rb index e8d11b4d85bc85..1aee1f49ae0b30 100644 --- a/ee/app/graphql/mutations/ai/self_hosted_models/update.rb +++ b/ee/app/graphql/mutations/ai/self_hosted_models/update.rb @@ -31,28 +31,18 @@ class Update < Base def resolve(**args) check_feature_access! - model = update_self_hosted_model(args) - - if model.errors.present? - { - self_hosted_model: nil, - errors: Array(model.errors) - } - else - { self_hosted_model: model, errors: [] } - end - end - - private + model = find_object(id: args.delete(:id)) - def update_self_hosted_model(args) - model = find_object(id: args[:id]) + result = ::Ai::SelfHostedModels::UpdateService.new(model, current_user, args).execute - model.update(args.except(:id)) - - model + { + self_hosted_model: result.success? ? result.payload : nil, + errors: result.error? ? Array.wrap(result.errors) : [] + } end + private + def find_object(id:) GitlabSchema.object_from_id(id, expected_type: ::Ai::SelfHostedModel).sync end diff --git a/ee/app/services/ai/self_hosted_models/update_service.rb b/ee/app/services/ai/self_hosted_models/update_service.rb new file mode 100644 index 00000000000000..fee1350cbf4a42 --- /dev/null +++ b/ee/app/services/ai/self_hosted_models/update_service.rb @@ -0,0 +1,40 @@ +# frozen_string_literal: true + +module Ai + module SelfHostedModels + class UpdateService + def initialize(self_hosted_model, user, update_params) + @self_hosted_model = self_hosted_model + @user = user + @params = update_params + end + + def execute + if self_hosted_model.update(params) + audit_event + + ServiceResponse.success(payload: self_hosted_model) + else + ServiceResponse.error(message: self_hosted_model.errors.full_messages.join(", ")) + end + end + + private + + attr_accessor :self_hosted_model, :user, :params + + def audit_event + model = self_hosted_model + audit_context = { + name: 'self_hosted_model_updated', + author: user, + scope: user, + target: model, + message: "Self-hosted model #{model.name}/#{model.model}/#{model.endpoint} updated" + } + + ::Gitlab::Audit::Auditor.audit(audit_context) + end + end + end +end diff --git a/ee/spec/services/ai/self_hosted_models/update_service_spec.rb b/ee/spec/services/ai/self_hosted_models/update_service_spec.rb new file mode 100644 index 00000000000000..842f9c36ebcd00 --- /dev/null +++ b/ee/spec/services/ai/self_hosted_models/update_service_spec.rb @@ -0,0 +1,53 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe ::Ai::SelfHostedModels::UpdateService, feature_category: :"self-hosted_models" do + let_it_be(:user) { create(:user) } + + let(:self_hosted_model) { create(:ai_self_hosted_model) } + + let(:params) { {} } + let(:service) { described_class.new(self_hosted_model, user, params) } + + let(:audit_event) do + model = self_hosted_model + { + name: 'self_hosted_model_updated', + author: user, + scope: user, + target: model, + message: "Self-hosted model new model name/#{model.model}/#{model.endpoint} updated" + } + end + + describe '#execute', :aggregate_failures do + subject(:result) { service.execute } + + context 'when the model is successfully updated' do + let(:params) { { name: "new model name" } } + + it 'returns a success response' do + expect(Gitlab::Audit::Auditor).to receive(:audit).with(audit_event) + + result + + expect(self_hosted_model.reload.name).to eq("new model name") + + expect(result).to be_success + expect(result.payload).to eq(self_hosted_model) + end + end + + context 'when the model fails to be updated' do + let(:params) { { endpoint: nil } } + + it 'returns an error response' do + expect(Gitlab::Audit::Auditor).not_to receive(:audit) + + expect(result).to be_error + expect(result.message).to eq("Endpoint can't be blank, Endpoint must be a valid URL") + end + end + end +end -- GitLab From 535247686f17ebf78649da063e2e39b2b03ec345 Mon Sep 17 00:00:00 2001 From: Eduardo Bonet Date: Tue, 10 Sep 2024 14:10:08 +0200 Subject: [PATCH 2/2] Fixes test to call audit --- ee/app/services/ai/self_hosted_models/update_service.rb | 4 ++-- ee/spec/services/ai/self_hosted_models/update_service_spec.rb | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/ee/app/services/ai/self_hosted_models/update_service.rb b/ee/app/services/ai/self_hosted_models/update_service.rb index fee1350cbf4a42..be16590430442f 100644 --- a/ee/app/services/ai/self_hosted_models/update_service.rb +++ b/ee/app/services/ai/self_hosted_models/update_service.rb @@ -11,7 +11,7 @@ def initialize(self_hosted_model, user, update_params) def execute if self_hosted_model.update(params) - audit_event + record_audit_event ServiceResponse.success(payload: self_hosted_model) else @@ -23,7 +23,7 @@ def execute attr_accessor :self_hosted_model, :user, :params - def audit_event + def record_audit_event model = self_hosted_model audit_context = { name: 'self_hosted_model_updated', diff --git a/ee/spec/services/ai/self_hosted_models/update_service_spec.rb b/ee/spec/services/ai/self_hosted_models/update_service_spec.rb index 842f9c36ebcd00..2474eb06c65576 100644 --- a/ee/spec/services/ai/self_hosted_models/update_service_spec.rb +++ b/ee/spec/services/ai/self_hosted_models/update_service_spec.rb @@ -28,7 +28,7 @@ let(:params) { { name: "new model name" } } it 'returns a success response' do - expect(Gitlab::Audit::Auditor).to receive(:audit).with(audit_event) + expect(Gitlab::Audit::Auditor).to receive(:audit).with(audit_event).and_call_original result -- GitLab