diff --git a/config/gitlab_loose_foreign_keys.yml b/config/gitlab_loose_foreign_keys.yml index 93fb42f8505f13e89798f61045ba38acd4079410..471475461b223f6acddb3f0a36321c8eb8e19513 100644 --- a/config/gitlab_loose_foreign_keys.yml +++ b/config/gitlab_loose_foreign_keys.yml @@ -415,6 +415,10 @@ geo_node_namespace_links: - table: namespaces column: namespace_id on_delete: async_delete +geo_node_organization_links: + - table: organizations + column: organization_id + on_delete: async_delete group_push_rules: - table: namespaces column: group_id @@ -467,7 +471,7 @@ merge_request_diff_commits: - table: merge_request_diffs column: merge_request_diff_id on_delete: async_delete - worker_class: 'LooseForeignKeys::MergeRequestDiffCommitCleanupWorker' + worker_class: LooseForeignKeys::MergeRequestDiffCommitCleanupWorker merge_request_metrics: - table: p_ci_pipelines column: pipeline_id diff --git a/db/docs/geo_node_organization_links.yml b/db/docs/geo_node_organization_links.yml new file mode 100644 index 0000000000000000000000000000000000000000..d0ae91e208af7b71ed0acfb12ceeae214d24606c --- /dev/null +++ b/db/docs/geo_node_organization_links.yml @@ -0,0 +1,11 @@ +--- +table_name: geo_node_organization_links +classes: +- Geo::GeoNodeOrganizationLink +feature_categories: +- geo_replication +description: Passthrough table for geo_nodes many-to-many organizations relation. +introduced_by_url: +milestone: '18.4' +gitlab_schema: gitlab_main_cell_local +table_size: small diff --git a/db/migrate/20250909140653_create_geo_node_organization_links.rb b/db/migrate/20250909140653_create_geo_node_organization_links.rb new file mode 100644 index 0000000000000000000000000000000000000000..930912a5a30864ca87aa9aa9a2c6b351279e9425 --- /dev/null +++ b/db/migrate/20250909140653_create_geo_node_organization_links.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +class CreateGeoNodeOrganizationLinks < Gitlab::Database::Migration[2.3] + # Because index_geo_node_organization_links_on_geo_node_id_and_organization_id is over the 63 char limit + INDEX_NAME = :index_geo_node_organization_links_on_geo_node_id_and_org_id + + milestone '18.4' + + def change + create_table :geo_node_organization_links do |t| + t.references :geo_node, null: false, foreign_key: { on_delete: :cascade }, index: false + t.references :organization, null: false + t.timestamps_with_timezone null: false + t.index [:geo_node_id, :organization_id], unique: true, name: INDEX_NAME + end + end +end diff --git a/db/schema_migrations/20250909140653 b/db/schema_migrations/20250909140653 new file mode 100644 index 0000000000000000000000000000000000000000..ec0215fa182d455e050c4d9ad810e2cbc74daff2 --- /dev/null +++ b/db/schema_migrations/20250909140653 @@ -0,0 +1 @@ +f05ee8a621716852e7a3cf203439b13eff1c830156abe8a9456a015e394c0f31 \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index e8c24387f37041009631ad1fd641cb739668c296..5a98fcfec225b545cbbdaed40eaf0081f39b8418 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -16319,6 +16319,23 @@ CREATE SEQUENCE geo_node_namespace_links_id_seq ALTER SEQUENCE geo_node_namespace_links_id_seq OWNED BY geo_node_namespace_links.id; +CREATE TABLE geo_node_organization_links ( + id bigint NOT NULL, + geo_node_id bigint NOT NULL, + organization_id bigint NOT NULL, + created_at timestamp with time zone NOT NULL, + updated_at timestamp with time zone NOT NULL +); + +CREATE SEQUENCE geo_node_organization_links_id_seq + START WITH 1 + INCREMENT BY 1 + NO MINVALUE + NO MAXVALUE + CACHE 1; + +ALTER SEQUENCE geo_node_organization_links_id_seq OWNED BY geo_node_organization_links.id; + CREATE TABLE geo_node_statuses ( id bigint NOT NULL, geo_node_id bigint NOT NULL, @@ -29801,6 +29818,8 @@ ALTER TABLE ONLY geo_events ALTER COLUMN id SET DEFAULT nextval('geo_events_id_s ALTER TABLE ONLY geo_node_namespace_links ALTER COLUMN id SET DEFAULT nextval('geo_node_namespace_links_id_seq'::regclass); +ALTER TABLE ONLY geo_node_organization_links ALTER COLUMN id SET DEFAULT nextval('geo_node_organization_links_id_seq'::regclass); + ALTER TABLE ONLY geo_node_statuses ALTER COLUMN id SET DEFAULT nextval('geo_node_statuses_id_seq'::regclass); ALTER TABLE ONLY geo_nodes ALTER COLUMN id SET DEFAULT nextval('geo_nodes_id_seq'::regclass); @@ -32600,6 +32619,9 @@ ALTER TABLE ONLY geo_events ALTER TABLE ONLY geo_node_namespace_links ADD CONSTRAINT geo_node_namespace_links_pkey PRIMARY KEY (id); +ALTER TABLE ONLY geo_node_organization_links + ADD CONSTRAINT geo_node_organization_links_pkey PRIMARY KEY (id); + ALTER TABLE ONLY geo_node_statuses ADD CONSTRAINT geo_node_statuses_pkey PRIMARY KEY (id); @@ -38695,6 +38717,10 @@ CREATE UNIQUE INDEX index_geo_node_namespace_links_on_geo_node_id_and_namespace_ CREATE INDEX index_geo_node_namespace_links_on_namespace_id ON geo_node_namespace_links USING btree (namespace_id); +CREATE UNIQUE INDEX index_geo_node_organization_links_on_geo_node_id_and_org_id ON geo_node_organization_links USING btree (geo_node_id, organization_id); + +CREATE INDEX index_geo_node_organization_links_on_organization_id ON geo_node_organization_links USING btree (organization_id); + CREATE UNIQUE INDEX index_geo_node_statuses_on_geo_node_id ON geo_node_statuses USING btree (geo_node_id); CREATE INDEX index_geo_nodes_on_access_key ON geo_nodes USING btree (access_key); @@ -50245,6 +50271,9 @@ ALTER TABLE ONLY gpg_signatures ALTER TABLE ONLY virtual_registries_container_upstreams ADD CONSTRAINT fk_rails_c97afd8bbd FOREIGN KEY (group_id) REFERENCES namespaces(id) ON DELETE CASCADE; +ALTER TABLE ONLY geo_node_organization_links + ADD CONSTRAINT fk_rails_c9bfa510d9 FOREIGN KEY (geo_node_id) REFERENCES geo_nodes(id) ON DELETE CASCADE; + ALTER TABLE ONLY board_group_recent_visits ADD CONSTRAINT fk_rails_ca04c38720 FOREIGN KEY (board_id) REFERENCES boards(id) ON DELETE CASCADE; diff --git a/ee/app/controllers/admin/geo/nodes_controller.rb b/ee/app/controllers/admin/geo/nodes_controller.rb index b7176c09608ed20b59c23e8ed2c56f078b828f50..20bf4e5be18187642461532d52c3f462ba4966c0 100644 --- a/ee/app/controllers/admin/geo/nodes_controller.rb +++ b/ee/app/controllers/admin/geo/nodes_controller.rb @@ -45,6 +45,12 @@ def update private def geo_node_params + permitted_hash_params = { + selective_sync_shards: [] + } + + permitted_hash_params[:organization_ids] = [] if ::Gitlab::Geo.geo_selective_sync_by_organizations_enabled? + params.require(:geo_node).permit( :name, :url, @@ -58,7 +64,7 @@ def geo_node_params :minimum_reverification_interval, :container_repositories_max_capacity, :sync_object_storage, - selective_sync_shards: [] + **permitted_hash_params ) end diff --git a/ee/app/helpers/ee/geo_helper.rb b/ee/app/helpers/ee/geo_helper.rb index b383d69496adc5db716d6a6fe54cb12c2f0c3564..2f01fd88c9e58c964bd1c9b79c3489b062dfce13 100644 --- a/ee/app/helpers/ee/geo_helper.rb +++ b/ee/app/helpers/ee/geo_helper.rb @@ -40,6 +40,13 @@ def selective_sync_types_json } } + if ::Gitlab::Geo.geo_selective_sync_by_organizations_enabled? + options[:ORGANIZATIONS] = { + label: s_('Geo|Projects in certain organizations'), + value: 'organizations' + } + end + options.to_json end diff --git a/ee/app/models/concerns/geo/selective_sync.rb b/ee/app/models/concerns/geo/selective_sync.rb index bc6706c16639290476331f801c72ed469bf0cf56..f3634e557b2333b9522257f76c1f6819596ce1ef 100644 --- a/ee/app/models/concerns/geo/selective_sync.rb +++ b/ee/app/models/concerns/geo/selective_sync.rb @@ -4,12 +4,19 @@ module Geo module SelectiveSync extend ActiveSupport::Concern - SELECTIVE_SYNC_TYPES = %w[namespaces shards].freeze + SELECTIVE_SYNC_TYPES = %w[namespaces shards organizations].freeze def selective_sync? validate_selective_sync_type! - selective_sync_type.present? + types = SELECTIVE_SYNC_TYPES + + # If someone enables the FF, tries selective sync by org, and finally disables the FF: + # 1. We won't raise unknown selective sync type error + # 2. But we will assume that they want selective sync to be disabled (sync everything). + types -= 'organizations' unless ::Gitlab::Geo.geo_selective_sync_by_organizations_enabled? + + types.include?(selective_sync_type) end def selective_sync_by_namespaces? @@ -24,6 +31,13 @@ def selective_sync_by_shards? selective_sync_type == 'shards' end + def selective_sync_by_organizations? + validate_selective_sync_type! + return false unless ::Gitlab::Geo.geo_selective_sync_by_organizations_enabled? + + selective_sync_type == 'organizations' + end + def validate_selective_sync_type! return if selective_sync_type.blank? return if SELECTIVE_SYNC_TYPES.include?(selective_sync_type) @@ -53,6 +67,10 @@ def namespaces_for_group_owned_replicables selected_namespaces_and_descendants elsif selective_sync_by_shards? selected_leaf_namespaces_and_ancestors + elsif selective_sync_by_organizations? + # TODO: https://gitlab.com/gitlab-org/gitlab/-/issues/534201 or other sibling issues that need this + # This should return all namespaces owned by the selected orgs. + Namespace.none else raise 'This scope should not be needed without selective sync' end diff --git a/ee/app/models/ee/upload.rb b/ee/app/models/ee/upload.rb index 96f935b41216cbe8d5d409c1dc6d8eb06864c636..f0c4553f817f84583b45871a565ccc03bc943357 100644 --- a/ee/app/models/ee/upload.rb +++ b/ee/app/models/ee/upload.rb @@ -64,6 +64,10 @@ def search(query) override :selective_sync_scope def selective_sync_scope(node, **_params) if node.selective_sync? + # TODO: Implement selective sync by organizations + # https://gitlab.com/gitlab-org/gitlab/-/issues/534193 + return none if node.selective_sync_by_organizations? + group_attachments(node).or(project_attachments(node)).or(other_attachments) else all diff --git a/ee/app/models/geo/geo_node_organization_link.rb b/ee/app/models/geo/geo_node_organization_link.rb new file mode 100644 index 0000000000000000000000000000000000000000..890d12c199c3740471290c7747c0a27d1f8bee57 --- /dev/null +++ b/ee/app/models/geo/geo_node_organization_link.rb @@ -0,0 +1,10 @@ +# frozen_string_literal: true + +module Geo + class GeoNodeOrganizationLink < ApplicationRecord + belongs_to :geo_node, inverse_of: :organizations, optional: false + belongs_to :organization, class_name: 'Organizations::Organization', optional: false + + validates :organization_id, uniqueness: { scope: [:geo_node_id] } + end +end diff --git a/ee/app/models/geo_node.rb b/ee/app/models/geo_node.rb index 48ea4257235a1232b5d12d190c35f82893670f78..94468eab75eaafce77c72a8f3a89607dfe41d860 100644 --- a/ee/app/models/geo_node.rb +++ b/ee/app/models/geo_node.rb @@ -15,6 +15,8 @@ class GeoNode < ApplicationRecord has_many :geo_node_namespace_links has_many :namespaces, through: :geo_node_namespace_links + has_many :geo_node_organization_links, class_name: 'Geo::GeoNodeOrganizationLink' + has_many :organizations, through: :geo_node_organization_links, class_name: 'Organizations::Organization' has_one :status, class_name: 'GeoNodeStatus' validates :name, presence: true, uniqueness: { case_sensitive: false }, length: { maximum: 255 } diff --git a/ee/app/services/geo/node_update_service.rb b/ee/app/services/geo/node_update_service.rb index d2ad1f81768cd28ab77f35932c4d1a647ecebffa..612c034dbaa7afc2d4944e2716cff24551ee2488 100644 --- a/ee/app/services/geo/node_update_service.rb +++ b/ee/app/services/geo/node_update_service.rb @@ -8,6 +8,7 @@ def initialize(geo_node, params) @geo_node = geo_node @params = params.dup @params[:namespace_ids] = @params[:namespace_ids].to_s.split(',') if @params[:namespace_ids].is_a? String + @params[:organization_ids] = @params[:organization_ids].to_s.split(',') if @params[:organization_ids].is_a? String end def execute diff --git a/ee/config/feature_flags/wip/geo_selective_sync_by_organizations.yml b/ee/config/feature_flags/wip/geo_selective_sync_by_organizations.yml new file mode 100644 index 0000000000000000000000000000000000000000..3b60c97ba525f6d77ca3f893afde9d4dbbf35175 --- /dev/null +++ b/ee/config/feature_flags/wip/geo_selective_sync_by_organizations.yml @@ -0,0 +1,10 @@ +--- +name: geo_selective_sync_by_organizations +description: +feature_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/514251 +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/201213 +rollout_issue_url: +milestone: '18.4' +group: group::geo +type: wip +default_enabled: false diff --git a/ee/lib/api/geo_nodes.rb b/ee/lib/api/geo_nodes.rb index 5dda5673a5db8cf8a83838f2ff732aaeb55beb89..32b96524890f8ff3612fca7ac4c634b09ee230ec 100644 --- a/ee/lib/api/geo_nodes.rb +++ b/ee/lib/api/geo_nodes.rb @@ -70,6 +70,11 @@ def update_geo_nodes_endpoint? optional :selective_sync_namespace_ids, as: :namespace_ids, type: Array[Integer], coerce_with: Validations::Types::CommaSeparatedToIntegerArray.coerce, desc: 'The IDs of groups that should be synced, if `selective_sync_type` == `namespaces`' + optional :selective_sync_organization_ids, + as: :organization_ids, + type: Array[Integer], + coerce_with: Validations::Types::CommaSeparatedToIntegerArray.coerce, + desc: 'The IDs of organizations that should be synced, if `selective_sync_type` == `organizations`' optional :minimum_reverification_interval, type: Integer, desc: 'The interval (in days) in which the repository verification is valid. Once expired, it will be ' \ 'reverified. This has no effect when set on a secondary node.' @@ -77,6 +82,8 @@ def update_geo_nodes_endpoint? post do create_params = declared_params(include_missing: false) + create_params.delete(:organization_ids) unless ::Gitlab::Geo.geo_selective_sync_by_organizations_enabled? + new_geo_node = ::Geo::NodeCreateService.new(create_params).execute if new_geo_node.persisted? @@ -256,6 +263,11 @@ def geo_node_status optional :selective_sync_namespace_ids, as: :namespace_ids, type: Array[Integer], coerce_with: Validations::Types::CommaSeparatedToIntegerArray.coerce, desc: 'The IDs of groups that should be synced, if `selective_sync_type` == `namespaces`' + optional :selective_sync_organization_ids, + as: :organization_ids, + type: Array[Integer], + coerce_with: Validations::Types::CommaSeparatedToIntegerArray.coerce, + desc: 'The IDs of organizations that should be synced, if `selective_sync_type` == `organizations`' optional :minimum_reverification_interval, type: Integer, desc: 'The interval (in days) in which the repository verification is valid. Once expired, it will be ' \ 'reverified. This has no effect when set on a secondary node.' @@ -265,6 +277,8 @@ def geo_node_status update_params = declared_params(include_missing: false) + update_params.delete(:organization_ids) unless ::Gitlab::Geo.geo_selective_sync_by_organizations_enabled? + updated_geo_node = ::Geo::NodeUpdateService.new(geo_node, update_params).execute if updated_geo_node diff --git a/ee/lib/api/geo_sites.rb b/ee/lib/api/geo_sites.rb index 0b00fbf6e63a61fad997c02964d69695f124a201..eab7a885526db9a28fcb4cf56ba903f48014b060 100644 --- a/ee/lib/api/geo_sites.rb +++ b/ee/lib/api/geo_sites.rb @@ -70,6 +70,11 @@ def update_geo_sites_endpoint? optional :selective_sync_namespace_ids, as: :namespace_ids, type: Array[Integer], coerce_with: Validations::Types::CommaSeparatedToIntegerArray.coerce, desc: 'The IDs of groups that should be synced, if `selective_sync_type` == `namespaces`' + optional :selective_sync_organization_ids, + as: :organization_ids, + type: Array[Integer], + coerce_with: Validations::Types::CommaSeparatedToIntegerArray.coerce, + desc: 'The IDs of organizations that should be synced, if `selective_sync_type` == `organizations`' optional :minimum_reverification_interval, type: Integer, desc: 'The interval (in days) in which the repository verification is valid. Once expired, it ' \ 'will be reverified. This has no effect when set on a secondary site.' @@ -256,6 +261,11 @@ def geo_site_status optional :selective_sync_namespace_ids, as: :namespace_ids, type: Array[Integer], coerce_with: Validations::Types::CommaSeparatedToIntegerArray.coerce, desc: 'The IDs of groups that should be synced, if `selective_sync_type` == `namespaces`' + optional :selective_sync_organization_ids, + as: :organization_ids, + type: Array[Integer], + coerce_with: Validations::Types::CommaSeparatedToIntegerArray.coerce, + desc: 'The IDs of organizations that should be synced, if `selective_sync_type` == `organizations`' optional :minimum_reverification_interval, type: Integer, desc: 'The interval (in days) in which the repository verification is valid. Once expired, it ' \ 'will be reverified. This has no effect when set on a secondary site.' diff --git a/ee/lib/ee/api/entities/geo_node.rb b/ee/lib/ee/api/entities/geo_node.rb index 0bb527974204d6c0b9ebc3e599f4d443520e1e86..7fab4f1ac4f6e8635b6739de386d793d89621817 100644 --- a/ee/lib/ee/api/entities/geo_node.rb +++ b/ee/lib/ee/api/entities/geo_node.rb @@ -22,6 +22,8 @@ class GeoNode < Grape::Entity expose :selective_sync_type expose :selective_sync_shards expose :namespace_ids, as: :selective_sync_namespace_ids + expose :organization_ids, as: :selective_sync_organization_ids, + if: ->(_, _) { ::Gitlab::Geo.geo_selective_sync_by_organizations_enabled? } expose :minimum_reverification_interval expose :sync_object_storage, if: ->(geo_node, _) { geo_node.secondary? } diff --git a/ee/lib/ee/api/entities/geo_site.rb b/ee/lib/ee/api/entities/geo_site.rb index a506a2f35e7b5040297059a730e5787826e02fe9..f9ca9a0106e798ef117193488205be5d19c707f3 100644 --- a/ee/lib/ee/api/entities/geo_site.rb +++ b/ee/lib/ee/api/entities/geo_site.rb @@ -22,6 +22,8 @@ class GeoSite < Grape::Entity expose :selective_sync_type expose :selective_sync_shards expose :namespace_ids, as: :selective_sync_namespace_ids + expose :organization_ids, as: :selective_sync_organization_ids, + if: ->(_, _) { ::Gitlab::Geo.geo_selective_sync_by_organizations_enabled? } expose :minimum_reverification_interval expose :sync_object_storage, if: ->(geo_site, _) { geo_site.secondary? } diff --git a/ee/lib/gitlab/geo.rb b/ee/lib/gitlab/geo.rb index 409d0ddaa8461dff1e5ad2338cd345adfa21fd94..a51659e7de5516f876578d3ea41cbe15e9c9e271 100644 --- a/ee/lib/gitlab/geo.rb +++ b/ee/lib/gitlab/geo.rb @@ -359,5 +359,9 @@ def self.postgresql_replication_agnostic_enabled? def self.org_mover_extend_selective_sync_to_primary_checksumming? Feature.enabled?(:org_mover_extend_selective_sync_to_primary_checksumming, type: :ops) # rubocop:disable Gitlab/FeatureFlagWithoutActor -- Checksumming is instance wide end + + def self.geo_selective_sync_by_organizations_enabled? + ::Feature.enabled?(:geo_selective_sync_by_organizations, :instance) + end end end diff --git a/ee/spec/controllers/admin/geo/nodes_controller_spec.rb b/ee/spec/controllers/admin/geo/nodes_controller_spec.rb index 8a8038941d993391f3405c7bc61fe3db58db4b41..ad67683d7416f444d5a61b4e88200d4cb0f3f1fd 100644 --- a/ee/spec/controllers/admin/geo/nodes_controller_spec.rb +++ b/ee/spec/controllers/admin/geo/nodes_controller_spec.rb @@ -108,6 +108,42 @@ def go expect(response).to redirect_to(admin_geo_nodes_path) end end + + context 'with organization_ids parameter' do + let(:organization1) { create(:organization) } + let(:organization2) { create(:organization) } + let(:geo_node_attributes) { { url: 'http://example.com', organization_ids: [organization1.id.to_s, organization2.id.to_s] } } + + context 'when geo_selective_sync_by_organizations feature flag is enabled' do + it 'includes organization_ids in the parameters passed to the service' do + expected_params = ActionController::Parameters.new(geo_node_attributes).permit! + + expect(Geo::NodeCreateService).to receive(:new) + .with(expected_params) + .and_call_original + + go + end + end + + context 'when geo_selective_sync_by_organizations feature flag is disabled' do + before do + stub_feature_flags(geo_selective_sync_by_organizations: false) + end + + it 'excludes organization_ids in the parameters passed to the service' do + expected_params = geo_node_attributes.dup + expected_params.delete(:organization_ids) + expected_params = ActionController::Parameters.new(expected_params).permit! + + expect(Geo::NodeCreateService).to receive(:new) + .with(expected_params) + .and_call_original + + go + end + end + end end end @@ -156,6 +192,49 @@ def go go end + + context 'with organization_ids parameter' do + let(:organization1) { create(:organization) } + let(:organization2) { create(:organization) } + let(:geo_node_attributes) do + { + url: 'http://example.com', + internal_url: 'http://internal-url.com', + selective_sync_shards: %w[foo bar], + organization_ids: [organization1.id.to_s, organization2.id.to_s] + } + end + + context 'when geo_selective_sync_by_organizations feature flag is enabled' do + it 'includes organization_ids in the parameters passed to the service' do + expected_params = ActionController::Parameters.new(geo_node_attributes).permit! + + expect(Geo::NodeUpdateService).to receive(:new) + .with(geo_node, expected_params) + .and_call_original + + go + end + end + + context 'when geo_selective_sync_by_organizations feature flag is disabled' do + before do + stub_feature_flags(geo_selective_sync_by_organizations: false) + end + + it 'excludes organization_ids in the parameters passed to the service' do + expected_params = geo_node_attributes.dup + expected_params.delete(:organization_ids) + expected_params = ActionController::Parameters.new(expected_params).permit! + + expect(Geo::NodeUpdateService).to receive(:new) + .with(geo_node, expected_params) + .and_call_original + + go + end + end + end end end end diff --git a/ee/spec/factories/geo/geo_node_organization_links.rb b/ee/spec/factories/geo/geo_node_organization_links.rb new file mode 100644 index 0000000000000000000000000000000000000000..6fa27827875fdbc5b17fbc494494d7e79b9cebd8 --- /dev/null +++ b/ee/spec/factories/geo/geo_node_organization_links.rb @@ -0,0 +1,8 @@ +# frozen_string_literal: true + +FactoryBot.define do + factory :geo_node_organization_link, class: "Geo::GeoNodeOrganizationLink" do + geo_node + organization + end +end diff --git a/ee/spec/fixtures/api/schemas/public_api/v4/geo_node.json b/ee/spec/fixtures/api/schemas/public_api/v4/geo_node.json index b385397672bacd4cbf397f63445602ff246041a1..236788648453f66e52ae06f1379b388a05b29b8e 100644 --- a/ee/spec/fixtures/api/schemas/public_api/v4/geo_node.json +++ b/ee/spec/fixtures/api/schemas/public_api/v4/geo_node.json @@ -14,6 +14,7 @@ "selective_sync_type", "selective_sync_shards", "selective_sync_namespace_ids", + "selective_sync_organization_ids", "minimum_reverification_interval", "clone_protocol", "web_edit_url", @@ -86,6 +87,15 @@ "type": "integer" } }, + "selective_sync_organization_ids": { + "type": [ + "array", + "null" + ], + "items": { + "type": "integer" + } + }, "minimum_reverification_interval": { "type": "integer" }, diff --git a/ee/spec/fixtures/api/schemas/public_api/v4/geo_site.json b/ee/spec/fixtures/api/schemas/public_api/v4/geo_site.json index 3187aa6b92fe9ea45f2400583669c4de1d7dc606..1aed068d44ab60b4076246d3ec1abe06d060ab2a 100644 --- a/ee/spec/fixtures/api/schemas/public_api/v4/geo_site.json +++ b/ee/spec/fixtures/api/schemas/public_api/v4/geo_site.json @@ -14,6 +14,7 @@ "selective_sync_type", "selective_sync_shards", "selective_sync_namespace_ids", + "selective_sync_organization_ids", "minimum_reverification_interval", "web_edit_url", "_links" @@ -85,6 +86,15 @@ "type": "integer" } }, + "selective_sync_organization_ids": { + "type": [ + "array", + "null" + ], + "items": { + "type": "integer" + } + }, "minimum_reverification_interval": { "type": "integer" }, diff --git a/ee/spec/models/ee/project_spec.rb b/ee/spec/models/ee/project_spec.rb index a5764e55ff7b974069c4b351f3fd5f27a3712d11..73888f3bfde43e68f7d853cd9f9205f79c826b37 100644 --- a/ee/spec/models/ee/project_spec.rb +++ b/ee/spec/models/ee/project_spec.rb @@ -5463,6 +5463,14 @@ def stub_default_url_options(host) expect(described_class.selective_sync_scope(node)).to match_array([project_1, project_2]) end + it 'returns none with selective sync by organizations, for now' do + organization_1 = create(:organization) + organization_2 = create(:organization) + node.update!(selective_sync_type: 'organizations', organizations: [organization_1, organization_2]) + + expect(described_class.selective_sync_scope(node)).to be_empty + end + it 'raises if an unrecognised selective sync type is used' do node.update_attribute(:selective_sync_type, 'unknown') diff --git a/ee/spec/models/geo/geo_node_organization_link_spec.rb b/ee/spec/models/geo/geo_node_organization_link_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..5d629f71474f4e2be377f9227d258eb09100d2a6 --- /dev/null +++ b/ee/spec/models/geo/geo_node_organization_link_spec.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Geo::GeoNodeOrganizationLink, :models, feature_category: :geo_replication do + describe 'relationships' do + it { is_expected.to belong_to(:geo_node).required } + it { is_expected.to belong_to(:organization).class_name('Organizations::Organization').required } + end + + describe 'validations' do + let!(:geo_node_organization_link) { create(:geo_node_organization_link) } + + it { is_expected.to validate_uniqueness_of(:organization_id).scoped_to(:geo_node_id) } + end + + context 'with loose foreign key on geo_node_organization_links.organization_id' do + it_behaves_like 'cleanup by a loose foreign key' do + let_it_be(:parent) { create(:organization) } + let_it_be(:model) { create(:geo_node_organization_link, organization: parent) } + end + end +end diff --git a/ee/spec/models/geo_node_spec.rb b/ee/spec/models/geo_node_spec.rb index 42a0c738c0f14c636c3e52b7320d7ff193443be4..1710fc51abab299a74e7277d3d37550166577779 100644 --- a/ee/spec/models/geo_node_spec.rb +++ b/ee/spec/models/geo_node_spec.rb @@ -22,6 +22,8 @@ it { is_expected.to have_many(:geo_node_namespace_links) } it { is_expected.to have_many(:namespaces).through(:geo_node_namespace_links) } + it { is_expected.to have_many(:geo_node_organization_links) } + it { is_expected.to have_many(:organizations).through(:geo_node_organization_links).class_name('Organizations::Organization') } end it_behaves_like 'encrypted attribute', :secret_access_key, :db_key_base_32 do @@ -42,6 +44,12 @@ it { is_expected.to validate_uniqueness_of(:name).case_insensitive } it { is_expected.to validate_length_of(:name).is_at_most(255) } + context 'when checking SELECTIVE_SYNC_TYPES constant' do + it 'has expected sync types' do + expect(GeoNode::SELECTIVE_SYNC_TYPES).to match_array(%w[namespaces shards organizations]) + end + end + context 'when validating primary node' do it 'cannot be disabled' do primary_node.enabled = false @@ -748,6 +756,12 @@ is_expected.to be_truthy end + it 'returns true when selective sync is by organizations' do + node.update!(selective_sync_type: 'organizations') + + is_expected.to be_truthy + end + it 'returns false when selective sync is disabled' do node.update!( selective_sync_type: '', @@ -771,6 +785,90 @@ end end + describe '#selective_sync_by_organizations?' do + subject(:selective_sync_by_organizations?) { node.selective_sync_by_organizations? } + + context 'when selective_sync_type is organizations' do + before do + node.update!(selective_sync_type: 'organizations') + end + + it 'returns true' do + is_expected.to be_truthy + end + end + + context 'when selective_sync_type is namespaces' do + before do + node.update!(selective_sync_type: 'namespaces') + end + + it 'returns false' do + is_expected.to be_falsy + end + end + + context 'when selective_sync_type is shards' do + before do + node.update!(selective_sync_type: 'shards') + end + + it 'returns false' do + is_expected.to be_falsy + end + end + + context 'when selective_sync_type is nil' do + before do + node.update!(selective_sync_type: nil) + end + + it 'returns false' do + is_expected.to be_falsy + end + end + + context 'when selective_sync_type is empty string' do + before do + node.update!(selective_sync_type: '') + end + + it 'returns false' do + is_expected.to be_falsy + end + end + + context 'when selective_sync_type is an invalid value' do + before do + node.update_column(:selective_sync_type, 'invalid') + end + + it 'raises an error' do + expect { selective_sync_by_organizations? }.to raise_error(Geo::Errors::UnknownSelectiveSyncType) + end + end + + context 'when selective_sync_type has mixed case' do + before do + node.update_column(:selective_sync_type, 'Organizations') + end + + it 'raises an error' do + expect { selective_sync_by_organizations? }.to raise_error(Geo::Errors::UnknownSelectiveSyncType) + end + end + + context 'when selective_sync_type has extra whitespace' do + before do + node.update_column(:selective_sync_type, ' organizations ') + end + + it 'raises an error' do + expect { selective_sync_by_organizations? }.to raise_error(Geo::Errors::UnknownSelectiveSyncType) + end + end + end + describe '#namespaces_for_group_owned_replicables' do subject(:namespace_ids_for_group_owned_replicables) { node.namespaces_for_group_owned_replicables.pluck(:id) } @@ -800,6 +898,14 @@ is_expected.to contain_exactly(selected_namespace.id, child_namespace.id) end end + + context 'when selective sync is by organization' do + it 'returns none' do + node.update!(selective_sync_type: 'organizations', organizations: []) + + is_expected.to be_empty + end + end end describe '#name=' do diff --git a/ee/spec/requests/api/geo_nodes_spec.rb b/ee/spec/requests/api/geo_nodes_spec.rb index 3d0c102d9b29790c2070472771deff6ff75c93c2..7500a8238073b0997721f391eadadaac70e1f33b 100644 --- a/ee/spec/requests/api/geo_nodes_spec.rb +++ b/ee/spec/requests/api/geo_nodes_spec.rb @@ -36,10 +36,65 @@ selective_sync_namespace_ids: group_to_sync.id, minimum_reverification_interval: 10 } - expect_any_instance_of(Geo::NodeCreateService).to receive(:execute).once.and_call_original + post api('/geo_nodes', admin, admin_mode: true), params: geo_node_params expect(response).to have_gitlab_http_status(:created) end + + context 'with organization_ids parameter' do + let(:organization1) { create(:organization) } + let(:organization2) { create(:organization) } + + context 'when geo_selective_sync_by_organizations feature flag is enabled' do + it 'accepts organization_ids parameter' do + geo_node_params = { + name: 'Test Node with Organizations', + url: 'http://example.com', + selective_sync_type: "organizations", + selective_sync_organization_ids: [organization1.id, organization2.id] + } + + post api('/geo_nodes', admin, admin_mode: true), params: geo_node_params + + expect(response).to have_gitlab_http_status(:created) + end + + it 'includes organization_ids in the response' do + geo_node_params = { + name: 'Test Node with Organizations', + url: 'http://example.com', + selective_sync_type: "organizations", + selective_sync_organization_ids: [organization1.id, organization2.id] + } + + post api('/geo_nodes', admin, admin_mode: true), params: geo_node_params + + expect(response).to have_gitlab_http_status(:created) + expect(json_response).to include('selective_sync_type' => 'organizations') + expect(json_response).to include('selective_sync_organization_ids' => [organization1.id, organization2.id]) + end + end + + context 'when geo_selective_sync_by_organizations feature flag is disabled' do + before do + stub_feature_flags(geo_selective_sync_by_organizations: false) + end + + it 'ignores organization_ids parameter when feature flag is disabled' do + geo_node_params = { + name: 'Test Node without Organizations', + url: 'http://example.com', + selective_sync_type: "organizations", + selective_sync_organization_ids: [organization1.id, organization2.id] + } + + post api('/geo_nodes', admin, admin_mode: true), params: geo_node_params + + expect(response).to have_gitlab_http_status(:created) + expect(json_response).not_to include('organization_ids') + end + end + end end describe 'GET /geo_nodes' do @@ -259,6 +314,67 @@ expect(json_response).to include(params) end + context 'with organization_ids parameter' do + let(:organization1) { create(:organization) } + let(:organization2) { create(:organization) } + + context 'when geo_selective_sync_by_organizations feature flag is enabled' do + it 'updates organization_ids parameter' do + params = { + selective_sync_type: "organizations", + selective_sync_organization_ids: [organization1.id, organization2.id] + } + + put api("/geo_nodes/#{secondary.id}", admin, admin_mode: true), params: params + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to match_response_schema('public_api/v4/geo_node', dir: 'ee') + expect(json_response).to include('selective_sync_type' => 'organizations') + expect(json_response).to include('selective_sync_organization_ids' => [organization1.id, organization2.id]) + end + + it 'can clear organization_ids by setting to empty array' do + # First set organizations + secondary.update!(selective_sync_type: 'organizations', organization_ids: [organization1.id]) + + params = { + selective_sync_type: "organizations", + selective_sync_organization_ids: [] + } + + put api("/geo_nodes/#{secondary.id}", admin, admin_mode: true), params: params + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to include('selective_sync_type' => 'organizations') + expect(json_response).to include('selective_sync_organization_ids' => []) + end + end + + context 'when geo_selective_sync_by_organizations feature flag is disabled' do + before do + stub_feature_flags(geo_selective_sync_by_organizations: false) + end + + it 'ignores organization_ids parameter' do + # Set some existing organizations + secondary.update!(selective_sync_type: 'organizations', organization_ids: [organization1.id]) + + params = { + selective_sync_type: "namespaces", + selective_sync_organization_ids: [organization2.id] + } + + put api("/geo_nodes/#{secondary.id}", admin, admin_mode: true), params: params + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to include('selective_sync_type' => 'namespaces') + expect(json_response).not_to include('organization_ids') + # Should preserve existing organization_ids since parameter was ignored + expect(secondary.reload.organization_ids).to eq([organization1.id]) + end + end + end + it 'can update primary' do params = { url: 'https://updated.example.com/' diff --git a/ee/spec/requests/api/geo_sites_spec.rb b/ee/spec/requests/api/geo_sites_spec.rb index 279aedd7397e715e652a20d5ed9b86bca641499b..d014255a23d346ac587272595206ff492176bc2b 100644 --- a/ee/spec/requests/api/geo_sites_spec.rb +++ b/ee/spec/requests/api/geo_sites_spec.rb @@ -61,6 +61,61 @@ expect(json_response).to include({ 'message' => { 'enabled' => ['Geo primary node cannot be disabled'], 'primary' => ['node already exists'] } }) end + + context 'with organization_ids parameter' do + let(:organization1) { create(:organization) } + let(:organization2) { create(:organization) } + + context 'when geo_selective_sync_by_organizations feature flag is enabled' do + it 'accepts organization_ids parameter' do + geo_node_params = { + name: 'Test Node with Organizations', + url: 'http://example.com', + selective_sync_type: "organizations", + selective_sync_organization_ids: [organization1.id, organization2.id] + } + + post api('/geo_nodes', admin, admin_mode: true), params: geo_node_params + + expect(response).to have_gitlab_http_status(:created) + end + + it 'includes organization_ids in the response' do + geo_node_params = { + name: 'Test Node with Organizations', + url: 'http://example.com', + selective_sync_type: "organizations", + selective_sync_organization_ids: [organization1.id, organization2.id] + } + + post api('/geo_nodes', admin, admin_mode: true), params: geo_node_params + + expect(response).to have_gitlab_http_status(:created) + expect(json_response).to include('selective_sync_type' => 'organizations') + expect(json_response).to include('selective_sync_organization_ids' => [organization1.id, organization2.id]) + end + end + + context 'when geo_selective_sync_by_organizations feature flag is disabled' do + before do + stub_feature_flags(geo_selective_sync_by_organizations: false) + end + + it 'ignores organization_ids parameter when feature flag is disabled' do + geo_node_params = { + name: 'Test Node without Organizations', + url: 'http://example.com', + selective_sync_type: "organizations", + selective_sync_organization_ids: [organization1.id, organization2.id] + } + + post api('/geo_nodes', admin, admin_mode: true), params: geo_node_params + + expect(response).to have_gitlab_http_status(:created) + expect(json_response).not_to include('organization_ids') + end + end + end end describe 'GET /geo_sites' do @@ -294,6 +349,60 @@ expect(json_response).to include(params) end + context 'with organization_ids parameter' do + let(:organization1) { create(:organization) } + let(:organization2) { create(:organization) } + + it 'updates organization_ids parameter' do + params = { + selective_sync_type: "organizations", + selective_sync_organization_ids: [organization1.id, organization2.id] + } + + put api("/geo_sites/#{secondary.id}", admin, admin_mode: true), params: params + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to match_response_schema('public_api/v4/geo_site', dir: 'ee') + expect(json_response).to include('selective_sync_type' => 'organizations') + expect(json_response).to include('selective_sync_organization_ids' => [organization1.id, organization2.id]) + end + + it 'can clear organization_ids by setting to empty array' do + # First set organizations + secondary.update!(selective_sync_type: 'organizations', organization_ids: [organization1.id]) + + params = { + selective_sync_type: "organizations", + selective_sync_organization_ids: [] + } + + put api("/geo_sites/#{secondary.id}", admin, admin_mode: true), params: params + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to include('selective_sync_type' => 'organizations') + expect(json_response).to include('selective_sync_organization_ids' => []) + end + + context 'when geo_selective_sync_by_organizations feature flag is disabled' do + before do + stub_feature_flags(geo_selective_sync_by_organizations: false) + end + + it 'does not include selective_sync_organization_ids in the response' do + # Set some organizations in the DB + secondary.update!(organization_ids: [organization1.id]) + + params = { files_max_capacity: 123 } + + put api("/geo_sites/#{secondary.id}", admin, admin_mode: true), params: params + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to include('files_max_capacity' => 123) + expect(json_response).not_to include('selective_sync_organization_ids') + end + end + end + it 'can update primary' do params = { url: 'https://updated.example.com/' diff --git a/ee/spec/services/geo/node_update_service_spec.rb b/ee/spec/services/geo/node_update_service_spec.rb index dcbf0f2bcf8d73cdaae414642e890248ae363b08..a6579039cfc62d30267b17e0caacc8aa28f2bec1 100644 --- a/ee/spec/services/geo/node_update_service_spec.rb +++ b/ee/spec/services/geo/node_update_service_spec.rb @@ -36,5 +36,50 @@ expect(service.execute).to eq false end + + context 'when params includes organization_ids' do + context 'when organization_ids is a string of comma-separated integers' do + it 'updates the organization links' do + organization_1 = create(:organization) + organization_2 = create(:organization) + + service = described_class.new(geo_node, { organization_ids: "#{organization_1.id},#{organization_2.id}" }) + expect(service.execute).to be true + geo_node.reload + + expect(geo_node.organizations).to match_array([organization_1, organization_2]) + end + end + + context 'when organization_ids is an empty string' do + it 'successfully removes organization links' do + organization = create(:organization) + geo_node.organizations << organization + geo_node.save! + expect(geo_node.organizations).not_to be_empty + + service = described_class.new(geo_node, { organization_ids: '' }) + expect(service.execute).to be true + geo_node.reload + + expect(geo_node.organizations).to be_empty + end + end + + context 'when organization_ids is an empty Array' do + it 'successfully removes organization links' do + organization = create(:organization) + geo_node.organizations << organization + geo_node.save! + expect(geo_node.organizations).not_to be_empty + + service = described_class.new(geo_node, { organization_ids: [] }) + expect(service.execute).to be true + geo_node.reload + + expect(geo_node.organizations).to be_empty + end + end + end end end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index b9a730112868ebc69187c80e7c03c282c8ed5be1..97f4a46ef95d945fa531892e1c89b9369f4e0bd5 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -29354,6 +29354,9 @@ msgstr "" msgid "Geo|Projects in certain groups" msgstr "" +msgid "Geo|Projects in certain organizations" +msgstr "" + msgid "Geo|Projects in certain storage shards" msgstr ""