Merge branch 'gitlab-pages-zip' into 'master'

Add support for ZIP build artifacts for Pages

This is based on https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/116

It adds support for creating pages from zip archives.

Omnibus change: https://gitlab.com/gitlab-org/omnibus-gitlab/issues/1057

See merge request !117

app/models/ci/build.rb

@@ -347,17 +347,17 @@ module Ci
     end
 
     def artifacts_browse_url
-      if artifacts_browser_supported?
+      if artifacts_metadata?
         browse_namespace_project_build_artifacts_path(project.namespace, project, self)
       end
     end
 
-    def artifacts_browser_supported?
+    def artifacts_metadata?
       artifacts? && artifacts_metadata.exists?
     end
 
-    def artifacts_metadata_entry(path)
-      Gitlab::Ci::Build::Artifacts::Metadata.new(artifacts_metadata.path, path).to_entry
+    def artifacts_metadata_entry(path, **options)
+      Gitlab::Ci::Build::Artifacts::Metadata.new(artifacts_metadata.path, path, **options).to_entry
     end
 
     private

app/services/projects/update_pages_service.rb

@@ -2,6 +2,7 @@ module Projects
   class UpdatePagesService < BaseService
     BLOCK_SIZE = 32.kilobytes
     MAX_SIZE = 1.terabyte
+    SITE_PATH = 'public/'
 
     attr_reader :build
 
@@ -60,13 +61,42 @@ module Projects
     end
 
     def extract_archive!(temp_path)
+      if artifacts.ends_with?('.tar.gz') || artifacts.ends_with?('.tgz')
+        extract_tar_archive!(temp_path)
+      elsif artifacts.ends_with?('.zip')
+        extract_zip_archive!(temp_path)
+      else
+        raise 'unsupported artifacts format'
+      end
+    end
+
+    def extract_tar_archive!(temp_path)
       results = Open3.pipeline(%W(gunzip -c #{artifacts}),
                                %W(dd bs=#{BLOCK_SIZE} count=#{blocks}),
-                               %W(tar -x -C #{temp_path} public/),
+                               %W(tar -x -C #{temp_path} #{SITE_PATH}),
                                err: '/dev/null')
       raise 'pages failed to extract' unless results.compact.all?(&:success?)
     end
 
+    def extract_zip_archive!(temp_path)
+      raise 'missing artifacts metadata' unless build.artifacts_metadata?
+
+      # Calculate page size after extract
+      public_entry = build.artifacts_metadata_entry(SITE_PATH, recursive: true)
+
+      if public_entry.total_size > max_size
+        raise "artifacts for pages are too large: #{total_size}"
+      end
+
+      # Requires UnZip at least 6.00 Info-ZIP.
+      # -n  never overwrite existing files
+      # We add * to end of SITE_PATH, because we want to extract SITE_PATH and all subdirectories
+      site_path = File.join(SITE_PATH, '*')
+      unless system(*%W(unzip -n #{artifacts} #{site_path} -d #{temp_path}))
+        raise 'pages failed to extract'
+      end
+    end
+
     def deploy_page!(archive_public_path)
       # Do atomic move of pages
       # Move and removal may not be atomic, but they are significantly faster then extracting and removal
@@ -91,10 +121,11 @@ module Projects
 
     def blocks
       # Calculate dd parameters: we limit the size of pages
-      max_size = current_application_settings.max_pages_size.megabytes
-      max_size ||= MAX_SIZE
-      blocks = 1 + max_size / BLOCK_SIZE
-      blocks
+      1 + max_size / BLOCK_SIZE
+    end
+
+    def max_size
+      current_application_settings.max_pages_size.megabytes || MAX_SIZE
     end
 
     def tmp_path

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

@@ -96,7 +96,7 @@
           .center
             .btn-group{ role: :group }
               = link_to "Download", @build.artifacts_download_url, class: 'btn btn-sm btn-primary'
-              - if @build.artifacts_browser_supported?
+              - if @build.artifacts_metadata?
                 = link_to "Browse", @build.artifacts_browse_url, class: 'btn btn-sm btn-primary'
 
       .build-widget

lib/gitlab/ci/build/artifacts/metadata.rb

@@ -13,8 +13,8 @@ module Gitlab
 
           attr_reader :file, :path, :full_version
 
-          def initialize(file, path)
-            @file, @path = file, path
+          def initialize(file, path, **opts)
+            @file, @path, @opts = file, path, opts
             @full_version = read_version
           end
 
@@ -52,7 +52,9 @@ module Gitlab
 
           def match_entries(gz)
             entries = {}
-            match_pattern = %r{^#{Regexp.escape(@path)}[^/]*/?$}
+
+            child_pattern = '[^/]*/?$' unless @opts[:recursive]
+            match_pattern = /^#{Regexp.escape(@path)}#{child_pattern}/
 
             until gz.eof? do
               begin

lib/gitlab/ci/build/artifacts/metadata/entry.rb

@@ -95,6 +95,13 @@ module Gitlab
           children.empty?
         end
 
+        def total_size
+          descendant_pattern = %r{^#{Regexp.escape(@path)}}
+          entries.sum do |path, entry|
+            (entry[:size] if path =~ descendant_pattern).to_i
+          end
+        end
+
         def to_s
           @path
         end

spec/lib/gitlab/ci/build/artifacts/metadata/entry_spec.rb

@@ -4,13 +4,13 @@ describe Gitlab::Ci::Build::Artifacts::Metadata::Entry do
   let(:entries) do
     { 'path/' => {},
       'path/dir_1/' => {},
-      'path/dir_1/file_1' => {},
-      'path/dir_1/file_b' => {},
+      'path/dir_1/file_1' => { size: 10 },
+      'path/dir_1/file_b' => { size: 10 },
       'path/dir_1/subdir/' => {},
-      'path/dir_1/subdir/subfile' => {},
+      'path/dir_1/subdir/subfile' => { size: 10 },
       'path/second_dir' => {},
-      'path/second_dir/dir_3/file_2' => {},
-      'path/second_dir/dir_3/file_3'=> {},
+      'path/second_dir/dir_3/file_2' => { size: 10 },
+      'path/second_dir/dir_3/file_3'=> { size: 10 },
       'another_directory/'=> {},
       'another_file' => {},
       '/file/with/absolute_path' => {} }
@@ -112,6 +112,11 @@ describe Gitlab::Ci::Build::Artifacts::Metadata::Entry do
         subject { |example| path(example).empty? }
         it { is_expected.to be false }
       end
+
+      describe '#total_size' do
+        subject { |example| path(example).total_size }
+        it { is_expected.to eq(30) }
+      end
     end
   end
 

spec/lib/gitlab/ci/build/artifacts/metadata_spec.rb

 require 'spec_helper'
 
 describe Gitlab::Ci::Build::Artifacts::Metadata do
-  def metadata(path = '')
-    described_class.new(metadata_file_path, path)
+  def metadata(path = '', **opts)
+    described_class.new(metadata_file_path, path, **opts)
   end
 
   let(:metadata_file_path) do
@@ -51,6 +51,19 @@ describe Gitlab::Ci::Build::Artifacts::Metadata do
       end
     end
 
+    describe '#find_entries! recursively for other_artifacts_0.1.2/' do
+      subject { metadata('other_artifacts_0.1.2/', recursive: true).find_entries! }
+
+      it 'matches correct paths' do
+        expect(subject.keys).
+          to contain_exactly 'other_artifacts_0.1.2/',
+                             'other_artifacts_0.1.2/doc_sample.txt',
+                             'other_artifacts_0.1.2/another-subdirectory/',
+                             'other_artifacts_0.1.2/another-subdirectory/empty_directory/',
+                             'other_artifacts_0.1.2/another-subdirectory/banana_sample.gif'
+      end
+    end
+
     describe '#to_entry' do
       subject { metadata('').to_entry }
       it { is_expected.to be_an_instance_of(Gitlab::Ci::Build::Artifacts::Metadata::Entry) }

spec/models/build_spec.rb

@@ -362,12 +362,12 @@ describe Ci::Build, models: true do
     subject { build.artifacts_browse_url }
 
     it "should be nil if artifacts browser is unsupported" do
-      allow(build).to receive(:artifacts_browser_supported?).and_return(false)
+      allow(build).to receive(:artifacts_metadata?).and_return(false)
       is_expected.to be_nil
     end
 
     it 'should not be nil if artifacts browser is supported' do
-      allow(build).to receive(:artifacts_browser_supported?).and_return(true)
+      allow(build).to receive(:artifacts_metadata?).and_return(true)
       is_expected.to_not be_nil
     end
   end
@@ -391,8 +391,8 @@ describe Ci::Build, models: true do
   end
 
 
-  describe :artifacts_browser_supported? do
-    subject { build.artifacts_browser_supported? }
+  describe :artifacts_metadata? do
+    subject { build.artifacts_metadata? }
     context 'artifacts metadata does not exist' do
       it { is_expected.to be_falsy }
     end

spec/services/projects/update_pages_worker_spec.rb

@@ -4,9 +4,7 @@ describe Projects::UpdatePagesService do
   let(:project) { create :project }
   let(:commit) { create :ci_commit, project: project, sha: project.commit('HEAD').sha }
   let(:build) { create :ci_build, commit: commit, ref: 'HEAD' }
-  let(:file) { fixture_file_upload(Rails.root + 'spec/fixtures/pages.tar.gz', 'application/octet-stream') }
-  let(:empty_file) { fixture_file_upload(Rails.root + 'spec/fixtures/pages_empty.tar.gz', 'application/octet-stream') }
-  let(:invalid_file) { fixture_file_upload(Rails.root + 'spec/fixtures/dk.png', 'application/octet-stream') }
+  let(:invalid_file) { fixture_file_upload(Rails.root + 'spec/fixtures/dk.png') }
   
   subject { described_class.new(project, build) }
 
@@ -14,27 +12,50 @@ describe Projects::UpdatePagesService do
     project.remove_pages
   end
 
-  context 'for valid file' do
-    before { build.update_attributes(artifacts_file: file) }
+  %w(tar.gz zip).each do |format|
+    context "for valid #{format}" do
+      let(:file) { fixture_file_upload(Rails.root + "spec/fixtures/pages.#{format}") }
+      let(:empty_file) { fixture_file_upload(Rails.root + "spec/fixtures/pages_empty.#{format}") }
+      let(:metadata) do
+        filename = Rails.root + "spec/fixtures/pages.#{format}.meta"
+        fixture_file_upload(filename) if File.exists?(filename)
+      end
 
-    it 'succeeds' do
-      expect(project.pages_url).to be_nil
-      expect(execute).to eq(:success)
-      expect(project.pages_url).to_not be_nil
-    end
+      before do
+        build.update_attributes(artifacts_file: file)
+        build.update_attributes(artifacts_metadata: metadata)
+      end
 
-    it 'limits pages size' do
-      stub_application_setting(max_pages_size: 1)
-      expect(execute).to_not eq(:success)
-    end
+      it 'succeeds' do
+        expect(project.pages_url).to be_nil
+        expect(execute).to eq(:success)
+        expect(project.pages_url).to_not be_nil
+      end
+
+      it 'limits pages size' do
+        stub_application_setting(max_pages_size: 1)
+        expect(execute).to_not eq(:success)
+      end
 
-    it 'removes pages after destroy' do
-      expect(PagesWorker).to receive(:perform_in)
-      expect(project.pages_url).to be_nil
-      expect(execute).to eq(:success)
-      expect(project.pages_url).to_not be_nil
-      project.destroy
-      expect(Dir.exist?(project.public_pages_path)).to be_falsey
+      it 'removes pages after destroy' do
+        expect(PagesWorker).to receive(:perform_in)
+        expect(project.pages_url).to be_nil
+        expect(execute).to eq(:success)
+        expect(project.pages_url).to_not be_nil
+        project.destroy
+        expect(Dir.exist?(project.public_pages_path)).to be_falsey
+      end
+
+      it 'fails if sha on branch is not latest' do
+        commit.update_attributes(sha: 'old_sha')
+        build.update_attributes(artifacts_file: file)
+        expect(execute).to_not eq(:success)
+      end
+
+      it 'fails for empty file fails' do
+        build.update_attributes(artifacts_file: empty_file)
+        expect(execute).to_not eq(:success)
+      end
     end
   end
 
@@ -48,21 +69,10 @@ describe Projects::UpdatePagesService do
     expect(execute).to_not eq(:success)
   end
 
-  it 'fails for empty file fails' do
-    build.update_attributes(artifacts_file: empty_file)
-    expect(execute).to_not eq(:success)
-  end
-
   it 'fails for invalid archive' do
     build.update_attributes(artifacts_file: invalid_file)
     expect(execute).to_not eq(:success)
   end
-
-  it 'fails if sha on branch is not latest' do
-    commit.update_attributes(sha: 'old_sha')
-    build.update_attributes(artifacts_file: file)
-    expect(execute).to_not eq(:success)
-  end
   
   def execute
     subject.execute[:status]