From f569ec3fbaca4f8a81df8e5c7be683e9c038245f Mon Sep 17 00:00:00 2001 From: Matthias Kaeppler Date: Thu, 26 Jun 2025 13:05:48 +0200 Subject: [PATCH 1/3] Remove method AvailableServices#free_access? We are retiting this class and all its uses. Not user-facing. --- .rubocop_todo/rspec/be_eq.yml | 1 - doc/development/cloud_connector/_index.md | 9 ----- .../models/concerns/ai/user_authorizable.rb | 6 ++- ee/lib/ai/amazon_q.rb | 6 ++- ee/lib/cloud_connector.rb | 8 ++++ .../base_available_service_data.rb | 6 --- .../cloud_connector/missing_service_data.rb | 4 -- .../self_signed/available_service_data.rb | 5 +++ ee/spec/lib/ai/amazon_q_spec.rb | 8 ++-- .../base_available_service_data_spec.rb | 16 -------- .../missing_service_data_spec.rb | 6 --- ee/spec/lib/cloud_connector_spec.rb | 39 +++++++++++++++++++ .../concerns/ai/user_authorizable_spec.rb | 16 +++++++- ee/spec/policies/global_policy_spec.rb | 1 - .../graphql/user_available_features_spec.rb | 2 +- .../merge_requests_controller_spec.rb | 1 - .../status_checks/probes/probe_result_spec.rb | 1 + lib/cloud_connector/connected_service.rb | 17 -------- .../cloud_connector/connected_service_spec.rb | 29 -------------- 19 files changed, 81 insertions(+), 100 deletions(-) delete mode 100644 lib/cloud_connector/connected_service.rb delete mode 100644 spec/lib/cloud_connector/connected_service_spec.rb diff --git a/.rubocop_todo/rspec/be_eq.yml b/.rubocop_todo/rspec/be_eq.yml index e39a2cd2d28c66..cf64309b4ca710 100644 --- a/.rubocop_todo/rspec/be_eq.yml +++ b/.rubocop_todo/rspec/be_eq.yml @@ -658,7 +658,6 @@ RSpec/BeEq: - 'spec/lib/click_house/migration_support/exclusive_lock_spec.rb' - 'spec/lib/click_house/models/ci/finished_pipelines_daily_spec.rb' - 'spec/lib/click_house/models/ci/finished_pipelines_hourly_spec.rb' - - 'spec/lib/cloud_connector/connected_service_spec.rb' - 'spec/lib/constraints/jira_encoded_url_constrainer_spec.rb' - 'spec/lib/container_registry/gitlab_api_client_spec.rb' - 'spec/lib/event_filter_spec.rb' diff --git a/doc/development/cloud_connector/_index.md b/doc/development/cloud_connector/_index.md index 894f3c477535b6..cd3f60a02b91ca 100644 --- a/doc/development/cloud_connector/_index.md +++ b/doc/development/cloud_connector/_index.md @@ -138,14 +138,6 @@ To decide if the service is available or visible to the end user, we need to: ::License.current&.online_cloud_license? ``` -- On GitLab.com and GitLab Self-Managed, check if the service has free access. - - The feature is considered free, if the [cut-off date](configuration.md) is not set, or it is set in the future. - - ```ruby - # Returns whether the service is free to access (no addon purchases is required) - CloudConnector::AvailableServices.find_by_name(:new_feature).free_access? - ``` - - Optional. If the service has free access, this usually means that the experimental features are subject to the [Testing Agreement](https://handbook.gitlab.com/handbook/legal/testing-agreement/). - For GitLab Duo features, the customer needs to enable [experimental toggle](../../user/gitlab_duo/turn_on_off.md#turn-on-beta-and-experimental-features) in order to use experimental features for free. @@ -193,7 +185,6 @@ return unauthorized! unless current_user.can?(:access_new_feature) # For Gitlab.com it will self-issue a token with scopes based on provided resource: # - For provided user, it will self-issue a token with scopes based on user assigment permissions # - For provided namespace, it will self-issue a token with scopes based on add-on purchased permissions -# - If service has free_access?, it will self-issue a token with all available scopes # # For SM, it will return :CloudConnector::ServiceAccessToken instance token, ignoring provided user, namespace and extra claims token = ::CloudConnector::AvailableServices.find_by_name(:new_feature).access_token(current_user) diff --git a/ee/app/models/concerns/ai/user_authorizable.rb b/ee/app/models/concerns/ai/user_authorizable.rb index 5e6a63de98267c..841e19a6e3d98f 100644 --- a/ee/app/models/concerns/ai/user_authorizable.rb +++ b/ee/app/models/concerns/ai/user_authorizable.rb @@ -107,13 +107,17 @@ def allowed_to_use(ai_feature, service_name: nil, licensed_feature: :ai_features # If the user doesn't have access through Duo add-ons # and the service isn't free, they don't have access - return denied_response unless service.free_access? + return denied_response unless service_free_access?(service) check_free_access(feature_data, licensed_feature) end private + def service_free_access?(service) + service.cut_off_date.nil? || service.cut_off_date&.future? + end + def check_add_on_purchases(service) # NOTE: We are passing `nil` as the resource to avoid filtering by namespace. # This is _not_ a good use of this API, and we should separate filtering by namespace diff --git a/ee/lib/ai/amazon_q.rb b/ee/lib/ai/amazon_q.rb index 9d20f242164787..309cec9b4191b5 100644 --- a/ee/lib/ai/amazon_q.rb +++ b/ee/lib/ai/amazon_q.rb @@ -3,12 +3,14 @@ module Ai module AmazonQ class << self + using ::CloudConnector::DataModelRefinements + def feature_available? return false unless License.feature_available?(:amazon_q) - service = CloudConnector::AvailableServices.find_by_name(:amazon_q_integration) + amazon_q_unit_primitive = Gitlab::CloudConnector::DataModel::UnitPrimitive.find_by_name(:amazon_q_integration) - if service.free_access? + if amazon_q_unit_primitive.free_access? !::GitlabSubscriptions::AddOnPurchase.for_duo_pro_or_duo_enterprise.active.exists? else ::GitlabSubscriptions::AddOnPurchase.for_duo_amazon_q.active.exists? diff --git a/ee/lib/cloud_connector.rb b/ee/lib/cloud_connector.rb index e77b4656645711..1d391c243e0131 100644 --- a/ee/lib/cloud_connector.rb +++ b/ee/lib/cloud_connector.rb @@ -1,6 +1,14 @@ # frozen_string_literal: true module CloudConnector + module DataModelRefinements + refine Gitlab::CloudConnector::DataModel::UnitPrimitive do + def free_access? + cut_off_date.nil? || cut_off_date&.future? + end + end + end + extend self GITLAB_REALM_SAAS = 'saas' diff --git a/ee/lib/cloud_connector/base_available_service_data.rb b/ee/lib/cloud_connector/base_available_service_data.rb index 4ae26700a9e41d..036be40adc04e4 100644 --- a/ee/lib/cloud_connector/base_available_service_data.rb +++ b/ee/lib/cloud_connector/base_available_service_data.rb @@ -10,17 +10,11 @@ def initialize(name, cut_off_date, add_on_names) @add_on_names = map_duo_pro(add_on_names) end - # Returns whether the service is free to access (no addon purchases is required) - def free_access? - cut_off_date.nil? || cut_off_date&.future? - end - # Returns CloudConnector access JWT token. # # For Gitlab.com it will self-issue a token with scopes based on provided resource: # - For provided user, it will self-issue a token with scopes based on user assigment permissions # - For provided namespace, it will self-issue a token with scopes based on add-on purchased permissions - # - If service has free_access?, it will self-issue a token with all available scopes # # For SM, it will return :CloudConnector::ServiceAccessToken instance token # diff --git a/ee/lib/cloud_connector/missing_service_data.rb b/ee/lib/cloud_connector/missing_service_data.rb index ee1b1b2c6904c7..03aae2247e1136 100644 --- a/ee/lib/cloud_connector/missing_service_data.rb +++ b/ee/lib/cloud_connector/missing_service_data.rb @@ -8,10 +8,6 @@ def name :missing_service end - def free_access? - false - end - def access_token(_resource = nil, **) nil end diff --git a/ee/lib/cloud_connector/self_signed/available_service_data.rb b/ee/lib/cloud_connector/self_signed/available_service_data.rb index de3d71180dc258..cc665b4a717999 100644 --- a/ee/lib/cloud_connector/self_signed/available_service_data.rb +++ b/ee/lib/cloud_connector/self_signed/available_service_data.rb @@ -38,6 +38,11 @@ def gitlab_org_group @gitlab_org_group ||= Group.find_by_path_or_name('gitlab-org') end + # Returns whether the service is free to access (no addon purchases is required) + def free_access? + cut_off_date.nil? || cut_off_date&.future? + end + def scopes_for(resource) free_access? ? allowed_scopes_during_free_access : allowed_scopes_from_purchased_bundles_for(resource) end diff --git a/ee/spec/lib/ai/amazon_q_spec.rb b/ee/spec/lib/ai/amazon_q_spec.rb index a3e461baef2035..a2563d7a76bc9d 100644 --- a/ee/spec/lib/ai/amazon_q_spec.rb +++ b/ee/spec/lib/ai/amazon_q_spec.rb @@ -41,11 +41,9 @@ with_them do before do - create(:cloud_connector_access, data: { - available_services: [ - { name: "amazon_q_integration", serviceStartTime: cut_off_date } - ] - }) + allow(Gitlab::CloudConnector::DataModel::UnitPrimitive).to receive(:find_by_name).and_return( + build(:cloud_connector_unit_primitive, name: 'amazon_q_integration', cut_off_date: cut_off_date) + ) stub_licensed_features(amazon_q: amazon_q_license_available) diff --git a/ee/spec/lib/cloud_connector/base_available_service_data_spec.rb b/ee/spec/lib/cloud_connector/base_available_service_data_spec.rb index cc0b54b2d1016e..acd44ed647c454 100644 --- a/ee/spec/lib/cloud_connector/base_available_service_data_spec.rb +++ b/ee/spec/lib/cloud_connector/base_available_service_data_spec.rb @@ -19,22 +19,6 @@ subject(:service_data) { described_class.new(service_name, cut_off_date, purchased_add_ons) } - describe '#free_access?' do - subject(:free_access) { service_data.free_access? } - - context 'when cut_off_date is in the past' do - let_it_be(:cut_off_date) { 1.day.ago } - - it { is_expected.to be false } - end - - context 'when cut_off_date is in the future' do - let_it_be(:cut_off_date) { 1.day.from_now } - - it { is_expected.to be true } - end - end - describe '#name' do subject(:name) { service_data.name } diff --git a/ee/spec/lib/cloud_connector/missing_service_data_spec.rb b/ee/spec/lib/cloud_connector/missing_service_data_spec.rb index 5cca2f325b1756..f17bac9a27592e 100644 --- a/ee/spec/lib/cloud_connector/missing_service_data_spec.rb +++ b/ee/spec/lib/cloud_connector/missing_service_data_spec.rb @@ -3,12 +3,6 @@ require 'spec_helper' RSpec.describe CloudConnector::MissingServiceData, feature_category: :plan_provisioning do - describe '#free_access?' do - subject(:free_access?) { described_class.new.free_access? } - - it { is_expected.to be false } - end - describe '#name' do subject(:name) { described_class.new.name } diff --git a/ee/spec/lib/cloud_connector_spec.rb b/ee/spec/lib/cloud_connector_spec.rb index ccc20d72b64226..2a1d4098ea39db 100644 --- a/ee/spec/lib/cloud_connector_spec.rb +++ b/ee/spec/lib/cloud_connector_spec.rb @@ -92,4 +92,43 @@ end end end + + describe 'data model refinements' do + let(:cut_off_date) { nil } + let(:unit_primitive) { build(:cloud_connector_unit_primitive, cut_off_date: cut_off_date) } + + context 'when not using the DataModelRefinements' do + specify 'unit primitives do not respond to free_access?' do + expect(unit_primitive.respond_to?(:free_access?)).not_to be(true) + end + end + + context 'when using DataModelRefinements' do + using ::CloudConnector::DataModelRefinements + + specify 'unit primitives respond to free_access?' do + expect(unit_primitive.respond_to?(:free_access?)).to be(true) + end + + describe '.free_access? refinement', :freeze_time do + subject(:free_access?) { unit_primitive.free_access? } + + context 'when cut_off_date is nil' do + it { is_expected.to be(true) } + end + + context 'when cut_off_date is in the future' do + let(:cut_off_date) { Time.current + 1.second } + + it { is_expected.to be(true) } + end + + context 'when cut_off_date is in the past' do + let(:cut_off_date) { Time.current - 1.second } + + it { is_expected.to be(false) } + end + end + end + end end diff --git a/ee/spec/models/concerns/ai/user_authorizable_spec.rb b/ee/spec/models/concerns/ai/user_authorizable_spec.rb index cfa6da61f97a1b..ebd480722fce8d 100644 --- a/ee/spec/models/concerns/ai/user_authorizable_spec.rb +++ b/ee/spec/models/concerns/ai/user_authorizable_spec.rb @@ -38,7 +38,7 @@ stub_const("Gitlab::Llm::Utils::AiFeaturesCatalogue::LIST", { ai_feature => { maturity: maturity } }) allow(CloudConnector::AvailableServices).to receive(:find_by_name).with(service_name).and_return(service) - allow(service).to receive(:free_access?).and_return(free_access) + allow(service).to receive(:cut_off_date).and_return(free_access ? nil : (Time.current - 1.month)) end subject { user.allowed_to_use(ai_feature) } @@ -352,6 +352,17 @@ }) end + # We need to stub both the service and the UP because: + # - We are not done migrating away from AvailableServices + # - The actual UP as defined in the library YAML does not specify a cut-off date + let_it_be(:amazon_q_unit_primitive) do + build(:cloud_connector_unit_primitive, + name: 'amazon_q_integration', + cut_off_date: Time.current - 1.month, + add_ons: build_list(:cloud_connector_add_on, 1, name: 'duo_amazon_q') + ) + end + let_it_be(:gitlab_subscription_add_on_purchase) do create(:gitlab_subscription_add_on_purchase, :self_managed, :duo_amazon_q) end @@ -378,6 +389,9 @@ before do Ai::Setting.instance.update!(amazon_q_ready: amazon_q_connected) stub_licensed_features(amazon_q: true) + + allow(Gitlab::CloudConnector::DataModel::UnitPrimitive).to receive(:find_by_name) + .and_return(amazon_q_unit_primitive) end it 'checks whether the feature is available in Amazon Q' do diff --git a/ee/spec/policies/global_policy_spec.rb b/ee/spec/policies/global_policy_spec.rb index 8527d488bfe366..a32faf7a0a093a 100644 --- a/ee/spec/policies/global_policy_spec.rb +++ b/ee/spec/policies/global_policy_spec.rb @@ -610,7 +610,6 @@ allow(CloudConnector::AvailableServices).to receive(:find_by_name).with(:self_hosted_models) .and_return(self_hosted_service_data) allow(current_user).to receive(:allowed_to_use?).and_return(allowed_to_use) - allow(self_hosted_service_data).to receive(:free_access?).and_return(free) end it { is_expected.to duo_chat_enabled_for_user } diff --git a/ee/spec/requests/api/graphql/user_available_features_spec.rb b/ee/spec/requests/api/graphql/user_available_features_spec.rb index b9246dd7f9bf92..9d9f5c3d158a5b 100644 --- a/ee/spec/requests/api/graphql/user_available_features_spec.rb +++ b/ee/spec/requests/api/graphql/user_available_features_spec.rb @@ -33,7 +33,7 @@ let(:service) do instance_double(::CloudConnector::BaseAvailableServiceData, - name: :any_name, add_on_names: ['code_suggestions'], free_access?: true) + name: :any_name, add_on_names: ['code_suggestions']) end before do diff --git a/ee/spec/requests/projects/merge_requests_controller_spec.rb b/ee/spec/requests/projects/merge_requests_controller_spec.rb index 6db23c3694bd09..6e57d8a2cb93c8 100644 --- a/ee/spec/requests/projects/merge_requests_controller_spec.rb +++ b/ee/spec/requests/projects/merge_requests_controller_spec.rb @@ -60,7 +60,6 @@ def get_public_show service = instance_double(CloudConnector::BaseAvailableServiceData) allow(::CloudConnector::AvailableServices).to receive(:find_by_name).and_return(service) - allow(service).to receive_messages({ free_access?: true }) allow(::Gitlab::Saas).to receive(:feature_available?).and_return(true) end diff --git a/ee/spec/services/cloud_connector/status_checks/probes/probe_result_spec.rb b/ee/spec/services/cloud_connector/status_checks/probes/probe_result_spec.rb index fa9410fee3ebc8..63e04c2cae644d 100644 --- a/ee/spec/services/cloud_connector/status_checks/probes/probe_result_spec.rb +++ b/ee/spec/services/cloud_connector/status_checks/probes/probe_result_spec.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true require 'fast_spec_helper' +require 'gitlab/cloud_connector' require_relative "../../../../../app/services/cloud_connector/status_checks/probes/probe_result" RSpec.describe CloudConnector::StatusChecks::Probes::ProbeResult, feature_category: :duo_setting do diff --git a/lib/cloud_connector/connected_service.rb b/lib/cloud_connector/connected_service.rb deleted file mode 100644 index 24770105a3be96..00000000000000 --- a/lib/cloud_connector/connected_service.rb +++ /dev/null @@ -1,17 +0,0 @@ -# frozen_string_literal: true - -# Presents a service enabled through Cloud Connector -module CloudConnector - class ConnectedService - def initialize(name:, cut_off_date:) - @name = name - @cut_off_date = cut_off_date - end - - def free_access? - cut_off_date.nil? || cut_off_date&.future? - end - - attr_accessor :name, :cut_off_date - end -end diff --git a/spec/lib/cloud_connector/connected_service_spec.rb b/spec/lib/cloud_connector/connected_service_spec.rb deleted file mode 100644 index 52db869b21616b..00000000000000 --- a/spec/lib/cloud_connector/connected_service_spec.rb +++ /dev/null @@ -1,29 +0,0 @@ -# frozen_string_literal: true - -require 'fast_spec_helper' - -RSpec.describe CloudConnector::ConnectedService, feature_category: :plan_provisioning do - describe '#free_access?' do - let(:service) { described_class.new(name: :code_suggestions, cut_off_date: cut_off_date) } - - subject { service.free_access? } - - context 'when the service cut off date is in the past' do - let(:cut_off_date) { Time.current - 1.second } - - it { is_expected.to eq(false) } - end - - context 'when the service cut off date is in the future' do - let(:cut_off_date) { Time.current + 1.second } - - it { is_expected.to eq(true) } - end - - context 'when the service cut off date is nil' do - let(:cut_off_date) { nil } - - it { is_expected.to eq(true) } - end - end -end -- GitLab From 5dbb7b2311a08e6c6685e81d83c30ee6c9025b68 Mon Sep 17 00:00:00 2001 From: Matthias Kaeppler Date: Wed, 9 Jul 2025 15:10:19 +0200 Subject: [PATCH 2/3] Remove refinement again in favor of inline helper --- ee/lib/ai/amazon_q.rb | 8 +++++--- ee/lib/cloud_connector.rb | 8 -------- 2 files changed, 5 insertions(+), 11 deletions(-) diff --git a/ee/lib/ai/amazon_q.rb b/ee/lib/ai/amazon_q.rb index 309cec9b4191b5..4b2c8760764998 100644 --- a/ee/lib/ai/amazon_q.rb +++ b/ee/lib/ai/amazon_q.rb @@ -3,14 +3,12 @@ module Ai module AmazonQ class << self - using ::CloudConnector::DataModelRefinements - def feature_available? return false unless License.feature_available?(:amazon_q) amazon_q_unit_primitive = Gitlab::CloudConnector::DataModel::UnitPrimitive.find_by_name(:amazon_q_integration) - if amazon_q_unit_primitive.free_access? + if amazon_q_free_access?(amazon_q_unit_primitive) !::GitlabSubscriptions::AddOnPurchase.for_duo_pro_or_duo_enterprise.active.exists? else ::GitlabSubscriptions::AddOnPurchase.for_duo_amazon_q.active.exists? @@ -57,6 +55,10 @@ def ensure_service_account_unblocked!(current_user:, service_account: nil) private + def amazon_q_free_access?(unit_primitive) + unit_primitive.cut_off_date.nil? || unit_primitive.cut_off_date.future? + end + def ai_settings Ai::Setting.instance end diff --git a/ee/lib/cloud_connector.rb b/ee/lib/cloud_connector.rb index 1d391c243e0131..e77b4656645711 100644 --- a/ee/lib/cloud_connector.rb +++ b/ee/lib/cloud_connector.rb @@ -1,14 +1,6 @@ # frozen_string_literal: true module CloudConnector - module DataModelRefinements - refine Gitlab::CloudConnector::DataModel::UnitPrimitive do - def free_access? - cut_off_date.nil? || cut_off_date&.future? - end - end - end - extend self GITLAB_REALM_SAAS = 'saas' -- GitLab From a79dd2eb9eadba15cf847bf409753fef0a0325a8 Mon Sep 17 00:00:00 2001 From: Matthias Kaeppler Date: Tue, 15 Jul 2025 12:05:21 +0200 Subject: [PATCH 3/3] Remove data models refinements spec --- ee/spec/lib/cloud_connector_spec.rb | 39 ----------------------------- 1 file changed, 39 deletions(-) diff --git a/ee/spec/lib/cloud_connector_spec.rb b/ee/spec/lib/cloud_connector_spec.rb index 2a1d4098ea39db..ccc20d72b64226 100644 --- a/ee/spec/lib/cloud_connector_spec.rb +++ b/ee/spec/lib/cloud_connector_spec.rb @@ -92,43 +92,4 @@ end end end - - describe 'data model refinements' do - let(:cut_off_date) { nil } - let(:unit_primitive) { build(:cloud_connector_unit_primitive, cut_off_date: cut_off_date) } - - context 'when not using the DataModelRefinements' do - specify 'unit primitives do not respond to free_access?' do - expect(unit_primitive.respond_to?(:free_access?)).not_to be(true) - end - end - - context 'when using DataModelRefinements' do - using ::CloudConnector::DataModelRefinements - - specify 'unit primitives respond to free_access?' do - expect(unit_primitive.respond_to?(:free_access?)).to be(true) - end - - describe '.free_access? refinement', :freeze_time do - subject(:free_access?) { unit_primitive.free_access? } - - context 'when cut_off_date is nil' do - it { is_expected.to be(true) } - end - - context 'when cut_off_date is in the future' do - let(:cut_off_date) { Time.current + 1.second } - - it { is_expected.to be(true) } - end - - context 'when cut_off_date is in the past' do - let(:cut_off_date) { Time.current - 1.second } - - it { is_expected.to be(false) } - end - end - end - end end -- GitLab