Implement Build Artifacts

- Offloads uploading to GitLab Workhorse
- Use /authorize request for fast uploading
- Added backup recipes for artifacts
- Support download acceleration using X-Sendfile

2 files changed, 192 insertions, 9 deletions

diff --git a/spec/requests/api/users_spec.rb b/spec/requests/api/users_spec.rb
index d26a300ed82..a9ef2fe5885 100644
--- a/spec/requests/api/users_spec.rb
+++ b/spec/requests/api/users_spec.rb
@@ -343,8 +343,9 @@ describe API::API, api: true  do
       end.to change{ user.keys.count }.by(1)
     end
 
-    it "should raise error for invalid ID" do
-      expect{post api("/users/ASDF/keys", admin) }.to raise_error(ActionController::RoutingError)
+    it "should return 405 for invalid ID" do
+      post api("/users/ASDF/keys", admin)
+      expect(response.status).to eq(405)
     end
   end
 
@@ -374,9 +375,9 @@ describe API::API, api: true  do
         expect(json_response.first['title']).to eq(key.title)
       end
 
-      it "should return 404 for invalid ID" do
+      it "should return 405 for invalid ID" do
         get api("/users/ASDF/keys", admin)
-        expect(response.status).to eq(404)
+        expect(response.status).to eq(405)
       end
     end
   end
@@ -434,7 +435,8 @@ describe API::API, api: true  do
     end
 
     it "should raise error for invalid ID" do
-      expect{post api("/users/ASDF/emails", admin) }.to raise_error(ActionController::RoutingError)
+      post api("/users/ASDF/emails", admin)
+      expect(response.status).to eq(405)
     end
   end
 
@@ -465,7 +467,8 @@ describe API::API, api: true  do
       end
 
       it "should raise error for invalid ID" do
-        expect{put api("/users/ASDF/emails", admin) }.to raise_error(ActionController::RoutingError)
+        put api("/users/ASDF/emails", admin)
+        expect(response.status).to eq(405)
       end
     end
   end
diff --git a/spec/requests/ci/api/builds_spec.rb b/spec/requests/ci/api/builds_spec.rb
index 88218a93e1f..92ea25a3723 100644
--- a/spec/requests/ci/api/builds_spec.rb
+++ b/spec/requests/ci/api/builds_spec.rb
@@ -41,7 +41,7 @@ describe Ci::API::API do
 
       it "should return 404 error if no builds for specific runner" do
         commit = FactoryGirl.create(:ci_commit, gl_project: shared_gl_project)
-        FactoryGirl.create(:ci_build, commit: commit, status: 'pending' )
+        FactoryGirl.create(:ci_build, commit: commit, status: 'pending')
 
         post ci_api("/builds/register"), token: runner.token
 
@@ -50,7 +50,7 @@ describe Ci::API::API do
 
       it "should return 404 error if no builds for shared runner" do
         commit = FactoryGirl.create(:ci_commit, gl_project: gl_project)
-        FactoryGirl.create(:ci_build, commit: commit, status: 'pending' )
+        FactoryGirl.create(:ci_build, commit: commit, status: 'pending')
 
         post ci_api("/builds/register"), token: shared_runner.token
 
@@ -79,7 +79,7 @@ describe Ci::API::API do
           { "key" => "CI_BUILD_NAME", "value" => "spinach", "public" => true },
           { "key" => "CI_BUILD_STAGE", "value" => "test", "public" => true },
           { "key" => "DB_NAME", "value" => "postgres", "public" => true },
-          { "key" => "SECRET_KEY", "value" => "secret_value", "public" => false },
+          { "key" => "SECRET_KEY", "value" => "secret_value", "public" => false }
         ])
       end
 
@@ -122,5 +122,185 @@ describe Ci::API::API do
         expect(build.reload.trace).to eq 'hello_world'
       end
     end
+
+    context "Artifacts" do
+      let(:file_upload) { fixture_file_upload(Rails.root + 'spec/fixtures/banana_sample.gif', 'image/gif') }
+      let(:file_upload2) { fixture_file_upload(Rails.root + 'spec/fixtures/dk.png', 'image/gif') }
+      let(:commit) { FactoryGirl.create(:ci_commit, gl_project: gl_project) }
+      let(:build) { FactoryGirl.create(:ci_build, commit: commit, runner_id: runner.id) }
+      let(:authorize_url) { ci_api("/builds/#{build.id}/artifacts/authorize") }
+      let(:post_url) { ci_api("/builds/#{build.id}/artifacts") }
+      let(:delete_url) { ci_api("/builds/#{build.id}/artifacts") }
+      let(:get_url) { ci_api("/builds/#{build.id}/artifacts") }
+      let(:headers) { { "Gitlab-Git-Http-Server" => "1.0" } }
+      let(:headers_with_token) { headers.merge(Ci::API::Helpers::BUILD_TOKEN_HEADER => build.project.token) }
+
+      describe "POST /builds/:id/artifacts/authorize" do
+        context "should authorize posting artifact to running build" do
+          before do
+            build.run!
+          end
+
+          it "using token as parameter" do
+            post authorize_url, { token: build.project.token }, headers
+            expect(response.status).to eq(200)
+            expect(json_response["temp_path"]).to_not be_nil
+          end
+
+          it "using token as header" do
+            post authorize_url, {}, headers_with_token
+            expect(response.status).to eq(200)
+            expect(json_response["temp_path"]).to_not be_nil
+          end
+        end
+
+        context "should fail to post too large artifact" do
+          before do
+            build.run!
+          end
+
+          it "using token as parameter" do
+            settings = Gitlab::CurrentSettings::current_application_settings
+            settings.update_attributes(max_artifacts_size: 0)
+            post authorize_url, { token: build.project.token, filesize: 100 }, headers
+            expect(response.status).to eq(413)
+          end
+
+          it "using token as header" do
+            settings = Gitlab::CurrentSettings::current_application_settings
+            settings.update_attributes(max_artifacts_size: 0)
+            post authorize_url, { filesize: 100 }, headers_with_token
+            expect(response.status).to eq(413)
+          end
+        end
+
+        context "should get denied" do
+          it do
+            post authorize_url, { token: 'invalid', filesize: 100 }
+            expect(response.status).to eq(403)
+          end
+        end
+      end
+
+      describe "POST /builds/:id/artifacts" do
+        context "Disable sanitizer" do
+          before do
+            # by configuring this path we allow to pass temp file from any path
+            allow(ArtifactUploader).to receive(:artifacts_upload_path).and_return('/')
+          end
+
+          context "should post artifact to running build" do
+            before do
+              build.run!
+            end
+
+            it do
+              upload_artifacts(file_upload, headers_with_token)
+              expect(response.status).to eq(201)
+              expect(json_response["artifacts_file"]["filename"]).to eq(file_upload.original_filename)
+            end
+
+            it "updates artifact" do
+              upload_artifacts(file_upload, headers_with_token)
+              upload_artifacts(file_upload2, headers_with_token)
+              expect(response.status).to eq(201)
+              expect(json_response["artifacts_file"]["filename"]).to eq(file_upload2.original_filename)
+            end
+          end
+
+          context "should fail to post too large artifact" do
+            before do
+              build.run!
+            end
+
+            it do
+              settings = Gitlab::CurrentSettings::current_application_settings
+              settings.update_attributes(max_artifacts_size: 0)
+              upload_artifacts(file_upload, headers_with_token)
+              expect(response.status).to eq(413)
+            end
+          end
+
+          context "should fail to post artifacts without file" do
+            before do
+              build.run!
+            end
+
+            it do
+              post post_url, {}, headers_with_token
+              expect(response.status).to eq(400)
+            end
+          end
+
+          context "should fail to post artifacts without GitLab-Workhorse" do
+            before do
+              build.run!
+            end
+
+            it do
+              post post_url, { token: build.project.token }, {}
+              expect(response.status).to eq(403)
+            end
+          end
+        end
+
+        context "should fail to post artifacts for outside of tmp path" do
+          before do
+            # by configuring this path we allow to pass file from @tmpdir only
+            # but all temporary files are stored in system tmp directory
+            @tmpdir = Dir.mktmpdir
+            allow(ArtifactUploader).to receive(:artifacts_upload_path).and_return(@tmpdir)
+            build.run!
+          end
+
+          after do
+            FileUtils.remove_entry @tmpdir
+          end
+
+          it do
+            upload_artifacts(file_upload, headers_with_token)
+            expect(response.status).to eq(400)
+          end
+        end
+
+        def upload_artifacts(file, headers = {})
+          params = {
+            file: file.path,
+            filename: file.original_filename,
+          }
+          post post_url, params, headers
+        end
+      end
+
+      describe "DELETE /builds/:id/artifacts" do
+        before do
+          build.run!
+          post delete_url, token: build.project.token, file: file_upload
+        end
+
+        it "should delete artifact build" do
+          build.success
+          delete delete_url, token: build.project.token
+          expect(response.status).to eq(200)
+        end
+      end
+
+      describe "GET /builds/:id/artifacts" do
+        before do
+          build.run!
+        end
+
+        it "should download artifact" do
+          build.update_attributes(artifacts_file: file_upload)
+          get get_url, token: build.project.token
+          expect(response.status).to eq(200)
+        end
+
+        it "should fail to download if no artifact uploaded" do
+          get get_url, token: build.project.token
+          expect(response.status).to eq(404)
+        end
+      end
+    end
   end
 end