Merge branch 'sh-fix-geo-file-transfers' into 'master'

Geo: Fix attachments/avatars saving to the wrong directory

Closes #3634

See merge request gitlab-org/gitlab-ee!3068
# Conflicts:
#	doc/gitlab-geo/updating_the_geo_nodes.md

changelogs/unreleased-ee/sh-fix-geo-file-transfers.yml

+---
+title: 'Geo: Fix attachments/avatars saving to the wrong directory'
+merge_request:
+author:
+type: fixed

db/geo/migrate/20171005045404_remove_file_uploads_from_registry.rb

+class RemoveFileUploadsFromRegistry < ActiveRecord::Migration
+  # Previous to GitLab 10.1, GitLab would save attachments/avatars to the
+  # wrong directory (/var/opt/gitlab/gitlab-rails/working). Destroy these
+  # entries so they will be downloaded again.
+  def up
+    Geo::BaseRegistry.connection.execute("DELETE FROM file_registry WHERE file_type != 'lfs'")
+  end
+end

db/geo/schema.rb

@@ -11,7 +11,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
-ActiveRecord::Schema.define(version: 20170906182752) do
+ActiveRecord::Schema.define(version: 20171005045404) do
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"

doc/development/geo.md

@@ -141,7 +141,7 @@ The database configuration is set in `config/database_geo.yml`.
 To write a migration for the database, use the `GeoMigrationGenerator`:
 ```
-rails g geo_generator [args] [options]
+rails g geo_generation [args] [options]
 ```
 To migrate the tracking database, run:

doc/gitlab-geo/updating_the_geo_nodes.md

@@ -14,6 +14,45 @@ all you need to do is update GitLab itself:
    the tracking database is enabled.
 1. [Test](#check-status-after-updating) primary and secondary nodes, and check version in each.
+## Upgrading to GitLab 10.0
+
+Since GitLab 10.0, we require all **Geo** systems to [use SSH key lookups via
+the database](ssh.md) to avoid having to maintain consistency of the
+`authorized_keys` file for SSH access. Failing to do this will prevent users
+from being able to clone via SSH.
+
+Note that in older versions of Geo, attachments downloaded on the secondary
+nodes would be saved to the wrong directory. We recommend that you do the
+following to clean this up.
+
+On the SECONDARY Geo nodes, run as root:
+
+```sh
+mv /var/opt/gitlab/gitlab-rails/working /var/opt/gitlab/gitlab-rails/working.old
+mkdir /var/opt/gitlab/gitlab-rails/working
+chmod 700 /var/opt/gitlab/gitlab-rails/working
+chown git:git /var/opt/gitlab/gitlab-rails/working
+```
+
+You may delete `/var/opt/gitlab/gitlab-rails/working.old` any time.
+
+## Upgrading from GitLab 9.3 or older
+
+If you started running Geo on GitLab 9.3 or older, we recommend that you
+resync your secondary PostgreSQL databases to use replication slots. If you
+started using Geo with GitLab 9.4 or 10.x, no further action should be
+required because replication slots are used by default. However, if you
+started with GitLab 9.3 and upgraded later, you should still follow the
+instructions below.
+
+When in doubt, it does not hurt to do a resync. The easiest way to do this in
+Omnibus is the following:
+
+  1. Install GitLab on the primary server
+  1. Run `gitlab-ctl reconfigure` and `gitlab-ctl restart postgresql`. This will enable replication slots on the primary database.
+  1. Install GitLab on the secondary server.
+  1. Re-run the [database replication process](database.md#step-3-initiate-the-replication-process).
+
 ## Special update notes for 9.0.x
 > **IMPORTANT**:

lib/gitlab/geo/file_transfer.rb

@@ -2,9 +2,11 @@ module Gitlab
   module Geo
     class FileTransfer < Transfer
       def initialize(file_type, upload)
+        uploader = upload.uploader.constantize
+
         @file_type = file_type
         @file_id = upload.id
-        @filename = upload.path
+        @filename = uploader.absolute_path(upload)
         @request_data = build_request_data(upload)
       end

spec/lib/gitlab/geo/file_transfer_spec.rb

+require 'spec_helper'
+
+describe Gitlab::Geo::FileTransfer do
+  let(:user) { create(:user, avatar: fixture_file_upload(Rails.root + 'spec/fixtures/dk.png', 'image/png')) }
+  let(:upload) { Upload.find_by(model: user, uploader: 'AvatarUploader') }
+
+  subject { described_class.new(:file, upload) }
+
+  describe '#execute' do
+    context 'user avatar' do
+      it 'sets an absolute path' do
+        expect(subject.file_type).to eq(:file)
+        expect(subject.file_id).to eq(upload.id)
+        expect(subject.filename).to eq(AvatarUploader.absolute_path(upload))
+        expect(Pathname.new(subject.filename).absolute?).to be_truthy
+        expect(subject.request_data).to eq({ id: upload.id,
+                                             type: 'User',
+                                             checksum: upload.checksum })
+      end
+    end
+  end
+end