Merge branch '472353-transition-dast-site-profiles' into 'master'

Transition dast_site_profiles to gitlab_sec schema

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



Merged-by: Andy Schoenen <asoiron@gitlab.com>
Approved-by: John Mason <9717668-johnmason@users.noreply.gitlab.com>
Approved-by: Andy Schoenen <asoiron@gitlab.com>
Approved-by: Dominic Bauer <dbauer@gitlab.com>
Approved-by: Jan Provaznik <jprovaznik@gitlab.com>
Co-authored-by: Arpit Gogia <12347103-arpitgogia@users.noreply.gitlab.com>

config/gitlab_loose_foreign_keys.yml

@@ -208,6 +208,10 @@ dast_scanner_profiles_builds:
   - table: p_ci_builds
     column: ci_build_id
     on_delete: async_delete
+dast_site_profiles:
+  - table: projects
+    column: project_id
+    on_delete: async_delete
 dast_site_profiles_builds:
   - table: ci_builds
     column: ci_build_id

db/docs/dast_site_profiles.yml

@@ -7,7 +7,7 @@ feature_categories:
 description: A site profile describes the attributes of a web site to scan on demand with DAST
 introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/36659
 milestone: '13.2'
-gitlab_schema: gitlab_main_cell
+gitlab_schema: gitlab_sec
 allow_cross_foreign_keys:
 - gitlab_main_clusterwide
 sharding_key:

db/post_migrate/20240819102932_remove_projects_dast_site_profiles_project_id_fk.rb

+# frozen_string_literal: true
+
+class RemoveProjectsDastSiteProfilesProjectIdFk < Gitlab::Database::Migration[2.2]
+  milestone '17.3'
+  disable_ddl_transaction!
+
+  FOREIGN_KEY_NAME = "fk_rails_83e309d69e"
+
+  def up
+    with_lock_retries do
+      remove_foreign_key_if_exists(:dast_site_profiles, :projects,
+        name: FOREIGN_KEY_NAME, reverse_lock_order: true)
+    end
+  end
+
+  def down
+    add_concurrent_foreign_key(:dast_site_profiles, :projects,
+      name: FOREIGN_KEY_NAME, column: :project_id,
+      target_column: :id, on_delete: :cascade)
+  end
+end

db/schema_migrations/20240819102932

+3ad9103f070e812b9ec7246e78f6fc440189565afbf7adb8f67f84e1fc20d388
\ No newline at end of file

db/structure.sql

@@ -35517,9 +35517,6 @@ ALTER TABLE ONLY cluster_enabled_grants
 ALTER TABLE ONLY virtual_registries_packages_maven_registry_upstreams
     ADD CONSTRAINT fk_rails_838d054752 FOREIGN KEY (group_id) REFERENCES namespaces(id) ON DELETE CASCADE;
-ALTER TABLE ONLY dast_site_profiles
-    ADD CONSTRAINT fk_rails_83e309d69e FOREIGN KEY (project_id) REFERENCES projects(id) ON DELETE CASCADE;
-
 ALTER TABLE ONLY dependency_list_export_parts
     ADD CONSTRAINT fk_rails_83f26c0e6f FOREIGN KEY (dependency_list_export_id) REFERENCES dependency_list_exports(id) ON DELETE CASCADE;

ee/app/models/dast_site_profile.rb

 # frozen_string_literal: true
 
-class DastSiteProfile < ApplicationRecord
+class DastSiteProfile < Gitlab::Database::SecApplicationRecord
   API_SECRETS_KEYS = [Dast::SiteProfileSecretVariable::PASSWORD,
                       Dast::SiteProfileSecretVariable::REQUEST_HEADERS].freeze
 
@@ -23,7 +23,11 @@ class DastSiteProfile < ApplicationRecord
   validate :excluded_urls_contains_valid_strings
   validate :scan_file_path_contains_valid_url
 
-  scope :with_dast_site_and_validation, -> { includes(dast_site: :dast_site_validation) }
+  scope :with_dast_site_and_validation, -> do
+    includes(dast_site: :dast_site_validation)
+      .allow_cross_joins_across_databases(url: 'https://gitlab.com/gitlab-org/gitlab/-/issues/480014')
+  end
+
   scope :with_name, ->(name) { where(name: name) }
   scope :with_project_id, ->(project_id) { where(project_id: project_id) }
   scope :with_project, -> { includes(:project) }

ee/app/services/app_sec/dast/site_profiles/create_service.rb

@@ -17,20 +17,25 @@ def initialize(errors)
         def execute(name:, target_url:, **params)
           return ServiceResponse.error(message: _('Insufficient permissions')) unless allowed?
 
-          ApplicationRecord.transaction do
-            @dast_site = ::AppSec::Dast::Sites::FindOrCreateService.new(project, current_user).execute!(url: target_url)
-            params.merge!(project: project, dast_site: dast_site, name: name).compact!
+          Gitlab::Database::QueryAnalyzers::PreventCrossDatabaseModification
+            .allow_cross_database_modification_within_transaction(
+              url: 'https://gitlab.com/gitlab-org/gitlab/-/issues/480014') do
+            ApplicationRecord.transaction do
+              @dast_site = ::AppSec::Dast::Sites::FindOrCreateService.new(project,
+                current_user).execute!(url: target_url)
+              params.merge!(project: project, dast_site: dast_site, name: name).compact!
 
-            @dast_site_validation = find_existing_dast_site_validation
-            associate_dast_site_validation! if dast_site_validation
+              @dast_site_validation = find_existing_dast_site_validation
+              associate_dast_site_validation! if dast_site_validation
 
-            @dast_site_profile = DastSiteProfile.create!(params.except(:request_headers, :auth_password))
-            create_secret_variable!(::Dast::SiteProfileSecretVariable::PASSWORD, params[:auth_password])
-            create_secret_variable!(::Dast::SiteProfileSecretVariable::REQUEST_HEADERS, params[:request_headers])
+              @dast_site_profile = DastSiteProfile.create!(params.except(:request_headers, :auth_password))
+              create_secret_variable!(::Dast::SiteProfileSecretVariable::PASSWORD, params[:auth_password])
+              create_secret_variable!(::Dast::SiteProfileSecretVariable::REQUEST_HEADERS, params[:request_headers])
 
-            create_audit_event
+              create_audit_event
 
-            ServiceResponse.success(payload: dast_site_profile)
+              ServiceResponse.success(payload: dast_site_profile)
+            end
           end
         rescue Rollback => e
           ServiceResponse.error(message: e.errors)

ee/app/services/app_sec/dast/site_profiles/update_service.rb

@@ -21,29 +21,33 @@ def execute(id:, **params)
 
           return ServiceResponse.error(message: _('Cannot modify %{profile_name} referenced in security policy') % { profile_name: dast_site_profile.name }) if referenced_in_security_policy?
 
-          ApplicationRecord.transaction do
-            auditor = AppSec::Dast::SiteProfiles::Audit::UpdateService.new(project, current_user, {
-              dast_site_profile: dast_site_profile,
-              new_params: params.dup,
-              old_params: dast_site_profile.attributes.symbolize_keys.merge(
-                target_url: dast_site_profile.dast_site.url
-              )
-            })
-
-            remove_secret_variables! if should_remove_secret_variables?(params)
-
-            if target_url = params.delete(:target_url)
-              params[:dast_site] = AppSec::Dast::Sites::FindOrCreateService.new(project, current_user).execute!(url: target_url)
+          Gitlab::Database::QueryAnalyzers::PreventCrossDatabaseModification
+            .allow_cross_database_modification_within_transaction(
+              url: 'https://gitlab.com/gitlab-org/gitlab/-/issues/480014') do
+            ApplicationRecord.transaction do
+              auditor = AppSec::Dast::SiteProfiles::Audit::UpdateService.new(project, current_user, {
+                dast_site_profile: dast_site_profile,
+                new_params: params.dup,
+                old_params: dast_site_profile.attributes.symbolize_keys.merge(
+                  target_url: dast_site_profile.dast_site.url
+                )
+              })
+
+              remove_secret_variables! if should_remove_secret_variables?(params)
+
+              if target_url = params.delete(:target_url)
+                params[:dast_site] = AppSec::Dast::Sites::FindOrCreateService.new(project, current_user).execute!(url: target_url)
+              end
+
+              handle_secret_variable!(params, :request_headers, ::Dast::SiteProfileSecretVariable::REQUEST_HEADERS)
+              handle_secret_variable!(params, :auth_password, ::Dast::SiteProfileSecretVariable::PASSWORD)
+
+              params.compact!
+              dast_site_profile.update!(params)
+              auditor.execute
+
+              ServiceResponse.success(payload: dast_site_profile)
             end
-
-            handle_secret_variable!(params, :request_headers, ::Dast::SiteProfileSecretVariable::REQUEST_HEADERS)
-            handle_secret_variable!(params, :auth_password, ::Dast::SiteProfileSecretVariable::PASSWORD)
-
-            params.compact!
-            dast_site_profile.update!(params)
-            auditor.execute
-
-            ServiceResponse.success(payload: dast_site_profile)
           end
         rescue Rollback => e
           ServiceResponse.error(message: e.errors)

ee/spec/lib/gitlab/database/desired_sharding_key_spec.rb

@@ -87,6 +87,9 @@
         },
         'dast_site_profiles_builds' => {
           'dast_site_profiles' => 'https://gitlab.com/gitlab-org/gitlab/-/issues/477706'
+        },
+        'dast_site_profile_secret_variables' => {
+          'dast_site_profiles' => 'https://gitlab.com/gitlab-org/gitlab/-/issues/480014'
         }
       }
     end

ee/spec/models/dast_site_profile_spec.rb

@@ -656,4 +656,11 @@
       end
     end
   end
+
+  context 'with loose foreign key on dast_site_profiles.project_id' do
+    it_behaves_like 'cleanup by a loose foreign key' do
+      let_it_be(:parent) { create(:project) }
+      let_it_be(:model) { create(:dast_site_profile, project: parent) }
+    end
+  end
 end

spec/lib/gitlab/database/no_cross_db_foreign_keys_spec.rb

@@ -72,7 +72,8 @@
       'dast_site_profiles.dast_site_id',
       'dast_site_profiles_builds.dast_site_profile_id',
       'dast_site_validations.dast_site_token_id',
-      'dast_sites.dast_site_validation_id'
+      'dast_sites.dast_site_validation_id',
+      'dast_site_profile_secret_variables.dast_site_profile_id'
     ]
   end
 

spec/support/shared_examples/lib/gitlab/background_migration/backfill_desired_sharding_key_shared_examples.rb

@@ -27,7 +27,11 @@
       vulnerability_flags: {
         vulnerability_occurrences: 'https://gitlab.com/gitlab-org/gitlab/-/issues/480354'
       },
-      dast_site_validations: { dast_site_tokens: 'https://gitlab.com/gitlab-org/gitlab/-/issues/474985' }
+      dast_site_validations: { dast_site_tokens: 'https://gitlab.com/gitlab-org/gitlab/-/issues/474985' },
+      dast_site_profile_secret_variables: {
+        dast_site_profiles: 'https://gitlab.com/gitlab-org/gitlab/-/issues/480014'
+      },
+      dast_site_profiles_builds: { dast_site_profiles: 'https://gitlab.com/gitlab-org/gitlab/-/issues/480014' }
     }
   end