Merge branch 'feature/incremental-build-trace-update' into 'master'

Add incremental build trace update API

Closes #14537 

Must be merged if we want changes from gitlab-org/gitlab-ci-multi-runner!124 to work.

See merge request !3447

CHANGELOG

@@ -105,6 +105,7 @@ v 8.7.0 (unreleased)
   - Allow enable/disable push events for system hooks
   - Fix GitHub project's link in the import page when provider has a custom URL
   - Add RAW build trace output and button on build page
+  - Add incremental build trace update into CI API
 v 8.6.7 (unreleased)
   - Fix persistent XSS vulnerability in `commit_person_link` helper

app/models/ci/build.rb

@@ -230,12 +230,33 @@ module Ci
       end
     end
+    def trace_length
+      if raw_trace
+        raw_trace.length
+      else
+        0
+      end
+    end
+
     def trace=(trace)
+      recreate_trace_dir
+      File.write(path_to_trace, trace)
+    end
+
+    def recreate_trace_dir
       unless Dir.exists?(dir_to_trace)
         FileUtils.mkdir_p(dir_to_trace)
       end
-      File.write(path_to_trace, trace)
+    end
+    private :recreate_trace_dir
+    def append_trace(trace_part, offset)
+      recreate_trace_dir
+
+      File.truncate(path_to_trace, offset) if File.exist?(path_to_trace)
+      File.open(path_to_trace, 'a') do |f|
+        f.write(trace_part)
+      end
     end
     def dir_to_trace

doc/ci/api/builds.md

@@ -26,48 +26,114 @@ This API uses two types of authentication:
 ### Runs oldest pending build by runner
-    POST /ci/api/v1/builds/register
-Parameters:
-  * `token` (required) - Unique runner token
+```
+POST /ci/api/v1/builds/register
+```
+| Attribute | Type    | Required | Description         |
+|-----------|---------|----------|---------------------|
+| `token`   | string  | yes      | Unique runner token |
+```
+curl -X POST "https://gitlab.example.com/ci/api/v1/builds/register" -F "token=t0k3n"
+```
 ### Update details of an existing build
-    PUT /ci/api/v1/builds/:id
+```
+PUT /ci/api/v1/builds/:id
+```
+
+| Attribute | Type    | Required | Description          |
+|-----------|---------|----------|----------------------|
+| `id`      | integer | yes      | The ID of a project  |
+| `token`   | string  | yes      | Unique runner token  |
+| `state`   | string  | no       | The state of a build |
+| `trace`   | string  | no       | The trace of a build |
+
+```
+curl -X PUT "https://gitlab.example.com/ci/api/v1/builds/1234" -F "token=t0k3n" -F "state=running" -F "trace=Running git clone...\n"
+```
+
+### Incremental build trace update
+
+Using this method you need to send trace content as a request body. You also need to provide the `Content-Range` header
+with a range of sent trace part. Note that you need to send parts in the proper order, so the begining of the part
+must start just after the end of the previous part. If you provide the wrong part, then GitLab CI API will return `416
+Range Not Satisfiable` response with a header `Range: 0-X`, where `X` is the current trace length.
+
+For example, if you receive `Range: 0-11` in the response, then your next part must contain a `Content-Range: 11-...`
+header and a trace part covered by this range.
+
+For a valid update API will return `202` response with:
+* `Build-Status: {status}` header containing current status of the build,
+* `Range: 0-{length}` header with the current trace length.
+
+```
+PATCH /ci/api/v1/builds/:id/trace.txt
+```
 Parameters:
-  * `id` (required) - The ID of a project
-  * `token` (required) - Unique runner token
-  * `state` (optional) - The state of a build
-  * `trace` (optional) - The trace of a build
+| Attribute | Type    | Required | Description          |
+|-----------|---------|----------|----------------------|
+| `id`      | integer | yes      | The ID of a build    |
+
+Headers:
+
+| Attribute       | Type    | Required | Description                       |
+|-----------------|---------|----------|-----------------------------------|
+| `BUILD-TOKEN`   | string  | yes      | The build authorization token     |
+| `Content-Range` | string  | yes      | Bytes range of trace that is sent |
+
+```
+curl -X PATCH "https://gitlab.example.com/ci/api/v1/builds/1234/trace.txt" -H "BUILD-TOKEN=build_t0k3n" -H "Content-Range=0-21" -d "Running git clone...\n"
+```
+
 ### Upload artifacts to build
-    POST /ci/api/v1/builds/:id/artifacts
-Parameters:
-  * `id` (required) - The ID of a build
-  * `token` (required) - The build authorization token
-  * `file` (required) - Artifacts file
+```
+POST /ci/api/v1/builds/:id/artifacts
+```
+| Attribute | Type    | Required | Description                   |
+|-----------|---------|----------|-------------------------------|
+| `id`      | integer | yes      | The ID of a build             |
+| `token`   | string  | yes      | The build authorization token |
+| `file`    | mixed   | yes      | Artifacts file                |
+```
+curl -X POST "https://gitlab.example.com/ci/api/v1/builds/1234/artifacts" -F "token=build_t0k3n" -F "file=@/path/to/file"
+```
 ### Download the artifacts file from build
-    GET /ci/api/v1/builds/:id/artifacts
-Parameters:
-  * `id` (required) - The ID of a build
-  * `token` (required) - The build authorization token
+```
+GET /ci/api/v1/builds/:id/artifacts
+```
+| Attribute | Type    | Required | Description                   |
+|-----------|---------|----------|-------------------------------|
+| `id`      | integer | yes      | The ID of a build             |
+| `token`   | string  | yes      | The build authorization token |
+```
+curl "https://gitlab.example.com/ci/api/v1/builds/1234/artifacts" -F "token=build_t0k3n"
+```
 ### Remove the artifacts file from build
-    DELETE /ci/api/v1/builds/:id/artifacts
-Parameters:
-  * ` id` (required) - The ID of a build
-  * `token` (required) - The build authorization token
+```
+DELETE /ci/api/v1/builds/:id/artifacts
+```
+| Attribute | Type    | Required | Description                   |
+|-----------|---------|----------|-------------------------------|
+| ` id`     | integer | yes      | The ID of a build             |
+| `token`   | string  | yes      | The build authorization token |
+```
+curl -X DELETE "https://gitlab.example.com/ci/api/v1/builds/1234/artifacts" -F "token=build_t0k3n"
+```

lib/ci/api/api.rb

@@ -23,6 +23,8 @@ module Ci
         rack_response({ 'message' => '500 Internal Server Error' }, 500)
       end
+      content_type :txt,  'text/plain'
+      content_type :json, 'application/json'
       format :json
       helpers ::Ci::API::Helpers

lib/ci/api/builds.rb

@@ -50,6 +50,39 @@ module Ci
           end
         end
+        # Send incremental log update - Runners only
+        #
+        # Parameters:
+        #   id (required) - The ID of a build
+        # Body:
+        #   content of logs to append
+        # Headers:
+        #   Content-Range (required) - range of content that was sent
+        #   BUILD-TOKEN (required) - The build authorization token
+        # Example Request:
+        #   PATCH /builds/:id/trace.txt
+        patch ":id/trace.txt" do
+          build = Ci::Build.find_by_id(params[:id])
+          not_found! unless build
+          authenticate_build_token!(build)
+          forbidden!('Build has been erased!') if build.erased?
+
+          error!('400 Missing header Content-Range', 400) unless request.headers.has_key?('Content-Range')
+          content_range = request.headers['Content-Range']
+          content_range = content_range.split('-')
+
+          current_length = build.trace_length
+          unless current_length == content_range[0].to_i
+            return error!('416 Range Not Satisfiable', 416, { 'Range' => "0-#{current_length}" })
+          end
+
+          build.append_trace(request.body.read, content_range[0].to_i)
+
+          status 202
+          header 'Build-Status', build.status
+          header 'Range', "0-#{build.trace_length}"
+        end
+
         # Authorize artifacts uploading for build - Runners only
         #
         # Parameters:

spec/requests/ci/api/builds_spec.rb

@@ -156,6 +156,52 @@ describe Ci::API::API do
       end
     end
+    describe 'PATCH /builds/:id/trace.txt' do
+      let(:build) { create(:ci_build, :trace, runner_id: runner.id) }
+      let(:headers) { { Ci::API::Helpers::BUILD_TOKEN_HEADER => build.token, 'Content-Type' => 'text/plain' } }
+      let(:headers_with_range) { headers.merge({ 'Content-Range' => '11-20' }) }
+
+      before do
+        build.run!
+        patch ci_api("/builds/#{build.id}/trace.txt"), ' appended', headers_with_range
+      end
+
+      context 'when request is valid' do
+        it { expect(response.status).to eq 202 }
+        it { expect(build.reload.trace).to eq 'BUILD TRACE appended' }
+        it { expect(response.header).to have_key 'Range' }
+        it { expect(response.header).to have_key 'Build-Status' }
+      end
+
+      context 'when content-range start is too big' do
+        let(:headers_with_range) { headers.merge({ 'Content-Range' => '15-20' }) }
+
+        it { expect(response.status).to eq 416 }
+        it { expect(response.header).to have_key 'Range' }
+        it { expect(response.header['Range']).to eq '0-11' }
+      end
+
+      context 'when content-range start is too small' do
+        let(:headers_with_range) { headers.merge({ 'Content-Range' => '8-20' }) }
+
+        it { expect(response.status).to eq 416 }
+        it { expect(response.header).to have_key 'Range' }
+        it { expect(response.header['Range']).to eq '0-11' }
+      end
+
+      context 'when Content-Range header is missing' do
+        let(:headers_with_range) { headers.merge({}) }
+
+        it { expect(response.status).to eq 400 }
+      end
+
+      context 'when build has been errased' do
+        let(:build) { create(:ci_build, runner_id: runner.id, erased_at: Time.now) }
+
+        it { expect(response.status).to eq 403 }
+      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') }