diff --git a/app/controllers/projects/ml/candidates_controller.rb b/app/controllers/projects/ml/candidates_controller.rb index ed7155fc5f48a198683ec7b9814115f2644f5149..9905e454acb819c5e095eba241be78f39df7aa4a 100644 --- a/app/controllers/projects/ml/candidates_controller.rb +++ b/app/controllers/projects/ml/candidates_controller.rb @@ -3,7 +3,9 @@ module Projects module Ml class CandidatesController < ApplicationController - before_action :check_feature_enabled, :set_candidate + before_action :set_candidate + before_action :check_read, only: [:show] + before_action :check_write, only: [:destroy] feature_category :mlops @@ -26,9 +28,13 @@ def set_candidate render_404 unless @candidate.present? end - def check_feature_enabled + def check_read render_404 unless can?(current_user, :read_model_experiments, @project) end + + def check_write + render_404 unless can?(current_user, :write_model_experiments, @project) + end end end end diff --git a/app/controllers/projects/ml/experiments_controller.rb b/app/controllers/projects/ml/experiments_controller.rb index a620e9919e745287142c38c70c4d3e97e5654e85..85e7f63779c53ead6c74691ebc982327c0c6ca0f 100644 --- a/app/controllers/projects/ml/experiments_controller.rb +++ b/app/controllers/projects/ml/experiments_controller.rb @@ -5,7 +5,8 @@ module Ml class ExperimentsController < ::Projects::ApplicationController include Projects::Ml::ExperimentsHelper - before_action :check_feature_enabled + before_action :check_read, only: [:show, :index] + before_action :check_write, only: [:destroy] before_action :set_experiment, only: [:show, :destroy] feature_category :mlops @@ -55,10 +56,14 @@ def destroy private - def check_feature_enabled + def check_read render_404 unless can?(current_user, :read_model_experiments, @project) end + def check_write + render_404 unless can?(current_user, :write_model_experiments, @project) + end + def set_experiment @experiment = ::Ml::Experiment.by_project_id_and_iid(@project.id, params[:iid]) diff --git a/app/policies/project_policy.rb b/app/policies/project_policy.rb index 0891c66853e367726d4f2bae8a8f2c19fa174c53..ad6155258abe8624b6667a33ea9833a6af190eac 100644 --- a/app/policies/project_policy.rb +++ b/app/policies/project_policy.rb @@ -905,6 +905,10 @@ class ProjectPolicy < BasePolicy enable :read_model_experiments end + rule { can?(:reporter_access) & model_experiments_enabled }.policy do + enable :write_model_experiments + end + rule { ~admin & created_and_owned_by_banned_user }.policy do prevent :read_project end diff --git a/doc/user/project/ml/experiment_tracking/index.md b/doc/user/project/ml/experiment_tracking/index.md index 584a6c0df5971412c562b65087bc273689b2f552..98359911c06877e17ea3a58b7b6cc58836c01fc6 100644 --- a/doc/user/project/ml/experiment_tracking/index.md +++ b/doc/user/project/ml/experiment_tracking/index.md @@ -14,6 +14,12 @@ On GitLab.com, this feature is enabled on all projects. NOTE: Model experiment tracking is an [experimental feature](../../../../policy/experiment-beta-support.md). Refer to for feedback and feature requests. +ACCESS LEVEL: +Model experiments [visibility level](../../../public_access.md) can be set to public, private or disabled. This options can +be configured under `Settings > General > Visibility, project features, permissions > Model experiments`. Users must have +at least [Reporter role](../../../permissions.md#roles) to modify or delete experiments +and candidate data. + When creating machine learning models, data scientists often experiment with different parameters, configurations, and feature engineering to improve the performance of the model. Keeping track of all this metadata and the associated artifacts so that the data scientist can later replicate the experiment is not trivial. Machine learning experiment @@ -64,10 +70,6 @@ on how to use GitLab as a backend for the MLflow Client. ## Explore model candidates -Prerequisites: - -- You must have at least the Developer role to view experiment data. - To list the current active experiments, either go to `https/-/ml/experiments` or: 1. On the left sidebar, at the top, select **Search GitLab** (**{search}**) to find your project. diff --git a/spec/policies/project_policy_spec.rb b/spec/policies/project_policy_spec.rb index 42fe301ea9eba170bb61a86c8d0d9750bf0ae7a4..602b7148d0e527f44512113fe5e9af9ee23856e4 100644 --- a/spec/policies/project_policy_spec.rb +++ b/spec/policies/project_policy_spec.rb @@ -3217,6 +3217,31 @@ def permissions_abilities(role) end end + describe ':write_model_experiments' do + using RSpec::Parameterized::TableSyntax + + where(:ff_ml_experiment_tracking, :current_user, :access_level, :allowed) do + false | ref(:owner) | Featurable::ENABLED | false + true | ref(:reporter) | Featurable::ENABLED | true + true | ref(:reporter) | Featurable::PRIVATE | true + true | ref(:reporter) | Featurable::DISABLED | false + true | ref(:guest) | Featurable::ENABLED | false + true | ref(:non_member) | Featurable::ENABLED | false + end + with_them do + before do + stub_feature_flags(ml_experiment_tracking: ff_ml_experiment_tracking) + project.project_feature.update!(model_experiments_access_level: access_level) + end + + if params[:allowed] + it { is_expected.to be_allowed(:write_model_experiments) } + else + it { is_expected.not_to be_allowed(:write_model_experiments) } + end + end + end + describe 'when project is created and owned by a banned user' do let_it_be(:project) { create(:project, :public) } diff --git a/spec/requests/projects/ml/candidates_controller_spec.rb b/spec/requests/projects/ml/candidates_controller_spec.rb index eec7af990637dc5f5f66984298f3a28f1e73f9f1..4c7491970e1255f8dc851716ed400f862a395600 100644 --- a/spec/requests/projects/ml/candidates_controller_spec.rb +++ b/spec/requests/projects/ml/candidates_controller_spec.rb @@ -10,13 +10,17 @@ let(:ff_value) { true } let(:candidate_iid) { candidate.iid } - let(:model_experiments_enabled) { true } + let(:read_model_experiments) { true } + let(:write_model_experiments) { true } before do allow(Ability).to receive(:allowed?).and_call_original allow(Ability).to receive(:allowed?) .with(user, :read_model_experiments, project) - .and_return(model_experiments_enabled) + .and_return(read_model_experiments) + allow(Ability).to receive(:allowed?) + .with(user, :write_model_experiments, project) + .and_return(write_model_experiments) sign_in(user) end @@ -34,9 +38,9 @@ end end - shared_examples '404 when model experiments is unavailable' do + shared_examples 'requires read_model_experiments' do context 'when user does not have access' do - let(:model_experiments_enabled) { false } + let(:read_model_experiments) { false } it_behaves_like 'renders 404' end @@ -61,7 +65,7 @@ end it_behaves_like '404 if candidate does not exist' - it_behaves_like '404 when model experiments is unavailable' + it_behaves_like 'requires read_model_experiments' end describe 'DELETE #destroy' do @@ -83,7 +87,14 @@ end it_behaves_like '404 if candidate does not exist' - it_behaves_like '404 when model experiments is unavailable' + + describe 'requires write_model_experiments' do + let(:write_model_experiments) { false } + + it 'is 404' do + expect(response).to have_gitlab_http_status(:not_found) + end + end end private diff --git a/spec/requests/projects/ml/experiments_controller_spec.rb b/spec/requests/projects/ml/experiments_controller_spec.rb index e2d26e84f752d8e09b1da3b4bf509bf1de6167b9..9440c716640f6bc72fbf53422d799266df9cb16e 100644 --- a/spec/requests/projects/ml/experiments_controller_spec.rb +++ b/spec/requests/projects/ml/experiments_controller_spec.rb @@ -15,13 +15,17 @@ let(:ff_value) { true } let(:basic_params) { { namespace_id: project.namespace.to_param, project_id: project } } let(:experiment_iid) { experiment.iid } - let(:model_experiments_enabled) { true } + let(:read_model_experiments) { true } + let(:write_model_experiments) { true } before do allow(Ability).to receive(:allowed?).and_call_original allow(Ability).to receive(:allowed?) .with(user, :read_model_experiments, project) - .and_return(model_experiments_enabled) + .and_return(read_model_experiments) + allow(Ability).to receive(:allowed?) + .with(user, :write_model_experiments, project) + .and_return(write_model_experiments) sign_in(user) end @@ -40,9 +44,9 @@ end end - shared_examples '404 when model experiments is unavailable' do + shared_examples 'requires read_model_experiments' do context 'when user does not have access' do - let(:model_experiments_enabled) { false } + let(:read_model_experiments) { false } it_behaves_like 'renders 404' end @@ -100,7 +104,7 @@ end end - it_behaves_like '404 when model experiments is unavailable' do + it_behaves_like 'requires read_model_experiments' do before do list_experiments end @@ -211,7 +215,7 @@ end it_behaves_like '404 if experiment does not exist' - it_behaves_like '404 when model experiments is unavailable' + it_behaves_like 'requires read_model_experiments' end end @@ -243,7 +247,7 @@ end it_behaves_like '404 if experiment does not exist' - it_behaves_like '404 when model experiments is unavailable' + it_behaves_like 'requires read_model_experiments' end end end @@ -268,7 +272,14 @@ end it_behaves_like '404 if experiment does not exist' - it_behaves_like '404 when model experiments is unavailable' + + describe 'requires write_model_experiments' do + let(:write_model_experiments) { false } + + it 'is 404' do + expect(response).to have_gitlab_http_status(:not_found) + end + end end private