From d047fbbed834ae6894a90593ff5f469014d36312 Mon Sep 17 00:00:00 2001 From: Jessie Young <jessieyoung@gitlab.com> Date: Thu, 8 Feb 2024 00:15:20 +0000 Subject: [PATCH] Use `duo_features_enabled` setting * Can be set on a project to block code suggestions * Later on, we will use the same setting name to block duo chat and other features on projects. We will also use the same setting for groups and subgroups. * This will not take effect until we enable the `purchase_code_suggestions` feature flag because until then the group setting for `code_suggestions` + Ultimate license enableds access. * After whereas after that flag is enabled, access will be determined by whether the user has a licensed seat. See https://gitlab.com/gitlab-org/gitlab/-/merge_requests/138334 * https://gitlab.com/groups/gitlab-org/-/epics/12404#note_1748487091 --- app/models/project.rb | 11 +-- ...uo_features_enabled_to_project_settings.rb | 11 +++ db/schema_migrations/20240124212938 | 1 + db/structure.sql | 1 + ee/app/models/ee/project.rb | 19 +++++ ee/spec/models/ee/project_spec.rb | 79 +++++++++++++++++++ ee/spec/models/namespace_setting_spec.rb | 2 +- ee/spec/requests/api/code_suggestions_spec.rb | 48 +++++------ lib/api/entities/project.rb | 1 - spec/factories/projects.rb | 8 +- spec/models/project_spec.rb | 8 ++ spec/requests/api/project_attributes.yml | 2 + 12 files changed, 156 insertions(+), 35 deletions(-) create mode 100644 db/migrate/20240124212938_add_duo_features_enabled_to_project_settings.rb create mode 100644 db/schema_migrations/20240124212938 diff --git a/app/models/project.rb b/app/models/project.rb index 7f1c3cd475c2..a5b33f654167 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -557,6 +557,7 @@ def self.integration_association_name(name) delegate :enforce_auth_checks_on_uploads, :enforce_auth_checks_on_uploads= delegate :warn_about_potentially_unwanted_characters, :warn_about_potentially_unwanted_characters= delegate :code_suggestions, :code_suggestions= + delegate :duo_features_enabled, :duo_features_enabled= end end @@ -3236,11 +3237,6 @@ def instance_runner_running_jobs_count end strong_memoize_attr :instance_runner_running_jobs_count - def code_suggestions_enabled? - code_suggestions && (group.nil? || group.code_suggestions) - end - strong_memoize_attr :code_suggestions_enabled? - # Overridden in EE def allows_multiple_merge_request_assignees? false @@ -3256,6 +3252,11 @@ def on_demand_dast_available? false end + # Overridden in EE + def code_suggestions_enabled? + false + end + private # overridden in EE diff --git a/db/migrate/20240124212938_add_duo_features_enabled_to_project_settings.rb b/db/migrate/20240124212938_add_duo_features_enabled_to_project_settings.rb new file mode 100644 index 000000000000..9dd09e1434aa --- /dev/null +++ b/db/migrate/20240124212938_add_duo_features_enabled_to_project_settings.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +class AddDuoFeaturesEnabledToProjectSettings < Gitlab::Database::Migration[2.2] + enable_lock_retries! + + milestone '16.9' + + def change + add_column :project_settings, :duo_features_enabled, :boolean, default: true, null: false + end +end diff --git a/db/schema_migrations/20240124212938 b/db/schema_migrations/20240124212938 new file mode 100644 index 000000000000..32e355dbeef6 --- /dev/null +++ b/db/schema_migrations/20240124212938 @@ -0,0 +1 @@ +1ab3946da575910f8ae9ab220d1e1da61619b66a9ad09a7c2a90c2abda5056d9 \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index c3e70f72ddfc..dd61d3f81efb 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -22722,6 +22722,7 @@ CREATE TABLE project_settings ( pages_multiple_versions_enabled boolean DEFAULT false NOT NULL, allow_merge_without_pipeline boolean DEFAULT false NOT NULL, code_suggestions boolean DEFAULT true NOT NULL, + duo_features_enabled boolean DEFAULT true NOT NULL, CONSTRAINT check_1a30456322 CHECK ((char_length(pages_unique_domain) <= 63)), CONSTRAINT check_3a03e7557a CHECK ((char_length(previous_default_branch) <= 4096)), CONSTRAINT check_3ca5cbffe6 CHECK ((char_length(issue_branch_template) <= 255)), diff --git a/ee/app/models/ee/project.rb b/ee/app/models/ee/project.rb index 59223b34f594..2e341f4b0b19 100644 --- a/ee/app/models/ee/project.rb +++ b/ee/app/models/ee/project.rb @@ -1243,12 +1243,31 @@ def on_demand_dast_available? ::Gitlab::FIPS.enabled? ? ::Feature.enabled?(:dast_ods_browser_based_scanner, self) : true end + override :code_suggestions_enabled? + def code_suggestions_enabled? + return super unless ::Gitlab.org_or_com? || ::License.feature_available?(:code_suggestions) + + if gitlab_com_and_feature_enabled? || self_managed_and_past_service_start_date? + duo_features_enabled + else + root_ancestor.code_suggestions + end + end + def gcp_artifact_registry_enabled? ::Feature.enabled?(:gcp_artifact_registry, self) && ::Gitlab::Saas.feature_available?(:google_artifact_registry) end private + def gitlab_com_and_feature_enabled? + ::Gitlab.org_or_com? && ::Feature.enabled?(:purchase_code_suggestions) + end + + def self_managed_and_past_service_start_date? + ::License.feature_available?(:code_suggestions) && ::CodeSuggestions::SelfManaged::SERVICE_START_DATE.past? + end + def latest_ingested_sbom_pipeline_id_redis_key "latest_ingested_sbom_pipeline_id/#{id}" end diff --git a/ee/spec/models/ee/project_spec.rb b/ee/spec/models/ee/project_spec.rb index a59b479500ee..b70e91fec58d 100644 --- a/ee/spec/models/ee/project_spec.rb +++ b/ee/spec/models/ee/project_spec.rb @@ -4441,5 +4441,84 @@ def stub_default_url_options(host) it { is_expected.to eq(false) } end + + describe '#code_suggestions_enabled?' do + let_it_be_with_reload(:project) { create(:project, :in_group) } + + context 'gitlab.com' do + where(:duo_features_enabled, :code_suggestions_enabled) do + true | true + false | false + end + + with_them do + context 'purchase_code_suggestions FF is enabled' do + before do + allow(::Gitlab).to receive(:org_or_com?).and_return(true) + project.project_setting.update!(duo_features_enabled: duo_features_enabled) + end + + it 'uses the duo_features_enabled project setting value' do + expect(project.code_suggestions_enabled?).to eq code_suggestions_enabled + end + end + end + + with_them do + context 'purchase_code_suggestions FF is not enabled' do + before do + allow(::Gitlab).to receive(:org_or_com?).and_return(true) + stub_feature_flags(purchase_code_suggestions: false) + project.root_ancestor.update!(code_suggestions: duo_features_enabled) + end + + it 'uses the legacy code_suggestions setting on the root group' do + expect(project.code_suggestions_enabled?).to eq code_suggestions_enabled + end + end + end + end + + context 'self-managed' do + where(:code_suggestions_license, :duo_features_enabled, :code_suggestions_enabled) do + true | true | true + true | false | false + false | true | false + false | false | false + end + + with_them do + context 'after service start date' do + before do + project.project_setting.update!(duo_features_enabled: duo_features_enabled) + allow(::Gitlab).to receive(:org_or_com?).and_return(false) + stub_licensed_features(code_suggestions: code_suggestions_license) + end + + it 'uses the duo_features_enabled project setting value' do + travel_to(::CodeSuggestions::SelfManaged::SERVICE_START_DATE + 1.day) do + expect(project.code_suggestions_enabled?).to eq code_suggestions_enabled + end + end + end + end + + with_them do + context 'before service start date' do + before do + project.root_ancestor.update!(code_suggestions: duo_features_enabled) + allow(::Gitlab).to receive(:org_or_com?).and_return(false) + stub_licensed_features(code_suggestions: code_suggestions_license) + end + + it 'uses the legacy code_suggestions setting on the root group' do + travel_to(::CodeSuggestions::SelfManaged::SERVICE_START_DATE - 1.day) do + expect(project.code_suggestions_enabled?).to eq code_suggestions_enabled + end + end + end + end + end + end end end diff --git a/ee/spec/models/namespace_setting_spec.rb b/ee/spec/models/namespace_setting_spec.rb index 445b1ed4e4fd..c59c8d5e0b4f 100644 --- a/ee/spec/models/namespace_setting_spec.rb +++ b/ee/spec/models/namespace_setting_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe NamespaceSetting do +RSpec.describe NamespaceSetting, feature_category: :groups_and_projects, type: :model do let(:group) { create(:group) } let(:setting) { group.namespace_settings } diff --git a/ee/spec/requests/api/code_suggestions_spec.rb b/ee/spec/requests/api/code_suggestions_spec.rb index 7c035a4b4975..16d16eaa032d 100644 --- a/ee/spec/requests/api/code_suggestions_spec.rb +++ b/ee/spec/requests/api/code_suggestions_spec.rb @@ -813,27 +813,20 @@ def get_user(session): end end - context 'when checking in project has code suggestions enabled' do - let_it_be(:enabled_project) { create(:project, :with_code_suggestions_enabled) } - let(:current_user) { authorized_user } - let_it_be(:disabled_project) { create(:project, :with_code_suggestions_disabled) } - let_it_be(:secret_project) { create(:project, :with_code_suggestions_enabled) } + context 'when checking if project has duo features enabled' do + let_it_be(:enabled_project) { create(:project, :in_group, :private, :with_duo_features_enabled) } + let_it_be(:disabled_project) { create(:project, :in_group, :with_duo_features_disabled) } - before_all do - enabled_project.add_maintainer(authorized_user) - disabled_project.add_maintainer(authorized_user) - end + let(:current_user) { authorized_user } subject { post api("/code_suggestions/enabled", current_user), params: { project_path: project_path } } - context 'when not logged in' do - let(:current_user) { nil } - let(:project_path) { enabled_project.full_path } - - it { is_expected.to eq(401) } - end + context 'when authorized to view project' do + before_all do + enabled_project.add_maintainer(authorized_user) + disabled_project.add_maintainer(authorized_user) + end - context 'when authorized' do context 'when enabled' do let(:project_path) { enabled_project.full_path } @@ -845,18 +838,25 @@ def get_user(session): it { is_expected.to eq(403) } end + end - context 'when user cannot access project' do - let(:project_path) { secret_project.full_path } + context 'when not logged in' do + let(:current_user) { nil } + let(:project_path) { enabled_project.full_path } - it { is_expected.to eq(404) } - end + it { is_expected.to eq(401) } + end - context 'when does not exist' do - let(:project_path) { 'not_a_real_project' } + context 'when logged in but not authorized to view project' do + let(:project_path) { enabled_project.full_path } - it { is_expected.to eq(404) } - end + it { is_expected.to eq(404) } + end + + context 'when project for project path does not exist' do + let(:project_path) { 'not_a_real_project' } + + it { is_expected.to eq(404) } end end end diff --git a/lib/api/entities/project.rb b/lib/api/entities/project.rb index fbde9164a4f5..14e275a927e3 100644 --- a/lib/api/entities/project.rb +++ b/lib/api/entities/project.rb @@ -41,7 +41,6 @@ class Project < BasicProjectDetails end end - expose :code_suggestions, documentation: { type: 'boolean' } expose :packages_enabled, documentation: { type: 'boolean' } expose :empty_repo?, as: :empty_repo, documentation: { type: 'boolean' } expose :archived?, as: :archived, documentation: { type: 'boolean' } diff --git a/spec/factories/projects.rb b/spec/factories/projects.rb index a2848bd02561..e751a0d74032 100644 --- a/spec/factories/projects.rb +++ b/spec/factories/projects.rb @@ -609,15 +609,15 @@ files { { 'README.md' => 'Hello World' } } end - trait :with_code_suggestions_enabled do + trait :with_duo_features_enabled do after(:create) do |project| - project.project_setting.update!(code_suggestions: true) + project.project_setting.update!(duo_features_enabled: true) end end - trait :with_code_suggestions_disabled do + trait :with_duo_features_disabled do after(:create) do |project| - project.project_setting.update!(code_suggestions: false) + project.project_setting.update!(duo_features_enabled: false) end end end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 3d5ccbc6feb1..865441d7dc45 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -9140,6 +9140,14 @@ def create_hook it { is_expected.to be_falsy } end + describe '#code_suggestions_enabled?' do + let(:project) { build_stubbed(:project) } + + subject(:code_suggestions_enabled?) { project.code_suggestions_enabled? } + + it { is_expected.to be_falsy } + end + private def finish_job(export_job) diff --git a/spec/requests/api/project_attributes.yml b/spec/requests/api/project_attributes.yml index ada2a917c316..ff1aac2f6f75 100644 --- a/spec/requests/api/project_attributes.yml +++ b/spec/requests/api/project_attributes.yml @@ -180,6 +180,8 @@ project_setting: - encrypted_product_analytics_configurator_connection_string - encrypted_product_analytics_configurator_connection_string_iv - product_analytics_configurator_connection_string + - code_suggestions + - duo_features_enabled build_service_desk_setting: # service_desk_setting unexposed_attributes: -- GitLab