From 35e47fbaedf350f59a5dc581f7d9b7472f7d7f97 Mon Sep 17 00:00:00 2001 From: Dominic Bauer Date: Mon, 18 Aug 2025 15:55:36 +0200 Subject: [PATCH 01/16] Add `securityPoliciesSyncUpdated` GraphQL subscription Changelog: changed EE: true --- doc/api/graphql/reference/_index.md | 15 + ee/app/graphql/ee/graphql_triggers.rb | 10 + ee/app/graphql/ee/types/subscription_type.rb | 5 + .../security/policies_sync_updated.rb | 31 ++ .../security/policies_sync_updated.rb | 36 ++ ...urity_policy_sync_propagation_tracking.yml | 10 + ee/lib/ee/gitlab/application_context.rb | 9 +- .../policy_sync_state.rb | 314 +++++++++++ ee/spec/graphql/graphql_triggers_spec.rb | 36 ++ .../security/policies_sync_updated_spec.rb | 62 +++ .../policy_sync_state_spec.rb | 497 ++++++++++++++++++ .../security/policies_sync_update/helper.rb | 42 ++ 12 files changed, 1065 insertions(+), 2 deletions(-) create mode 100644 ee/app/graphql/subscriptions/security/policies_sync_updated.rb create mode 100644 ee/app/graphql/types/gitlab_subscriptions/security/policies_sync_updated.rb create mode 100644 ee/config/feature_flags/gitlab_com_derisk/security_policy_sync_propagation_tracking.yml create mode 100644 ee/lib/security/security_orchestration_policies/policy_sync_state.rb create mode 100644 ee/spec/graphql/subscriptions/security/policies_sync_updated_spec.rb create mode 100644 ee/spec/lib/security/security_orchestration_policies/policy_sync_state_spec.rb create mode 100644 ee/spec/support/helpers/graphql/subscriptions/security/policies_sync_update/helper.rb diff --git a/doc/api/graphql/reference/_index.md b/doc/api/graphql/reference/_index.md index 495cd3f1610a54..3c8d1278e73ef6 100644 --- a/doc/api/graphql/reference/_index.md +++ b/doc/api/graphql/reference/_index.md @@ -2139,6 +2139,7 @@ Input type: `AdminSidekiqQueuesDeleteJobsInput` | `mergeActionStatus` | [`String`](#string) | Delete jobs matching merge_action_status in the context metadata. | | `organizationId` | [`String`](#string) | Delete jobs matching organization_id in the context metadata. | | `pipelineId` | [`String`](#string) | Delete jobs matching pipeline_id in the context metadata. | +| `policySyncConfigId` | [`String`](#string) | Delete jobs matching policy_sync_config_id in the context metadata. | | `project` | [`String`](#string) | Delete jobs matching project in the context metadata. | | `queueName` | [`String!`](#string) | Name of the queue to delete jobs from. | | `relatedClass` | [`String`](#string) | Delete jobs matching related_class in the context metadata. | @@ -37503,6 +37504,20 @@ Check permissions for the current user on a vulnerability finding. | `owner` | [`UserCore!`](#usercore) | Owner of the pipeline trigger token. | | `token` | [`String!`](#string) | Value of the pipeline trigger token. | +### `PoliciesSyncUpdated` + +Security policy state synchronization update. + +#### Fields + +| Name | Type | Description | +| ---- | ---- | ----------- | +| `failedProjects` | [`[String!]`](#string) | IDs of failed projects. | +| `mergeRequests` | [`Float`](#float) | Status of merge requests sync in percentage. | +| `mergeRequestsTotal` | [`Int`](#int) | Total number of merge requests synced. | +| `projects` | [`Float`](#float) | Status of projects sync in percentage. | +| `projectsTotal` | [`Int`](#int) | Total number of projects synced. | + ### `PolicyAnyMergeRequestViolation` Represents policy violation for `any_merge_request` report_type. diff --git a/ee/app/graphql/ee/graphql_triggers.rb b/ee/app/graphql/ee/graphql_triggers.rb index d612c957ab61f0..eb52af403efd1b 100644 --- a/ee/app/graphql/ee/graphql_triggers.rb +++ b/ee/app/graphql/ee/graphql_triggers.rb @@ -68,6 +68,16 @@ def self.security_policy_project_created(container, status, security_policy_proj { status: status, errors: errors, error_message: error_message, project: security_policy_project } ) end + + def self.security_policies_sync_updated( + container, projects, projects_total, failed_projects, merge_requests, merge_requests_total) + ::GitlabSchema.subscriptions.trigger( + :security_policies_sync_updated, + { full_path: container.full_path }, + { projects: projects, projects_total: projects_total, failed_projects: failed_projects, + merge_requests: merge_requests, merge_requests_total: merge_requests_total } + ) + end end end end diff --git a/ee/app/graphql/ee/types/subscription_type.rb b/ee/app/graphql/ee/types/subscription_type.rb index 8d5aa918566eff..507dc4e8fc917c 100644 --- a/ee/app/graphql/ee/types/subscription_type.rb +++ b/ee/app/graphql/ee/types/subscription_type.rb @@ -40,6 +40,11 @@ def self.authorization_scopes subscription: Subscriptions::Security::PolicyProjectCreated, null: true, description: 'Triggered when the security policy project is created for a specific group or project.', experiment: { milestone: '17.3' } + + field :security_policies_sync_updated, + subscription: Subscriptions::Security::PoliciesSyncUpdated, null: true, + description: 'Triggered when the security policies are being synced and the status is updated.', + experiment: { milestone: '18.3' } end end end diff --git a/ee/app/graphql/subscriptions/security/policies_sync_updated.rb b/ee/app/graphql/subscriptions/security/policies_sync_updated.rb new file mode 100644 index 00000000000000..3a875d4f87eaf7 --- /dev/null +++ b/ee/app/graphql/subscriptions/security/policies_sync_updated.rb @@ -0,0 +1,31 @@ +# frozen_string_literal: true + +module Subscriptions + module Security + class PoliciesSyncUpdated < ::Subscriptions::BaseSubscription + include Gitlab::Graphql::Laziness + + payload_type Types::GitlabSubscriptions::Security::PoliciesSyncUpdated + + argument :full_path, GraphQL::Types::String, + required: true, + description: 'Full path of the project or group.' + + def authorized?(full_path:) + container = Routable.find_by_full_path(full_path) + + Ability.allowed?(current_user, :update_security_orchestration_policy_project, container) + end + + def update(_) + { + projects: object[:projects], + projects_total: object[:projects_total], + failed_projects: object[:failed_projects], + merge_requests: object[:merge_requests], + merge_requests_total: object[:merge_requests_total] + } + end + end + end +end diff --git a/ee/app/graphql/types/gitlab_subscriptions/security/policies_sync_updated.rb b/ee/app/graphql/types/gitlab_subscriptions/security/policies_sync_updated.rb new file mode 100644 index 00000000000000..7c4387c925d19c --- /dev/null +++ b/ee/app/graphql/types/gitlab_subscriptions/security/policies_sync_updated.rb @@ -0,0 +1,36 @@ +# frozen_string_literal: true + +module Types + module GitlabSubscriptions + module Security + # rubocop:disable Graphql/AuthorizeTypes -- Authorization is handled in subscription + # rubocop:disable GraphQL/ExtractType -- Not worth combining (merge_requests, merge_requests_total) into a newtype + class PoliciesSyncUpdated < ::Types::BaseObject + graphql_name 'PoliciesSyncUpdated' + description 'Security policy state synchronization update' + + field :projects, GraphQL::Types::Float, + null: true, + description: 'Status of projects sync in percentage.' + + field :projects_total, GraphQL::Types::Int, + null: true, + description: 'Total number of projects synced.' + + field :failed_projects, [GraphQL::Types::String], + null: true, + description: 'IDs of failed projects.' + + field :merge_requests, GraphQL::Types::Float, + null: true, + description: 'Status of merge requests sync in percentage.' + + field :merge_requests_total, GraphQL::Types::Int, + null: true, + description: 'Total number of merge requests synced.' + end + # rubocop:enable GraphQL/ExtractType + # rubocop:enable Graphql/AuthorizeTypes + end + end +end diff --git a/ee/config/feature_flags/gitlab_com_derisk/security_policy_sync_propagation_tracking.yml b/ee/config/feature_flags/gitlab_com_derisk/security_policy_sync_propagation_tracking.yml new file mode 100644 index 00000000000000..66ca0ae2a90919 --- /dev/null +++ b/ee/config/feature_flags/gitlab_com_derisk/security_policy_sync_propagation_tracking.yml @@ -0,0 +1,10 @@ +--- +name: security_policy_sync_propagation_tracking +description: +feature_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/559273 +introduced_by_url: +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/561007 +milestone: '18.4' +group: group::security policies +type: gitlab_com_derisk +default_enabled: false diff --git a/ee/lib/ee/gitlab/application_context.rb b/ee/lib/ee/gitlab/application_context.rb index 67177f6326403a..27fb0c2071d2bd 100644 --- a/ee/lib/ee/gitlab/application_context.rb +++ b/ee/lib/ee/gitlab/application_context.rb @@ -10,11 +10,13 @@ module ApplicationContext EE_KNOWN_KEYS = [ :subscription_plan, - :ai_resource + :ai_resource, + ::Security::SecurityOrchestrationPolicies::PolicySyncState::POLICY_SYNC_CONTEXT_KEY ].freeze EE_APPLICATION_ATTRIBUTES = [ - Attribute.new(:ai_resource, ::GlobalID) + Attribute.new(:ai_resource, ::GlobalID), + Attribute.new(::Security::SecurityOrchestrationPolicies::PolicySyncState::POLICY_SYNC_CONTEXT_KEY, Integer) ].freeze class_methods do @@ -35,6 +37,9 @@ def application_attributes def to_lazy_hash super.tap do |hash| assign_hash_if_value(hash, :ai_resource) + # rubocop:disable Layout/LineLength -- namespaced constant + assign_hash_if_value(hash, ::Security::SecurityOrchestrationPolicies::PolicySyncState::POLICY_SYNC_CONTEXT_KEY) + # rubocop:enable Layout/LineLength hash[:subscription_plan] = -> { subcription_plan_name } if include_namespace? end diff --git a/ee/lib/security/security_orchestration_policies/policy_sync_state.rb b/ee/lib/security/security_orchestration_policies/policy_sync_state.rb new file mode 100644 index 00000000000000..c162d7d7c4907b --- /dev/null +++ b/ee/lib/security/security_orchestration_policies/policy_sync_state.rb @@ -0,0 +1,314 @@ +# frozen_string_literal: true + +module Security + module SecurityOrchestrationPolicies + module PolicySyncState + POLICY_SYNC_TTL = 24.hours.to_i + POLICY_SYNC_CONTEXT_KEY = :policy_sync_config_id + + class State + include Gitlab::Utils::StrongMemoize + + def self.from_application_context + config_id = Gitlab::ApplicationContext.current_context_attribute(POLICY_SYNC_CONTEXT_KEY)&.to_i || return + + new(config_id) + end + + attr_reader :config_id + + def initialize(config_id) + @config_id = config_id + end + + # Appends project IDs, adding to the pending set and incrementing the total counter + def append_projects(project_ids) + return if feature_disabled? || project_ids.empty? + + with_redis do |redis| + redis.multi do |multi| + multi.sadd(projects_sync_key, project_ids) + multi.incrby(total_projects_key, project_ids.size) + + multi.expire(projects_sync_key, POLICY_SYNC_TTL) + multi.expire(total_projects_key, POLICY_SYNC_TTL) + end + end + end + + # Marks the project ID as successfully synced and triggers a status update + def finish_project(project_id) + return if feature_disabled? + + with_redis do |redis| + redis.srem(projects_sync_key, project_id.to_s) + end + + trigger_subscription + end + + # Marks the project ID as failed to sync and triggers a status update + def fail_project(project_id) + return if feature_disabled? + + with_redis do |redis| + redis.multi do |multi| + multi.sadd(failed_projects_sync_key, project_id) + multi.srem(projects_sync_key, project_id.to_s) + + multi.expire(failed_projects_sync_key, POLICY_SYNC_TTL) + end + end + + trigger_subscription + end + + # Registers an MR for tracking and initializes the worker counter + def start_merge_request(merge_request_id) + return if feature_disabled? + + with_redis do |redis| + redis.multi do |multi| + multi.sadd(merge_requests_sync_key, merge_request_id) + multi.incr(total_merge_requests_key) + multi.set(merge_request_workers_sync_key(merge_request_id), 0) + + multi.expire(merge_requests_sync_key, POLICY_SYNC_TTL) + multi.expire(total_merge_requests_key, POLICY_SYNC_TTL) + multi.expire(merge_request_workers_sync_key(merge_request_id), POLICY_SYNC_TTL) + end + end + end + + # Increments the worker counter for a given merge request + def start_merge_request_worker(merge_request_id) + return if feature_disabled? + + with_redis do |redis| + redis.incr(merge_request_workers_sync_key(merge_request_id)) + end + end + + # Decrements the merge request worker count and if it hits zero, marks the merge request + # as fully synced + def finish_merge_request_worker(merge_request_id) + return if feature_disabled? + + with_redis do |redis| + new_value = redis.decr(merge_request_workers_sync_key(merge_request_id)) + + if new_value <= 0 + redis.srem(merge_requests_sync_key, merge_request_id.to_s) + + trigger_subscription + end + end + end + + def sync_in_progress? + return false if feature_disabled? + + with_redis do |redis| + conditions = redis.multi do |multi| + # rubocop:disable CodeReuse/ActiveRecord -- false positive + multi.exists?(total_projects_key) + multi.exists?(total_merge_requests_key) + # rubocop:enable CodeReuse/ActiveRecord + + multi.scard(projects_sync_key) + multi.scard(merge_requests_sync_key) + end + + # rubocop:disable Layout/LineLength -- TODO + conditions.then do |total_projects, total_merge_requests, project_pending_count, merge_request_pending_count| + total_projects && total_merge_requests && (project_pending_count > 0 || merge_request_pending_count > 0) + end + # rubocop:enable Layout/LineLength + end + end + + def clear + return if feature_disabled? + + clear_pending_projects + clear_pending_merge_requests + end + + # Pending project IDs. + def pending_projects + get_items(projects_sync_key) + end + + # Failed project IDs. + def failed_projects + get_items(failed_projects_sync_key) + end + + # Pending merge request IDs. + def pending_merge_requests + get_items(merge_requests_sync_key) + end + + # Total number of pending merge requests + def total_merge_request_workers_count(merge_request_id) + with_redis do |redis| + redis.get(merge_request_workers_sync_key(merge_request_id))&.to_i + end + end + + # Total number of pending projects + def total_project_count + with_redis do |redis| + redis.get(total_projects_key)&.to_i + end + end + + # Total number of pending merge requests + def total_merge_request_count + with_redis do |redis| + redis.get(total_merge_requests_key)&.to_i + end + end + + private + + def redis_key_tag + "{security_policy_sync:#{config_id}}" + end + + # Set: project IDs pending synchronization + def projects_sync_key + "#{redis_key_tag}:projects" + end + + # Integer: initial total number of projects for percentage calculation + def total_projects_key + "#{redis_key_tag}:total_projects" + end + + # Set: merge request IDs with at least one active sync worker + def merge_requests_sync_key + "#{redis_key_tag}:merge_requests" + end + + # Integer: Countdown for active downstream workers for a merge request + def merge_request_workers_sync_key(merge_request_id) + "#{redis_key_tag}:merge_requests:#{merge_request_id}:workers" + end + + # Integer: Total number of unique merge requests processed during sync + def total_merge_requests_key + "#{redis_key_tag}:total_merge_requests" + end + + # Set: Project IDs that failed to sync after all retries + def failed_projects_sync_key + "#{redis_key_tag}:failed_projects" + end + + # Deletes keys for the policy configuration sync + def clear_pending_projects + with_redis do |redis| + redis.del(projects_sync_key) + redis.del(total_projects_key) + redis.del(failed_projects_sync_key) + end + end + + # Deletes merge request-related keys for the policy cofniguration sync + def clear_pending_merge_requests + with_redis do |redis| + redis.del(merge_requests_sync_key) + redis.del(total_merge_requests_key) + end + end + + def get_progress(pending, total) + return if total == 0 + + ((total - pending).to_f / total * 100).round + end + + def trigger_subscription + projects_pending = items_count(projects_sync_key) + projects_total = get_counter(total_projects_key) + merge_requests_pending = items_count(merge_requests_sync_key) + merge_requests_total = get_counter(total_merge_requests_key) + + GraphqlTriggers.security_policies_sync_updated( + policy_project, + get_progress(projects_pending, projects_total), + projects_total, + failed_projects, + get_progress(merge_requests_pending, merge_requests_total), + merge_requests_total + ) + end + + def feature_disabled? + strong_memoize_with(:feature_disabled, config_id) do + Feature.disabled?(:security_policy_sync_propagation_tracking, policy_project) + end + end + + def policy_project + strong_memoize_with(:policy_project, config_id) do + Security::OrchestrationPolicyConfiguration.find_by_id(config_id)&.security_policy_management_project + end + end + + def get_counter(key) + with_redis do |redis| + redis.get(key).to_i + end + end + + def get_items(key) + with_redis do |redis| + redis.smembers(key) + end + end + + def items_count(key) + with_redis do |redis| + redis.scard(key) + end + end + + def with_redis(&block) + Gitlab::Redis::SharedState.with(&block) # rubocop:disable CodeReuse/ActiveRecord -- false positive + end + end + + module Callbacks + def clear_policy_sync_state(config_id) + State.new(config_id).clear + end + + def append_projects_to_sync(config_id, project_ids) + State.new(config_id).append_projects(project_ids) + end + + def finish_project_policy_sync(project_id) + State.from_application_context&.finish_project(project_id) + end + + def fail_project_policy_sync(project_id) + State.from_application_context&.fail_project(project_id) + end + + def start_merge_request_policy_sync(merge_request_id) + State.from_application_context&.start_merge_request(merge_request_id) + end + + def start_merge_request_worker_policy_sync(merge_request_id) + State.from_application_context&.start_merge_request_worker(merge_request_id) + end + + def finish_merge_request_worker_policy_sync(merge_request_id) + State.from_application_context&.finish_merge_request_worker(merge_request_id) + end + end + end + end +end diff --git a/ee/spec/graphql/graphql_triggers_spec.rb b/ee/spec/graphql/graphql_triggers_spec.rb index fdf23b054f62b4..d88fd092a9a225 100644 --- a/ee/spec/graphql/graphql_triggers_spec.rb +++ b/ee/spec/graphql/graphql_triggers_spec.rb @@ -188,4 +188,40 @@ end end end + + describe '.security_policies_sync_updated', feature_category: :security_policy_management do + subject(:trigger) do + described_class.security_policies_sync_updated( + container, + projects, + projects_total, + failed_projects, + merge_requests, + merge_requests_total + ) + end + + let_it_be(:container) { create(:project) } + let(:projects) { 75.5 } + let(:projects_total) { 100 } + let(:failed_projects) { [non_existing_record_id.to_s] } + let(:merge_requests) { 50.0 } + let(:merge_requests_total) { 200 } + + specify do + expect(GitlabSchema.subscriptions).to receive(:trigger).with( + :security_policies_sync_updated, + { full_path: container.full_path }, + { + projects: projects, + projects_total: projects_total, + failed_projects: failed_projects, + merge_requests: merge_requests, + merge_requests_total: merge_requests_total + } + ).and_call_original + + trigger + end + end end diff --git a/ee/spec/graphql/subscriptions/security/policies_sync_updated_spec.rb b/ee/spec/graphql/subscriptions/security/policies_sync_updated_spec.rb new file mode 100644 index 00000000000000..f16aa1f748e6d3 --- /dev/null +++ b/ee/spec/graphql/subscriptions/security/policies_sync_updated_spec.rb @@ -0,0 +1,62 @@ +# frozen_string_literal: true + +require "spec_helper" + +RSpec.describe Subscriptions::Security::PoliciesSyncUpdated, feature_category: :security_policy_management do + include GraphqlHelpers + include ::Graphql::Subscriptions::Security::PoliciesSyncUpdated::Helper + + let_it_be(:policy_configuration) { create(:security_orchestration_policy_configuration, :namespace) } + let_it_be(:security_policy_project) { policy_configuration.security_policy_management_project } + let_it_be(:full_path) { security_policy_project.full_path } + let_it_be(:current_user) { security_policy_project.creator } + + let(:projects) { 75.5 } + let(:projects_total) { 100 } + let(:failed_projects) { ["123"] } + let(:merge_requests) { 50 } + let(:merge_requests_total) { 200 } + + let(:subscribe) { security_policies_sync_updated_subscription(security_policy_project, current_user) } + + before do + stub_licensed_features(security_orchestration_policies: true) + + stub_const('GitlabSchema', Graphql::Subscriptions::ActionCable::MockGitlabSchema) + Graphql::Subscriptions::ActionCable::MockActionCable.clear_mocks + end + + subject(:response) do + subscription_response do + GraphqlTriggers.security_policies_sync_updated( + security_policy_project, + projects, + projects_total, + failed_projects, + merge_requests, + merge_requests_total) + end + end + + context 'when authorized' do + subject(:success_response) do + graphql_dig_at(graphql_data(response[:result]), :securityPoliciesSyncUpdated) + end + + specify do + expect(success_response).to eq({ + "projects" => projects, + "projectsTotal" => projects_total, + "failedProjects" => failed_projects, + "mergeRequests" => merge_requests, + "mergeRequestsTotal" => merge_requests_total + }) + end + end + + context 'when unauthorized' do + let_it_be(:current_user) { create(:user) } + + it { is_expected.to be_nil } + end +end diff --git a/ee/spec/lib/security/security_orchestration_policies/policy_sync_state_spec.rb b/ee/spec/lib/security/security_orchestration_policies/policy_sync_state_spec.rb new file mode 100644 index 00000000000000..65c68c7c21652e --- /dev/null +++ b/ee/spec/lib/security/security_orchestration_policies/policy_sync_state_spec.rb @@ -0,0 +1,497 @@ +# frozen_string_literal: true + +require 'spec_helper' + +# rubocop:disable RSpec/SpecFilePathFormat -- callbacks have indirect test coverage +RSpec.describe Security::SecurityOrchestrationPolicies::PolicySyncState::State, feature_category: :security_policy_management do + # rubocop:enable RSpec/SpecFilePathFormat + let(:merge_request_id) { 1 } + let(:project_id) { 1 } + let(:other_project_id) { 2 } + + let_it_be(:policy_configuration) { create(:security_orchestration_policy_configuration, :namespace) } + let(:policy_configuration_id) { policy_configuration.id } + + subject(:state) { described_class.new(policy_configuration.id) } + + before do + Gitlab::Redis::SharedState.with(&:flushall) + end + + describe '.from_application_context' do + before do + allow(Gitlab::ApplicationContext).to receive(:current_context_attribute).and_return(context_value) + end + + subject(:state) { described_class.from_application_context } + + context 'with context key' do + let(:context_value) { policy_configuration.id.to_s } + + let_it_be(:policy_configuration) { create(:security_orchestration_policy_configuration, :namespace) } + + it { is_expected.to be_a(described_class) } + end + + context 'without context key' do + let(:context_value) { nil } + + it { is_expected.to be_nil } + end + end + + describe '#append_projects' do + context 'when adding new project IDs' do + it 'adds project IDs as pending' do + expect { state.append_projects([project_id, other_project_id]) }.to change { + state.pending_projects + }.from(be_empty).to(contain_exactly(project_id.to_s, other_project_id.to_s)) + end + + it 'maintains set membership' do + expect { 2.times { state.append_projects([project_id]) } }.to change { + state.pending_projects + }.from(be_empty).to(contain_exactly(project_id.to_s)) + end + + it 'increments the total counter by the number of projects' do + state.append_projects([1, 2, 3]) + state.append_projects([4, 5]) + + expect(state.total_project_count).to be(5) + end + end + + context 'with feature disabled' do + before do + stub_feature_flags(security_policy_sync_propagation_tracking: false) + end + + it 'does not add pending IDs' do + expect { state.append_projects([project_id, other_project_id]) }.not_to change { + state.pending_projects + }.from(be_empty) + end + end + end + + describe '#finish_project' do + before do + state.append_projects([project_id, other_project_id]) + end + + it 'removes a pending project ID' do + expect { state.finish_project(project_id) }.to change { + state.pending_projects + }.from(contain_exactly(project_id.to_s, other_project_id.to_s)).to(contain_exactly(other_project_id.to_s)) + end + + context 'when project ID is not pending' do + it 'does not alter pending project IDs' do + expect { state.finish_project(4) }.not_to change { + state.pending_projects + }.from(contain_exactly(project_id.to_s, other_project_id.to_s)) + end + end + + it 'triggers subscription' do + expect(state).to receive(:trigger_subscription).exactly(:once).and_call_original + + state.finish_project(other_project_id) + end + + context 'with feature disabled' do + before do + stub_feature_flags(security_policy_sync_propagation_tracking: false) + + state.clear_memoization(:feature_disabled) + end + + it 'does not remove a pending project ID' do + expect { state.finish_project(project_id) }.not_to change { + state.pending_projects + }.from(contain_exactly(project_id.to_s, other_project_id.to_s)) + end + + it 'does not trigger subscription' do + expect(state).not_to receive(:trigger_subscription) + + state.finish_project(other_project_id) + end + end + end + + describe '#fail_project' do + it 'adds a project ID as failed' do + expect { state.fail_project(project_id) }.to change { + state.failed_projects + }.from(be_empty).to(contain_exactly(project_id.to_s)) + end + + it 'removes a project ID as pending' do + state.append_projects([project_id]) + + expect { state.fail_project(project_id) }.to change { + state.pending_projects + }.from(contain_exactly(project_id.to_s)).to(be_empty) + end + + it 'maintains set membership' do + expect { 2.times { state.fail_project(project_id) } }.to change { + state.failed_projects + }.from(be_empty).to(contain_exactly(project_id.to_s)) + end + + it 'triggers subscription' do + expect(state).to receive(:trigger_subscription).exactly(:once).and_call_original + + state.finish_project(project_id) + end + + context 'with feature disabled' do + before do + stub_feature_flags(security_policy_sync_propagation_tracking: false) + end + + it 'does not add a project ID as failed' do + expect { state.fail_project(project_id) }.not_to change { + state.failed_projects + }.from(be_empty) + end + + it 'does not trigger subscription' do + expect(state).not_to receive(:trigger_subscription) + + state.finish_project(project_id) + end + end + end + + describe '#start_merge_request' do + it 'adds a merge request ID as pending' do + expect { state.start_merge_request(merge_request_id) }.to change { + state.pending_merge_requests + }.from(be_empty).to(match_array("1")) + end + + it 'maintains set membership' do + expect { 2.times { state.start_merge_request(merge_request_id) } }.to change { + state.pending_merge_requests + }.from(be_empty).to(match_array("1")) + end + + it 'increases total merge request count' do + expect { state.start_merge_request(merge_request_id) }.to change { + state.total_merge_request_count + }.from(nil).to(1) + end + + it 'initializes merge request worker count' do + expect { state.start_merge_request(merge_request_id) }.to change { + state.total_merge_request_workers_count(merge_request_id) + }.from(nil).to(0) + end + + context 'with feature disabled' do + before do + stub_feature_flags(security_policy_sync_propagation_tracking: false) + end + + it 'does not add a merge request ID as pending' do + expect { state.start_merge_request(merge_request_id) }.not_to change { + state.pending_merge_requests + }.from(be_empty) + end + + it 'does not increase total merge request count' do + expect { state.start_merge_request(merge_request_id) }.not_to change { + state.total_merge_request_count + }.from(nil) + end + + it 'does not initialize merge request worker count' do + expect { state.start_merge_request(merge_request_id) }.not_to change { + state.total_merge_request_workers_count(merge_request_id) + }.from(nil) + end + end + end + + describe '#start_merge_request_worker' do + it 'increases merge request worker count', :aggregate_failures do + expect { state.start_merge_request_worker(merge_request_id) }.to change { + state.total_merge_request_workers_count(merge_request_id) + }.from(nil).to(1) + + expect { state.start_merge_request_worker(merge_request_id) }.to change { + state.total_merge_request_workers_count(merge_request_id) + }.from(1).to(2) + end + + context 'with feature disabled' do + before do + stub_feature_flags(security_policy_sync_propagation_tracking: false) + end + + it 'does not increase merge request worker count', :aggregate_failures do + expect { state.start_merge_request_worker(merge_request_id) }.not_to change { + state.total_merge_request_workers_count(merge_request_id) + }.from(nil) + + expect { state.start_merge_request_worker(merge_request_id) }.not_to change { + state.total_merge_request_workers_count(merge_request_id) + }.from(nil) + end + end + end + + describe '#finish_merge_request_worker' do + before do + state.start_merge_request(merge_request_id) + end + + context 'with last worker' do + before do + state.start_merge_request_worker(merge_request_id) + end + + it 'decrements merge request worker count' do + expect { state.finish_merge_request_worker(merge_request_id) }.to change { + state.total_merge_request_workers_count(merge_request_id) + }.from(1).to(0) + end + + it 'removes merge request from pending set' do + expect { state.finish_merge_request_worker(merge_request_id) }.to change { + state.pending_merge_requests + }.from(contain_exactly(merge_request_id.to_s)).to(be_empty) + end + + it 'triggers subscription' do + expect(state).to receive(:trigger_subscription).exactly(:once).and_call_original + + state.finish_merge_request_worker(merge_request_id) + end + + context 'with feature disabled' do + before do + stub_feature_flags(security_policy_sync_propagation_tracking: false) + + state.clear_memoization(:feature_disabled) + end + + it 'does not decrement merge request worker count' do + expect { state.finish_merge_request_worker(merge_request_id) }.not_to change { + state.total_merge_request_workers_count(merge_request_id) + }.from(1) + end + + it 'does not remove merge request from pending set' do + expect { state.finish_merge_request_worker(merge_request_id) }.not_to change { + state.pending_merge_requests + }.from(contain_exactly(merge_request_id.to_s)) + end + + it 'does not trigger subscription' do + expect(state).not_to receive(:trigger_subscription) + + state.finish_merge_request_worker(merge_request_id) + end + end + end + + context 'with remaining workers' do + before do + 2.times { state.start_merge_request_worker(merge_request_id) } + end + + it 'decrements merge request worker count' do + expect { state.finish_merge_request_worker(merge_request_id) }.to change { + state.total_merge_request_workers_count(merge_request_id) + }.from(2).to(1) + end + + it 'does not remove merge request from pending set' do + expect { state.finish_merge_request_worker(merge_request_id) }.not_to change { + state.pending_merge_requests + }.from(contain_exactly(merge_request_id.to_s)) + end + + it 'does not trigger subscription' do + expect(state).not_to receive(:trigger_subscription) + + state.finish_merge_request_worker(merge_request_id) + end + + context 'with feature disabled' do + before do + stub_feature_flags(security_policy_sync_propagation_tracking: false) + + state.clear_memoization(:feature_disabled) + end + + it 'does not decrement merge request worker count' do + expect { state.finish_merge_request_worker(merge_request_id) }.not_to change { + state.total_merge_request_workers_count(merge_request_id) + }.from(2) + end + + it 'does not trigger subscription' do + expect(state).not_to receive(:trigger_subscription) + + state.finish_merge_request_worker(merge_request_id) + end + end + end + end + + describe '#sync_in_progress?' do + subject(:sync_in_progress?) { state.sync_in_progress? } + + it { is_expected.to be(false) } + + context 'with project and merge request' do + before do + state.append_projects([project_id]) + state.start_merge_request(merge_request_id) + state.start_merge_request_worker(merge_request_id) + end + + it { is_expected.to be(true) } + + context 'with feature disabled' do + before do + stub_feature_flags(security_policy_sync_propagation_tracking: false) + + state.clear_memoization(:feature_disabled) + end + + it { is_expected.to be(false) } + end + + context 'with all projects processed' do + before do + state.finish_project(project_id) + end + + it { is_expected.to be(true) } + end + + context 'with all merge request processed' do + before do + state.finish_merge_request_worker(merge_request_id) + end + + it { is_expected.to be(true) } + end + + context 'with all projects and merge requests processed' do + before do + state.finish_project(project_id) + state.finish_merge_request_worker(merge_request_id) + end + + it { is_expected.to be(false) } + end + end + end + + describe '#clear' do + subject(:clear) { state.clear } + + before do + state.append_projects([project_id]) + state.start_merge_request(merge_request_id) + end + + it 'resets pending project IDs' do + expect { clear }.to change { state.pending_projects }.from(contain_exactly(project_id.to_s)).to(be_empty) + end + + it 'resets pending merge request IDs' do + expect { clear }.to change { + state.pending_merge_requests + }.from(contain_exactly(merge_request_id.to_s)).to(be_empty) + end + + context 'with feature disabled' do + before do + stub_feature_flags(security_policy_sync_propagation_tracking: false) + + state.clear_memoization(:feature_disabled) + end + + it 'does not reset pending project IDs' do + expect { clear }.not_to change { state.pending_projects }.from(contain_exactly(project_id.to_s)) + end + + it 'does not reset pending merge request IDs' do + expect { clear }.not_to change { + state.pending_merge_requests + }.from(contain_exactly(merge_request_id.to_s)) + end + end + end + + describe '#trigger_subscription' do + let_it_be(:policy_configuration) { create(:security_orchestration_policy_configuration, :namespace) } + + subject(:state) { described_class.new(policy_configuration.id) } + + before do + 10.times do |i| + state.append_projects([i]) + state.start_merge_request(i) + state.start_merge_request_worker(i) + end + end + + it 'publishes with correct values at each step' do + expect(GraphqlTriggers).to receive(:security_policies_sync_updated).with( + policy_configuration.security_policy_management_project, + 10, # project progress: (10 total - 9 pending) / 10 * 100 = 10% + 10, # projects total + [], # no failed projects yet + 0, # merge request progress: (10 total - 10 pending) / 10 * 100 = 0% + 10 # merge requests total + ).ordered + state.finish_project(1) + + expect(GraphqlTriggers).to receive(:security_policies_sync_updated).with( + policy_configuration.security_policy_management_project, + 10, # project progress: still 10% + 10, # projects total + [], # no failed projects yet + 10, # merge request progress: (10 total - 9 pending) / 10 * 100 = 10% + 10 # merge requests total + ).ordered + state.finish_merge_request_worker(1) + + expect(GraphqlTriggers).to receive(:security_policies_sync_updated).with( + policy_configuration.security_policy_management_project, + 20, # project progress: (10 total - 8 pending) / 10 * 100 = 20% + 10, # projects total + ["2"], # failed project 2 + 10, # merge request progress: still 10% + 10 # merge requests total + ).ordered + state.fail_project(2) + end + + it 'shows increasing progress as items complete' do + 5.times do |i| + allow(GraphqlTriggers).to receive(:security_policies_sync_updated) + state.finish_project(i) + end + + expect(GraphqlTriggers).to have_received(:security_policies_sync_updated).with( + policy_configuration.security_policy_management_project, + 50, # project progress: (10 - 5) / 10 * 100 = 50% + 10, # projects total + [], # no failed projects + 0, # merge request progress: still 0% + 10 # merge requests total + ) + end + end +end diff --git a/ee/spec/support/helpers/graphql/subscriptions/security/policies_sync_update/helper.rb b/ee/spec/support/helpers/graphql/subscriptions/security/policies_sync_update/helper.rb new file mode 100644 index 00000000000000..e1b374aa5a21cb --- /dev/null +++ b/ee/spec/support/helpers/graphql/subscriptions/security/policies_sync_update/helper.rb @@ -0,0 +1,42 @@ +# frozen_string_literal: true + +module Graphql + module Subscriptions + module Security + module PoliciesSyncUpdated + module Helper + def subscription_response + subscription_channel = subscribe + yield + subscription_channel.mock_broadcasted_messages.first + end + + def security_policies_sync_updated_subscription(container, current_user) + mock_channel = Graphql::Subscriptions::ActionCable::MockActionCable.get_mock_channel + query = security_policies_sync_updated_subscription_query(container) + + GitlabSchema.execute(query, context: { current_user: current_user, channel: mock_channel }) + + mock_channel + end + + private + + def security_policies_sync_updated_subscription_query(container) + <<~SUBSCRIPTION + subscription { + securityPoliciesSyncUpdated(fullPath: \"#{container.full_path}\") { + projects + projectsTotal + failedProjects + mergeRequests + mergeRequestsTotal + } + } + SUBSCRIPTION + end + end + end + end + end +end -- GitLab From 20da8c489a9979246d324d4435502f390809f11c Mon Sep 17 00:00:00 2001 From: Dominic Bauer Date: Mon, 18 Aug 2025 16:25:26 +0200 Subject: [PATCH 02/16] Update feature flag manifest --- .../security_policy_sync_propagation_tracking.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ee/config/feature_flags/gitlab_com_derisk/security_policy_sync_propagation_tracking.yml b/ee/config/feature_flags/gitlab_com_derisk/security_policy_sync_propagation_tracking.yml index 66ca0ae2a90919..c8fe9136f2a8d8 100644 --- a/ee/config/feature_flags/gitlab_com_derisk/security_policy_sync_propagation_tracking.yml +++ b/ee/config/feature_flags/gitlab_com_derisk/security_policy_sync_propagation_tracking.yml @@ -2,7 +2,7 @@ name: security_policy_sync_propagation_tracking description: feature_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/559273 -introduced_by_url: +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/201777 rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/561007 milestone: '18.4' group: group::security policies -- GitLab From f7f89cc6d41b8a89cb1832c0b81a1b629226a06b Mon Sep 17 00:00:00 2001 From: Dominic Bauer Date: Mon, 18 Aug 2025 16:29:32 +0200 Subject: [PATCH 03/16] Bump milestone to %18.4 --- ee/app/graphql/ee/types/subscription_type.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ee/app/graphql/ee/types/subscription_type.rb b/ee/app/graphql/ee/types/subscription_type.rb index 507dc4e8fc917c..842c6bc9c8ded9 100644 --- a/ee/app/graphql/ee/types/subscription_type.rb +++ b/ee/app/graphql/ee/types/subscription_type.rb @@ -44,7 +44,7 @@ def self.authorization_scopes field :security_policies_sync_updated, subscription: Subscriptions::Security::PoliciesSyncUpdated, null: true, description: 'Triggered when the security policies are being synced and the status is updated.', - experiment: { milestone: '18.3' } + experiment: { milestone: '18.4' } end end end -- GitLab From 84069e6e25814364b1d07bf60cb7ac7df6687cd8 Mon Sep 17 00:00:00 2001 From: Dominic Bauer Date: Tue, 19 Aug 2025 08:02:43 +0200 Subject: [PATCH 04/16] Update GraphQL descriptions --- doc/api/graphql/reference/_index.md | 5 +++-- ee/app/graphql/ee/types/subscription_type.rb | 2 +- .../gitlab_subscriptions/security/policies_sync_updated.rb | 5 +++-- 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/doc/api/graphql/reference/_index.md b/doc/api/graphql/reference/_index.md index 3c8d1278e73ef6..0207ad6233f46a 100644 --- a/doc/api/graphql/reference/_index.md +++ b/doc/api/graphql/reference/_index.md @@ -37506,7 +37506,8 @@ Check permissions for the current user on a vulnerability finding. ### `PoliciesSyncUpdated` -Security policy state synchronization update. +Security policy state synchronization update. \ + Returns `null` if the `security_policy_sync_propagation_tracking` feature flag is disabled. #### Fields @@ -37515,7 +37516,7 @@ Security policy state synchronization update. | `failedProjects` | [`[String!]`](#string) | IDs of failed projects. | | `mergeRequests` | [`Float`](#float) | Status of merge requests sync in percentage. | | `mergeRequestsTotal` | [`Int`](#int) | Total number of merge requests synced. | -| `projects` | [`Float`](#float) | Status of projects sync in percentage. | +| `projects` | [`Float`](#float) | Percentage of projects synced. | | `projectsTotal` | [`Int`](#int) | Total number of projects synced. | ### `PolicyAnyMergeRequestViolation` diff --git a/ee/app/graphql/ee/types/subscription_type.rb b/ee/app/graphql/ee/types/subscription_type.rb index 842c6bc9c8ded9..90ec9c5fa418fa 100644 --- a/ee/app/graphql/ee/types/subscription_type.rb +++ b/ee/app/graphql/ee/types/subscription_type.rb @@ -43,7 +43,7 @@ def self.authorization_scopes field :security_policies_sync_updated, subscription: Subscriptions::Security::PoliciesSyncUpdated, null: true, - description: 'Triggered when the security policies are being synced and the status is updated.', + description: 'Triggered when the security policy sync updates the status.', experiment: { milestone: '18.4' } end end diff --git a/ee/app/graphql/types/gitlab_subscriptions/security/policies_sync_updated.rb b/ee/app/graphql/types/gitlab_subscriptions/security/policies_sync_updated.rb index 7c4387c925d19c..3f6dc329071762 100644 --- a/ee/app/graphql/types/gitlab_subscriptions/security/policies_sync_updated.rb +++ b/ee/app/graphql/types/gitlab_subscriptions/security/policies_sync_updated.rb @@ -7,11 +7,12 @@ module Security # rubocop:disable GraphQL/ExtractType -- Not worth combining (merge_requests, merge_requests_total) into a newtype class PoliciesSyncUpdated < ::Types::BaseObject graphql_name 'PoliciesSyncUpdated' - description 'Security policy state synchronization update' + description 'Security policy state synchronization update. \ + Returns `null` if the `security_policy_sync_propagation_tracking` feature flag is disabled.' field :projects, GraphQL::Types::Float, null: true, - description: 'Status of projects sync in percentage.' + description: 'Percentage of projects synced.' field :projects_total, GraphQL::Types::Int, null: true, -- GitLab From f0514efcbcef47d2466393262cd39d3e51c5c26e Mon Sep 17 00:00:00 2001 From: Dominic Bauer Date: Tue, 19 Aug 2025 08:04:29 +0200 Subject: [PATCH 05/16] Fix typo in comment --- .../security_orchestration_policies/policy_sync_state.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ee/lib/security/security_orchestration_policies/policy_sync_state.rb b/ee/lib/security/security_orchestration_policies/policy_sync_state.rb index c162d7d7c4907b..3abcbaa8859a21 100644 --- a/ee/lib/security/security_orchestration_policies/policy_sync_state.rb +++ b/ee/lib/security/security_orchestration_policies/policy_sync_state.rb @@ -215,7 +215,7 @@ def clear_pending_projects end end - # Deletes merge request-related keys for the policy cofniguration sync + # Deletes merge request-related keys for the policy configuration sync def clear_pending_merge_requests with_redis do |redis| redis.del(merge_requests_sync_key) -- GitLab From 867e25ceed52f86218ba3506f7a03eb063aac5ad Mon Sep 17 00:00:00 2001 From: Dominic Bauer Date: Wed, 20 Aug 2025 08:56:20 +0200 Subject: [PATCH 06/16] Update GraphQL field documentation --- doc/api/graphql/reference/_index.md | 5 ++--- .../gitlab_subscriptions/security/policies_sync_updated.rb | 7 ++++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/doc/api/graphql/reference/_index.md b/doc/api/graphql/reference/_index.md index 0207ad6233f46a..21690828fcf004 100644 --- a/doc/api/graphql/reference/_index.md +++ b/doc/api/graphql/reference/_index.md @@ -37506,15 +37506,14 @@ Check permissions for the current user on a vulnerability finding. ### `PoliciesSyncUpdated` -Security policy state synchronization update. \ - Returns `null` if the `security_policy_sync_propagation_tracking` feature flag is disabled. +Security policy state synchronization update. Returns `null` if the `security_policy_sync_propagation_tracking` feature flag is disabled. #### Fields | Name | Type | Description | | ---- | ---- | ----------- | | `failedProjects` | [`[String!]`](#string) | IDs of failed projects. | -| `mergeRequests` | [`Float`](#float) | Status of merge requests sync in percentage. | +| `mergeRequests` | [`Float`](#float) | Percentage of merge requests synced. | | `mergeRequestsTotal` | [`Int`](#int) | Total number of merge requests synced. | | `projects` | [`Float`](#float) | Percentage of projects synced. | | `projectsTotal` | [`Int`](#int) | Total number of projects synced. | diff --git a/ee/app/graphql/types/gitlab_subscriptions/security/policies_sync_updated.rb b/ee/app/graphql/types/gitlab_subscriptions/security/policies_sync_updated.rb index 3f6dc329071762..570846354a9421 100644 --- a/ee/app/graphql/types/gitlab_subscriptions/security/policies_sync_updated.rb +++ b/ee/app/graphql/types/gitlab_subscriptions/security/policies_sync_updated.rb @@ -7,8 +7,9 @@ module Security # rubocop:disable GraphQL/ExtractType -- Not worth combining (merge_requests, merge_requests_total) into a newtype class PoliciesSyncUpdated < ::Types::BaseObject graphql_name 'PoliciesSyncUpdated' - description 'Security policy state synchronization update. \ - Returns `null` if the `security_policy_sync_propagation_tracking` feature flag is disabled.' + # rubocop:disable Layout/LineLength -- Ensures correct Markdown rendering + description 'Security policy state synchronization update. Returns `null` if the `security_policy_sync_propagation_tracking` feature flag is disabled.' + # rubocop:enable Layout/LineLength field :projects, GraphQL::Types::Float, null: true, @@ -24,7 +25,7 @@ class PoliciesSyncUpdated < ::Types::BaseObject field :merge_requests, GraphQL::Types::Float, null: true, - description: 'Status of merge requests sync in percentage.' + description: 'Percentage of merge requests synced.' field :merge_requests_total, GraphQL::Types::Int, null: true, -- GitLab From 9ac322e640063be79f7c2cab28f69c5678f8eaa3 Mon Sep 17 00:00:00 2001 From: Dominic Bauer Date: Wed, 20 Aug 2025 11:37:39 +0200 Subject: [PATCH 07/16] Namespace `State` specs --- .../state_spec.rb} | 2 -- 1 file changed, 2 deletions(-) rename ee/spec/lib/security/security_orchestration_policies/{policy_sync_state_spec.rb => policy_sync_state/state_spec.rb} (99%) diff --git a/ee/spec/lib/security/security_orchestration_policies/policy_sync_state_spec.rb b/ee/spec/lib/security/security_orchestration_policies/policy_sync_state/state_spec.rb similarity index 99% rename from ee/spec/lib/security/security_orchestration_policies/policy_sync_state_spec.rb rename to ee/spec/lib/security/security_orchestration_policies/policy_sync_state/state_spec.rb index 65c68c7c21652e..fff51713b217db 100644 --- a/ee/spec/lib/security/security_orchestration_policies/policy_sync_state_spec.rb +++ b/ee/spec/lib/security/security_orchestration_policies/policy_sync_state/state_spec.rb @@ -2,9 +2,7 @@ require 'spec_helper' -# rubocop:disable RSpec/SpecFilePathFormat -- callbacks have indirect test coverage RSpec.describe Security::SecurityOrchestrationPolicies::PolicySyncState::State, feature_category: :security_policy_management do - # rubocop:enable RSpec/SpecFilePathFormat let(:merge_request_id) { 1 } let(:project_id) { 1 } let(:other_project_id) { 2 } -- GitLab From 046566b1118b074ed2ddd324144dfafb99215b4d Mon Sep 17 00:00:00 2001 From: Dominic Bauer Date: Thu, 21 Aug 2025 09:30:31 +0200 Subject: [PATCH 08/16] Subscribe by policy configuration GlobalID instead of `full_path` --- doc/api/graphql/reference/_index.md | 6 ++++++ ee/app/graphql/ee/graphql_triggers.rb | 4 ++-- .../security/policies_sync_updated.rb | 21 ++++++++++++++----- .../policy_sync_state.rb | 14 ++++++++----- ee/spec/graphql/graphql_triggers_spec.rb | 6 +++--- .../security/policies_sync_updated_spec.rb | 5 ++--- .../policy_sync_state/state_spec.rb | 8 +++---- .../helper.rb | 8 +++---- 8 files changed, 46 insertions(+), 26 deletions(-) rename ee/spec/support/helpers/graphql/subscriptions/security/{policies_sync_update => policies_sync_updated}/helper.rb (80%) diff --git a/doc/api/graphql/reference/_index.md b/doc/api/graphql/reference/_index.md index 21690828fcf004..6add9247f27cd4 100644 --- a/doc/api/graphql/reference/_index.md +++ b/doc/api/graphql/reference/_index.md @@ -50906,6 +50906,12 @@ A `SbomOccurrenceID` is a global ID. It is encoded as a string. An example `SbomOccurrenceID` is: `"gid://gitlab/Sbom::Occurrence/1"`. +### `SecurityOrchestrationPolicyConfigurationID` + +A `SecurityOrchestrationPolicyConfigurationID` is a global ID. It is encoded as a string. + +An example `SecurityOrchestrationPolicyConfigurationID` is: `"gid://gitlab/Security::OrchestrationPolicyConfiguration/1"`. + ### `SecurityProjectSecurityExclusionID` A `SecurityProjectSecurityExclusionID` is a global ID. It is encoded as a string. diff --git a/ee/app/graphql/ee/graphql_triggers.rb b/ee/app/graphql/ee/graphql_triggers.rb index eb52af403efd1b..fa0a0e0adc434c 100644 --- a/ee/app/graphql/ee/graphql_triggers.rb +++ b/ee/app/graphql/ee/graphql_triggers.rb @@ -70,10 +70,10 @@ def self.security_policy_project_created(container, status, security_policy_proj end def self.security_policies_sync_updated( - container, projects, projects_total, failed_projects, merge_requests, merge_requests_total) + policy_configuration, projects, projects_total, failed_projects, merge_requests, merge_requests_total) ::GitlabSchema.subscriptions.trigger( :security_policies_sync_updated, - { full_path: container.full_path }, + { policy_configuration_id: policy_configuration.to_global_id }, { projects: projects, projects_total: projects_total, failed_projects: failed_projects, merge_requests: merge_requests, merge_requests_total: merge_requests_total } ) diff --git a/ee/app/graphql/subscriptions/security/policies_sync_updated.rb b/ee/app/graphql/subscriptions/security/policies_sync_updated.rb index 3a875d4f87eaf7..39b0079144daa9 100644 --- a/ee/app/graphql/subscriptions/security/policies_sync_updated.rb +++ b/ee/app/graphql/subscriptions/security/policies_sync_updated.rb @@ -7,14 +7,19 @@ class PoliciesSyncUpdated < ::Subscriptions::BaseSubscription payload_type Types::GitlabSubscriptions::Security::PoliciesSyncUpdated - argument :full_path, GraphQL::Types::String, + argument :policy_configuration_id, ::Types::GlobalIDType[::Security::OrchestrationPolicyConfiguration], required: true, - description: 'Full path of the project or group.' + description: 'ID of the security orchestration policy configuration.' - def authorized?(full_path:) - container = Routable.find_by_full_path(full_path) + def authorized?(policy_configuration_id:) + policy_configuration = find_policy_configuration(policy_configuration_id) || unauthorized! - Ability.allowed?(current_user, :update_security_orchestration_policy_project, container) + if current_user.can?(:update_security_orchestration_policy_project, + policy_configuration.security_policy_management_project) + return true + end + + unauthorized! end def update(_) @@ -26,6 +31,12 @@ def update(_) merge_requests_total: object[:merge_requests_total] } end + + private + + def find_policy_configuration(id) + force(GitlabSchema.find_by_gid(id)) + end end end end diff --git a/ee/lib/security/security_orchestration_policies/policy_sync_state.rb b/ee/lib/security/security_orchestration_policies/policy_sync_state.rb index 3abcbaa8859a21..dc76b281150abc 100644 --- a/ee/lib/security/security_orchestration_policies/policy_sync_state.rb +++ b/ee/lib/security/security_orchestration_policies/policy_sync_state.rb @@ -236,7 +236,7 @@ def trigger_subscription merge_requests_total = get_counter(total_merge_requests_key) GraphqlTriggers.security_policies_sync_updated( - policy_project, + policy_configuration, get_progress(projects_pending, projects_total), projects_total, failed_projects, @@ -247,13 +247,17 @@ def trigger_subscription def feature_disabled? strong_memoize_with(:feature_disabled, config_id) do - Feature.disabled?(:security_policy_sync_propagation_tracking, policy_project) + project = policy_configuration&.security_policy_management_project + + break true unless project + + Feature.disabled?(:security_policy_sync_propagation_tracking, project) end end - def policy_project - strong_memoize_with(:policy_project, config_id) do - Security::OrchestrationPolicyConfiguration.find_by_id(config_id)&.security_policy_management_project + def policy_configuration + strong_memoize_with(:policy_configuration, config_id) do + Security::OrchestrationPolicyConfiguration.find_by_id(config_id) end end diff --git a/ee/spec/graphql/graphql_triggers_spec.rb b/ee/spec/graphql/graphql_triggers_spec.rb index d88fd092a9a225..fd6bf600c46edd 100644 --- a/ee/spec/graphql/graphql_triggers_spec.rb +++ b/ee/spec/graphql/graphql_triggers_spec.rb @@ -192,7 +192,7 @@ describe '.security_policies_sync_updated', feature_category: :security_policy_management do subject(:trigger) do described_class.security_policies_sync_updated( - container, + policy_configuration, projects, projects_total, failed_projects, @@ -201,7 +201,7 @@ ) end - let_it_be(:container) { create(:project) } + let_it_be(:policy_configuration) { create(:security_orchestration_policy_configuration) } let(:projects) { 75.5 } let(:projects_total) { 100 } let(:failed_projects) { [non_existing_record_id.to_s] } @@ -211,7 +211,7 @@ specify do expect(GitlabSchema.subscriptions).to receive(:trigger).with( :security_policies_sync_updated, - { full_path: container.full_path }, + { policy_configuration_id: policy_configuration.to_global_id }, { projects: projects, projects_total: projects_total, diff --git a/ee/spec/graphql/subscriptions/security/policies_sync_updated_spec.rb b/ee/spec/graphql/subscriptions/security/policies_sync_updated_spec.rb index f16aa1f748e6d3..ab307fc38fbdcd 100644 --- a/ee/spec/graphql/subscriptions/security/policies_sync_updated_spec.rb +++ b/ee/spec/graphql/subscriptions/security/policies_sync_updated_spec.rb @@ -8,7 +8,6 @@ let_it_be(:policy_configuration) { create(:security_orchestration_policy_configuration, :namespace) } let_it_be(:security_policy_project) { policy_configuration.security_policy_management_project } - let_it_be(:full_path) { security_policy_project.full_path } let_it_be(:current_user) { security_policy_project.creator } let(:projects) { 75.5 } @@ -17,7 +16,7 @@ let(:merge_requests) { 50 } let(:merge_requests_total) { 200 } - let(:subscribe) { security_policies_sync_updated_subscription(security_policy_project, current_user) } + let(:subscribe) { security_policies_sync_updated_subscription(policy_configuration, current_user) } before do stub_licensed_features(security_orchestration_policies: true) @@ -29,7 +28,7 @@ subject(:response) do subscription_response do GraphqlTriggers.security_policies_sync_updated( - security_policy_project, + policy_configuration, projects, projects_total, failed_projects, diff --git a/ee/spec/lib/security/security_orchestration_policies/policy_sync_state/state_spec.rb b/ee/spec/lib/security/security_orchestration_policies/policy_sync_state/state_spec.rb index fff51713b217db..bf88bc81fcdb17 100644 --- a/ee/spec/lib/security/security_orchestration_policies/policy_sync_state/state_spec.rb +++ b/ee/spec/lib/security/security_orchestration_policies/policy_sync_state/state_spec.rb @@ -446,7 +446,7 @@ it 'publishes with correct values at each step' do expect(GraphqlTriggers).to receive(:security_policies_sync_updated).with( - policy_configuration.security_policy_management_project, + policy_configuration, 10, # project progress: (10 total - 9 pending) / 10 * 100 = 10% 10, # projects total [], # no failed projects yet @@ -456,7 +456,7 @@ state.finish_project(1) expect(GraphqlTriggers).to receive(:security_policies_sync_updated).with( - policy_configuration.security_policy_management_project, + policy_configuration, 10, # project progress: still 10% 10, # projects total [], # no failed projects yet @@ -466,7 +466,7 @@ state.finish_merge_request_worker(1) expect(GraphqlTriggers).to receive(:security_policies_sync_updated).with( - policy_configuration.security_policy_management_project, + policy_configuration, 20, # project progress: (10 total - 8 pending) / 10 * 100 = 20% 10, # projects total ["2"], # failed project 2 @@ -483,7 +483,7 @@ end expect(GraphqlTriggers).to have_received(:security_policies_sync_updated).with( - policy_configuration.security_policy_management_project, + policy_configuration, 50, # project progress: (10 - 5) / 10 * 100 = 50% 10, # projects total [], # no failed projects diff --git a/ee/spec/support/helpers/graphql/subscriptions/security/policies_sync_update/helper.rb b/ee/spec/support/helpers/graphql/subscriptions/security/policies_sync_updated/helper.rb similarity index 80% rename from ee/spec/support/helpers/graphql/subscriptions/security/policies_sync_update/helper.rb rename to ee/spec/support/helpers/graphql/subscriptions/security/policies_sync_updated/helper.rb index e1b374aa5a21cb..9dfc233fa7921f 100644 --- a/ee/spec/support/helpers/graphql/subscriptions/security/policies_sync_update/helper.rb +++ b/ee/spec/support/helpers/graphql/subscriptions/security/policies_sync_updated/helper.rb @@ -11,9 +11,9 @@ def subscription_response subscription_channel.mock_broadcasted_messages.first end - def security_policies_sync_updated_subscription(container, current_user) + def security_policies_sync_updated_subscription(policy_configuration, current_user) mock_channel = Graphql::Subscriptions::ActionCable::MockActionCable.get_mock_channel - query = security_policies_sync_updated_subscription_query(container) + query = security_policies_sync_updated_subscription_query(policy_configuration) GitlabSchema.execute(query, context: { current_user: current_user, channel: mock_channel }) @@ -22,10 +22,10 @@ def security_policies_sync_updated_subscription(container, current_user) private - def security_policies_sync_updated_subscription_query(container) + def security_policies_sync_updated_subscription_query(policy_configuration) <<~SUBSCRIPTION subscription { - securityPoliciesSyncUpdated(fullPath: \"#{container.full_path}\") { + securityPoliciesSyncUpdated(policyConfigurationId: \"#{policy_configuration.to_global_id}\") { projects projectsTotal failedProjects -- GitLab From aa05720889ded922bcc46a774e8f42d5c8be17bb Mon Sep 17 00:00:00 2001 From: Dominic Bauer Date: Thu, 21 Aug 2025 12:27:54 +0200 Subject: [PATCH 09/16] Clean up keys in single transaction --- .../policy_sync_state.rb | 26 +++++-------------- 1 file changed, 7 insertions(+), 19 deletions(-) diff --git a/ee/lib/security/security_orchestration_policies/policy_sync_state.rb b/ee/lib/security/security_orchestration_policies/policy_sync_state.rb index dc76b281150abc..3b7f0ec1a1c7a5 100644 --- a/ee/lib/security/security_orchestration_policies/policy_sync_state.rb +++ b/ee/lib/security/security_orchestration_policies/policy_sync_state.rb @@ -130,8 +130,13 @@ def sync_in_progress? def clear return if feature_disabled? - clear_pending_projects - clear_pending_merge_requests + with_redis do |redis| + redis.del(projects_sync_key) + redis.del(total_projects_key) + redis.del(failed_projects_sync_key) + redis.del(merge_requests_sync_key) + redis.del(total_merge_requests_key) + end end # Pending project IDs. @@ -206,23 +211,6 @@ def failed_projects_sync_key "#{redis_key_tag}:failed_projects" end - # Deletes keys for the policy configuration sync - def clear_pending_projects - with_redis do |redis| - redis.del(projects_sync_key) - redis.del(total_projects_key) - redis.del(failed_projects_sync_key) - end - end - - # Deletes merge request-related keys for the policy configuration sync - def clear_pending_merge_requests - with_redis do |redis| - redis.del(merge_requests_sync_key) - redis.del(total_merge_requests_key) - end - end - def get_progress(pending, total) return if total == 0 -- GitLab From 343416094cb74e96296c14fbe088e84e49f944bd Mon Sep 17 00:00:00 2001 From: Dominic Bauer Date: Thu, 21 Aug 2025 12:29:22 +0200 Subject: [PATCH 10/16] Clean test Redis state by tag --- .../policy_sync_state/state_spec.rb | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/ee/spec/lib/security/security_orchestration_policies/policy_sync_state/state_spec.rb b/ee/spec/lib/security/security_orchestration_policies/policy_sync_state/state_spec.rb index bf88bc81fcdb17..e23c7c0af94f64 100644 --- a/ee/spec/lib/security/security_orchestration_policies/policy_sync_state/state_spec.rb +++ b/ee/spec/lib/security/security_orchestration_policies/policy_sync_state/state_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Security::SecurityOrchestrationPolicies::PolicySyncState::State, feature_category: :security_policy_management do +RSpec.describe Security::SecurityOrchestrationPolicies::PolicySyncState::State, :clean_gitlab_redis_shared_state, feature_category: :security_policy_management do let(:merge_request_id) { 1 } let(:project_id) { 1 } let(:other_project_id) { 2 } @@ -12,10 +12,6 @@ subject(:state) { described_class.new(policy_configuration.id) } - before do - Gitlab::Redis::SharedState.with(&:flushall) - end - describe '.from_application_context' do before do allow(Gitlab::ApplicationContext).to receive(:current_context_attribute).and_return(context_value) -- GitLab From de6b078f9d3f70a693718107023381882ffabeed Mon Sep 17 00:00:00 2001 From: Dominic Bauer Date: Thu, 21 Aug 2025 12:45:50 +0200 Subject: [PATCH 11/16] Add `progress` suffix to percentage fields --- doc/api/graphql/reference/_index.md | 4 ++-- .../subscriptions/security/policies_sync_updated.rb | 4 ++-- .../security/policies_sync_updated.rb | 4 ++-- .../security/policies_sync_updated_spec.rb | 12 ++++++------ .../security/policies_sync_updated/helper.rb | 4 ++-- 5 files changed, 14 insertions(+), 14 deletions(-) diff --git a/doc/api/graphql/reference/_index.md b/doc/api/graphql/reference/_index.md index 6add9247f27cd4..1e4b242757cee1 100644 --- a/doc/api/graphql/reference/_index.md +++ b/doc/api/graphql/reference/_index.md @@ -37513,9 +37513,9 @@ Security policy state synchronization update. Returns `null` if the `security_po | Name | Type | Description | | ---- | ---- | ----------- | | `failedProjects` | [`[String!]`](#string) | IDs of failed projects. | -| `mergeRequests` | [`Float`](#float) | Percentage of merge requests synced. | +| `mergeRequestsProgress` | [`Float`](#float) | Percentage of merge requests synced. | | `mergeRequestsTotal` | [`Int`](#int) | Total number of merge requests synced. | -| `projects` | [`Float`](#float) | Percentage of projects synced. | +| `projectsProgress` | [`Float`](#float) | Percentage of projects synced. | | `projectsTotal` | [`Int`](#int) | Total number of projects synced. | ### `PolicyAnyMergeRequestViolation` diff --git a/ee/app/graphql/subscriptions/security/policies_sync_updated.rb b/ee/app/graphql/subscriptions/security/policies_sync_updated.rb index 39b0079144daa9..f0b59303259efc 100644 --- a/ee/app/graphql/subscriptions/security/policies_sync_updated.rb +++ b/ee/app/graphql/subscriptions/security/policies_sync_updated.rb @@ -24,10 +24,10 @@ def authorized?(policy_configuration_id:) def update(_) { - projects: object[:projects], + projects_progress: object[:projects], projects_total: object[:projects_total], failed_projects: object[:failed_projects], - merge_requests: object[:merge_requests], + merge_requests_progress: object[:merge_requests], merge_requests_total: object[:merge_requests_total] } end diff --git a/ee/app/graphql/types/gitlab_subscriptions/security/policies_sync_updated.rb b/ee/app/graphql/types/gitlab_subscriptions/security/policies_sync_updated.rb index 570846354a9421..e297921bedffd9 100644 --- a/ee/app/graphql/types/gitlab_subscriptions/security/policies_sync_updated.rb +++ b/ee/app/graphql/types/gitlab_subscriptions/security/policies_sync_updated.rb @@ -11,7 +11,7 @@ class PoliciesSyncUpdated < ::Types::BaseObject description 'Security policy state synchronization update. Returns `null` if the `security_policy_sync_propagation_tracking` feature flag is disabled.' # rubocop:enable Layout/LineLength - field :projects, GraphQL::Types::Float, + field :projects_progress, GraphQL::Types::Float, null: true, description: 'Percentage of projects synced.' @@ -23,7 +23,7 @@ class PoliciesSyncUpdated < ::Types::BaseObject null: true, description: 'IDs of failed projects.' - field :merge_requests, GraphQL::Types::Float, + field :merge_requests_progress, GraphQL::Types::Float, null: true, description: 'Percentage of merge requests synced.' diff --git a/ee/spec/graphql/subscriptions/security/policies_sync_updated_spec.rb b/ee/spec/graphql/subscriptions/security/policies_sync_updated_spec.rb index ab307fc38fbdcd..9492f71de95f33 100644 --- a/ee/spec/graphql/subscriptions/security/policies_sync_updated_spec.rb +++ b/ee/spec/graphql/subscriptions/security/policies_sync_updated_spec.rb @@ -10,10 +10,10 @@ let_it_be(:security_policy_project) { policy_configuration.security_policy_management_project } let_it_be(:current_user) { security_policy_project.creator } - let(:projects) { 75.5 } + let(:projects_progress) { 75.5 } let(:projects_total) { 100 } let(:failed_projects) { ["123"] } - let(:merge_requests) { 50 } + let(:merge_requests_progress) { 50 } let(:merge_requests_total) { 200 } let(:subscribe) { security_policies_sync_updated_subscription(policy_configuration, current_user) } @@ -29,10 +29,10 @@ subscription_response do GraphqlTriggers.security_policies_sync_updated( policy_configuration, - projects, + projects_progress, projects_total, failed_projects, - merge_requests, + merge_requests_progress, merge_requests_total) end end @@ -44,10 +44,10 @@ specify do expect(success_response).to eq({ - "projects" => projects, + "projectsProgress" => projects_progress, "projectsTotal" => projects_total, "failedProjects" => failed_projects, - "mergeRequests" => merge_requests, + "mergeRequestsProgress" => merge_requests_progress, "mergeRequestsTotal" => merge_requests_total }) end diff --git a/ee/spec/support/helpers/graphql/subscriptions/security/policies_sync_updated/helper.rb b/ee/spec/support/helpers/graphql/subscriptions/security/policies_sync_updated/helper.rb index 9dfc233fa7921f..87bd6464c6a3ce 100644 --- a/ee/spec/support/helpers/graphql/subscriptions/security/policies_sync_updated/helper.rb +++ b/ee/spec/support/helpers/graphql/subscriptions/security/policies_sync_updated/helper.rb @@ -26,10 +26,10 @@ def security_policies_sync_updated_subscription_query(policy_configuration) <<~SUBSCRIPTION subscription { securityPoliciesSyncUpdated(policyConfigurationId: \"#{policy_configuration.to_global_id}\") { - projects + projectsProgress projectsTotal failedProjects - mergeRequests + mergeRequestsProgress mergeRequestsTotal } } -- GitLab From 21e76fd0359b84f59ea65b47b143754f9493298f Mon Sep 17 00:00:00 2001 From: Dominic Bauer Date: Thu, 21 Aug 2025 12:54:06 +0200 Subject: [PATCH 12/16] Make `#trigger_subscription` use single Redis TX --- .../policy_sync_state.rb | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/ee/lib/security/security_orchestration_policies/policy_sync_state.rb b/ee/lib/security/security_orchestration_policies/policy_sync_state.rb index 3b7f0ec1a1c7a5..3c037e55ef38cf 100644 --- a/ee/lib/security/security_orchestration_policies/policy_sync_state.rb +++ b/ee/lib/security/security_orchestration_policies/policy_sync_state.rb @@ -218,16 +218,22 @@ def get_progress(pending, total) end def trigger_subscription - projects_pending = items_count(projects_sync_key) - projects_total = get_counter(total_projects_key) - merge_requests_pending = items_count(merge_requests_sync_key) - merge_requests_total = get_counter(total_merge_requests_key) + projects_pending, projects_total, all_failed_projects, merge_requests_pending, merge_requests_total = + with_redis do |redis| + [ + redis.scard(projects_sync_key), + redis.get(total_projects_key).to_i, + redis.smembers(failed_projects_sync_key), + redis.scard(merge_requests_sync_key), + redis.get(total_merge_requests_key).to_i + ] + end GraphqlTriggers.security_policies_sync_updated( policy_configuration, get_progress(projects_pending, projects_total), projects_total, - failed_projects, + all_failed_projects, get_progress(merge_requests_pending, merge_requests_total), merge_requests_total ) -- GitLab From caec8487e16e138b7f09668b3061793e29fe3618 Mon Sep 17 00:00:00 2001 From: Dominic Bauer Date: Thu, 21 Aug 2025 13:23:32 +0200 Subject: [PATCH 13/16] Fix subscription trigger --- ee/app/graphql/ee/graphql_triggers.rb | 12 +++++++++--- .../subscriptions/security/policies_sync_updated.rb | 4 ++-- ee/spec/graphql/graphql_triggers_spec.rb | 4 ++-- 3 files changed, 13 insertions(+), 7 deletions(-) diff --git a/ee/app/graphql/ee/graphql_triggers.rb b/ee/app/graphql/ee/graphql_triggers.rb index fa0a0e0adc434c..9269de6a64451d 100644 --- a/ee/app/graphql/ee/graphql_triggers.rb +++ b/ee/app/graphql/ee/graphql_triggers.rb @@ -70,12 +70,18 @@ def self.security_policy_project_created(container, status, security_policy_proj end def self.security_policies_sync_updated( - policy_configuration, projects, projects_total, failed_projects, merge_requests, merge_requests_total) + policy_configuration, + projects_progress, + projects_total, + failed_projects, + merge_requests_progress, + merge_requests_total + ) ::GitlabSchema.subscriptions.trigger( :security_policies_sync_updated, { policy_configuration_id: policy_configuration.to_global_id }, - { projects: projects, projects_total: projects_total, failed_projects: failed_projects, - merge_requests: merge_requests, merge_requests_total: merge_requests_total } + { projects_progress: projects_progress, projects_total: projects_total, failed_projects: failed_projects, + merge_requests_progress: merge_requests_progress, merge_requests_total: merge_requests_total } ) end end diff --git a/ee/app/graphql/subscriptions/security/policies_sync_updated.rb b/ee/app/graphql/subscriptions/security/policies_sync_updated.rb index f0b59303259efc..37ca81ef9318ae 100644 --- a/ee/app/graphql/subscriptions/security/policies_sync_updated.rb +++ b/ee/app/graphql/subscriptions/security/policies_sync_updated.rb @@ -24,10 +24,10 @@ def authorized?(policy_configuration_id:) def update(_) { - projects_progress: object[:projects], + projects_progress: object[:projects_progress], projects_total: object[:projects_total], failed_projects: object[:failed_projects], - merge_requests_progress: object[:merge_requests], + merge_requests_progress: object[:merge_requests_progress], merge_requests_total: object[:merge_requests_total] } end diff --git a/ee/spec/graphql/graphql_triggers_spec.rb b/ee/spec/graphql/graphql_triggers_spec.rb index fd6bf600c46edd..e6115a6fc92c10 100644 --- a/ee/spec/graphql/graphql_triggers_spec.rb +++ b/ee/spec/graphql/graphql_triggers_spec.rb @@ -213,10 +213,10 @@ :security_policies_sync_updated, { policy_configuration_id: policy_configuration.to_global_id }, { - projects: projects, + projects_progress: projects, projects_total: projects_total, failed_projects: failed_projects, - merge_requests: merge_requests, + merge_requests_progress: merge_requests, merge_requests_total: merge_requests_total } ).and_call_original -- GitLab From cd97b7eca236f4fa2bb83aa6a9e90b6ddac344dd Mon Sep 17 00:00:00 2001 From: Dominic Bauer Date: Thu, 21 Aug 2025 13:24:01 +0200 Subject: [PATCH 14/16] Remove unused `#items_count` --- .../security_orchestration_policies/policy_sync_state.rb | 6 ------ 1 file changed, 6 deletions(-) diff --git a/ee/lib/security/security_orchestration_policies/policy_sync_state.rb b/ee/lib/security/security_orchestration_policies/policy_sync_state.rb index 3c037e55ef38cf..2a2e18e2c360c5 100644 --- a/ee/lib/security/security_orchestration_policies/policy_sync_state.rb +++ b/ee/lib/security/security_orchestration_policies/policy_sync_state.rb @@ -267,12 +267,6 @@ def get_items(key) end end - def items_count(key) - with_redis do |redis| - redis.scard(key) - end - end - def with_redis(&block) Gitlab::Redis::SharedState.with(&block) # rubocop:disable CodeReuse/ActiveRecord -- false positive end -- GitLab From 022c19158b496758fb8879a011e19d210d693478 Mon Sep 17 00:00:00 2001 From: Dominic Bauer Date: Thu, 21 Aug 2025 13:24:31 +0200 Subject: [PATCH 15/16] Remove unused `#items_counter` --- .../security_orchestration_policies/policy_sync_state.rb | 6 ------ 1 file changed, 6 deletions(-) diff --git a/ee/lib/security/security_orchestration_policies/policy_sync_state.rb b/ee/lib/security/security_orchestration_policies/policy_sync_state.rb index 2a2e18e2c360c5..5ad5452ac72b1b 100644 --- a/ee/lib/security/security_orchestration_policies/policy_sync_state.rb +++ b/ee/lib/security/security_orchestration_policies/policy_sync_state.rb @@ -255,12 +255,6 @@ def policy_configuration end end - def get_counter(key) - with_redis do |redis| - redis.get(key).to_i - end - end - def get_items(key) with_redis do |redis| redis.smembers(key) -- GitLab From 772e82ad91acea1afe862ae6b5570e982cc21425 Mon Sep 17 00:00:00 2001 From: Dominic Bauer Date: Fri, 22 Aug 2025 08:22:22 +0200 Subject: [PATCH 16/16] Add callback tests --- .../policy_sync_state/callbacks_spec.rb | 89 +++++++++++++++++++ .../policy_sync_state_shared_examples.rb | 13 +++ 2 files changed, 102 insertions(+) create mode 100644 ee/spec/lib/security/security_orchestration_policies/policy_sync_state/callbacks_spec.rb create mode 100644 spec/support/shared_examples/policies/policy_sync_state_shared_examples.rb diff --git a/ee/spec/lib/security/security_orchestration_policies/policy_sync_state/callbacks_spec.rb b/ee/spec/lib/security/security_orchestration_policies/policy_sync_state/callbacks_spec.rb new file mode 100644 index 00000000000000..ad2174e1ad283c --- /dev/null +++ b/ee/spec/lib/security/security_orchestration_policies/policy_sync_state/callbacks_spec.rb @@ -0,0 +1,89 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Security::SecurityOrchestrationPolicies::PolicySyncState::Callbacks, :clean_gitlab_redis_shared_state, feature_category: :security_policy_management do + include described_class + + include_context 'with policy sync state' + + let(:merge_request_id) { 1 } + let(:project_id) { 1 } + + describe '#clear_policy_sync_state' do + specify do + expect_next_instance_of(Security::SecurityOrchestrationPolicies::PolicySyncState::State, + policy_configuration_id) do |state| + expect(state).to receive(:clear) + end + + clear_policy_sync_state(policy_configuration_id) + end + end + + describe '#append_projects_to_sync' do + specify do + expect_next_instance_of(Security::SecurityOrchestrationPolicies::PolicySyncState::State, + policy_configuration_id) do |state| + expect(state).to receive(:append_projects).with([project_id]) + end + + append_projects_to_sync(policy_configuration_id, [project_id]) + end + end + + describe '#finish_project_policy_sync' do + specify do + expect_next_instance_of(Security::SecurityOrchestrationPolicies::PolicySyncState::State, + policy_configuration_id) do |state| + expect(state).to receive(:finish_project).with(project_id) + end + + finish_project_policy_sync(project_id) + end + end + + describe '#fail_project_policy_sync' do + specify do + expect_next_instance_of(Security::SecurityOrchestrationPolicies::PolicySyncState::State, + policy_configuration_id) do |state| + expect(state).to receive(:fail_project).with(project_id) + end + + fail_project_policy_sync(project_id) + end + end + + describe '#start_merge_request_policy_sync' do + specify do + expect_next_instance_of(Security::SecurityOrchestrationPolicies::PolicySyncState::State, + policy_configuration_id) do |state| + expect(state).to receive(:start_merge_request).with(merge_request_id) + end + + start_merge_request_policy_sync(merge_request_id) + end + end + + describe '#start_merge_request_worker_policy_sync' do + specify do + expect_next_instance_of(Security::SecurityOrchestrationPolicies::PolicySyncState::State, + policy_configuration_id) do |state| + expect(state).to receive(:start_merge_request_worker).with(merge_request_id) + end + + start_merge_request_worker_policy_sync(merge_request_id) + end + end + + describe '#finish_merge_request_worker_policy_sync' do + specify do + expect_next_instance_of(Security::SecurityOrchestrationPolicies::PolicySyncState::State, + policy_configuration_id) do |state| + expect(state).to receive(:finish_merge_request_worker).with(merge_request_id) + end + + finish_merge_request_worker_policy_sync(merge_request_id) + end + end +end diff --git a/spec/support/shared_examples/policies/policy_sync_state_shared_examples.rb b/spec/support/shared_examples/policies/policy_sync_state_shared_examples.rb new file mode 100644 index 00000000000000..115eca76faaceb --- /dev/null +++ b/spec/support/shared_examples/policies/policy_sync_state_shared_examples.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +RSpec.shared_context 'with policy sync state', :clean_gitlab_redis_shared_state do + let_it_be(:policy_configuration) { create(:security_orchestration_policy_configuration, :namespace) } + + let(:policy_configuration_id) { policy_configuration.id } + let(:state) { Security::SecurityOrchestrationPolicies::PolicySyncState::State.new(policy_configuration_id) } + + before do + allow(Gitlab::ApplicationContext).to receive(:current_context_attribute) + .and_return(policy_configuration_id.to_s) + end +end -- GitLab