From 1099fdf32425b9d162299eae919a0eae5459a713 Mon Sep 17 00:00:00 2001 From: Eduardo Bonet Date: Fri, 6 Sep 2024 12:55:56 +0200 Subject: [PATCH 1/3] Adds audit events for self-hosted model creation Also refactors model creation into its own service Changelog: added --- .../types/self_hosted_model_created.yml | 9 +++ doc/user/compliance/audit_event_types.md | 6 ++ .../admin/ai/self_hosted_models_controller.rb | 7 +- .../mutations/ai/self_hosted_models/create.rb | 30 ++++---- .../ai/self_hosted_models/create_service.rb | 46 +++++++++++ .../ai/self_hosted_models/create_spec.rb | 2 +- .../self_hosted_models/create_service_spec.rb | 76 +++++++++++++++++++ 7 files changed, 156 insertions(+), 20 deletions(-) create mode 100644 config/audit_events/types/self_hosted_model_created.yml create mode 100644 ee/app/services/ai/self_hosted_models/create_service.rb create mode 100644 ee/spec/services/ai/self_hosted_models/create_service_spec.rb diff --git a/config/audit_events/types/self_hosted_model_created.yml b/config/audit_events/types/self_hosted_model_created.yml new file mode 100644 index 00000000000000..fa6203a73b23e1 --- /dev/null +++ b/config/audit_events/types/self_hosted_model_created.yml @@ -0,0 +1,9 @@ +name: self_hosted_model_created +description: A new self-hosted model configuration was added +introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/477999 +introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/165303 +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..cc1fc93a229a39 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_created`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/165303) | A new self-hosted model configuration was added | **{check-circle}** Yes | **{check-circle}** Yes | GitLab [17.4](https://gitlab.com/gitlab-org/gitlab/-/issues/477999) | 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..4a662b0664b573 100644 --- a/ee/app/controllers/admin/ai/self_hosted_models_controller.rb +++ b/ee/app/controllers/admin/ai/self_hosted_models_controller.rb @@ -20,9 +20,12 @@ def new end def create - @self_hosted_model = ::Ai::SelfHostedModel.create(self_hosted_models_params) + service_result = ::Ai::SelfHostedModels::CreateService.new( + self_hosted_models_params.merge(user: current_user) + ).execute - if @self_hosted_model.persisted? + if service_result.success? + @self_hosted_model = service_result.payload redirect_to admin_ai_self_hosted_models_url, notice: _("Self-Hosted Model was created") else render :new diff --git a/ee/app/graphql/mutations/ai/self_hosted_models/create.rb b/ee/app/graphql/mutations/ai/self_hosted_models/create.rb index 57599ac73955e7..ed7f0a427cf8d0 100644 --- a/ee/app/graphql/mutations/ai/self_hosted_models/create.rb +++ b/ee/app/graphql/mutations/ai/self_hosted_models/create.rb @@ -25,23 +25,19 @@ class Create < Base def resolve(**args) check_feature_access! - # TODO: We should create a service this is done for MVP sake - model = ::Ai::SelfHostedModel.create!( - name: args[:name], - model: args[:model], - endpoint: args[:endpoint], - api_token: args[:api_token] - ) - - { - self_hosted_model: model, - errors: [] # Errors are rescued below - } - rescue ActiveRecord::RecordInvalid => e - { - self_hosted_model: nil, - errors: [e.message] - } + result = ::Ai::SelfHostedModels::CreateService.new(args.merge(user: current_user)).execute + + if result.success? + { + self_hosted_model: result.payload, + errors: [] # Errors are rescued below + } + else + { + self_hosted_model: nil, + errors: [result.message] + } + end end end end diff --git a/ee/app/services/ai/self_hosted_models/create_service.rb b/ee/app/services/ai/self_hosted_models/create_service.rb new file mode 100644 index 00000000000000..70174058ea9aa5 --- /dev/null +++ b/ee/app/services/ai/self_hosted_models/create_service.rb @@ -0,0 +1,46 @@ +# frozen_string_literal: true + +module Ai + module SelfHostedModels + class CreateService + def initialize(params) + @name = params[:name] + @model = params[:model] + @endpoint = params[:endpoint] + @api_token = params[:api_token] + @user = params[:user] + end + + def execute + @self_hosted_model = ::Ai::SelfHostedModel.new(name: name, model: model, endpoint: endpoint, + api_token: api_token) + + if @self_hosted_model.save + audit_creation_event + + ServiceResponse.success(payload: @self_hosted_model) + else + ServiceResponse.error(message: @self_hosted_model.errors.full_messages.join(", ")) + end + end + + private + + def audit_creation_event + model = self_hosted_model + + audit_context = { + name: 'self_hosted_model_created', + author: user, + scope: user, + target: self_hosted_model, + message: "Self-hosted model #{model.name}/#{model.model}/#{model.endpoint} created" + } + + ::Gitlab::Audit::Auditor.audit(audit_context) + end + + attr_accessor :name, :model, :endpoint, :api_token, :self_hosted_model, :user + end + end +end diff --git a/ee/spec/requests/api/graphql/ai/self_hosted_models/create_spec.rb b/ee/spec/requests/api/graphql/ai/self_hosted_models/create_spec.rb index 1f9be3d8c2ca2c..a6dab4ee908d63 100644 --- a/ee/spec/requests/api/graphql/ai/self_hosted_models/create_spec.rb +++ b/ee/spec/requests/api/graphql/ai/self_hosted_models/create_spec.rb @@ -64,7 +64,7 @@ post_graphql_mutation(mutation, current_user: current_user) expect(mutation_response['selfHostedModel']).to be(nil) - expect(mutation_response['errors']).to include("Validation failed: Name can't be blank") + expect(mutation_response['errors']).to include("Name can't be blank") end it 'does not create a self-hosted model' do diff --git a/ee/spec/services/ai/self_hosted_models/create_service_spec.rb b/ee/spec/services/ai/self_hosted_models/create_service_spec.rb new file mode 100644 index 00000000000000..4401a41101734e --- /dev/null +++ b/ee/spec/services/ai/self_hosted_models/create_service_spec.rb @@ -0,0 +1,76 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe ::Ai::SelfHostedModels::CreateService, feature_category: :"self-hosted_models" do + let(:user) { create(:user) } + let(:base_params) do + { + name: 'Test Model', + model: 'codestral', + endpoint: 'https://api.example.com', + api_token: 'test_token', + user: user + } + end + + let(:params) { base_params } + let(:service) { described_class.new(params) } + + before do + allow(Gitlab::Audit::Auditor).to receive(:audit).and_call_original + end + + describe '#execute', :aggregate_failures do + subject(:result) { service.execute } + + let(:model) { result.payload } + let(:audit_event) do + { + name: 'self_hosted_model_created', + author: user, + scope: user, + target: model, + message: "Self-hosted model #{params[:name]}/#{params[:model]}/#{params[:endpoint]} created" + } + end + + context 'when params are valid' do + it 'creates the model' do + expect { result }.to change { ::Ai::SelfHostedModel.count }.by(1) + expect(result).to be_success + + expect(model.name).to eq('Test Model') + expect(model.model).to eq('codestral') + expect(model.endpoint).to eq('https://api.example.com') + expect(model.api_token).to eq('test_token') + + expect(Gitlab::Audit::Auditor).to have_received(:audit).with(audit_event) + end + end + + context 'when model is invalid' do + let(:params) { base_params.merge(model: 'invalid_model') } + + it 'raises error' do + expect { result }.to raise_error(ArgumentError, /'invalid_model' is not a valid model/) + + expect(Gitlab::Audit::Auditor).not_to receive(:audit).with(hash_including(name: "self_hosted_model_created")) + end + end + + context 'when params are invalid' do + let(:params) { base_params.merge(name: '', endpoint: 'not_a_url') } + + it 'returns an error response' do + expect { result }.not_to change { ::Ai::SelfHostedModel.count } + + expect(result).to be_error + expect(result.message).to include("Name can't be blank") + expect(result.message).to include("Endpoint is blocked: Only allowed schemes are http, https") + + expect(Gitlab::Audit::Auditor).not_to receive(:audit).with(hash_including(name: "self_hosted_model_created")) + end + end + end +end -- GitLab From 08fed2688b61e1aabb8a87f26d555d2b0ede0c19 Mon Sep 17 00:00:00 2001 From: Eduardo Bonet Date: Wed, 11 Sep 2024 11:43:09 +0200 Subject: [PATCH 2/3] Improves service codebase --- .../admin/ai/self_hosted_models_controller.rb | 4 +--- .../mutations/ai/self_hosted_models/create.rb | 2 +- .../ai/self_hosted_models/create_service.rb | 22 +++++++------------ 3 files changed, 10 insertions(+), 18 deletions(-) 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 3267d7ca8709f9..6bb87a3ceb9496 100644 --- a/ee/app/controllers/admin/ai/self_hosted_models_controller.rb +++ b/ee/app/controllers/admin/ai/self_hosted_models_controller.rb @@ -20,9 +20,7 @@ def new end def create - service_result = ::Ai::SelfHostedModels::CreateService.new( - self_hosted_models_params.merge(user: current_user) - ).execute + service_result = ::Ai::SelfHostedModels::CreateService.new(current_user, self_hosted_models_params).execute if service_result.success? @self_hosted_model = service_result.payload diff --git a/ee/app/graphql/mutations/ai/self_hosted_models/create.rb b/ee/app/graphql/mutations/ai/self_hosted_models/create.rb index ed7f0a427cf8d0..db6068b35f800d 100644 --- a/ee/app/graphql/mutations/ai/self_hosted_models/create.rb +++ b/ee/app/graphql/mutations/ai/self_hosted_models/create.rb @@ -25,7 +25,7 @@ class Create < Base def resolve(**args) check_feature_access! - result = ::Ai::SelfHostedModels::CreateService.new(args.merge(user: current_user)).execute + result = ::Ai::SelfHostedModels::CreateService.new(current_user, args).execute if result.success? { diff --git a/ee/app/services/ai/self_hosted_models/create_service.rb b/ee/app/services/ai/self_hosted_models/create_service.rb index 70174058ea9aa5..eee98bd1d2ba9b 100644 --- a/ee/app/services/ai/self_hosted_models/create_service.rb +++ b/ee/app/services/ai/self_hosted_models/create_service.rb @@ -3,20 +3,16 @@ module Ai module SelfHostedModels class CreateService - def initialize(params) - @name = params[:name] - @model = params[:model] - @endpoint = params[:endpoint] - @api_token = params[:api_token] - @user = params[:user] + def initialize(current_user, params) + @params = params + @user = current_user end def execute - @self_hosted_model = ::Ai::SelfHostedModel.new(name: name, model: model, endpoint: endpoint, - api_token: api_token) + @self_hosted_model = ::Ai::SelfHostedModel.new(params) if @self_hosted_model.save - audit_creation_event + audit_creation_event(@self_hosted_model) ServiceResponse.success(payload: @self_hosted_model) else @@ -26,21 +22,19 @@ def execute private - def audit_creation_event - model = self_hosted_model - + def audit_creation_event(model) audit_context = { name: 'self_hosted_model_created', author: user, scope: user, - target: self_hosted_model, + target: model, message: "Self-hosted model #{model.name}/#{model.model}/#{model.endpoint} created" } ::Gitlab::Audit::Auditor.audit(audit_context) end - attr_accessor :name, :model, :endpoint, :api_token, :self_hosted_model, :user + attr_accessor :user, :params end end end -- GitLab From 7f65fb8a44429382fd6207e11d1795d28c6e5f60 Mon Sep 17 00:00:00 2001 From: Eduardo Bonet Date: Wed, 11 Sep 2024 13:54:43 +0200 Subject: [PATCH 3/3] Fixes failing test --- .../services/ai/self_hosted_models/create_service_spec.rb | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/ee/spec/services/ai/self_hosted_models/create_service_spec.rb b/ee/spec/services/ai/self_hosted_models/create_service_spec.rb index 4401a41101734e..b121aef26ba2cc 100644 --- a/ee/spec/services/ai/self_hosted_models/create_service_spec.rb +++ b/ee/spec/services/ai/self_hosted_models/create_service_spec.rb @@ -9,13 +9,12 @@ name: 'Test Model', model: 'codestral', endpoint: 'https://api.example.com', - api_token: 'test_token', - user: user + api_token: 'test_token' } end let(:params) { base_params } - let(:service) { described_class.new(params) } + let(:service) { described_class.new(user, params) } before do allow(Gitlab::Audit::Auditor).to receive(:audit).and_call_original -- GitLab