Add latest changes from gitlab-org/gitlab@master

app/controllers/concerns/lfs_request.rb

@@ -116,6 +116,10 @@ module LfsRequest
     @objects ||= (params[:objects] || []).to_a
   end
+  def objects_oids
+    objects.map { |o| o['oid'].to_s }
+  end
+
   def has_authentication_ability?(capability)
     (authentication_abilities || []).include?(capability)
   end

app/controllers/repositories/lfs_api_controller.rb

@@ -45,15 +45,9 @@ module Repositories
       params[:operation] == 'upload'
     end
-    # rubocop: disable CodeReuse/ActiveRecord
-    def existing_oids
-      @existing_oids ||= begin
-        project.all_lfs_objects.where(oid: objects.map { |o| o['oid'].to_s }).pluck(:oid)
-      end
-    end
-    # rubocop: enable CodeReuse/ActiveRecord
-
     def download_objects!
+      existing_oids = project.all_lfs_objects_oids(oids: objects_oids)
+
       objects.each do |object|
         if existing_oids.include?(object[:oid])
           object[:actions] = download_actions(object)
@@ -68,13 +62,17 @@ module Repositories
           }
         end
       end
+
       objects
     end
     def upload_objects!
+      existing_oids = project.lfs_objects_oids(oids: objects_oids)
+
       objects.each do |object|
         object[:actions] = upload_actions(object) unless existing_oids.include?(object[:oid])
       end
+
       objects
     end

app/models/project.rb

@@ -1398,12 +1398,17 @@ class Project < ApplicationRecord
       .where(lfs_objects_projects: { project_id: [self, lfs_storage_project] })
   end
-  # TODO: Call `#lfs_objects` instead once all LfsObjectsProject records are
-  # backfilled. At that point, projects can look at their own `lfs_objects`.
+  # TODO: Remove this method once all LfsObjectsProject records are backfilled
+  # for forks. At that point, projects can look at their own `lfs_objects` so
+  # `lfs_objects_oids` can be used instead.
   #
   # See https://gitlab.com/gitlab-org/gitlab/issues/122002 for more info.
-  def lfs_objects_oids
-    all_lfs_objects.pluck(:oid)
+  def all_lfs_objects_oids(oids: [])
+    oids(all_lfs_objects, oids: oids)
+  end
+
+  def lfs_objects_oids(oids: [])
+    oids(lfs_objects, oids: oids)
   end
   def personal?
@@ -2515,6 +2520,12 @@ class Project < ApplicationRecord
     reset
     retry
   end
+
+  def oids(objects, oids: [])
+    collection = oids.any? ? objects.where(oid: oids) : objects
+
+    collection.pluck(:oid)
+  end
 end
 Project.prepend_if_ee('EE::Project')

app/models/user.rb

@@ -60,6 +60,7 @@ class User < ApplicationRecord
   MINIMUM_INACTIVE_DAYS = 180
   enum bot_type: ::UserBotTypeEnums.bots
+  enum user_type: ::UserTypeEnums.types
   # Override Devise::Models::Trackable#update_tracked_fields!
   # to limit database writes to at most once every hour
@@ -336,7 +337,7 @@ class User < ApplicationRecord
   scope :with_dashboard, -> (dashboard) { where(dashboard: dashboard) }
   scope :with_public_profile, -> { where(private_profile: false) }
   scope :bots, -> { where.not(bot_type: nil) }
-  scope :humans, -> { where(bot_type: nil) }
+  scope :humans, -> { where(user_type: nil, bot_type: nil) }
   scope :with_expiring_and_not_notified_personal_access_tokens, ->(at) do
     where('EXISTS (?)',

app/models/user_bot_type_enums.rb

@@ -2,7 +2,9 @@
 module UserBotTypeEnums
   def self.bots
-    # When adding a new key, please ensure you are not conflicting with EE-only keys in app/models/user_bot_type_enums.rb
+    # When adding a new key, please ensure you are not conflicting
+    # with EE-only keys in app/models/user_type_enums.rb
+    # or app/models/user_bot_type_enums.rb
     {
       alert_bot: 2
     }

app/models/user_type_enums.rb

+# frozen_string_literal: true
+
+module UserTypeEnums
+  def self.types
+    # When adding a new key, please ensure you are not conflicting
+    # with EE-only keys in app/models/user_type_enums.rb
+    # or app/models/user_bot_type_enums.rb
+    bots
+  end
+
+  def self.bots
+    {
+      AlertBot: 2
+    }
+  end
+end
+
+UserTypeEnums.prepend_if_ee('EE::UserTypeEnums')

app/workers/repository_fork_worker.rb

@@ -52,7 +52,7 @@ class RepositoryForkWorker # rubocop:disable Scalability/IdempotentWorker
   def link_lfs_objects(source_project, target_project)
     Projects::LfsPointers::LfsLinkService
         .new(target_project)
-        .execute(source_project.lfs_objects_oids)
+        .execute(source_project.all_lfs_objects_oids)
   rescue Projects::LfsPointers::LfsLinkService::TooManyOidsError
     raise_fork_failure(
       source_project,

changelogs/unreleased/207216-lfs-batch-upload-fix.yml

+---
+title: Mark existing LFS object for upload for forks
+merge_request: 26344
+author:
+type: fixed

db/migrate/20200304085423_add_user_type.rb

+# frozen_string_literal: true
+
+class AddUserType < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def up
+    with_lock_retries do
+      add_column :users, :user_type, :integer, limit: 2
+    end
+  end
+
+  def down
+    with_lock_retries do
+      remove_column :users, :user_type
+    end
+  end
+end

db/migrate/20200304090155_add_user_type_index.rb

+# frozen_string_literal: true
+
+class AddUserTypeIndex < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def up
+    add_concurrent_index :users, :user_type
+  end
+
+  def down
+    remove_concurrent_index :users, :user_type
+  end
+end

db/schema.rb

@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
-ActiveRecord::Schema.define(version: 2020_03_03_074328) do
+ActiveRecord::Schema.define(version: 2020_03_04_090155) do
   # These are extensions that must be enabled in order to support this database
   enable_extension "pg_trgm"
@@ -4325,6 +4325,7 @@ ActiveRecord::Schema.define(version: 2020_03_03_074328) do
     t.string "last_name", limit: 255
     t.string "static_object_token", limit: 255
     t.integer "role", limit: 2
+    t.integer "user_type", limit: 2
     t.index "lower((name)::text)", name: "index_on_users_name_lower"
     t.index ["accepted_term_id"], name: "index_users_on_accepted_term_id"
     t.index ["admin"], name: "index_users_on_admin"
@@ -4347,6 +4348,7 @@ ActiveRecord::Schema.define(version: 2020_03_03_074328) do
     t.index ["state"], name: "index_users_on_state_and_internal_ee", where: "((ghost IS NOT TRUE) AND (bot_type IS NULL))"
     t.index ["static_object_token"], name: "index_users_on_static_object_token", unique: true
     t.index ["unconfirmed_email"], name: "index_users_on_unconfirmed_email", where: "(unconfirmed_email IS NOT NULL)"
+    t.index ["user_type"], name: "index_users_on_user_type"
     t.index ["username"], name: "index_users_on_username"
     t.index ["username"], name: "index_users_on_username_trigram", opclass: :gin_trgm_ops, using: :gin
   end

doc/api/group_clusters.md

@@ -160,7 +160,7 @@ Parameters:
 | `managed` | boolean | no | Determines if GitLab will manage namespaces and service accounts for this cluster, defaults to true |
 | `platform_kubernetes_attributes[api_url]` | string | yes | The URL to access the Kubernetes API |
 | `platform_kubernetes_attributes[token]` | string | yes | The token to authenticate against Kubernetes |
-| `platform_kubernetes_attributes[ca_cert]` | string | no | TLS certificate (needed if API is using a self-signed TLS certificate |
+| `platform_kubernetes_attributes[ca_cert]` | string | no | TLS certificate. Required if API is using a self-signed TLS certificate. |
 | `platform_kubernetes_attributes[authorization_type]` | string | no | The cluster authorization type: `rbac`, `abac` or `unknown_authorization`. Defaults to `rbac`. |
 | `environment_scope` | string | no | The associated environment to the cluster. Defaults to `*` **(PREMIUM)** |
@@ -227,7 +227,7 @@ Parameters:
 | `domain` | string | no | The [base domain](../user/group/clusters/index.md#base-domain) of the cluster |
 | `platform_kubernetes_attributes[api_url]` | string | no | The URL to access the Kubernetes API |
 | `platform_kubernetes_attributes[token]` | string | no | The token to authenticate against Kubernetes |
-| `platform_kubernetes_attributes[ca_cert]` | string | no | TLS certificate (needed if API is using a self-signed TLS certificate |
+| `platform_kubernetes_attributes[ca_cert]` | string | no | TLS certificate. Required if API is using a self-signed TLS certificate. |
 | `environment_scope` | string | no | The associated environment to the cluster **(PREMIUM)** |
 NOTE: **Note:**

doc/development/geo/framework.md

@@ -142,3 +142,212 @@ ActiveRecord hooks:
 The framework behind all this is located in
 [`ee/lib/gitlab/geo/`](https://gitlab.com/gitlab-org/gitlab/-/tree/master/ee/lib/gitlab/geo).
+
+## Existing Replicator Strategies
+
+Before writing a new kind of Replicator Strategy, check below to see if your
+resource can already be handled by one of the existing strategies. Consult with
+the Geo team if you are unsure.
+
+### Blob Replicator Strategy
+
+Models that use
+[CarrierWave's](https://github.com/carrierwaveuploader/carrierwave) `Uploader::Base`
+can be easily supported by Geo with the `Geo::BlobReplicatorStrategy` module.
+
+First, each file should have its own primary ID and model. Geo strongly
+recommends treating *every single file* as a first-class citizen, because in
+our experience this greatly simplifies tracking replication and verification
+state.
+
+For example, to add support for files referenced by a `Widget` model with a
+`widgets` table, you would perform the following steps:
+
+1. Add verification state fields to the `widgets` table so the Geo primary can
+   track verification state:
+
+   ```ruby
+   # frozen_string_literal: true
+
+   class AddVerificationStateToWidgets < ActiveRecord::Migration[6.0]
+     DOWNTIME = false
+
+     def change
+       add_column :widgets, :verification_retry_at,  :datetime_with_timezone
+       add_column :widgets, :last_verification_ran_at,  :datetime_with_timezone
+       add_column :widgets, :verification_checksum, :string
+       add_column :widgets, :verification_failure, :string
+       add_column :widgets, :verification_retry_count, :integer
+     end
+   end
+   ```
+
+1. Add a partial index on `verification_failure` to ensure re-verification can
+   be performed efficiently:
+
+   ```ruby
+   # frozen_string_literal: true
+
+   class AddVerificationFailureIndexToWidgets < ActiveRecord::Migration[6.0]
+     include Gitlab::Database::MigrationHelpers
+
+     DOWNTIME = false
+
+     disable_ddl_transaction!
+
+     def up
+       add_concurrent_index :widgets, :verification_failure, where: "(verification_failure IS NOT NULL)", name: "widgets_verification_failure_partial"
+     end
+
+     def down
+       remove_concurrent_index :widgets, :verification_failure
+     end
+   end
+   ```
+
+1. Include `Gitlab::Geo::ReplicableModel` in the `Widget` class, and specify
+   the Replicator class `with_replicator Geo::WidgetReplicator`.
+
+   At this point the `Widget` class should look like this:
+
+   ```ruby
+   # frozen_string_literal: true
+
+   class Widget < ApplicationRecord
+     include ::Gitlab::Geo::ReplicableModel
+
+     with_replicator Geo::WidgetReplicator
+
+     mount_uploader :file, WidgetUploader
+
+     ...
+   end
+   ```
+
+1. Create `ee/app/replicators/geo/widget_replicator.rb`. Implement the
+   `#carrierwave_uploader` method which should return a `CarrierWave::Uploader`.
+   And implement the private `#model` method to return the `Widget` class.
+
+   ```ruby
+   # frozen_string_literal: true
+
+   module Geo
+     class WidgetReplicator < Gitlab::Geo::Replicator
+       include ::Geo::BlobReplicatorStrategy
+
+       def carrierwave_uploader
+         model_record.file
+       end
+
+       private
+
+       def model
+         ::Widget
+       end
+     end
+   end
+   ```
+
+1. Create `ee/spec/replicators/geo/widget_replicator_spec.rb` and perform
+   the setup necessary to define the `model_record` variable for the shared
+   examples.
+
+   ```ruby
+   # frozen_string_literal: true
+
+   require 'spec_helper'
+
+   describe Geo::WidgetReplicator do
+     let(:model_record) { build(:widget) }
+
+     it_behaves_like 'a blob replicator'
+   end
+   ```
+
+1. Create the `widget_registry` table so Geo secondaries can track the sync and
+   verification state of each Widget's file:
+
+   ```ruby
+   # frozen_string_literal: true
+
+   class CreateWidgetRegistry < ActiveRecord::Migration[5.2]
+     DOWNTIME = false
+
+     def change
+       create_table :widget_registry, id: :serial, force: :cascade do |t|
+         t.integer :widget_id, null: false
+         t.integer :state, default: 0, null: false
+         t.integer :retry_count, default: 0
+         t.string :last_sync_failure, limit: 255
+         t.datetime_with_timezone :retry_at
+         t.datetime_with_timezone :last_synced_at
+         t.datetime_with_timezone :created_at, null: false
+
+         t.index :widget_id, name:  :index_widget_registry_on_repository_id, using: :btree
+         t.index :retry_at, name: :index_widget_registry_on_retry_at,  using: :btree
+         t.index :state, name: :index_widget_registry_on_state, using:  :btree
+       end
+     end
+   end
+   ```
+
+1. Create `ee/app/models/geo/widget_registry.rb`:
+
+   ```ruby
+   # frozen_string_literal: true
+
+   class Geo::WidgetRegistry < Geo::BaseRegistry
+     include Geo::StateMachineRegistry
+
+     belongs_to :widget, class_name: 'Widget'
+   end
+   ```
+
+1. Create `ee/spec/factories/geo/widget_registry.rb`:
+
+   ```ruby
+   # frozen_string_literal: true
+
+   FactoryBot.define do
+     factory :widget_registry, class: 'Geo::WidgetRegistry' do
+       widget
+       state { Geo::WidgetRegistry.state_value(:pending) }
+
+       trait :synced do
+         state { Geo::WidgetRegistry.state_value(:synced) }
+         last_synced_at { 5.days.ago }
+       end
+
+       trait :failed do
+         state { Geo::WidgetRegistry.state_value(:failed) }
+         last_synced_at { 1.day.ago }
+         retry_count { 2 }
+         last_sync_failure { 'Random error' }
+       end
+
+       trait :started do
+         state { Geo::WidgetRegistry.state_value(:started) }
+         last_synced_at { 1.day.ago }
+         retry_count { 0 }
+       end
+     end
+   end
+   ```
+
+1. Create `ee/spec/models/geo/widget_registry.rb`:
+
+   ```ruby
+   # frozen_string_literal: true
+
+   require 'spec_helper'
+
+   describe Geo::WidgetRegistry, :geo, type: :model do
+     let_it_be(:registry) { create(:widget_registry) }
+
+     specify 'factory is valid' do
+       expect(registry).to be_valid
+     end
+   end
+   ```
+
+Widget files should now be replicated and verified by Geo!

doc/install/aws/index.md

@@ -293,11 +293,11 @@ On the EC2 dashboard, look for Load Balancer in the left navigation bar:
 1. Click the **Create Load Balancer** button.
    1. Choose the **Classic Load Balancer**.
-   1. Give it a name (`gitlab-loadbalancer`) and for the **Create LB Inside** option, select `gitlab-vpc` from the dropdown menu.
+   1. Give it a name (we'll use `gitlab-loadbalancer`) and for the **Create LB Inside** option, select `gitlab-vpc` from the dropdown menu.
    1. In the **Listeners** section, set HTTP port 80, HTTPS port 443, and TCP port 22 for both load balancer and instance protocols and ports.
    1. In the **Select Subnets** section, select both public subnets from the list.
 1. Click **Assign Security Groups** and select **Create a new security group**, give it a name
-   (`gitlab-loadbalancer-sec-group`) and description, and allow both HTTP and HTTPS traffic
+   (we'll use `gitlab-loadbalancer-sec-group`) and description, and allow both HTTP and HTTPS traffic
    from anywhere (`0.0.0.0/0, ::/0`).
 1. Click **Configure Security Settings** and select an SSL/TLS certificate from ACM or upload a certificate to IAM.
 1. Click **Configure Health Check** and set up a health check for your EC2 instances.

spec/models/project_spec.rb

@@ -5692,6 +5692,53 @@ describe Project do
     end
   end
+  describe '#all_lfs_objects_oids' do
+    let(:project) { create(:project) }
+    let(:lfs_object) { create(:lfs_object) }
+    let(:another_lfs_object) { create(:lfs_object) }
+
+    subject { project.all_lfs_objects_oids }
+
+    context 'when project has associated LFS objects' do
+      before do
+        create(:lfs_objects_project, lfs_object: lfs_object, project: project)
+        create(:lfs_objects_project, lfs_object: another_lfs_object, project: project)
+      end
+
+      it 'returns OIDs of LFS objects' do
+        expect(subject).to match_array([lfs_object.oid, another_lfs_object.oid])
+      end
+
+      context 'and there are specified oids' do
+        subject { project.all_lfs_objects_oids(oids: [lfs_object.oid]) }
+
+        it 'returns OIDs of LFS objects that match specified oids' do
+          expect(subject).to eq([lfs_object.oid])
+        end
+      end
+    end
+
+    context 'when fork has associated LFS objects to itself and source' do
+      let(:source) { create(:project) }
+      let(:project) { fork_project(source) }
+
+      before do
+        create(:lfs_objects_project, lfs_object: lfs_object, project: source)
+        create(:lfs_objects_project, lfs_object: another_lfs_object, project: project)
+      end
+
+      it 'returns OIDs of LFS objects' do
+        expect(subject).to match_array([lfs_object.oid, another_lfs_object.oid])
+      end
+    end
+
+    context 'when project has no associated LFS objects' do
+      it 'returns empty array' do
+        expect(subject).to be_empty
+      end
+    end
+  end
+
   describe '#lfs_objects_oids' do
     let(:project) { create(:project) }
     let(:lfs_object) { create(:lfs_object) }
@@ -5708,6 +5755,14 @@ describe Project do
       it 'returns OIDs of LFS objects' do
         expect(subject).to match_array([lfs_object.oid, another_lfs_object.oid])
       end
+
+      context 'and there are specified oids' do
+        subject { project.lfs_objects_oids(oids: [lfs_object.oid]) }
+
+        it 'returns OIDs of LFS objects that match specified oids' do
+          expect(subject).to eq([lfs_object.oid])
+        end
+      end
     end
     context 'when project has no associated LFS objects' do

spec/requests/lfs_http_spec.rb

@@ -690,22 +690,34 @@ describe 'Git LFS API and storage' do
           end
           context 'when pushing an LFS object that already exists' do
+            shared_examples_for 'batch upload with existing LFS object' do
+              it_behaves_like 'LFS http 200 response'
+
+              it 'responds with links the object to the project' do
+                expect(json_response['objects']).to be_kind_of(Array)
+                expect(json_response['objects'].first).to include(sample_object)
+                expect(lfs_object.projects.pluck(:id)).not_to include(project.id)
+                expect(lfs_object.projects.pluck(:id)).to include(other_project.id)
+                expect(json_response['objects'].first['actions']['upload']['href']).to eq(objects_url(project, sample_oid, sample_size))
+                expect(json_response['objects'].first['actions']['upload']['header']).to include('Content-Type' => 'application/octet-stream')
+              end
+
+              it_behaves_like 'process authorization header', renew_authorization: true
+            end
+
             let(:update_lfs_permissions) do
               other_project.lfs_objects << lfs_object
             end
-            it_behaves_like 'LFS http 200 response'
-
-            it 'responds with links the object to the project' do
-              expect(json_response['objects']).to be_kind_of(Array)
-              expect(json_response['objects'].first).to include(sample_object)
-              expect(lfs_object.projects.pluck(:id)).not_to include(project.id)
-              expect(lfs_object.projects.pluck(:id)).to include(other_project.id)
-              expect(json_response['objects'].first['actions']['upload']['href']).to eq(objects_url(project, sample_oid, sample_size))
-              expect(json_response['objects'].first['actions']['upload']['header']).to include('Content-Type' => 'application/octet-stream')
+            context 'in another project' do
+              it_behaves_like 'batch upload with existing LFS object'
             end
-            it_behaves_like 'process authorization header', renew_authorization: true
+            context 'in source of fork project' do
+              let(:project) { fork_project(other_project) }
+
+              it_behaves_like 'batch upload with existing LFS object'
+            end
           end
           context 'when pushing a LFS object that does not exist' do

spec/support/shared_examples/models/concerns/blob_replicator_strategy_shared_examples.rb

+# frozen_string_literal: true
+
+# Include these shared examples in specs of Replicators that include
+# BlobReplicatorStrategy.
+#
+# A let variable called model_record should be defined in the spec. It should be
+# a valid, unpersisted instance of the model class.
+#
+RSpec.shared_examples 'a blob replicator' do
+  include EE::GeoHelpers
+
+  let_it_be(:primary) { create(:geo_node, :primary) }
+  let_it_be(:secondary) { create(:geo_node) }
+
+  subject(:replicator) { model_record.replicator }
+
+  before do
+    stub_current_geo_node(primary)
+  end
+
+  describe '#handle_after_create_commit' do
+    it 'creates a Geo::Event' do
+      expect do
+        replicator.handle_after_create_commit
+      end.to change { ::Geo::Event.count }.by(1)
+
+      expect(::Geo::Event.last.attributes).to include(
+        "replicable_name" => replicator.replicable_name, "event_name" => "created", "payload" => { "model_record_id" => replicator.model_record.id })
+    end
+  end
+
+  describe '#consume_created_event' do
+    it 'invokes Geo::BlobDownloadService' do
+      service = double(:service)
+
+      expect(service).to receive(:execute)
+      expect(::Geo::BlobDownloadService).to receive(:new).with(replicator: replicator).and_return(service)
+
+      replicator.consume_created_event
+    end
+  end
+
+  describe '#carrierwave_uploader' do
+    it 'is implemented' do
+      expect do
+        replicator.carrierwave_uploader
+      end.not_to raise_error
+    end
+  end
+
+  describe '#model' do
+    let(:invoke_model) { replicator.send(:model) }
+
+    it 'is implemented' do
+      expect do
+        invoke_model
+      end.not_to raise_error
+    end
+
+    it 'is a Class' do
+      expect(invoke_model).to be_a(Class)
+    end
+
+    # For convenience (and reliability), instead of asking developers to include shared examples on each model spec as well
+    context 'replicable model' do
+      it 'defines #replicator' do
+        expect(model_record).to respond_to(:replicator)
+      end
+
+      it 'invokes replicator.handle_after_create_commit on create' do
+        expect(replicator).to receive(:handle_after_create_commit)
+
+        model_record.save!
+      end
+    end
+  end
+end

spec/workers/repository_fork_worker_spec.rb

@@ -83,7 +83,7 @@ describe RepositoryForkWorker do
       it 'calls Projects::LfsPointers::LfsLinkService#execute with OIDs of source project LFS objects' do
         expect_fork_repository.and_return(true)
         expect_next_instance_of(Projects::LfsPointers::LfsLinkService) do |service|
-          expect(service).to receive(:execute).with(project.lfs_objects_oids)
+          expect(service).to receive(:execute).with(project.all_lfs_objects_oids)
         end
         perform!