Merge pull request #1076 from travis-ci/ha-json-log-egalitarian

Serve JSON data for archived job log requests, regardless of the User-Agent value

Author: BanzaiMan

lib/travis/api/app/endpoint/jobs.rb

@@ -92,7 +92,7 @@ class Jobs < Endpoint
         elsif resource.archived?
           # the way we use responders makes it hard to validate proper format
           # automatically here, so we need to check it explicitly
-          if accepts?('text/plain') || request.user_agent.to_s.start_with?('Travis')
+          if accepts?('text/plain')
             archived_log_path = resource.archived_url
 
             if params[:cors_hax]
@@ -103,6 +103,9 @@ class Jobs < Endpoint
             else
               redirect archived_log_path, 307
             end
+          elsif accepts?('application/json')
+            attach_log_token if job.try(:private?)
+            respond_with resource.as_json
           else
             status 406
           end

lib/travis/api/app/endpoint/logs.rb

@@ -16,8 +16,10 @@ class Logs < Endpoint
         elsif resource.archived?
           # the way we use responders makes it hard to validate proper format
           # automatically here, so we need to check it explicitly
-          if accepts?('text/plain') || request.user_agent.to_s.start_with?('Travis')
+          if accepts?('text/plain')
             redirect resource.archived_url, 307
+          elsif accepts?('application/json')
+            respond_with resource.as_json
           else
             status 406
           end

lib/travis/remote_log.rb

@@ -75,6 +75,10 @@ def archived_url(expires: nil)
       @archived_url ||= remote.fetch_archived_url(job_id, expires: expires)
     end
 
+    def archived_log_content
+      @archived_content ||= remote.fetch_archived_log_content(job_id)
+    end
+
     def to_json(chunked: false, after: nil, part_numbers: [])
       as_json(
         chunked: chunked,
@@ -101,7 +105,7 @@ def as_json(chunked: false, after: nil, part_numbers: [])
           part_numbers: part_numbers
         ).map(&:as_json)
       else
-        ret['body'] = content
+        ret['body'] = archived_log_content || content
       end
 
       { 'log' => ret }
@@ -243,6 +247,12 @@ def fetch_archived_url(job_id, expires: nil)
         file.url(expires)
       end
 
+      def fetch_archived_log_content(job_id)
+        file = fetch_archived(job_id)
+        return "" if file.nil?
+        file.body
+      end
+
       private def fetch_archived(job_id)
         candidates = s3.directories.get(
           bucket_name,
@@ -268,7 +278,7 @@ class Remote
       def_delegators :client, :find_by_job_id, :find_by_id,
         :find_id_by_job_id, :find_parts_by_job_id, :write_content_for_job_id
 
-      def_delegators :archive_client, :fetch_archived_url
+      def_delegators :archive_client, :fetch_archived_url, :fetch_archived_log_content
 
       attr_accessor :platform
 

spec/auth/v2.1/jobs_spec.rb

@@ -4,8 +4,35 @@
   let(:job)  { repo.builds.first.matrix.first }
   let(:log)  { %({"job_id": #{job.id}, "content": "content"}) }
 
-  let(:log_url) { "#{Travis.config[:logs_api][:url]}/logs/#{job.id}?by=job_id&source=api" }
-  before { stub_request(:get, log_url).to_return(status: 200, body: log) }
+  let :log_from_api do
+    {
+      aggregated_at: Time.now,
+      archive_verified: true,
+      archived_at: Time.now,
+      archiving: false,
+      content: 'hello world. this is a really cool log',
+      created_at: Time.now,
+      id: 1,
+      job_id: job.id,
+      purged_at: nil,
+      removed_at: nil,
+      removed_by_id: nil,
+      updated_at: Time.now
+    }
+  end
+
+  let(:archived_content) { "$ git clean -fdx\nRemoving Gemfile.lock\n$ git fetch" }
+
+  let(:log_url) { "#{Travis.config[:logs_api][:url]}/logs/1?by=id&source=api" }
+
+  before do
+    remote = double('remote')
+    allow(Travis::RemoteLog::Remote).to receive(:new).and_return(remote)
+    allow(remote).to receive(:find_by_job_id).and_return(Travis::RemoteLog.new(log_from_api))
+    allow(remote).to receive(:find_by_id).and_return(Travis::RemoteLog.new(log_from_api))
+    allow(remote).to receive(:fetch_archived_url).and_return('https://s3.amazonaws.com/STUFFS')
+  end
+
   before { Job.update_all(state: :started) }
 
   # TODO

spec/auth/v2.1/logs_spec.rb

@@ -5,8 +5,34 @@
   let(:job)   { build.matrix.first }
   let(:log)   { double(id: 1) }
 
+  let :log_from_api do
+    {
+      aggregated_at: Time.now,
+      archive_verified: true,
+      archived_at: Time.now,
+      archiving: false,
+      content: 'hello world. this is a really cool log',
+      created_at: Time.now,
+      id: 1,
+      job_id: job.id,
+      purged_at: nil,
+      removed_at: nil,
+      removed_by_id: nil,
+      updated_at: Time.now
+    }
+  end
+
+  let(:archived_content) { "$ git clean -fdx\nRemoving Gemfile.lock\n$ git fetch" }
+
   let(:log_url) { "#{Travis.config[:logs_api][:url]}/logs/1?by=id&source=api" }
-  before { stub_request(:get, log_url).to_return(status: 200, body: %({"job_id": #{job.id}, "content": "content"})) }
+
+  before do
+    remote = double('remote')
+    allow(Travis::RemoteLog::Remote).to receive(:new).and_return(remote)
+    allow(remote).to receive(:find_by_job_id).and_return(Travis::RemoteLog.new(log_from_api))
+    allow(remote).to receive(:find_by_id).and_return(Travis::RemoteLog.new(log_from_api))
+    allow(remote).to receive(:fetch_archived_url).and_return('https://s3.amazonaws.com/STUFFS')
+  end
 
   describe 'in public mode, with a private repo', mode: :public, repo: :private do
     describe 'GET /logs/%{log.id}' do

spec/auth/v2/jobs_spec.rb

@@ -5,7 +5,36 @@
   let(:log)  { %({"job_id": #{job.id}, "content": "content"}) }
 
   let(:log_url) { "#{Travis.config[:logs_api][:url]}/logs/#{job.id}?by=job_id&source=api" }
-  before { stub_request(:get, log_url).to_return(status: 200, body: log) }
+
+  let :log_from_api do
+    {
+      aggregated_at: Time.now,
+      archive_verified: true,
+      archived_at: Time.now,
+      archiving: false,
+      content: 'hello world. this is a really cool log',
+      created_at: Time.now,
+      id: 1,
+      job_id: job.id,
+      purged_at: nil,
+      removed_at: nil,
+      removed_by_id: nil,
+      updated_at: Time.now
+    }
+  end
+
+  let(:archived_content) { "$ git clean -fdx\nRemoving Gemfile.lock\n$ git fetch" }
+
+  let(:log_url) { "#{Travis.config[:logs_api][:url]}/logs/1?by=id&source=api" }
+
+  before do
+    remote = double('remote')
+    allow(Travis::RemoteLog::Remote).to receive(:new).and_return(remote)
+    allow(remote).to receive(:find_by_job_id).and_return(Travis::RemoteLog.new(log_from_api))
+    allow(remote).to receive(:find_by_id).and_return(Travis::RemoteLog.new(log_from_api))
+    allow(remote).to receive(:fetch_archived_url).and_return('https://s3.amazonaws.com/STUFFS')
+  end
+
   before { Job.update_all(state: :started) }
 
   # TODO

spec/auth/v2/logs_spec.rb

@@ -5,8 +5,34 @@
   let(:job)   { build.matrix.first }
   let(:log)   { double(id: 1) }
 
+  let :log_from_api do
+    {
+      aggregated_at: Time.now,
+      archive_verified: true,
+      archived_at: Time.now,
+      archiving: false,
+      content: 'hello world. this is a really cool log',
+      created_at: Time.now,
+      id: 1,
+      job_id: job.id,
+      purged_at: nil,
+      removed_at: nil,
+      removed_by_id: nil,
+      updated_at: Time.now
+    }
+  end
+
+  let(:archived_content) { "$ git clean -fdx\nRemoving Gemfile.lock\n$ git fetch" }
+
   let(:log_url) { "#{Travis.config[:logs_api][:url]}/logs/1?by=id&source=api" }
-  before { stub_request(:get, log_url).to_return(status: 200, body: %({"job_id": #{job.id}, "content": "content"})) }
+
+  before do
+    remote = double('remote')
+    allow(Travis::RemoteLog::Remote).to receive(:new).and_return(remote)
+    allow(remote).to receive(:find_by_job_id).and_return(Travis::RemoteLog.new(log_from_api))
+    allow(remote).to receive(:find_by_id).and_return(Travis::RemoteLog.new(log_from_api))
+    allow(remote).to receive(:fetch_archived_url).and_return('https://s3.amazonaws.com/STUFFS')
+  end
 
   describe 'in public mode, with a private repo', mode: :public, repo: :private do
     describe 'GET /logs/%{log.id}' do

spec/integration/v2/jobs_spec.rb

@@ -6,6 +6,39 @@
   let(:job) { jobs.first }
   let(:headers) { { 'HTTP_ACCEPT' => 'application/vnd.travis-ci.2+json' } }
 
+  let :log_from_api do
+    {
+      aggregated_at: Time.now,
+      archive_verified: true,
+      archived_at: Time.now,
+      archiving: false,
+      content: 'hello world. this is a really cool log',
+      created_at: Time.now,
+      id: 1,
+      job_id: job.id,
+      purged_at: nil,
+      removed_at: nil,
+      removed_by_id: nil,
+      updated_at: Time.now
+    }
+  end
+
+  let(:archived_log_url) { 'https://s3.amazonaws.com/STUFFS' } # bogus URL unused anywhere
+  let(:archived_content) { "$ git clean -fdx\nRemoving Gemfile.lock\n$ git fetch" }
+
+  let(:log_url) { "#{Travis.config[:logs_api][:url]}/logs/1?by=id&source=api" }
+
+  before do
+    remote = double('remote')
+    allow(Travis::RemoteLog::Remote).to receive(:new).and_return(remote)
+    allow(remote).to receive(:find_by_job_id).and_return(Travis::RemoteLog.new(log_from_api))
+    allow(remote).to receive(:find_by_id).and_return(Travis::RemoteLog.new(log_from_api))
+    allow(remote).to receive(:fetch_archived_url).and_return(archived_log_url)
+    allow(remote).to receive(:fetch_archived_log_content).and_return(archived_content)
+    allow(remote).to receive(:write_content_for_job_id).and_return(remote)
+    allow(remote).to receive(:send).and_return(remote) # ignore attribute updates
+  end
+
   it '/jobs?queue=builds.common' do
     skip('querying with a queue does not appear to be used anymore')
     response = get '/jobs', { queue: 'builds.common' }, headers
@@ -38,11 +71,15 @@
   end
 
   context 'GET /jobs/:job_id/log.txt' do
-    it 'returns log for a job' do
-      stub_request(:get, "#{Travis.config.logs_api.url}/logs/#{job.id}?by=job_id&source=api")
-        .to_return(status: 200, body: JSON.dump(content: 'the log'))
+    it 'returns redirects to archived log url' do
+      response = get("/jobs/#{job.id}/log.txt")
+      expect(response.status).to eq(307)
+      expect(response.location).to eq(archived_log_url)
+    end
+
+    it 'returns 406 (Unprocessable) if Accept header requests JSON' do
       response = get("/jobs/#{job.id}/log.txt", {}, headers)
-      expect(response).to deliver_as_txt('the log', version: 'v2')
+      expect(response.status).to eq(406)
     end
 
     context 'when log is archived' do
@@ -75,18 +112,16 @@
     end
 
     context 'when log is missing' do
-      it 'responds with an empty representation' do
+      it 'redirects to archived log url' do
         stub_request(:get, "#{Travis.config.logs_api.url}/logs/#{job.id}?by=job_id&source=api")
           .to_return(status: 404, body: '')
         response = get(
           "/jobs/#{job.id}/log.txt",
           {},
           { 'HTTP_ACCEPT' => 'text/plain; version=2' }
         )
-        expect(response.status).to eq(200)
-        expect(JSON.parse(response.body, symbolize_names: true)).to eq(
-          { log: { job_id: job.id, parts: [], :@type => 'Log' } }
-        )
+        expect(response.status).to eq(307)
+        expect(response.location).to eq(archived_log_url)
       end
     end
 
@@ -118,7 +153,7 @@
     end
 
     context 'with chunked log requested' do
-      it 'always responds with 406' do
+      it 'succeeds' do
         stub_request(:get, "#{Travis.config.logs_api.url}/logs/#{job.id}?by=job_id&source=api")
           .to_return(
             status: 200,
@@ -134,7 +169,7 @@
           {},
           { 'HTTP_ACCEPT' => 'application/json; version=2; chunked=true' }
         )
-        expect(response.status).to eq(406)
+        expect(response.status).to eq(200)
       end
     end
   end

spec/integration/visibility_spec.rb

@@ -6,6 +6,8 @@
   let(:response) { send(method, path, {}, { 'HTTP_ACCEPT' => 'application/vnd.travis-ci.2.1+json' }) }
   let(:status)   { response.status }
   let(:body)     { JSON.parse(response.body).deep_symbolize_keys }
+  let(:job_id)   { 42864 }
+  let(:archived_content) { 'hello world!'}
 
   before { repo.update_attributes(private: false) }
   before { requests.update_all(private: true) }
@@ -14,6 +16,26 @@
   before { requests[0].update_attributes(private: false) }
   before { builds[0].update_attributes(private: false) }
   before { jobs[0].update_attributes(private: false) }
+  before :each do
+    Fog.mock!
+    storage = Fog::Storage.new({
+      aws_access_key_id: 'key',
+      aws_secret_access_key: 'secret',
+      provider: 'AWS'
+    })
+    bucket = storage.directories.create(key: 'archive.travis-ci.org')
+    file = bucket.files.create(
+      key: "jobs/#{job_id}/log.txt",
+      body: archived_content
+    )
+
+    allow_any_instance_of(Travis::RemoteLog::ArchiveClient).to receive(:fetch_archived).and_return(file)
+  end
+
+  after do
+    Fog.unmock!
+    Fog::Mock.reset
+  end
 
   let(:public_request)  { requests[0] }
   let(:public_build)    { builds[0] }

spec/lib/travis/remote_log_spec.rb

@@ -2,11 +2,32 @@
 
 describe Travis::RemoteLog do
   subject { described_class.new(attrs) }
-  let(:attrs) { { id: 4, content: 'huh', job_id: 5 } }
+  let(:job_id) { 5 }
+  let(:attrs) { { id: 4, content: archived_content, job_id: job_id } }
+  let(:archived_content) { 'hello world' }
 
   before :each do
     described_class::Remote.instance_variable_set(:@clients, nil)
     described_class::Remote.instance_variable_set(:@archive_clients, nil)
+
+    Fog.mock!
+    storage = Fog::Storage.new({
+      aws_access_key_id: 'key',
+      aws_secret_access_key: 'secret',
+      provider: 'AWS'
+    })
+    bucket = storage.directories.create(key: 'archive.travis-ci.org')
+    file = bucket.files.create(
+      key: "jobs/#{job_id}/log.txt",
+      body: archived_content
+    )
+
+    allow_any_instance_of(Travis::RemoteLog::ArchiveClient).to receive(:fetch_archived).and_return(file)
+  end
+
+  after do
+    Fog.unmock!
+    Fog::Mock.reset
   end
 
   it 'has a default client' do