Merge branch 'zj-object-store-artifacts' into 'master'

Object store for artifacts

Closes gitlab-ce#29203

See merge request !1762

app/models/ci/build.rb

@@ -295,17 +295,27 @@ module Ci
       !artifacts_expired? && artifacts_file.exists?
     end
+    def browsable_artifacts?
+      artifacts_metadata?
+    end
+
+    def downloadable_single_artifacts_file?
+      artifacts_metadata? && artifacts_file.file_storage?
+    end
+
     def artifacts_metadata?
       artifacts? && artifacts_metadata.exists?
     end
     def artifacts_metadata_entry(path, **options)
-      metadata = Gitlab::Ci::Build::Artifacts::Metadata.new(
-        artifacts_metadata.path,
-        path,
-        **options)
-      metadata.to_entry
+      artifacts_metadata.use_file do |metadata_path|
+        metadata = Gitlab::Ci::Build::Artifacts::Metadata.new(
+          metadata_path,
+          path,
+          **options)
+        metadata.to_entry
+      end
     end
     def erase_artifacts!

app/services/projects/update_pages_service.rb

@@ -65,9 +65,9 @@ module Projects
     end
     def extract_archive!(temp_path)
-      if artifacts.ends_with?('.tar.gz') || artifacts.ends_with?('.tgz')
+      if artifacts_filename.ends_with?('.tar.gz') || artifacts_filename.ends_with?('.tgz')
         extract_tar_archive!(temp_path)
-      elsif artifacts.ends_with?('.zip')
+      elsif artifacts_filename.ends_with?('.zip')
         extract_zip_archive!(temp_path)
       else
         raise 'unsupported artifacts format'
@@ -75,11 +75,13 @@ module Projects
     end
     def extract_tar_archive!(temp_path)
-      results = Open3.pipeline(%W(gunzip -c #{artifacts}),
-                               %W(dd bs=#{BLOCK_SIZE} count=#{blocks}),
-                               %W(tar -x -C #{temp_path} #{SITE_PATH}),
-                               err: '/dev/null')
-      raise 'pages failed to extract' unless results.compact.all?(&:success?)
+      build.artifacts_file.use_file do |artifacts_path|
+        results = Open3.pipeline(%W(gunzip -c #{artifacts_path}),
+                                %W(dd bs=#{BLOCK_SIZE} count=#{blocks}),
+                                %W(tar -x -C #{temp_path} #{SITE_PATH}),
+                                err: '/dev/null')
+        raise 'pages failed to extract' unless results.compact.all?(&:success?)
+      end
     end
     def extract_zip_archive!(temp_path)
@@ -97,8 +99,10 @@ module Projects
       # -n  never overwrite existing files
       # We add * to end of SITE_PATH, because we want to extract SITE_PATH and all subdirectories
       site_path = File.join(SITE_PATH, '*')
-      unless system(*%W(unzip -qq -n #{artifacts} #{site_path} -d #{temp_path}))
-        raise 'pages failed to extract'
+      build.artifacts_file.use_file do |artifacts_path|
+        unless system(*%W(unzip -n #{artifacts_path} #{site_path} -d #{temp_path}))
+          raise 'pages failed to extract'
+        end
       end
     end
@@ -129,6 +133,10 @@ module Projects
       1 + max_size / BLOCK_SIZE
     end
+    def artifacts_filename
+      build.artifacts_file.filename
+    end
+
     def max_size
       current_application_settings.max_pages_size.megabytes || MAX_SIZE
     end
@@ -153,10 +161,6 @@ module Projects
       build.ref
     end
-    def artifacts
-      build.artifacts_file.path
-    end
-
     def latest_sha
       project.commit(build.ref).try(:sha).to_s
     end

app/uploaders/artifact_uploader.rb

-class ArtifactUploader < GitlabUploader
-  storage :file
-
-  attr_reader :job, :field
+class ArtifactUploader < ObjectStoreUploader
+  storage_options Gitlab.config.artifacts
   def self.local_artifacts_store
     Gitlab.config.artifacts.path
@@ -11,12 +9,12 @@ class ArtifactUploader < GitlabUploader
     File.join(self.local_artifacts_store, 'tmp/uploads/')
   end
-  def initialize(job, field)
-    @job, @field = job, field
-  end
-
   def store_dir
-    default_local_path
+    if file_storage?
+      default_local_path
+    else
+      default_path
+    end
   end
   def cache_dir
@@ -34,6 +32,6 @@ class ArtifactUploader < GitlabUploader
   end
   def default_path
-    File.join(job.created_at.utc.strftime('%Y_%m'), job.project_id.to_s, job.id.to_s)
+    File.join(subject.created_at.utc.strftime('%Y_%m'), subject.project_id.to_s, subject.id.to_s)
   end
 end

app/uploaders/object_store_uploader.rb

+require 'fog/aws'
+require 'carrierwave/storage/fog'
+
+class ObjectStoreUploader < GitlabUploader
+  before :store, :set_default_local_store
+  before :store, :verify_license!
+
+  LOCAL_STORE = 1
+  REMOTE_STORE = 2
+
+  class << self
+    def storage_options(options)
+      @storage_options = options
+    end
+
+    def object_store_options
+      @storage_options&.object_store
+    end
+
+    def object_store_enabled?
+      object_store_options&.enabled
+    end
+  end
+
+  attr_reader :subject, :field
+
+  def initialize(subject, field)
+    @subject = subject
+    @field = field
+  end
+
+  def object_store
+    subject.public_send(:"#{field}_store")
+  end
+
+  def object_store=(value)
+    @storage = nil
+    subject.public_send(:"#{field}_store=", value)
+  end
+
+  def use_file
+    if file_storage?
+      return yield path
+    end
+
+    begin
+      cache_stored_file!
+      yield cache_path
+    ensure
+      cache_storage.delete_dir!(cache_path(nil))
+    end
+  end
+
+  def filename
+    super || file&.filename
+  end
+
+  def migrate!(new_store)
+    raise 'Undefined new store' unless new_store
+
+    return unless object_store != new_store
+    return unless file
+
+    old_file = file
+    old_store = object_store
+
+    # for moving remote file we need to first store it locally
+    cache_stored_file! unless file_storage?
+
+    # change storage
+    self.object_store = new_store
+
+    storage.store!(file).tap do |new_file|
+      # since we change storage store the new storage
+      # in case of failure delete new file
+      begin
+        subject.save!
+      rescue => e
+        new_file.delete
+        self.object_store = old_store
+        raise e
+      end
+
+      old_file.delete
+    end
+  end
+
+  def fog_directory
+    self.class.object_store_options.remote_directory
+  end
+
+  def fog_credentials
+    self.class.object_store_options.connection
+  end
+
+  def fog_public
+    false
+  end
+
+  def move_to_store
+    file.try(:storage) == storage
+  end
+
+  def move_to_cache
+    file.try(:storage) == cache_storage
+  end
+
+  # We block storing artifacts on Object Storage, not receiving
+  def verify_license!(new_file)
+    return if file_storage?
+
+    raise 'Object Storage feature is missing' unless subject.project.feature_available?(:object_storage)
+  end
+
+  private
+
+  def set_default_local_store(new_file)
+    self.object_store = LOCAL_STORE unless self.object_store
+  end
+
+  def storage
+    @storage ||=
+      if object_store == REMOTE_STORE
+        remote_storage
+      else
+        local_storage
+      end
+  end
+
+  def remote_storage
+    raise 'Object Storage is not enabled' unless self.class.object_store_enabled?
+
+    CarrierWave::Storage::Fog.new(self)
+  end
+
+  def local_storage
+    CarrierWave::Storage::File.new(self)
+  end
+end

app/views/projects/artifacts/_tree_file.html.haml

-- path_to_file = file_project_job_artifacts_path(@project, @build, path: file.path)
+- path_to_file = file_namespace_project_job_artifacts_path(@project.namespace, @project, @build, path: file.path) if @build.downloadable_single_artifacts_file?
 %tr.tree-item{ 'data-link' => path_to_file }
   - blob = file.blob
   %td.tree-item-file-name
     = tree_icon('file', blob.mode, blob.name)
-    = link_to path_to_file do
-      %span.str-truncated= blob.name
+    %span.str-truncated
+      - if path_to_file
+        = link_to file.name, path_to_file
+      - else
+        = file.name
   %td
     = number_to_human_size(blob.size, precision: 2)

app/views/projects/jobs/_sidebar.html.haml

@@ -32,8 +32,8 @@
             = link_to download_project_job_artifacts_path(@project, @build), rel: 'nofollow', download: '', class: 'btn btn-sm btn-default' do
               Download
-            - if @build.artifacts_metadata?
-              = link_to browse_project_job_artifacts_path(@project, @build), class: 'btn btn-sm btn-default' do
+            - if @build.browsable_artifacts?
+              = link_to browse_namespace_project_job_artifacts_path(@project.namespace, @project, @build), class: 'btn btn-sm btn-default' do
                 Browse
     - if @build.trigger_request

changelogs/unreleased-ee/allow-to-store-artifacts-on-object-storage.yml

+---
+title: Allow to Store Artifacts on Object Storage
+merge_request:
+author:

config/gitlab.yml.example

@@ -138,6 +138,14 @@ production: &base
     enabled: true
     # The location where build artifacts are stored (default: shared/artifacts).
     # path: shared/artifacts
+    # object_store:
+    #   enabled: false
+    #   remote_directory: artifacts
+    #   connection:
+    #     provider: AWS # Only AWS supported at the moment
+    #     aws_access_key_id: AWS_ACCESS_KEY_ID
+    #     aws_secret_access_key: AWS_SECRET_ACCESS_KEY
+    #     region: eu-central-1
   ## Git LFS
   lfs:

config/initializers/1_settings.rb

@@ -268,6 +268,12 @@ Settings.artifacts['enabled']      = true if Settings.artifacts['enabled'].nil?
 Settings.artifacts['path']         = Settings.absolute(Settings.artifacts['path'] || File.join(Settings.shared['path'], "artifacts"))
 Settings.artifacts['max_size']   ||= 100 # in megabytes
+Settings.artifacts['object_store'] ||= Settingslogic.new({})
+Settings.artifacts['object_store']['enabled'] = false if Settings.artifacts['object_store']['enabled'].nil?
+Settings.artifacts['object_store']['remote_directory'] ||= nil
+# Convert upload connection settings to use symbol keys, to make Fog happy
+Settings.artifacts['object_store']['connection']&.deep_symbolize_keys!
+
 #
 # Registry
 #

db/migrate/20170601163708_add_artifacts_store_to_ci_build.rb

+class AddArtifactsStoreToCiBuild < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def up
+    add_column_with_default(:ci_builds, :artifacts_file_store, :integer, default: 1)
+    add_column_with_default(:ci_builds, :artifacts_metadata_store, :integer, default: 1)
+  end
+
+  def down
+    remove_column(:ci_builds, :artifacts_file_store)
+    remove_column(:ci_builds, :artifacts_metadata_store)
+  end
+end

db/schema.rb

@@ -238,6 +238,8 @@ ActiveRecord::Schema.define(version: 20170707184244) do
     t.integer "auto_canceled_by_id"
     t.boolean "retried"
     t.integer "stage_id"
+    t.integer "artifacts_file_store", default: 1, null: false
+    t.integer "artifacts_metadata_store", default: 1, null: false
   end
   add_index "ci_builds", ["auto_canceled_by_id"], name: "index_ci_builds_on_auto_canceled_by_id", using: :btree

doc/administration/job_artifacts.md

@@ -85,12 +85,41 @@ _The artifacts are stored by default in
 1. Save the file and [restart GitLab][] for the changes to take effect.
-### Using object storage
-In [GitLab Enterprise Edition Premium][eep] you can use an object storage like
-AWS S3 to store the artifacts.
-[Learn how to use the object storage option.][ee-os]
+---
+
+**Using Object Store**
+
+The previously mentioned methods use the local disk to store artifacts. However,
+there is the option to use object stores like AWS' S3. To do this, set the
+`object_store` in your `gitlab.yml`. This relies on valid AWS
+credentials to be configured already. 
+    ```yaml
+    artifacts:
+        enabled: true
+        path: /mnt/storage/artifacts
+        object_store:
+          enabled: true
+          remote_directory: my-bucket-name
+          connection:
+            provider: AWS
+            aws_access_key_id: S3_KEY_ID
+            aws_secret_key_id: S3_SECRET_KEY_ID
+            region: eu-central-1
+    ```
+
+This will allow you to migrate existing artifacts to object store,
+but all new artifacts will still be stored on the local disk.
+In the future you will be given an option to define a default storage artifacts
+for all new files. Currently the artifacts migration has to be executed manually:
+
+    ```bash
+    gitlab-rake gitlab:artifacts:migrate
+    ```
+Please note, that enabling this feature
+will have the effect that artifacts are _not_ browsable anymore through the web
+interface. This limitation will be removed in one of the upcoming releases.
 ## Expiring artifacts

lib/api/helpers.rb

@@ -318,7 +318,7 @@ module API
       if artifacts_file.file_storage?
         present_file!(artifacts_file.path, artifacts_file.filename)
       else
-        redirect_to(artifacts_file.url)
+        redirect(artifacts_file.url)
       end
     end

lib/ci/api/builds.rb

@@ -192,7 +192,7 @@ module Ci
           end
           unless artifacts_file.file_storage?
-            return redirect_to build.artifacts_file.url
+            return redirect(build.artifacts_file.url)
           end
           present_file!(artifacts_file.path, artifacts_file.filename)

lib/tasks/gitlab/artifacts.rake

+desc "GitLab | Migrate files for artifacts to comply with new storage format"
+namespace :gitlab do
+  namespace :artifacts do
+    task migrate: :environment do
+      puts 'Artifacts'.color(:yellow)
+      Ci::Build.joins(:project).with_artifacts
+        .where(artifacts_file_store: ArtifactUploader::LOCAL_STORE)
+        .find_each(batch_size: 100) do |issue|
+        begin
+          build.artifacts_file.migrate!(ArtifactUploader::REMOTE_STORE)
+          build.artifacts_metadata.migrate!(ArtifactUploader::REMOTE_STORE)
+          print '.'
+        rescue
+          print 'F'
+        end
+      end
+    end
+  end
+end

spec/factories/ci/builds.rb

@@ -163,6 +163,11 @@ FactoryGirl.define do
       end
     end
+    trait :remote_store do
+      artifacts_file_store ArtifactUploader::REMOTE_STORE
+      artifacts_metadata_store ArtifactUploader::REMOTE_STORE
+    end
+
     trait :artifacts_expired do
       after(:create) do |build, _|
         build.artifacts_file =

spec/lib/gitlab/import_export/safe_model_attributes.yml

@@ -252,7 +252,9 @@ CommitStatus:
 - target_url
 - description
 - artifacts_file
+- artifacts_file_store
 - artifacts_metadata
+- artifacts_metadata_store
 - erased_by_id
 - erased_at
 - artifacts_expire_at

spec/models/ci/build_spec.rb

@@ -124,6 +124,50 @@ describe Ci::Build, :models do
     end
   end
+  describe '#browsable_artifacts?' do
+    subject { build.browsable_artifacts? }
+  
+    context 'artifacts metadata does not exist' do
+      before do
+        build.update_attributes(artifacts_metadata: nil)
+      end
+
+      it { is_expected.to be_falsy }
+    end
+
+    context 'artifacts metadata does exists' do
+      let(:build) { create(:ci_build, :artifacts) }
+
+      it { is_expected.to be_truthy }
+    end
+  end
+
+  describe '#downloadable_single_artifacts_file?' do
+    let(:build) { create(:ci_build, :artifacts, artifacts_file_store: store) }
+
+    subject { build.downloadable_single_artifacts_file? }
+
+    before do
+      expect_any_instance_of(Ci::Build).to receive(:artifacts_metadata?).and_call_original
+    end
+
+    context 'artifacts are stored locally' do
+      let(:store) { ObjectStoreUploader::LOCAL_STORE }
+
+      it { is_expected.to be_truthy }
+    end
+
+    context 'artifacts are stored remotely' do
+      let(:store) { ObjectStoreUploader::REMOTE_STORE }
+
+      before do
+        stub_artifacts_object_storage
+      end
+
+      it { is_expected.to be_falsey }
+    end
+  end
+
   describe '#artifacts_expired?' do
     subject { build.artifacts_expired? }

spec/requests/api/jobs_spec.rb

 require 'spec_helper'
 describe API::Jobs, :api do
-  let!(:project) do
+  let(:project) do
     create(:project, :repository, public_builds: false)
   end
-  let!(:pipeline) do
+  let(:pipeline) do
     create(:ci_empty_pipeline, project: project,
                                sha: project.commit.id,
                                ref: project.default_branch)
   end
-  let!(:job) { create(:ci_build, pipeline: pipeline) }
+  let(:job) { create(:ci_build, pipeline: pipeline) }
   let(:user) { create(:user) }
   let(:api_user) { user }
@@ -26,6 +26,7 @@ describe API::Jobs, :api do
     let(:query) { Hash.new }
     before do
+      job
       get api("/projects/#{project.id}/jobs", api_user), query
     end
@@ -89,6 +90,7 @@ describe API::Jobs, :api do
     let(:query) { Hash.new }
     before do
+      job
       get api("/projects/#{project.id}/pipelines/#{pipeline.id}/jobs", api_user), query
     end
@@ -190,30 +192,41 @@ describe API::Jobs, :api do
   describe 'GET /projects/:id/jobs/:job_id/artifacts' do
     before do
+      stub_artifacts_object_storage
       get api("/projects/#{project.id}/jobs/#{job.id}/artifacts", api_user)
     end
     context 'job with artifacts' do
-      let(:job) { create(:ci_build, :artifacts, pipeline: pipeline) }
-      context 'authorized user' do
-        let(:download_headers) do
-          { 'Content-Transfer-Encoding' => 'binary',
-            'Content-Disposition' => 'attachment; filename=ci_build_artifacts.zip' }
+      context 'when artifacts are stored locally' do
+        let(:job) { create(:ci_build, :artifacts, pipeline: pipeline) }
+        context 'authorized user' do
+          let(:download_headers) do
+            { 'Content-Transfer-Encoding' => 'binary',
+              'Content-Disposition' => 'attachment; filename=ci_build_artifacts.zip' }
+          end
+
+          it 'returns specific job artifacts' do
+            expect(response).to have_http_status(200)
+            expect(response.headers).to include(download_headers)
+            expect(response.body).to match_file(job.artifacts_file.file.file)
+          end
         end
-        it 'returns specific job artifacts' do
-          expect(response).to have_http_status(200)
-          expect(response.headers).to include(download_headers)
-          expect(response.body).to match_file(job.artifacts_file.file.file)
+        context 'unauthorized user' do
+          let(:api_user) { nil }
+
+          it 'does not return specific job artifacts' do
+            expect(response).to have_http_status(401)
+          end
         end
       end
-      context 'unauthorized user' do
-        let(:api_user) { nil }
-        it 'does not return specific job artifacts' do
-          expect(response).to have_http_status(401)
+      context 'when artifacts are stored remotely' do
+        let(:job) { create(:ci_build, :artifacts, :remote_store, pipeline: pipeline) }
+        it 'returns location redirect' do
+          expect(response).to have_http_status(302)
         end
       end
     end
@@ -228,6 +241,7 @@ describe API::Jobs, :api do
     let(:job) { create(:ci_build, :artifacts, pipeline: pipeline) }
     before do
+      stub_artifacts_object_storage
       job.success
     end
@@ -283,14 +297,24 @@ describe API::Jobs, :api do
     context 'find proper job' do
       shared_examples 'a valid file' do
-        let(:download_headers) do
-          { 'Content-Transfer-Encoding' => 'binary',
-            'Content-Disposition' =>
-              "attachment; filename=#{job.artifacts_file.filename}" }
+        context 'when artifacts are stored locally' do
+          let(:download_headers) do
+            { 'Content-Transfer-Encoding' => 'binary',
+              'Content-Disposition' =>
+                "attachment; filename=#{job.artifacts_file.filename}" }
+          end
+
+          it { expect(response).to have_http_status(200) }
+          it { expect(response.headers).to include(download_headers) }
         end
-        it { expect(response).to have_http_status(200) }
-        it { expect(response.headers).to include(download_headers) }
+        context 'when artifacts are stored remotely' do
+          let(:job) { create(:ci_build, :artifacts, :remote_store, pipeline: pipeline) }
+
+          it 'returns location redirect' do
+            expect(response).to have_http_status(302)
+          end
+        end
       end
       context 'with regular branch' do

spec/requests/api/runner_spec.rb

@@ -185,7 +185,7 @@ describe API::Runner do
     let(:project) { create(:empty_project, shared_runners_enabled: false) }
     let(:pipeline) { create(:ci_pipeline_without_jobs, project: project, ref: 'master') }
     let(:runner) { create(:ci_runner) }
-    let!(:job) do
+    let(:job) do
       create(:ci_build, :artifacts, :extended_options,
              pipeline: pipeline, name: 'spinach', stage: 'test', stage_idx: 0, commands: "ls\ndate")
     end
@@ -200,6 +200,7 @@ describe API::Runner do
       let(:user_agent) { 'gitlab-runner 9.0.0 (9-0-stable; go1.7.4; linux/amd64)' }
       before do
+        job
         stub_container_registry_config(enabled: false)
       end
@@ -815,6 +816,7 @@ describe API::Runner do
       let(:file_upload2) { fixture_file_upload(Rails.root + 'spec/fixtures/dk.png', 'image/gif') }
       before do
+        stub_artifacts_object_storage
         job.run!
       end
@@ -1116,15 +1118,26 @@ describe API::Runner do
         context 'when job has artifacts' do
           let(:job) { create(:ci_build, :artifacts) }
-          let(:download_headers) do
-            { 'Content-Transfer-Encoding' => 'binary',
-              'Content-Disposition' => 'attachment; filename=ci_build_artifacts.zip' }
-          end
           context 'when using job token' do
-            it 'download artifacts' do
-              expect(response).to have_http_status(200)
-              expect(response.headers).to include download_headers
+            context 'when artifacts are stored locally' do
+              let(:download_headers) do
+                { 'Content-Transfer-Encoding' => 'binary',
+                  'Content-Disposition' => 'attachment; filename=ci_build_artifacts.zip' }
+              end
+
+              it 'download artifacts' do
+                expect(response).to have_http_status(200)
+                expect(response.headers).to include download_headers
+              end
+            end
+
+            context 'when artifacts are stored remotely' do
+              let(:job) { create(:ci_build, :artifacts, :remote_store) }
+
+              it 'download artifacts' do
+                expect(response).to have_http_status(302)
+              end
             end
           end