From 190831bff0c152eac613b89a3188c7901d5a2c01 Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Wed, 27 Sep 2023 22:34:09 +0000 Subject: Add latest changes from gitlab-org/security/gitlab@16-2-stable-ee --- .../projects/ml/candidates_controller.rb | 4 ++- app/helpers/projects/ml/experiments_helper.rb | 8 ++--- app/presenters/ml/candidate_details_presenter.rb | 7 ++-- app/views/projects/ml/candidates/show.html.haml | 2 +- app/views/projects/ml/experiments/show.html.haml | 2 +- .../Jobs/DAST-Default-Branch-Deploy.gitlab-ci.yml | 2 +- lib/gitlab/ci/templates/Jobs/Deploy.gitlab-ci.yml | 2 +- .../ci/templates/Jobs/Deploy.latest.gitlab-ci.yml | 2 +- .../helpers/projects/ml/experiments_helper_spec.rb | 14 +++++++- spec/models/group_spec.rb | 39 ++++++++++++++++++++++ .../ml/candidate_details_presenter_spec.rb | 14 +++++++- .../projects/ml/candidates_controller_spec.rb | 26 ++++++++++++++- 12 files changed, 106 insertions(+), 16 deletions(-) diff --git a/app/controllers/projects/ml/candidates_controller.rb b/app/controllers/projects/ml/candidates_controller.rb index 9905e454acb..12111c45fde 100644 --- a/app/controllers/projects/ml/candidates_controller.rb +++ b/app/controllers/projects/ml/candidates_controller.rb @@ -9,7 +9,9 @@ module Projects feature_category :mlops - def show; end + def show + @include_ci_info = @candidate.from_ci? && can?(current_user, :read_build, @candidate.ci_build) + end def destroy @experiment = @candidate.experiment diff --git a/app/helpers/projects/ml/experiments_helper.rb b/app/helpers/projects/ml/experiments_helper.rb index 7aef208447a..d5b2c3cd36a 100644 --- a/app/helpers/projects/ml/experiments_helper.rb +++ b/app/helpers/projects/ml/experiments_helper.rb @@ -14,12 +14,12 @@ module Projects Gitlab::Json.generate(data) end - def candidates_table_items(candidates) + def candidates_table_items(candidates, user) items = candidates.map do |candidate| { **candidate.params.to_h { |p| [p.name, p.value] }, **candidate.latest_metrics.to_h { |m| [m.name, number_with_precision(m.value, precision: 4)] }, - ci_job: job_info(candidate), + ci_job: job_info(candidate, user), artifact: link_to_artifact(candidate), details: link_to_details(candidate), name: candidate.name, @@ -72,8 +72,8 @@ module Projects project_ml_candidate_path(candidate.project, candidate.iid) end - def job_info(candidate) - return unless candidate.from_ci? + def job_info(candidate, user) + return unless candidate.from_ci? && can?(user, :read_build, candidate.ci_build) build = candidate.ci_build diff --git a/app/presenters/ml/candidate_details_presenter.rb b/app/presenters/ml/candidate_details_presenter.rb index 7f0bd9d6c11..29d4617903f 100644 --- a/app/presenters/ml/candidate_details_presenter.rb +++ b/app/presenters/ml/candidate_details_presenter.rb @@ -4,8 +4,9 @@ module Ml class CandidateDetailsPresenter include Rails.application.routes.url_helpers - def initialize(candidate) + def initialize(candidate, include_ci_job = false) @candidate = candidate + @include_ci_job = include_ci_job end def present @@ -32,10 +33,10 @@ module Ml private - attr_reader :candidate + attr_reader :candidate, :include_ci_job def job_info - return unless candidate.from_ci? + return unless include_ci_job && candidate.from_ci? build = candidate.ci_build diff --git a/app/views/projects/ml/candidates/show.html.haml b/app/views/projects/ml/candidates/show.html.haml index 5d5059551d3..979bc107fd2 100644 --- a/app/views/projects/ml/candidates/show.html.haml +++ b/app/views/projects/ml/candidates/show.html.haml @@ -3,6 +3,6 @@ - add_to_breadcrumbs experiment.name, project_ml_experiment_path(@project, experiment.iid) - breadcrumb_title "Candidate #{@candidate.iid}" - add_page_specific_style 'page_bundles/ml_experiment_tracking' -- presenter = ::Ml::CandidateDetailsPresenter.new(@candidate) +- presenter = ::Ml::CandidateDetailsPresenter.new(@candidate, @include_ci_info) #js-show-ml-candidate{ data: { view_model: presenter.present } } diff --git a/app/views/projects/ml/experiments/show.html.haml b/app/views/projects/ml/experiments/show.html.haml index d4a05b613c9..85f3f85fd8b 100644 --- a/app/views/projects/ml/experiments/show.html.haml +++ b/app/views/projects/ml/experiments/show.html.haml @@ -4,7 +4,7 @@ - add_page_specific_style 'page_bundles/ml_experiment_tracking' - experiment = experiment_as_data(@experiment) -- items = candidates_table_items(@candidates) +- items = candidates_table_items(@candidates, current_user) - metrics = unique_logged_names(@candidates, &:latest_metrics) - params = unique_logged_names(@candidates, &:params) - page_info = formatted_page_info(@page_info) diff --git a/lib/gitlab/ci/templates/Jobs/DAST-Default-Branch-Deploy.gitlab-ci.yml b/lib/gitlab/ci/templates/Jobs/DAST-Default-Branch-Deploy.gitlab-ci.yml index b1e498a9d09..77528082069 100644 --- a/lib/gitlab/ci/templates/Jobs/DAST-Default-Branch-Deploy.gitlab-ci.yml +++ b/lib/gitlab/ci/templates/Jobs/DAST-Default-Branch-Deploy.gitlab-ci.yml @@ -1,5 +1,5 @@ variables: - DAST_AUTO_DEPLOY_IMAGE_VERSION: 'v2.51.0' + DAST_AUTO_DEPLOY_IMAGE_VERSION: 'v2.55.0' .dast-auto-deploy: image: "${CI_TEMPLATE_REGISTRY_HOST}/gitlab-org/cluster-integration/auto-deploy-image:${DAST_AUTO_DEPLOY_IMAGE_VERSION}" diff --git a/lib/gitlab/ci/templates/Jobs/Deploy.gitlab-ci.yml b/lib/gitlab/ci/templates/Jobs/Deploy.gitlab-ci.yml index 5a7e69b62d9..65f3e259c3c 100644 --- a/lib/gitlab/ci/templates/Jobs/Deploy.gitlab-ci.yml +++ b/lib/gitlab/ci/templates/Jobs/Deploy.gitlab-ci.yml @@ -1,5 +1,5 @@ variables: - AUTO_DEPLOY_IMAGE_VERSION: 'v2.51.0' + AUTO_DEPLOY_IMAGE_VERSION: 'v2.55.0' .auto-deploy: image: "${CI_TEMPLATE_REGISTRY_HOST}/gitlab-org/cluster-integration/auto-deploy-image:${AUTO_DEPLOY_IMAGE_VERSION}" diff --git a/lib/gitlab/ci/templates/Jobs/Deploy.latest.gitlab-ci.yml b/lib/gitlab/ci/templates/Jobs/Deploy.latest.gitlab-ci.yml index dac559db8d5..0211b296b22 100644 --- a/lib/gitlab/ci/templates/Jobs/Deploy.latest.gitlab-ci.yml +++ b/lib/gitlab/ci/templates/Jobs/Deploy.latest.gitlab-ci.yml @@ -1,5 +1,5 @@ variables: - AUTO_DEPLOY_IMAGE_VERSION: 'v2.51.0' + AUTO_DEPLOY_IMAGE_VERSION: 'v2.55.0' .auto-deploy: image: "${CI_TEMPLATE_REGISTRY_HOST}/gitlab-org/cluster-integration/auto-deploy-image:${AUTO_DEPLOY_IMAGE_VERSION}" diff --git a/spec/helpers/projects/ml/experiments_helper_spec.rb b/spec/helpers/projects/ml/experiments_helper_spec.rb index 021d518a329..569fd0f9ec5 100644 --- a/spec/helpers/projects/ml/experiments_helper_spec.rb +++ b/spec/helpers/projects/ml/experiments_helper_spec.rb @@ -36,7 +36,7 @@ RSpec.describe Projects::Ml::ExperimentsHelper, feature_category: :mlops do let_it_be(:candidates) { [candidate0, candidate1] } describe '#candidates_table_items' do - subject { Gitlab::Json.parse(helper.candidates_table_items(candidates)) } + subject { Gitlab::Json.parse(helper.candidates_table_items(candidates, project.creator)) } it 'creates the correct model for the table', :aggregate_failures do expected_values = [ @@ -72,6 +72,18 @@ RSpec.describe Projects::Ml::ExperimentsHelper, feature_category: :mlops do expect(subject[0]['user']).to be_nil end end + + context 'when user is not allowed to read the project' do + before do + allow(Ability).to receive(:allowed?) + .with(project.creator, :read_build, build) + .and_return(false) + end + + it 'does not include ci info' do + expect(subject[0]['ci_job']).to be_nil + end + end end describe '#unique_logged_names' do diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index 01fd17bfe10..5457fe2abaf 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -1504,6 +1504,45 @@ RSpec.describe Group, feature_category: :groups_and_projects do it { expect(subject.parent).to be_kind_of(described_class) } end + describe '#member?' do + let_it_be(:group) { create(:group) } + let_it_be(:user) { create(:user) } + + before_all do + group.add_developer(user) + end + + subject { group.member?(user) } + + context 'when user is a developer' do + it 'returns true' do + expect(group.member?(user)).to be_truthy + end + + it 'returns false with maintainer as min_access_level param' do + expect(group.member?(user, Gitlab::Access::MAINTAINER)).to be_falsey + end + end + + context 'in shared group' do + let(:shared_group) { create(:group) } + let(:member_shared) { create(:user) } + + before do + create(:group_group_link, shared_group: group, shared_with_group: shared_group) + shared_group.add_developer(member_shared) + end + + it 'return true for shared group member' do + expect(group.member?(member_shared)).to be_truthy + end + + it 'returns false with maintainer as min_access_level param' do + expect(group.member?(member_shared, Gitlab::Access::MAINTAINER)).to be_falsey + end + end + end + describe '#max_member_access_for_user' do let_it_be(:group_user) { create(:user) } diff --git a/spec/presenters/ml/candidate_details_presenter_spec.rb b/spec/presenters/ml/candidate_details_presenter_spec.rb index 9d1f6f634e4..0ecf80b683e 100644 --- a/spec/presenters/ml/candidate_details_presenter_spec.rb +++ b/spec/presenters/ml/candidate_details_presenter_spec.rb @@ -25,7 +25,9 @@ RSpec.describe ::Ml::CandidateDetailsPresenter, feature_category: :mlops do ] end - subject { Gitlab::Json.parse(described_class.new(candidate).present)['candidate'] } + let(:include_ci_job) { true } + + subject { Gitlab::Json.parse(described_class.new(candidate, include_ci_job).present)['candidate'] } before do allow(candidate).to receive(:latest_metrics).and_return(metrics) @@ -68,6 +70,8 @@ RSpec.describe ::Ml::CandidateDetailsPresenter, feature_category: :mlops do let_it_be(:pipeline) { build_stubbed(:ci_pipeline, project: project, user: user) } let_it_be(:build) { candidate.ci_build = build_stubbed(:ci_build, pipeline: pipeline, user: user) } + let(:can_read_build) { true } + it 'generates the correct ci' do expected_info = { 'path' => "/#{project.full_path}/-/jobs/#{build.id}", @@ -109,6 +113,14 @@ RSpec.describe ::Ml::CandidateDetailsPresenter, feature_category: :mlops do expect(subject.dig('info', 'ci_job', 'merge_request')).to include(expected_info) end end + + context 'when ci job is not to be added' do + let(:include_ci_job) { false } + + it 'ci_job is nil' do + expect(subject.dig('info', 'ci_job')).to be_nil + end + end end end end diff --git a/spec/requests/projects/ml/candidates_controller_spec.rb b/spec/requests/projects/ml/candidates_controller_spec.rb index 4c7491970e1..78f31be26d1 100644 --- a/spec/requests/projects/ml/candidates_controller_spec.rb +++ b/spec/requests/projects/ml/candidates_controller_spec.rb @@ -6,7 +6,11 @@ RSpec.describe Projects::Ml::CandidatesController, feature_category: :mlops do let_it_be(:project) { create(:project, :repository) } let_it_be(:user) { project.first_owner } let_it_be(:experiment) { create(:ml_experiments, project: project, user: user) } - let_it_be(:candidate) { create(:ml_candidates, experiment: experiment, user: user, project: project) } + let_it_be(:candidate) do + create(:ml_candidates, experiment: experiment, user: user, project: project).tap do |c| + c.update!(ci_build: create(:ci_build)) + end + end let(:ff_value) { true } let(:candidate_iid) { candidate.iid } @@ -47,7 +51,13 @@ RSpec.describe Projects::Ml::CandidatesController, feature_category: :mlops do end describe 'GET show' do + let(:can_read_build) { true } + before do + allow(Ability).to receive(:allowed?) + .with(user, :read_build, candidate.ci_build) + .and_return(can_read_build) + show_candidate end @@ -64,6 +74,20 @@ RSpec.describe Projects::Ml::CandidatesController, feature_category: :mlops do expect { show_candidate }.not_to exceed_all_query_limit(control_count) end + context 'when user has permission to read the build' do + it 'includes ci build info' do + expect(assigns[:include_ci_info]).to eq(true) + end + end + + context 'when user has no permission to read the build' do + let(:can_read_build) { false } + + it 'sets include_ci_job to false' do + expect(assigns[:include_ci_info]).to eq(false) + end + end + it_behaves_like '404 if candidate does not exist' it_behaves_like 'requires read_model_experiments' end -- cgit v1.2.3