diff --git a/db/docs/approval_policy_rules.yml b/db/docs/approval_policy_rules.yml index aacee84f1ca75c384895cd57656445c0f32cc2b4..d8ad54f1bcbbdc60cebb2192a0e65a56d2fa93dd 100644 --- a/db/docs/approval_policy_rules.yml +++ b/db/docs/approval_policy_rules.yml @@ -7,6 +7,4 @@ feature_categories: description: Stores approval policy rules. introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/146504 milestone: '16.11' -gitlab_schema: gitlab_main_cell -sharding_key: - security_policy_management_project_id: projects +gitlab_schema: gitlab_main diff --git a/db/docs/security_policies.yml b/db/docs/security_policies.yml index e0787ea1d20e7848bb65cef994ca697ed1760969..a73f1eb9a5921b8eb9e8b7d82538b2310c5023b2 100644 --- a/db/docs/security_policies.yml +++ b/db/docs/security_policies.yml @@ -7,6 +7,4 @@ feature_categories: description: Stores policy data. introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/146504 milestone: '16.11' -gitlab_schema: gitlab_main_cell -sharding_key: - security_policy_management_project_id: projects +gitlab_schema: gitlab_main diff --git a/db/migrate/20240416123401_add_security_policy_management_project_id_to_security_policies.rb b/db/migrate/20240416123401_add_security_policy_management_project_id_to_security_policies.rb deleted file mode 100644 index 5258edcea9693e36979f863e2c116c8efb0920eb..0000000000000000000000000000000000000000 --- a/db/migrate/20240416123401_add_security_policy_management_project_id_to_security_policies.rb +++ /dev/null @@ -1,15 +0,0 @@ -# frozen_string_literal: true - -class AddSecurityPolicyManagementProjectIdToSecurityPolicies < Gitlab::Database::Migration[2.2] - milestone '17.0' - - def up - # rubocop:disable Rails/NotNullColumn -- table is empty - add_column :security_policies, :security_policy_management_project_id, :bigint, null: false - # rubocop:enable Rails/NotNullColumn - end - - def down - remove_column :security_policies, :security_policy_management_project_id - end -end diff --git a/db/migrate/20240416123402_add_security_policy_management_project_id_fk_to_security_policies.rb b/db/migrate/20240416123402_add_security_policy_management_project_id_fk_to_security_policies.rb deleted file mode 100644 index 2bda539ea3c9a68b917715f5e5578a578d981ba9..0000000000000000000000000000000000000000 --- a/db/migrate/20240416123402_add_security_policy_management_project_id_fk_to_security_policies.rb +++ /dev/null @@ -1,17 +0,0 @@ -# frozen_string_literal: true - -class AddSecurityPolicyManagementProjectIdFkToSecurityPolicies < Gitlab::Database::Migration[2.2] - milestone '17.0' - disable_ddl_transaction! - - def up - add_concurrent_foreign_key :security_policies, - :projects, - column: :security_policy_management_project_id, - on_delete: :cascade - end - - def down - remove_foreign_key_if_exists :security_policies, column: :security_policy_management_project_id - end -end diff --git a/db/migrate/20240416123403_add_index_security_policy_management_project_id_on_security_policies.rb b/db/migrate/20240416123403_add_index_security_policy_management_project_id_on_security_policies.rb deleted file mode 100644 index 665c7c8865f9e28ccb4c67d9f5af80098a10f24f..0000000000000000000000000000000000000000 --- a/db/migrate/20240416123403_add_index_security_policy_management_project_id_on_security_policies.rb +++ /dev/null @@ -1,16 +0,0 @@ -# frozen_string_literal: true - -class AddIndexSecurityPolicyManagementProjectIdOnSecurityPolicies < Gitlab::Database::Migration[2.2] - milestone '17.0' - disable_ddl_transaction! - - INDEX_NAME = 'index_security_policies_on_policy_management_project_id' - - def up - add_concurrent_index :security_policies, :security_policy_management_project_id, name: INDEX_NAME - end - - def down - remove_concurrent_index_by_name :security_policies, INDEX_NAME - end -end diff --git a/db/migrate/20240416123404_add_security_policy_management_project_id_to_approval_policy_rules.rb b/db/migrate/20240416123404_add_security_policy_management_project_id_to_approval_policy_rules.rb deleted file mode 100644 index 1a5459d115bbd1b0e5250067b0af60cc1f16468e..0000000000000000000000000000000000000000 --- a/db/migrate/20240416123404_add_security_policy_management_project_id_to_approval_policy_rules.rb +++ /dev/null @@ -1,15 +0,0 @@ -# frozen_string_literal: true - -class AddSecurityPolicyManagementProjectIdToApprovalPolicyRules < Gitlab::Database::Migration[2.2] - milestone '17.0' - - def up - # rubocop:disable Rails/NotNullColumn -- table is empty - add_column :approval_policy_rules, :security_policy_management_project_id, :bigint, null: false - # rubocop:enable Rails/NotNullColumn - end - - def down - remove_column :approval_policy_rules, :security_policy_management_project_id - end -end diff --git a/db/migrate/20240416123405_add_security_policy_management_project_id_fk_to_approval_policy_rules.rb b/db/migrate/20240416123405_add_security_policy_management_project_id_fk_to_approval_policy_rules.rb deleted file mode 100644 index c7b8fd6a06340ee1c10b07e2a325e17899446b2c..0000000000000000000000000000000000000000 --- a/db/migrate/20240416123405_add_security_policy_management_project_id_fk_to_approval_policy_rules.rb +++ /dev/null @@ -1,17 +0,0 @@ -# frozen_string_literal: true - -class AddSecurityPolicyManagementProjectIdFkToApprovalPolicyRules < Gitlab::Database::Migration[2.2] - milestone '17.0' - disable_ddl_transaction! - - def up - add_concurrent_foreign_key :approval_policy_rules, - :projects, - column: :security_policy_management_project_id, - on_delete: :cascade - end - - def down - remove_foreign_key_if_exists :approval_policy_rules, column: :security_policy_management_project_id - end -end diff --git a/db/migrate/20240416123406_add_index_security_policy_management_project_id_on_approval_policy_rules.rb b/db/migrate/20240416123406_add_index_security_policy_management_project_id_on_approval_policy_rules.rb deleted file mode 100644 index db3441fe91d93943cd810927386212e582bab06f..0000000000000000000000000000000000000000 --- a/db/migrate/20240416123406_add_index_security_policy_management_project_id_on_approval_policy_rules.rb +++ /dev/null @@ -1,16 +0,0 @@ -# frozen_string_literal: true - -class AddIndexSecurityPolicyManagementProjectIdOnApprovalPolicyRules < Gitlab::Database::Migration[2.2] - milestone '17.0' - disable_ddl_transaction! - - INDEX_NAME = 'index_approval_policy_rules_on_policy_management_project_id' - - def up - add_concurrent_index :approval_policy_rules, :security_policy_management_project_id, name: INDEX_NAME - end - - def down - remove_concurrent_index_by_name :approval_policy_rules, INDEX_NAME - end -end diff --git a/db/schema_migrations/20240416123401 b/db/schema_migrations/20240416123401 deleted file mode 100644 index 8fc379fe815b690da0c1ba13d246cc26d8f0f3f7..0000000000000000000000000000000000000000 --- a/db/schema_migrations/20240416123401 +++ /dev/null @@ -1 +0,0 @@ -5961e034705392a82a709fccddc32d2cbbde016d0c0b1db3f66af1be0573928b \ No newline at end of file diff --git a/db/schema_migrations/20240416123402 b/db/schema_migrations/20240416123402 deleted file mode 100644 index 272a686370c1aa8ee0a2926677edc78d3956e67d..0000000000000000000000000000000000000000 --- a/db/schema_migrations/20240416123402 +++ /dev/null @@ -1 +0,0 @@ -17716005af88da4cb7905faeba87e31462f50f61eef25f671b338c4cb01025d6 \ No newline at end of file diff --git a/db/schema_migrations/20240416123403 b/db/schema_migrations/20240416123403 deleted file mode 100644 index f6e5c4bab3c3753925f453a59e16abe78a015a91..0000000000000000000000000000000000000000 --- a/db/schema_migrations/20240416123403 +++ /dev/null @@ -1 +0,0 @@ -f5c9b34ef88af798bc6f6918d9a04aef29377d9c9f4d07a488f890cb8d1527c2 \ No newline at end of file diff --git a/db/schema_migrations/20240416123404 b/db/schema_migrations/20240416123404 deleted file mode 100644 index 3567e4ebc3ae99a15d2bfb72d03ccce293e5a27f..0000000000000000000000000000000000000000 --- a/db/schema_migrations/20240416123404 +++ /dev/null @@ -1 +0,0 @@ -7eacbbbe4f7e0e0fc2cc15369558126d03e6a8dd58eafc7281eb79fd4d0aa80f \ No newline at end of file diff --git a/db/schema_migrations/20240416123405 b/db/schema_migrations/20240416123405 deleted file mode 100644 index e2be5e658fd75542aaf725adafb21353ca2f1c42..0000000000000000000000000000000000000000 --- a/db/schema_migrations/20240416123405 +++ /dev/null @@ -1 +0,0 @@ -295e692b5ada0d84cf4ba1b64f6e56237e2d09e1be3cb0674b61ab238884bb2c \ No newline at end of file diff --git a/db/schema_migrations/20240416123406 b/db/schema_migrations/20240416123406 deleted file mode 100644 index a8a0b2f81a1a85efcc90a1f224931479f1ba2e16..0000000000000000000000000000000000000000 --- a/db/schema_migrations/20240416123406 +++ /dev/null @@ -1 +0,0 @@ -50b095186f8ebaa709f7918307b77224ea35ebb56bd9c8e3fe5031ed977af1be \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index c08e5a22f08830db7427f5dda8c732ff937d9dbd..4a030710d3bac1723b03594673384024421e1663 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -4602,8 +4602,7 @@ CREATE TABLE approval_policy_rules ( updated_at timestamp with time zone NOT NULL, rule_index smallint NOT NULL, type smallint NOT NULL, - content jsonb DEFAULT '{}'::jsonb NOT NULL, - security_policy_management_project_id bigint NOT NULL + content jsonb DEFAULT '{}'::jsonb NOT NULL ); CREATE SEQUENCE approval_policy_rules_id_seq @@ -15776,7 +15775,6 @@ CREATE TABLE security_policies ( scope jsonb DEFAULT '{}'::jsonb NOT NULL, actions jsonb DEFAULT '[]'::jsonb NOT NULL, approval_settings jsonb DEFAULT '{}'::jsonb NOT NULL, - security_policy_management_project_id bigint NOT NULL, CONSTRAINT check_3fa0f29e4b CHECK ((char_length(name) <= 255)), CONSTRAINT check_966e08b242 CHECK ((char_length(checksum) <= 255)), CONSTRAINT check_99c8e08928 CHECK ((char_length(description) <= 255)) @@ -24300,8 +24298,6 @@ CREATE UNIQUE INDEX index_approval_merge_request_rules_users_1 ON approval_merge CREATE INDEX index_approval_merge_request_rules_users_2 ON approval_merge_request_rules_users USING btree (user_id); -CREATE INDEX index_approval_policy_rules_on_policy_management_project_id ON approval_policy_rules USING btree (security_policy_management_project_id); - CREATE UNIQUE INDEX index_approval_policy_rules_on_unique_policy_rule_index ON approval_policy_rules USING btree (security_policy_id, rule_index); CREATE UNIQUE INDEX index_approval_project_rules_groups_1 ON approval_project_rules_groups USING btree (approval_project_rule_id, group_id); @@ -27128,8 +27124,6 @@ CREATE INDEX p_ci_builds_name_id_idx ON ONLY p_ci_builds USING btree (name, id) CREATE INDEX index_security_ci_builds_on_name_and_id_parser_features ON ci_builds USING btree (name, id) WHERE (((name)::text = ANY (ARRAY[('container_scanning'::character varying)::text, ('dast'::character varying)::text, ('dependency_scanning'::character varying)::text, ('license_management'::character varying)::text, ('sast'::character varying)::text, ('secret_detection'::character varying)::text, ('coverage_fuzzing'::character varying)::text, ('license_scanning'::character varying)::text, ('apifuzzer_fuzz'::character varying)::text, ('apifuzzer_fuzz_dnd'::character varying)::text])) AND ((type)::text = 'Ci::Build'::text)); -CREATE INDEX index_security_policies_on_policy_management_project_id ON security_policies USING btree (security_policy_management_project_id); - CREATE UNIQUE INDEX index_security_policies_on_unique_config_type_policy_index ON security_policies USING btree (security_orchestration_policy_configuration_id, type, policy_index); CREATE INDEX index_security_scans_for_non_purged_records ON security_scans USING btree (created_at, id) WHERE (status <> 6); @@ -29763,9 +29757,6 @@ ALTER TABLE ONLY merge_requests ALTER TABLE ONLY sbom_occurrences_vulnerabilities ADD CONSTRAINT fk_07b81e3a81 FOREIGN KEY (vulnerability_id) REFERENCES vulnerabilities(id) ON DELETE CASCADE; -ALTER TABLE ONLY security_policies - ADD CONSTRAINT fk_08722e8ac7 FOREIGN KEY (security_policy_management_project_id) REFERENCES projects(id) ON DELETE CASCADE; - ALTER TABLE ONLY abuse_report_user_mentions ADD CONSTRAINT fk_088018ecd8 FOREIGN KEY (abuse_report_id) REFERENCES abuse_reports(id) ON DELETE CASCADE; @@ -30786,9 +30777,6 @@ ALTER TABLE p_ci_builds_metadata ALTER TABLE ONLY gitlab_subscriptions ADD CONSTRAINT fk_e2595d00a1 FOREIGN KEY (namespace_id) REFERENCES namespaces(id) ON DELETE CASCADE; -ALTER TABLE ONLY approval_policy_rules - ADD CONSTRAINT fk_e344cb2d35 FOREIGN KEY (security_policy_management_project_id) REFERENCES projects(id) ON DELETE CASCADE; - ALTER TABLE ONLY abuse_events ADD CONSTRAINT fk_e5ce49c215 FOREIGN KEY (user_id) REFERENCES users(id) ON DELETE SET NULL; diff --git a/ee/app/models/security/approval_policy_rule.rb b/ee/app/models/security/approval_policy_rule.rb index 938f7d61d0997828a6ee341e414c7d735c829c20..d753d1937030adba42b695be1f9c7b7eec3de039 100644 --- a/ee/app/models/security/approval_policy_rule.rb +++ b/ee/app/models/security/approval_policy_rule.rb @@ -11,11 +11,10 @@ class ApprovalPolicyRule < ApplicationRecord validates :typed_content, json_schema: { filename: "approval_policy_rule_content" } - def self.attributes_from_rule_hash(rule_hash, policy_configuration) + def self.attributes_from_rule_hash(rule_hash) { type: rule_hash[:type], - content: rule_hash.without(:type), - security_policy_management_project_id: policy_configuration.security_policy_management_project_id + content: rule_hash.without(:type) } end diff --git a/ee/app/models/security/policy.rb b/ee/app/models/security/policy.rb index eee6b04f50d3d7e6d6340927875fea02870963ee..fea42a26a66627a01559afd1f8e1f9f1fdd7fb09 100644 --- a/ee/app/models/security/policy.rb +++ b/ee/app/models/security/policy.rb @@ -27,7 +27,7 @@ def self.checksum(policy_hash) Digest::SHA256.hexdigest(policy_hash.to_json) end - def self.attributes_from_policy_hash(policy_hash, policy_configuration) + def self.attributes_from_policy_hash(policy_hash) { type: :approval_policy, name: policy_hash[:name], @@ -36,20 +36,19 @@ def self.attributes_from_policy_hash(policy_hash, policy_configuration) actions: policy_hash[:actions], approval_settings: policy_hash[:approval_settings], scope: policy_hash.fetch(:policy_scope, {}), - checksum: checksum(policy_hash), - security_policy_management_project_id: policy_configuration.security_policy_management_project_id + checksum: checksum(policy_hash) } end - def self.upsert_policy(policies, policy_hash, policy_index, policy_configuration, policy_type: :approval_policy) + def self.upsert_policy(policies, policy_hash, policy_index, policy_type: :approval_policy) transaction do policy = policies.find_or_initialize_by(policy_index: policy_index, type: policy_type) - policy.update!(attributes_from_policy_hash(policy_hash, policy_configuration)) + policy.update!(attributes_from_policy_hash(policy_hash)) policy_hash[:rules].map.with_index do |rule_hash, rule_index| Security::ApprovalPolicyRule .find_or_initialize_by(security_policy_id: policy.id, rule_index: rule_index) - .update!(Security::ApprovalPolicyRule.attributes_from_rule_hash(rule_hash, policy_configuration)) + .update!(Security::ApprovalPolicyRule.attributes_from_rule_hash(rule_hash)) end end end diff --git a/ee/app/services/security/security_orchestration_policies/persist_policy_service.rb b/ee/app/services/security/security_orchestration_policies/persist_policy_service.rb index f1f62843acd25ca6ec72a2cd64975e9be5c7676b..7bebd89168c425c3cc66d98d16dd2a9d2aed11bc 100644 --- a/ee/app/services/security/security_orchestration_policies/persist_policy_service.rb +++ b/ee/app/services/security/security_orchestration_policies/persist_policy_service.rb @@ -83,7 +83,7 @@ def update_rearranged_policies(updated_policies) end def upsert_policy(policy_hash, policy_index) - Security::Policy.upsert_policy(security_policies, policy_hash, policy_index, policy_configuration) + Security::Policy.upsert_policy(security_policies, policy_hash, policy_index) end end end diff --git a/ee/spec/factories/security/approval_policy_rules.rb b/ee/spec/factories/security/approval_policy_rules.rb index 605d092d780a66ee271639bb749299dbfa154991..b64d5d508fb2a6316660e51321c03db72adc0057 100644 --- a/ee/spec/factories/security/approval_policy_rules.rb +++ b/ee/spec/factories/security/approval_policy_rules.rb @@ -4,9 +4,6 @@ factory :approval_policy_rule, class: 'Security::ApprovalPolicyRule' do security_policy sequence(:rule_index) - security_policy_management_project_id do - security_policy.security_orchestration_policy_configuration.security_policy_management_project_id - end scan_finding trait :scan_finding do diff --git a/ee/spec/factories/security/policies.rb b/ee/spec/factories/security/policies.rb index 77f78331f5a222de1ff86dcb2f612fe39a03e5a4..899c748b9bfdde1fab2c87ed592b96624df42abc 100644 --- a/ee/spec/factories/security/policies.rb +++ b/ee/spec/factories/security/policies.rb @@ -10,9 +10,6 @@ enabled { true } scope { {} } approval_settings { {} } - security_policy_management_project_id do - security_orchestration_policy_configuration.security_policy_management_project_id - end require_approval trait :require_approval do diff --git a/ee/spec/models/security/policy_spec.rb b/ee/spec/models/security/policy_spec.rb index 15c671e9abf19bbd11f39d1d6b871367164c5d49..7ec2fbef5b5611a2758e2638d30911c713951443 100644 --- a/ee/spec/models/security/policy_spec.rb +++ b/ee/spec/models/security/policy_spec.rb @@ -83,7 +83,7 @@ let(:policy_hash) { build(:scan_result_policy, name: "foobar") } let(:policy_index) { 0 } - subject(:upsert!) { described_class.upsert_policy(policies, policy_hash, policy_index, policy_configuration) } + subject(:upsert!) { described_class.upsert_policy(policies, policy_hash, policy_index) } context 'when the policy does not exist' do let(:upserted_policy) { policy_configuration.security_policies.last } diff --git a/ee/spec/services/security/security_orchestration_policies/persist_policy_service_spec.rb b/ee/spec/services/security/security_orchestration_policies/persist_policy_service_spec.rb index 9f6ab7b840ec76ef108bfbba83bcf081afccdea1..8522a9b047eed7f9679be106c0862fc1e1e908d4 100644 --- a/ee/spec/services/security/security_orchestration_policies/persist_policy_service_spec.rb +++ b/ee/spec/services/security/security_orchestration_policies/persist_policy_service_spec.rb @@ -103,8 +103,7 @@ def persist!(policies) enabled: true, scope: scan_finding_policy[:policy_scope].deep_stringify_keys, actions: scan_finding_policy[:actions].map(&:stringify_keys), - approval_settings: scan_finding_policy[:approval_settings].stringify_keys, - security_policy_management_project_id: policy_configuration.security_policy_management_project_id + approval_settings: scan_finding_policy[:approval_settings].stringify_keys } end @@ -121,8 +120,7 @@ def persist!(policies) security_policy_id: policy_configuration.security_policies.first.id, type: 'scan_finding', rule_index: 0, - content: scan_finding_policy[:rules].first.except(:type).stringify_keys, - security_policy_management_project_id: policy_configuration.security_policy_management_project_id + content: scan_finding_policy[:rules].first.except(:type).stringify_keys } end diff --git a/spec/lib/gitlab/database/no_new_tables_with_gitlab_main_schema_spec.rb b/spec/lib/gitlab/database/no_new_tables_with_gitlab_main_schema_spec.rb index 704c6929c8a86324d587a2cc350ea6b47d638c82..6528e06837f15f67ba141081215f1f8f3bd6f694 100644 --- a/spec/lib/gitlab/database/no_new_tables_with_gitlab_main_schema_spec.rb +++ b/spec/lib/gitlab/database/no_new_tables_with_gitlab_main_schema_spec.rb @@ -12,6 +12,8 @@ # Specific tables can be exempted from this requirement, and such tables must be added to the `exempted_tables` list. let!(:exempted_tables) do [ + "approval_policy_rules", # https://gitlab.com/gitlab-org/gitlab/-/issues/452380 + "security_policies", # https://gitlab.com/gitlab-org/gitlab/-/issues/452380 "audit_events_instance_amazon_s3_configurations", # https://gitlab.com/gitlab-org/gitlab/-/issues/431327 "sbom_source_packages" # https://gitlab.com/gitlab-org/gitlab/-/issues/437718 ]