Merge remote-tracking branch 'upstream/master' into ce-to-ee-2018-12-10

app/assets/javascripts/image_diff/helpers/badge_helper.js

@@ -12,7 +12,7 @@ export function createImageBadge(noteId, { x, y }, classNames = []) {
 }
 
 export function addImageBadge(containerEl, { coordinate, badgeText, noteId }) {
-  const buttonEl = createImageBadge(noteId, coordinate, ['badge']);
+  const buttonEl = createImageBadge(noteId, coordinate, ['badge', 'badge-pill']);
   buttonEl.innerText = badgeText;
 
   containerEl.appendChild(buttonEl);

app/assets/stylesheets/pages/diff.scss

@@ -914,6 +914,7 @@
     padding: 0;
     width: (2px * $image-comment-cursor-left-offset);
     height: (2px * $image-comment-cursor-top-offset);
+    color: $blue-400;
     // center the indicator to match the top left click region
     margin-top: (-1px * $image-comment-cursor-top-offset) + 2;
     margin-left: (-1px * $image-comment-cursor-left-offset) + 1;

app/models/project.rb

@@ -656,6 +656,11 @@ class Project < ActiveRecord::Base
     end
   end
 
+  def latest_successful_build_for(job_name, ref = default_branch)
+    builds = latest_successful_builds_for(ref)
+    builds.find_by!(name: job_name)
+  end
+
   def merge_base_commit(first_commit_id, second_commit_id)
     sha = repository.merge_base(first_commit_id, second_commit_id)
     commit_by(oid: sha) if sha

changelogs/unreleased/54160-use-reports-syntax-for-sast-in-auto-devops.yml

+---
+title: Use reports syntax for SAST in Auto DevOps
+merge_request: 23163
+author:
+type: changed

changelogs/unreleased/54626-able-to-download-a-single-archive-file-with-api-by-ref-name.yml

+---
+title: Add new endpoint to download single artifact file for a ref
+merge_request: 23538
+author:
+type: added

changelogs/unreleased/commit-badge-style-fix.yml

+---
+title: Fixed styling of image comment badges on commits
+merge_request:
+author:
+type: fixed

changelogs/unreleased/osw-update-mr-metrics-with-events-data.yml

+---
+title: Populate MR metrics with events table information (migration)
+merge_request: 23564
+author:
+type: performance

db/post_migrate/20181204154019_populate_mr_metrics_with_events_data.rb

+# frozen_string_literal: true
+
+# See http://doc.gitlab.com/ce/development/migration_style_guide.html
+# for more information on how to write migrations for GitLab.
+
+class PopulateMrMetricsWithEventsData < ActiveRecord::Migration[4.2]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+  BATCH_SIZE = 10_000
+  MIGRATION = 'PopulateMergeRequestMetricsWithEventsDataImproved'
+  PREVIOUS_MIGRATION = 'PopulateMergeRequestMetricsWithEventsData'
+
+  disable_ddl_transaction!
+
+  def up
+    # Perform any ongoing background migration that might still be running from
+    # previous try (see https://gitlab.com/gitlab-org/gitlab-ce/issues/47676).
+    Gitlab::BackgroundMigration.steal(PREVIOUS_MIGRATION)
+
+    say 'Scheduling `PopulateMergeRequestMetricsWithEventsData` jobs'
+    # It will update around 4_000_000 records in batches of 10_000 merge
+    # requests (running between 5 minutes) and should take around 53 hours to complete.
+    # Apparently, production PostgreSQL is able to vacuum 10k-20k dead_tuples
+    # per minute. So this should give us enough space.
+    #
+    # More information about the updates in `PopulateMergeRequestMetricsWithEventsDataImproved` class.
+    #
+    MergeRequest.all.each_batch(of: BATCH_SIZE) do |relation, index|
+      range = relation.pluck('MIN(id)', 'MAX(id)').first
+
+      BackgroundMigrationWorker.perform_in(index * 8.minutes, MIGRATION, range)
+    end
+  end
+
+  def down
+  end
+end

doc/api/jobs.md

@@ -438,7 +438,7 @@ Example response:
 
 [ce-5347]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/5347
 
-## Download a single artifact file
+## Download a single artifact file by job ID
 
 > Introduced in GitLab 10.0
 
@@ -472,6 +472,41 @@ Example response:
 | 400       | Invalid path provided                |
 | 404       | Build not found or no file/artifacts |
 
+## Download a single artifact file from specific tag or branch
+
+> [Introduced](https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/23538) in GitLab 11.5.
+
+Download a single artifact file from a specific tag or branch from within the
+job's artifacts archive. The file is extracted from the archive and streamed to
+the client.
+
+```
+GET /projects/:id/jobs/artifacts/:ref_name/raw/*artifact_path?job=name
+```
+
+Parameters:
+
+| Attribute       | Type           | Required | Description                                                                                                      |
+|-----------------|----------------|----------|------------------------------------------------------------------------------------------------------------------|
+| `id`            | integer/string | yes      | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) owned by the authenticated user. |
+| `ref_name`      | string         | yes      | Branch or tag name in repository. HEAD or SHA references are not supported.                                      |
+| `artifact_path` | string         | yes      | Path to a file inside the artifacts archive.                                                                     |
+| `job`           | string         | yes      | The name of the job.                                                                                             |
+
+Example request:
+
+```sh
+curl --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.com/api/v4/projects/1/jobs/artifacts/master/raw/some/release/file.pdf?job=pdf"
+```
+
+Possible response status codes:
+
+| Status    | Description                          |
+|-----------|--------------------------------------|
+| 200       | Sends a single artifact file         |
+| 400       | Invalid path provided                |
+| 404       | Build not found or no file/artifacts |
+
 ## Get a trace file
 
 Get a trace of a specific job of a project

lib/api/job_artifacts.rb

@@ -37,6 +37,29 @@ module API
       end
       # rubocop: enable CodeReuse/ActiveRecord
 
+      desc 'Download a specific file from artifacts archive from a ref' do
+        detail 'This feature was introduced in GitLab 11.5'
+      end
+      params do
+        requires :ref_name, type: String, desc: 'The ref from repository'
+        requires :job, type: String, desc: 'The name for the job'
+        requires :artifact_path, type: String, desc: 'Artifact path'
+      end
+      get ':id/jobs/artifacts/:ref_name/raw/*artifact_path',
+          format: false,
+          requirements: { ref_name: /.+/ } do
+        authorize_download_artifacts!
+
+        build = user_project.latest_successful_build_for(params[:job], params[:ref_name])
+
+        path = Gitlab::Ci::Build::Artifacts::Path
+                 .new(params[:artifact_path])
+
+        bad_request! unless path.valid?
+
+        send_artifacts_entry(build, path)
+      end
+
       desc 'Download the artifacts archive from a job' do
         detail 'This feature was introduced in GitLab 8.5'
       end
@@ -67,6 +90,7 @@ module API
 
         path = Gitlab::Ci::Build::Artifacts::Path
           .new(params[:artifact_path])
+
         bad_request! unless path.valid?
 
         send_artifacts_entry(build, path)

lib/gitlab/background_migration/populate_merge_request_metrics_with_events_data_improved.rb

+# frozen_string_literal: true
+# rubocop:disable Style/Documentation
+
+module Gitlab
+  module BackgroundMigration
+    class PopulateMergeRequestMetricsWithEventsDataImproved
+      CLOSED_EVENT_ACTION = 3
+      MERGED_EVENT_ACTION = 7
+
+      def perform(min_merge_request_id, max_merge_request_id)
+        insert_metrics_for_range(min_merge_request_id, max_merge_request_id)
+        update_metrics_with_events_data(min_merge_request_id, max_merge_request_id)
+      end
+
+      # Inserts merge_request_metrics records for merge_requests without it for
+      # a given merge request batch.
+      def insert_metrics_for_range(min, max)
+        metrics_not_exists_clause =
+          <<-SQL.strip_heredoc
+            NOT EXISTS (SELECT 1 FROM merge_request_metrics
+                        WHERE merge_request_metrics.merge_request_id = merge_requests.id)
+          SQL
+
+        MergeRequest.where(metrics_not_exists_clause).where(id: min..max).each_batch do |batch|
+          select_sql = batch.select(:id, :created_at, :updated_at).to_sql
+
+          execute("INSERT INTO merge_request_metrics (merge_request_id, created_at, updated_at) #{select_sql}")
+        end
+      end
+
+      def update_metrics_with_events_data(min, max)
+        if Gitlab::Database.postgresql?
+          psql_update_metrics_with_events_data(min, max)
+        else
+          mysql_update_metrics_with_events_data(min, max)
+        end
+      end
+
+      def psql_update_metrics_with_events_data(min, max)
+        update_sql = <<-SQL.strip_heredoc
+          UPDATE merge_request_metrics
+          SET (latest_closed_at,
+             latest_closed_by_id) =
+          ( SELECT updated_at,
+                   author_id
+           FROM events
+           WHERE target_id = merge_request_id
+             AND target_type = 'MergeRequest'
+             AND action = #{CLOSED_EVENT_ACTION}
+           ORDER BY id DESC
+           LIMIT 1 ),
+             merged_by_id =
+          ( SELECT author_id
+           FROM events
+           WHERE target_id = merge_request_id
+             AND target_type = 'MergeRequest'
+             AND action = #{MERGED_EVENT_ACTION}
+           ORDER BY id DESC
+           LIMIT 1 )
+          WHERE merge_request_id BETWEEN #{min} AND #{max}
+        SQL
+
+        execute(update_sql)
+      end
+
+      def mysql_update_metrics_with_events_data(min, max)
+        closed_updated_at_subquery = mysql_events_select(:updated_at, CLOSED_EVENT_ACTION)
+        closed_author_id_subquery = mysql_events_select(:author_id, CLOSED_EVENT_ACTION)
+        merged_author_id_subquery = mysql_events_select(:author_id, MERGED_EVENT_ACTION)
+
+        update_sql = <<-SQL.strip_heredoc
+          UPDATE merge_request_metrics
+          SET latest_closed_at = (#{closed_updated_at_subquery}),
+              latest_closed_by_id = (#{closed_author_id_subquery}),
+              merged_by_id = (#{merged_author_id_subquery})
+          WHERE merge_request_id BETWEEN #{min} AND #{max}
+        SQL
+
+        execute(update_sql)
+      end
+
+      def mysql_events_select(column, action)
+        <<-SQL.strip_heredoc
+          SELECT #{column} FROM events
+           WHERE target_id = merge_request_id
+             AND target_type = 'MergeRequest'
+             AND action = #{action}
+           ORDER BY id DESC
+           LIMIT 1
+        SQL
+      end
+
+      def execute(sql)
+        @connection ||= ActiveRecord::Base.connection
+        @connection.execute(sql)
+      end
+    end
+  end
+end

lib/gitlab/ci/templates/Auto-DevOps.gitlab-ci.yml

@@ -164,7 +164,8 @@ sast:
     - setup_docker
     - sast
   artifacts:
-    paths: [gl-sast-report.json]
+    reports:
+      sast: gl-sast-report.json
   only:
     refs:
       - branches

spec/javascripts/image_diff/helpers/badge_helper_spec.js

@@ -61,6 +61,10 @@ describe('badge helper', () => {
       expect(buttonEl).toBeDefined();
     });
 
+    it('should add badge classes', () => {
+      expect(buttonEl.className).toContain('badge badge-pill');
+    });
+
     it('should set the badge text', () => {
       expect(buttonEl.innerText).toEqual(badgeText);
     });

spec/lib/gitlab/background_migration/populate_merge_request_metrics_with_events_data_improved_spec.rb

+# frozen_string_literal: true
+
+require 'rails_helper'
+
+describe Gitlab::BackgroundMigration::PopulateMergeRequestMetricsWithEventsDataImproved, :migration, schema: 20181204154019 do
+  let(:namespaces) { table(:namespaces) }
+  let(:projects) { table(:projects) }
+  let(:users) { table(:users) }
+  let(:events) { table(:events) }
+
+  let(:user) { users.create!(email: 'test@example.com', projects_limit: 100, username: 'test') }
+
+  let(:namespace) { namespaces.create(name: 'gitlab', path: 'gitlab-org') }
+  let(:project) { projects.create(namespace_id: namespace.id, name: 'foo') }
+  let(:merge_requests) { table(:merge_requests) }
+
+  def create_merge_request(id, params = {})
+    params.merge!(id: id,
+                  target_project_id: project.id,
+                  target_branch: 'master',
+                  source_project_id: project.id,
+                  source_branch: 'mr name',
+                  title: "mr name#{id}")
+
+    merge_requests.create(params)
+  end
+
+  def create_merge_request_event(id, params = {})
+    params.merge!(id: id,
+                  project_id: project.id,
+                  author_id: user.id,
+                  target_type: 'MergeRequest')
+
+    events.create(params)
+  end
+
+  describe '#perform' do
+    it 'creates and updates closed and merged events' do
+      timestamp = Time.new('2018-01-01 12:00:00').utc
+
+      create_merge_request(1)
+      create_merge_request_event(1, target_id: 1, action: 3, updated_at: timestamp)
+      create_merge_request_event(2, target_id: 1, action: 3, updated_at: timestamp + 10.seconds)
+
+      create_merge_request_event(3, target_id: 1, action: 7, updated_at: timestamp)
+      create_merge_request_event(4, target_id: 1, action: 7, updated_at: timestamp + 10.seconds)
+
+      subject.perform(1, 1)
+
+      merge_request = MergeRequest.first
+
+      expect(merge_request.metrics).to have_attributes(latest_closed_by_id: user.id,
+                                                       latest_closed_at: timestamp + 10.seconds,
+                                                       merged_by_id: user.id)
+    end
+  end
+end

spec/migrations/populate_mr_metrics_with_events_data_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+require Rails.root.join('db', 'post_migrate', '20181204154019_populate_mr_metrics_with_events_data.rb')
+
+describe PopulateMrMetricsWithEventsData, :migration, :sidekiq do
+  let(:namespaces) { table(:namespaces) }
+  let(:projects) { table(:projects) }
+  let(:namespace) { namespaces.create(name: 'gitlab', path: 'gitlab-org') }
+  let(:project) { projects.create(namespace_id: namespace.id, name: 'foo') }
+  let(:merge_requests) { table(:merge_requests) }
+
+  def create_merge_request(id)
+    params = {
+      id: id,
+      target_project_id: project.id,
+      target_branch: 'master',
+      source_project_id: project.id,
+      source_branch: 'mr name',
+      title: "mr name#{id}"
+    }
+
+    merge_requests.create!(params)
+  end
+
+  it 'correctly schedules background migrations' do
+    create_merge_request(1)
+    create_merge_request(2)
+    create_merge_request(3)
+
+    stub_const("#{described_class.name}::BATCH_SIZE", 2)
+
+    Sidekiq::Testing.fake! do
+      Timecop.freeze do
+        migrate!
+
+        expect(described_class::MIGRATION)
+          .to be_scheduled_delayed_migration(8.minutes, 1, 2)
+
+        expect(described_class::MIGRATION)
+          .to be_scheduled_delayed_migration(16.minutes, 3, 3)
+
+        expect(BackgroundMigrationWorker.jobs.size).to eq(2)
+      end
+    end
+  end
+end

spec/models/project_spec.rb

@@ -2060,7 +2060,7 @@ describe Project do
     end
   end
 
-  describe '#latest_successful_builds_for' do
+  describe '#latest_successful_builds_for and #latest_successful_build_for' do
     def create_pipeline(status = 'success')
       create(:ci_pipeline, project: project,
                            sha: project.commit.sha,
@@ -2082,14 +2082,16 @@ describe Project do
       it 'gives the latest builds from latest pipeline' do
         pipeline1 = create_pipeline
         pipeline2 = create_pipeline
-        build1_p2 = create_build(pipeline2, 'test')
         create_build(pipeline1, 'test')
         create_build(pipeline1, 'test2')
+        build1_p2 = create_build(pipeline2, 'test')
         build2_p2 = create_build(pipeline2, 'test2')
 
         latest_builds = project.latest_successful_builds_for
+        single_build = project.latest_successful_build_for(build1_p2.name)
 
         expect(latest_builds).to contain_exactly(build2_p2, build1_p2)
+        expect(single_build).to eq(build1_p2)
       end
     end
 
@@ -2099,16 +2101,22 @@ describe Project do
       context 'standalone pipeline' do
         it 'returns builds for ref for default_branch' do
           builds = project.latest_successful_builds_for
+          single_build = project.latest_successful_build_for(build.name)
 
           expect(builds).to contain_exactly(build)
+          expect(single_build).to eq(build)
         end
 
-        it 'returns empty relation if the build cannot be found' do
+        it 'returns empty relation if the build cannot be found for #latest_successful_builds_for' do
           builds = project.latest_successful_builds_for('TAIL')
 
           expect(builds).to be_kind_of(ActiveRecord::Relation)
           expect(builds).to be_empty
         end
+
+        it 'returns exception if the build cannot be found for #latest_successful_build_for' do
+          expect { project.latest_successful_build_for(build.name, 'TAIL') }.to raise_error(ActiveRecord::RecordNotFound)
+        end
       end
 
       context 'with some pending pipeline' do
@@ -2117,9 +2125,11 @@ describe Project do
         end
 
         it 'gives the latest build from latest pipeline' do
-          latest_build = project.latest_successful_builds_for
+          latest_builds = project.latest_successful_builds_for
+          last_single_build = project.latest_successful_build_for(build.name)
 
-          expect(latest_build).to contain_exactly(build)
+          expect(latest_builds).to contain_exactly(build)
+          expect(last_single_build).to eq(build)
         end
       end
     end

spec/requests/api/jobs_spec.rb

@@ -610,6 +610,136 @@ describe API::Jobs do
     end
   end
 
+  describe 'GET id/jobs/artifacts/:ref_name/raw/*artifact_path?job=name' do
+    context 'when job has artifacts' do
+      let(:job) { create(:ci_build, :artifacts, pipeline: pipeline, user: api_user) }
+      let(:artifact) { 'other_artifacts_0.1.2/another-subdirectory/banana_sample.gif' }
+      let(:visibility_level) { Gitlab::VisibilityLevel::PUBLIC }
+      let(:public_builds) { true }
+
+      before do
+        stub_artifacts_object_storage
+        job.success
+
+        project.update(visibility_level: visibility_level,
+                       public_builds: public_builds)
+
+        get_artifact_file(artifact)
+      end
+
+      context 'when user is anonymous' do
+        let(:api_user) { nil }
+
+        context 'when project is public' do
+          let(:visibility_level) { Gitlab::VisibilityLevel::PUBLIC }
+          let(:public_builds) { true }
+
+          it 'allows to access artifacts' do
+            expect(response).to have_gitlab_http_status(200)
+            expect(response.headers.to_h)
+              .to include('Content-Type' => 'application/json',
+                          'Gitlab-Workhorse-Send-Data' => /artifacts-entry/)
+          end
+        end
+
+        context 'when project is public with builds access disabled' do
+          let(:visibility_level) { Gitlab::VisibilityLevel::PUBLIC }
+          let(:public_builds) { false }
+
+          it 'rejects access to artifacts' do
+            expect(response).to have_gitlab_http_status(403)
+            expect(json_response).to have_key('message')
+            expect(response.headers.to_h)
+              .not_to include('Gitlab-Workhorse-Send-Data' => /artifacts-entry/)
+          end
+        end
+
+        context 'when project is private' do
+          let(:visibility_level) { Gitlab::VisibilityLevel::PRIVATE }
+          let(:public_builds) { true }
+
+          it 'rejects access and hides existence of artifacts' do
+            expect(response).to have_gitlab_http_status(404)
+            expect(json_response).to have_key('message')
+            expect(response.headers.to_h)
+              .not_to include('Gitlab-Workhorse-Send-Data' => /artifacts-entry/)
+          end
+        end
+      end
+
+      context 'when user is authorized' do
+        let(:visibility_level) { Gitlab::VisibilityLevel::PRIVATE }
+        let(:public_builds) { true }
+
+        it 'returns a specific artifact file for a valid path' do
+          expect(Gitlab::Workhorse)
+            .to receive(:send_artifacts_entry)
+                  .and_call_original
+
+          get_artifact_file(artifact)
+
+          expect(response).to have_gitlab_http_status(200)
+          expect(response.headers.to_h)
+            .to include('Content-Type' => 'application/json',
+                        'Gitlab-Workhorse-Send-Data' => /artifacts-entry/)
+        end
+      end
+
+      context 'with branch name containing slash' do
+        before do
+          pipeline.reload
+          pipeline.update(ref: 'improve/awesome',
+                          sha: project.commit('improve/awesome').sha)
+        end
+
+        it 'returns a specific artifact file for a valid path' do
+          get_artifact_file(artifact, 'improve/awesome')
+
+          expect(response).to have_gitlab_http_status(200)
+          expect(response.headers.to_h)
+            .to include('Content-Type' => 'application/json',
+                        'Gitlab-Workhorse-Send-Data' => /artifacts-entry/)
+        end
+      end
+
+      context 'non-existing job' do
+        shared_examples 'not found' do
+          it { expect(response).to have_gitlab_http_status(:not_found) }
+        end
+
+        context 'has no such ref' do
+          before do
+            get_artifact_file('some/artifact', 'wrong-ref')
+          end
+
+          it_behaves_like 'not found'
+        end
+
+        context 'has no such job' do
+          before do
+            get_artifact_file('some/artifact', pipeline.ref, 'wrong-job-name')
+          end
+
+          it_behaves_like 'not found'
+        end
+      end
+    end
+
+    context 'when job does not have artifacts' do
+      let(:job) { create(:ci_build, pipeline: pipeline, user: api_user) }
+
+      it 'does not return job artifact file' do
+        get_artifact_file('some/artifact')
+
+        expect(response).to have_gitlab_http_status(404)
+      end
+    end
+
+    def get_artifact_file(artifact_path, ref = pipeline.ref, job_name = job.name)
+      get api("/projects/#{project.id}/jobs/artifacts/#{ref}/raw/#{artifact_path}", api_user), job: job_name
+    end
+  end
+
   describe 'GET /projects/:id/jobs/:job_id/trace' do
     before do
       get api("/projects/#{project.id}/jobs/#{job.id}/trace", api_user)