Add latest changes from gitlab-org/gitlab@master

spec/fixtures/lib/gitlab/import_export/light/project.json

@@ -111,6 +111,28 @@
       "active": false,
       "properties": {},
       "template": true,
+      "instance": false,
+      "push_events": true,
+      "issues_events": true,
+      "merge_requests_events": true,
+      "tag_push_events": true,
+      "note_events": true,
+      "job_events": true,
+      "type": "TeamcityService",
+      "category": "ci",
+      "default": false,
+      "wiki_page_events": true
+    },
+    {
+      "id": 101,
+      "title": "JetBrains TeamCity CI",
+      "project_id": 5,
+      "created_at": "2016-06-14T15:01:51.315Z",
+      "updated_at": "2016-06-14T15:01:51.315Z",
+      "active": false,
+      "properties": {},
+      "template": false,
+      "instance": true,
       "push_events": true,
       "issues_events": true,
       "merge_requests_events": true,

spec/frontend/monitoring/components/charts/time_series_spec.js

@@ -69,7 +69,7 @@ describe('Time series component', () => {
         mockedQueryResultFixture,
       );
       // dashboard is a dynamically generated fixture and stored at environment_metrics_dashboard.json
-      [mockGraphData] = store.state.monitoringDashboard.dashboard.panelGroups[0].panels;
+      [mockGraphData] = store.state.monitoringDashboard.dashboard.panelGroups[1].panels;
     });
     describe('general functions', () => {

spec/frontend/monitoring/store/getters_spec.js

@@ -89,8 +89,8 @@ describe('Monitoring store Getters', () => {
         expect(getMetricStates()).toEqual([metricStates.OK]);
         // Filtered by groups
-        expect(getMetricStates(state.dashboard.panelGroups[0].key)).toEqual([metricStates.OK]);
-        expect(getMetricStates(state.dashboard.panelGroups[1].key)).toEqual([]);
+        expect(getMetricStates(state.dashboard.panelGroups[1].key)).toEqual([metricStates.OK]);
+        expect(getMetricStates(state.dashboard.panelGroups[2].key)).toEqual([]);
       });
       it('on multiple metrics errors', () => {
         mutations[types.RECEIVE_METRICS_DATA_SUCCESS](state, metricsDashboardPayload);
@@ -118,18 +118,18 @@ describe('Monitoring store Getters', () => {
         mutations[types.RECEIVE_METRIC_RESULT_SUCCESS](state, mockedQueryResultFixture);
         // An error in 2 groups
         mutations[types.RECEIVE_METRIC_RESULT_FAILURE](state, {
-          metricId: groups[0].panels[1].metrics[0].metricId,
+          metricId: groups[1].panels[1].metrics[0].metricId,
         });
         mutations[types.RECEIVE_METRIC_RESULT_FAILURE](state, {
-          metricId: groups[1].panels[0].metrics[0].metricId,
+          metricId: groups[2].panels[0].metrics[0].metricId,
         });
         expect(getMetricStates()).toEqual([metricStates.OK, metricStates.UNKNOWN_ERROR]);
-        expect(getMetricStates(groups[0].key)).toEqual([
+        expect(getMetricStates(groups[1].key)).toEqual([
           metricStates.OK,
           metricStates.UNKNOWN_ERROR,
         ]);
-        expect(getMetricStates(groups[1].key)).toEqual([metricStates.UNKNOWN_ERROR]);
+        expect(getMetricStates(groups[2].key)).toEqual([metricStates.UNKNOWN_ERROR]);
       });
     });
   });
@@ -210,13 +210,13 @@ describe('Monitoring store Getters', () => {
         mutations[types.RECEIVE_METRIC_RESULT_SUCCESS](state, mockedQueryResultFixtureStatusCode);
         // First group has metrics
-        expect(metricsWithData(state.dashboard.panelGroups[0].key)).toEqual([
+        expect(metricsWithData(state.dashboard.panelGroups[1].key)).toEqual([
           mockedQueryResultFixture.metricId,
           mockedQueryResultFixtureStatusCode.metricId,
         ]);
         // Second group has no metrics
-        expect(metricsWithData(state.dashboard.panelGroups[1].key)).toEqual([]);
+        expect(metricsWithData(state.dashboard.panelGroups[2].key)).toEqual([]);
       });
     });
   });

spec/frontend/monitoring/store/mutations_spec.js

@@ -32,12 +32,13 @@ describe('Monitoring mutations', () => {
       mutations[types.RECEIVE_METRICS_DATA_SUCCESS](stateCopy, payload);
       const groups = getGroups();
-      expect(groups[0].key).toBe('response-metrics-nginx-ingress-vts-0');
-      expect(groups[1].key).toBe('response-metrics-nginx-ingress-1');
+      expect(groups[0].key).toBe('system-metrics-kubernetes-0');
+      expect(groups[1].key).toBe('response-metrics-nginx-ingress-vts-1');
+      expect(groups[2].key).toBe('response-metrics-nginx-ingress-2');
     });
     it('normalizes values', () => {
       mutations[types.RECEIVE_METRICS_DATA_SUCCESS](stateCopy, payload);
-      const expectedLabel = '5xx Errors (%)';
+      const expectedLabel = 'Pod average (MB)';
       const { label, queryRange } = getGroups()[0].panels[2].metrics[0];
       expect(label).toEqual(expectedLabel);
@@ -51,7 +52,7 @@ describe('Monitoring mutations', () => {
       expect(groups).toBeDefined();
       expect(groups).toHaveLength(6);
-      expect(groups[0].panels).toHaveLength(3);
+      expect(groups[0].panels).toHaveLength(7);
       expect(groups[0].panels[0].metrics).toHaveLength(1);
       expect(groups[0].panels[1].metrics).toHaveLength(1);
       expect(groups[0].panels[2].metrics).toHaveLength(1);
@@ -65,9 +66,12 @@ describe('Monitoring mutations', () => {
       const groups = getGroups();
       expect(groups[0].panels[0].metrics[0].metricId).toEqual(
-        'undefined_response_metrics_nginx_ingress_throughput_status_code',
+        'undefined_system_metrics_kubernetes_container_memory_total',
       );
       expect(groups[1].panels[0].metrics[0].metricId).toEqual(
+        'undefined_response_metrics_nginx_ingress_throughput_status_code',
+      );
+      expect(groups[2].panels[0].metrics[0].metricId).toEqual(
         'undefined_response_metrics_nginx_ingress_16_throughput_status_code',
       );
     });
@@ -135,7 +139,7 @@ describe('Monitoring mutations', () => {
       },
     ];
     const dashboard = metricsDashboardPayload;
-    const getMetric = () => stateCopy.dashboard.panelGroups[0].panels[0].metrics[0];
+    const getMetric = () => stateCopy.dashboard.panelGroups[1].panels[0].metrics[0];
     describe('REQUEST_METRIC_RESULT', () => {
       beforeEach(() => {

spec/lib/banzai/filter/inline_grafana_metrics_filter_spec.rb

@@ -46,11 +46,9 @@ describe Banzai::Filter::InlineGrafanaMetricsFilter do
   end
   context 'when "panelId" parameter is missing' do
-    let(:dashboard_path) { '/d/XDaNK6amz/gitlab-omnibus-redis' }
-    it 'leaves the markdown unchanged' do
-      expect(unescape(doc.to_s)).to eq(input)
-    end
+    let(:dashboard_path) { '/d/XDaNK6amz/gitlab-omnibus-redis?from=1570397739557&to=1570484139557' }
+    it_behaves_like 'a metrics embed filter'
   end
   context 'when time window parameters are missing' do
@@ -86,6 +84,14 @@ describe Banzai::Filter::InlineGrafanaMetricsFilter do
     end
   end
+  context 'when no parameters are provided' do
+    let(:dashboard_path) { '/d/XDaNK6amz/gitlab-omnibus-redis' }
+
+    it 'inserts a placeholder' do
+      expect(embed_url).to be_present
+    end
+  end
+
   private
   # Nokogiri escapes the URLs, but we don't care about that

spec/lib/gitlab/import_export/project/tree_restorer_spec.rb

@@ -703,6 +703,12 @@ describe Gitlab::ImportExport::Project::TreeRestorer do
         expect(project.services.where(template: true).count).to eq(0)
       end
+      it 'does not import any instance services' do
+        expect(restored_project_json).to eq(true)
+
+        expect(project.services.where(instance: true).count).to eq(0)
+      end
+
       it 'imports labels' do
         create(:group_label, name: 'Another label', group: project.group)

spec/lib/gitlab/import_export/safe_model_attributes.yml

@@ -459,6 +459,7 @@ Service:
 - active
 - properties
 - template
+- instance
 - push_events
 - issues_events
 - commit_events

spec/lib/gitlab/metrics/dashboard/stages/grafana_formatter_spec.rb

@@ -11,8 +11,9 @@ describe Gitlab::Metrics::Dashboard::Stages::GrafanaFormatter do
   describe '#transform!' do
     let(:grafana_dashboard) { JSON.parse(fixture_file('grafana/simplified_dashboard_response.json'), symbolize_names: true) }
     let(:datasource) { JSON.parse(fixture_file('grafana/datasource_response.json'), symbolize_names: true) }
-    let(:dashboard) { described_class.new(project, {}, params).transform! }
+    let(:expected_dashboard) { JSON.parse(fixture_file('grafana/expected_grafana_embed.json'), symbolize_names: true) }
+    subject(:dashboard) { described_class.new(project, {}, params).transform! }
     let(:params) do
       {
@@ -23,83 +24,34 @@ describe Gitlab::Metrics::Dashboard::Stages::GrafanaFormatter do
     end
     context 'when the query and resources are configured correctly' do
-      let(:expected_dashboard) { JSON.parse(fixture_file('grafana/expected_grafana_embed.json'), symbolize_names: true) }
-
-      it 'generates a gitlab-yml formatted dashboard' do
-        expect(dashboard).to eq(expected_dashboard)
-      end
+      it { is_expected.to eq expected_dashboard }
     end
-    context 'when the inputs are invalid' do
-      shared_examples_for 'processing error' do
-        it 'raises a processing error' do
-          expect { dashboard }
-            .to raise_error(Gitlab::Metrics::Dashboard::Stages::InputFormatValidator::DashboardProcessingError)
-        end
-      end
-
-      context 'when the datasource is not proxyable' do
-        before do
-          params[:datasource][:access] = 'not-proxy'
-        end
-
-        it_behaves_like 'processing error'
+    context 'when a panelId is not included in the grafana_url' do
+      before do
+        params[:grafana_url].gsub('&panelId=8', '')
       end
-      context 'when query param "panelId" is not specified' do
-        before do
-          params[:grafana_url].gsub!('panelId=8', '')
-        end
-
-        it_behaves_like 'processing error'
-      end
-
-      context 'when query param "from" is not specified' do
-        before do
-          params[:grafana_url].gsub!('from=1570397739557', '')
-        end
-
-        it_behaves_like 'processing error'
-      end
-      context 'when query param "to" is not specified' do
+      it { is_expected.to eq expected_dashboard }
+      context 'when there is also no valid panel in the dashboard' do
         before do
-          params[:grafana_url].gsub!('to=1570484139557', '')
+          params[:grafana_dashboard][:dashboard][:panels] = []
         end
-        it_behaves_like 'processing error'
-      end
-
-      context 'when the panel is not a graph' do
-        before do
-          params[:grafana_dashboard][:dashboard][:panels][0][:type] = 'singlestat'
+        it 'raises a processing error' do
+          expect { dashboard }.to raise_error(::Gitlab::Metrics::Dashboard::Errors::DashboardProcessingError)
         end
-
-        it_behaves_like 'processing error'
       end
-      context 'when the panel is not a line graph' do
-        before do
-          params[:grafana_dashboard][:dashboard][:panels][0][:lines] = false
-        end
-
-        it_behaves_like 'processing error'
-      end
-
-      context 'when the query dashboard includes undefined variables' do
-        before do
-          params[:grafana_url].gsub!('&var-instance=localhost:9121', '')
-        end
-
-        it_behaves_like 'processing error'
+    end
+    context 'when an input is invalid' do
+      before do
+        params[:datasource][:access] = 'not-proxy'
       end
-      context 'when the expression contains unsupported global variables' do
-        before do
-          params[:grafana_dashboard][:dashboard][:panels][0][:targets][0][:expr] = 'sum(important_metric[$__interval_ms])'
-        end
-
-        it_behaves_like 'processing error'
+      it 'raises a processing error' do
+        expect { dashboard }.to raise_error(::Gitlab::Metrics::Dashboard::Errors::DashboardProcessingError)
       end
     end
   end

spec/lib/grafana/validator_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Grafana::Validator do
+  let(:grafana_dashboard) { JSON.parse(fixture_file('grafana/simplified_dashboard_response.json'), symbolize_names: true) }
+  let(:datasource) { JSON.parse(fixture_file('grafana/datasource_response.json'), symbolize_names: true) }
+  let(:panel) { grafana_dashboard[:dashboard][:panels].first }
+
+  let(:query_params) do
+    {
+      from: '1570397739557',
+      to: '1570484139557',
+      panelId: '8',
+      'var-instance': 'localhost:9121'
+    }
+  end
+
+  describe 'validate!' do
+    shared_examples_for 'processing error' do |message|
+      it 'raises a processing error' do
+        expect { subject }
+          .to raise_error(::Grafana::Validator::Error, message)
+      end
+    end
+
+    subject { described_class.new(grafana_dashboard, datasource, panel, query_params).validate! }
+
+    it 'does not raise an error' do
+      expect { subject }.not_to raise_error
+    end
+
+    context 'when query param "from" is not specified' do
+      before do
+        query_params.delete(:from)
+      end
+
+      it_behaves_like 'processing error', 'Grafana query parameters must include from and to.'
+    end
+
+    context 'when query param "to" is not specified' do
+      before do
+        query_params.delete(:to)
+      end
+
+      it_behaves_like 'processing error', 'Grafana query parameters must include from and to.'
+    end
+
+    context 'when the panel is not provided' do
+      let(:panel) { nil }
+
+      it_behaves_like 'processing error', 'Panel type must be a line graph.'
+    end
+
+    context 'when the panel is not a graph' do
+      before do
+        panel[:type] = 'singlestat'
+      end
+
+      it_behaves_like 'processing error', 'Panel type must be a line graph.'
+    end
+
+    context 'when the panel is not a line graph' do
+      before do
+        panel[:lines] = false
+      end
+
+      it_behaves_like 'processing error', 'Panel type must be a line graph.'
+    end
+
+    context 'when the query dashboard includes undefined variables' do
+      before do
+        query_params.delete(:'var-instance')
+      end
+
+      it_behaves_like 'processing error', 'All Grafana variables must be defined in the query parameters.'
+    end
+
+    context 'when the expression contains unsupported global variables' do
+      before do
+        grafana_dashboard[:dashboard][:panels][0][:targets][0][:expr] = 'sum(important_metric[$__interval_ms])'
+      end
+
+      it_behaves_like 'processing error', "Prometheus must not include #{described_class::UNSUPPORTED_GRAFANA_GLOBAL_VARS}"
+    end
+
+    context 'when the datasource is not proxyable' do
+      before do
+        datasource[:access] = 'not-proxy'
+      end
+
+      it_behaves_like 'processing error', 'Only Prometheus datasources with proxy access in Grafana are supported.'
+    end
+
+    # Skipping datasource validation allows for checks to be
+    # run without a secondary call to Grafana API
+    context 'when the datasource is not provided' do
+      let(:datasource) { nil }
+
+      it 'does not raise an error' do
+        expect { subject }.not_to raise_error
+      end
+    end
+  end
+
+  describe 'valid?' do
+    subject { described_class.new(grafana_dashboard, datasource, panel, query_params).valid? }
+
+    context 'with valid arguments' do
+      it { is_expected.to be true }
+    end
+
+    context 'with invalid arguments' do
+      let(:query_params) { {} }
+
+      it { is_expected.to be false }
+    end
+  end
+end

spec/models/service_spec.rb

@@ -18,6 +18,20 @@ describe Service do
       expect(build(:service, project_id: nil, template: false)).to be_invalid
     end
+    it 'validates presence of project_id if not instance', :aggregate_failures do
+      expect(build(:service, project_id: nil, instance: true)).to be_valid
+      expect(build(:service, project_id: nil, instance: false)).to be_invalid
+    end
+
+    it 'validates absence of project_id if instance', :aggregate_failures do
+      expect(build(:service, project_id: nil, instance: true)).to be_valid
+      expect(build(:service, instance: true)).to be_invalid
+    end
+
+    it 'validates service is template or instance' do
+      expect(build(:service, project_id: nil, template: true, instance: true)).to be_invalid
+    end
+
     context 'with an existing service template' do
       before do
         create(:service, type: 'Service', template: true)
@@ -27,6 +41,16 @@ describe Service do
         expect(build(:service, type: 'Service', template: true)).to be_invalid
       end
     end
+
+    context 'with an existing instance service' do
+      before do
+        create(:service, :instance)
+      end
+
+      it 'validates only one service instance per type' do
+        expect(build(:service, :instance)).to be_invalid
+      end
+    end
   end
   describe 'Scopes' do

spec/services/boards/issues/list_service_spec.rb

@@ -33,11 +33,11 @@ describe Boards::Issues::ListService do
       let!(:list1_issue3) { create(:labeled_issue, project: project, milestone: m1, labels: [development, p1]) }
       let!(:list2_issue1) { create(:labeled_issue, project: project, milestone: m1, labels: [testing]) }
-      let!(:closed_issue1) { create(:labeled_issue, :closed, project: project, labels: [bug]) }
-      let!(:closed_issue2) { create(:labeled_issue, :closed, project: project, labels: [p3]) }
-      let!(:closed_issue3) { create(:issue, :closed, project: project) }
-      let!(:closed_issue4) { create(:labeled_issue, :closed, project: project, labels: [p1]) }
-      let!(:closed_issue5) { create(:labeled_issue, :closed, project: project, labels: [development]) }
+      let!(:closed_issue1) { create(:labeled_issue, :closed, project: project, labels: [bug], closed_at: 1.day.ago) }
+      let!(:closed_issue2) { create(:labeled_issue, :closed, project: project, labels: [p3], closed_at: 2.days.ago) }
+      let!(:closed_issue3) { create(:issue, :closed, project: project, closed_at: 1.week.ago) }
+      let!(:closed_issue4) { create(:labeled_issue, :closed, project: project, labels: [p1], closed_at: 1.year.ago) }
+      let!(:closed_issue5) { create(:labeled_issue, :closed, project: project, labels: [development], closed_at: 2.years.ago) }
       let(:parent) { project }
@@ -94,11 +94,11 @@ describe Boards::Issues::ListService do
       let!(:list1_issue3) { create(:labeled_issue, project: project1, milestone: m1, labels: [development, p1, p1_project1]) }
       let!(:list2_issue1) { create(:labeled_issue, project: project1, milestone: m1, labels: [testing]) }
-      let!(:closed_issue1) { create(:labeled_issue, :closed, project: project, labels: [bug]) }
-      let!(:closed_issue2) { create(:labeled_issue, :closed, project: project, labels: [p3, p3_project]) }
-      let!(:closed_issue3) { create(:issue, :closed, project: project1) }
-      let!(:closed_issue4) { create(:labeled_issue, :closed, project: project1, labels: [p1, p1_project1]) }
-      let!(:closed_issue5) { create(:labeled_issue, :closed, project: project1, labels: [development]) }
+      let!(:closed_issue1) { create(:labeled_issue, :closed, project: project, labels: [bug], closed_at: 1.day.ago) }
+      let!(:closed_issue2) { create(:labeled_issue, :closed, project: project, labels: [p3, p3_project], closed_at: 2.days.ago) }
+      let!(:closed_issue3) { create(:issue, :closed, project: project1, closed_at: 1.week.ago) }
+      let!(:closed_issue4) { create(:labeled_issue, :closed, project: project1, labels: [p1, p1_project1], closed_at: 1.year.ago) }
+      let!(:closed_issue5) { create(:labeled_issue, :closed, project: project1, labels: [development], closed_at: 2.years.ago) }
       before do
         group.add_developer(user)

spec/support/shared_examples/services/boards/issues_list_service_shared_examples.rb

@@ -36,20 +36,22 @@ RSpec.shared_examples 'issues list service' do
       expect(issues).to eq [opened_issue2, reopened_issue1, opened_issue1]
     end
-    it 'returns closed issues when listing issues from Closed' do
-      params = { board_id: board.id, id: closed.id }
+    it 'returns opened issues that have label list applied when listing issues from a label list' do
+      params = { board_id: board.id, id: list1.id }
       issues = described_class.new(parent, user, params).execute
-      expect(issues).to eq [closed_issue4, closed_issue2, closed_issue5, closed_issue3, closed_issue1]
+      expect(issues).to eq [list1_issue3, list1_issue1, list1_issue2]
     end
-    it 'returns opened issues that have label list applied when listing issues from a label list' do
-      params = { board_id: board.id, id: list1.id }
+  end
+  context 'issues are ordered by date of closing' do
+    it 'returns closed issues when listing issues from Closed' do
+      params = { board_id: board.id, id: closed.id }
       issues = described_class.new(parent, user, params).execute
-      expect(issues).to eq [list1_issue3, list1_issue1, list1_issue2]
+      expect(issues).to eq [closed_issue1, closed_issue2, closed_issue3, closed_issue4, closed_issue5]
     end
   end

spec/support/shared_examples/services/metrics/dashboard_shared_examples.rb

@@ -67,6 +67,7 @@ RSpec.shared_examples 'valid dashboard cloning process' do |dashboard_template,
     it 'delegates commit creation to Files::CreateService', :aggregate_failures do
       service_instance = instance_double(::Files::CreateService)
+      allow(::Gitlab::Metrics::Dashboard::Processor).to receive(:new).and_return(double(process: file_content_hash))
       expect(::Files::CreateService).to receive(:new).with(project, user, dashboard_attrs).and_return(service_instance)
       expect(service_instance).to receive(:execute).and_return(status: :success)