Automatic merge of gitlab-org/gitlab master

danger/product_intelligence/Dangerfile

@@ -15,8 +15,8 @@ MSG
 product_intelligence_paths_to_review = helper.changes_by_category[:product_intelligence]
 labels_to_add = product_intelligence.missing_labels
 
-return if product_intelligence_paths_to_review.empty? || labels_to_add.empty?
+return if product_intelligence_paths_to_review.empty?
 
-warn format(CHANGED_FILES_MESSAGE, changed_files: helper.markdown_list(product_intelligence_paths_to_review))
+warn format(CHANGED_FILES_MESSAGE, changed_files: helper.markdown_list(product_intelligence_paths_to_review)) unless product_intelligence.has_approved_label?
 
-project_helper.labels_to_add.concat(labels_to_add)
+project_helper.labels_to_add.concat(labels_to_add) unless labels_to_add.empty?

db/fixtures/development/02_users.rb

@@ -41,11 +41,12 @@ class Gitlab::Seeder::Users
     relation = User.where(admin: false)
     Gitlab::Seeder.with_mass_insert(relation.count, Namespace) do
       ActiveRecord::Base.connection.execute <<~SQL
-        INSERT INTO namespaces (name, path, owner_id)
+        INSERT INTO namespaces (name, path, owner_id, type)
         SELECT
           username,
           username,
-          id
+          id,
+          'User'
         FROM users WHERE NOT admin
       SQL
     end

db/post_migrate/20211101222614_consume_remaining_user_namespace_jobs.rb

+# frozen_string_literal: true
+
+class ConsumeRemainingUserNamespaceJobs < Gitlab::Database::Migration[1.0]
+  MIGRATION = 'BackfillUserNamespace'
+  BATCH_SIZE = 200
+  DEFAULT_VALUE = 'User'
+
+  disable_ddl_transaction!
+
+  def up
+    Gitlab::BackgroundMigration.steal(MIGRATION)
+
+    # Do a manual update in case we lost BG jobs. The expected record count should be 0 or very low.
+    define_batchable_model('namespaces').where(type: nil).each_batch(of: BATCH_SIZE) do |batch|
+      min, max = batch.pluck('MIN(id), MAX(id)').flatten
+
+      Gitlab::BackgroundMigration::BackfillUserNamespace.new.perform(min, max, :namespaces, :id, BATCH_SIZE, 0)
+    end
+
+    change_column_null :namespaces, :type, false
+  end
+
+  def down
+    change_column_null :namespaces, :type, true
+  end
+end

db/schema_migrations/20211101222614

+a579b14aff1d186d89173e383442f2ffbd69b1baed3f9a4c758fbb001b445139
\ No newline at end of file

db/structure.sql

@@ -16419,7 +16419,7 @@ CREATE TABLE namespaces (
     owner_id integer,
     created_at timestamp without time zone,
     updated_at timestamp without time zone,
-    type character varying DEFAULT 'User'::character varying,
+    type character varying DEFAULT 'User'::character varying NOT NULL,
     description character varying DEFAULT ''::character varying NOT NULL,
     avatar character varying,
     membership_lock boolean DEFAULT false,

doc/user/search/advanced_search.md

@@ -52,13 +52,13 @@ The Advanced Search can be useful in various scenarios:
 
 ## Use the Advanced Search syntax
 
-Elasticsearch has only data for the default branch. That means that if you go
+Elasticsearch has data for the default branch only. That means that if you go
 to the repository tree and switch the branch from the default to something else,
-then the "Code" tab in the search result page will be served by the basic
+then the **Code** tab in the search result page is served by the basic
 search even if Elasticsearch is enabled.
 
 The Advanced Search syntax supports fuzzy or exact search queries with prefixes,
-boolean operators, and much more. Use the search as before and GitLab will show
+boolean operators, and much more. Use the search as before and GitLab shows
 you matching code from each project you have access to.
 
 ![Advanced Search](img/advanced_search_v13.10.png)
@@ -67,7 +67,7 @@ Full details can be found in the [Elasticsearch documentation](https://www.elast
 here's a quick guide:
 
 - Searches look for all the words in a query, in any order - for example: searching
-  issues for [`display bug`](https://gitlab.com/search?snippets=&scope=issues&repository_ref=&search=display+bug&group_id=9970&project_id=278964) and [`bug display`](https://gitlab.com/search?snippets=&scope=issues&repository_ref=&search=bug+Display&group_id=9970&project_id=278964) will return the same results.
+  issues for [`display bug`](https://gitlab.com/search?snippets=&scope=issues&repository_ref=&search=display+bug&group_id=9970&project_id=278964) and [`bug display`](https://gitlab.com/search?snippets=&scope=issues&repository_ref=&search=bug+Display&group_id=9970&project_id=278964) return the same results.
 - To find the exact phrase (stemming still applies), use double quotes: [`"display bug"`](https://gitlab.com/search?snippets=&scope=issues&repository_ref=&search=%22display+bug%22&group_id=9970&project_id=278964)
 - To find bugs not mentioning display, use `-`: [`bug -display`](https://gitlab.com/search?snippets=&scope=issues&repository_ref=&search=bug+-display&group_id=9970&project_id=278964)
 - To find a bug in display or banner, use `|`: [`bug display | banner`](https://gitlab.com/search?snippets=&scope=issues&repository_ref=&search=bug+display+%7C+banner&group_id=9970&project_id=278964)
@@ -85,7 +85,7 @@ Advanced Search also supports the use of filters. The available filters are:
 - `blob`: Filters by Git `object ID`. Exact match only.
 
 To use them, add them to your keyword in the format `<filter_name>:<value>` without
-any spaces between the colon (`:`) and the value. When no keyword is provided, an asterisk (`*`) will be used as the keyword.
+any spaces between the colon (`:`) and the value. When no keyword is provided, an asterisk (`*`) is used as the keyword.
 
 Examples:
 

ee/spec/helpers/billing_plans_helper_spec.rb

@@ -167,7 +167,7 @@ RSpec.describe BillingPlansHelper, :saas do
   end
 
   describe '#use_new_purchase_flow?' do
-    where type: [Group.sti_name, nil],
+    where type: [Group.sti_name, Namespaces::UserNamespace.sti_name],
       plan: Plan.all_plans,
       trial_active: [true, false]
 
@@ -345,7 +345,7 @@ RSpec.describe BillingPlansHelper, :saas do
     end
   end
 
-  describe "#plan_purchase_or_upgrade_url" do
+  describe '#plan_purchase_or_upgrade_url' do
     let(:plan) { double('Plan') }
 
     it 'is upgradable' do
@@ -364,7 +364,7 @@ RSpec.describe BillingPlansHelper, :saas do
     end
   end
 
-  describe "#plan_purchase_url" do
+  describe '#plan_purchase_url' do
     let_it_be(:group) { create(:group) }
     let_it_be(:user) { create(:user) }
 
@@ -390,7 +390,7 @@ RSpec.describe BillingPlansHelper, :saas do
     end
   end
 
-  describe "#hand_raise_props" do
+  describe '#hand_raise_props' do
     let_it_be(:namespace) { create(:namespace) }
     let_it_be(:user) { create(:user, username: 'Joe', first_name: 'Joe', last_name: 'Doe', organization: 'ACME') }
 

ee/spec/lib/ee/gitlab/background_migration/populate_uuids_for_security_findings_spec.rb

@@ -23,7 +23,7 @@ RSpec.describe ::Gitlab::BackgroundMigration::PopulateUuidsForSecurityFindings,
   let(:fingerprint_4) { Digest::SHA1.hexdigest(SecureRandom.uuid) }
 
   let(:user) { users.create!(email: 'test@gitlab.com', projects_limit: 5) }
-  let(:namespace) { namespaces.create!(name: 'gitlab', path: 'gitlab-org') }
+  let(:namespace) { namespaces.create!(name: 'gitlab', path: 'gitlab-org', type: Namespaces::UserNamespace.sti_name) }
   let(:project) { projects.create!(namespace_id: namespace.id, name: 'foo') }
   let(:ci_pipeline) { ci_pipelines.create!(project_id: project.id, ref: 'master', sha: 'adf43c3a', status: 'success') }
   let(:ci_build_1) { ci_builds.create!(commit_id: ci_pipeline.id, retried: false, type: 'Ci::Build') }

ee/spec/lib/ee/gitlab/background_migration/update_vulnerability_occurrences_location_spec.rb

@@ -4,7 +4,7 @@ require 'spec_helper'
 
 RSpec.describe Gitlab::BackgroundMigration::UpdateVulnerabilityOccurrencesLocation, schema: 20211102114802 do
   let_it_be(:namespaces) { table(:namespaces) }
-  let_it_be(:group) { namespaces.create!(name: 'foo', path: 'foo') }
+  let_it_be(:group) { namespaces.create!(name: 'foo', path: 'foo', type: Group.sti_name) }
   let_it_be(:projects) { table(:projects) }
   let_it_be(:findings) { table(:vulnerability_occurrences) }
   let_it_be(:scanners) { table(:vulnerability_scanners) }

ee/spec/migrations/update_vulnerability_occurrences_location_spec.rb

@@ -7,7 +7,7 @@ RSpec.describe UpdateVulnerabilityOccurrencesLocation, :migration do
   let(:migration_name) { 'UpdateVulnerabilityOccurrencesLocation' }
 
   let_it_be(:namespaces) { table(:namespaces) }
-  let_it_be(:group) { namespaces.create!(name: 'foo', path: 'foo') }
+  let_it_be(:group) { namespaces.create!(name: 'foo', path: 'foo', type: Group.sti_name) }
   let_it_be(:projects) { table(:projects) }
   let_it_be(:findings) { table(:vulnerability_occurrences) }
   let_it_be(:scanners) { table(:vulnerability_scanners) }

lib/bulk_imports/projects/pipelines/repository_pipeline.rb

@@ -16,6 +16,8 @@ module BulkImports
 
         def load(context, data)
           url = data['httpUrlToRepo']
+          return unless url.present?
+
           url = url.sub("://", "://oauth2:#{context.configuration.access_token}@")
           project = context.portable
 

lib/tasks/gitlab/update_templates.rake

@@ -32,7 +32,7 @@ namespace :gitlab do
 
     tmp_namespace_path = "tmp-project-import-#{Time.now.to_i}"
     puts "Creating temporary namespace #{tmp_namespace_path}"
-    tmp_namespace = Namespace.create!(owner: admin, name: tmp_namespace_path, path: tmp_namespace_path)
+    tmp_namespace = Namespace.create!(owner: admin, name: tmp_namespace_path, path: tmp_namespace_path, type: Namespaces::UserNamespace.sti_name)
 
     templates = if template_names.empty?
                   Gitlab::ProjectTemplate.all

spec/lib/bulk_imports/projects/pipelines/repository_pipeline_spec.rb

@@ -47,6 +47,17 @@ RSpec.describe BulkImports::Projects::Pipelines::RepositoryPipeline do
       end
     end
 
+    context 'project has no repository' do
+      let(:project_data) { { 'httpUrlToRepo' => '' } }
+
+      it 'skips repository import' do
+        expect(context.portable).not_to receive(:ensure_repository)
+        expect(context.portable.repository).not_to receive(:fetch_as_mirror)
+
+        pipeline.run
+      end
+    end
+
     context 'blocked local networks' do
       let(:project_data) { { 'httpUrlToRepo' => 'http://localhost/foo.git' } }
 

spec/lib/gitlab/background_migration/drop_invalid_security_findings_spec.rb

@@ -2,7 +2,7 @@
 require 'spec_helper'
 
 RSpec.describe Gitlab::BackgroundMigration::DropInvalidSecurityFindings, schema: 20211108211434 do
-  let(:namespace) { table(:namespaces).create!(name: 'user', path: 'user') }
+  let(:namespace) { table(:namespaces).create!(name: 'user', path: 'user', type: Namespaces::UserNamespace.sti_name) }
   let(:project) { table(:projects).create!(namespace_id: namespace.id) }
 
   let(:pipelines) { table(:ci_pipelines) }

spec/lib/gitlab/background_migration/remove_vulnerability_finding_links_spec.rb

@@ -6,7 +6,7 @@ RSpec.describe Gitlab::BackgroundMigration::RemoveVulnerabilityFindingLinks, :mi
   let(:vulnerability_findings) { table(:vulnerability_occurrences) }
   let(:finding_links) { table(:vulnerability_finding_links) }
 
-  let(:namespace) { table(:namespaces).create!(name: 'user', path: 'user') }
+  let(:namespace) { table(:namespaces).create!(name: 'user', path: 'user', type: Namespaces::UserNamespace.sti_name) }
   let(:project) { table(:projects).create!(namespace_id: namespace.id) }
   let(:scanner) { table(:vulnerability_scanners).create!(project_id: project.id, external_id: 'scanner', name: 'scanner') }
   let(:vulnerability_identifier) do

spec/lib/gitlab/database/migration_helpers_spec.rb

@@ -2431,7 +2431,7 @@ RSpec.describe Gitlab::Database::MigrationHelpers do
     let(:issues)         { table(:issues) }
 
     def setup
-      namespace = namespaces.create!(name: 'foo', path: 'foo')
+      namespace = namespaces.create!(name: 'foo', path: 'foo', type: Namespaces::UserNamespace.sti_name)
       projects.create!(namespace_id: namespace.id)
     end
 

spec/migrations/20211101222614_consume_remaining_user_namespace_jobs_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+require_migration!
+
+RSpec.describe ConsumeRemainingUserNamespaceJobs do
+  let(:namespaces) { table(:namespaces) }
+  let!(:namespace) { namespaces.create!(name: 'gitlab', path: 'gitlab-org', type: nil) }
+
+  context 'when Namespaces with nil `type` still exist' do
+    it 'steals sidekiq jobs from BackfillUserNamespace background migration' do
+      expect(Gitlab::BackgroundMigration).to receive(:steal).with('BackfillUserNamespace')
+
+      migrate!
+    end
+
+    it 'migrates namespaces without type' do
+      expect { migrate! }.to change { namespaces.where(type: 'User').count }.from(0).to(1)
+    end
+  end
+end

spec/migrations/20211110151350_schedule_drop_invalid_security_findings_spec.rb

@@ -6,7 +6,7 @@ require_migration!
 RSpec.describe ScheduleDropInvalidSecurityFindings, :migration, schema: 20211108211434 do
   let_it_be(:background_migration_jobs) { table(:background_migration_jobs) }
 
-  let_it_be(:namespace) { table(:namespaces).create!(name: 'user', path: 'user') }
+  let_it_be(:namespace) { table(:namespaces).create!(name: 'user', path: 'user', type: Namespaces::UserNamespace.sti_name) }
   let_it_be(:project) { table(:projects).create!(namespace_id: namespace.id) }
 
   let_it_be(:pipelines) { table(:ci_pipelines) }

spec/migrations/20211116091751_change_namespace_type_default_to_user_spec.rb

 # frozen_string_literal: true
 
-require 'spec_helper'
-
-require_migration!
-
-RSpec.describe ChangeNamespaceTypeDefaultToUser do
-  let(:namespaces) { table(:namespaces) }
-
-  it 'defaults type to User' do
-    some_namespace1 = namespaces.create!(name: 'magic namespace1', path: 'magicnamespace1', type: nil)
-
-    expect(some_namespace1.reload.type).to be_nil
-
-    migrate!
-
-    some_namespace2 = namespaces.create!(name: 'magic namespace2', path: 'magicnamespace2', type: nil)
-
-    expect(some_namespace1.reload.type).to be_nil
-    expect(some_namespace2.reload.type).to eq 'User'
-  end
-end
+# With https://gitlab.com/gitlab-org/gitlab/-/merge_requests/73495, we no longer allow
+# a Namespace type to be nil. There is nothing left to test for this migration,
+# but we'll keep this file here as a tombstone.

spec/models/concerns/case_sensitivity_spec.rb

@@ -13,8 +13,8 @@ RSpec.describe CaseSensitivity do
       end
     end
 
-    let_it_be(:model_1) { model.create!(path: 'mOdEl-1', name: 'mOdEl 1') }
-    let_it_be(:model_2) { model.create!(path: 'mOdEl-2', name: 'mOdEl 2') }
+    let_it_be(:model_1) { model.create!(path: 'mOdEl-1', name: 'mOdEl 1', type: Namespaces::UserNamespace.sti_name) }
+    let_it_be(:model_2) { model.create!(path: 'mOdEl-2', name: 'mOdEl 2', type: Group.sti_name) }
 
     it 'finds a single instance by a single attribute regardless of case' do
       expect(model.iwhere(path: 'MODEL-1')).to contain_exactly(model_1)

spec/models/namespace_spec.rb

@@ -259,13 +259,12 @@ RSpec.describe Namespace do
       end
     end
 
-    context 'creating a Namespace with nil type' do
+    context 'unable to create a Namespace with nil type' do
+      let(:namespace) { nil }
       let(:namespace_type) { nil }
 
-      it 'is the correct type of namespace' do
-        expect(Namespace.find(namespace.id)).to be_a(Namespace)
-        expect(namespace.kind).to eq('user')
-        expect(namespace.user_namespace?).to be_truthy
+      it 'raises ActiveRecord::NotNullViolation' do
+        expect { create(:namespace, type: namespace_type, parent: parent) }.to raise_error(ActiveRecord::NotNullViolation)
       end
     end
 

tooling/danger/product_intelligence.rb

@@ -4,9 +4,12 @@
 module Tooling
   module Danger
     module ProductIntelligence
+      APPROVED_LABEL = 'product intelligence::approved'
+      REVIEW_LABEL = 'product intelligence::review pending'
+
       WORKFLOW_LABELS = [
-        'product intelligence::approved',
-        'product intelligence::review pending'
+        APPROVED_LABEL,
+        REVIEW_LABEL
       ].freeze
 
       def missing_labels
@@ -14,11 +17,15 @@ module Tooling
 
         labels = []
         labels << 'product intelligence' unless helper.mr_has_labels?('product intelligence')
-        labels << 'product intelligence::review pending' unless has_workflow_labels?
+        labels << REVIEW_LABEL unless has_workflow_labels?
 
         labels
       end
 
+      def has_approved_label?
+        helper.mr_labels.include?(APPROVED_LABEL)
+      end
+
       private
 
       def has_workflow_labels?