Merge branch 'ci-add-job-trace-size-limit' into 'master'

Add limits to job logs

See merge request gitlab-org/gitlab!65349

app/models/concerns/enums/ci/commit_status.rb

@@ -25,6 +25,7 @@ module Enums
           ci_quota_exceeded: 16,
           pipeline_loop_detected: 17,
           no_matching_runner: 18, # not used anymore, but cannot be deleted because of old data
+          trace_size_exceeded: 19,
           insufficient_bridge_permissions: 1_001,
           downstream_bridge_project_not_found: 1_002,
           invalid_bridge_trigger: 1_003,

app/presenters/commit_status_presenter.rb

@@ -26,7 +26,8 @@ class CommitStatusPresenter < Gitlab::View::Presenter::Delegated
     project_deleted: 'The job belongs to a deleted project',
     user_blocked: 'The user who created this job is blocked',
     ci_quota_exceeded: 'No more CI minutes available',
-    no_matching_runner: 'No matching runner available'
+    no_matching_runner: 'No matching runner available',
+    trace_size_exceeded: 'The job log size limit was reached'
   }.freeze
 
   private_constant :CALLOUT_FAILURE_MESSAGES

app/services/ci/append_build_trace_service.rb

@@ -24,6 +24,12 @@ module Ci
       body_start = content_range[0].to_i
       body_end = body_start + body_data.bytesize
 
+      if trace_size_exceeded?(body_end)
+        build.drop(:trace_size_exceeded)
+
+        return Result.new(status: 403)
+      end
+
       stream_size = build.trace.append(body_data, body_start)
 
       unless stream_size == body_end
@@ -37,6 +43,8 @@ module Ci
 
     private
 
+    delegate :project, to: :build
+
     def stream_range
       params.fetch(:content_range)
     end
@@ -61,5 +69,10 @@ module Ci
       ::Gitlab::ErrorTracking
         .log_exception(TraceRangeError.new, extra)
     end
+
+    def trace_size_exceeded?(size)
+      Feature.enabled?(:ci_jobs_trace_size_limit, project, default_enabled: :yaml) &&
+        project.actual_limits.exceeded?(:ci_jobs_trace_size_limit, size / 1.megabyte)
+    end
   end
 end

config/feature_flags/development/ci_jobs_trace_size_limit.yml

+---
+name: ci_jobs_trace_size_limit
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/65349
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/335259
+milestone: '14.1'
+type: development
+group: group::pipeline execution
+default_enabled: false

db/migrate/20210702124842_add_ci_job_trace_size_to_plan_limits.rb

+# frozen_string_literal: true
+
+class AddCiJobTraceSizeToPlanLimits < ActiveRecord::Migration[6.1]
+  def change
+    add_column(:plan_limits, :ci_jobs_trace_size_limit, :integer, default: 100, null: false)
+  end
+end

db/schema_migrations/20210702124842

+8c4c92c4606cf406def47829ce16e903b3b2da00cbbdccfe6f0af5fa249be862
\ No newline at end of file

db/structure.sql

@@ -16363,7 +16363,8 @@ CREATE TABLE plan_limits (
     web_hook_calls integer DEFAULT 0 NOT NULL,
     ci_daily_pipeline_schedule_triggers integer DEFAULT 0 NOT NULL,
     ci_max_artifact_size_running_container_scanning integer DEFAULT 0 NOT NULL,
-    ci_max_artifact_size_cluster_image_scanning integer DEFAULT 0 NOT NULL
+    ci_max_artifact_size_cluster_image_scanning integer DEFAULT 0 NOT NULL,
+    ci_jobs_trace_size_limit integer DEFAULT 100 NOT NULL
 );
 
 CREATE SEQUENCE plan_limits_id_seq

doc/administration/instance_limits.md

@@ -482,6 +482,46 @@ A runner's registration fails if it exceeds the limit for the scope determined b
     Plan.default.actual_limits.update!(ci_registered_project_runners: 100)
     ```
 
+### Maximum file size for job logs
+
+> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/276192) in GitLab 14.1.
+> - [Deployed behind a feature flag](../user/feature_flags.md), disabled by default.
+> - Disabled on GitLab.com.
+> - Not recommended for production use.
+> - To use in GitLab self-managed instances, ask a GitLab administrator to [enable it](#enable-or-disable-job-log-limits). **(FREE SELF)**
+
+This in-development feature might not be available for your use. There can be
+[risks when enabling features still in development](../user/feature_flags.md#risks-when-enabling-features-still-in-development).
+Refer to this feature's version history for more details.
+
+The job log file size limit is 100 megabytes by default. Any job that exceeds this value is dropped.
+
+You can change the limit in the [GitLab Rails console](operations/rails_console.md#starting-a-rails-console-session).
+Update `ci_jobs_trace_size_limit` with the new value in megabytes:
+
+```ruby
+Plan.default.actual_limits.update!(ci_jobs_trace_size_limit: 125)
+```
+
+#### Enable or disable job log limits **(FREE SELF)**
+
+This feature is under development and not ready for production use. It is
+deployed behind a feature flag that is **disabled by default**.
+[GitLab administrators with access to the GitLab Rails console](feature_flags.md)
+can enable it.
+
+To enable it:
+
+```ruby
+Feature.enable(:ci_jobs_trace_size_limit)
+```
+
+To disable it:
+
+```ruby
+Feature.disable(:ci_jobs_trace_size_limit)
+```
+
 ## Instance monitoring and metrics
 
 ### Incident Management inbound alert limits

lib/api/ci/runner.rb

@@ -214,6 +214,10 @@ module API
             .new(job, content_range: content_range)
             .execute(request.body.read)
 
+          if result.status == 403
+            break error!('403 Forbidden', 403)
+          end
+
           if result.status == 416
             break error!('416 Range Not Satisfiable', 416, { 'Range' => "0-#{result.stream_size}" })
           end

lib/gitlab/ci/status/build/failed.rb

@@ -31,7 +31,8 @@ module Gitlab
             project_deleted: 'pipeline project was deleted',
             user_blocked: 'pipeline user was blocked',
             ci_quota_exceeded: 'no more CI minutes available',
-            no_matching_runner: 'no matching runner available'
+            no_matching_runner: 'no matching runner available',
+            trace_size_exceeded: 'log size limit exceeded'
           }.freeze
 
           private_constant :REASONS

spec/requests/api/ci/runner/jobs_trace_spec.rb

@@ -272,6 +272,18 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do
         it { expect(response).to have_gitlab_http_status(:forbidden) }
       end
 
+      context 'when the job trace is too big' do
+        before do
+          project.actual_limits.update!(ci_jobs_trace_size_limit: 1)
+        end
+
+        it 'returns 403 Forbidden' do
+          patch_the_trace(' appended', headers.merge({ 'Content-Range' => "#{1.megabyte}-#{1.megabyte + 9}" }))
+
+          expect(response).to have_gitlab_http_status(:forbidden)
+        end
+      end
+
       def patch_the_trace(content = ' appended', request_headers = nil, job_id: job.id)
         unless request_headers
           job.trace.read do |stream|

spec/services/ci/append_build_trace_service_spec.rb

@@ -5,7 +5,7 @@ require 'spec_helper'
 RSpec.describe Ci::AppendBuildTraceService do
   let_it_be(:project) { create(:project) }
   let_it_be(:pipeline) { create(:ci_pipeline, project: project) }
-  let_it_be(:build) { create(:ci_build, :running, pipeline: pipeline) }
+  let_it_be_with_reload(:build) { create(:ci_build, :running, pipeline: pipeline) }
 
   before do
     stub_feature_flags(ci_enable_live_trace: true)
@@ -54,4 +54,46 @@ RSpec.describe Ci::AppendBuildTraceService do
       expect(result.stream_size).to eq 4
     end
   end
+
+  context 'when the trace size is exceeded' do
+    before do
+      project.actual_limits.update!(ci_jobs_trace_size_limit: 1)
+    end
+
+    it 'returns 403 status code' do
+      stream_size = 1.25.megabytes
+      body_data = 'x' * stream_size
+      content_range = "0-#{stream_size}"
+
+      result = described_class
+        .new(build, content_range: content_range)
+        .execute(body_data)
+
+      expect(result.status).to eq 403
+      expect(result.stream_size).to be_nil
+      expect(build.trace_chunks.count).to eq 0
+      expect(build.reload).to be_failed
+      expect(build.failure_reason).to eq 'trace_size_exceeded'
+    end
+
+    context 'when the feature flag is disabled' do
+      before do
+        stub_feature_flags(ci_jobs_trace_size_limit: false)
+      end
+
+      it 'appends trace chunks' do
+        stream_size = 1.25.megabytes
+        body_data = 'x' * stream_size
+        content_range = "0-#{stream_size}"
+
+        result = described_class
+          .new(build, content_range: content_range)
+          .execute(body_data)
+
+        expect(result.status).to eq 202
+        expect(result.stream_size).to eq stream_size
+        expect(build.trace_chunks.count).to eq 10
+      end
+    end
+  end
 end