diff --git a/app/models/ci/job_variable.rb b/app/models/ci/job_variable.rb index 5ec3b44b7680f92c08c635133a7f6f9ba569ed6e..407e40e2e6c6905ef955ccd91864b7561c90530c 100644 --- a/app/models/ci/job_variable.rb +++ b/app/models/ci/job_variable.rb @@ -5,6 +5,7 @@ class JobVariable < Ci::ApplicationRecord include Ci::Partitionable include Ci::NewHasVariable include Ci::RawVariable + include Ci::ProjectsWithVariablesQuery before_validation :set_project_id, on: :create diff --git a/app/models/ci/pipeline_variable.rb b/app/models/ci/pipeline_variable.rb index 4988c338167d048ad62d99ab79a1dc321a328229..ee9b7afcb0f00025421745280e46ff59b3c94e84 100644 --- a/app/models/ci/pipeline_variable.rb +++ b/app/models/ci/pipeline_variable.rb @@ -5,6 +5,7 @@ class PipelineVariable < Ci::ApplicationRecord include Ci::Partitionable include Ci::HasVariable include Ci::RawVariable + include Ci::ProjectsWithVariablesQuery before_validation :ensure_project_id diff --git a/app/models/concerns/ci/projects_with_variables_query.rb b/app/models/concerns/ci/projects_with_variables_query.rb new file mode 100644 index 0000000000000000000000000000000000000000..ec33d6f0bc8bde209796bc891b5951cdb4c2304f --- /dev/null +++ b/app/models/concerns/ci/projects_with_variables_query.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +module Ci + module ProjectsWithVariablesQuery + extend ActiveSupport::Concern + + class_methods do + def projects_with_variables(project_ids, limit) + project_ids_sql = <<~SQL.squish + WITH input_projects AS ( + SELECT unnest(ARRAY[?]) AS project_id + ) + SELECT input_projects.project_id + FROM input_projects + WHERE EXISTS ( + SELECT 1 + FROM #{quoted_table_name} + WHERE #{quoted_table_name}.project_id = input_projects.project_id + ) + LIMIT ? + SQL + + connection.select_values(sanitize_sql_array([project_ids_sql, project_ids, limit])) + end + end + end +end diff --git a/app/models/project_ci_cd_setting.rb b/app/models/project_ci_cd_setting.rb index e369865b13f9c8c76ab0b1b3b03d11faac356a7e..21c40dbcf52ce1e00733bde711c19ca3242e2feb 100644 --- a/app/models/project_ci_cd_setting.rb +++ b/app/models/project_ci_cd_setting.rb @@ -56,16 +56,34 @@ class ProjectCiCdSetting < ApplicationRecord chronic_duration_attr :runner_token_expiration_interval_human_readable, :runner_token_expiration_interval chronic_duration_attr_writer :delete_pipelines_in_human_readable, :delete_pipelines_in_seconds + scope :for_namespace, ->(namespace) { joins(:project).where({ projects: { namespace: namespace } }) } scope :for_project, ->(ids) { where(project_id: ids) } scope :order_project_id_asc, -> { order(project_id: :asc) } scope :configured_to_delete_old_pipelines, -> do where.not(delete_pipelines_in_seconds: nil) end + scope :with_pipeline_variables_enabled, -> do + where.not(pipeline_variables_minimum_override_role: NO_ONE_ALLOWED_ROLE) + end def self.pluck_project_id(limit) limit(limit).pluck(:project_id) end + def self.bulk_restrict_pipeline_variables!(project_ids:) + for_project(project_ids).update_all(pipeline_variables_minimum_override_role: NO_ONE_ALLOWED_ROLE) + end + + def self.project_ids_not_using_variables(settings_relation, limit) + project_ids = settings_relation.limit(limit).pluck(:project_id) + + projects_with_vars = + Ci::PipelineVariable.projects_with_variables(project_ids, limit) + + Ci::JobVariable.projects_with_variables(project_ids, limit) + + project_ids - projects_with_vars + end + def keep_latest_artifacts_available? # The project level feature can only be enabled when the feature is enabled instance wide Gitlab::CurrentSettings.current_application_settings.keep_latest_artifact? && keep_latest_artifact? diff --git a/app/services/ci/safe_disable_pipeline_variables_service.rb b/app/services/ci/safe_disable_pipeline_variables_service.rb new file mode 100644 index 0000000000000000000000000000000000000000..a43317d311920838f7f60ceff90e70dfb609728e --- /dev/null +++ b/app/services/ci/safe_disable_pipeline_variables_service.rb @@ -0,0 +1,43 @@ +# frozen_string_literal: true + +module Ci + class SafeDisablePipelineVariablesService < BaseService + PROJECTS_BATCH_SIZE = 500 + + def initialize(current_user:, group:) + @current_user = current_user + @parent_group = group + end + + def execute + return ServiceResponse.error(message: 'You are not authorized to perform this action') unless authorized? + + updated_count = 0 + + parent_group.self_and_descendants.each_batch do |group_batch| + all_projects_ci_cd_settings = ProjectCiCdSetting.for_namespace(group_batch) + .with_pipeline_variables_enabled + + all_projects_ci_cd_settings + .each_batch(of: PROJECTS_BATCH_SIZE) do |ci_cd_settings| + batch_updated_count = + ProjectCiCdSetting.bulk_restrict_pipeline_variables!( + project_ids: ProjectCiCdSetting.project_ids_not_using_variables(ci_cd_settings, PROJECTS_BATCH_SIZE) + ) + + updated_count += batch_updated_count + end + end + + ServiceResponse.success(payload: { updated_count: updated_count }) + end + + private + + attr_reader :current_user, :parent_group + + def authorized? + current_user.can?(:admin_group, parent_group) + end + end +end diff --git a/app/workers/all_queues.yml b/app/workers/all_queues.yml index e125b63f2a80d24f831885404d859ed243fc5075..325e1113a658da0e21dbafda39ee3be8e334efa8 100644 --- a/app/workers/all_queues.yml +++ b/app/workers/all_queues.yml @@ -3487,6 +3487,16 @@ :idempotent: true :tags: [] :queue_namespace: +- :name: ci_safe_disable_pipeline_variables + :worker_name: Ci::SafeDisablePipelineVariablesWorker + :feature_category: :ci_variables + :has_external_dependencies: false + :urgency: :low + :resource_boundary: :unknown + :weight: 1 + :idempotent: true + :tags: [] + :queue_namespace: - :name: ci_unlock_pipelines_in_queue :worker_name: Ci::UnlockPipelinesInQueueWorker :feature_category: :job_artifacts diff --git a/app/workers/ci/safe_disable_pipeline_variables_worker.rb b/app/workers/ci/safe_disable_pipeline_variables_worker.rb new file mode 100644 index 0000000000000000000000000000000000000000..8b80c88fb311b12f77aafb144b914b45639f41b6 --- /dev/null +++ b/app/workers/ci/safe_disable_pipeline_variables_worker.rb @@ -0,0 +1,33 @@ +# frozen_string_literal: true + +module Ci + class SafeDisablePipelineVariablesWorker + include ApplicationWorker + + data_consistency :sticky + + feature_category :ci_variables + urgency :low + idempotent! + + defer_on_database_health_signal :gitlab_main, [:namespaces, :project_ci_cd_settings], 1.minute + + attr_accessor :group_id, :current_user_id + + def perform(current_user_id, group_id) + current_user = UserFinder.new(current_user_id).find_by_id + return unless current_user + + group = ::Group.find_by_id(group_id) + return unless group + + result = Ci::SafeDisablePipelineVariablesService.new( + current_user: current_user, group: group + ).execute + + return unless result.success? + + log_extra_metadata_on_done(:disabled_pipeline_variables_count, result.payload[:updated_count]) + end + end +end diff --git a/config/sidekiq_queues.yml b/config/sidekiq_queues.yml index c97424e2868d714f3a12945519143b243b827745..aa86ff815e2f2efe299850991b87e34d4fe636a5 100644 --- a/config/sidekiq_queues.yml +++ b/config/sidekiq_queues.yml @@ -215,6 +215,8 @@ - 1 - - ci_runners_update_project_runners_owner - 1 +- - ci_safe_disable_pipeline_variables + - 1 - - ci_unlock_pipelines_in_queue - 1 - - ci_update_approval_rules_for_related_mrs diff --git a/spec/models/ci/job_variable_spec.rb b/spec/models/ci/job_variable_spec.rb index c26dfefa4c188bdce6cc069f16ad09ca4fe6768d..6224cff394020fce55116b513c211a439aa3bf81 100644 --- a/spec/models/ci/job_variable_spec.rb +++ b/spec/models/ci/job_variable_spec.rb @@ -93,4 +93,12 @@ let!(:model) { create(:ci_job_variable, project_id: parent.id) } end end + + describe 'projects_with_pipeline_variables_query concern' do + def create_variable(project) + create(:ci_job_variable, job: create(:ci_build, project: project)) + end + + it_behaves_like 'projects_with_variables_query' + end end diff --git a/spec/models/ci/pipeline_variable_spec.rb b/spec/models/ci/pipeline_variable_spec.rb index 5e6f50cda3a5f248b573005ae85446b7fd7e5a4d..1a90885c875c9a5d7d3d56fc204bf6eeba662168 100644 --- a/spec/models/ci/pipeline_variable_spec.rb +++ b/spec/models/ci/pipeline_variable_spec.rb @@ -63,4 +63,12 @@ end.not_to change { variable.project_id }.from(another_project.id) end end + + describe 'projects_with_pipeline_variables_query concern' do + def create_variable(project) + create(:ci_pipeline_variable, pipeline: create(:ci_pipeline, project: project)) + end + + it_behaves_like 'projects_with_variables_query' + end end diff --git a/spec/services/ci/safe_disable_pipeline_variables_service_spec.rb b/spec/services/ci/safe_disable_pipeline_variables_service_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..b7ffa214b68356d94f7590d57b9eef2447e45a41 --- /dev/null +++ b/spec/services/ci/safe_disable_pipeline_variables_service_spec.rb @@ -0,0 +1,87 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Ci::SafeDisablePipelineVariablesService, feature_category: :ci_variables do + let(:log_output) { StringIO.new } + let(:service) { described_class.new(current_user: current_user, group: parent_group) } + + describe '#execute' do + subject(:update_role) { service.execute } + + context 'when group_id correctly supplied' do + let_it_be(:parent_group) { create(:group) } + let_it_be(:sub_group1) { create(:group, parent: parent_group) } + let_it_be(:project_in_group1) { create(:project, group: parent_group) } + let_it_be(:project_in_sub_group1) { create(:project, group: sub_group1) } + let_it_be(:expected_updated_projects) { [project_in_group1, project_in_sub_group1] } + + let_it_be(:project_with_variables_in_group1) { create(:project, group: parent_group) } + let_it_be(:project_with_job_variables_in_group1) { create(:project, group: parent_group) } + let_it_be(:expected_not_updated_projects) do + [project_with_variables_in_group1, project_with_job_variables_in_group1] + end + + let_it_be(:all_projects) { expected_updated_projects + expected_not_updated_projects } + + let_it_be(:current_user) { create(:user) } + let_it_be_with_reload(:pipeline1) { create(:ci_pipeline, project: project_with_variables_in_group1) } + let_it_be_with_reload(:pipeline3) { create(:ci_pipeline, project: project_with_job_variables_in_group1) } + let(:build) { create(:ci_build, pipeline: pipeline3) } + + before do + create(:ci_pipeline_variable, pipeline: pipeline1, key: :TRIGGER_KEY_1, value: 'TRIGGER_VALUE_1') + + create(:ci_job_variable, job: build) + end + + before_all do + ProjectCiCdSetting.update_all(pipeline_variables_minimum_override_role: :developer) + end + + context 'and current user has permissions to update a group' do + before_all do + parent_group.add_owner(current_user) + end + + it 'updates ci cd settings with minimum override role to no one allowed' do + response = update_role + + expected_updated_settings = ProjectCiCdSetting.where(project: expected_updated_projects) + expected_not_updated_settings = ProjectCiCdSetting.where(project: expected_not_updated_projects) + + expect(expected_updated_settings.pluck(:pipeline_variables_minimum_override_role)) + .to eq(%w[no_one_allowed no_one_allowed]) + + expect(expected_not_updated_settings.pluck(:pipeline_variables_minimum_override_role)) + .to eq(%w[developer developer]) + + expect(response.status).to eq(:success) + expect(response.payload[:updated_count]).to eq(2) + end + + it 'is idempotent' do + response = service.execute + expect(response.status).to eq(:success) + + second_response = service.execute + expect(second_response.status).to eq(:success) + expect(second_response.payload[:updated_count]).to eq(0) + end + end + + context 'and current user has no permissions to update a group' do + it 'does not updates ci cd settings with minimum override role to no one allowed' do + response = update_role + + expect(response.status).to eq(:error) + expect(response.message).to eq('You are not authorized to perform this action') + expected_updated_settings = ProjectCiCdSetting.where(project: expected_updated_projects) + + expect(expected_updated_settings.pluck(:pipeline_variables_minimum_override_role)) + .to eq(%w[developer developer]) + end + end + end + end +end diff --git a/spec/support/shared_examples/projects_with_variables_query_shared_example.rb b/spec/support/shared_examples/projects_with_variables_query_shared_example.rb new file mode 100644 index 0000000000000000000000000000000000000000..f06dacbbc08a7c8e6b43c48b6ee80128dc921594 --- /dev/null +++ b/spec/support/shared_examples/projects_with_variables_query_shared_example.rb @@ -0,0 +1,64 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.shared_examples 'projects_with_variables_query' do + describe '.projects_with_variables' do + let_it_be(:project1) { create(:project) } + let_it_be(:project2) { create(:project) } + let_it_be(:project3) { create(:project) } + let_it_be(:project4) { create(:project) } + + let(:project_ids) { [project1.id, project2.id, project3.id, project4.id] } + + before do + # Create variables for some projects + create_variable(project1) + create_variable(project1) # Multiple variables for same project + create_variable(project3) + # project2 and project4 have no variables + end + + context 'when projects have variables' do + it 'returns project IDs that have variables' do + result = described_class.projects_with_variables(project_ids, 10) + + expect(result).to match_array([project1.id, project3.id]) + end + end + + context 'when no projects have variables' do + let(:empty_project_ids) { [project2.id, project4.id] } + + it 'returns empty array' do + result = described_class.projects_with_variables(empty_project_ids, 10) + + expect(result).to be_empty + end + end + + context 'when project_ids contains non-existent project IDs' do + let(:mixed_project_ids) { [project1.id, project4.id + 1, project3.id, project4.id + 2] } + + it 'only returns existing project IDs that have variables' do + result = described_class.projects_with_variables(mixed_project_ids, 10) + + expect(result).to match_array([project1.id, project3.id]) + end + end + + context 'with limit parameter' do + it 'respects the limit when more projects than limit have variables' do + result = described_class.projects_with_variables(project_ids, 1) + + expect(result.size).to eq(1) + end + + it 'returns all matching projects when limit is higher than matches' do + result = described_class.projects_with_variables(project_ids, 10) + + expect(result).to match_array([project1.id, project3.id]) + end + end + end +end diff --git a/spec/workers/ci/safe_disable_pipeline_variables_worker_spec.rb b/spec/workers/ci/safe_disable_pipeline_variables_worker_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..c1a03415ef42a83f3715aea1ff7d69ce7fa8c3c7 --- /dev/null +++ b/spec/workers/ci/safe_disable_pipeline_variables_worker_spec.rb @@ -0,0 +1,72 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Ci::SafeDisablePipelineVariablesWorker, feature_category: :ci_variables do + let_it_be(:group_id) { create(:group).id } + let_it_be(:current_user_id) { create(:user).id } + let(:worker) { described_class.new } + + describe '#perform' do + subject(:perform_worker) { worker.perform(*job_args) } + + let(:job_args) { [current_user_id, group_id] } + + it_behaves_like 'an idempotent worker' + + shared_examples 'fails to execute the service' do + it 'fails to execute' do + expect(worker).not_to receive(:log_extra_metadata_on_done) + expect(::Ci::SafeDisablePipelineVariablesService).not_to receive(:new) + + perform_worker + end + end + + it 'executes SafeDisablePipelineVariablesService' do + expected_response = ServiceResponse.success(payload: { updated_count: 5 }) + + expect_next_instance_of(::Ci::SafeDisablePipelineVariablesService) do |service| + expect(service).to receive(:execute).and_return(expected_response) + end + + expect(worker).to receive(:log_extra_metadata_on_done).with(:disabled_pipeline_variables_count, 5) + + perform_worker + end + + context 'when service response is an error' do + before do + expected_response = ServiceResponse.error(message: 'error') + + expect_next_instance_of(::Ci::SafeDisablePipelineVariablesService) do |service| + allow(service).to receive(:execute).and_return(expected_response) + end + end + + it_behaves_like 'fails to execute the service' + end + + context 'when group or user is not found' do + before do + expected_response = ServiceResponse.success(payload: { updated_count: 5 }) + + allow_next_instance_of(::Ci::SafeDisablePipelineVariablesService) do |service| + allow(service).to receive(:execute).and_return(expected_response) + end + end + + context 'and group does not exist' do + let(:job_args) { [current_user_id, group_id + 1] } + + it_behaves_like 'fails to execute the service' + end + + context 'and user does not exist' do + let(:job_args) { [current_user_id + 1, group_id] } + + it_behaves_like 'fails to execute the service' + end + end + end +end