Merge branch 'revert-ba5ad943' into 'master'

Revert "Add sharding keys and update schema for security policy tables"

See merge request https://gitlab.com/gitlab-org/gitlab/-/merge_requests/150200



Merged-by: Max Fan <mfan@gitlab.com>
Approved-by: Max Fan <mfan@gitlab.com>
Approved-by: Alex Ives <aives@gitlab.com>
Co-authored-by: Nao Hashizume <nhashizume@gitlab.com>

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

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

db/migrate/20240416123401_add_security_policy_management_project_id_to_security_policies.rb

-# 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

db/migrate/20240416123402_add_security_policy_management_project_id_fk_to_security_policies.rb

-# 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

db/migrate/20240416123403_add_index_security_policy_management_project_id_on_security_policies.rb

-# 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

db/migrate/20240416123404_add_security_policy_management_project_id_to_approval_policy_rules.rb

-# 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

db/migrate/20240416123405_add_security_policy_management_project_id_fk_to_approval_policy_rules.rb

-# 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

db/migrate/20240416123406_add_index_security_policy_management_project_id_on_approval_policy_rules.rb

-# 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

db/schema_migrations/20240416123401

-5961e034705392a82a709fccddc32d2cbbde016d0c0b1db3f66af1be0573928b
\ No newline at end of file

db/schema_migrations/20240416123402

-17716005af88da4cb7905faeba87e31462f50f61eef25f671b338c4cb01025d6
\ No newline at end of file

db/schema_migrations/20240416123403

-f5c9b34ef88af798bc6f6918d9a04aef29377d9c9f4d07a488f890cb8d1527c2
\ No newline at end of file

db/schema_migrations/20240416123404

-7eacbbbe4f7e0e0fc2cc15369558126d03e6a8dd58eafc7281eb79fd4d0aa80f
\ No newline at end of file

db/schema_migrations/20240416123405

-295e692b5ada0d84cf4ba1b64f6e56237e2d09e1be3cb0674b61ab238884bb2c
\ No newline at end of file

db/schema_migrations/20240416123406

-50b095186f8ebaa709f7918307b77224ea35ebb56bd9c8e3fe5031ed977af1be
\ No newline at end of file

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;

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
 

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

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

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

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