From 5c964a1fc888a7f9ffc205b31980a4d215ebc4b4 Mon Sep 17 00:00:00 2001 From: Imam Hossain Date: Tue, 21 Oct 2025 12:52:16 +0600 Subject: [PATCH 1/5] Unassign policies for expired Ultimate licenses Introduces a cron worker to automatically unassign security policy configurations when an Ultimate license or plan expires. This worker identifies namespaces with expired Ultimate subscriptions (SaaS) or expired Ultimate licenses (self-managed) and schedules a dedicated worker to remove their associated policy configurations. This ensures that security policies are only active with a valid Ultimate plan. --- app/models/plan.rb | 4 ++ config/initializers/1_settings.rb | 3 + config/sidekiq_queues.yml | 2 + ee/app/models/ee/plan.rb | 6 ++ .../subscription_history.rb | 6 ++ ee/app/workers/all_queues.yml | 20 ++++++ ...ations_for_expired_licenses_cron_worker.rb | 64 +++++++++++++++++++ ...igurations_for_expired_namespace_worker.rb | 52 +++++++++++++++ 8 files changed, 157 insertions(+) create mode 100644 ee/app/workers/security/unassign_policy_configurations_for_expired_licenses_cron_worker.rb create mode 100644 ee/app/workers/security/unassign_policy_configurations_for_expired_namespace_worker.rb diff --git a/app/models/plan.rb b/app/models/plan.rb index 071944b2cc5d56..f545d03f42f868 100644 --- a/app/models/plan.rb +++ b/app/models/plan.rb @@ -53,6 +53,10 @@ def default? def paid? false end + + def ultimate? + false + end end Plan.prepend_mod_with('Plan') diff --git a/config/initializers/1_settings.rb b/config/initializers/1_settings.rb index d53e5b031b3ff4..d16eb9106516de 100644 --- a/config/initializers/1_settings.rb +++ b/config/initializers/1_settings.rb @@ -947,6 +947,9 @@ Settings.cron_jobs['security_pipeline_execution_policies_schedule_worker'] ||= {} Settings.cron_jobs['security_pipeline_execution_policies_schedule_worker']['cron'] ||= '* * * * *' Settings.cron_jobs['security_pipeline_execution_policies_schedule_worker']['job_class'] = 'Security::PipelineExecutionPolicies::ScheduleWorker' + Settings.cron_jobs['security_unassign_policy_configurations_for_expired_licenses_worker'] ||= {} + Settings.cron_jobs['security_unassign_policy_configurations_for_expired_licenses_worker']['cron'] ||= '0 0 * * *' + Settings.cron_jobs['security_unassign_policy_configurations_for_expired_licenses_worker']['job_class'] = 'Security::UnassignPolicyConfigurationsForExpiredLicensesCronWorker' Settings.cron_jobs['users_security_policy_bot_cleanup_cron_worker'] ||= {} Settings.cron_jobs['users_security_policy_bot_cleanup_cron_worker']['cron'] ||= '0 * * * *' Settings.cron_jobs['users_security_policy_bot_cleanup_cron_worker']['job_class'] = 'Users::SecurityPolicyBotCleanupCronWorker' diff --git a/config/sidekiq_queues.yml b/config/sidekiq_queues.yml index 9f2e704d5f589c..5ecd58c36b4d7a 100644 --- a/config/sidekiq_queues.yml +++ b/config/sidekiq_queues.yml @@ -1075,6 +1075,8 @@ - 1 - - security_sync_scan_policies - 1 +- - security_unassign_policy_configurations_for_expired_namespace + - 1 - - security_unassign_redundant_policy_configurations - 1 - - security_unenforceable_policy_rules_notification diff --git a/ee/app/models/ee/plan.rb b/ee/app/models/ee/plan.rb index e1b719a3e465fd..5e458c38e83a2a 100644 --- a/ee/app/models/ee/plan.rb +++ b/ee/app/models/ee/plan.rb @@ -35,6 +35,7 @@ module Plan TOP_PLANS = [GOLD, ULTIMATE, OPEN_SOURCE].freeze CURRENT_ACTIVE_PLANS = [FREE, PREMIUM, ULTIMATE].freeze ULTIMATE_TRIAL_PLANS = [ULTIMATE_TRIAL, ULTIMATE_TRIAL_PAID_CUSTOMER].freeze + ALL_ULTIMATE_PLANS = [ULTIMATE, *ULTIMATE_TRIAL_PLANS].freeze has_many :hosted_subscriptions, class_name: 'GitlabSubscription', foreign_key: 'hosted_plan_id' has_many :gitlab_subscription_histories, inverse_of: :hosted_plan, @@ -77,6 +78,11 @@ def paid? PAID_HOSTED_PLANS.include?(name) end + override :ultimate? + def ultimate? + ALL_ULTIMATE_PLANS.include?(name) + end + def paid_excluding_trials?(exclude_oss: false) paid_hosted_plans = PAID_HOSTED_PLANS.dup paid_hosted_plans.delete(OPEN_SOURCE) if exclude_oss diff --git a/ee/app/models/gitlab_subscriptions/subscription_history.rb b/ee/app/models/gitlab_subscriptions/subscription_history.rb index 0e128b95c87408..8bf1b8e97c9eda 100644 --- a/ee/app/models/gitlab_subscriptions/subscription_history.rb +++ b/ee/app/models/gitlab_subscriptions/subscription_history.rb @@ -55,6 +55,12 @@ class SubscriptionHistory < ApplicationRecord ) end + scope :with_a_ultimate_hosted_plan, -> do + joins(:hosted_plan).where(hosted_plan: { name: EE::Plan::ALL_ULTIMATE_PLANS }) + end + + scope :ended_on, ->(date) { where(end_date: date) } + def self.create_from_change(change_type, attrs) create_attrs = attrs .slice(*TRACKED_ATTRIBUTES) diff --git a/ee/app/workers/all_queues.yml b/ee/app/workers/all_queues.yml index cda2e6e11ecf29..20f62a936e96d0 100644 --- a/ee/app/workers/all_queues.yml +++ b/ee/app/workers/all_queues.yml @@ -893,6 +893,16 @@ :idempotent: true :tags: [] :queue_namespace: :cronjob +- :name: cronjob:security_unassign_policy_configurations_for_expired_licenses_cron + :worker_name: Security::UnassignPolicyConfigurationsForExpiredLicensesCronWorker + :feature_category: :security_policy_management + :has_external_dependencies: false + :urgency: :low + :resource_boundary: :unknown + :weight: 1 + :idempotent: true + :tags: [] + :queue_namespace: :cronjob - :name: cronjob:security_vulnerability_scanning_destroy_expired_sbom_scans :worker_name: Security::VulnerabilityScanning::DestroyExpiredSbomScansWorker :feature_category: :software_composition_analysis @@ -4176,6 +4186,16 @@ :idempotent: true :tags: [] :queue_namespace: +- :name: security_unassign_policy_configurations_for_expired_namespace + :worker_name: Security::UnassignPolicyConfigurationsForExpiredNamespaceWorker + :feature_category: :security_policy_management + :has_external_dependencies: false + :urgency: :low + :resource_boundary: :unknown + :weight: 1 + :idempotent: true + :tags: [] + :queue_namespace: - :name: security_unassign_redundant_policy_configurations :worker_name: Security::UnassignRedundantPolicyConfigurationsWorker :feature_category: :security_policy_management diff --git a/ee/app/workers/security/unassign_policy_configurations_for_expired_licenses_cron_worker.rb b/ee/app/workers/security/unassign_policy_configurations_for_expired_licenses_cron_worker.rb new file mode 100644 index 00000000000000..30334813b43a0a --- /dev/null +++ b/ee/app/workers/security/unassign_policy_configurations_for_expired_licenses_cron_worker.rb @@ -0,0 +1,64 @@ +# frozen_string_literal: true + +module Security + class UnassignPolicyConfigurationsForExpiredLicensesCronWorker + include ApplicationWorker + include CronjobQueue + + idempotent! + data_consistency :sticky + feature_category :security_policy_management + + BUFFER_DATE = 5.days.ago.freeze + BATCH_SIZE = 100 + + def perform + if saas? + handle_sass + else + handle_self_managed + end + end + + private + + def handle_saas + expired_ultimate_subscriptions.find_each(batch_size: BATCH_SIZE) do |subscription| # rubocop:disable CodeReuse/ActiveRecord -- the logic is specific to this worker + namespace = subscription.namespace + namespace_subscription = namespace&.gitlab_subscription + next unless namespace_subscription + next if namespace_subscription&.hosted_plan&.ultimate? + + schedule_unassign_worker_for_namespace(namespace) + end + end + + def handle_self_managed + license = License.current + return if license&.ultimate? + return unless license&.starts_at && license.starts_at >= BUFFER_DATE + + return if Security::OrchestrationPolicyConfiguration.none? + + Namespace.top_level.find_each(batch_size: BATCH_SIZE) do |namespace| # rubocop:disable CodeReuse/ActiveRecord -- the logic is specific to this worker + schedule_unassign_worker_for_namespace(namespace) + end + end + + def schedule_unassign_worker_for_namespace(namespace) + with_context(namespace: namespace) do + Security::UnassignPolicyConfigurationsForExpiredNamespaceWorker.perform_async(namespace.id) + end + end + + def expired_ultimate_subscriptions + GitlabSubscriptions::SubscriptionHistory + .ended_on(BUFFER_DATE) + .with_a_ultimate_hosted_plan + end + + def sass? + ::Gitlab::Saas.feature_available?(:gitlab_com_subscriptions) + end + end +end diff --git a/ee/app/workers/security/unassign_policy_configurations_for_expired_namespace_worker.rb b/ee/app/workers/security/unassign_policy_configurations_for_expired_namespace_worker.rb new file mode 100644 index 00000000000000..ccbf8d2d6a9ea1 --- /dev/null +++ b/ee/app/workers/security/unassign_policy_configurations_for_expired_namespace_worker.rb @@ -0,0 +1,52 @@ +# frozen_string_literal: true + +module Security + class UnassignPolicyConfigurationsForExpiredNamespaceWorker + include ApplicationWorker + + feature_category :security_policy_management + data_consistency :sticky + deduplicate :until_executed + idempotent! + + BATCH_SIZE = 100 + + def perform(namespace_id) + namespace = Namespace.find_by_id(namespace_id) + return unless namespace + + namespace_ids = namespace.self_and_descendants.pluck_primary_key + project_ids = namespace.all_project_ids + + policy_configurations(namespace_ids, project_ids).find_each(batch_size: BATCH_SIZE) do |configuration| # rubocop:disable CodeReuse/ActiveRecord -- the logic is specific to this worker + configuration.transaction do + configuration.delete_scan_result_policy_reads + configuration.delete + end + + audit_config_cleanup(configuration) + end + end + + private + + def policy_configurations(namespace_ids, project_ids) + Security::OrchestrationPolicyConfiguration + .for_namespace_and_projects(namespace_ids, project_ids) + end + + def audit_config_cleanup(configuration) + policy_project = configuration.security_policy_management_project + return unless policy_project + + ::Gitlab::Audit::Auditor.audit( + name: 'policy_project_updated', + author: ::Gitlab::Audit::DeletedAuthor.new(id: -4, name: 'System User'), + scope: configuration.project || configuration.namespace, + target: policy_project, + message: "Automatically unlinked #{policy_project.name} as the security policy project " \ + "because the license is no longer available." + ) + end + end +end -- GitLab From a9ebbfe7d3a6fdaea47981a2ddebabd8d7961c34 Mon Sep 17 00:00:00 2001 From: Imam Hossain Date: Wed, 22 Oct 2025 18:11:07 +0600 Subject: [PATCH 2/5] Auto-unassign security policies on license expiry This change introduces an automated process to unassign security policies from namespaces and projects when their associated GitLab Ultimate license or SaaS subscription expires or downgrades. --- ...security_policies_for_expired_licenses.yml | 10 ++ .../orchestration/unassign_service.rb | 4 +- ...ations_for_expired_licenses_cron_worker.rb | 37 ++--- ...igurations_for_expired_namespace_worker.rb | 33 ++-- ee/spec/models/ee/plan_spec.rb | 26 +++ .../subscription_history_spec.rb | 43 +++++ .../orchestration/unassign_service_spec.rb | 11 ++ ...s_for_expired_licenses_cron_worker_spec.rb | 152 ++++++++++++++++++ ...tions_for_expired_namespace_worker_spec.rb | 89 ++++++++++ spec/models/plan_spec.rb | 6 + 10 files changed, 373 insertions(+), 38 deletions(-) create mode 100644 config/feature_flags/gitlab_com_derisk/automatically_unassign_security_policies_for_expired_licenses.yml create mode 100644 ee/spec/workers/security/unassign_policy_configurations_for_expired_licenses_cron_worker_spec.rb create mode 100644 ee/spec/workers/security/unassign_policy_configurations_for_expired_namespace_worker_spec.rb diff --git a/config/feature_flags/gitlab_com_derisk/automatically_unassign_security_policies_for_expired_licenses.yml b/config/feature_flags/gitlab_com_derisk/automatically_unassign_security_policies_for_expired_licenses.yml new file mode 100644 index 00000000000000..9b100b05836e27 --- /dev/null +++ b/config/feature_flags/gitlab_com_derisk/automatically_unassign_security_policies_for_expired_licenses.yml @@ -0,0 +1,10 @@ +--- +name: automatically_unassign_security_policies_for_expired_licenses +description: +feature_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/431229 +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/209602 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/577920 +milestone: '18.6' +group: group::security policies +type: gitlab_com_derisk +default_enabled: false diff --git a/ee/app/services/security/orchestration/unassign_service.rb b/ee/app/services/security/orchestration/unassign_service.rb index aa0cd2d198a225..0108cd5a4247cb 100644 --- a/ee/app/services/security/orchestration/unassign_service.rb +++ b/ee/app/services/security/orchestration/unassign_service.rb @@ -5,10 +5,10 @@ module Orchestration class UnassignService < ::BaseContainerService include Gitlab::Utils::StrongMemoize - def execute(delete_bot: true) + def execute(delete_bot: true, skip_csp: true) return error(_('Policy project doesn\'t exist')) unless security_orchestration_policy_configuration - if container.designated_as_csp? + if container.designated_as_csp? && skip_csp return error(_("You cannot unassign security policy project for group designated as CSP.")) end diff --git a/ee/app/workers/security/unassign_policy_configurations_for_expired_licenses_cron_worker.rb b/ee/app/workers/security/unassign_policy_configurations_for_expired_licenses_cron_worker.rb index 30334813b43a0a..9644d1c7bb8ecc 100644 --- a/ee/app/workers/security/unassign_policy_configurations_for_expired_licenses_cron_worker.rb +++ b/ee/app/workers/security/unassign_policy_configurations_for_expired_licenses_cron_worker.rb @@ -9,12 +9,12 @@ class UnassignPolicyConfigurationsForExpiredLicensesCronWorker data_consistency :sticky feature_category :security_policy_management - BUFFER_DATE = 5.days.ago.freeze + BUFFER_DATE = 3.days.ago.to_date.freeze BATCH_SIZE = 100 def perform if saas? - handle_sass + handle_saas else handle_self_managed end @@ -22,23 +22,34 @@ def perform private + def saas? + ::Gitlab::Saas.feature_available?(:gitlab_com_subscriptions) + end + def handle_saas expired_ultimate_subscriptions.find_each(batch_size: BATCH_SIZE) do |subscription| # rubocop:disable CodeReuse/ActiveRecord -- the logic is specific to this worker namespace = subscription.namespace - namespace_subscription = namespace&.gitlab_subscription - next unless namespace_subscription - next if namespace_subscription&.hosted_plan&.ultimate? + next unless namespace + + namespace_subscription = namespace.gitlab_subscription + next if namespace_subscription && namespace_subscription.hosted_plan&.ultimate? schedule_unassign_worker_for_namespace(namespace) end end + def expired_ultimate_subscriptions + GitlabSubscriptions::SubscriptionHistory + .ended_on(BUFFER_DATE) + .with_a_ultimate_hosted_plan + end + def handle_self_managed + return if Security::OrchestrationPolicyConfiguration.none? + license = License.current return if license&.ultimate? - return unless license&.starts_at && license.starts_at >= BUFFER_DATE - - return if Security::OrchestrationPolicyConfiguration.none? + return unless license&.starts_at && license.starts_at < BUFFER_DATE Namespace.top_level.find_each(batch_size: BATCH_SIZE) do |namespace| # rubocop:disable CodeReuse/ActiveRecord -- the logic is specific to this worker schedule_unassign_worker_for_namespace(namespace) @@ -50,15 +61,5 @@ def schedule_unassign_worker_for_namespace(namespace) Security::UnassignPolicyConfigurationsForExpiredNamespaceWorker.perform_async(namespace.id) end end - - def expired_ultimate_subscriptions - GitlabSubscriptions::SubscriptionHistory - .ended_on(BUFFER_DATE) - .with_a_ultimate_hosted_plan - end - - def sass? - ::Gitlab::Saas.feature_available?(:gitlab_com_subscriptions) - end end end diff --git a/ee/app/workers/security/unassign_policy_configurations_for_expired_namespace_worker.rb b/ee/app/workers/security/unassign_policy_configurations_for_expired_namespace_worker.rb index ccbf8d2d6a9ea1..79d00324020b90 100644 --- a/ee/app/workers/security/unassign_policy_configurations_for_expired_namespace_worker.rb +++ b/ee/app/workers/security/unassign_policy_configurations_for_expired_namespace_worker.rb @@ -15,16 +15,18 @@ def perform(namespace_id) namespace = Namespace.find_by_id(namespace_id) return unless namespace + return unless ::Feature.enabled?(:automatically_unassign_security_policies_for_expired_licenses, namespace) + namespace_ids = namespace.self_and_descendants.pluck_primary_key project_ids = namespace.all_project_ids - policy_configurations(namespace_ids, project_ids).find_each(batch_size: BATCH_SIZE) do |configuration| # rubocop:disable CodeReuse/ActiveRecord -- the logic is specific to this worker - configuration.transaction do - configuration.delete_scan_result_policy_reads - configuration.delete - end + policy_configurations(namespace_ids, project_ids).find_each(batch_size: BATCH_SIZE) do |configuration| # rubocop:disable CodeReuse/ActiveRecord -- the logic is specific to this service + container = configuration.project || configuration.namespace + admin_bot = admin_bot_for_container_organization(container) - audit_config_cleanup(configuration) + Security::Orchestration::UnassignService + .new(container: container, current_user: admin_bot) + .execute(delete_bot: true, skip_csp: false) end end @@ -35,18 +37,13 @@ def policy_configurations(namespace_ids, project_ids) .for_namespace_and_projects(namespace_ids, project_ids) end - def audit_config_cleanup(configuration) - policy_project = configuration.security_policy_management_project - return unless policy_project - - ::Gitlab::Audit::Auditor.audit( - name: 'policy_project_updated', - author: ::Gitlab::Audit::DeletedAuthor.new(id: -4, name: 'System User'), - scope: configuration.project || configuration.namespace, - target: policy_project, - message: "Automatically unlinked #{policy_project.name} as the security policy project " \ - "because the license is no longer available." - ) + def configuration_container(configuration) + configuration.project || configuration.namespace + end + + def admin_bot_for_container_organization(container) + @admin_bots ||= {} + @admin_bots[container.organization_id] ||= Users::Internal.for_organization(container.organization_id).admin_bot end end end diff --git a/ee/spec/models/ee/plan_spec.rb b/ee/spec/models/ee/plan_spec.rb index 79cfebd345b27f..06b102099e7ed3 100644 --- a/ee/spec/models/ee/plan_spec.rb +++ b/ee/spec/models/ee/plan_spec.rb @@ -96,6 +96,26 @@ end end + describe '#ultimate?' do + subject { plan.ultimate? } + + Plan.default_plans.each do |plan| + context "when '#{plan}'" do + let(:plan) { build(:"#{plan}_plan") } + + it { is_expected.to be_falsey } + end + end + + Plan::ALL_ULTIMATE_PLANS.each do |plan| + context "when '#{plan}'" do + let(:plan) { build(:"#{plan}_plan") } + + it { is_expected.to be_truthy } + end + end + end + describe '::PLANS_ELIGIBLE_FOR_TRIAL' do subject { described_class::PLANS_ELIGIBLE_FOR_TRIAL } @@ -108,6 +128,12 @@ it { is_expected.to match_array(%w[ultimate_trial ultimate_trial_paid_customer]) } end + describe '::ALL_ULTIMATE_PLANS' do + subject { described_class::ALL_ULTIMATE_PLANS } + + it { is_expected.to match_array(%w[ultimate ultimate_trial ultimate_trial_paid_customer]) } + end + describe '.with_subscriptions' do it 'includes plans that have attached subscriptions', :saas do group = create(:group_with_plan, plan: :free_plan) diff --git a/ee/spec/models/gitlab_subscriptions/subscription_history_spec.rb b/ee/spec/models/gitlab_subscriptions/subscription_history_spec.rb index 5de1cbb6b2f478..11c65f2410d44d 100644 --- a/ee/spec/models/gitlab_subscriptions/subscription_history_spec.rb +++ b/ee/spec/models/gitlab_subscriptions/subscription_history_spec.rb @@ -65,6 +65,49 @@ it { expect(transitioning_to_plan_after).to eq([]) } end end + + describe '.with_a_ultimate_hosted_plan' do + let_it_be(:ultimate_plan) { create(:ultimate_plan) } + let_it_be(:premium_plan) { create(:premium_plan) } + + let_it_be(:history_with_ultimate_plan) do + create(:gitlab_subscription_history, hosted_plan: ultimate_plan) + end + + let_it_be(:history_with_premium_plan) do + create(:gitlab_subscription_history, hosted_plan: premium_plan) + end + + it 'returns histories with any ultimate plan' do + ultimate_trial_plan = create(:ultimate_trial_plan) + history_with_ultimate_trial_plan = create(:gitlab_subscription_history, hosted_plan: ultimate_trial_plan) + + expect(described_class.with_a_ultimate_hosted_plan).to match_array([ + history_with_ultimate_plan, + history_with_ultimate_trial_plan + ]) + end + end + + describe '.ended_on' do + let_it_be(:history1) do + create(:gitlab_subscription_history, end_date: Date.current) + end + + let_it_be(:history2) do + create(:gitlab_subscription_history, end_date: Date.current + 1.day) + end + + let_it_be(:history3) do + create(:gitlab_subscription_history, end_date: Date.yesterday) + end + + it 'returns histories that ended on a given date' do + date = Date.yesterday + + expect(described_class.ended_on(date)).to contain_exactly(history3) + end + end end describe '.create_from_change' do diff --git a/ee/spec/services/security/orchestration/unassign_service_spec.rb b/ee/spec/services/security/orchestration/unassign_service_spec.rb index a86cd8ed5dc765..84d1d65706c772 100644 --- a/ee/spec/services/security/orchestration/unassign_service_spec.rb +++ b/ee/spec/services/security/orchestration/unassign_service_spec.rb @@ -68,11 +68,22 @@ include_context 'with csp group configuration' let(:service) { described_class.new(container: csp_group, current_user: current_user) } + let(:policy_configuration) { csp_group.security_orchestration_policy_configuration } it 'respond with an error', :aggregate_failures do expect(result).not_to be_success expect(result.message).to eq('You cannot unassign security policy project for group designated as CSP.') end + + context 'when skip_csp is set to false' do + subject(:result) { service.execute(skip_csp: false) } + + it 'unassigns policy project from the project', :aggregate_failures do + expect(result).to be_success + exists = Security::OrchestrationPolicyConfiguration.exists?(policy_configuration.id) + expect(exists).to be(false) + end + end end end diff --git a/ee/spec/workers/security/unassign_policy_configurations_for_expired_licenses_cron_worker_spec.rb b/ee/spec/workers/security/unassign_policy_configurations_for_expired_licenses_cron_worker_spec.rb new file mode 100644 index 00000000000000..04702638ff8374 --- /dev/null +++ b/ee/spec/workers/security/unassign_policy_configurations_for_expired_licenses_cron_worker_spec.rb @@ -0,0 +1,152 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Security::UnassignPolicyConfigurationsForExpiredLicensesCronWorker, feature_category: :security_policy_management do + subject(:worker) { described_class.new } + + describe '#perform' do + let(:namespace_unassign_worker) { Security::UnassignPolicyConfigurationsForExpiredNamespaceWorker } + + context 'when on SaaS', :saas do + let_it_be(:ultimate_plan) { create(:ultimate_plan) } + let_it_be(:premium_plan) { create(:premium_plan) } + + let_it_be(:expired_subscription_history) do + create( + :gitlab_subscription_history, + hosted_plan: ultimate_plan, + end_date: 3.days.ago.to_date + ) + end + + let_it_be(:non_ultimate_subscription_history) do + create( + :gitlab_subscription_history, + hosted_plan: premium_plan, + end_date: 3.days.ago.to_date + ) + end + + let_it_be(:not_expired_subscription_history) do + create( + :gitlab_subscription_history, + hosted_plan: ultimate_plan, + end_date: nil + ) + end + + before do + stub_saas_features(gitlab_com_subscriptions: true) + end + + it 'schedules unassign namespace worker for namespaces with expired ultimate plans' do + expect(namespace_unassign_worker).to receive(:perform_async).with(expired_subscription_history.namespace_id) + expect(namespace_unassign_worker).not_to receive(:perform_async).with( + non_ultimate_subscription_history.namespace_id + ) + expect(namespace_unassign_worker).not_to receive(:perform_async).with( + not_expired_subscription_history.namespace_id + ) + + worker.perform + end + + context 'when the namespace have a current subscription' do + let_it_be(:with_active_premium_subscription_history) do + create( + :gitlab_subscription_history, + hosted_plan: ultimate_plan, + end_date: 3.days.ago.to_date + ) + end + + before do + with_active_premium_subscription_history.namespace.gitlab_subscription.update!(hosted_plan: premium_plan) + expired_subscription_history.namespace.gitlab_subscription.update!(hosted_plan: ultimate_plan) + end + + it 'schedules unassign namespace worker for namespaces with expired ultimate plans' do + expect(namespace_unassign_worker).to receive(:perform_async).with( + with_active_premium_subscription_history.namespace_id + ) + expect(namespace_unassign_worker).not_to receive(:perform_async).with( + expired_subscription_history.namespace_id + ) + expect(namespace_unassign_worker).not_to receive(:perform_async).with( + non_ultimate_subscription_history.namespace_id + ) + + worker.perform + end + end + end + + context 'when on self-managed' do + let_it_be(:namespace) { create(:group) } + + before do + stub_saas_features(gitlab_com_subscriptions: false) + create(:security_orchestration_policy_configuration, :namespace, namespace: namespace) + end + + context 'when license is ultimate' do + before do + create(:license, plan: License::ULTIMATE_PLAN) + end + + it 'does not schedule any workers' do + expect(Security::UnassignPolicyConfigurationsForExpiredNamespaceWorker).not_to receive(:perform_async) + + worker.perform + end + end + + context 'when license is not ultimate' do + let!(:license) do + create(:license, plan: License::PREMIUM_PLAN, data: create(:gitlab_license, starts_at: starts_at).export) + end + + context 'when license started before the buffer date' do + let(:starts_at) { 4.days.ago.to_date } + + it 'schedules UnassignPolicyConfigurationsForExpiredNamespaceWorker for all top-level namespaces' do + expect(Security::UnassignPolicyConfigurationsForExpiredNamespaceWorker).to receive(:perform_async).with( + User.first.namespace_id + ) + + expect(Security::UnassignPolicyConfigurationsForExpiredNamespaceWorker).to receive(:perform_async).with( + namespace.id + ) + + worker.perform + end + end + + context 'when license started after the buffer date' do + let(:starts_at) { 2.days.ago.to_date } + + it 'does not schedule any workers' do + expect(Security::UnassignPolicyConfigurationsForExpiredNamespaceWorker).not_to receive(:perform_async) + + worker.perform + end + end + end + + context 'when there are no policy configurations' do + before do + create(:license, plan: License::PREMIUM_PLAN, + data: create(:gitlab_license, starts_at: 5.days.ago.to_date).export) + Security::OrchestrationPolicyConfiguration.delete_all + end + + it 'does not schedule any workers' do + expect(Security::UnassignPolicyConfigurationsForExpiredNamespaceWorker).not_to receive(:perform_async) + + worker.perform + end + end + end + end +end diff --git a/ee/spec/workers/security/unassign_policy_configurations_for_expired_namespace_worker_spec.rb b/ee/spec/workers/security/unassign_policy_configurations_for_expired_namespace_worker_spec.rb new file mode 100644 index 00000000000000..84c4ca1a057e9a --- /dev/null +++ b/ee/spec/workers/security/unassign_policy_configurations_for_expired_namespace_worker_spec.rb @@ -0,0 +1,89 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Security::UnassignPolicyConfigurationsForExpiredNamespaceWorker, feature_category: :security_policy_management do + subject(:worker) { described_class.new } + + describe '#perform' do + let_it_be(:organization) { create(:organization) } + let_it_be(:group) { create(:group, organization: organization) } + let_it_be(:subgroup) { create(:group, parent: group) } + let_it_be(:project) { create(:project, group: group) } + let_it_be(:subproject) { create(:project, group: subgroup) } + let_it_be(:project_without_config) { create(:project, group: group) } + + let_it_be(:admin_bot) { create(:user, :admin_bot, organization: organization) } + + let_it_be(:group_policy_config) do + create(:security_orchestration_policy_configuration, :namespace, namespace: group) + end + + let_it_be(:subgroup_policy_config) do + create(:security_orchestration_policy_configuration, :namespace, namespace: subgroup) + end + + let_it_be(:project_policy_config) do + create(:security_orchestration_policy_configuration, project: project) + end + + let_it_be(:subproject_policy_config) do + create(:security_orchestration_policy_configuration, project: subproject) + end + + let_it_be(:other_policy_config) do + create(:security_orchestration_policy_configuration, :namespace) + end + + let(:unassign_service) { instance_double(Security::Orchestration::UnassignService) } + + before do + allow(Security::Orchestration::UnassignService).to receive(:new).and_return(unassign_service) + allow(unassign_service).to receive(:execute) + end + + context 'when namespace does not exist' do + it 'does not call UnassignService' do + worker.perform(non_existing_record_id) + + expect(Security::Orchestration::UnassignService).not_to have_received(:new) + end + end + + context 'when feature flag is disabled' do + before do + stub_feature_flags(automatically_unassign_security_policies_for_expired_licenses: false) + end + + it 'does not call UnassignService' do + worker.perform(group.id) + + expect(Security::Orchestration::UnassignService).not_to have_received(:new) + end + end + + context 'when namespace exists and feature flag is enabled' do + it 'calls UnassignService for each policy configuration in the namespace hierarchy' do + [group, subgroup, project, subproject].each do |container| + expect(Security::Orchestration::UnassignService).to receive(:new).with( + container: container, current_user: admin_bot + ).and_return(unassign_service) + end + + expect(unassign_service).to receive(:execute).with(delete_bot: true, skip_csp: false).exactly(4).times + + worker.perform(group.id) + end + + it 'does not call UnassignService for configurations outside the namespace hierarchy' do + [other_policy_config.namespace, project_without_config].each do |container| + expect(Security::Orchestration::UnassignService).not_to receive(:new).with( + container: container, current_user: anything + ) + end + + worker.perform(group.id) + end + end + end +end diff --git a/spec/models/plan_spec.rb b/spec/models/plan_spec.rb index e6717dd5c43bbc..de5bfd3adb27b0 100644 --- a/spec/models/plan_spec.rb +++ b/spec/models/plan_spec.rb @@ -77,4 +77,10 @@ it { is_expected.to eq([default_plan.name]) } end + + describe '#ultimate?' do + subject { described_class.new.ultimate? } + + it { is_expected.to be_falsey } + end end -- GitLab From 1f459d5454633b0d28db4c5a408c5f9e92a5865a Mon Sep 17 00:00:00 2001 From: Imam Hossain Date: Thu, 23 Oct 2025 02:46:45 +0600 Subject: [PATCH 3/5] Spec for unassign namespace worker --- ...ations_for_expired_licenses_cron_worker.rb | 15 +- ee/spec/initializers/1_settings_spec.rb | 1 + ...s_for_expired_licenses_cron_worker_spec.rb | 147 ++++++++---------- 3 files changed, 81 insertions(+), 82 deletions(-) diff --git a/ee/app/workers/security/unassign_policy_configurations_for_expired_licenses_cron_worker.rb b/ee/app/workers/security/unassign_policy_configurations_for_expired_licenses_cron_worker.rb index 9644d1c7bb8ecc..1e6b597932cc01 100644 --- a/ee/app/workers/security/unassign_policy_configurations_for_expired_licenses_cron_worker.rb +++ b/ee/app/workers/security/unassign_policy_configurations_for_expired_licenses_cron_worker.rb @@ -31,8 +31,8 @@ def handle_saas namespace = subscription.namespace next unless namespace - namespace_subscription = namespace.gitlab_subscription - next if namespace_subscription && namespace_subscription.hosted_plan&.ultimate? + current_subscription = namespace.gitlab_subscription + next if current_subscription && !current_subscription.expired? && current_subscription.hosted_plan&.ultimate? schedule_unassign_worker_for_namespace(namespace) end @@ -48,14 +48,21 @@ def handle_self_managed return if Security::OrchestrationPolicyConfiguration.none? license = License.current - return if license&.ultimate? - return unless license&.starts_at && license.starts_at < BUFFER_DATE + return if license&.ultimate? || latest_license_after_buffer_date? Namespace.top_level.find_each(batch_size: BATCH_SIZE) do |namespace| # rubocop:disable CodeReuse/ActiveRecord -- the logic is specific to this worker schedule_unassign_worker_for_namespace(namespace) end end + def latest_license_after_buffer_date? + license = License.current || License.history.first + return false unless license + + comparison_date = license.expired? ? license.expires_at : license.starts_at + comparison_date > BUFFER_DATE + end + def schedule_unassign_worker_for_namespace(namespace) with_context(namespace: namespace) do Security::UnassignPolicyConfigurationsForExpiredNamespaceWorker.perform_async(namespace.id) diff --git a/ee/spec/initializers/1_settings_spec.rb b/ee/spec/initializers/1_settings_spec.rb index 2d6aa24c8998d4..21a64534ec70f5 100644 --- a/ee/spec/initializers/1_settings_spec.rb +++ b/ee/spec/initializers/1_settings_spec.rb @@ -171,6 +171,7 @@ security_orchestration_policy_rule_schedule_worker security_pipeline_execution_policies_schedule_worker security_scans_purge_worker + security_unassign_policy_configurations_for_expired_licenses_worker service_desk_custom_email_verification_cleanup ssh_keys_expired_notification_worker ssh_keys_expiring_soon_notification_worker diff --git a/ee/spec/workers/security/unassign_policy_configurations_for_expired_licenses_cron_worker_spec.rb b/ee/spec/workers/security/unassign_policy_configurations_for_expired_licenses_cron_worker_spec.rb index 04702638ff8374..57afa2fd822532 100644 --- a/ee/spec/workers/security/unassign_policy_configurations_for_expired_licenses_cron_worker_spec.rb +++ b/ee/spec/workers/security/unassign_policy_configurations_for_expired_licenses_cron_worker_spec.rb @@ -5,84 +5,76 @@ RSpec.describe Security::UnassignPolicyConfigurationsForExpiredLicensesCronWorker, feature_category: :security_policy_management do subject(:worker) { described_class.new } + let(:namespace_unassign_worker) { Security::UnassignPolicyConfigurationsForExpiredNamespaceWorker } + describe '#perform' do - let(:namespace_unassign_worker) { Security::UnassignPolicyConfigurationsForExpiredNamespaceWorker } + shared_examples 'does not schedule unassign workers' do + it 'does not schedule any unassign workers' do + expect(namespace_unassign_worker).not_to receive(:perform_async) + worker.perform + end + end + + shared_examples 'schedules unassign workers for all top-level namespaces' do + it 'schedules unassign workers for all top-level namespaces' do + [User.first.namespace_id, namespace.id].each do |namespace_id| + expect(namespace_unassign_worker).to receive(:perform_async).with(namespace_id) + end + + worker.perform + end + end context 'when on SaaS', :saas do let_it_be(:ultimate_plan) { create(:ultimate_plan) } let_it_be(:premium_plan) { create(:premium_plan) } - let_it_be(:expired_subscription_history) do - create( - :gitlab_subscription_history, - hosted_plan: ultimate_plan, - end_date: 3.days.ago.to_date - ) + let_it_be(:expired_ultimate_history) do + create(:gitlab_subscription_history, hosted_plan: ultimate_plan, end_date: 3.days.ago.to_date) end - let_it_be(:non_ultimate_subscription_history) do - create( - :gitlab_subscription_history, - hosted_plan: premium_plan, - end_date: 3.days.ago.to_date - ) + let_it_be(:expired_premium_history) do + create(:gitlab_subscription_history, hosted_plan: premium_plan, end_date: 3.days.ago.to_date) end - let_it_be(:not_expired_subscription_history) do - create( - :gitlab_subscription_history, - hosted_plan: ultimate_plan, - end_date: nil - ) + let_it_be(:active_ultimate_history) do + create(:gitlab_subscription_history, hosted_plan: ultimate_plan, end_date: nil) end before do stub_saas_features(gitlab_com_subscriptions: true) end - it 'schedules unassign namespace worker for namespaces with expired ultimate plans' do - expect(namespace_unassign_worker).to receive(:perform_async).with(expired_subscription_history.namespace_id) - expect(namespace_unassign_worker).not_to receive(:perform_async).with( - non_ultimate_subscription_history.namespace_id - ) - expect(namespace_unassign_worker).not_to receive(:perform_async).with( - not_expired_subscription_history.namespace_id - ) + it 'only schedules worker for expired ultimate subscriptions' do + expect(namespace_unassign_worker).to receive(:perform_async).with(expired_ultimate_history.namespace_id) + [expired_premium_history, active_ultimate_history].each do |history| + expect(namespace_unassign_worker).not_to receive(:perform_async).with(history.namespace_id) + end worker.perform end - context 'when the namespace have a current subscription' do - let_it_be(:with_active_premium_subscription_history) do - create( - :gitlab_subscription_history, - hosted_plan: ultimate_plan, - end_date: 3.days.ago.to_date - ) + context 'when the namespace has a current subscription' do + let_it_be(:expired_but_downgraded_history) do + create(:gitlab_subscription_history, hosted_plan: ultimate_plan, end_date: 3.days.ago.to_date) end before do - with_active_premium_subscription_history.namespace.gitlab_subscription.update!(hosted_plan: premium_plan) - expired_subscription_history.namespace.gitlab_subscription.update!(hosted_plan: ultimate_plan) + expired_but_downgraded_history.namespace.gitlab_subscription.update!(hosted_plan: premium_plan) + expired_ultimate_history.namespace.gitlab_subscription.update!(hosted_plan: ultimate_plan) end - it 'schedules unassign namespace worker for namespaces with expired ultimate plans' do - expect(namespace_unassign_worker).to receive(:perform_async).with( - with_active_premium_subscription_history.namespace_id - ) - expect(namespace_unassign_worker).not_to receive(:perform_async).with( - expired_subscription_history.namespace_id - ) - expect(namespace_unassign_worker).not_to receive(:perform_async).with( - non_ultimate_subscription_history.namespace_id - ) + it 'skips expired ultimate plans if currently active under ultimate' do + expect(namespace_unassign_worker).to receive(:perform_async).with(expired_but_downgraded_history.namespace_id) + expect(namespace_unassign_worker).not_to receive(:perform_async).with(expired_premium_history.namespace_id) + expect(namespace_unassign_worker).not_to receive(:perform_async).with(expired_ultimate_history.namespace_id) worker.perform end end end - context 'when on self-managed' do + context 'when on self-managed', :without_license do let_it_be(:namespace) { create(:group) } before do @@ -90,62 +82,61 @@ create(:security_orchestration_policy_configuration, :namespace, namespace: namespace) end - context 'when license is ultimate' do + context 'when current license plan is ultimate' do before do create(:license, plan: License::ULTIMATE_PLAN) end - it 'does not schedule any workers' do - expect(Security::UnassignPolicyConfigurationsForExpiredNamespaceWorker).not_to receive(:perform_async) - - worker.perform - end + include_examples 'does not schedule unassign workers' end - context 'when license is not ultimate' do + context 'when current license plan is not ultimate' do let!(:license) do create(:license, plan: License::PREMIUM_PLAN, data: create(:gitlab_license, starts_at: starts_at).export) end - context 'when license started before the buffer date' do + context 'when start date is before the buffer date' do let(:starts_at) { 4.days.ago.to_date } - it 'schedules UnassignPolicyConfigurationsForExpiredNamespaceWorker for all top-level namespaces' do - expect(Security::UnassignPolicyConfigurationsForExpiredNamespaceWorker).to receive(:perform_async).with( - User.first.namespace_id - ) + include_examples 'schedules unassign workers for all top-level namespaces' + end - expect(Security::UnassignPolicyConfigurationsForExpiredNamespaceWorker).to receive(:perform_async).with( - namespace.id - ) + context 'when start date is after the buffer date' do + let(:starts_at) { 2.days.ago.to_date } - worker.perform - end + include_examples 'does not schedule unassign workers' end + end - context 'when license started after the buffer date' do - let(:starts_at) { 2.days.ago.to_date } + context 'with latest license is expired' do + let!(:license) do + create(:license, plan: License::ULTIMATE_PLAN, data: create(:gitlab_license, expires_at: expires_at).export) + end + + context 'when expired before buffer date' do + let(:expires_at) { 4.days.ago.to_date } + + include_examples 'schedules unassign workers for all top-level namespaces' + end - it 'does not schedule any workers' do - expect(Security::UnassignPolicyConfigurationsForExpiredNamespaceWorker).not_to receive(:perform_async) + context 'when expired after buffer date' do + let(:expires_at) { 2.days.ago.to_date } - worker.perform - end + include_examples 'does not schedule unassign workers' end end - context 'when there are no policy configurations' do + context 'without any license' do + include_examples 'schedules unassign workers for all top-level namespaces' + end + + context 'without any policy configurations' do before do - create(:license, plan: License::PREMIUM_PLAN, - data: create(:gitlab_license, starts_at: 5.days.ago.to_date).export) + create(:license, plan: License::ULTIMATE_PLAN) Security::OrchestrationPolicyConfiguration.delete_all end - it 'does not schedule any workers' do - expect(Security::UnassignPolicyConfigurationsForExpiredNamespaceWorker).not_to receive(:perform_async) - - worker.perform - end + include_examples 'does not schedule unassign workers' end end end -- GitLab From 3bdf5a39820aaa443f530d9dabefae0e53b67596 Mon Sep 17 00:00:00 2001 From: Imam Hossain Date: Thu, 23 Oct 2025 11:27:23 +0600 Subject: [PATCH 4/5] Refactor: Simplify license expiration checks Extracts the active ultimate subscription check into a helper method to improve readability in the cron worker. Also removes an unused method from the namespace worker, and adds minor spec improvements. --- ...cy_configurations_for_expired_licenses_cron_worker.rb | 9 ++++++--- ...policy_configurations_for_expired_namespace_worker.rb | 4 ---- ...nfigurations_for_expired_licenses_cron_worker_spec.rb | 2 ++ 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/ee/app/workers/security/unassign_policy_configurations_for_expired_licenses_cron_worker.rb b/ee/app/workers/security/unassign_policy_configurations_for_expired_licenses_cron_worker.rb index 1e6b597932cc01..74ac5bad424c84 100644 --- a/ee/app/workers/security/unassign_policy_configurations_for_expired_licenses_cron_worker.rb +++ b/ee/app/workers/security/unassign_policy_configurations_for_expired_licenses_cron_worker.rb @@ -32,7 +32,7 @@ def handle_saas next unless namespace current_subscription = namespace.gitlab_subscription - next if current_subscription && !current_subscription.expired? && current_subscription.hosted_plan&.ultimate? + next if active_ultimate_subscription?(current_subscription) schedule_unassign_worker_for_namespace(namespace) end @@ -44,11 +44,14 @@ def expired_ultimate_subscriptions .with_a_ultimate_hosted_plan end + def active_ultimate_subscription?(subscription) + subscription && !subscription.expired? && subscription.hosted_plan&.ultimate? + end + def handle_self_managed return if Security::OrchestrationPolicyConfiguration.none? - license = License.current - return if license&.ultimate? || latest_license_after_buffer_date? + return if License.current&.ultimate? || latest_license_after_buffer_date? Namespace.top_level.find_each(batch_size: BATCH_SIZE) do |namespace| # rubocop:disable CodeReuse/ActiveRecord -- the logic is specific to this worker schedule_unassign_worker_for_namespace(namespace) diff --git a/ee/app/workers/security/unassign_policy_configurations_for_expired_namespace_worker.rb b/ee/app/workers/security/unassign_policy_configurations_for_expired_namespace_worker.rb index 79d00324020b90..3dd09fa2c23664 100644 --- a/ee/app/workers/security/unassign_policy_configurations_for_expired_namespace_worker.rb +++ b/ee/app/workers/security/unassign_policy_configurations_for_expired_namespace_worker.rb @@ -37,10 +37,6 @@ def policy_configurations(namespace_ids, project_ids) .for_namespace_and_projects(namespace_ids, project_ids) end - def configuration_container(configuration) - configuration.project || configuration.namespace - end - def admin_bot_for_container_organization(container) @admin_bots ||= {} @admin_bots[container.organization_id] ||= Users::Internal.for_organization(container.organization_id).admin_bot diff --git a/ee/spec/workers/security/unassign_policy_configurations_for_expired_licenses_cron_worker_spec.rb b/ee/spec/workers/security/unassign_policy_configurations_for_expired_licenses_cron_worker_spec.rb index 57afa2fd822532..ee3e2646a7111a 100644 --- a/ee/spec/workers/security/unassign_policy_configurations_for_expired_licenses_cron_worker_spec.rb +++ b/ee/spec/workers/security/unassign_policy_configurations_for_expired_licenses_cron_worker_spec.rb @@ -11,6 +11,7 @@ shared_examples 'does not schedule unassign workers' do it 'does not schedule any unassign workers' do expect(namespace_unassign_worker).not_to receive(:perform_async) + worker.perform end end @@ -47,6 +48,7 @@ it 'only schedules worker for expired ultimate subscriptions' do expect(namespace_unassign_worker).to receive(:perform_async).with(expired_ultimate_history.namespace_id) + [expired_premium_history, active_ultimate_history].each do |history| expect(namespace_unassign_worker).not_to receive(:perform_async).with(history.namespace_id) end -- GitLab From d828c374a16fe85c9daedc78e5b588ae824ddd45 Mon Sep 17 00:00:00 2001 From: Imam Hossain Date: Thu, 23 Oct 2025 18:42:06 +0600 Subject: [PATCH 5/5] Preload subscriptions and namespace --- .../subscription_history.rb | 2 ++ ...ations_for_expired_licenses_cron_worker.rb | 1 + .../subscription_history_spec.rb | 20 +++++++++++++++++++ ...s_for_expired_licenses_cron_worker_spec.rb | 12 +++++++++++ 4 files changed, 35 insertions(+) diff --git a/ee/app/models/gitlab_subscriptions/subscription_history.rb b/ee/app/models/gitlab_subscriptions/subscription_history.rb index 8bf1b8e97c9eda..e4d6f43336bffc 100644 --- a/ee/app/models/gitlab_subscriptions/subscription_history.rb +++ b/ee/app/models/gitlab_subscriptions/subscription_history.rb @@ -61,6 +61,8 @@ class SubscriptionHistory < ApplicationRecord scope :ended_on, ->(date) { where(end_date: date) } + scope :with_namespace_subscription, -> { includes(namespace: [gitlab_subscription: :hosted_plan]) } + def self.create_from_change(change_type, attrs) create_attrs = attrs .slice(*TRACKED_ATTRIBUTES) diff --git a/ee/app/workers/security/unassign_policy_configurations_for_expired_licenses_cron_worker.rb b/ee/app/workers/security/unassign_policy_configurations_for_expired_licenses_cron_worker.rb index 74ac5bad424c84..1c17cf151b0513 100644 --- a/ee/app/workers/security/unassign_policy_configurations_for_expired_licenses_cron_worker.rb +++ b/ee/app/workers/security/unassign_policy_configurations_for_expired_licenses_cron_worker.rb @@ -42,6 +42,7 @@ def expired_ultimate_subscriptions GitlabSubscriptions::SubscriptionHistory .ended_on(BUFFER_DATE) .with_a_ultimate_hosted_plan + .with_namespace_subscription end def active_ultimate_subscription?(subscription) diff --git a/ee/spec/models/gitlab_subscriptions/subscription_history_spec.rb b/ee/spec/models/gitlab_subscriptions/subscription_history_spec.rb index 11c65f2410d44d..4d698de3e3ad5b 100644 --- a/ee/spec/models/gitlab_subscriptions/subscription_history_spec.rb +++ b/ee/spec/models/gitlab_subscriptions/subscription_history_spec.rb @@ -108,6 +108,26 @@ expect(described_class.ended_on(date)).to contain_exactly(history3) end end + + describe '.with_namespace_subscription' do + let_it_be(:history_with_subscription) { create(:gitlab_subscription_history) } + let_it_be(:history_without_subscription) { create(:gitlab_subscription_history) } + + let(:query) do + described_class.with_namespace_subscription.find_each do |history| + history.namespace&.gitlab_subscription&.hosted_plan + end + end + + it 'preloads namespace and gitlab_subscription to avoid N+1 queries' do + control = ActiveRecord::QueryRecorder.new { query } + + create(:gitlab_subscription_history) + create(:gitlab_subscription_history) + + expect { query }.not_to exceed_query_limit(control) + end + end end describe '.create_from_change' do diff --git a/ee/spec/workers/security/unassign_policy_configurations_for_expired_licenses_cron_worker_spec.rb b/ee/spec/workers/security/unassign_policy_configurations_for_expired_licenses_cron_worker_spec.rb index ee3e2646a7111a..9cd21f13548023 100644 --- a/ee/spec/workers/security/unassign_policy_configurations_for_expired_licenses_cron_worker_spec.rb +++ b/ee/spec/workers/security/unassign_policy_configurations_for_expired_licenses_cron_worker_spec.rb @@ -56,6 +56,18 @@ worker.perform end + it 'avoids N+1 queries' do + control = ActiveRecord::QueryRecorder.new { worker.perform } + + create(:gitlab_subscription_history, hosted_plan: ultimate_plan, end_date: 3.days.ago.to_date) + create(:gitlab_subscription_history, hosted_plan: ultimate_plan, end_date: 3.days.ago.to_date) + + # +2 for using with_context(namespace: namespace) when scheduling worker + # SELECT "routes".* FROM "routes" WHERE "routes"."source_id" = ? AND "routes"."source_type" = 'Namespace' + # SELECT "routes".* FROM "routes" WHERE "routes"."source_id" = ? AND "routes"."source_type" = 'Namespace' + expect { worker.perform }.not_to exceed_query_limit(control).with_threshold(7) + end + context 'when the namespace has a current subscription' do let_it_be(:expired_but_downgraded_history) do create(:gitlab_subscription_history, hosted_plan: ultimate_plan, end_date: 3.days.ago.to_date) -- GitLab