Add owner validation for project namespaces

Only user namespaces must have an owner

Changelog: added

app/models/namespace.rb

@@ -57,7 +57,7 @@ class Namespace < ApplicationRecord
   has_one :admin_note, inverse_of: :namespace
   accepts_nested_attributes_for :admin_note, update_only: true
 
-  validates :owner, presence: true, if: ->(n) { n.type.nil? }
+  validates :owner, presence: true, if: ->(n) { n.owner_required? }
   validates :name,
     presence: true,
     length: { maximum: 255 }
@@ -266,6 +266,10 @@ class Namespace < ApplicationRecord
     type.nil? || type == Namespaces::UserNamespace.sti_name || !(group? || project?)
   end
 
+  def owner_required?
+    user?
+  end
+
   def find_fork_of(project)
     return unless project.fork_network
 

app/models/namespaces/project_namespace.rb

@@ -4,6 +4,8 @@ module Namespaces
   class ProjectNamespace < Namespace
     has_one :project, foreign_key: :project_namespace_id, inverse_of: :project_namespace
 
+    validates :project, presence: true
+
     def self.sti_name
       'Project'
     end

app/models/project.rb

@@ -211,6 +211,7 @@ class Project < ApplicationRecord
   has_one :unify_circuit_integration, class_name: 'Integrations::UnifyCircuit'
   has_one :webex_teams_integration, class_name: 'Integrations::WebexTeams'
   has_one :youtrack_integration, class_name: 'Integrations::Youtrack'
+  has_one :zentao_integration, class_name: 'Integrations::Zentao'
 
   has_one :root_of_fork_network,
           foreign_key: 'root_project_id',
@@ -340,6 +341,7 @@ class Project < ApplicationRecord
   # build traces. Currently there's no efficient way of removing this data in
   # bulk that doesn't involve loading the rows into memory. As a result we're
   # still using `dependent: :destroy` here.
+  has_many :pending_builds, class_name: 'Ci::PendingBuild'
   has_many :builds, class_name: 'Ci::Build', inverse_of: :project, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent
   has_many :processables, class_name: 'Ci::Processable', inverse_of: :project
   has_many :build_trace_chunks, class_name: 'Ci::BuildTraceChunk', through: :builds, source: :trace_chunks
@@ -1458,7 +1460,7 @@ class Project < ApplicationRecord
   end
 
   def disabled_integrations
-    []
+    [:zentao]
   end
 
   def find_or_initialize_integration(name)
@@ -1677,6 +1679,10 @@ class Project < ApplicationRecord
     end
   end
 
+  def membership_locked?
+    false
+  end
+
   def bots
     users.project_bot
   end
@@ -1784,6 +1790,9 @@ class Project < ApplicationRecord
     Ci::Runner.from_union([runners, group_runners, available_shared_runners])
   end
 
+  # Once issue 339937 is fixed, please search for all mentioned of
+  # https://gitlab.com/gitlab-org/gitlab/-/issues/339937,
+  # and remove the allow_cross_joins_across_databases.
   def active_runners
     strong_memoize(:active_runners) do
       all_available_runners.active
@@ -1791,7 +1800,9 @@ class Project < ApplicationRecord
   end
 
   def any_online_runners?(&block)
-    online_runners_with_tags.any?(&block)
+    ::Gitlab::Database.allow_cross_joins_across_databases(url: 'https://gitlab.com/gitlab-org/gitlab/-/issues/339937') do
+      online_runners_with_tags.any?(&block)
+    end
   end
 
   def valid_runners_token?(token)
@@ -1800,7 +1811,15 @@ class Project < ApplicationRecord
 
   # rubocop: disable CodeReuse/ServiceClass
   def open_issues_count(current_user = nil)
-    Projects::OpenIssuesCountService.new(self, current_user).count
+    return Projects::OpenIssuesCountService.new(self, current_user).count unless current_user.nil?
+
+    BatchLoader.for(self).batch(replace_methods: false) do |projects, loader|
+      issues_count_per_project = ::Projects::BatchOpenIssuesCountService.new(projects).refresh_cache_and_retrieve_data
+
+      issues_count_per_project.each do |project, count|
+        loader.call(project, count)
+      end
+    end
   end
   # rubocop: enable CodeReuse/ServiceClass
 
@@ -2259,7 +2278,7 @@ class Project < ApplicationRecord
 
   # rubocop: disable CodeReuse/ServiceClass
   def forks_count
-    BatchLoader.for(self).batch do |projects, loader|
+    BatchLoader.for(self).batch(replace_methods: false) do |projects, loader|
       fork_count_per_project = ::Projects::BatchForksCountService.new(projects).refresh_cache_and_retrieve_data
 
       fork_count_per_project.each do |project, count|
@@ -2511,6 +2530,10 @@ class Project < ApplicationRecord
     ci_config_path.blank? || ci_config_path == Gitlab::FileDetector::PATTERNS[:gitlab_ci]
   end
 
+  def uses_external_project_ci_config?
+    !!(ci_config_path =~ %r{@.+/.+})
+  end
+
   def limited_protected_branches(limit)
     protected_branches.limit(limit)
   end
@@ -2619,6 +2642,10 @@ class Project < ApplicationRecord
     repository.gitlab_ci_yml_for(sha, ci_config_path_or_default)
   end
 
+  def ci_config_external_project
+    Project.find_by_full_path(ci_config_path.split('@', 2).last)
+  end
+
   def enabled_group_deploy_keys
     return GroupDeployKey.none unless group
 
@@ -2881,12 +2908,8 @@ class Project < ApplicationRecord
     update_column(:has_external_issue_tracker, integrations.external_issue_trackers.any?) if Gitlab::Database.read_write?
   end
 
-  def active_runners_with_tags
-    @active_runners_with_tags ||= active_runners.with_tags
-  end
-
   def online_runners_with_tags
-    @online_runners_with_tags ||= active_runners_with_tags.online
+    @online_runners_with_tags ||= active_runners.with_tags.online
   end
 end
 

db/migrate/20210824102624_add_project_namespace_index_to_project.rb

-# frozen_string_literal: true
-
-class AddProjectNamespaceIndexToProject < ActiveRecord::Migration[6.1]
-  include Gitlab::Database::MigrationHelpers
-
-  disable_ddl_transaction!
-
-  INDEX_NAME = 'index_projects_on_project_namespace_id'
-
-  def up
-    add_concurrent_index :projects, :project_namespace_id, name: INDEX_NAME, unique: true
-  end
-
-  def down
-    remove_concurrent_index_by_name :projects, INDEX_NAME
-  end
-end

db/migrate/20210824102750_add_project_namespace_foreign_key_to_project.rb

-# frozen_string_literal: true
-
-class AddProjectNamespaceForeignKeyToProject < ActiveRecord::Migration[6.1]
-  include Gitlab::Database::MigrationHelpers
-
-  disable_ddl_transaction!
-
-  TARGET_COLUMN = :project_namespace_id
-
-  def up
-    add_concurrent_foreign_key :projects, :namespaces, column: TARGET_COLUMN, on_delete: :cascade
-  end
-
-  def down
-    with_lock_retries do
-      remove_foreign_key_if_exists(:projects, column: TARGET_COLUMN)
-    end
-  end
-end

spec/models/namespace_spec.rb

@@ -344,6 +344,12 @@ RSpec.describe Namespace do
     end
   end
 
+  describe '#owner_required?' do
+    specify { expect(build(:project_namespace).owner_required?).to be_falsey }
+    specify { expect(build(:group).owner_required?).to be_falsey }
+    specify { expect(build(:namespace).owner_required?).to be_truthy }
+  end
+
   describe '#visibility_level_field' do
     it { expect(namespace.visibility_level_field).to eq(:visibility_level) }
   end

spec/models/namespaces/project_namespace_spec.rb

@@ -7,6 +7,10 @@ RSpec.describe Namespaces::ProjectNamespace, type: :model do
     it { is_expected.to have_one(:project).with_foreign_key(:project_namespace_id).inverse_of(:project_namespace) }
   end
 
+  describe 'validations' do
+    it { is_expected.not_to validate_presence_of :owner }
+  end
+
   context 'when deleting project namespace' do
     # using delete rather than destroy due to `delete` skipping AR hooks/callbacks
     # so it's ensured to work at the DB level. Uses ON DELETE CASCADE on foreign key

spec/models/project_spec.rb

@@ -138,6 +138,7 @@ RSpec.describe Project, factory_default: :keep do
     it { is_expected.to have_many(:timelogs) }
     it { is_expected.to have_many(:error_tracking_errors).class_name('ErrorTracking::Error') }
     it { is_expected.to have_many(:error_tracking_client_keys).class_name('ErrorTracking::ClientKey') }
+    it { is_expected.to have_many(:pending_builds).class_name('Ci::PendingBuild') }
 
     # GitLab Pages
     it { is_expected.to have_many(:pages_domains) }
@@ -617,6 +618,12 @@ RSpec.describe Project, factory_default: :keep do
     end
   end
 
+  describe '#membership_locked?' do
+    it 'returns false' do
+      expect(build(:project)).not_to be_membership_locked
+    end
+  end
+
   describe '#autoclose_referenced_issues' do
     context 'when DB entry is nil' do
       let(:project) { build(:project, autoclose_referenced_issues: nil) }
@@ -1066,12 +1073,12 @@ RSpec.describe Project, factory_default: :keep do
       project.open_issues_count(user)
     end
 
-    it 'invokes the count service with no current_user' do
-      count_service = instance_double(Projects::OpenIssuesCountService)
-      expect(Projects::OpenIssuesCountService).to receive(:new).with(project, nil).and_return(count_service)
-      expect(count_service).to receive(:count)
+    it 'invokes the batch count service with no current_user' do
+      count_service = instance_double(Projects::BatchOpenIssuesCountService)
+      expect(Projects::BatchOpenIssuesCountService).to receive(:new).with([project]).and_return(count_service)
+      expect(count_service).to receive(:refresh_cache_and_retrieve_data).and_return({})
 
-      project.open_issues_count
+      project.open_issues_count.to_s
     end
   end
 
@@ -2555,7 +2562,7 @@ RSpec.describe Project, factory_default: :keep do
   end
 
   describe '#uses_default_ci_config?' do
-    let(:project) { build(:project)}
+    let(:project) { build(:project) }
 
     it 'has a custom ci config path' do
       project.ci_config_path = 'something_custom'
@@ -2576,6 +2583,44 @@ RSpec.describe Project, factory_default: :keep do
     end
   end
 
+  describe '#uses_external_project_ci_config?' do
+    subject(:uses_external_project_ci_config) { project.uses_external_project_ci_config? }
+
+    let(:project) { build(:project) }
+
+    context 'when ci_config_path is configured with external project' do
+      before do
+        project.ci_config_path = '.gitlab-ci.yml@hello/world'
+      end
+
+      it { is_expected.to eq(true) }
+    end
+
+    context 'when ci_config_path is nil' do
+      before do
+        project.ci_config_path = nil
+      end
+
+      it { is_expected.to eq(false) }
+    end
+
+    context 'when ci_config_path is configured with a file in the project' do
+      before do
+        project.ci_config_path = 'hello/world/gitlab-ci.yml'
+      end
+
+      it { is_expected.to eq(false) }
+    end
+
+    context 'when ci_config_path is configured with remote file' do
+      before do
+        project.ci_config_path = 'https://example.org/file.yml'
+      end
+
+      it { is_expected.to eq(false) }
+    end
+  end
+
   describe '#latest_successful_build_for_ref' do
     let_it_be(:project) { create(:project, :repository) }
     let_it_be(:pipeline) { create_pipeline(project) }
@@ -7028,6 +7073,15 @@ RSpec.describe Project, factory_default: :keep do
     end
   end
 
+  describe '#ci_config_external_project' do
+    subject(:ci_config_external_project) { project.ci_config_external_project }
+
+    let(:other_project) { create(:project) }
+    let(:project) { build(:project, ci_config_path: ".gitlab-ci.yml@#{other_project.full_path}") }
+
+    it { is_expected.to eq(other_project) }
+  end
+
   describe '#enabled_group_deploy_keys' do
     let_it_be(:project) { create(:project) }