Add latest changes from gitlab-org/gitlab@master

app/assets/javascripts/clusters/components/ingress_modsecurity_settings.vue

@@ -93,7 +93,10 @@ export default {
       return [UPDATING].includes(this.ingress.status);
     },
     saveButtonDisabled() {
-      return [UNINSTALLING, UPDATING, INSTALLING].includes(this.ingress.status);
+      return (
+        [UNINSTALLING, UPDATING, INSTALLING].includes(this.ingress.status) ||
+        this.ingress.updateAvailable
+      );
     },
     saveButtonLabel() {
       return this.saving ? __('Saving') : __('Save changes');
@@ -105,13 +108,14 @@ export default {
      *     neither getting installed nor updated.
      */
     showButtons() {
-      return (
-        this.saving || (this.hasValueChanged && [INSTALLED, UPDATED].includes(this.ingress.status))
-      );
+      return this.saving || this.valuesChangedByUser;
     },
     modSecurityModeName() {
       return this.modes[this.ingress.modsecurity_mode].name;
     },
+    valuesChangedByUser() {
+      return this.hasValueChanged && [INSTALLED, UPDATED].includes(this.ingress.status);
+    },
   },
   methods: {
     updateApplication() {

app/assets/javascripts/clusters/stores/clusters_store.js

@@ -59,6 +59,7 @@ export default class ClusterStore {
           isEditingModSecurityEnabled: false,
           isEditingModSecurityMode: false,
           updateFailed: false,
+          updateAvailable: false,
         },
         cert_manager: {
           ...applicationInitialState,
@@ -213,6 +214,7 @@ export default class ClusterStore {
       if (appId === INGRESS) {
         this.state.applications.ingress.externalIp = serverAppEntry.external_ip;
         this.state.applications.ingress.externalHostname = serverAppEntry.external_hostname;
+        this.state.applications.ingress.updateAvailable = updateAvailable;
         if (!this.state.applications.ingress.isEditingModSecurityEnabled) {
           this.state.applications.ingress.modsecurity_enabled = serverAppEntry.modsecurity_enabled;
         }

app/controllers/projects/merge_requests/diffs_controller.rb

@@ -98,7 +98,7 @@ class Projects::MergeRequests::DiffsController < Projects::MergeRequests::Applic
         @merge_request.merge_request_diff
       end
 
-    return unless @merge_request_diff
+    return unless @merge_request_diff&.id
 
     @comparable_diffs = @merge_request_diffs.select { |diff| diff.id < @merge_request_diff.id }
 

app/controllers/projects/prometheus/alerts_controller.rb

+# frozen_string_literal: true
+
+module Projects
+  module Prometheus
+    class AlertsController < Projects::ApplicationController
+      include MetricsDashboard
+
+      respond_to :json
+
+      protect_from_forgery except: [:notify]
+
+      skip_before_action :project, only: [:notify]
+
+      prepend_before_action :repository, :project_without_auth, only: [:notify]
+
+      before_action :authorize_read_prometheus_alerts!, except: [:notify]
+      before_action :alert, only: [:update, :show, :destroy, :metrics_dashboard]
+
+      def index
+        render json: serialize_as_json(alerts)
+      end
+
+      def show
+        render json: serialize_as_json(alert)
+      end
+
+      def notify
+        token = extract_alert_manager_token(request)
+
+        if notify_service.execute(token)
+          head :ok
+        else
+          head :unprocessable_entity
+        end
+      end
+
+      def create
+        @alert = create_service.execute
+
+        if @alert.persisted?
+          schedule_prometheus_update!
+
+          render json: serialize_as_json(@alert)
+        else
+          head :no_content
+        end
+      end
+
+      def update
+        if update_service.execute(alert)
+          schedule_prometheus_update!
+
+          render json: serialize_as_json(alert)
+        else
+          head :no_content
+        end
+      end
+
+      def destroy
+        if destroy_service.execute(alert)
+          schedule_prometheus_update!
+
+          head :ok
+        else
+          head :no_content
+        end
+      end
+
+      private
+
+      def alerts_params
+        params.permit(:operator, :threshold, :environment_id, :prometheus_metric_id)
+      end
+
+      def notify_service
+        Projects::Prometheus::Alerts::NotifyService
+          .new(project, current_user, params.permit!)
+      end
+
+      def create_service
+        Projects::Prometheus::Alerts::CreateService
+          .new(project, current_user, alerts_params)
+      end
+
+      def update_service
+        Projects::Prometheus::Alerts::UpdateService
+          .new(project, current_user, alerts_params)
+      end
+
+      def destroy_service
+        Projects::Prometheus::Alerts::DestroyService
+          .new(project, current_user, nil)
+      end
+
+      def schedule_prometheus_update!
+        ::Clusters::Applications::ScheduleUpdateService.new(application, project).execute
+      end
+
+      def serialize_as_json(alert_obj)
+        serializer.represent(alert_obj)
+      end
+
+      def serializer
+        PrometheusAlertSerializer
+          .new(project: project, current_user: current_user)
+      end
+
+      def alerts
+        alerts_finder.execute
+      end
+
+      def alert
+        @alert ||= alerts_finder(metric: params[:id]).execute.first || render_404
+      end
+
+      def alerts_finder(opts = {})
+        Projects::Prometheus::AlertsFinder.new({
+          project: project,
+          environment: params[:environment_id]
+        }.reverse_merge(opts))
+      end
+
+      def application
+        @application ||= alert.environment.cluster_prometheus_adapter
+      end
+
+      def extract_alert_manager_token(request)
+        Doorkeeper::OAuth::Token.from_bearer_authorization(request)
+      end
+
+      def project_without_auth
+        @project ||= Project
+          .find_by_full_path("#{params[:namespace_id]}/#{params[:project_id]}")
+      end
+
+      def prometheus_alerts
+        project.prometheus_alerts.for_environment(params[:environment_id])
+      end
+
+      def metrics_dashboard_params
+        {
+          embedded: true,
+          prometheus_alert_id: alert.id
+        }
+      end
+    end
+  end
+end

changelogs/unreleased/212398-harden-optimie-jira-usage-data.yml

+---
+title: Harden jira usage data
+merge_request: 27973
+author:
+type: performance

changelogs/unreleased/add_restriction_for_ingress_update.yml

+---
+title: WAF settings will be read-only if there is a new version of ingress available
+merge_request: 27845
+author:
+type: changed

db/migrate/20200213100530_add_verification_columns_to_packages.rb

+# frozen_string_literal: true
+
+class AddVerificationColumnsToPackages < ActiveRecord::Migration[6.0]
+  DOWNTIME = false
+
+  def change
+    add_column :packages_package_files, :verification_retry_at, :datetime_with_timezone
+    add_column :packages_package_files, :verified_at, :datetime_with_timezone
+    add_column :packages_package_files, :verification_checksum, :string, limit: 255
+    add_column :packages_package_files, :verification_failure, :string, limit: 255
+    add_column :packages_package_files, :verification_retry_count, :integer
+  end
+end

db/structure.sql

@@ -4364,6 +4364,11 @@ CREATE TABLE public.packages_package_files (
     file_sha1 bytea,
     file_name character varying NOT NULL,
     file text NOT NULL,
+    verification_retry_at timestamp with time zone,
+    verified_at timestamp with time zone,
+    verification_checksum character varying(255),
+    verification_failure character varying(255),
+    verification_retry_count integer,
     file_sha256 bytea
 );
 
@@ -12720,6 +12725,7 @@ COPY "schema_migrations" (version) FROM STDIN;
 20200212133945
 20200212134201
 20200213093702
+20200213100530
 20200213155311
 20200213204737
 20200213220159

doc/development/geo/framework.md

@@ -174,7 +174,7 @@ For example, to add support for files referenced by a `Widget` model with a
 
      def change
        add_column :widgets, :verification_retry_at,  :datetime_with_timezone
-       add_column :widgets, :last_verification_ran_at,  :datetime_with_timezone
+       add_column :widgets, :verified_at,  :datetime_with_timezone
        add_column :widgets, :verification_checksum, :string
        add_column :widgets, :verification_failure, :string
        add_column :widgets, :verification_retry_count, :integer

lib/gitlab/usage_data.rb

@@ -202,7 +202,7 @@ module Gitlab
         results = {
           projects_jira_server_active: 0,
           projects_jira_cloud_active: 0,
-          projects_jira_active: -1
+          projects_jira_active: 0
         }
 
         Service.active
@@ -217,14 +217,12 @@ module Gitlab
 
           results[:projects_jira_server_active] += counts[:server].count if counts[:server]
           results[:projects_jira_cloud_active] += counts[:cloud].count if counts[:cloud]
-          if results[:projects_jira_active] == -1
-            results[:projects_jira_active] = services.size
-          else
-            results[:projects_jira_active] += services.size
-          end
+          results[:projects_jira_active] += services.size
         end
 
         results
+      rescue ActiveRecord::StatementInvalid
+        { projects_jira_server_active: -1, projects_jira_cloud_active: -1, projects_jira_active: -1 }
       end
       # rubocop: enable CodeReuse/ActiveRecord
 

spec/controllers/projects/merge_requests/diffs_controller_spec.rb

@@ -16,6 +16,18 @@ describe Projects::MergeRequests::DiffsController do
         expect(response).to have_gitlab_http_status(:not_found)
       end
     end
+
+    context 'when the merge_request_diff.id is blank' do
+      it 'returns 404' do
+        allow_next_instance_of(MergeRequest) do |instance|
+          allow(instance).to receive(:merge_request_diff).and_return(MergeRequestDiff.new(merge_request_id: instance.id))
+
+          go
+
+          expect(response).to have_gitlab_http_status(:not_found)
+        end
+      end
+    end
   end
 
   shared_examples 'forked project with submodules' do

spec/controllers/projects/prometheus/alerts_controller_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Projects::Prometheus::AlertsController do
+  let_it_be(:user) { create(:user) }
+  let_it_be(:project) { create(:project) }
+  let_it_be(:environment) { create(:environment, project: project) }
+  let_it_be(:metric) { create(:prometheus_metric, project: project) }
+
+  before do
+    project.add_maintainer(user)
+    sign_in(user)
+  end
+
+  shared_examples 'unprivileged' do
+    before do
+      project.add_developer(user)
+    end
+
+    it 'returns not_found' do
+      make_request
+
+      expect(response).to have_gitlab_http_status(:not_found)
+    end
+  end
+
+  shared_examples 'project non-specific environment' do |status|
+    let(:other) { create(:environment) }
+
+    it "returns #{status}" do
+      make_request(environment_id: other)
+
+      expect(response).to have_gitlab_http_status(status)
+    end
+
+    if status == :ok
+      it 'returns no prometheus alerts' do
+        make_request(environment_id: other)
+
+        expect(json_response).to be_empty
+      end
+    end
+  end
+
+  shared_examples 'project non-specific metric' do |status|
+    let(:other) { create(:prometheus_alert) }
+
+    it "returns #{status}" do
+      make_request(id: other.prometheus_metric_id)
+
+      expect(response).to have_gitlab_http_status(status)
+    end
+  end
+
+  describe 'GET #index' do
+    def make_request(opts = {})
+      get :index, params: request_params(opts, environment_id: environment)
+    end
+
+    context 'when project has no prometheus alert' do
+      it 'returns an empty response' do
+        make_request
+
+        expect(response).to have_gitlab_http_status(:ok)
+        expect(json_response).to be_empty
+      end
+    end
+
+    context 'when project has prometheus alerts' do
+      let(:production) { create(:environment, project: project) }
+      let(:staging) { create(:environment, project: project) }
+      let(:json_alert_ids) { json_response.map { |alert| alert['id'] } }
+
+      let!(:production_alerts) do
+        create_list(:prometheus_alert, 2, project: project, environment: production)
+      end
+
+      let!(:staging_alerts) do
+        create_list(:prometheus_alert, 1, project: project, environment: staging)
+      end
+
+      it 'contains prometheus alerts only for the production environment' do
+        make_request(environment_id: production)
+
+        expect(response).to have_gitlab_http_status(:ok)
+        expect(json_response.count).to eq(2)
+        expect(json_alert_ids).to eq(production_alerts.map(&:id))
+      end
+
+      it 'contains prometheus alerts only for the staging environment' do
+        make_request(environment_id: staging)
+
+        expect(response).to have_gitlab_http_status(:ok)
+        expect(json_response.count).to eq(1)
+        expect(json_alert_ids).to eq(staging_alerts.map(&:id))
+      end
+
+      it 'does not return prometheus alerts without environment' do
+        make_request(environment_id: nil)
+
+        expect(response).to have_gitlab_http_status(:ok)
+        expect(json_response).to be_empty
+      end
+    end
+
+    it_behaves_like 'unprivileged'
+    it_behaves_like 'project non-specific environment', :ok
+  end
+
+  describe 'GET #show' do
+    let(:alert) do
+      create(:prometheus_alert,
+             project: project,
+             environment: environment,
+             prometheus_metric: metric)
+    end
+
+    def make_request(opts = {})
+      get :show, params: request_params(
+        opts,
+        id: alert.prometheus_metric_id,
+        environment_id: environment
+      )
+    end
+
+    context 'when alert does not exist' do
+      it 'returns not_found' do
+        make_request(id: 0)
+
+        expect(response).to have_gitlab_http_status(:not_found)
+      end
+    end
+
+    context 'when alert exists' do
+      let(:alert_params) do
+        {
+          'id' => alert.id,
+          'title' => alert.title,
+          'query' => alert.query,
+          'operator' => alert.computed_operator,
+          'threshold' => alert.threshold,
+          'alert_path' => alert_path(alert)
+        }
+      end
+
+      it 'renders the alert' do
+        make_request
+
+        expect(response).to have_gitlab_http_status(:ok)
+        expect(json_response).to include(alert_params)
+      end
+
+      it_behaves_like 'unprivileged'
+      it_behaves_like 'project non-specific environment', :not_found
+      it_behaves_like 'project non-specific metric', :not_found
+    end
+  end
+
+  describe 'POST #notify' do
+    let(:notify_service) { spy }
+
+    before do
+      sign_out(user)
+
+      expect(Projects::Prometheus::Alerts::NotifyService)
+        .to receive(:new)
+        .with(project, nil, duck_type(:permitted?))
+        .and_return(notify_service)
+    end
+
+    it 'returns ok if notification succeeds' do
+      expect(notify_service).to receive(:execute).and_return(true)
+
+      post :notify, params: project_params, session: { as: :json }
+
+      expect(response).to have_gitlab_http_status(:ok)
+    end
+
+    it 'returns unprocessable entity if notification fails' do
+      expect(notify_service).to receive(:execute).and_return(false)
+
+      post :notify, params: project_params, session: { as: :json }
+
+      expect(response).to have_gitlab_http_status(:unprocessable_entity)
+    end
+
+    context 'bearer token' do
+      context 'when set' do
+        it 'extracts bearer token' do
+          request.headers['HTTP_AUTHORIZATION'] = 'Bearer some token'
+
+          expect(notify_service).to receive(:execute).with('some token')
+
+          post :notify, params: project_params, as: :json
+        end
+
+        it 'pass nil if cannot extract a non-bearer token' do
+          request.headers['HTTP_AUTHORIZATION'] = 'some token'
+
+          expect(notify_service).to receive(:execute).with(nil)
+
+          post :notify, params: project_params, as: :json
+        end
+      end
+
+      context 'when missing' do
+        it 'passes nil' do
+          expect(notify_service).to receive(:execute).with(nil)
+
+          post :notify, params: project_params, as: :json
+        end
+      end
+    end
+  end
+
+  describe 'POST #create' do
+    let(:schedule_update_service) { spy }
+
+    let(:alert_params) do
+      {
+        'title' => metric.title,
+        'query' => metric.query,
+        'operator' => '>',
+        'threshold' => 1.0
+      }
+    end
+
+    def make_request(opts = {})
+      post :create, params: request_params(
+        opts,
+        operator: '>',
+        threshold: '1',
+        environment_id: environment,
+        prometheus_metric_id: metric
+      )
+    end
+
+    it 'creates a new prometheus alert' do
+      allow(::Clusters::Applications::ScheduleUpdateService)
+        .to receive(:new).and_return(schedule_update_service)
+
+      make_request
+
+      expect(schedule_update_service).to have_received(:execute)
+      expect(response).to have_gitlab_http_status(:ok)
+      expect(json_response).to include(alert_params)
+    end
+
+    it 'returns no_content for an invalid metric' do
+      make_request(prometheus_metric_id: 'invalid')
+
+      expect(response).to have_gitlab_http_status(:no_content)
+    end
+
+    it_behaves_like 'unprivileged'
+    it_behaves_like 'project non-specific environment', :no_content
+  end
+
+  describe 'PUT #update' do
+    let(:schedule_update_service) { spy }
+
+    let(:alert) do
+      create(:prometheus_alert,
+             project: project,
+             environment: environment,
+             prometheus_metric: metric)
+    end
+
+    let(:alert_params) do
+      {
+        'id' => alert.id,
+        'title' => alert.title,
+        'query' => alert.query,
+        'operator' => '<',
+        'threshold' => alert.threshold,
+        'alert_path' => alert_path(alert)
+      }
+    end
+
+    before do
+      allow(::Clusters::Applications::ScheduleUpdateService)
+        .to receive(:new).and_return(schedule_update_service)
+    end
+
+    def make_request(opts = {})
+      put :update, params: request_params(
+        opts,
+        id: alert.prometheus_metric_id,
+        operator: '<',
+        environment_id: alert.environment
+      )
+    end
+
+    it 'updates an already existing prometheus alert' do
+      expect { make_request(operator: '<') }
+        .to change { alert.reload.operator }.to('lt')
+
+      expect(schedule_update_service).to have_received(:execute)
+      expect(response).to have_gitlab_http_status(:ok)
+      expect(json_response).to include(alert_params)
+    end
+
+    it_behaves_like 'unprivileged'
+    it_behaves_like 'project non-specific environment', :not_found
+    it_behaves_like 'project non-specific metric', :not_found
+  end
+
+  describe 'DELETE #destroy' do
+    let(:schedule_update_service) { spy }
+
+    let!(:alert) do
+      create(:prometheus_alert, project: project, prometheus_metric: metric)
+    end
+
+    before do
+      allow(::Clusters::Applications::ScheduleUpdateService)
+        .to receive(:new).and_return(schedule_update_service)
+    end
+
+    def make_request(opts = {})
+      delete :destroy, params: request_params(
+        opts,
+        id: alert.prometheus_metric_id,
+        environment_id: alert.environment
+      )
+    end
+
+    it 'destroys the specified prometheus alert' do
+      expect { make_request }.to change { PrometheusAlert.count }.by(-1)
+
+      expect(schedule_update_service).to have_received(:execute)
+    end
+
+    it_behaves_like 'unprivileged'
+    it_behaves_like 'project non-specific environment', :not_found
+    it_behaves_like 'project non-specific metric', :not_found
+  end
+
+  describe 'GET #metrics_dashboard' do
+    let!(:alert) do
+      create(:prometheus_alert,
+             project: project,
+             environment: environment,
+             prometheus_metric: metric)
+    end
+
+    it 'returns a json object with the correct keys' do
+      get :metrics_dashboard, params: request_params(id: metric.id, environment_id: alert.environment.id), format: :json
+
+      expect(response).to have_gitlab_http_status(:ok)
+      expect(json_response.keys).to contain_exactly('dashboard', 'status')
+    end
+
+    it 'is the correct embed' do
+      get :metrics_dashboard, params: request_params(id: metric.id, environment_id: alert.environment.id), format: :json
+
+      title = json_response['dashboard']['panel_groups'][0]['panels'][0]['title']
+
+      expect(title).to eq(metric.title)
+    end
+
+    it 'finds the first alert embed without environment_id' do
+      get :metrics_dashboard, params: request_params(id: metric.id), format: :json
+
+      title = json_response['dashboard']['panel_groups'][0]['panels'][0]['title']
+
+      expect(title).to eq(metric.title)
+    end
+
+    it 'returns 404 for non-existant alerts' do
+      get :metrics_dashboard, params: request_params(id: 0), format: :json
+
+      expect(response).to have_gitlab_http_status(:not_found)
+    end
+  end
+
+  def project_params(opts = {})
+    opts.reverse_merge(namespace_id: project.namespace, project_id: project)
+  end
+
+  def request_params(opts = {}, defaults = {})
+    project_params(opts.reverse_merge(defaults))
+  end
+
+  def alert_path(alert)
+    project_prometheus_alert_path(
+      project,
+      alert.prometheus_metric_id,
+      environment_id: alert.environment,
+      format: :json
+    )
+  end
+end

spec/frontend/clusters/components/ingress_modsecurity_settings_spec.js

@@ -14,6 +14,7 @@ describe('IngressModsecuritySettings', () => {
     status: 'installable',
     installed: false,
     modsecurity_mode: 'logging',
+    updateAvailable: false,
   };
 
   const createComponent = (props = defaultProps) => {
@@ -61,6 +62,11 @@ describe('IngressModsecuritySettings', () => {
         expect(findCancelButton().exists()).toBe(true);
       });
 
+      it('enables related toggle and buttons', () => {
+        expect(findSaveButton().attributes().disabled).toBeUndefined();
+        expect(findCancelButton().attributes().disabled).toBeUndefined();
+      });
+
       describe('with dropdown changed by the user', () => {
         beforeEach(() => {
           findModSecurityDropdown().vm.$children[1].$emit('click');
@@ -105,6 +111,25 @@ describe('IngressModsecuritySettings', () => {
           expect(findCancelButton().exists()).toBe(false);
         });
       });
+
+      describe('with a new version available', () => {
+        beforeEach(() => {
+          wrapper.setProps({
+            ingress: {
+              ...defaultProps,
+              installed: true,
+              status: 'installed',
+              modsecurity_enabled: true,
+              updateAvailable: true,
+            },
+          });
+        });
+
+        it('disables related toggle and buttons', () => {
+          expect(findSaveButton().attributes().disabled).toBe('true');
+          expect(findCancelButton().attributes().disabled).toBe('true');
+        });
+      });
     });
 
     it('triggers set event to be propagated with the current modsecurity value', () => {

spec/lib/gitlab/usage_data_spec.rb

@@ -85,6 +85,13 @@ describe Gitlab::UsageData, :aggregate_failures do
 
         expect { subject }.not_to raise_error
       end
+
+      it 'jira usage works when queries time out' do
+        allow_any_instance_of(ActiveRecord::Relation)
+          .to receive(:find_in_batches).and_raise(ActiveRecord::StatementInvalid.new(''))
+
+        expect { described_class.jira_usage }.not_to raise_error
+      end
     end
 
     describe '#usage_data_counters' do

spec/support/shared_examples/models/concerns/blob_replicator_strategy_shared_examples.rb

@@ -27,6 +27,45 @@ RSpec.shared_examples 'a blob replicator' do
       expect(::Geo::Event.last.attributes).to include(
         "replicable_name" => replicator.replicable_name, "event_name" => "created", "payload" => { "model_record_id" => replicator.model_record.id })
     end
+
+    it 'schedules the checksum calculation if needed' do
+      expect(Geo::BlobVerificationPrimaryWorker).to receive(:perform_async)
+      expect(replicator).to receive(:needs_checksum?).and_return(true)
+
+      replicator.handle_after_create_commit
+    end
+
+    it 'does not schedule the checksum calculation if feature flag is disabled' do
+      stub_feature_flags(geo_self_service_framework: false)
+
+      expect(Geo::BlobVerificationPrimaryWorker).not_to receive(:perform_async)
+      allow(replicator).to receive(:needs_checksum?).and_return(true)
+
+      replicator.handle_after_create_commit
+    end
+  end
+
+  describe '#calculate_checksum!' do
+    it 'calculates the checksum' do
+      model_record.save!
+
+      replicator.calculate_checksum!
+
+      expect(model_record.reload.verification_checksum).not_to be_nil
+    end
+
+    it 'saves the error message and increments retry counter' do
+      model_record.save!
+
+      allow(model_record).to receive(:calculate_checksum!) do
+        raise StandardError.new('Failure to calculate checksum')
+      end
+
+      replicator.calculate_checksum!
+
+      expect(model_record.reload.verification_failure).to eq 'Failure to calculate checksum'
+      expect(model_record.verification_retry_count).to be 1
+    end
   end
 
   describe '#consume_created_event' do