Merge branch 'sh-force-encoding-job-logs' into 'master'

Force ASCII-8BIT encodings in CI job trace

See merge request gitlab-org/gitlab!64631

app/models/ci/build_trace_chunk.rb

@@ -121,7 +121,7 @@ module Ci
       raise ArgumentError, 'Offset is out of range' if offset > size || offset < 0
       return if offset == size # Skip the following process as it doesn't affect anything
 
-      self.append("", offset)
+      self.append(+"", offset)
     end
 
     def append(new_data, offset)

app/models/ci/build_trace_chunks/fog.rb

@@ -25,10 +25,18 @@ module Ci
         files.create(create_attributes(model, new_data))
       end
 
+      # This is the sequence that causes append_data to be called:
+      #
+      # 1. Runner sends a PUT /api/v4/jobs/:id to indicate the job is canceled or finished.
+      # 2. UpdateBuildStateService#accept_build_state! persists all live job logs to object storage (or filesystem).
+      # 3. UpdateBuildStateService#accept_build_state! returns a 202 to the runner.
+      # 4. The runner continues to send PATCH requests with job logs until all logs have been sent and received.
+      # 5. If the last PATCH request arrives after the job log has been persisted, we
+      #    retrieve the data from object storage to append the remaining lines.
       def append_data(model, new_data, offset)
         if offset > 0
           truncated_data = data(model).to_s.byteslice(0, offset)
-          new_data = truncated_data + new_data
+          new_data = append_strings(truncated_data, new_data)
         end
 
         set_data(model, new_data)
@@ -71,6 +79,17 @@ module Ci
 
       private
 
+      def append_strings(old_data, new_data)
+        if Feature.enabled?(:ci_job_trace_force_encode, default_enabled: :yaml)
+          # When object storage is in use, old_data may be retrieved in UTF-8.
+          old_data = old_data.force_encoding(Encoding::ASCII_8BIT)
+          # new_data should already be in ASCII-8BIT, but just in case it isn't, do this.
+          new_data = new_data.force_encoding(Encoding::ASCII_8BIT)
+        end
+
+        old_data + new_data
+      end
+
       def key(model)
         key_raw(model.build_id, model.chunk_index)
       end

config/feature_flags/development/ci_job_trace_force_encode.yml

+---
+name: ci_job_trace_force_encode
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/64631
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/333452
+milestone: '14.1'
+type: development
+group: group::verify
+default_enabled: false

spec/models/ci/build_trace_chunks/fog_spec.rb

@@ -103,19 +103,53 @@ RSpec.describe Ci::BuildTraceChunks::Fog do
   end
 
   describe '#append_data' do
-    let(:model) { create(:ci_build_trace_chunk, :fog_with_data, initial_data: (+'😺').force_encoding('ASCII-8BIT')) }
+    let(:initial_data) { (+'😺').force_encoding(Encoding::ASCII_8BIT) }
+    let(:model) { create(:ci_build_trace_chunk, :fog_with_data, initial_data: initial_data) }
     let(:data) { data_store.data(model) }
 
-    it 'appends ASCII data' do
-      data_store.append_data(model, 'hello world', 4)
+    context 'when ci_job_trace_force_encode is enabled' do
+      it 'appends ASCII data' do
+        data_store.append_data(model, +'hello world', 4)
 
-      expect(data.encoding).to eq(Encoding.find('ASCII-8BIT'))
-      expect(data.force_encoding('UTF-8')).to eq('😺hello world')
+        expect(data.encoding).to eq(Encoding::ASCII_8BIT)
+        expect(data.force_encoding(Encoding::UTF_8)).to eq('😺hello world')
+      end
+
+      it 'appends UTF-8 data' do
+        data_store.append_data(model, +'Résumé', 4)
+
+        expect(data.encoding).to eq(Encoding::ASCII_8BIT)
+        expect(data.force_encoding(Encoding::UTF_8)).to eq("😺Résumé")
+      end
+
+      context 'when initial data is UTF-8' do
+        let(:initial_data) { +'😺' }
+
+        it 'appends ASCII data' do
+          data_store.append_data(model, +'hello world', 4)
+
+          expect(data.encoding).to eq(Encoding::ASCII_8BIT)
+          expect(data.force_encoding(Encoding::UTF_8)).to eq('😺hello world')
+        end
+      end
     end
 
-    it 'throws an exception when appending UTF-8 data' do
-      expect(Gitlab::ErrorTracking).to receive(:track_and_raise_exception).and_call_original
-      expect { data_store.append_data(model, 'Résumé', 4) }.to raise_exception(Encoding::CompatibilityError)
+    context 'when ci_job_trace_force_encode is disabled' do
+      before do
+        stub_feature_flags(ci_job_trace_force_encode: false)
+      end
+
+      it 'appends ASCII data' do
+        data_store.append_data(model, +'hello world', 4)
+
+        expect(data.encoding).to eq(Encoding::ASCII_8BIT)
+        expect(data.force_encoding(Encoding::UTF_8)).to eq('😺hello world')
+      end
+
+      it 'throws an exception when appending UTF-8 data' do
+        expect(Gitlab::ErrorTracking).to receive(:track_and_raise_exception).and_call_original
+        expect { data_store.append_data(model, +'Résumé', 4) }.to raise_exception(Encoding::CompatibilityError)
+      end
     end
   end