Merge branch 'ci/view-build-artifacts' into 'master'

Add browser for build artifacts

Discussion in #3426, closes #3426.

See merge request !2123

CHANGELOG

@@ -50,6 +50,7 @@ v 8.4.0 (unreleased)
 v 8.3.4
   - Use gitlab-workhorse 0.5.4 (fixes API routing bug)
+  - Add build artifacts browser
 v 8.3.3
   - Preserve CE behavior with JIRA integration by only calling API if URL is set

app/controllers/projects/artifacts_controller.rb

+class Projects::ArtifactsController < Projects::ApplicationController
+  layout 'project'
+  before_action :authorize_read_build_artifacts!
+
+  def download
+    unless artifacts_file.file_storage?
+      return redirect_to artifacts_file.url
+    end
+
+    unless artifacts_file.exists?
+      return not_found!
+    end
+
+    send_file artifacts_file.path, disposition: 'attachment'
+  end
+
+  def browse
+    return render_404 unless build.artifacts?
+
+    directory = params[:path] ? "#{params[:path]}/" : ''
+    @entry = build.artifacts_metadata_entry(directory)
+
+    return render_404 unless @entry.exists?
+  end
+
+  def file
+    entry = build.artifacts_metadata_entry(params[:path])
+
+    if entry.exists?
+      render json: { archive: build.artifacts_file.path,
+                     entry: Base64.encode64(entry.path) }
+    else
+      render json: {}, status: 404
+    end
+  end
+
+  private
+
+  def build
+    @build ||= project.builds.unscoped.find_by!(id: params[:build_id])
+  end
+
+  def artifacts_file
+    @artifacts_file ||= build.artifacts_file
+  end
+
+  def authorize_read_build_artifacts!
+    unless can?(current_user, :read_build_artifacts, @project)
+      if current_user.nil?
+        return authenticate_user!
+      else
+        return render_404
+      end
+    end
+  end
+end

app/controllers/projects/builds_controller.rb

@@ -2,7 +2,6 @@ class Projects::BuildsController < Projects::ApplicationController
   before_action :build, except: [:index, :cancel_all]
   before_action :authorize_manage_builds!, except: [:index, :show, :status]
-  before_action :authorize_download_build_artifacts!, only: [:download]
   layout "project"
@@ -51,18 +50,6 @@ class Projects::BuildsController < Projects::ApplicationController
     redirect_to build_path(build)
   end
-  def download
-    unless artifacts_file.file_storage?
-      return redirect_to artifacts_file.url
-    end
-
-    unless artifacts_file.exists?
-      return not_found!
-    end
-
-    send_file artifacts_file.path, disposition: 'attachment'
-  end
-
   def status
     render json: @build.to_json(only: [:status, :id, :sha, :coverage], methods: :sha)
   end
@@ -79,10 +66,6 @@ class Projects::BuildsController < Projects::ApplicationController
     @build ||= project.builds.unscoped.find_by!(id: params[:id])
   end
-  def artifacts_file
-    build.artifacts_file
-  end
-
   def build_path(build)
     namespace_project_build_path(build.project.namespace, build.project, build)
   end
@@ -92,14 +75,4 @@ class Projects::BuildsController < Projects::ApplicationController
       return page_404
     end
   end
-
-  def authorize_download_build_artifacts!
-    unless can?(current_user, :download_build_artifacts, @project)
-      if current_user.nil?
-        return authenticate_user!
-      else
-        return render_404
-      end
-    end
-  end
 end

app/models/ability.rb

@@ -175,7 +175,7 @@ class Ability
         :create_merge_request,
         :create_wiki,
         :manage_builds,
-        :download_build_artifacts,
+        :read_build_artifacts,
         :push_code
       ]
     end

app/models/ci/build.rb

@@ -30,10 +30,12 @@
 #  description        :string(255)
 #  artifacts_file     :text
 #  gl_project_id      :integer
+#  artifacts_metadata :text
 #
 module Ci
   class Build < CommitStatus
+    include Gitlab::Application.routes.url_helpers
     LAZY_ATTRIBUTES = ['trace']
     belongs_to :runner, class_name: 'Ci::Runner'
@@ -49,6 +51,7 @@ module Ci
     scope :similar, ->(build) { where(ref: build.ref, tag: build.tag, trigger_request_id: build.trigger_request_id) }
     mount_uploader :artifacts_file, ArtifactUploader
+    mount_uploader :artifacts_metadata, ArtifactUploader
     acts_as_taggable
@@ -291,21 +294,18 @@ module Ci
     end
     def target_url
-      Gitlab::Application.routes.url_helpers.
-        namespace_project_build_url(project.namespace, project, self)
+      namespace_project_build_url(project.namespace, project, self)
     end
     def cancel_url
       if active?
-        Gitlab::Application.routes.url_helpers.
-          cancel_namespace_project_build_path(project.namespace, project, self)
+        cancel_namespace_project_build_path(project.namespace, project, self)
       end
     end
     def retry_url
       if retryable?
-        Gitlab::Application.routes.url_helpers.
-          retry_namespace_project_build_path(project.namespace, project, self)
+        retry_namespace_project_build_path(project.namespace, project, self)
       end
     end
@@ -321,20 +321,35 @@ module Ci
       pending? && !any_runners_online?
     end
-    def download_url
-      if artifacts_file.exists?
-        Gitlab::Application.routes.url_helpers.
-          download_namespace_project_build_path(project.namespace, project, self)
-      end
-    end
-
     def execute_hooks
       build_data = Gitlab::BuildDataBuilder.build(self)
       project.execute_hooks(build_data.dup, :build_hooks)
       project.execute_services(build_data.dup, :build_hooks)
     end
+    def artifacts?
+      artifacts_file.exists?
+    end
+
+    def artifacts_download_url
+      if artifacts?
+        download_namespace_project_build_artifacts_path(project.namespace, project, self)
+      end
+    end
+    def artifacts_browse_url
+      if artifacts_browser_supported?
+        browse_namespace_project_build_artifacts_path(project.namespace, project, self)
+      end
+    end
+
+    def artifacts_browser_supported?
+      artifacts? && artifacts_metadata.exists?
+    end
+
+    def artifacts_metadata_entry(path)
+      Gitlab::Ci::Build::Artifacts::Metadata.new(artifacts_metadata.path, path).to_entry
+    end
     private

app/models/commit_status.rb

@@ -131,7 +131,11 @@ class CommitStatus < ActiveRecord::Base
     false
   end
-  def download_url
+  def artifacts_download_url
+    nil
+  end
+
+  def artifacts_browse_url
     nil
   end
 end

app/views/admin/builds/_build.html.haml

@@ -60,8 +60,8 @@
   %td
     .pull-right
-      - if current_user && can?(current_user, :download_build_artifacts, project) && build.download_url
-        = link_to build.download_url, title: 'Download artifacts' do
+      - if current_user && can?(current_user, :read_build_artifacts, project) && build.artifacts?
+        = link_to build.artifacts_download_url, title: 'Download artifacts' do
           %i.fa.fa-download
       - if current_user && can?(current_user, :manage_builds, build.project)
         - if build.active?

app/views/projects/artifacts/_tree_directory.html.haml

+%tr{ class: 'tree-item' }
+  %td.tree-item-file-name
+    = tree_icon('folder', '755', directory.name)
+    %span.str-truncated
+      = link_to directory.name, browse_namespace_project_build_artifacts_path(@project.namespace, @project, @build, path: directory.path)
+  %td
+  %td

app/views/projects/artifacts/_tree_file.html.haml

+%tr{ class: 'tree-item' }
+  %td.tree-item-file-name
+    = tree_icon('file', '664', file.name)
+    %span.str-truncated
+      = file.name
+  %td
+    = number_to_human_size(file.metadata[:size], precision: 2)
+  %td
+    = link_to file_namespace_project_build_artifacts_path(@project.namespace, @project, @build, path: file.path),
+      class: 'btn btn-xs btn-default artifact-download' do
+      = icon('download')

app/views/projects/artifacts/browse.html.haml

+- page_title 'Artifacts', "#{@build.name} (##{@build.id})", 'Builds'
+= render 'projects/builds/header_title'
+
+#tree-holder.tree-holder
+  .gray-content-block.top-block.clearfix
+    .pull-right
+      = link_to download_namespace_project_build_artifacts_path(@project.namespace, @project, @build),
+        class: 'btn btn-default' do
+        = icon('download')
+        Download artifacts archive
+
+%div.tree-content-holder
+  .table-holder
+    %table.table.tree-table.table-striped
+      %thead
+        %tr
+          %th Name
+          %th Size
+          %th Download
+      = render partial: 'tree_directory', collection: @entry.directories(parent: true), as: :directory
+      = render partial: 'tree_file', collection: @entry.files, as: :file
+
+- if @entry.empty?
+  .center Empty

app/views/projects/builds/show.html.haml

@@ -89,9 +89,15 @@
             Test coverage
           %h1 #{@build.coverage}%
-      - if current_user && can?(current_user, :download_build_artifacts, @project) && @build.download_url
-        .build-widget.center
-          = link_to "Download artifacts", @build.download_url, class: 'btn btn-sm btn-primary'
+      - if current_user && can?(current_user, :read_build_artifacts, @project) && @build.artifacts?
+
+        .build-widget.artifacts
+          %h4.title Build artifacts
+          .center
+            .btn-group{ role: :group }
+              = link_to "Download", @build.artifacts_download_url, class: 'btn btn-sm btn-primary'
+              - if @build.artifacts_browser_supported?
+                = link_to "Browse", @build.artifacts_browse_url, class: 'btn btn-sm btn-primary'
       .build-widget
         %h4.title

app/views/projects/commit_statuses/_commit_status.html.haml

@@ -66,8 +66,8 @@
   %td
     .pull-right
-      - if current_user && can?(current_user, :download_build_artifacts, commit_status.project) && commit_status.download_url
-        = link_to commit_status.download_url, title: 'Download artifacts' do
+      - if current_user && can?(current_user, :read_build_artifacts, commit_status.project) && commit_status.artifacts?
+        = link_to commit_status.artifacts_download_url, title: 'Download artifacts' do
           %i.fa.fa-download
       - if current_user && can?(current_user, :manage_builds, commit_status.project)
         - if commit_status.active?

config/routes.rb

@@ -604,9 +604,14 @@ Rails.application.routes.draw do
           member do
             get :status
             post :cancel
-            get :download
             post :retry
           end
+
+          resource :artifacts, only: [] do
+            get :download
+            get :browse, path: 'browse(/*path)', format: false
+            get :file, path: 'file/*path', format: false
+          end
         end
         resources :hooks, only: [:index, :create, :destroy], constraints: { id: /\d+/ } do

db/fixtures/development/14_builds.rb

+class Gitlab::Seeder::Builds
+  BUILD_STATUSES = %w(running pending success failed canceled)
+
+  def initialize(project)
+    @project = project
+  end
+
+  def seed!
+    ci_commits.each do |ci_commit|
+      build = Ci::Build.new(build_attributes_for(ci_commit))
+
+      artifacts_cache_file(artifacts_archive_path) do |file|
+        build.artifacts_file = file
+      end
+
+      artifacts_cache_file(artifacts_metadata_path) do |file|
+        build.artifacts_metadata = file
+      end
+
+      begin
+        build.save!
+        print '.'
+      rescue ActiveRecord::RecordInvalid
+        print 'F'
+      end
+    end
+  end
+
+  def ci_commits
+    commits = @project.repository.commits('master', nil, 5)
+    commits_sha = commits.map { |commit| commit.raw.id }
+    commits_sha.map do |sha|
+      @project.ensure_ci_commit(sha)
+    end
+  rescue
+    []
+  end
+
+  def build_attributes_for(ci_commit)
+    { name: 'test build', commands: "$ build command",
+      stage: 'test', stage_idx: 1, ref: 'master',
+      user_id: build_user, gl_project_id: @project.id,
+      status: build_status, commit_id: ci_commit.id,
+      created_at: Time.now, updated_at: Time.now }
+  end
+
+  def build_user
+    @project.team.users.sample
+  end
+
+  def build_status
+    BUILD_STATUSES.sample
+  end
+
+  def artifacts_archive_path
+    Rails.root + 'spec/fixtures/ci_build_artifacts.zip'
+  end
+
+  def artifacts_metadata_path
+    Rails.root + 'spec/fixtures/ci_build_artifacts_metadata.gz'
+
+  end
+
+  def artifacts_cache_file(file_path)
+    cache_path = file_path.to_s.gsub('ci_', "p#{@project.id}_")
+
+    FileUtils.copy(file_path, cache_path)
+    File.open(cache_path) do |file|
+      yield file
+    end
+  end
+end
+
+Gitlab::Seeder.quiet do
+  Project.all.sample(10).each do |project|
+    project_builds = Gitlab::Seeder::Builds.new(project)
+    project_builds.seed!
+  end
+end

db/migrate/20151230132518_add_artifacts_metadata_to_ci_build.rb

+class AddArtifactsMetadataToCiBuild < ActiveRecord::Migration
+  def change
+    add_column :ci_builds, :artifacts_metadata, :text
+  end
+end

db/schema.rb

@@ -123,6 +123,7 @@ ActiveRecord::Schema.define(version: 20160113111034) do
     t.string   "description"
     t.text     "artifacts_file"
     t.integer  "gl_project_id"
+    t.text     "artifacts_metadata"
   end
   add_index "ci_builds", ["commit_id", "stage_idx", "created_at"], name: "index_ci_builds_on_commit_id_and_stage_idx_and_created_at", using: :btree

features/project/builds.feature

+Feature: Project Builds
+  Background:
+    Given I sign in as a user
+    And I own a project
+    And CI is enabled
+    And I have recent build for my project
+
+  Scenario: I browse build summary page
+    When I visit recent build summary page
+    Then I see summary for build
+    And I see build trace
+
+  Scenario: I download build artifacts
+    Given recent build has artifacts available
+    When I visit recent build summary page
+    And I click artifacts download button
+    Then download of build artifacts archive starts
+
+  Scenario: I browse build artifacts
+    Given recent build has artifacts available
+    And recent build has artifacts metadata available
+    When I visit recent build summary page
+    And I click artifacts browse button
+    Then I should see content of artifacts archive
+
+  Scenario: I browse subdirectory of build artifacts
+    Given recent build has artifacts available
+    And recent build has artifacts metadata available
+    When I visit recent build summary page
+    And I click artifacts browse button
+    And I click link to subdirectory within build artifacts
+    Then I should see content of subdirectory within artifacts archive
+
+  Scenario: I browse directory with UTF-8 characters in name
+    Given recent build has artifacts available
+    And recent build has artifacts metadata available
+    And recent build artifacts contain directory with UTF-8 characters
+    When I visit recent build summary page
+    And I click artifacts browse button
+    And I navigate to directory with UTF-8 characters in name
+    Then I should see content of directory with UTF-8 characters in name
+
+  Scenario: I try to browse directory with invalid UTF-8 characters in name
+    Given recent build has artifacts available
+    And recent build has artifacts metadata available
+    And recent build artifacts contain directory with invalid UTF-8 characters
+    When I visit recent build summary page
+    And I click artifacts browse button
+    And I navigate to parent directory of directory with invalid name
+    Then I should not see directory with invalid name on the list
+
+  Scenario: I download a single file from build artifacts
+    Given recent build has artifacts available
+    And recent build has artifacts metadata available
+    When I visit recent build summary page
+    And I click artifacts browse button
+    And I click download button for a file within build artifacts
+    Then download of a file extracted from build artifacts should start

features/steps/project/builds.rb

+class Spinach::Features::ProjectBuilds < Spinach::FeatureSteps
+  include SharedAuthentication
+  include SharedProject
+  include SharedBuilds
+  include RepoHelpers
+
+  step 'I see summary for build' do
+    expect(page).to have_content "Build ##{@build.id}"
+  end
+
+  step 'I see build trace' do
+    expect(page).to have_css '#build-trace'
+  end
+
+  step 'I click artifacts download button' do
+    page.within('.artifacts') { click_link 'Download' }
+  end
+
+  step 'download of build artifacts archive starts' do
+    expect(page.response_headers['Content-Type']).to eq 'application/zip'
+    expect(page.response_headers['Content-Transfer-Encoding']).to eq 'binary'
+  end
+
+  step 'I click artifacts browse button' do
+    page.within('.artifacts') { click_link 'Browse' }
+  end
+
+  step 'I should see content of artifacts archive' do
+    page.within('.tree-table') do
+      expect(page).to have_no_content '..'
+      expect(page).to have_content 'other_artifacts_0.1.2'
+      expect(page).to have_content 'ci_artifacts.txt'
+      expect(page).to have_content 'rails_sample.jpg'
+    end
+  end
+
+  step 'I click link to subdirectory within build artifacts' do
+    page.within('.tree-table') { click_link 'other_artifacts_0.1.2' }
+  end
+
+  step 'I should see content of subdirectory within artifacts archive' do
+    page.within('.tree-table') do
+      expect(page).to have_content '..'
+      expect(page).to have_content 'another-subdirectory'
+      expect(page).to have_content 'doc_sample.txt'
+    end
+  end
+
+  step 'recent build artifacts contain directory with UTF-8 characters' do
+    # metadata fixture contains relevant directory
+  end
+
+  step 'I navigate to directory with UTF-8 characters in name' do
+    page.within('.tree-table') { click_link 'tests_encoding' }
+    page.within('.tree-table') { click_link 'utf8 test dir ✓' }
+  end
+
+  step 'I should see content of directory with UTF-8 characters in name' do
+    page.within('.tree-table') do
+      expect(page).to have_content '..'
+      expect(page).to have_content 'regular_file_2'
+    end
+  end
+
+  step 'recent build artifacts contain directory with invalid UTF-8 characters' do
+    # metadata fixture contains relevant directory
+  end
+
+  step 'I navigate to parent directory of directory with invalid name' do
+    page.within('.tree-table') { click_link 'tests_encoding' }
+  end
+
+  step 'I should not see directory with invalid name on the list' do
+    page.within('.tree-table') do
+      expect(page).to have_no_content('non-utf8-dir')
+    end
+  end
+
+  step 'I click download button for a file within build artifacts' do
+    page.within('.tree-table') { first('.artifact-download').click }
+  end
+
+  step 'download of a file extracted from build artifacts should start' do
+    # this will be accelerated by Workhorse
+    response_json = JSON.parse(page.body, symbolize_names: true)
+    expect(response_json[:archive]).to end_with('build_artifacts.zip')
+    expect(response_json[:entry]).to eq Base64.encode64('ci_artifacts.txt')
+  end
+end

features/steps/shared/builds.rb

+module SharedBuilds
+  include Spinach::DSL
+
+  step 'CI is enabled' do
+    @project.enable_ci
+  end
+
+  step 'I have recent build for my project' do
+    ci_commit = create :ci_commit, project: @project, sha: sample_commit.id
+    @build = create :ci_build, commit: ci_commit
+  end
+
+  step 'I visit recent build summary page' do
+    visit namespace_project_build_path(@project.namespace, @project, @build)
+  end
+
+  step 'recent build has artifacts available' do
+    artifacts = Rails.root + 'spec/fixtures/ci_build_artifacts.zip'
+    archive = fixture_file_upload(artifacts, 'application/zip')
+    @build.update_attributes(artifacts_file: archive)
+  end
+
+  step 'recent build has artifacts metadata available' do
+    metadata = Rails.root + 'spec/fixtures/ci_build_artifacts_metadata.gz'
+    gzip = fixture_file_upload(metadata, 'application/x-gzip')
+    @build.update_attributes(artifacts_metadata: gzip)
+  end
+end

lib/api/helpers.rb

@@ -289,12 +289,14 @@ module API
     # file helpers
-    def uploaded_file!(field, uploads_path)
+    def uploaded_file(field, uploads_path)
       if params[field]
         bad_request!("#{field} is not a file") unless params[field].respond_to?(:filename)
         return params[field]
       end
+      return nil unless params["#{field}.path"] && params["#{field}.name"]
+
       # sanitize file paths
       # this requires all paths to exist
       required_attributes! %W(#{field}.path)